Package: guix-patches;
Reported by: Romain GARBAGE <romain.garbage <at> inria.fr>
Date: Thu, 24 Apr 2025 13:54:02 UTC
Severity: normal
Tags: patch
To reply to this bug, email your comments to 78034 AT debbugs.gnu.org.
Toggle the display of automated, internal messages from the tracker.
View this report as an mbox folder, status mbox, maintainer mbox
guix <at> cbaines.net, dev <at> jpoiret.xyz, ludo <at> gnu.org, othacehe <at> gnu.org, maxim.cournoyer <at> gmail.com, zimon.toutoune <at> gmail.com, me <at> tobias.gr, guix-patches <at> gnu.org:bug#78034; Package guix-patches.
(Thu, 24 Apr 2025 13:54:02 GMT) Full text and rfc822 format available.Romain GARBAGE <romain.garbage <at> inria.fr>:guix <at> cbaines.net, dev <at> jpoiret.xyz, ludo <at> gnu.org, othacehe <at> gnu.org, maxim.cournoyer <at> gmail.com, zimon.toutoune <at> gmail.com, me <at> tobias.gr, guix-patches <at> gnu.org.
(Thu, 24 Apr 2025 13:54:02 GMT) Full text and rfc822 format available.Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Romain GARBAGE <romain.garbage <at> inria.fr> To: guix-patches <at> gnu.org Cc: ludovic.courtes <at> inria.fr, Romain GARBAGE <romain.garbage <at> inria.fr> Subject: [PATCH] guix: transformations: git source transformations honour RECURSIVE?. Date: Thu, 24 Apr 2025 15:52:39 +0200
* guix/transformations.scm (package-git-url+recursive?): New variable.
(package-git-url): Remove variable.
(evaluate-git-replacement-specs): Use package-git-url+recursive?.
(transform-package-source-branch, transform-package-source-commit, transform-package-source-git-url): Update
according to changes above.
* doc/guix.texi (Package Transformation Options): Update documentation.
* tests/transformations.scm: Update tests.
Change-Id: Id6a5e6957a9955c8173b06b3e14f2986c6dfc4bc
---
doc/guix.texi | 8 ++++---
guix/transformations.scm | 50 +++++++++++++++++++++++++--------------
tests/transformations.scm | 6 ++---
3 files changed, 39 insertions(+), 25 deletions(-)
diff --git a/doc/guix.texi b/doc/guix.texi
index 32ef844f5c..3d5a1238a6 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -13584,9 +13584,11 @@ Package Transformation Options
@item --with-git-url=@var{package}=@var{url}
@cindex Git, using the latest commit
@cindex latest commit, building
-Build @var{package} from the latest commit of the @code{master} branch of the
-Git repository at @var{url}. Git sub-modules of the repository are fetched,
-recursively.
+Build @var{package} from the latest commit of the @code{master} branch
+of the Git repository at @var{url}. Git sub-modules of the repository
+are fetched, recursively, if @var{package} @code{source} is not a Git
+repository, otherwise it depends on the inherited value of
+@code{recursive?}.
For example, the following command builds the NumPy Python library against the
latest commit of the master branch of Python itself:
diff --git a/guix/transformations.scm b/guix/transformations.scm
index 2887d91a34..7715373c43 100644
--- a/guix/transformations.scm
+++ b/guix/transformations.scm
@@ -29,8 +29,13 @@ (define-module (guix transformations)
#:use-module (guix profiles)
#:use-module (guix diagnostics)
#:autoload (guix download) (download-to-store)
- #:autoload (guix git-download) (git-reference? git-reference-url)
- #:autoload (guix git) (git-checkout git-checkout? git-checkout-url)
+ #:autoload (guix git-download) (git-reference?
+ git-reference-url
+ git-reference-recursive?)
+ #:autoload (guix git) (git-checkout
+ git-checkout?
+ git-checkout-url
+ git-checkout-recursive?)
#:autoload (guix upstream) (upstream-source
package-latest-release
preferred-upstream-source
@@ -234,15 +239,18 @@ (define (transform-package-inputs/graft replacement-specs)
(define %not-equal
(char-set-complement (char-set #\=)))
-(define (package-git-url package)
- "Return the URL of the Git repository for package, or raise an error if
-the source of PACKAGE is not fetched from a Git repository."
+(define (package-git-url+recursive? package)
+ "Return two values: the URL of the Git repository for package and a boolean
+indicating if the repository has to be recursively cloned, or raise an error if the
+source of PACKAGE is not fetched from a Git repository."
(let ((source (package-source package)))
(cond ((and (origin? source)
(git-reference? (origin-uri source)))
- (git-reference-url (origin-uri source)))
+ (values (git-reference-url (origin-uri source))
+ (git-reference-recursive? (origin-uri source))))
((git-checkout? source)
- (git-checkout-url source))
+ (values (git-checkout-url source)
+ (git-checkout-recursive? source)))
(else
(raise
(formatted-message (G_ "the source of ~a is not a Git reference")
@@ -257,9 +265,9 @@ (define (evaluate-git-replacement-specs specs proc)
(match (string-tokenize spec %not-equal)
((spec branch-or-commit)
(define (replace old)
- (let* ((source (package-source old))
- (url (package-git-url old)))
- (proc old url branch-or-commit)))
+ (let* ((source (package-source old))
+ (url recursive? (package-git-url+recursive? old)))
+ (proc old url branch-or-commit recursive?)))
(cons spec replace))
(_
@@ -273,7 +281,7 @@ (define (transform-package-source-branch replacement-specs)
dependencies according to REPLACEMENT-SPECS. REPLACEMENT-SPECS is a list of
strings like \"guile-next=stable-3.0\" meaning that packages are built using
'guile-next' from the latest commit on its 'stable-3.0' branch."
- (define (replace old url branch)
+ (define (replace old url branch recursive?)
(package
(inherit old)
(version (string-append "git." (string-map (match-lambda
@@ -281,7 +289,7 @@ (define (transform-package-source-branch replacement-specs)
(chr chr))
branch)))
(source (git-checkout (url url) (branch branch)
- (recursive? #t)))))
+ (recursive? recursive?)))))
(let* ((replacements (evaluate-git-replacement-specs replacement-specs
replace))
@@ -315,12 +323,12 @@ (define (transform-package-source-commit replacement-specs)
dependencies according to REPLACEMENT-SPECS. REPLACEMENT-SPECS is a list of
strings like \"guile-next=cabba9e\" meaning that packages are built using
'guile-next' from commit 'cabba9e'."
- (define (replace old url commit)
+ (define (replace old url commit recursive?)
(package
(inherit old)
(version (commit->version-string commit))
(source (git-checkout (url url) (commit commit)
- (recursive? #t)))))
+ (recursive? recursive?)))))
(let* ((replacements (evaluate-git-replacement-specs replacement-specs
replace))
@@ -341,10 +349,16 @@ (define (transform-package-source-git-url replacement-specs)
((spec url)
(cons spec
(lambda (old)
- (package
- (inherit old)
- (source (git-checkout (url url)
- (recursive? #t)))))))
+ ;; Try to propagate RECURSIVE? from the package source when it
+ ;; is a git-checkout or a git-reference, keeping TRUE as
+ ;; default in other cases.
+ (let ((url recursive? (guard (c ((formatted-message? c )
+ (values url #t)))
+ (package-git-url+recursive? old))))
+ (package
+ (inherit old)
+ (source (git-checkout (url url)
+ (recursive? recursive?))))))))
(_
(raise
(formatted-message
diff --git a/tests/transformations.scm b/tests/transformations.scm
index 5285d98f17..844dac43fc 100644
--- a/tests/transformations.scm
+++ b/tests/transformations.scm
@@ -217,8 +217,7 @@ (define-module (test-transformations)
(test-equal "options->transformation, with-branch"
(git-checkout (url "https://example.org")
- (branch "devel")
- (recursive? #t))
+ (branch "devel"))
(let* ((p (dummy-package "guix.scm"
(inputs `(("foo" ,grep)
("bar" ,(dummy-package "chbouib"
@@ -240,8 +239,7 @@ (define-module (test-transformations)
(test-equal "options->transformation, with-commit"
(git-checkout (url "https://example.org")
- (commit "abcdef")
- (recursive? #t))
+ (commit "abcdef"))
(let* ((p (dummy-package "guix.scm"
(inputs `(("foo" ,grep)
("bar" ,(dummy-package "chbouib"
base-commit: b12d44dd5e35ac236bf3fbb5619b9c8c2f42c902
--
2.49.0
guix-patches <at> gnu.org:bug#78034; Package guix-patches.
(Thu, 24 Apr 2025 15:51:06 GMT) Full text and rfc822 format available.Message #8 received at 78034 <at> debbugs.gnu.org (full text, mbox):
From: Ludovic Courtès <ludovic.courtes <at> inria.fr> To: Romain GARBAGE <romain.garbage <at> inria.fr> Cc: Josselin Poiret <dev <at> jpoiret.xyz>, Maxim Cournoyer <maxim.cournoyer <at> gmail.com>, Simon Tournier <zimon.toutoune <at> gmail.com>, 78034 <at> debbugs.gnu.org, Tobias Geerinckx-Rice <me <at> tobias.gr>, ludovic.courtes <at> inria.fr, Mathieu Othacehe <othacehe <at> gnu.org>, Christopher Baines <guix <at> cbaines.net> Subject: Re: [bug#78034] [PATCH] guix: transformations: git source transformations honour RECURSIVE?. Date: Thu, 24 Apr 2025 16:33:35 +0200
Hello! Overall LGTM! Minor suggestions: Romain GARBAGE <romain.garbage <at> inria.fr> writes: > + ;; Try to propagate RECURSIVE? from the package source when it > + ;; is a git-checkout or a git-reference, keeping TRUE as > + ;; default in other cases. > + (let ((url recursive? (guard (c ((formatted-message? c ) > + (values url #t))) > + (package-git-url+recursive? old)))) Instead of catching ‘formatted-message?’, I would rather add an ‘if’ for origin + git-reference or git-checkout. > --- a/tests/transformations.scm > +++ b/tests/transformations.scm > @@ -217,8 +217,7 @@ (define-module (test-transformations) > > (test-equal "options->transformation, with-branch" > (git-checkout (url "https://example.org") > - (branch "devel") > - (recursive? #t)) > + (branch "devel")) IWBN to add a test where (recursive? #t) is inherited. Thanks, Ludo’.
guix <at> cbaines.net, dev <at> jpoiret.xyz, ludo <at> gnu.org, othacehe <at> gnu.org, maxim.cournoyer <at> gmail.com, zimon.toutoune <at> gmail.com, me <at> tobias.gr, guix-patches <at> gnu.org:bug#78034; Package guix-patches.
(Fri, 25 Apr 2025 08:33:02 GMT) Full text and rfc822 format available.Message #11 received at 78034 <at> debbugs.gnu.org (full text, mbox):
From: Romain GARBAGE <romain.garbage <at> inria.fr> To: 78034 <at> debbugs.gnu.org Cc: ludovic.courtes <at> inria.fr, Romain GARBAGE <romain.garbage <at> inria.fr> Subject: [PATCH v2] guix: transformations: git source transformations honour RECURSIVE?. Date: Fri, 25 Apr 2025 10:32:20 +0200
* guix/transformations.scm (package-git-url+recursive?): New variable.
(package-git-url): Remove variable.
(evaluate-git-replacement-specs): Use package-git-url+recursive?.
(transform-package-source-branch, transform-package-source-commit, transform-package-source-git-url): Update
according to changes above.
* doc/guix.texi (Package Transformation Options): Update documentation.
* tests/transformations.scm: Update tests. Add tests for RECURSIVE?
inheritance with WITH-COMMIT and WITH-SOURCE.
Change-Id: Id6a5e6957a9955c8173b06b3e14f2986c6dfc4bc
---
doc/guix.texi | 8 +++---
guix/transformations.scm | 53 ++++++++++++++++++++++++++-------------
tests/transformations.scm | 52 +++++++++++++++++++++++++++++++++++---
3 files changed, 89 insertions(+), 24 deletions(-)
diff --git a/doc/guix.texi b/doc/guix.texi
index 32ef844f5c..3d5a1238a6 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -13584,9 +13584,11 @@ Package Transformation Options
@item --with-git-url=@var{package}=@var{url}
@cindex Git, using the latest commit
@cindex latest commit, building
-Build @var{package} from the latest commit of the @code{master} branch of the
-Git repository at @var{url}. Git sub-modules of the repository are fetched,
-recursively.
+Build @var{package} from the latest commit of the @code{master} branch
+of the Git repository at @var{url}. Git sub-modules of the repository
+are fetched, recursively, if @var{package} @code{source} is not a Git
+repository, otherwise it depends on the inherited value of
+@code{recursive?}.
For example, the following command builds the NumPy Python library against the
latest commit of the master branch of Python itself:
diff --git a/guix/transformations.scm b/guix/transformations.scm
index 2887d91a34..19a1cba206 100644
--- a/guix/transformations.scm
+++ b/guix/transformations.scm
@@ -29,8 +29,13 @@ (define-module (guix transformations)
#:use-module (guix profiles)
#:use-module (guix diagnostics)
#:autoload (guix download) (download-to-store)
- #:autoload (guix git-download) (git-reference? git-reference-url)
- #:autoload (guix git) (git-checkout git-checkout? git-checkout-url)
+ #:autoload (guix git-download) (git-reference?
+ git-reference-url
+ git-reference-recursive?)
+ #:autoload (guix git) (git-checkout
+ git-checkout?
+ git-checkout-url
+ git-checkout-recursive?)
#:autoload (guix upstream) (upstream-source
package-latest-release
preferred-upstream-source
@@ -234,15 +239,18 @@ (define (transform-package-inputs/graft replacement-specs)
(define %not-equal
(char-set-complement (char-set #\=)))
-(define (package-git-url package)
- "Return the URL of the Git repository for package, or raise an error if
-the source of PACKAGE is not fetched from a Git repository."
+(define (package-git-url+recursive? package)
+ "Return two values: the URL of the Git repository for package and a boolean
+indicating if the repository has to be recursively cloned, or raise an error if the
+source of PACKAGE is not fetched from a Git repository."
(let ((source (package-source package)))
(cond ((and (origin? source)
(git-reference? (origin-uri source)))
- (git-reference-url (origin-uri source)))
+ (values (git-reference-url (origin-uri source))
+ (git-reference-recursive? (origin-uri source))))
((git-checkout? source)
- (git-checkout-url source))
+ (values (git-checkout-url source)
+ (git-checkout-recursive? source)))
(else
(raise
(formatted-message (G_ "the source of ~a is not a Git reference")
@@ -257,9 +265,9 @@ (define (evaluate-git-replacement-specs specs proc)
(match (string-tokenize spec %not-equal)
((spec branch-or-commit)
(define (replace old)
- (let* ((source (package-source old))
- (url (package-git-url old)))
- (proc old url branch-or-commit)))
+ (let* ((source (package-source old))
+ (url recursive? (package-git-url+recursive? old)))
+ (proc old url branch-or-commit recursive?)))
(cons spec replace))
(_
@@ -273,7 +281,7 @@ (define (transform-package-source-branch replacement-specs)
dependencies according to REPLACEMENT-SPECS. REPLACEMENT-SPECS is a list of
strings like \"guile-next=stable-3.0\" meaning that packages are built using
'guile-next' from the latest commit on its 'stable-3.0' branch."
- (define (replace old url branch)
+ (define (replace old url branch recursive?)
(package
(inherit old)
(version (string-append "git." (string-map (match-lambda
@@ -281,7 +289,7 @@ (define (transform-package-source-branch replacement-specs)
(chr chr))
branch)))
(source (git-checkout (url url) (branch branch)
- (recursive? #t)))))
+ (recursive? recursive?)))))
(let* ((replacements (evaluate-git-replacement-specs replacement-specs
replace))
@@ -315,12 +323,12 @@ (define (transform-package-source-commit replacement-specs)
dependencies according to REPLACEMENT-SPECS. REPLACEMENT-SPECS is a list of
strings like \"guile-next=cabba9e\" meaning that packages are built using
'guile-next' from commit 'cabba9e'."
- (define (replace old url commit)
+ (define (replace old url commit recursive?)
(package
(inherit old)
(version (commit->version-string commit))
(source (git-checkout (url url) (commit commit)
- (recursive? #t)))))
+ (recursive? recursive?)))))
(let* ((replacements (evaluate-git-replacement-specs replacement-specs
replace))
@@ -341,10 +349,19 @@ (define (transform-package-source-git-url replacement-specs)
((spec url)
(cons spec
(lambda (old)
- (package
- (inherit old)
- (source (git-checkout (url url)
- (recursive? #t)))))))
+ ;; Propagate RECURSIVE? from the package source when it is a
+ ;; git-checkout or a git-reference, keeping TRUE as default in
+ ;; other cases.
+ (let* ((uri (and (origin? (package-source old))
+ (origin-uri (package-source old))))
+ (recursive? (if (or (git-checkout? uri)
+ (git-reference? uri))
+ (package-git-url+recursive? old)
+ #t)))
+ (package
+ (inherit old)
+ (source (git-checkout (url url)
+ (recursive? recursive?))))))))
(_
(raise
(formatted-message
diff --git a/tests/transformations.scm b/tests/transformations.scm
index 5285d98f17..9f5aacc41b 100644
--- a/tests/transformations.scm
+++ b/tests/transformations.scm
@@ -217,8 +217,7 @@ (define-module (test-transformations)
(test-equal "options->transformation, with-branch"
(git-checkout (url "https://example.org")
- (branch "devel")
- (recursive? #t))
+ (branch "devel"))
(let* ((p (dummy-package "guix.scm"
(inputs `(("foo" ,grep)
("bar" ,(dummy-package "chbouib"
@@ -238,7 +237,53 @@ (define-module (test-transformations)
(string=? (package-name dep2) "chbouib")
(package-source dep2))))))))
+(test-equal "options->transformation, with-branch, recursive? inheritance"
+ (git-checkout (url "https://example.org")
+ (branch "devel")
+ (recursive? #t))
+ (let* ((p (dummy-package "guix.scm"
+ (inputs `(("foo" ,grep)
+ ("bar" ,(dummy-package "chbouib"
+ (source (origin
+ (method git-fetch)
+ (uri (git-reference
+ (url "https://example.org")
+ (commit "cabba9e")
+ (recursive? #t)))
+ (sha256 #f)))))))))
+ (t (options->transformation '((with-branch . "chbouib=devel")))))
+ (let ((new (t p)))
+ (and (not (eq? new p))
+ (match (package-inputs new)
+ ((("foo" dep1) ("bar" dep2))
+ (and (string=? (package-full-name dep1)
+ (package-full-name grep))
+ (string=? (package-name dep2) "chbouib")
+ (package-source dep2))))))))
+
(test-equal "options->transformation, with-commit"
+ (git-checkout (url "https://example.org")
+ (commit "abcdef"))
+ (let* ((p (dummy-package "guix.scm"
+ (inputs `(("foo" ,grep)
+ ("bar" ,(dummy-package "chbouib"
+ (source (origin
+ (method git-fetch)
+ (uri (git-reference
+ (url "https://example.org")
+ (commit "cabba9e")))
+ (sha256 #f)))))))))
+ (t (options->transformation '((with-commit . "chbouib=abcdef")))))
+ (let ((new (t p)))
+ (and (not (eq? new p))
+ (match (package-inputs new)
+ ((("foo" dep1) ("bar" dep2))
+ (and (string=? (package-full-name dep1)
+ (package-full-name grep))
+ (string=? (package-name dep2) "chbouib")
+ (package-source dep2))))))))
+
+(test-equal "options->transformation, with-commit, recursive? inheritance"
(git-checkout (url "https://example.org")
(commit "abcdef")
(recursive? #t))
@@ -249,7 +294,8 @@ (define-module (test-transformations)
(method git-fetch)
(uri (git-reference
(url "https://example.org")
- (commit "cabba9e")))
+ (commit "cabba9e")
+ (recursive? #t)))
(sha256 #f)))))))))
(t (options->transformation '((with-commit . "chbouib=abcdef")))))
(let ((new (t p)))
base-commit: b12d44dd5e35ac236bf3fbb5619b9c8c2f42c902
--
2.49.0
guix-patches <at> gnu.org:bug#78034; Package guix-patches.
(Fri, 25 Apr 2025 08:35:02 GMT) Full text and rfc822 format available.Message #14 received at 78034 <at> debbugs.gnu.org (full text, mbox):
From: Romain Garbage <romain.garbage <at> inria.fr> To: Ludovic Courtès <ludovic.courtes <at> inria.fr> Cc: Josselin Poiret <dev <at> jpoiret.xyz>, Maxim Cournoyer <maxim.cournoyer <at> gmail.com>, Simon Tournier <zimon.toutoune <at> gmail.com>, 78034 <at> debbugs.gnu.org, Tobias Geerinckx-Rice <me <at> tobias.gr>, Mathieu Othacehe <othacehe <at> gnu.org>, Christopher Baines <guix <at> cbaines.net> Subject: Re: [bug#78034] [PATCH] guix: transformations: git source transformations honour RECURSIVE?. Date: Fri, 25 Apr 2025 10:34:39 +0200
Ludovic Courtès <ludovic.courtes <at> inria.fr> writes: > Hello! Hi Ludo' > Overall LGTM! Minor suggestions: > > Romain GARBAGE <romain.garbage <at> inria.fr> writes: > >> + ;; Try to propagate RECURSIVE? from the package source when it >> + ;; is a git-checkout or a git-reference, keeping TRUE as >> + ;; default in other cases. >> + (let ((url recursive? (guard (c ((formatted-message? c ) >> + (values url #t))) >> + (package-git-url+recursive? old)))) > > Instead of catching ‘formatted-message?’, I would rather add an ‘if’ for > origin + git-reference or git-checkout. Done in v2. >> --- a/tests/transformations.scm >> +++ b/tests/transformations.scm >> @@ -217,8 +217,7 @@ (define-module (test-transformations) >> >> (test-equal "options->transformation, with-branch" >> (git-checkout (url "https://example.org") >> - (branch "devel") >> - (recursive? #t)) >> + (branch "devel")) > > IWBN to add a test where (recursive? #t) is inherited. Done in v2: I added 2 tests, one form WITH-COMMIT and one for WITH-BRANCH. Romain
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.