Hi, Leo Prikler schreef op za 07-08-2021 om 21:47 [+0200]: > > > > The commit id is > > > > +sometimes missing. The descriptions are in the Markdown format, > > > > but > > > > +Guix uses Texinfo instead. Texture packs and subgames are > > > > unsupported. > > > What is the "commit id"? Is it the hash? A tag? Anything that > > > resolves to a commit? > > > > It's the SHA-1 of the Git commit. I changes this to ‘the commit's > > SHA-1’. > We usually call it the hash around here :) I adjusted the documentation to call it ‘the commit hash’. > > I tried implementing "guix refresh -t minetest ...". It seems to > > work, but requires some changes to (guix upstream) that needs some > > more work, so I left it out of the revised patch set. The refresher > > needs to know the author name (or perform extra HTTP requests), so I > > added 'upstream-name' the package properties. > Could we somehow define a (minetest-uri) procedure that can be supplied > to (guix download)? Somehow Minetest must get the package to > installations across operating systems, so surely there's some download > link somewhere. What is this 'minetest-uri' supposed to do? I assume it would be a procedure like 'pypi-uri', 'cran-uri', 'crate-uri', which takes a package name, the version and should return an URL string pointing to a tarball. That would be possible. ContentDB allows for downloading zips: https://github.com/minetest/contentdb/blob/master/docs/minetest_client.md#downloading-and-installing The URL would look like: /packages///releases//download/. Here, is a the ‘release id’, which is an integer (e.g. 4209). It is _not_ the version number, but it is monotonically increasing. There are some problems however: * Old archives are sometimes deleted. TenPlus1/ethereal was added to ContentDB on 2018-02-23, but it only has a single release on ContentDB, from 2021-07-28 . Likewise for TenPlus1/bakedclay, TenPlus1/wine, TenPlus1/bees. Most other mods still have the old archives though (e.g., Jeija/mesecons, sfan5/worldedit, PilzAdam/nether). The mods by TenPlus1 seems to be an exception. * The version number is not included in the download URL, the release id is. So IIUC, update-package-source in (guix upstream) would still need to be adjusted somewhat to support Minetest packages. +(define* (make-package-sexp #:key > > + (guix-name "minetest-foo") > > + (home-page "https://example.org/foo") > > + (repo "https://example.org/foo.git") > > + (synopsis "synopsis") > > + (guix-description "description") > > + (guix-license > > + '(list license:cc-by-sa4.0 > > license:lgpl3+)) > > + (inputs '()) > > + (upstream-name "Author/foo") > > + #:allow-other-keys) > > + [...] > As noted above, this procedure would be somewhat simplified if we could > define a (mintest-uri). > I don't see how a 'minetest-uri' would simplify the definition of 'make-package-sexp'. Using 'minetest-uri' would avoid the need to specify the commit, but 'minetest-uri' needs a release id anyway, so no simplification there. I guess it would avoid the 'download-git-repository' procedure and 'vcs-file?', but see two points above. Also, 'latest-repository-commit' returns a store path, which does not include the '.git' directory, so 'vcs-file?' isn't necessary, so I removed 'vcs-file?'. > > The revised patch series is attached. It can also be found at > > ;;. It > > includes the latest MINETEST_MOD_PATH patch. I'll make the patch to > > export more things in (guix build utils) later (for core-updates). > For the rest of this, I'll only look over 06/20 v2. I'll assume you > did nothing naughty to 01..04. > > > +;; A structure returned by the /api/packages/?fmt=keys endpoint > > +(define-json-mapping make-package/keys package/keys? > > + json->package/keys > > + (author package/keys-author) ; string > > + (name package/keys-name) ; string > > + (type package/keys-type)) ; string > Not sure about this slash, as it typically specifies extension of some > sort. Perhaps just naming this package-keys would be better? Done. > > +(define (package-author/name package) > > + "Given a object, return the corresponding AUTHOR/NAME > > string." > > + (string-append (package-author package) "/" (package-name > > package))) > > + > > +(define (package/keys-author/name package) > > + "Given a object, return the corresponding > > AUTHOR/NAME string." > > + (string-append (package/keys-author package) > > + "/" (package/keys-name package))) > I think it's probably be better to merge this into a single procedure > called "package-full-name", "package-unique-name" or "package-id" > (whichever you prefer naming-wise), which handles both cases. I like the name 'package-full-name'. and are rather different structures and used in different contexts though, so I kept package-full-name and package-keys-full-name separate. FWIW, I added a procedure (define (%construct-full-name author name) (string-append author "/" name)) used by 'package-full-name' and 'package-keys-full-name'. > > +(define (contentdb->package-name name) > > + "Given the NAME of a package on ContentDB, return a Guix-compliant > > name for the > > +package." > > + ;; The author is not included, as the names of popular mods > > + ;; tend to be unique. > > + (string-append "minetest-" (snake-case name))) > I'd at least add an option to generate full names instead, for cases in > which as before we warn about non-uniqueness. Though actually, this > comment is a little misleading as the actual stripping happens... > > + (name ,(contentdb->package-name (author/name->name > > author/name))) > here ContentDB has a policy of requiring mod names to be unique in order to be a ‘approved’, so I don't think name conflict will be a problem in practice. If full names were generated, keep in mind that dependencies would need to use the full names a well. I couldn't find any mods with name conflicts. I would just emit a warning for now. contentdb->package-name was always used together with 'author/name->name', so I modified contentdb->package-name to call author/name->name as you seem to suggest. It maked the code a little simpler. Thanks, Maxime.