-
-
Notifications
You must be signed in to change notification settings - Fork 14k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
llvmPackages_{14,15,16,17}.lldb: use SWIG 4; swig3: drop #341384
Conversation
These have been failing to build on Hydra for over two months. It looks like it might be a Python 3.12‐related thing, but the SWIG 4 patches wouldn’t apply cleanly and these old versions should probably just be dropped anyway.
The patches are already included in LLVM 16 and 17.
96ecbfe
to
fce8148
Compare
@@ -1482,6 +1482,9 @@ mapAliases ({ | |||
swift-im = throw "swift-im has been removed as it is unmaintained and depends on deprecated Python 2 / Qt WebKit"; # Added 2023-01-06 | |||
swig1 = throw "swig1 has been removed as it is obsolete"; # Added 2024-08-23 | |||
swig2 = throw "swig2 has been removed as it is obsolete"; # Added 2024-08-23 | |||
swig3 = throw "swig3 has been removed as it is obsolete"; # Added 2024-09-12 | |||
swig4 = swig; # Added 2024-09-12 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit curious about the rationale for making swig4
an alias rather than leaving it as a definition in all-packages.nix
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My hope is that we can stop carrying multiple versions, so this is to forbid pinning the current version in‐tree, with a view towards dropping the alias. If you think that’s too ambitious then I can put it back (though the convention would be swig_4
, which is another reason I felt like we should just drop it). So far it’s seemed like the number of things that break from new versions is relatively limited, and no one package has seemed like it has taken an inordinate amount of effort to fix during a normal staging
cycle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should be fine given recent experience? (Just wanted to confirm the intent was in fact to forbid pinning.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, my thinking is that we can always move it back if SWIG 5 somehow breaks everything, but that at present nobody really has any way of knowing that’s likely so pinning is a bad idea. (Same reason I unpinned all the FFmpeg 7 users.)
fce8148
to
932e637
Compare
Updated to include the LLDB patches for Swift too; building that locally on |
This version is no longer used for anything in the tree.
Let’s see if we can’t keep only one version of this going forward. `swigWithJava` has been an alias since 2009(!), so let’s just drop it.
This no longer uses the `swig3` package.
932e637
to
db5f130
Compare
I’m bored of waiting for Result of 7 packages marked as broken and skipped:
41 packages built:
|
(Swift finished building on |
nixpkgs-review happy, x86_64-linux:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, LGTM.
Description of changes
Turns out we do need the upstream patches after all, but it was relatively pain‐free. Hopefully we can drop LLVM 12 and 13 entirely soon.
Result of
nixpkgs-review
run on x86_64-linux 17 packages marked as broken and skipped:
20 packages built:
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.