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

gen-oath-safe: add dependency on file command #88629

Merged
merged 1 commit into from Jul 11, 2020

Conversation

wamserma
Copy link
Member

@wamserma wamserma commented May 22, 2020

Motivation for this change

file was not listed as a dependency, causing the program to throw an error.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@wamserma
Copy link
Member Author

@grahamc OfBorg seems stuck on "Fetching PR" - is this caused by GitHub being a bit wobbly yesterday or simply high load on OfBorg?

@wamserma
Copy link
Member Author

wamserma commented Jun 4, 2020

ping @makefu (maintainer)

Copy link
Contributor

@makefu makefu left a comment

Choose a reason for hiding this comment

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

LGTM

@wamserma
Copy link
Member Author

@FRidh merge?

@wamserma
Copy link
Member Author

/marvin opt-in

@marvin-mk2
Copy link

marvin-mk2 bot commented Jul 10, 2020

Hi! I'm an experimental bot. My goal is to guide this PR through its stages, hopefully ending with a merge. You can read up on the usage here.

@marvin-mk2 marvin-mk2 bot added the marvin label Jul 10, 2020
@wamserma
Copy link
Member Author

/status needs-merger

@timokau
Copy link
Member

timokau commented Jul 11, 2020

You have a little typo, the status is called needs_merger (and you need to set to needs_reviewer first anyway).

Diff looks good to me. Would be nice if the tool had a little sanity-check test script, but at least this fixes it for now. ofBorg failure is indeed weird, lets see if we can fix it by turning it off and on again.

@GrahamcOfBorg eval
@GrahamcOfBorg build gen-oath-safe

@timokau
Copy link
Member

timokau commented Jul 11, 2020

On IRC by cole-h:

Notice: ofborg evaluators and x86_64-linux builders are down. Buildkite seems to be having trouble, so I can't try to redeploy, either. Sory for the inconvenience.

Though it is weird that this PR already got stuck in May. Anyway, nix-review passes and this looks safe enough.

@timokau
Copy link
Member

timokau commented Jul 11, 2020

Just for testing
/status needs_merger

@marvin-mk2 marvin-mk2 bot requested a review from timokau July 11, 2020 08:34
@timokau timokau merged commit 16bfe5b into NixOS:master Jul 11, 2020
@wamserma wamserma deleted the gen-oath-safe-fix-deps branch July 11, 2020 10:49
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.

None yet

3 participants