* Re: 02/03: build: go-build-system: Use WHEN for side-effect conditionals. [not found] ` <20190506000950.59FE5207E0@vcs0.savannah.gnu.org> @ 2019-05-06 1:56 ` Mark H Weaver 2019-05-06 2:50 ` Maxim Cournoyer 0 siblings, 1 reply; 4+ messages in thread From: Mark H Weaver @ 2019-05-06 1:56 UTC (permalink / raw) To: Maxim Cournoyer; +Cc: guix-devel Hi Maxim, guix-commits@gnu.org writes: > apteryx pushed a commit to branch master > in repository guix. > > commit 7e84d3eef724ef18f8e1c1b0932b6f74d3ae3e35 > Author: Maxim Cournoyer <maxim.cournoyer@gmail.com> > Date: Thu Apr 4 23:26:04 2019 -0400 > > build: go-build-system: Use WHEN for side-effect conditionals. > > * guix/build/go-build-system.scm (unpack): Replace single branch `if' by > `when'. The summary above does reflect the changes made: > diff --git a/guix/build/go-build-system.scm b/guix/build/go-build-system.scm > index 973ee6e..92a5c86 100644 > --- a/guix/build/go-build-system.scm > +++ b/guix/build/go-build-system.scm > @@ -158,6 +158,10 @@ unpacking." > ((display "WARNING: The Go import path is unset.\n"))) > (if (string-null? unpack-path) > (set! unpack-path import-path)) > + (when (string-null? import-path) > + ((display "WARNING: The Go import path is unset.\n"))) > + (when (string-null? unpack-path) > + (set! unpack-path import-path)) > (let ((dest (string-append (getenv "GOPATH") "/src/" unpack-path))) > (mkdir-p dest) > (if (file-is-directory? source) You added the 'when's, but left the 'if's, so now it's redundant. Also, please remove the extra pair of parentheses around the WARNING. If that code is ever run, it will result in an error, because it will try to call (as a procedure with no arguments) the value returned by 'display'. Thanks, Mark ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: 02/03: build: go-build-system: Use WHEN for side-effect conditionals. 2019-05-06 1:56 ` 02/03: build: go-build-system: Use WHEN for side-effect conditionals Mark H Weaver @ 2019-05-06 2:50 ` Maxim Cournoyer 0 siblings, 0 replies; 4+ messages in thread From: Maxim Cournoyer @ 2019-05-06 2:50 UTC (permalink / raw) To: Mark H Weaver; +Cc: guix-devel Hi Mark, Mark H Weaver <mhw@netris.org> writes: > Hi Maxim, > > guix-commits@gnu.org writes: > >> apteryx pushed a commit to branch master >> in repository guix. >> >> commit 7e84d3eef724ef18f8e1c1b0932b6f74d3ae3e35 >> Author: Maxim Cournoyer <maxim.cournoyer@gmail.com> >> Date: Thu Apr 4 23:26:04 2019 -0400 >> >> build: go-build-system: Use WHEN for side-effect conditionals. >> >> * guix/build/go-build-system.scm (unpack): Replace single branch `if' by >> `when'. > > The summary above does reflect the changes made: > >> diff --git a/guix/build/go-build-system.scm b/guix/build/go-build-system.scm >> index 973ee6e..92a5c86 100644 >> --- a/guix/build/go-build-system.scm >> +++ b/guix/build/go-build-system.scm >> @@ -158,6 +158,10 @@ unpacking." >> ((display "WARNING: The Go import path is unset.\n"))) >> (if (string-null? unpack-path) >> (set! unpack-path import-path)) >> + (when (string-null? import-path) >> + ((display "WARNING: The Go import path is unset.\n"))) >> + (when (string-null? unpack-path) >> + (set! unpack-path import-path)) >> (let ((dest (string-append (getenv "GOPATH") "/src/" unpack-path))) >> (mkdir-p dest) >> (if (file-is-directory? source) > > You added the 'when's, but left the 'if's, so now it's redundant. Good observation! As you observed in another reply, the removal of the IFs was recorded as part of the following commit... Sorry for the confusion! > Also, please remove the extra pair of parentheses around the WARNING. > If that code is ever run, it will result in an error, because it will > try to call (as a procedure with no arguments) the value returned by > 'display'. Fixed in 2edec51c5e. Thanks! ^ permalink raw reply [flat|nested] 4+ messages in thread
[parent not found: <20190506000950.7F51D206AB@vcs0.savannah.gnu.org>]
* Re: 03/03: build: go-build-system: Ensure uniform unpacking directory. [not found] ` <20190506000950.7F51D206AB@vcs0.savannah.gnu.org> @ 2019-05-06 2:01 ` Mark H Weaver 2019-05-06 3:09 ` Maxim Cournoyer 0 siblings, 1 reply; 4+ messages in thread From: Mark H Weaver @ 2019-05-06 2:01 UTC (permalink / raw) To: Maxim Cournoyer; +Cc: guix-devel Hello again, guix-commits@gnu.org writes: > apteryx pushed a commit to branch master > in repository guix. > > commit f42e4ebb56fe4f16991ca6c6e060c8f3535865cb > Author: Maxim Cournoyer <maxim.cournoyer@gmail.com> > Date: Fri Apr 5 00:00:08 2019 -0400 > > build: go-build-system: Ensure uniform unpacking directory. > > Depending on whether the source is a directory or an archive, we strip the > source directory or preserve it, respectively. This change makes it so that > whether the type of the source, it is unpacked at the expected location given > by the IMPORT-PATH of the Go build system. > > * guix/build/go-build-system.scm: Add the (ice-9 ftw) module. > (unpack): Add inner procedure to maybe strip the top level directory of an > archive, document it and use it. > --- > guix/build/go-build-system.scm | 36 +++++++++++++++++++++++++++--------- > 1 file changed, 27 insertions(+), 9 deletions(-) > > diff --git a/guix/build/go-build-system.scm b/guix/build/go-build-system.scm > index 92a5c86..d106e70 100644 > --- a/guix/build/go-build-system.scm > +++ b/guix/build/go-build-system.scm > @@ -1,6 +1,7 @@ > ;;; GNU Guix --- Functional package management for GNU > ;;; Copyright © 2016 Petter <petter@mykolab.ch> > ;;; Copyright © 2017, 2019 Leo Famulari <leo@famulari.name> > +;;; Copyright © 2019 Maxim Cournoyer <maxim.cournoyer@gmail.com> > ;;; > ;;; This file is part of GNU Guix. > ;;; > @@ -22,6 +23,7 @@ > #:use-module (guix build union) > #:use-module (guix build utils) > #:use-module (ice-9 match) > + #:use-module (ice-9 ftw) > #:use-module (srfi srfi-1) > #:use-module (rnrs io ports) > #:use-module (rnrs bytevectors) > @@ -151,13 +153,31 @@ dependencies, so it should be self-contained." > #t) > > (define* (unpack #:key source import-path unpack-path #:allow-other-keys) > - "Relative to $GOPATH, unpack SOURCE in the UNPACK-PATH, or the IMPORT-PATH is > -the UNPACK-PATH is unset. When SOURCE is a directory, copy it instead of > + "Relative to $GOPATH, unpack SOURCE in UNPACK-PATH, or IMPORT-PATH when > +UNPACK-PATH is unset. If the SOURCE archive has a single top level directory, > +it is stripped so that the sources appear directly under UNPACK-PATH. When > +SOURCE is a directory, copy its content into UNPACK-PATH instead of > unpacking." > - (if (string-null? import-path) > - ((display "WARNING: The Go import path is unset.\n"))) > - (if (string-null? unpack-path) > - (set! unpack-path import-path)) I see this should have been in the earlier commit, but okay. However, I have some other comments: > + (define (unpack-maybe-strip source dest) > + (let* ((scratch-dir (string-append (or (getenv "TMPDIR") "/tmp") > + "/scratch-dir")) > + (out (mkdir-p scratch-dir))) > + (with-directory-excursion scratch-dir > + (if (string-suffix? ".zip" source) > + (invoke "unzip" source) > + (invoke "tar" "-xvf" source)) > + (let ((top-level-files (remove (lambda (x) > + (member x '("." ".."))) > + (scandir ".")))) > + (match top-level-files > + ((top-level-file) > + (when (file-is-directory? top-level-file) > + (copy-recursively top-level-file dest #:keep-mtime? #t))) > + (_ > + (copy-recursively "." dest #:keep-mtime? #t))) > + #t)) > + (delete-file-recursively scratch-dir))) > + > (when (string-null? import-path) > ((display "WARNING: The Go import path is unset.\n"))) > (when (string-null? unpack-path) > @@ -168,9 +188,7 @@ unpacking." > (begin > (copy-recursively source dest #:keep-mtime? #t) > #t) > - (if (string-suffix? ".zip" source) > - (invoke "unzip" "-d" dest source) > - (invoke "tar" "-C" dest "-xvf" source))))) > + (unpack-maybe-strip source dest)))) Please make sure to return #t from this phase procedure. It did so before these changes. You might as well remove the #t from 'unpack-maybe-strip', which will be ignored anyway. Thanks, Mark ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: 03/03: build: go-build-system: Ensure uniform unpacking directory. 2019-05-06 2:01 ` 03/03: build: go-build-system: Ensure uniform unpacking directory Mark H Weaver @ 2019-05-06 3:09 ` Maxim Cournoyer 0 siblings, 0 replies; 4+ messages in thread From: Maxim Cournoyer @ 2019-05-06 3:09 UTC (permalink / raw) To: Mark H Weaver; +Cc: guix-devel Hi again :-) Mark H Weaver <mhw@netris.org> writes: > Hello again, > > guix-commits@gnu.org writes: > >> apteryx pushed a commit to branch master >> in repository guix. >> >> commit f42e4ebb56fe4f16991ca6c6e060c8f3535865cb >> Author: Maxim Cournoyer <maxim.cournoyer@gmail.com> >> Date: Fri Apr 5 00:00:08 2019 -0400 >> >> build: go-build-system: Ensure uniform unpacking directory. >> >> Depending on whether the source is a directory or an archive, we strip the >> source directory or preserve it, respectively. This change makes it so that >> whether the type of the source, it is unpacked at the expected location given >> by the IMPORT-PATH of the Go build system. >> >> * guix/build/go-build-system.scm: Add the (ice-9 ftw) module. >> (unpack): Add inner procedure to maybe strip the top level directory of an >> archive, document it and use it. >> --- >> guix/build/go-build-system.scm | 36 +++++++++++++++++++++++++++--------- >> 1 file changed, 27 insertions(+), 9 deletions(-) >> >> diff --git a/guix/build/go-build-system.scm b/guix/build/go-build-system.scm >> index 92a5c86..d106e70 100644 >> --- a/guix/build/go-build-system.scm >> +++ b/guix/build/go-build-system.scm >> @@ -1,6 +1,7 @@ >> ;;; GNU Guix --- Functional package management for GNU >> ;;; Copyright © 2016 Petter <petter@mykolab.ch> >> ;;; Copyright © 2017, 2019 Leo Famulari <leo@famulari.name> >> +;;; Copyright © 2019 Maxim Cournoyer <maxim.cournoyer@gmail.com> >> ;;; >> ;;; This file is part of GNU Guix. >> ;;; >> @@ -22,6 +23,7 @@ >> #:use-module (guix build union) >> #:use-module (guix build utils) >> #:use-module (ice-9 match) >> + #:use-module (ice-9 ftw) >> #:use-module (srfi srfi-1) >> #:use-module (rnrs io ports) >> #:use-module (rnrs bytevectors) >> @@ -151,13 +153,31 @@ dependencies, so it should be self-contained." >> #t) >> >> (define* (unpack #:key source import-path unpack-path #:allow-other-keys) >> - "Relative to $GOPATH, unpack SOURCE in the UNPACK-PATH, or the IMPORT-PATH is >> -the UNPACK-PATH is unset. When SOURCE is a directory, copy it instead of >> + "Relative to $GOPATH, unpack SOURCE in UNPACK-PATH, or IMPORT-PATH when >> +UNPACK-PATH is unset. If the SOURCE archive has a single top level directory, >> +it is stripped so that the sources appear directly under UNPACK-PATH. When >> +SOURCE is a directory, copy its content into UNPACK-PATH instead of >> unpacking." >> - (if (string-null? import-path) >> - ((display "WARNING: The Go import path is unset.\n"))) >> - (if (string-null? unpack-path) >> - (set! unpack-path import-path)) > > I see this should have been in the earlier commit, but okay. > However, I have some other comments: > >> + (define (unpack-maybe-strip source dest) >> + (let* ((scratch-dir (string-append (or (getenv "TMPDIR") "/tmp") >> + "/scratch-dir")) >> + (out (mkdir-p scratch-dir))) >> + (with-directory-excursion scratch-dir >> + (if (string-suffix? ".zip" source) >> + (invoke "unzip" source) >> + (invoke "tar" "-xvf" source)) >> + (let ((top-level-files (remove (lambda (x) >> + (member x '("." ".."))) >> + (scandir ".")))) >> + (match top-level-files >> + ((top-level-file) >> + (when (file-is-directory? top-level-file) >> + (copy-recursively top-level-file dest #:keep-mtime? #t))) >> + (_ >> + (copy-recursively "." dest #:keep-mtime? #t))) >> + #t)) >> + (delete-file-recursively scratch-dir))) >> + >> (when (string-null? import-path) >> ((display "WARNING: The Go import path is unset.\n"))) >> (when (string-null? unpack-path) >> @@ -168,9 +188,7 @@ unpacking." >> (begin >> (copy-recursively source dest #:keep-mtime? #t) >> #t) >> - (if (string-suffix? ".zip" source) >> - (invoke "unzip" "-d" dest source) >> - (invoke "tar" "-C" dest "-xvf" source))))) >> + (unpack-maybe-strip source dest)))) > > Please make sure to return #t from this phase procedure. > It did so before these changes. > > You might as well remove the #t from 'unpack-maybe-strip', which will be > ignored anyway. Another sharp observation! I've pushed a321312e3a which implements these corrections. Thank you! Maxim ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-05-06 3:09 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20190506000949.28953.40456@vcs0.savannah.gnu.org>
[not found] ` <20190506000950.59FE5207E0@vcs0.savannah.gnu.org>
2019-05-06 1:56 ` 02/03: build: go-build-system: Use WHEN for side-effect conditionals Mark H Weaver
2019-05-06 2:50 ` Maxim Cournoyer
[not found] ` <20190506000950.7F51D206AB@vcs0.savannah.gnu.org>
2019-05-06 2:01 ` 03/03: build: go-build-system: Ensure uniform unpacking directory Mark H Weaver
2019-05-06 3:09 ` Maxim Cournoyer
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/guix.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).