- not saving, just warning. Then possible problems (things we developers overlooked) might materialise later, and might go unnoticed by the user
On 31.05.25 10:41, Kristian Amlie wrote:
But isn't all migration done during load? What is kept in memory is really just a mirror of the most recent config format, isn't it? Because all problems should already become apparent without saving then.
This is indeed the fact. The point I wanted to make is, that many of the config settings have to deal with fine points, which are not immediately obvious when using yoshimi. I think that is the reasoning behind that warning. We developers might have overlooked something, which only is of relevance for some specific use case, which we weren't aware of. On 31.05.25 11:47, Will Godfrey wrote:
As far as I recall, we only automatically save the config on startup if it is missing or unreadable.
It may be surprising, but that is not the case. There always used to be a special test for the version number, somewhere hidden in the very long function which establishes the config at start-up. This test was even more hidden, since the information regarding incompatible config was passed on quite indirectly, with the help of two version infos (major, minor), which were set by a very tricky side-effect hidden in the enterbranch() function. Furthermore, this save was problematic, since at that point the banks were not yet loaded, which meant, that existing Yoshimi versions always lost the current bank and root information on each version upgrade (even minor version upgrades). I was often annoyed by that, but was never able to spot the reason or reproduce it, until I now went through the full analysis how this version handling worked. Now I have consolidated that as follows: - we have now a all the checks in a single function in Config::verifyVersion() which also must be called explicitly now after creating an XMLStore. - and we do the actual migration at a later ponint now, after banks are loaded and the primary synth is fully booted up. I made a new function for that purpose Config::maybeMigrateConfig() (which is located in the code of Config.cpp close to the previously mentioned check function, so hopefully it is easier to discover)
However uniquely, if we alter a config setting this immediately re-saves the entire config. This is so it is always up to date, and there is no longer a need to save it at exit, which previously was causing some annoyance.
Seemingly many user like it more this way. It has the downside though that you need to touch some config setting in order to cause a save of the config. So it is now kind of confusing when we warn and ask the user to save the config, while there is no obvious way to do so any more (it is hidden in the CLI, where you can still save the config, and I am glad this option is still there for the more technically savy people). But my point of view is very biased in that respect. I am an old timer, with an "automatically hit ctrl-s every 5 seconds" habit I hate software that automatically saves things behind my back. ;-) On 31.05.25 10:41, Kristian Amlie wrote:
I would follow the semantic versioning mantra: Warn when major is different, and when minor is higher, otherwise don't warn.
So to summarise: Kristian you propose the following change: - remove the more elaborate check - rather define "Incompatible" as major differs or minor of the config is higher than current software - never save automatically for migration any more, just warn Basically I am not very passionate with anything of this version logic. I would be happy to implement whatever the consensus is we agree on. -- Hermann _______________________________________________ Yoshimi-devel mailing list Yoshimi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/yoshimi-devel
