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

Drumkit.xml files should be upgraded at build-time #1481

Open
sten0 opened this issue Feb 8, 2022 · 27 comments
Open

Drumkit.xml files should be upgraded at build-time #1481

sten0 opened this issue Feb 8, 2022 · 27 comments

Comments

@sten0
Copy link

sten0 commented Feb 8, 2022

Hydrogen version * : 1.1.1
Operating system + version : Debian sid/unstable (will become 12 "Bookworm")
Audio driver + version : NA


Dear Hydrogen development team,

I'm the Debian maintainer for this software, and I noticed the following errors with the latest release:

(E) Drumkit::upgrade_drumkit Drumkit /usr/share/hydrogen/data/drumkits/Audiophob/drumkit.xml is out of date but can not be upgraded since path is not writable (please copy it to your user's home instead)
(E) Drumkit::upgrade_drumkit Drumkit /usr/share/hydrogen/data/drumkits/BJA_Pacific/drumkit.xml is out of date but can not be upgraded since path is not writable (please copy it to your user's home instead)
(E) Drumkit::upgrade_drumkit Drumkit /usr/share/hydrogen/data/drumkits/circAfrique v4/drumkit.xml is out of date but can not be upgraded since path is not writable (please copy it to your user's home instead)

Steps to reproduce:

  1. the Debian source package uses CMake-defined methods to install these files.
  2. install Debian sid/unstable
  3. sudo apt install hydrogen
  4. launch hydrogen from the console
  5. see the errors in red text

Expected behaviour:
All drumkit.xml files should be "updated" when the package is built, and this seems like something that should be done by default in the upstream rather than worked around during build time in every distribution. Alternatively maybe these files shouldn't be installed at all, but should instead be generated in $HOME.


This also affects Ubuntu and its derivatives, and the deadline for getting this fix into Ubuntu 22.04 (and derivatives) LTS is 16 Feb, because it needs time to migrate through Debian's QA before Ubuntu does their last sync on 23 Feb.

@sten0
Copy link
Author

sten0 commented Feb 8, 2022

P.S. I'm not interested in Ubuntu myself, but I want to do what I can to provide downstream users with a great experience. You know, because these users tend to be stuck on the same Hydrogen version for two years or more ;-) If HEAD is in a good state for a point release, then maybe that's an option too. No worries or pressure of course, it's just that time of year on an even-numbered year!

@theGreatWhiteShark
Copy link
Contributor

Nice timing! I just skimmed through a user's log the other day and was quite confused to find those kits being installed on system level and not in the user's HOME.

All drumkit.xml files should be "updated" when the package is built, and this seems like something that should be done by default in the upstream rather than worked around during build time in every distribution. Alternatively maybe these files shouldn't be installed at all, but should instead be generated in $HOME.

Hydrogen itself does feature a dialog for downloading (the most recent version of) all drumkits. And it does so since the beginning of our git history in 2007. There are two kits that should be bundled with the data folder installed on system-level in order for the user to get started right away and we do our best to keep them up-to-date.

On Devuan Beowulf - which most probably uses the Debian repo as upstream - I do see two packages bundling various kits: hydrogen-drumkits and hydrogen-drumkits-effects. Do you know why or since when they exist? Maybe this is something predating Hydrogen's internal download dialog. Don't know.

From my point of view dropping these two bundle packages would be best. Else we have to deal with kits being present at multiple locations, can not upgrade kits as they are read-only, and do not have control over the versions shipped.

@mauser
Copy link
Member

mauser commented Feb 12, 2022

Hi,

@theGreatWhiteShark, you seem to be correct there. The hydrogen-drumkits packages seems to date from 2006 (Hydrogen 0.9.3), the sound library manager got introduced in 0.9.4. But i'm not sure if the online import was there in a different form before..
One has to consider that back then not everyone had fast internet access and it was nice when such packages could be found on one of the cdroms of your favourite distribution :)

@sten0
Copy link
Author

sten0 commented Feb 13, 2022 via email

@mauser
Copy link
Member

mauser commented Feb 13, 2022

Hi,

first of all, let me say something about the drumkits in your package: Those drumkits we're sent to us by users who ask us to host them at our drumkit server. Most of the time, we do not touch them or develop them further. They are also not part of any kind of git repository, they live at a file storage at Sourceforge. In the past we only touched them if they had major errors.
In most cases, we also did not upgrade them to newer drumkit versions - mostly because our online drumkit service is not able to provide multiple versions of a file, which leads to compatibility problems (all versions of Hydrogen use the same drumkit server). I hope this makes it a bit better understandable why we don't upgrade all of those drumkits on our servers.

In contrast to this, you want to make shure that the xml version of the kits in your package match the version of your current Hydrogen binary. This is a different requirement/use case then we have for our hosted kits. In my view this means that the kits for your package need active maintenance and must be upgraded and tested for each new version of Hydrogen. I'm currently not active in Hydrogen development so i can't really judge if this is feasible for Hydrogen or not, but this creates some additional work for the Hydrogen team. There could also be a problem of putting also those kits in git repos at Github because of file size limitations (i have to check on that).

@sten0
Copy link
Author

sten0 commented Feb 13, 2022 via email

@theGreatWhiteShark
Copy link
Contributor

Given that hydrogen refreshes the xml files at runtime, there must be a
function like rebuild_refresh_drumkit_xml(drumkit_path_or_xml_path).
It seems like it would be enough to resolve this issue if that function
was exposed as command line argument so that hydrogen could provide this
utility function. Something like hydrogen --refresh-drumkit $PATH_or_xml_PATH. If this required running a GUI, then I'd run it
with QT_QPA_PLATFORM=offscreen or even xvfb-run if necessary, but it
would be really nice if it didn't require a GUI. Accepting either a
basedir of the drumkit, or the path to the xml file as an argument would
do the trick.

I'm hoping that it is trivially easy for someone familiar with Hydrogen
to expose this utility function as a command line argument.
Unfortunately the kits won't be in the expected locations, because
they'll be unpacked to a tmp build dir, which is why I'll need to
provide something like $temp_build_dir/$drumkit_name_basedir[.xml] as an
argument. That requirement/limitation should be easy to resolve too.

I think that's feasible and it also might be helpful while resolving other issues.

I would add this to the current master branch and not v1.1.1. Is this okay with you?

@sten0
Copy link
Author

sten0 commented Feb 14, 2022 via email

@theGreatWhiteShark
Copy link
Contributor

I'll definitely make an attempt to backport the
commit[s] and, if successful, can start a 1.1.1 (or 1.1.x) branch on my
remote, then submit it for review with nice changelog entries etc.
'hopefully the drumkit.xml work done on master in early 2021 won't make
this too tricky ;)

Backporting shouldn't be too difficult as there were not much changes in this part of code. But I can also provide you a patched version of v1.1.1 in a custom branch.

Btw. I'm almost done. I think I can give you a working version tomorrow afternoon latest.

@theGreatWhiteShark
Copy link
Contributor

@sten0 I created a PR in #1490. It still needs a little bit of testing (and documentation) but the API should be stable.

I tested the upgrade for all the .h2drumkit files hosted at our SourceForge page, which should be a superset of what is contained in the Debian package and - at least as far as my unit test is concerned - it should work for all of them. But please have an open eye on the results.

@theGreatWhiteShark
Copy link
Contributor

Alright. I'm done. I also rebased all commits of #1490 on tag 1.1.1 in branch drumkit-cli-1.1.1.

@sten0
Copy link
Author

sten0 commented Feb 21, 2022 via email

@sten0
Copy link
Author

sten0 commented Feb 23, 2022 via email

@theGreatWhiteShark
Copy link
Contributor

Hey @sten0 ,

Sorry for the delay. Life.

Don't worry.

Does SourceForge track download counts these days?

I think so. At least they are shown for the individual files when browsing through the uploaded folders. But I know frightening little about SourceForge.

Meanwhile, for the Ubuntu LTS consideration, I realised that it would be
better to not rush an update. It seems safer to request an update once
our new solution has been tested for at least a week or two.

I think so too. After all it's just about not flooding the console in case user decides to open Hydrogen via terminal and a minimal increase of startup time.

More importantly though, I read some comments in eab4acc that indicated
that an absolute path to the drumkit is needed?

This is mainly because previously I did everything using OSC commands and there relative paths are not an option. But I definitely took a liking to our CLI during the last days and I already added support for relative paths locally.

Also, I found a bug in my PR. It only affects exporting drumkits via the GUI but not upgrading them using the CLI. I will provide a patch for branch drumkit-cli-1.1.1 soon regardlessly (which will also contain the relative path support).

As I understand these triggers, the solution will work
thus: when a new version of Hydrogen is installed, it triggers a refresh
of the system copy of the drumkits; when a new version of
hydrogen-drumkits is installed, it of course also activates the same
function.

So, this will happen locally on the user's devices and you intend to release the patched hydrogen version in branch drumkit-cli-1.1.1 using the "hydrogen" package, right?

@theGreatWhiteShark
Copy link
Contributor

@sten0 are there still some open points or is everything working fine with the CLI solution we came up with?

@sten0
Copy link
Author

sten0 commented Apr 9, 2022 via email

@theGreatWhiteShark
Copy link
Contributor

@sten0 could you try #1564? If everything works fine, it will be part of the upcoming 1.1.3 release.

@sten0
Copy link
Author

sten0 commented Apr 12, 2022 via email

@theGreatWhiteShark
Copy link
Contributor

Sadly all of my
free time went to resolving the delta between 1.1.1 release tarball and
a snapshot of the master branch, so I've had to defer implementation of
the trigger to batch-upgrade all drumkit.xml files.

If such a thing happens at some point in the future again: please feel free to ping us and to ask to resolve such conflicts. If we have some time at hand, we might help :)

By the way, is the new tutorial, in English only, the only available
documentation, or will the old docs be added back for 1.1.2?

The docs are actually cutting-edge right now. I rewrote them for the 1.1 release and they work for all patch releases in the 1.1.* series too. But the commit checked out in the submodule might not the the proper one. We also wanted to keep the tags in the hydrogen repo and those in the documentation consistent. But it seems we didn't 😁 . I'll have a look.

The tutorial is a different story. It is as old as (git) history itself (at least 15 years). Maybe I find time to update it for 1.2. But right now it can be considered seriously out of date.

@theGreatWhiteShark
Copy link
Contributor

We also wanted to keep the tags in the hydrogen repo and those in the documentation consistent. But it seems we didn't grin . I'll have a look.

Fixed

@sten0
Copy link
Author

sten0 commented Apr 21, 2022 via email

@theGreatWhiteShark
Copy link
Contributor

Have they also been translated, or would
you like me to ask if anyone on the Debian translation teams might be
interested?

Well, there were some translations at some point. But after the rewrite none of them reached 30% coverage. We discussed about dropping them (or moving them to some point they won't be installed anymore) but we haven't done it yet.

Yes, that would be awesome. I'm not a 100% sure at which point to do so, though. I will apply some changes during the next month to keep everything up-to-date for the 1.2.0. Afterwards, the document will probably we stable for quite some time.

Would you say this is a case where "some
translated documentation" is actually worse than no documentation? If
so, please let me know, and I'll drop it from the Debian package so as
not to mislead users.

Hmm. Not sure. But I think it's still better than other stuff out there. E.g. there are tutorials on youtube working with version such as 0.7.5 which is almost 20 years old.

In other news, it seems I've run out of free time again. Please
continue to ping me about implementing the drumkit.xml file upgrade
functionality.

At least as far as I can see installation, extraction, validation, and upgrading is now working fine both with absolute and relative paths. Also, the code is in both master and releases/1.1, which will be the basis for the upcoming 1.1.3 release. So, AFAICS this feature is implemented and works.

@sten0
Copy link
Author

sten0 commented Sep 28, 2022

Have they also been translated, or would
you like me to ask if anyone on the Debian translation teams might be
interested?

Well, there were some translations at some point. But after the rewrite none of them reached 30% coverage. We discussed about dropping them (or moving them to some point they won't be installed anymore) but we haven't done it yet.

Yes, that would be awesome. I'm not a 100% sure at which point to do so, though. I will apply some changes during the next month to keep everything up-to-date for the 1.2.0. Afterwards, the document will probably we stable for quite some time.

Is this something that is delaying the 1.20 release?

Ah, now I see, the documentation has its own separate project now. It sounds like it will be best to just create a new Debian package for the new docs :) For lots of reasons. I'm currently blocked by missing copyright and licence

In other news, it seems I've run out of free time again. Please
continue to ping me about implementing the drumkit.xml file upgrade
functionality.

At least as far as I can see installation, extraction, validation, and upgrading is now working fine both with absolute and relative paths. Also, the code is in both master and releases/1.1, which will be the basis for the upcoming 1.1.3 release. So, AFAICS this feature is implemented and works.

Thanks for the ACK! I think at this point I'll need to delegate, because this Debian work will need to be complete in the next three months.

@theGreatWhiteShark
Copy link
Contributor

Is this something that is delaying the 1.20 release?

No, not at all. I already did the update of the docs and they will be tag as soon as the 1.2.0 beta release of Hydrogen itself is done. The latter is, unfortunately, blocked due to me. Apart from all the visual improvement the main change in the upcoming version is a rewrite of large parts of Hydrogen's audio engine to get rid off long standing glitches, bugs, and inconsistencies. But recently we found that there are still occasional glitches. I'm working to kill them and already covered a lot of ground. But I do not want to make a release without those fixes because consistency in the audio transport and rendering is so essential it will be worth the wait (at least I hope so).

Ah, now I see, the documentation has its own separate project now. It sounds like it will be best to just create a new Debian package for the new docs :) For lots of reasons. I'm currently blocked by hydrogen-music/documentation#73

At least as far as git history is concerned the docs were always separated from the main repo.

By the way: it is possible access a local copy of the docs from within Hydrogen using Info > User Manual. So, the docs need to be a dependency of the main repo.

@theGreatWhiteShark
Copy link
Contributor

Hey @sten0,

CLI options to upgrade kits are now released as part of version 1.2.0. Are there still aspects of your issue that aren't covered yet or are we done for now?

@sten0
Copy link
Author

sten0 commented May 11, 2023 via email

@theGreatWhiteShark
Copy link
Contributor

In other words, having missed the hard freeze deadline will inconvenience to users

I know. It's not ideal. But we still doing our best with limited resources and Debian users will get a hardened and more stable patch release of 1.2. right away.

Then let's keep this one open for now.

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

No branches or pull requests

3 participants