Adding a minimally-patched Guix repo to the org

dongcarl commented at 5:43 pm on May 9, 2022: contributor

As of e6a94d44469f90f4dc88a07a5a8587730811c705, we use Guix’s upstream git repo on https://git.savannah.gnu.org/git/guix.git for our guix time-machine-powered pinning.

However, it does seem that we occasionally will encounter problems with Guix’s upstream repo which are not easily fixed using the Guix package transformation flags.

In particular:

A test of gnutls failed because of a chronological error whereby a certificate used in a test expired. (#21203)
- We waited until Guix v1.3.0 was released to bump our time-machine to a Guix with a newer version of gnutls that had the test fixed.
A test of libgit2 failed because of a chronological error whereby a test relied on relative time calculations from the present, which needed to be bumped every year (#25082)

/me shakes fist at upstream packages writing tests that depend on the current time

Both of these problems are in packages that “underpin Guix”, which means that

It is in the best interest of Guix (and its users) to keep testing these packages instead of turning off tests entirely
They are not subject to the Guix package transformation flags. (--without-tests=blah, etc.)
Changes to these packages will cause a mass-rebuild of almost all of Guix’s packages, so these changes will be in Guix’s “core-updates” branch for a while before hitting master (see the Guix Manual section on Submitting Patches).

I think the better way might be to just maintain our own fork of Guix, that is minimally patched compared to upstream, so that we have more flexibility when deploying fixes like these and not have to wait for upstream (Guix or the actual package itself) to take our patches, go through their branching process, and hit Guix master.

What do people think? @fanquake @hebasto @laanwj

laanwj added the label Build system on May 9, 2022

laanwj commented at 5:56 pm on May 9, 2022: member

Agree with the motivation. We could add our own fork of guix to the bitcoin-core organiation like we do for univalue, leveldb, crc32c, and so on. Though I’m always hesitant to add any extra maintenance work to our scope.

luke-jr commented at 8:42 pm on May 9, 2022: member

As noted, upstream needs these issues fixed too, so I don’t think we would have much to gain from a fork.

They are not subject to the Guix package transformation flags. (–without-tests=blah, etc.)

This isn’t true for libgit2 from what I can tell. My problem was that the OS libgit2 was an older version, and I don’t know how to get the Bitcoin Core guix tooling to pass the flag tiself.

dongcarl commented at 10:45 pm on May 9, 2022: contributor

@luke-jr Okay… I’m not sure if I’m understanding totally correctly, but to answer your question you can specify ADDITIONAL_GUIX_ENVIRONMENT_FLAGS. See https://github.com/bitcoin/bitcoin/tree/master/contrib/guix#recognized-environment-variables for more

laanwj commented at 7:05 am on May 10, 2022: member

As noted, upstream needs these issues fixed too, so I don’t think we would have much to gain from a fork.

Like with the other repos, the aim would be is to use it for temporary patch-set on top, for things not yet upstreamed.

jamesob commented at 2:03 pm on June 6, 2022: member

I’m for this. Guix is a crucial part of the distribution mechanism for this project, and if we can make safety improvements with relatively minor patches that are slated for upstreaming, the additional maintenance seems justified.

laanwj commented at 8:03 pm on June 7, 2022: member

Ok, let’s do it then. How to call the repo? guix-fork or so? Our other upstreams are called *-subtree but that’s not meaningful here.

fanquake commented at 8:40 pm on June 7, 2022: member

and if we can make safety improvements with relatively minor patches that are slated for upstreaming, the additional maintenance seems justified.

Can you elaborate on some of the patches you have in mind? I don’t think there’s any point in us doing this until we actually have something concrete we want to do, otherwise it’s just complicating our ability to update our time-machine environment, without any benefit (we can still have a fork under bitcoin/ that is’t actually used by our repo).

Before using that fork, I think we should wait until we’ve at least got a change, which we don’t think is viable for, or going to be upstreamed in a reasonable time-frame.

luke-jr commented at 9:08 pm on June 7, 2022: member

Is it possible to make a Guix branch for v23.0 with #25082 fixed?

Sjors commented at 8:45 am on October 20, 2022: member

@luke-jr are you suggesting we maintain Guix branches with backports for our older releases? It seems easier to just bump the time-machine hash on our end and backport that. But maybe I’m missing something (the fix is already in master for Guix, but maybe it wasn’t when you posted this).

maflcko commented at 3:57 pm on December 8, 2022: member

Closing for now, as it is not clear if there is a need for this. Can be reopened if there is one.

maflcko closed this on Dec 8, 2022

theuni commented at 4:18 pm on May 30, 2023: member

Reopening for #27676.

One of those commits introduces a new guix fork in order to patch the LLVM build. In this case the typical overrides don’t work, and forking llvm as @dongcarl suggested here seems to be the most straightforward solution. I intend to upstream the changes eventually, but even if accepted that means a (potentially lengthy) delay and a time machine bump.

If there are no objections, I propose creating bitcoin-core/guix repo so that I may point our manifest to it rather than my personal repo.

theuni reopened this on May 30, 2023

maflcko commented at 4:26 pm on May 30, 2023: member

even if accepted that means a (potentially lengthy) delay

Have you tried it? If this goes into upstream within a week or two and all what is left for us to bump the time machine, this doesn’t seem like a reason to create the fork, only to remove it again in two weeks.

theuni commented at 4:32 pm on May 30, 2023: member

even if accepted that means a (potentially lengthy) delay

Have you tried it? If this goes into upstream within a week or two and all what is left for us to bump the time machine, this doesn’t seem like a reason to create the fork, only to remove it again in two weeks. @fanquake mentioned that we’re currently blocked from bumping our time machine for an unrelated reason, so I figured the fork would be necessary either way.

But no, I haven’t yet submitted the patch upstream. I’m guessing it’ll be rejected as-is because it probably breaks something else, so I’m operating on the assumption that it’ll take a while to get in.

Will work on upstreaming today/tomorrow.

fanquake referenced this in commit 0c84a0e484 on Jun 22, 2023

maflcko commented at 6:26 pm on February 8, 2024: member

Closing (again) for now, as it is not clear if there is a need for this. Can be reopened (again) if there is one. :)

maflcko closed this on Feb 8, 2024

Adding a minimally-patched Guix repo to the org #25098