| Message ID | d811c6461c7d653834c6a04290cacda5a72680b2.1717566375.git.mail@nicolasgoaziou.fr |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#71371] gnu: svn-fetch: Make revision field optional. | expand |
Hello! Nicolas Goaziou <mail@nicolasgoaziou.fr> skribis: > -@item @code{revision} > -This string denotes revision to fetch specified as a number. > +@item @code{revision} (default: @code{#f}) > +This field denotes the revision to fetch, as a number. It can also be > +set to @code{#f}, for example when @var{url} contains a tag reference. Hmm, IIRC, tags in svn are mutable, no? My recollection is that there’s no distinction between a directory, a branch, and a tag: tags and branches are just a copy (‘svn cp’) of a directory that can change over time. Thus, you can’t rely on a tag as an unambiguous reference. Am I right? Thanks, Ludo’.
Hi Nicolas, On Wed, 05 Jun 2024 at 07:52, Nicolas Goaziou via Guix-patches via <guix-patches@gnu.org> wrote: > * guix/svn-download.scm (<svn-reference>): Set default value for REVISION > field to #F. > (svn-fetch): > (svn-multi-fetch): Take into consideration the revision can be a number or #F. > * guix/build/svn.scm (svn-fetch): Skip "-r" argument when revision is #F. > * doc/guix.texi (origin Reference): Document changes about REVISION field. Does it make sense to have a Subversion reference without any specific revision? > - (string->number (getenv "svn revision")) > + (match (getenv "svn revision") > + ("#f" #f) > + (s (string->number s))) [...] > - (string->number (getenv "svn revision")) > + (match (getenv "svn revision") > + ("#f" #f) > + (s (string-to-number s))) I am probably missing something, why ’string-to-number’ and not ’string->number’? Cheers, simon
Hello, Ludovic Courtès <ludo@gnu.org> writes: > Nicolas Goaziou <mail@nicolasgoaziou.fr> skribis: > >> -@item @code{revision} >> -This string denotes revision to fetch specified as a number. >> +@item @code{revision} (default: @code{#f}) >> +This field denotes the revision to fetch, as a number. It can also be >> +set to @code{#f}, for example when @var{url} contains a tag reference. > > Hmm, IIRC, tags in svn are mutable, no? Disclaimer: prior to this patch, I knew nothing about SVN. So feel free to take my answers with a grain of salt. Now to answer your question, yes, they are mutable. But in practice, altering tagged contents seems to be frowned upon in the trunk-branch-tag trilogy. > My recollection is that there’s no distinction between a directory, a > branch, and a tag: tags and branches are just a copy (‘svn cp’) of a > directory that can change over time. Thus, you can’t rely on a tag as > an unambiguous reference. > > Am I right? You are certainly right, but I think this patch still makes sense for projects that swear to never change tagged contents, which could possibly be the case for most of the projects using SVN. Every now and then, we will encounter a project that does modify such contents, but it also currently happens with tarballs: sometimes, upstream modifies the contents of a tarball without changing its name, inducing a hash mismatch. My concern here is that some Guix packages relying on `svn-fetch' provide both a tag and a revision, which is redundant. We could get rid of the tag, but the revision is not human-friendly. Regards,
diff --git a/doc/guix.texi b/doc/guix.texi index 9b60e6c603..5e1173b8c6 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -8263,8 +8263,9 @@ origin Reference @item @code{url} The URL of the Subversion repository to clone. -@item @code{revision} -This string denotes revision to fetch specified as a number. +@item @code{revision} (default: @code{#f}) +This field denotes the revision to fetch, as a number. It can also be +set to @code{#f}, for example when @var{url} contains a tag reference. @item @code{recursive?} (default: @code{#f}) This Boolean indicates whether to recursively fetch Subversion diff --git a/guix/build/svn.scm b/guix/build/svn.scm index 875d3c50ca..3ae6519628 100644 --- a/guix/build/svn.scm +++ b/guix/build/svn.scm @@ -46,8 +46,7 @@ (define* (svn-fetch url revision directory ;; Trust the server certificate. This is OK as we ;; verify the checksum later. This can be removed when ;; ca-certificates package is added. - "--trust-server-cert" "-r" (number->string revision) - + "--trust-server-cert" ;; Disable keyword substitutions (keywords are CVS-like strings ;; like "$Date$", "$Id$", and so on) for two reasons: (1) some ;; expansions depend on the local time zone, and (2) SWH disables @@ -61,6 +60,9 @@ (define* (svn-fetch url revision directory ,@(if recursive? '() (list "--ignore-externals")) + ,@(if revision + (list "-r" (number->string revision)) + '()) ,url ,directory)) #t)) diff --git a/guix/svn-download.scm b/guix/svn-download.scm index bdd9c39eb5..dc9856cfb1 100644 --- a/guix/svn-download.scm +++ b/guix/svn-download.scm @@ -63,7 +63,7 @@ (define-record-type* <svn-reference> svn-reference make-svn-reference svn-reference? (url svn-reference-url) ; string - (revision svn-reference-revision) ; number + (revision svn-reference-revision (default #f)) ; number or #f (recursive? svn-reference-recursive? (default #f)) (user-name svn-reference-user-name (default #f)) (password svn-reference-password (default #f))) @@ -120,7 +120,9 @@ (define* (svn-fetch ref hash-algo hash (or (and (download-method-enabled? 'upstream) (svn-fetch (getenv "svn url") - (string->number (getenv "svn revision")) + (match (getenv "svn revision") + ("#f" #f) + (s (string->number s))) #$output #:svn-command #+(file-append svn "/bin/svn") #:recursive? (match (getenv "svn recursive?") @@ -145,7 +147,7 @@ (define* (svn-fetch ref hash-algo hash #:env-vars `(("svn url" . ,(svn-reference-url ref)) ("svn revision" - . ,(number->string (svn-reference-revision ref))) + . ,(object->string (svn-reference-revision ref))) ,@(if (svn-reference-recursive? ref) `(("svn recursive?" . "yes")) '()) @@ -173,7 +175,7 @@ (define-record-type* <svn-multi-reference> svn-multi-reference make-svn-multi-reference svn-multi-reference? (url svn-multi-reference-url) ; string - (revision svn-multi-reference-revision) ; number + (revision svn-multi-reference-revision) ; number or #f (locations svn-multi-reference-locations) ; list of strings (recursive? svn-multi-reference-recursive? (default #f)) (user-name svn-multi-reference-user-name (default #f)) @@ -233,7 +235,9 @@ (define* (svn-multi-fetch ref hash-algo hash (mkdir-p (string-append #$output "/" (dirname location)))) (and (download-method-enabled? 'upstream) (svn-fetch (string-append (getenv "svn url") "/" location) - (string->number (getenv "svn revision")) + (match (getenv "svn revision") + ("#f" #f) + (s (string-to-number s))) (if (string-suffix? "/" location) (string-append #$output "/" location) (string-append #$output "/" (dirname location))) @@ -271,7 +275,7 @@ (define* (svn-multi-fetch ref hash-algo hash ("svn locations" . ,(object->string (svn-multi-reference-locations ref))) ("svn revision" - . ,(number->string (svn-multi-reference-revision ref))) + . ,(object->string (svn-multi-reference-revision ref))) ,@(if (svn-multi-reference-recursive? ref) `(("svn recursive?" . "yes")) '())