bug#43893: make update-guix-package produced an incorrect hash

[Maxim Cournoyer]

Hello Danny,

Thanks for the quick investigation.

Danny Milosavljevic <dannym@scratchpost.org> writes:

> 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.

That's a good observation; it is indeed dangerous.  In my case, my tree
looks clean (no modified git-tracked files), but I had in fact
modifications made to .dir-locals that I've been testing and these were
hidden from the view by using:

$ git update-index --skip-worktree .dir-locals

But since the .dir-locals file is known to git, it was probably picked
up with my invisible changes, causing the hash mismatch.

> 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.

Yes, that's my understanding of how it works too.  This means you have
to be extra careful doing this while no-one else is commiting changes,
else you have to start over because rebasing is not an option (it'd
change the hashes, breaking the computed Guix hash).  That's how I broke
'guix pull' the first time I used 'make update-guix-package' :-).  But I
think it's inevitable, so perhaps the best we can do is documement it
well and print a warning when running the target.

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

What do you mean by nested-nested Guix? Are there valid uses of such a
thing?

> @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.

Indeed.

[...]

> 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.

Yes, that's a good step.  Actually I just had an idea to use a clean
worktree to do the computation, because that's even safer as it prevents
subtle things like "git update-index --skip-worktree some/path" from
interacting with the computed hash too.

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

I'll send a first commit.  I haven't found a way to build it locally
with the command in the message; it seems to create a cycle.  Let me
know what you think.

Maxim