Sarah Morgensen schreef op ma 16-08-2021 om 12:56 [-0700]: > Hi Maxime, > > Thanks for taking a look at this. :) > > Maxime Devos writes: > > > Sarah Morgensen schreef op zo 15-08-2021 om 16:25 [-0700]: > > > * guix/git-download.scm (checkout-to-store): New procedure. > > > * guix/upstream.scm (guess-version-transform) > > > (package-update/git-fetch): New procedures. > > > (%method-updates): Add GIT-FETCH mapping. > > > > Does it support packages defined like (a) > > > > (define-public gnash > > (let ((commit "583ccbc1275c7701dc4843ec12142ff86bb305b4") > > (revision "0")) > > (package > > (name "gnash") > > (version (git-version "0.8.11" revision commit)) > > (source (git-reference > > (url "https://example.org") > > (commit commit))) > > [...]))) > > No, it doesn't. Since the commit definition isn't part of the actual > package definition, the current code has no way of updating it. It > would require a rewrite of the edit-in-place logic with probably a lot > of special-casing. Perhaps a 'surrounding-expression-location' procedure can be defined? (define (surrounding-expression-location inner-location) "Determine the location of the S-expression that surrounds the S-expression at INNER-LOCATION, or #false if the inner S-expression is at the top-level." ??? Something like 'read', but in reverse, maybe? Doesn't need to support every construct, just "string without escapes" and (parentheses other-things) might be good enough in practice for now) Seems tricky to implement, but it would be more robust than relying on conventions like ‘the surrounding 'let' can be found by moving two columns and two lines backwards’. Or see another method (let&) below that is actually implemented ... > There are currently ~1250 package which use this format, though, so it > could be worth it... Perhaps what we actually need is a better idiom to > express this situation. Package properties ('git-commit)? A 'git-version*'? > > --8<---------------cut here---------------start------------->8--- > (define (git-version* version revision) > (let* ((source (package-source this-package)) > (commit (git-reference-commit (origin-uri source)))) > (git-version version revision commit))) > --8<---------------cut here---------------end--------------->8--- > > I'm not sure if binding order would be an issue with that. The 'file-name' field of 'origin' is not thunked, and refers to the 'version' field of the 'package' (also not thunked). If 'version' would use the 'git-version*' from above, then there would be a loop (I'm having the 'gnash' package in mind, see "guix edit gnash"). And git-version* cannot be a procedure, it must be a macro, as it used 'this-package', which can only be expanded inside a package definition. Alternatively, what do you think of a let& macro, that adjusts the inner expression to have the source location of the 'let&' form: (define-syntax with-source-location (lambda (s) (syntax-case s () ((_ (exp . exp*) source) "Expand to (EXP . EXP*), but with the source location replaced by the source location of SOURCE." (datum->syntax s (cons #'exp #'exp*) #:source (syntax-source #'source)))))) (define-syntax let& (lambda (s) "Like 'let', but let the inner expression have the location of the 'let&' form when it is expanded. Only a single inner expression is allowed." (syntax-case s () ((_ bindings exp) #'(let bindings (with-source-location exp s)))))) That way, 'update-package-source' doesn't need to know about the surrounding 'let' form; it would simply use 'edit-expression' as usual (though something like ,@(if (and old-commit new-commit) `((,old-commit . ,new-commit)) '()) would need to be added, and something to replace ‘(revision "N")’ with ‘(revision "N+1")’.) A complete example is attached (a.scm). The previous usages of (let ((commit ...) (revision ...)) ...) would need to be adjusted to use let& instead (build-aux/update-guix-package.scm needs to be adjusted as well). Personally, I'd go with the 'let&' form > > and (b) > > > > (define-public gnash > > (package > > (name "gnash") > > (version "0.8.11") > > (source (git-reference > > (url "https://example.org") > > (commit commit)) > > [...])) > > ? > > Is this missing a definition for commit? If it's like above, the same > applies. Or if you mean > > --8<---------------cut here---------------start------------->8--- > (source (git-reference > (url "https://example.org") > (commit "583ccbc1275c7701dc4843ec12142ff86bb305b")) > --8<---------------cut here---------------end--------------->8--- The latter. > Then that wouldn't be too hard to support. There seem to be ~136 > packages with this idiom. FWIW, the patch I sent modified 'update-package-source' to replace the commit in this case (b) (but not case (a)). > > [the patch Maxime sent] > > > > upstream-source? > > (package upstream-source-package) ;string > > (version upstream-source-version) ;string > > - (urls upstream-source-urls) ;list of strings > > + ; list of strings or a > > + (urls upstream-source-urls) > > Is it possible for an updater to want to return a list of > ? No, 'git-fetch' from (guix git-download) only accepts a single object, it doesn't support lists of . It will throw a type error if a list is passed. Compare with 'url-fetch*', which does accept a list of URLs (in which case it will fall-back to the second, the third, the fourth ... entry when the first entry gives a 404 or something). > I'm still not sure what the purpose of multiple urls > is, since nearly everthing seems to just take (first urls)... As I understand it, the second, third, fourth ... URL (when using url-fetch) are fall-backs. Also, (guix upstream) sometimes distinguishes between the different URLs, see e.g. package-update/url-fetch, which will try to choose a tarball with the same kind of extension (.zip, .tar.gz, .tar.xz, ...) as the original URI. > > (signature-urls upstream-source-signature-urls ;#f | list of strings > > (default #f)) > > (input-changes upstream-source-input-changes > > @@ -361,6 +368,11 @@ values: 'interactive' (default), 'always', and 'never'." > > system target) > > "Download SOURCE from its first URL and lower it as a fixed-output > > derivation that would fetch it." > > + (define url > > + (match (upstream-source-urls source) > > + ((first . _) first) > > + (_ (raise (formatted-message > > + (G_ "git origins are unsupported by --with-latest")))))) > > (mlet* %store-monad ((url -> (first (upstream-source-urls source))) > > (signature > > -> (and=> (upstream-source-signature-urls source) > > @@ -430,9 +442,23 @@ SOURCE, an ." > > #:key-download key-download))) > > (values version tarball source)))))) > > What is this 'upstream-source-compiler' actually used for? I couldn't > figure that out, so I just left it untouched. It is used to ‘lower’ objects. More specifically, transform-package-latest from (guix transformations) will sometimes replace the 'source' of a package with a object, and 'upstream-source-compiler' is used to turn the into a (fixed-output) derivation that can be built into a /gnu/store/...-checkout or /gnu/store/...-version.tar.gz file in the store. Greetings, Maxime