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

Gaffer : Version the preferences location #5555

Closed
wants to merge 2 commits into from

Conversation

johnhaddon
Copy link
Member

This avoids problems caused by saving preferences in a later version in a format not compatible with an earlier version. The most common problem is saving a layout containing an editor that doesn't exist in the earlier version, and version 1.4 will shortly be introducing a new editor.

Fixes #368
Fixes #2882

@johnhaddon johnhaddon self-assigned this Nov 20, 2023
@johnhaddon
Copy link
Member Author

Pushed a commit that should fix the Windows test failure - hopefully it makes sense @ericmehl.

@ericmehl
Copy link
Collaborator

The WIndows fix looks good to me.

@johnhaddon
Copy link
Member Author

We don't like mixing startup/<version> alongside startup<component> (e.g startup/1.4 and startup/gui), so the plan is to update this to use startup as it was before (applying to all versions) and introduce a new startup-<version> directory.

johnhaddon and others added 2 commits January 5, 2024 13:09
This avoids problems caused by saving preferences from a later version in a format not compatible with an earlier version. The most common problem is saving a layout containing an editor that doesn't exist in the earlier version, and version 1.4 will shortly be introducing a new editor.

Fixes GafferHQ#368
Fixes GafferHQ#2882
@johnhaddon johnhaddon force-pushed the preferencesLocation branch from 5959585 to 821a2b4 Compare January 9, 2024 12:45
@johnhaddon
Copy link
Member Author

the plan is to update this to use startup as it was before (applying to all versions) and introduce a new startup- directory.

I've now pushed a new version which implements this plan. But I don't think we should merge it, for the following reasons :

  • It's pretty broken for layouts.py. When startup/gui/layouts.py runs, it calls Layout.add( ..., persistent = True ), which immediately saves over startup-1.4/gui/layouts.py, clobbering anything version specific in there. Similar problems likely exist for bookmarks.py and recentFiles.py although I haven't investigated those thoroughly. Might be possible to work around this by looking at the call stack and not saving the files if we detect we're currently loading configs.
  • Even if the above is fixed, it still kindof sucks for users :
    • 1.3 will save layouts in startup, and 1.4 will save layouts in startup-1.4, and you'll be managing two sets of layouts.
    • We'll need to educate everyone as to when to put custom scripts in startup and startup-1.4.
    • The startup sequence will be more complex and harder to debug.
  • The long term result is going to be a directory full of startup-1.4, startup-1.5...startup-1.N` subdirectories that nobody wants to look at.

This all seems like using a sledgehammer to crack a nut when the only problem we've ever had between versions is layouts with editors that don't exist in the previous version. I've opened PR #5619 to deal with that directly, in a way that allows the user to manage a single set of layouts, gracefully handling editors that aren't currently available.

I propose we merge #5619, close this PR, and close #368 as "ain't broke; won't fix".

@johnhaddon
Copy link
Member Author

Closing in favour of #5619.

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

Successfully merging this pull request may close these issues.

Running new version of gaffer breaks old version. Version the preferences location
2 participants