Fixed error messages after multiple runs of GrandOrgue ftom Appimage with a demo organ https://github.com/GrandOrgue/grandorgue/issues/1660 #1681
Conversation
|
@oleg68 Would it be an option to just remove the demo sample set from the AppImage build? |
|
@oleg68 Actually, earlier we had a separate build artifact for only the demo sample set to download. Perhaps it would be a good idea to remove the default inclusion of the demo sample set from all builds if not explicitly set to be included and have a separate target for only the demo sample set (or even just include a link to it as it doesn't change that often and could be just downloaded once...). What this would do is to reduce the sheer amounts of different demo sample sets some of us have floating around when we're testing different builds... Installation of the demo sample set would be a choice and exist only in one instance (if not the user decides otherwise when building from source). |
|
@larspalo I object to remove the demo from appimage because i is intended for using by not advanced users for whom it is difficult to download a separate sample set and to open it with GrandOrgue. The next PR will automatically remove unexisting organs that were opened from a temporary directory. So unpacking GO under the temporary directory will solve your problem of testing multiple versions. |
No, it won't. Every time I download an archive of GO to test a PR a new demo sample set is registered with it.
One doesn't "Open" a package in GO, it's "Loaded". If it's too difficult to download and place the .orgue package in the package folder for GO and then start GO - the intelligence of people in the world is really going downhill... |
|
@oleg68 Just to prove my point, here is a screenshot of the organ list from GrandOrgue on my computer.
The fact that most of the demo sample sets listed are actually deleted but GO doesn't report them as missing is a bug in itself... I usually clean up to list from time to time, but it's quite un-necessary that we include the demo sample set in any build that is not in fact a release. |
I hope this bug is fixed with #1682. Earlier the existance was checked by ArchiveId rather than by path, so GO considered an absent archive as existing because there was another existing archive with the same id. |
#1682 will help you if you unpack GO under the temp dir.
The package folder does not exist before GO runs for the first time. The current behaviour is the most simple: to download appimage, to set it executable, to double click and to play without any extra steps. Necessarity of downloading a sample set would add more complexity. So I'd leave the demo organ packaged in appimage. |
Ok, but it should be removed from all others, and instead be made available as an optional download. |
A true solution must work no matter what directory is used. Each version of an archive will be extracted to another base directory by default. An executable, archive (.tar.gz, .zip or whatever really) should not be required to be extracted to a specific place, it is the .orgue package that should be placed in a specific place... |
I ageree with removing the demo from .zip and .tar.gz, because they usually used for resting where GO has already installed. But I still insist on presenting the demoin all installation packages, because they are overwritten by the next installation and no multiplication occurs. But changing shipping the demo is a subject of a separate issue. @larspalo do you have any reasons why not to get rid of error messages when missing archives exist, but the current organ is loaded from an existing one? |
There was a problem hiding this comment.
@oleg68 What I have the most problem with is the approach of trying to hide a bigger issue like this. The correct solution, which you know, is that organ packages should always be placed in the package directory that GrandOrgue searches, where any duplicates never could occur anyway (because they would be overwritten). During an installation, the demo package should be moved to the correct directory which should be created if not existing. So if the installation process would do the correct thing then no problem would arise.
But more generally, I think that this full path to the organ packages is actually a very good idea! As I mentioned in #1544 (reply in thread) it would be really great if we can reference to samples existing within packages for re-using without needing to un-pack them first. As I mention and show in that post, this is partially possible already but since the paths are not complete the risk is present that conflicting "paths" can occur when trying to access the samples if multiple organs are used that have the same/similar naming structure. I've not had the time to investigate this but I do see a potential here!
|
There are lots of places in GO where organ packages are referenced by archive id instead of by path. This PR eliminates one of them - loading an organ, #1682 fixes another place - checking the organ for existence. But, of cause, a dependency between organ packages are still by id rather than by full path and now I don't have any ideas how to improve it. Lets continue the discussion in the discussion area, not here. |
This is the first PR related to #1660
Because running GrandOrgue from applimage causing unpacking it to a temporary directory, the demo Organ may be registered multiple time with different paths. Old paths are not more valid, because the old directories have already been deleted.
Earlier opening an organ from a package were only by ArchiveId and the path was not taken in account. It caused trying to open the archive from all paths had been registered and logging error messages on each invalid path.
This PR adds passing the archive path to GOArchiveManager::LoadArchive that makes unnecessary looking over invalid paths. So this PR eliminates logging extra error messages.
Multiple demo organs registered are still an issue and will be fixed with another PR.