Skip to content
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

stdenv: use disallowedRequisites to check forbidden requisites #168590

Merged
merged 1 commit into from Jul 17, 2022
Merged

stdenv: use disallowedRequisites to check forbidden requisites #168590

merged 1 commit into from Jul 17, 2022

Conversation

ghost
Copy link

@ghost ghost commented Apr 14, 2022

Description of changes

This commit uses the disallowedRequisites derivation attribute to
check the condition described in the comment above it: "avoid
reference to bootstrap tools".

Things done
  • Built on platform(s)
    • x86_64-linux
    • powerpc64le-linux
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@ghost ghost marked this pull request as ready for review April 14, 2022 16:33
@ghost ghost requested review from Ericson2314 and matthewbauer as code owners April 14, 2022 16:33
@ghost ghost changed the base branch from master to staging April 14, 2022 17:18
@Artturin
Copy link
Member

Result of nixpkgs-review pr 168590 run on x86_64-linux 1

2 packages built:
  • ripgrep
  • sway

@ghost
Copy link
Author

ghost commented Apr 15, 2022

2 packages built:

Hrm, that doesn't sound right. This PR should cause absolutely everything to rebuild.

Is there some way to get nixpkgs-review to not hide the build logs from me? This is the main reason I'm not in the habit of using it.

A lot of my PRs are world-rebuilding things, and wasting ~6 hours build time because it was doing the wrong thing or doomed to fail is no fun. I can usually tell by watching nix-build or nix build -L if things have gone off-course.

@Artturin
Copy link
Member

I used -p sway ripgrep with nixpkgs-review

@ghost
Copy link
Author

ghost commented Apr 16, 2022

This should wait until after staging reopens on 2022-May-15; the benefits are long-term and not worth the risk of breaking things right now.

@ghost ghost marked this pull request as draft April 16, 2022 21:00
@Artturin Artturin marked this pull request as ready for review June 3, 2022 00:26
@lovesegfault
Copy link
Member

@amjoseph-nixpkgs Is this ready for merging?

@Artturin Artturin self-requested a review July 17, 2022 01:45
@ghost
Copy link
Author

ghost commented Jul 17, 2022

@amjoseph-nixpkgs Is this ready for merging?

It should be, but since the PR is three months old let me run a rebuild real quick. I should have powerpc64le results in less than an hour and x86_64 results in about two hours.

@Artturin
Copy link
Member

@amjoseph-nixpkgs Is this ready for merging?

It should be, but since the PR is three months old let me run a rebuild real quick. I should have powerpc64le results in less than an hour and x86_64 results in about two hours.

I suggest rebasing on staging if you didn't already do it locally

@ghost
Copy link
Author

ghost commented Jul 17, 2022

I suggest rebasing on staging if you didn't already do it locally

Yes, that's where I'm building from. Unfortunately I got a nasty surprise and had to restart the build. Should be done in ~30 minutes.

@ghost
Copy link
Author

ghost commented Jul 17, 2022

@amjoseph-nixpkgs Is this ready for merging?

Yes, this PR is ready to merge.

nix build -f . -L stdenv built successfully on top of 0e48ac0 (i.e. today's staging) on x86_64-linux.

@ghost
Copy link
Author

ghost commented Jul 17, 2022

nix build -f . -L stdenv built successfully on top of 0e48ac0 (i.e. today's staging) on x86_64-linux.

Also on powerpc64le-linux, on top of the same commit, plus #181802.

@Artturin Artturin merged commit 111abd8 into NixOS:staging Jul 17, 2022
@ghost ghost deleted the stdenv-disallowedReferences branch July 18, 2022 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants