Clean root/env directory before saving config

[bmtcril]

Bug description
Because the env dir is not cleaned when running tutor config save, any changes which delete previously rendered files can leave orphaned files in the env and result in undefined / unpredictable behavior. The bigger the upgrade, the more likely that orphaned files will exist and the more unpredictable the behavior is likely to be.

I feel like we've discussed this before and it was counted as desired behavior, but I can't find the discussion or remember why it would be desirable.

How to reproduce
Example: when squashing migration files for Aspects between v0.65.1 and v0.66.1 we deleted several Alembic migrations. Upon upgrade, each operator needs to delete their env directory (or at least certain subdirectories) and re-run tutor config save otherwise Alembic will find the old migration files and throw an error.

1. Install the tutor-contrib-aspects==v0.65.1 plugin and enable it
2. tutor config save
3. List the files in {tutor root}/env/plugins/aspects/apps/aspects/migrations/alembic/versions/
4. Upgrade to tutor-contrib-aspects==v0.66.1
5. tutor config save
6. List the files in {tutor root}/env/plugins/aspects/apps/aspects/migrations/alembic/versions/ and note that several files which have been deleted from the project still exist, additionally there maybe compiled python files and __pycache__ etc created there which may be out of date. Running a tutor init in this state will cause errors.
7. Delete {tutor root}/env/plugins/aspects/apps/aspects/migrations/alembic/versions/ (or just the entire env directory)
8. tutor config save
9. List the files in {tutor root}/env/plugins/aspects/apps/aspects/migrations/alembic/versions/ and note that there are now the correct number of them.

Environment
At least Mac OS and Ubuntu 22.04, Tutor 15/16/17 but probably impacts all environments and Tutor versions.

Additional context
This has come up as a consistent pain point with front-end development as well, @brian-smith-tcril may have more context about those issues.

[regisb]

Hi Brian! I understand that extra files lying around in the tutor environment may be causing errors. Yet, we cannot delete the environment on every config save, because users may have manually customised their environment in unexpected ways. For instance, Tutor supports custom translation strings (though we expect that these will go away in Redwood). There may be other edge cases that I'm not aware of, so I'm not too keen on pushing a potentially important breaking change.

I see two possible solutions:

1. Add instructions to the Aspects README to rm -rf the migrations folder prior to config save.
2. Make that process automatic within the Aspects plugin. For instance, you could use the PROJECT_ROOT_READY action. Alternatively, we could create CONFIG_SAVE and ENV_SAVE actions.

Would any of these solutions work for you?

[bmtcril]

Thanks for the response! I didn't think that was a supported use case, but I can see why it may be necessary for some advanced customization.

Would you consider adding a tutor config save --clean option that implements this behavior? It seems like needing a fresh environment is a fairly frequent use case, and I wouldn't think it should be a thing that plugin authors should need to (or expect to) manage any time they want to delete or move files between versions.

Barring that, I think an action could work as long as they're explicitly pre-hooks called before the config is written.

[brian-smith-tcril]

Would you consider adding a tutor config save --clean option that implements this behavior? It seems like needing a fresh environment is a fairly frequent use case

This would be wonderful. I often want to test things from a "clean slate," so having a user friendly way to "clean the slate" from within tutor would be amazing.

[kdmccormick]

In favor of tutor config save --clean:

• It would help plugin developers, like both Brians here.
• There's a nice git parallel: git will not remove untracked files, unless you run git clean -f.
• It's backwards-compatible.
• As I've found with Devstack, if you don't give devs an obvious way to reset some of their stack when they get stuck, then many of them will waste a bunch of time completely destroying it and then re-provisioning when they get stuck. Advanced users will know that they can just rm -rf env/, but new devs won't necessarily remember that or understand what it means.

Against tutor config save --clean:

• Every new option adds some more mental overhead to Tutor.

Personally, I think the pros outweigh the cons.

Thinking longer term, I do think we should discourage people from editing env/ by hand. It's not reproducible, which seems counter to the ethos of Tutor. Instead of hand-editing, wouldn't we rather that folks write a plugin?

[regisb]

I'm also in favor of a tutor config save --clean option. We should just make it clear that this is not an option that users should have to use frequently. Thus, I propose the following:

1. tutor config save --clean/-c will delete env/ prior to updating the configuration and environment.
2. The command should not crash when env/ does not exist.
3. The command should print a warning explaining that all files in env/ are deleted.
4. In interactive mode (config save -i) the command should pause for confirmation.

[bmtcril]

That sounds great, thanks!

[misilot]

This would be neat, especially with themes as files do not get cleaned up as they are renamed / moved/ deleted etc.. Thanks!