Message ID | 0eceb36ac47fee789ebaa551cc3b041e777bbce1.camel@telenet.be |
---|---|
Headers | show |
Series | Update some minetest packages | expand |
Hey Maxime, Welcome back! ;-) Maxime Devos <maximedevos@telenet.be> skribis: > This patch series updates some minetest packages. > They were made with 'guix refresh -u -t minetest', using the 'git- > fetch' updater from <https://issues.guix.gnu.org/50072#24>. Oh nice. > Checklist: > * [x] version, commit and sha256/base32 updated > * [x] it builds: ./pre-inst-env guix build minetest-homedecor-modpack > minetest-unified-inventory minetest-mobs-animal minetest-mobs minetest- > basic-materials minetest-technic --rounds=2 > * [x] linter is satisfied: ./pre-inst-env guix lint minetest- > homedecor-modpack minetest-unified-inventory minetest-mobs-animal > minetest-mobs minetest-basic-materials minetest-technic > * [x] source code diff reasonable > > minetest-technic: some small documentation and code changes > minetest-basic-materials: new subdirectory '.github'. > minetest-mobs: some code changes, updated translations > minetest-mobs-animal: Some code changes (chickens are different > now?) > minetest-unified-inventory: Lots of code changes, some > documentation changes, updated translations and images > minetest-homedecor-modpack: Some code changes. > * [x] Minetest doesn't crash > ./pre-inst-env guix shell --pure minetest-homedecor-modpack > minetest-unified-inventory minetest-mobs-animal minetest-mobs minetest- > basic-materials minetest-technic minetest -- minetest > Create world with all mods enabled. > + [x] Inventory mode appears to work > + [x] spawning chickens appears to work > + [x] rotating blocks from homedecor works Neat. I applied them all. > A TODO for the git-fetch updater: > * [ ] following redirects (to avoid mixed case -> lower case changes > and .git suffixes in GitHub URLs) > * [ ] Support (let ((commit ...) (revision ...)) [...]) so more > packages can be updated automatically Hmm wasn’t it the reason we introduced ‘package-definition-location’? Thanks! Ludo’.
Ludovic Courtès schreef op wo 05-01-2022 om 23:30 [+0100]: > > A TODO for the git-fetch updater: > > * [ ] following redirects (to avoid mixed case -> lower case > > changes > > and .git suffixes in GitHub URLs) > > * [ ] Support (let ((commit ...) (revision ...)) [...]) so more > > packages can be updated automatically > > Hmm wasn’t it the reason we introduced ‘package-definition-location’? Yes, but it has been a while ago so I preferred to not overcomplicate the patch series more. There's also the complication that (revision ...) might need to be incremented. Yet another complication: minetest mods can have two separate version schemes: the ‘release titles’ on content.minetest.net, and the version they use in forum posts or git repos. For minetest-ethereal, they are different, and worse, the latest version on content.minetest.net doesn't have a corresponding version on the forum. Hopefully there's some kind of solution, and presumably package-definition-location will be part of that solution, but it will probably be messy. Or maybe the solution is to ask upstream nicely to give every release a version number and switch to ContentDB release titles if they disagree. Anyway, I'd prefer to leave these complications for future patches. Greetings, Maxime.
Ludovic Courtès schreef op wo 05-01-2022 om 23:30 [+0100]: > Hey Maxime, > > Welcome back! ;-) > > Maxime Devos <maximedevos@telenet.be> skribis: > > > This patch series updates some minetest packages. > > They were made with 'guix refresh -u -t minetest', using the 'git- > > fetch' updater from <https://issues.guix.gnu.org/50072#24> > > btw I left a sneek message on #guix with information on where v5 can be found, apparently it didn't make it onto issues.guix.gnu.org somehow.
Hi, Am Donnerstag, dem 06.01.2022 um 00:18 +0100 schrieb Maxime Devos: > Yes, but it has been a while ago so I preferred to not overcomplicate > the patch series more. There's also the complication that > (revision ...) might need to be incremented. Yet another > complication: > minetest mods can have two separate version schemes: the > ‘release titles’ on content.minetest.net, and the version they use in > forum posts or git repos. For minetest-ethereal, they are different, > and worse, the latest version on content.minetest.net doesn't have a > corresponding version on the forum. If I recall correctly, this was also a point of debate in the initial series that added the importer. Can we establish an ordering/heuristic here? E.g. "if we have git tags use those, otherwise use contentdb", "always use contentdb" or "always use whatever was edited most recently"?
Hi, Liliana Marie Prikler schreef op do 06-01-2022 om 01:52 [+0100]: > > [...] > If I recall correctly, this was also a point of debate in the initial > series that added the importer. Can we establish an ordering/heuristic > here? E.g. "if we have git tags use those, otherwise use contentdb", > "always use contentdb" or "always use whatever was edited most > recently"? Keep in mind that the minetest importer doesn't know about git tag -- the only interaction it has with git is cloning repositories and checking out commits by the commit id provided by ContentDB. I'm assuming you're referring to the generic-git updater here, or a hypothetical minetest updater that has been modified to interact with git tags. * Problem with using git tags: git tags sometimes disappear. E.g., in minetest-ethereal, there's currently a tag 2021-04-06 and 2021-09-23, but there's no tag for 2021-07-28 (the version currently in guix). This could be resolved by including the commit instead of the tag in the package definition, and still searching for the git tag, but as I understand it, there have been some objections to including the commit in the package definition (https://lists.gnu.org/archive/html/guix-devel/2021-12/msg00259.html). Even then, there's another problem: sometimes releases are made without a corresponding git tag. E.g., on ContentDB there's a version 2022-01-05 but there's no 2022-01-05 tag in the git repository. That could be resolved by ‘always use contentdb for minetest packages’ or ‘always use whatever was edited most recently’. * Problem with ‘whatever was edited most recently’: AFAIK git tags don't carry that information. Though the commit time/modification time in the commit it points to might be a decent approximation in practice. ContentDB has some information on when a release was released (release_date, see https://content.minetest.net/help/api/). I suppose this could work, though there's a slight problem: The version scheme in guix would occassionally switch between x.y.z and YYYY-MM-DD, which would confuse the ‘these packages have been upgraded’ logic. I suppose the best option would be to always use the version from ContentDB (*), because the exact versioning scheme used doesn't matter much, as long as it remains consistent over time, and just using ContentDB is convenient. (*) Unless it isn't on ContentDB of course, though all minetest packages currently in Guix are on ContentDB. Additionally, if the forum versions / git tags / contentdb releases are inconsistent (e.g. the forum and git tags are x.y.z and the releases are YYYY-MM-DD), we could inform upstream that guix uses the release titles because otherwise things become complicated for guix, so if upstream doesn't want that, they need to use x.y.z in their release titles as well Does that seem reasonable to you? I could write a patch to that effect. Greetings, Maxime.
Hi, Am Donnerstag, dem 06.01.2022 um 09:33 +0000 schrieb Maxime Devos: > Hi, > > Liliana Marie Prikler schreef op do 06-01-2022 om 01:52 [+0100]: > > > [...] > > If I recall correctly, this was also a point of debate in the initial > > series that added the importer. Can we establish an > > ordering/heuristic here? E.g. "if we have git tags use those, > > otherwise use contentdb", "always use contentdb" or "always use > > whatever was edited most recently"? > > Keep in mind that the minetest importer doesn't know about git tag > -- the only interaction it has with git is cloning repositories > and checking out commits by the commit id provided by ContentDB. > I'm assuming you're referring to the generic-git updater here, > or a hypothetical minetest updater that has been modified to > interact with git tags. Ahh, sure, but imho it could try to make a best effort guess. E.g. if git tags are named like Minetest releases and exist in equal count, assume a mapping from one to the other. However if I recall correctly there was a ContentDB policy to only tag once, which makes the mapping from ContentDB version to git commit unique. Do I remember correctly? > * Problem with using git tags: git tags sometimes disappear. > E.g., in minetest-ethereal, there's currently a tag > 2021-04-06 and 2021-09-23, but there's no tag for 2021-07-28 > (the version currently in guix). > > This could be resolved by including the commit instead of the > tag in the package definition, and still searching for the git tag, > but as I understand it, there have been some objections to > including the commit in the package definition > (https://lists.gnu.org/archive/html/guix-devel/2021-12/msg00259.html). Thanks for linking my thread :) I do cite minetest.scm as an exception there, since your comments make very clear what you're doing and why. However, my actual intent is blown way out of proportion, which has led to a lot of confusion on all sides and a rather long discussion. To clarify it here, I am trying to avoid the following pattern: (package (name "hello") (version "1.0") (source (origin (method git-fetch) (uri (git-reference (url some-url) (commit "abcdef..."))) [...])) [...]) while encouraging both (package (name "hello") (version "1.0") (source (origin (method git-fetch) (uri (git-reference (url some-url) (commit (version->git-tag version)))) [...])) [...]) and (package (name "hello") (version (git-version "1.0" revision commit)) (source (origin (method git-fetch) (uri (git-reference (url some-url) (commit commit))) [...])) [...]) In the latter, revision and commit are let-bound as per Guix' standards. Now if you say "minetest mod packages are generally unreliable, git-version everywhere", that is completely fine by me. > Even then, there's another problem: sometimes releases are made > without a corresponding git tag. E.g., on ContentDB there's a > version 2022-01-05 but there's no 2022-01-05 tag in the git > repository. > > That could be resolved by ‘always use contentdb for minetest > packages’ or ‘always use whatever was edited most recently’. > > * Problem with ‘whatever was edited most recently’: AFAIK git tags > don't carry that information. Though the commit time/modification > time in the commit it points to might be a decent approximation > in practice. > > ContentDB has some information on when a release was released > (release_date, see https://content.minetest.net/help/api/). > > I suppose this could work, though there's a slight problem: > > The version scheme in guix would occassionally switch between x.y.z > and YYYY-MM-DD, which would confuse the ‘these packages have been > upgraded’ logic. Does ContentDB always use CalVer or are the repo owners in control of the tags? If there's a SemVer/CalVer conflict, I would say doing (latest-semver)-(calver)-commit would probably be acceptable. At least I hope none of these mods release twice daily. > I suppose the best option would be to always use the version from > ContentDB (*), because the exact versioning scheme used doesn't > matter much, as long as it remains consistent over time, and just > using ContentDB is convenient. > > (*) Unless it isn't on ContentDB of course, though all minetest > packages currently in Guix are on ContentDB. If a package was used outside of ContentDB, that's not ContentDB's requirement, is it? Now perhaps there is an issue if the contentdb updater relies on the minetest-mod-build-system being used to determine that its suitable, rather than something else. > Additionally, if the forum versions / git tags / contentdb releases > are inconsistent (e.g. the forum and git tags are x.y.z and the > releases are YYYY-MM-DD), we could inform upstream that guix uses the > release titles because otherwise things become complicated for guix, > so if upstream doesn't want that, they need to use x.y.z in their > release titles as well > > Does that seem reasonable to you? I could write a patch to that > effect. Poking upstream maintainers sounds fun, but before making a hard decisions, perhaps we should reach out to them and ask what they consider the most reliable. If it's a wild mess, we could also use package-properties with an assumed default of "release title = git tag = forum post", e.g. (upstream-versioning . forum-post), (upstream- versioning . contentdb) and (upstream-versioning . git-version) to indicate who's right in case of a mismatch. WDYT?
Maxime Devos <maximedevos@telenet.be> skribis: > Ludovic Courtès schreef op wo 05-01-2022 om 23:30 [+0100]: >> > A TODO for the git-fetch updater: >> > * [ ] following redirects (to avoid mixed case -> lower case >> > changes >> > and .git suffixes in GitHub URLs) >> > * [ ] Support (let ((commit ...) (revision ...)) [...]) so more >> > packages can be updated automatically >> >> Hmm wasn’t it the reason we introduced ‘package-definition-location’? > > Yes, but it has been a while ago so I preferred to not overcomplicate > the patch series more. OK, I thought we more or less had a solution in place back then and just missed ‘package-definition-location’. I think we could do something along these lines: (edit-expression (location->source-properties (package-definition-location p)) (lambda (str) ;; substitute commit ID in STR …)) But yes, let’s keep that for a subsequent patch. Thanks, Ludo’.
Ludovic Courtès schreef op di 11-01-2022 om 13:53 [+0100]: > Maxime Devos <maximedevos@telenet.be> skribis: > > > Ludovic Courtès schreef op wo 05-01-2022 om 23:30 [+0100]: > > > > A TODO for the git-fetch updater: > > > > * [ ] following redirects (to avoid mixed case -> lower case > > > > changes > > > > and .git suffixes in GitHub URLs) > > > > * [ ] Support (let ((commit ...) (revision ...)) [...]) so more > > > > packages can be updated automatically > > > > > > Hmm wasn’t it the reason we introduced ‘package-definition-location’? > > > > Yes, but it has been a while ago so I preferred to not overcomplicate > > the patch series more. > > OK, I thought we more or less had a solution in place back then and just > missed ‘package-definition-location’. > > I think we could do something along these lines: > > (edit-expression > (location->source-properties (package-definition-location p)) > (lambda (str) > ;; substitute commit ID in STR > …)) > > But yes, let’s keep that for a subsequent patch. See <https://debbugs.gnu.org/cgi/bugreport.cgi?bug=53144>, in particular the new 'latest-git' updater. Greetings, Maxime.