Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move layout files to workflow settings folder #1956

Merged
merged 9 commits into from
Feb 19, 2025

Conversation

glopesdev
Copy link
Member

@glopesdev glopesdev commented Aug 6, 2024

As detailed in #1955 this PR moves the .layout files to the new workflow settings folder which aims to declutter project working directories and is designed to be conveniently ignored by version control systems.

New example folder structure for layout files:

📦ProjectFolder
 ┣ 📂.bonsai
 ┃ ┗ 📂Settings
 ┃   ┣ 📜Workflow1.layout
 ┃   ┣ 📜Workflow2.layout
 ┃   ┗ 📂SubFolder
 ┃     ┗ 📜Workflow3.layout
 ┣ 📜Workflow1.bonsai
 ┣ 📜Workflow2.bonsai
 ┗ 📂SubFolder
   ┗ 📜Workflow3.bonsai

As suggested by @PathogenDavid, the subfolder hierarchy from the base path where .bonsai is rooted will be preserved inside the Settings folder to avoid name clashes with duplicate file names.

Further work in #1870 will allow building on this new organization to save other editor state, and possibly address some of the drawbacks outlined in #1955.

The PR also moves scanning of extension workflow files into the new Project helper class to begin consolidating project path manipulation operators in advance of refactoring towards a cross-platform visual editor.

Fixes #1955

@glopesdev glopesdev added the feature New planned feature label Aug 6, 2024
@glopesdev glopesdev added this to the 2.9 milestone Aug 6, 2024
@glopesdev glopesdev force-pushed the issue-1955 branch 2 times, most recently from c992a45 to 3a0218a Compare August 6, 2024 10:58
@glopesdev

This comment was marked as resolved.

@PathogenDavid

This comment was marked as resolved.

@PathogenDavid
Copy link
Member

During our meeting today we decided to go with the central .bonsai approach I demonstrated here.

Bonsai will prefer any .bonsai folder found in the directory tree up from the current workflow, and if none exists it will create one in the same folder as the current workflow.


Another thing I wanted to mention but didn't want to continue derailing taskboard: Since we want to discourage people from checking .layout files into source control, I think we should also generate a self-ignoring .bonsai/Settings/.gitignore file that simply contains * (which ignores all files and directories under Settings, including the .gitignore itself.)

We should create this .gitignore only if the file doesn't already exist. Anyone who wants to ignore the recommendation and commit them anyway can just replace it with an empty file.

@glopesdev
Copy link
Member Author

glopesdev commented Feb 15, 2025

Another thing I wanted to mention but didn't want to continue derailing taskboard: Since we want to discourage people from checking .layout files into source control, I think we should also generate a self-ignoring .bonsai/Settings/.gitignore file that simply contains * (which ignores all files and directories under Settings, including the .gitignore itself.)

@PathogenDavid having this possibility makes sense, but I feel weird baking into the language IDE an assumption about a specific version control system, especially given that Git is not assumed anywhere else in the language / IDE.

We actually even started out the project in Mercurial, and we already had to move everything from hg to git. I know it is hard to imagine right now Git ever losing its dominant position, but still there are other DVCS out there such as Perforce, and I wouldn't want to introduce weird assumptions into possibly valid workflows (we did consider using Perforce ourselves for large asset projects before, so definitely not outside the realm of possibility).

That said, what about we bake this idea into a new project repository template instead?

We did discuss before about version-controlled Bonsai project best practices which would be nice to capture in a template anyway. Because these practices also include bootstrapping an environment from a .bonsai folder, it feels like this structure would already capture whichever workflows are created downstream from the root repository folder. If we just add a .gitignore to that template maybe we could sidestep the need to bake this into the IDE itself?

@glopesdev
Copy link
Member Author

glopesdev commented Feb 15, 2025

Working further on this implementation, I am growing increasingly worried about cache invalidation problems where the .layout files may inadvertently become out-of-sync with the main workflow in confusing ways:

  1. If the workflow file is renamed from the file explorer, the layout loses sync but remains "lost" in the cache.
  2. If some other workflow file is later renamed to match the previously "lost" file, a potentially entirely unrelated layout file may be loaded and lead to arbitrary errors / code execution.
  3. This is a problem even if file names never change. For example in a version-controlled project with a local layout cache in .gitignore, simply changing to a different revision may break the association between layout and workflow file, and lead to confusion.
  4. These issues remain whether we allow searching for shared settings up the path hierarchy or not (the only difference is the probability of conflict / confusion likely increases the further you move up the hierarchy).

I am not sure how other IDEs such as VS or VS Code deal with such cache problems and VCS for example. I thought of a few possibilities but am not particularly happy with any of them. Most of them are vulnerable to 3. and therefore of reduced utility.

Garbage collection

One option is to run "garbage collection" on the cache every time the IDE loads or a project is opened. Any file settings not matched to an existing workflow file would be deleted.

This wouldn't necessarily resolve any of the above issues, but might prevent the probability of confusion by limiting cache file growth. What I find confusing here is you might be inadvertently deleting someone else's layout by simply opening a file with the repo in a different state.

Renaming workflows through the IDE

Introduce some procedure for renaming workflow files explicitly in the IDE. The renaming procedure would itself take care of moving all workflow cache files. This might improve regular experience when working inside the IDE, but would still not resolve issues when renaming files in the explorer or when changing revisions in the git repo.

Store metadata about the workflow file

A potentially more interesting option is to save a "hash" or "timestamp"" of the workflow file together with the cache files. This would allow us to quickly detect any silent link breakage between workflow file contents and the cache files. Any inconsistent settings would be ignored / deleted.

The main outstanding issue is that sometimes we may want to change a workflow file deliberately and still use the layout cache files (e.g. we may know they would work or want them to be used anyway to speed up development). Maybe there could be an option in the editor where it would be possible to "force" the editor to load cache files regardless of matching hash contents.

@glopesdev
Copy link
Member Author

Another possibility is to allow inconsistent cache files to be present, but adopt better standards for robustness against partial and incomplete layout files, as proposed in #2144.

This final option would also mean we wouldn't have to worry about merging this PR as-is and in general might be more future-proof.

@glopesdev
Copy link
Member Author

glopesdev commented Feb 17, 2025

Investigate following edge cases:

  • Access denied or other IO exceptions when recursing up, break if any exception happens
  • Check permissions of the parent folder, could consider breaking search if permissions change

Other options for scanning parent directories:

  • Only accept .bonsai folders which contain a bootstrapper
  • Only accept the .bonsai folder that contains the editor/bootstrapper which is currently running
    • This could create confusion if we open the workflow file with a bootstrapper located in another location, since it would then skip the centralized settings folder and create a more local one, which would then override the original if/when going back to the original environment.

@glopesdev
Copy link
Member Author

In a2fcf3e we revised the search strategy to use a centralized settings folder if and only if the workflow file is sitting in a path relative to the parent of the boostrapper folder (i.e. the folder where the executable is launched from).

This is less ambiguous than traversing the full path up to the root, since there are only two possible paths for workflow settings:

  1. the path where the executable is running from (if rooted in a shared path);
  2. the .bonsai folder in the local workflow file if no common path is found.

This strategy avoids the above issues with permissions, since it assumes the running executable must be able to write into its own folder. If this is not allowed, other much more serious catastrophic errors will occur either before or after opening the workflow file, since the editor requires read-write access to its own folder during normal operation.

Finally, this strategy also allows more readily for different versions of bonsai to open workflows in remote paths without disrupting their cached development environment, since at most the editor will cache the settings in its own folder, and never in the settings folder of a different environment.

@PathogenDavid
Copy link
Member

Git awareness

but I feel weird baking into the language IDE an assumption about a specific version control system, especially given that Git is not assumed anywhere else in the language / IDE.

I definitely don't love it either, but I'm mainly thinking about creating a pit of success for our users since Git tends to be what we push people towards. The best best practices are defaults. (And for anyone not using Git the .gitignore would be a sign for them to create their own equivalent.)

I don't feel strongly about this though, just wanted to throw the idea out there.

Stale layout file problems

I am not sure how other IDEs such as VS or VS Code deal with such cache problems and VCS for example.

In my experience they best effort ignore the invalid data. Sometimes things barf, but not usually. (C++ IntelliSense still sometimes needs a manual brain refresh.)

Garbage collection

Not a fan of this solution. Similar to what you alluded to, this would be annoying if you were switching between revisions and like you said only hides the problem.

As for useless layout files sitting around...they're honestly not big enough to worry about IMO.

Renaming workflows through the IDE

So I don't think this is a good solution to this problem simply because people won't remember to use it (and I don't think they should have to. I've used VCSes that make you do this and it always annoyed me.)

However I do think that this could be a good idea to do as part of the new workflow explorer. (Probably not as part of #1870, but maybe as a feature for it later on.) There's already a node for the root workflow, so it makes sense that you could rename it to rename the file, and this could be nice for child nodes too.

Store metadata about the workflow file

Ideally we should really just have more robust layout file stuff, but yeah I think this is probably the best short-term solution. I was going to say we got away with not having this for a long time now, but I guess if layout files aren't being committed to Git that mismatch issues are much more likely.

Another idea if you just want a best-effort heuristic thing with minimal changes, you could say that if the workflow modified time is in the future relative to the layout file, then that layout file is invalid. This doesn't catch every scenario, but I think it catches all the typical ones.


Investigate following edge cases: [IO exceptions / write permissions]

We talked about this on Teams but for posterity: I think these edge cases are too weird to worry about and layout files being saved on a best-effort basis is fine.

(But I also think the final solution of using the Bonsai environment only when one's being used is also fine.)

This could create confusion if we open the workflow file with a bootstrapper located in another location, since it would then skip the centralized settings folder and create a more local one, which would then override the original if/when going back to the original environment.

Not a fan of this edge-case though, but I suppose this just adds more motivation towards improving the experience of double-clicking a .bonsai file when there's a local environment in the repo.

Case sensitivity in paths results from a combination of filesystem
and OS-level attributes and so it cannot be resolved with a single
condition on the OS.

We are leaving it hard-coded until a better solution is available
through the .NET runtime.
@glopesdev
Copy link
Member Author

But I also think the final solution of using the Bonsai environment only when one's being used is also fine.

@PathogenDavid sounds good, let's go with this for now and we can always extend it later if necessary.

Not a fan of this edge-case though, but I suppose this just adds more motivation towards improving the experience of double-clicking a .bonsai file when there's a local environment in the repo.

This is actually a pretty good idea, might be worth raising an issue for keeping track of it on its own. The current experience is pretty horrible, but maybe since the bootstrapper is already launching subprocesses anyway there could be a way where it just passes the ball to the right environment if one is detected.

@glopesdev glopesdev merged commit 6d5491e into bonsai-rx:main Feb 19, 2025
10 checks passed
@glopesdev glopesdev deleted the issue-1955 branch February 19, 2025 14:28
@PathogenDavid
Copy link
Member

This is actually a pretty good idea, might be worth raising an issue for keeping track of it on its own.

Done! #2148

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New planned feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reorganize per-workflow settings file structure
2 participants