From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp0 ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id SC6+OfwTAmAMUQAA0tVLHw (envelope-from ) for ; Fri, 15 Jan 2021 22:15:24 +0000 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp0 with LMTPS id gMmKNfwTAmBzNAAA1q6Kng (envelope-from ) for ; Fri, 15 Jan 2021 22:15:24 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 27DBC9403D5 for ; Fri, 15 Jan 2021 22:15:23 +0000 (UTC) Received: from localhost ([::1]:44290 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1l0XNK-0004gt-Mz for larch@yhetil.org; Fri, 15 Jan 2021 17:15:22 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:45636) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1l0XN0-0004f3-Mq for guix-patches@gnu.org; Fri, 15 Jan 2021 17:15:03 -0500 Received: from debbugs.gnu.org ([209.51.188.43]:58600) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1l0XN0-0002Fc-FG for guix-patches@gnu.org; Fri, 15 Jan 2021 17:15:02 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1l0XN0-0001rd-Ae for guix-patches@gnu.org; Fri, 15 Jan 2021 17:15:02 -0500 X-Loop: help-debbugs@gnu.org Subject: [bug#45891] [PATCH] packages: 'patch-and-repack' returns a directory when given a directory. Resent-From: Maxim Cournoyer Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Fri, 15 Jan 2021 22:15:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 45891 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Ludovic =?UTF-8?Q?Court=C3=A8s?= Received: via spool by 45891-submit@debbugs.gnu.org id=B45891.16107488817126 (code B ref 45891); Fri, 15 Jan 2021 22:15:02 +0000 Received: (at 45891) by debbugs.gnu.org; 15 Jan 2021 22:14:41 +0000 Received: from localhost ([127.0.0.1]:41913 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1l0XMf-0001qr-6P for submit@debbugs.gnu.org; Fri, 15 Jan 2021 17:14:41 -0500 Received: from mail-qk1-f169.google.com ([209.85.222.169]:44192) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1l0XMd-0001qe-SZ for 45891@debbugs.gnu.org; Fri, 15 Jan 2021 17:14:40 -0500 Received: by mail-qk1-f169.google.com with SMTP id v126so13224866qkd.11 for <45891@debbugs.gnu.org>; Fri, 15 Jan 2021 14:14:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version:content-transfer-encoding; bh=eLnVEG1Qsced4dYQ+Ap9iMwDjBK5tWRuQM1IS/tKagU=; b=Br9+uoeVQBLGHruBQjcx1ZiitRzsSUHVq0ykQRqEtUlfkGSV5hCQxH8xRdn2b8i9LJ XQZxghdMjxe+MPQHkCtT2h0KKrbj5F+FNN5eMYEm2+t9iC+vePMUkqRRK0RZrDDPOT/v 286OF1WfLzp8v+3R9UZmOCEYNo1KOz0/29BCoFhu2jNYow06brUbEDspL96Cnvmk6vIo 99l1RHPM9v03oAQw73AZNTcuJG4TW+NwOFQdK/5VdjVSFGYZnBswfMJsVfNAIGs6DY6X x3z49PVg5S5nS4tmWY4V5aV+gFlr1+jnS2aJ+Y6Yf9a7jzH4H79PNJpdwcQfmkBn4s0p rzFA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=eLnVEG1Qsced4dYQ+Ap9iMwDjBK5tWRuQM1IS/tKagU=; b=KSmqCp+QFuNQh2EKL6Vj73rGuxb5fQ/5W/4hpUqYjcljnS5FzXlWpTSsiJQdXb/LcX tePGBThV3EpiHw+v6byIystRyag5VQBkbKzeE/MU2M/L7oh+7wWv86gkJtaAs9QUEhRv zD0bSlBpVFCpKwLL5KAU0htN8pRCeZzZ+q7+g3DeK6LR/KFGZ2gIc8jcPFEV6UalWlOC Wo8zC1RK8BqXw6sezZ3GZWJSWTzHhfAm8NS3mcp3D7J+ScbJwpk0Z1AtFfhu6qe9kjRc st72n5h4b7colTzsEVfCetQL7y01ZEuLnKS7FV8QKLR11GpomfaEFEBxoatc7KbA9YnG QPrQ== X-Gm-Message-State: AOAM533cwa+p3gnYrFi0xhBvT4W9qAXYWQfqXzr7e5EoKG9CQJh9gAYY 1QZpfU39n7C0dsF5K/hg0BD1xygeh5Y= X-Google-Smtp-Source: ABdhPJyIBCX+T1ghSudwMAoD0bEjlAnrHsyXyTqAiV9raim9YAIjfMYvT2iRgiQIfmkH+LlW9RabaQ== X-Received: by 2002:a37:a984:: with SMTP id s126mr14550626qke.407.1610748874100; Fri, 15 Jan 2021 14:14:34 -0800 (PST) Received: from hurd (dsl-205-233-125-239.b2b2c.ca. [205.233.125.239]) by smtp.gmail.com with ESMTPSA id d1sm5970346qkf.102.2021.01.15.14.14.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 15 Jan 2021 14:14:33 -0800 (PST) From: Maxim Cournoyer References: <20210115131548.8792-1-ludo@gnu.org> Date: Fri, 15 Jan 2021 17:14:32 -0500 In-Reply-To: <20210115131548.8792-1-ludo@gnu.org> ("Ludovic =?UTF-8?Q?Court=C3=A8s?="'s message of "Fri, 15 Jan 2021 14:15:48 +0100") Message-ID: <878s8tluev.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: 45891@debbugs.gnu.org Errors-To: guix-patches-bounces+larch=yhetil.org@gnu.org Sender: "Guix-patches" X-Migadu-Flow: FLOW_IN X-Migadu-Spam-Score: -1.26 Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=gmail.com header.s=20161025 header.b=Br9+uoeV; dmarc=fail reason="SPF not aligned (relaxed)" header.from=gmail.com (policy=none); spf=pass (aspmx1.migadu.com: domain of guix-patches-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-patches-bounces@gnu.org X-Migadu-Queue-Id: 27DBC9403D5 X-Spam-Score: -1.26 X-Migadu-Scanner: scn0.migadu.com X-TUID: O3I8qN5DFpSz Hello! Ludovic Court=C3=A8s writes: > Previously, 'patch-and-repack' would always create a tar.xz archive as a > result, even if the input was a directory (a checkout). This change > reduces gratuitous CPU and storage overhead. I like it! Note that on core-updates, xz compression is relatively fast on modern machines as it can do multi-threading. About space the savings; could the 'temporary' pristine source be cleared from the store always? This would prevent keeping nonfree cruft under /gnu/store until the next garbage collection run, for those sources that are cleaned up. > * guix/packages.scm (patch-and-repack)[tarxz-name]: Remove 'checkout?' ca= se. > [build](repack): New procedure, with "tar" invocation formerly at the > top level. > If SOURCE is a directory, call 'copy-recursively'; otherwise, call > 'repack'. > Change NAME to ORIGINAL-FILE-NAME when it matches 'checkout?'. > --- > guix/packages.scm | 65 ++++++++++++++++++++++++++--------------------- > 1 file changed, 36 insertions(+), 29 deletions(-) > > Hi! > > This change is a followup to a recent IRC discussion: it makes > =E2=80=98patch-and-repack=E2=80=99 preserve the =E2=80=9Cdirectoriness=E2= =80=9D of its input. > > It conflicts with other changes Maxim posted at > , though that could be addressed. > > Thoughts? > > Ludo=E2=80=99. > > diff --git a/guix/packages.scm b/guix/packages.scm > index 4caaa9cb79..cd2cded9ee 100644 > --- a/guix/packages.scm > +++ b/guix/packages.scm > @@ -1,5 +1,5 @@ > ;;; GNU Guix --- Functional package management for GNU > -;;; Copyright =C2=A9 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 202= 0 Ludovic Court=C3=A8s > +;;; Copyright =C2=A9 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 202= 0, 2021 Ludovic Court=C3=A8s > ;;; Copyright =C2=A9 2014, 2015, 2017, 2018 Mark H Weaver > ;;; Copyright =C2=A9 2015 Eric Bavier > ;;; Copyright =C2=A9 2016 Alex Kost > @@ -635,11 +635,9 @@ specifies modules in scope when evaluating SNIPPET." >=20=20 > (define (tarxz-name file-name) > ;; Return a '.tar.xz' file name based on FILE-NAME. > - (let ((base (cond ((numeric-extension? file-name) > - original-file-name) > - ((checkout? file-name) > - (string-drop-right file-name 9)) > - (else (file-sans-extension file-name))))) > + (let ((base (if (numeric-extension? file-name) > + original-file-name > + (file-sans-extension file-name)))) This is not new code, but I'm wondering what's the purpose of numeric-extension? What kind of files does it expect to catch? Also, what happened to stripping the '-checkout' suffix that used to be done? It doesn't seem like it will happen anymore. > (string-append base > (if (equal? (file-extension base) "tar") > ".xz" > @@ -689,6 +687,29 @@ specifies modules in scope when evaluating SNIPPET." > (lambda (name) > (not (member name '("." ".."))))))) >=20=20 > + (define (repack directory output) > + ;; Write to OUTPUT a compressed tarball containing DIRECTO= RY. > + (unless tar-supports-sort? > + (call-with-output-file ".file_list" > + (lambda (port) > + (for-each (lambda (name) > + (format port "~a~%" name)) > + (find-files directory > + #:directories? #t > + #:fail-on-error? #t))))) > + > + (apply invoke #+(file-append tar "/bin/tar") > + "cvfa" output > + ;; Avoid non-determinism in the archive. Set the m= time > + ;; to 1 as is the case in the store (software like = gzip > + ;; behaves differently when it stumbles upon mtime = =3D 0). > + "--mtime=3D@1" > + "--owner=3Droot:0" "--group=3Droot:0" > + (if tar-supports-sort? > + `("--sort=3Dname" ,directory) > + '("--no-recursion" > + "--files-from=3D.file_list")))) > + > ;; Encoding/decoding errors shouldn't be silent. > (fluid-set! %default-port-conversion-strategy 'error) >=20=20 > @@ -742,30 +763,16 @@ specifies modules in scope when evaluating SNIPPET." >=20=20 > (chdir "..") >=20=20 > - (unless tar-supports-sort? > - (call-with-output-file ".file_list" > - (lambda (port) > - (for-each (lambda (name) > - (format port "~a~%" name)) > - (find-files directory > - #:directories? #t > - #:fail-on-error? #t))))) > - (apply invoke > - (string-append #+tar "/bin/tar") > - "cvfa" #$output > - ;; Avoid non-determinism in the archive. Set the m= time > - ;; to 1 as is the case in the store (software like = gzip > - ;; behaves differently when it stumbles upon mtime = =3D 0). > - "--mtime=3D@1" > - "--owner=3Droot:0" > - "--group=3Droot:0" > - (if tar-supports-sort? > - `("--sort=3Dname" > - ,directory) > - '("--no-recursion" > - "--files-from=3D.file_list"))))))) > + ;; If SOURCE is a directory (such as a checkout), return a > + ;; directory. Otherwise create a tarball. > + (if (file-is-directory? #+source) > + (copy-recursively directory #$output > + #:log (%make-void-port "w")) > + (repack directory #$output)))))) >=20=20 > - (let ((name (tarxz-name original-file-name))) > + (let ((name (if (checkout? original-file-name) > + original-file-name > + (tarxz-name original-file-name)))) > (gexp->derivation name build > #:graft? #f > #:system system Was these cases (tar archive source derivation, directory source derivation) already covered by tests under tests/packages.cm? How did you otherwise test it? World rebuilding changes are not fun to test without unit tests. I've run that test module like this: $ make check TESTS=3Dtests/packages.scm SCM_LOG_DRIVER_FLAGS=3D' --brief=3D= no' VERBOSE=3D1 with your change and it passed. Thanks, Maxim