Skip to content

glibc: 2.38-44 -> 2.39-5#287594

Merged
vcunat merged 5 commits intoNixOS:stagingfrom
Ma27:glibc-2.39
Mar 24, 2024
Merged

glibc: 2.38-44 -> 2.39-5#287594
vcunat merged 5 commits intoNixOS:stagingfrom
Ma27:glibc-2.39

Conversation

@Ma27
Copy link
Copy Markdown
Member

@Ma27 Ma27 commented Feb 9, 2024

Description of changes

Announcement: https://lists.gnu.org/archive/html/info-gnu/2024-01/msg00017.html

This release seems relatively harmless in terms of potential fallout.
Most notably is the removal of crypt(3) in favor of libxcrypt which
we've done already and compatibility from ISO C2X.

Also decided to drop the old *.gz approach in favor of inlining the
patch with the changes from the release branch directly: it's relatively
small in contrast to certain lockfiles in this repo and having a textual
version makes reviews & diffs easier. See also
#258972 (comment) for
more context.


cc @vcunat would you mind creating a hydra jobset in the nixpkgs project for that?
I got relatively far already (built firefox on x86_64-linux and stdenv on aarch64 and aarch64 cross), but I'd still like to see a little more.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@Ma27 Ma27 mentioned this pull request Feb 9, 2024
12 tasks
@ofborg ofborg bot added the 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch. label Feb 10, 2024
@ofborg ofborg bot requested review from ConnorBaker and edolstra February 10, 2024 00:13
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Feb 10, 2024
@vcunat
Copy link
Copy Markdown
Member

vcunat commented Feb 10, 2024

  • For the jobset and doing comparisons, isn't it better to base the branch on some commit that's known to be relatively good? I see some commits there that are only present on staging so far.
  • I assume you'll prefer x86_64-linux for the jobset, right? (Starting fixes with just one platform seems the best approach.)

@vcunat
Copy link
Copy Markdown
Member

vcunat commented Feb 10, 2024

it's relatively small in contrast to certain lockfiles

Well, we're only at -2 here. IIRC the textual patches really were quite large sometimes, but I don't recall investigating deeply whether to compress or not.

@Ma27
Copy link
Copy Markdown
Member Author

Ma27 commented Feb 11, 2024

Well, we're only at -2 here

This was not about the current state of course given that the release is only 11 days old.

The glibc with most patches on top was 2.35-224 according to https://www.nixhub.io/packages/glibc.
So I built the tarball on 6b70761:

  • with .patch.gz it's 25628896 bytes according to stat.
  • with .patch and .patch.gz (w/o .patch.gz I'd have to rebuild quite a bunch of stuff and we're only interested in the size, so I went for having both in the tarball) it's 25944372 bytes.

This is an increase of 1.2% in a very bad case (224 additional patches).

I assume you'll prefer x86_64-linux for the jobset, right?

Yes. When we're sufficiently confident in x86_64-linux, we can probably do another eval (perhaps even aarch64-linux only) to see if there are big regressions (only built the stdenv there so far).

For the jobset and doing comparisons, isn't it better to base the branch on some commit that's known to be relatively good? I see some commits there that are only present on staging so far.

Considering how far I got I assumed the rev to be sufficiently OK. But it's perhaps better for Hydra comparisons, so I guess I'll find a merge-base of staging{,-next}.

@Ma27
Copy link
Copy Markdown
Member Author

Ma27 commented Feb 11, 2024

@vcunat rebased onto 74098ff, that's on both staging & staging-next. Also it seems as if staging-next regressed a little after that and the rev is still rather recent, so it seems like a reasonable choice.

@vcunat
Copy link
Copy Markdown
Member

vcunat commented Feb 11, 2024

There's nothing in master..staging-next, but OK, not a significant difference.

@vcunat
Copy link
Copy Markdown
Member

vcunat commented Feb 11, 2024

x86_64-linux queue has lots of jobs from staging-next-23.11 right now, but I think this will go next after it shortens a bit.

@Ma27
Copy link
Copy Markdown
Member Author

Ma27 commented Feb 11, 2024

There's nothing in master..staging-next, but OK, not a significant difference.

I'm afraid, I'm not sure what you mean here?
Just went by git revs that are also Hydra evals, so I can do direct jobset eval comparisons later.

x86_64-linux queue has lots of jobs from staging-next-23.11 right now, but I think this will go next after it shortens a bit.

It's not urgent at all, I can wait :)

@vcunat
Copy link
Copy Markdown
Member

vcunat commented Feb 11, 2024

I mean that all of the current staging-next has been merged to master. There are only the automatic merge commits (which don't cause rebuilds).

@vcunat
Copy link
Copy Markdown
Member

vcunat commented Feb 15, 2024

Hydra will now have time due to catching up on darwin work:
https://hydra.nixos.org/jobset/nixpkgs/pr-287594-glibc-2.39

@vcunat
Copy link
Copy Markdown
Member

vcunat commented Feb 15, 2024

The libpng thing is unrelated. I'll deal with that and restart failures after it succeeds. EDIT: done now.

@risicle
Copy link
Copy Markdown
Contributor

risicle commented Feb 25, 2024

What's the status on this? I'm quite interested in being able to start getting more CET stuff in place (e.g. #288052)

@vcunat
Copy link
Copy Markdown
Member

vcunat commented Feb 26, 2024

Well, anyone could triage the regressions, as it's on public Hydra (and in cache.nixos.org even!)

@Ma27
Copy link
Copy Markdown
Member Author

Ma27 commented Feb 26, 2024

It's on my list high up (I even have a lot of tabs open for potential regressions I need to check) and I'll try to get to it this week! :)

@risicle risicle mentioned this pull request Feb 26, 2024
13 tasks
@risicle
Copy link
Copy Markdown
Contributor

risicle commented Feb 27, 2024

Going through the list of failures, I'm finding the vast majority to just be flakey builds that have subsequently succeeded for me.

@Ma27
Copy link
Copy Markdown
Member Author

Ma27 commented Feb 27, 2024

So, the only regression I found is https://hydra.nixos.org/build/249763145 (_389ds-base). See also 389ds/389-ds-base#5962. However, the package is pretty outdated (not even the latest patch-level for the minor we have) and the patch doesn't apply, so I'll just mark it as broken and leave it to the maintainer.

Haven't seen anything else. I only built aarch64-linux's stdenv so far, nothing more. So perhaps we want to do one Hydra eval of aarch64-linux as well?

@Ma27
Copy link
Copy Markdown
Member Author

Ma27 commented Feb 27, 2024

cc @ners re _389ds-base.

@Ma27 Ma27 marked this pull request as ready for review February 27, 2024 22:46
@risicle
Copy link
Copy Markdown
Contributor

risicle commented Feb 27, 2024

Indeed, see #285042. 2.4.5 seems to fix it for me, and the suggested patch un-breaks 32bit.

@vcunat
Copy link
Copy Markdown
Member

vcunat commented Feb 28, 2024

Failures retried, so that it's easier to focus on real regressions.

@vcunat
Copy link
Copy Markdown
Member

vcunat commented Feb 29, 2024

aarch64-linux added, as there's free capacity now.

@artemist artemist mentioned this pull request Mar 1, 2024
14 tasks
@Ma27
Copy link
Copy Markdown
Member Author

Ma27 commented Mar 2, 2024

So, out of direct comparison with https://hydra.nixos.org/eval/1804335?compare=1804178&full=0#tabs-now-fail:

  • boulder breaks with Retries should've taken at least 4.4 seconds. To me this seems like a textbook flaky test which also explains why it's newly failing.
  • same for duckdb-engine: Stopped because test was flaky says the log.
  • haskellPackages.stm-queue is also suspicious at least: https://hydra.nixos.org/build/249748595/log/tail

The following things are reproducible regressions on x86_64-linux and don't seem like flaky builds:

  • dolphin-emu
  • swift
    Will look into both.

openobserve is still building.

Seems as if the tzdata problem on aarch64-linux is a general issue on staging. Will rebase onto a newer revision with this included and start a new eval after that.

@Ma27
Copy link
Copy Markdown
Member Author

Ma27 commented Mar 2, 2024

openobserve seems OK as well to me.

Regarding swift (https://hydra.nixos.org/build/249763077/nixlog/12): the issue here is apparently glibc commit 64b1a44183a3094672ed304532bedb9acc707554 (marks FILE* arguments as nonnull in certain headers).
If I understand the compiler output correctly (https://hydra.nixos.org/build/249763077/nixlog/12), fwrite/ferror get an optional (i.e. nullable) type which is wrong now.
I can probably file a band-aid fix, but I don't swift well enough to build something that's supported on older glibcs as well.
cc @stephank @Trundle @dduan @trepetti @dtzWill

Ma27 added 3 commits March 2, 2024 19:01
Announcement: https://lists.gnu.org/archive/html/info-gnu/2024-01/msg00017.html

This release seems relatively harmless in terms of potential fallout.
Most notably is the removal of `crypt(3)` in favor of libxcrypt which
we've done already and compatibility from ISO C2X.

Also decided to drop the old *.gz approach in favor of inlining the
patch with the changes from the release branch directly: it's relatively
small in contrast to certain lockfiles in this repo and having a textual
version makes reviews & diffs easier. See also
NixOS#258972 (comment) for
more context.
Doesn't build with glibc 2.39. There's a potential fix documented in
389ds/389-ds-base#5332, but the package is too
old for the patch to apply, so I'll mark it as broken for now and leave
it to the maintainer to update & fix.
Failing Hydra build: https://hydra.nixos.org/build/249763077/nixlog/12

The problem is that glibc commit
64b1a44183a3094672ed304532bedb9acc707554 marked the `FILE*` argument of a few
functions including `fread` & `ferror` as non-null. The applied patch
("Android: add better nullability checks for nullability annotations added in NDK 26")
is targeted for the Android platform, but fixes said issue as well: the
handle returned from `fopen` is of type `Optional<T>` and the `guard`
expression unwraps that now (and throws an exception if `nil` is
returned). The previous `nil`-check didn't modify the type of `fp`, but
only raised the exception and moved on with `Optional<T>`.

It's a little sad that the patch needs to be applied at so many places,
but I guess that's what you get with language-level package managers 🤷
Also, seems good-enough to me given that it's actually temporary, the
patch is already upstream and will probably be obsolete at one of the
next Swift updates.
@Ma27 Ma27 changed the title glibc: 2.38-44 -> 2.39-2 glibc: 2.38-44 -> 2.39-5 Mar 2, 2024
@Ma27
Copy link
Copy Markdown
Member Author

Ma27 commented Mar 2, 2024

Rebased onto the rev from https://hydra.nixos.org/eval/1804706, that should give us a good comparison. Starting an eval now.

@ofborg ofborg bot requested review from Trundle, dduan, dtzWill, stephank and trepetti March 2, 2024 20:41
@Ma27
Copy link
Copy Markdown
Member Author

Ma27 commented Mar 22, 2024

Two more fixes pushed. Seems as if the rest are flaky tests (or failures, but no fresh regressions): https://hydra.nixos.org/eval/1804710?full=1&compare=1804706#tabs-now-fail

wdyt?

@vcunat
Copy link
Copy Markdown
Member

vcunat commented Mar 23, 2024

Uh, lots of newly failing and newly succeeding in there. (even after retrying everything; though most "new failures" is moto which I know to be flaky)

@vcunat
Copy link
Copy Markdown
Member

vcunat commented Mar 23, 2024

Anyway, if you've passed through a diff and found no (significant) regressions, it should be good to merge, I believe.

@Ma27
Copy link
Copy Markdown
Member Author

Ma27 commented Mar 23, 2024

Uh, lots of newly failing and newly succeeding in there. (even after retrying everything; though most "new failures" is moto which I know to be flaky)

I don't see any new build failures and just to be sure I picked a few things that seemingly failed during e.g. the checkPhase and built locally. Some of them needed a few tries, but that's nothing that should block this branch.

@vcunat vcunat merged commit ddc361c into NixOS:staging Mar 24, 2024
@Ma27 Ma27 deleted the glibc-2.39 branch March 24, 2024 06:53
@quentinmit
Copy link
Copy Markdown
Contributor

This PR broke the use of pkgs.pkgsCross.gnu64.stdenv.cc on Darwin. The new 2.39-master.patch tries to create a file named ADVISORIES and a directory named advisories, which conflict on a case-insensitive filesystem. The old 2.38-master.patch.gz appears to have not created the file ADVISORIES, so it didn't have this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants