From debbugs-submit-bounces@debbugs.gnu.org Fri Oct 30 23:56:19 2020 Received: (at 43893) by debbugs.gnu.org; 31 Oct 2020 03:56:19 +0000 Received: from localhost ([127.0.0.1]:60299 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kYi03-0005An-60 for submit@debbugs.gnu.org; Fri, 30 Oct 2020 23:56:19 -0400 Received: from mail-io1-f46.google.com ([209.85.166.46]:46237) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kYi01-0005Aa-BT for 43893@debbugs.gnu.org; Fri, 30 Oct 2020 23:56:17 -0400 Received: by mail-io1-f46.google.com with SMTP id s24so2727033ioj.13 for <43893@debbugs.gnu.org>; Fri, 30 Oct 2020 20:56:17 -0700 (PDT) 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=/d9Vz9+xNBlJo5gor/5DHjL9VosZmrkXRD3H1h1KdSc=; b=YGbW30klSOFeyeSDbbawTYQqAz/muJidUWB6cRwueBe+0iR9/i61KGFvXPjwF6og8m f834EB3frRPoH0WezW8Ht0fqnANnRQa4sFK+RhCm6ncH44FwkSKqM01gBLn7oV2Am2f7 cs0PpwLoOzK/3gcIYwE1Uz7rmKLAJ3pcz/H7dx1CaUHMRV3nAJskgUrcQkCWkD6YHClq fLcttBTHiZjQIP6DN2lD9puq58lTJxsQw3YVhC5H97c9nHH7EXZOhubMQ7t/3P5cbvdH n8L8TrBmGTnp1nO0OdMujxVUcreEdsv4tykm21L4HvnIYnA9/g6vubRRjdswSPtcIwVU B5Yw== 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=/d9Vz9+xNBlJo5gor/5DHjL9VosZmrkXRD3H1h1KdSc=; b=Noz7wNEifLS5paeWfdCX9/3yqaE2Cf3f8ArdFmEr+HpY8Ex2YeSz37UI999Ggg3GsQ kLrI4KO3dTbB+WrC6j3oro3ZO34nrmQRmmqFrytNFCZeGojs8RRTX2dv4vED1v44elV7 Z9FHY8mG1788uhfB1ovziXj5eM0OHM/PDe3nYcXbs1eWPKqhpxbY/VKRc9N7lSCMA7Dk fteYEdu2ezlpPM4rcAmDkqVbb7IrM7Uv71glEwju7QlI8Hv+x6NiMfVbhriV40DEnmcK ZP9x+xg9wfrHAsCaplPa8pDN7WTZkFHJsTVHWrIeaMsHzD4NtUye4X9tglS9RiVIKogr nwhw== X-Gm-Message-State: AOAM532lryIuF+dKelrMhfJ54R8onvrDY+P/v+nUJ+YbofF2vGRkIY49 LOEzbjYaxHUxYfIS8QyL8Hk5lnLOdcY= X-Google-Smtp-Source: ABdhPJzVXBWwPX326RsiyhICuamq6I1fzPjBAhWrKDYw4drDBYVtrdF0LbsFObpdK1Z2bQ9WoJcWiA== X-Received: by 2002:a05:6602:2d4e:: with SMTP id d14mr4233110iow.105.1604116571472; Fri, 30 Oct 2020 20:56:11 -0700 (PDT) Received: from hurd (dsl-236-123-160.b2b2c.ca. [207.236.123.160]) by smtp.gmail.com with ESMTPSA id b70sm6999472ilb.51.2020.10.30.20.56.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 30 Oct 2020 20:56:10 -0700 (PDT) From: Maxim Cournoyer To: Ludovic =?utf-8?Q?Court=C3=A8s?= Subject: Re: bug#43893: [PATCH v3] maint: update-guix-package: Prevent accidentally breaking guix pull. References: <87imbedsko.fsf@gnu.org> <20201014031705.4516-1-maxim.cournoyer@gmail.com> <87lfg0bo9m.fsf@gnu.org> <87v9f44852.fsf@gmail.com> <875z749czt.fsf@gnu.org> <87eelpd0af.fsf@gmail.com> <87k0vhm1fg.fsf@gnu.org> <87d0172adj.fsf@gmail.com> <87k0vefjgv.fsf@gnu.org> <874kmifhn2.fsf@gnu.org> Date: Fri, 30 Oct 2020 23:56:09 -0400 In-Reply-To: <874kmifhn2.fsf@gnu.org> ("Ludovic =?utf-8?Q?Court=C3=A8s=22'?= =?utf-8?Q?s?= message of "Sun, 25 Oct 2020 16:29:53 +0100") Message-ID: <87d00zyrom.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-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 43893 Cc: 43893@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) Hi Ludovic, Ludovic Court=C3=A8s writes: > Hi again, > > How about this variant of the initial script? I think it addresses the > main issues we discussed here: > > 1. By default it doesn=E2=80=99t re-add the source in the store, so wro= ng > commit/hash issues are caught when running =E2=80=98guix build guix= =E2=80=99. > > 2. It diagnoses dirty trees early on. It does not explicitly diagnose > missing upstream commits though, but again they=E2=80=99re caught wh= en > running =E2=80=98guix build guix=E2=80=99. > > WDYT? > > Sorry for all the back-and-forth on what looks like a tiny issue. I do > think we=E2=80=99re making progress anyway! > > Thanks, > Ludo=E2=80=99 Reproducing as a diff over the original script for brevity: > @@ -23,12 +24,15 @@ > ;;; > ;;; Code: > -(use-modules (guix) > +(use-modules (git) > + (guix) > (guix git-download) > (guix upstream) > (guix utils) > (guix base32) > (guix build utils) > + (guix i18n) > + (guix diagnostics) > (gnu packages package-management) > (ice-9 match)) > @@ -101,7 +105,43 @@ COMMIT." > (exp > (error "'guix' package definition is not as expected" exp))))) > - > +(define (keep-source-in-store store source) > + "Add SOURCE to the store under the name that the 'guix' package expect= s." > + > + ;; Add SOURCE to the store, but this time under the real name used in = the > + ;; 'origin'. This allows us to build the package without having to ma= ke a > + ;; real checkout; thus, it also works when working on a private branch. > + (reload-module > + (resolve-module '(gnu packages package-management))) > + > + (let* ((source (add-to-store store > + (origin-file-name (package-source guix)) > + #t "sha256" source)) > + (root (store-path-package-name source))) > + > + ;; Add an indirect GC root for SOURCE in the current directory. > + (false-if-exception (delete-file root)) > + (symlink source root) > + (add-indirect-root store > + (string-append (getcwd) "/" root)) > + > + (info (G_ "source code kept in ~a (GC root: ~a)~%") > + source root))) > + > +(define (assert-clean-checkout repository) > + "Error out if the working directory at REPOSITORY contains local > +modifications." > + (define description > + (let ((format-options (make-describe-format-options > + #:dirty-suffix "-dirty"))) > + (describe-format (describe-workdir repository) format-options))) > + > + (when (string-suffix? "-dirty" description) > + (leave (G_ "attempt to update 'guix' package from a dirty tree (~a)~= %") > + description)) > + > + (info (G_ "updating 'guix' package to '~a'~%") description)) Unfortunately this doesn't catch the case where git has explicitly been told to '--skip-worktree' on a path or file (the original cause of this bug report). See https://debbugs.gnu.org/cgi/bugreport.cgi?bug=3D43893#11. > (define (main . args) > (match args > ((commit version) > @@ -113,32 +153,20 @@ COMMIT." > (hash (query-path-hash store source)) > (location (package-definition-location)) > (old-hash (content-hash-value > - (origin-hash (package-source guix))))) > + (origin-hash (package-source guix))))) > + > + (unless (getenv "GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT") > + (let ((repository (repository-open "."))) > + (assert-clean-checkout repository) > + (repository-close! repository))) > + > (edit-expression location > (update-definition commit hash > #:old-hash old-hash > #:version version)) > - ;; Re-add SOURCE to the store, but this time under the real nam= e used > - ;; in the 'origin'. This allows us to build the package without > - ;; having to make a real checkout; thus, it also works when wor= king > - ;; on a private branch. > - (reload-module > - (resolve-module '(gnu packages package-management))) > - > - (let* ((source (add-to-store store > - (origin-file-name (package-source = guix)) > - #t "sha256" source)) > - (root (store-path-package-name source))) > - > - ;; Add an indirect GC root for SOURCE in the current director= y. > - (false-if-exception (delete-file root)) > - (symlink source root) > - (add-indirect-root store > - (string-append (getcwd) "/" root)) > - > - (format #t "source code for commit ~a: ~a (GC root: ~a)~%" > - commit source root))))) > + (when (getenv "GUIX_ALLOW_ME_TO_USE_PRIVATE_COMMIT") > + (keep-source-in-store store source))))) That environment variable would now do more than it advertises. I'd prefer to keep the behavior consistent in both modes, unless there's a very good reason not too? Thanks, Maxim