unofficial mirror of bug-guix@gnu.org 
 help / color / mirror / code / Atom feed
From: Danny Milosavljevic <dannym@scratchpost.org>
To: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Cc: 43893@debbugs.gnu.org
Subject: bug#43893: make update-guix-package produced an incorrect hash
Date: Sat, 10 Oct 2020 02:04:10 +0200	[thread overview]
Message-ID: <20201010020410.3a301654@scratchpost.org> (raw)
In-Reply-To: <87eem7qcxc.fsf@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 3600 bytes --]

I'm guessing it has something to do with update-guix-package using git-predicate
to add only git-known (but not necessarily committed) files to the store and then
calculating the checksum of that--but the git-fetch for the guix package not
necessarily doing the same.

Then update-guix-package.scm does one worse and actively prevents guix from
doing the checkout from git when building that "guix" package.  That means the
person invoking update-guix-package.scm can't notice even when the sha256 hash
is definitely wrong--because guix will have the source for package "guix" in
the store already (a faked entry added by update-guix-package.scm) and thus
won't fetch it again.

Also, doesn't this entire approach have a problem?

If you make a commit into the git repo of guix in order to update the
package "guix" to commit A, at that point you can't know what commit hash
commit A will have (since you haven't committed it yet) and yet you have
to know the commit hash of commit A in order to write it into the package
definition of package "guix".

That cannot work.

The only way it works, more or less by accident is that,

(1) At first, update-guix-package.scm does NOT update the "guix" package
inside, and calculates the hash of the working copy (hash A).
(2) Then, it updates the "guix" package inside to refer to hash A and to a
USER-SPECIFIED COMMIT HASH (the latter is determined by the user via
git rev-parse HEAD).
(3) Then, it commits that changed working copy as commit B.  Commit B is
essentially not referred-to by anyone--it's just to make it to the
git repository so guix pull can pick it up.

That works only as long as there will be no reference to a nested-nested "guix"
package, by the eventual user.

@Maxim: I think this entire thing has to assume that

  git rev-parse HEAD

(which it did at the very beginning of make update-guix-package) actually
refers to a commit that is available on the guix git repository on savannah.

That means as soon as you change anything (no matter what) (and not actually
commit that) before invoking

  make update-guix-package

the commit it refers to in the "guix" package will be one which cannot be
resolved by users.

Worse, if you change anything but not commit it (even locally), then that
surely counts as "part of the checkout" for make update-guix-package, so
the hash will be calculated including those change--but the changes are
not committed, so no one can build the resulting guix package (because
of a hash mismatch).  That can happen automatically very easily if "make"
updates po files.

An easy fix, also done by a lot of other such release tools, is to make

  make update-guix-package

first check whether there are any uncommitted changes.  If so, make it fail.

There's

  guix build guix --with-git-url=guix=.

but even that won't work with (locally) uncommitted changes.

Note: uncommitted and unpushed are different.

It's totally fine to have UNPUSHED changes and then use

  ./pre-inst-env guix build guix --with-git-url=guix=`pwd`

in order to build it anyway.

But it's not fine to do that with UNCOMMITTED changes--because the sha256
hash will include those, but the commit id won't.

Long story short, we should make "make update-guix-package" check for
uncommitted changes in the working copy, and fail if any such exist[1].
There are no downsides that I can see.  Even building from local working
copy still works then.

Also, let's please document update-guix-package.

[1] git diff-index --quiet HEAD || echo fail

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2020-10-10  0:05 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-09 21:58 bug#43893: make update-guix-package produced an incorrect hash Maxim Cournoyer
2020-10-10  0:04 ` Danny Milosavljevic [this message]
2020-10-10  5:08   ` Maxim Cournoyer
2020-10-10  5:08 ` bug#43893: [PATCH] maint: update-guix-package: Ensure sources are clean Maxim Cournoyer
2020-10-10 11:59   ` Danny Milosavljevic
2020-10-11  2:35     ` Maxim Cournoyer
2020-10-10 20:08   ` Ludovic Courtès
2020-10-10 21:14     ` Danny Milosavljevic
2020-10-12  4:40       ` Maxim Cournoyer
2020-10-12  9:40       ` Ludovic Courtès
2020-10-12 14:18         ` Danny Milosavljevic
2020-10-11 19:43     ` Maxim Cournoyer
2020-10-12  9:43       ` Ludovic Courtès
2020-10-13  1:33         ` Maxim Cournoyer
2020-10-11 19:57 ` bug#43893: [PATCH v2] maint: update-guix-package: Prevent accidentally breaking guix pull Maxim Cournoyer
2020-10-13 16:00   ` Marius Bakke
2020-10-14  3:17     ` bug#43893: [PATCH v3] " Maxim Cournoyer
2020-10-20 21:06       ` Ludovic Courtès
2020-10-21  2:36         ` Maxim Cournoyer
2020-10-21  8:53           ` Ludovic Courtès
2020-10-23  4:38             ` Maxim Cournoyer
2020-10-23 15:01               ` Ludovic Courtès
2020-10-25  4:32                 ` Maxim Cournoyer
2020-10-25 14:50                   ` Ludovic Courtès
2020-10-25 15:29                     ` Ludovic Courtès
2020-10-31  3:56                       ` Maxim Cournoyer
2020-10-31 10:42                         ` Ludovic Courtès
2020-11-09 19:28                           ` Maxim Cournoyer
2020-11-09 22:03                             ` Ludovic Courtès
2020-11-10 14:31                               ` Maxim Cournoyer
2020-11-09 19:29                           ` bug#43893: [PATCH] maint: update-guix-package: Optionally add sources to store Maxim Cournoyer
2020-11-09 22:18                             ` Ludovic Courtès
2020-11-10 14:02                               ` Maxim Cournoyer
2020-11-10 14:48                                 ` Ludovic Courtès
2020-11-10 15:18                                   ` Maxim Cournoyer
2020-11-09 22:44                           ` bug#43893: [PATCH v5] " Maxim Cournoyer
2020-11-10  9:32                             ` Ludovic Courtès
2020-10-25 14:41       ` bug#43893: [PATCH v3] maint: update-guix-package: Prevent accidentally breaking guix pull Ludovic Courtès
2020-10-25 19:17         ` Maxim Cournoyer
2020-10-14  4:10     ` bug#43893: [PATCH v2] " Maxim Cournoyer
2020-10-19 18:04       ` Maxim Cournoyer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201010020410.3a301654@scratchpost.org \
    --to=dannym@scratchpost.org \
    --cc=43893@debbugs.gnu.org \
    --cc=maxim.cournoyer@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).