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

p11-kit: Fix builds on non-NixOS systems (single-user install) #278099

Merged
merged 1 commit into from
Jan 27, 2024

Conversation

jonathanlking
Copy link
Contributor

Description of changes

Discussed in #96715.

In 3ca33e5 the FAKED_MODE environment variable was no longer set. This causes tests to be run (and fail) that were previously skipped, see https://github.com/p11-glue/p11-kit/blob/81715a26a36599562793f5fe103b6bee1a141fdb/p11-kit/test-conf.c#L472-L473.

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.

@jonathanlking jonathanlking marked this pull request as draft January 1, 2024 17:11
@@ -63,6 +63,13 @@ stdenv.mkDerivation rec {
]))
];

# Tests run in fakeroot for non-root users
preCheck = ''
if [ "$(id -u)" != "0" ]; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this break things if we set this variable always? Otherwise people on just nixos cannot know, if it continue to works on other platforms.

Alsop lease follow the contributing guide when naming your commits.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for such a speedy review!

Alsop lease follow the contributing guide when naming your commits.

Apologies, I believe it now follows the convention!

Does this break things if we set this variable always? Otherwise people on just nixos cannot know, if it continue to works on other platforms.

I haven't thought about this change too much, I'm just reverting it back to how it was before 3ca33e5#diff-6e5cbf6c584d88a4b8326fc03daf8650866eed33b1651b38ab379da599f57109L72-L77.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would skip the tests from running when building on NixOS/multi-user as well.

Copy link
Member

@jtojnar jtojnar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please include the reference to the commit that removed it in the commit message itself so that the info is preserved for Git blame, without having to dig into GitHub?

pkgs/development/libraries/p11-kit/default.nix Outdated Show resolved Hide resolved
@jtojnar
Copy link
Member

jtojnar commented Jan 1, 2024

Also please target the staging branch, since this will cause a mass rebuild.

Add back FAKED_MODE environment variable.
This was removed in 3ca33e5, which
caused tests to be run (and fail) that were previously skipped.
@jtojnar
Copy link
Member

jtojnar commented Jan 1, 2024

Thanks.

@jtojnar jtojnar changed the title Fix p11-kit builds on non-NixOS systems (single-user install) p11-kit: Fix builds on non-NixOS systems (single-user install) Jan 1, 2024
jonathanlking added a commit to jonathanlking/nixpkgs that referenced this pull request Jan 1, 2024
Add back FAKED_MODE environment variable.
This was removed in 3ca33e5, which
caused tests to be run (and fail) that were previously skipped.

nixpkgs upstreaming PR: NixOS#278099
@Artturin Artturin merged commit 4757f0e into NixOS:staging Jan 27, 2024
28 checks passed
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.

6 participants