From mboxrd@z Thu Jan 1 00:00:00 1970 From: Maxim Cournoyer Subject: Re: 03/03: build: go-build-system: Ensure uniform unpacking directory. Date: Sun, 05 May 2019 23:09:20 -0400 Message-ID: <87imuocm8v.fsf@gmail.com> References: <20190506000949.28953.40456@vcs0.savannah.gnu.org> <20190506000950.7F51D206AB@vcs0.savannah.gnu.org> <87y33kqr2d.fsf@netris.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([209.51.188.92]:56284) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hNU0K-0003M8-9I for guix-devel@gnu.org; Sun, 05 May 2019 23:09:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hNU0J-0005zr-32 for guix-devel@gnu.org; Sun, 05 May 2019 23:09:24 -0400 Received: from mail-qk1-x742.google.com ([2607:f8b0:4864:20::742]:39922) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hNU0I-0005zN-TS for guix-devel@gnu.org; Sun, 05 May 2019 23:09:23 -0400 Received: by mail-qk1-x742.google.com with SMTP id z128so4441465qkb.6 for ; Sun, 05 May 2019 20:09:22 -0700 (PDT) In-Reply-To: <87y33kqr2d.fsf@netris.org> (Mark H. Weaver's message of "Sun, 05 May 2019 22:01:03 -0400") List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: Mark H Weaver Cc: guix-devel@gnu.org Hi again :-) Mark H Weaver writes: > Hello again, > > guix-commits@gnu.org writes: > >> apteryx pushed a commit to branch master >> in repository guix. >> >> commit f42e4ebb56fe4f16991ca6c6e060c8f3535865cb >> Author: Maxim Cournoyer >> Date: Fri Apr 5 00:00:08 2019 -0400 >> >> build: go-build-system: Ensure uniform unpacking directory. >>=20=20=20=20=20 >> Depending on whether the source is a directory or an archive, we str= ip 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 locat= ion given >> by the IMPORT-PATH of the Go build system. >>=20=20=20=20=20 >> * 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 =C2=A9 2016 Petter >> ;;; Copyright =C2=A9 2017, 2019 Leo Famulari >> +;;; Copyright =C2=A9 2019 Maxim Cournoyer >> ;;; >> ;;; 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) >>=20=20 >> (define* (unpack #:key source import-path unpack-path #:allow-other-key= s) >> - "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 wh= en >> +UNPACK-PATH is unset. If the SOURCE archive has a single top level dir= ectory, >> +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 co= rrections. Thank you! Maxim