-
-
Notifications
You must be signed in to change notification settings - Fork 14.8k
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
nixVersions.nix_2_26: init at 2.26.1 #375856
base: master
Are you sure you want to change the base?
Conversation
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
nativeBuildInputs = [ nix ] would have no effect before this change. The pkg-config test on the dev output probably didn't work before, so I've removed it for now.
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
}; | ||
|
||
# TODO Hack until https://github.com/NixOS/nixpkgs/issues/45462 is fixed. | ||
boost = |
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.
Just a heads up. We didn't use to have this override in nixpkgs. Question is if we want this now?
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.
Afaik this would only save us downloading icu.
}).overrideAttrs | ||
(old: { | ||
# Need to remove `--with-*` to use `--with-libraries=...` | ||
buildPhase = lib.replaceStrings [ "--without-python" ] [ "" ] old.buildPhase; |
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.
boost seems to not have python enabled by default. So we can drop the overrideAttrs here and in the nix repository.
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.
It seems that @Ericson2314 was working around something.
I don't know the intricacies of this.
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've switched it to the default boost
package for now.
let | ||
cpp = fileset.fileFilter (file: file.hasExt "cc" || file.hasExt "h"); | ||
in | ||
fileset.unions [ |
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 don't think filesets are allows in nixpkgs at the moment because of that nix bug.
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.
This is the tracking issue: #369694
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.
The filesets are intentionally dead code to keep the files similar to upstream.
So we're not affected by the bug, but we may remove these filesets anyway. I'm neither convinced to remove them nor keep them.
This is the first Nixpkgs build of Nix that uses meson and uses the componentized build.
Related
Future versions may use a similar method of packaging (copying files mostly as-is), or we could "make it our own" here in Nixpkgs and support multiple versions using conditionals, etc.
This initial version probably leaves one or two things to be desired, but let's get the ball rolling first.
TODO
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/
)Context:
Add a 👍 reaction to pull requests you find important.