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

grex: init at 1.1.0 #95316

Closed
wants to merge 1 commit into from
Closed

grex: init at 1.1.0 #95316

wants to merge 1 commit into from

Conversation

@colemickens
Copy link
Member

@colemickens colemickens commented Aug 13, 2020

Fixes #95315.

This introduces grex to nixpkgs:

Motivation for this change
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.
@colemickens
Copy link
Member Author

@colemickens colemickens commented Aug 13, 2020

> nixpkgs-review rev 2a737ef26bcb683384261a9b8c8ce0065a86ba61
$ git -c fetch.prune=false fetch --force https://github.com/NixOS/nixpkgs master:refs/nixpkgs-review/0
$ git worktree add /home/cole/.cache/nixpkgs-review/rev-2a737ef26bcb683384261a9b8c8ce0065a86ba61/nixpkgs 28224516e5cfce2782fc8bbc700ca814d4648cb7
Preparing worktree (detached HEAD 28224516e5c)
Updating files: 100% (22432/22432), done.
HEAD is now at 28224516e5c Merge pull request #92964 from lopsided98/initrd-secrets-compressor
$ nix-env -f /home/cole/.cache/nixpkgs-review/rev-2a737ef26bcb683384261a9b8c8ce0065a86ba61/nixpkgs -qaP --xml --out-path --show-trace
warning: unknown setting 'experimental-features'
$ git merge --no-commit 2a737ef26bcb683384261a9b8c8ce0065a86ba61
Auto-merging pkgs/top-level/all-packages.nix
Automatic merge went well; stopped before committing as requested
$ nix-env -f /home/cole/.cache/nixpkgs-review/rev-2a737ef26bcb683384261a9b8c8ce0065a86ba61/nixpkgs -qaP --xml --out-path --show-trace --meta
warning: unknown setting 'experimental-features'
1 package added:
grex (init at 1.1.0)

warning: unknown setting 'experimental-features'
$ nix build --no-link --keep-going --option build-use-sandbox relaxed -f /home/cole/.cache/nixpkgs-review/rev-2a737ef26bcb683384261a9b8c8ce0065a86ba61/build.nix
warning: unknown setting 'experimental-features'
[4 built, 63 copied (1178.2 MiB), 205.8 MiB DL]
1 package built:
grex

warning: unknown setting 'experimental-features'
$ nix-shell /home/cole/.cache/nixpkgs-review/rev-2a737ef26bcb683384261a9b8c8ce0065a86ba61/shell.nix
warning: unknown setting 'experimental-features'

[nix-shell:~/.cache/nixpkgs-review/rev-2a737ef26bcb683384261a9b8c8ce0065a86ba61]$ 

@TomSmeets
Copy link
Contributor

@TomSmeets TomSmeets commented Aug 14, 2020

Note that another pull-request is open at #76539 for the 1.0.0 version

Copy link
Contributor

@jonringer jonringer left a comment

otherwise LGTM

doCheck = false;

Copy link
Contributor

@jonringer jonringer Aug 24, 2020

don't feel really strongly about this, but I can't think of another way to very that darwin works:

Suggested change
doCheck = false;
doCheck = false;
doInstallCheck = true;
installCheckPhase = ''
$out/bin/grex --help > /dev/null
'';

with rustPlatform;

buildRustPackage rec {
Copy link
Contributor

@zowoq zowoq Aug 25, 2020

Suggested change
with rustPlatform;
buildRustPackage rec {
rustPlatform.buildRustPackage rec {

sha256 = "1viph7ki6f2akc5mpbgycacndmxnv088ybfji2bfdbi5jnpyavvs";
};

nativeBuildInputs = [ cmake pkgconfig perl installShellFiles ];
Copy link
Contributor

@zowoq zowoq Aug 25, 2020

None of these seem to be needed.

Suggested change
nativeBuildInputs = [ cmake pkgconfig perl installShellFiles ];

buildInputs = [ zlib ]
++ stdenv.lib.optionals stdenv.isDarwin [
libiconv darwin.apple_sdk.frameworks.Security ]
;
Copy link
Contributor

@zowoq zowoq Aug 25, 2020

Only Security seems to be needed.

Suggested change
buildInputs = [ zlib ]
++ stdenv.lib.optionals stdenv.isDarwin [
libiconv darwin.apple_sdk.frameworks.Security ]
;
buildInputs = stdenv.lib.optionals stdenv.isDarwin [ darwin.apple_sdk.frameworks.Security ];

# Some tests fail, but Travis ensures a proper build
doCheck = false;
Copy link
Contributor

@zowoq zowoq Aug 25, 2020

The check passes on darwin and linux.

Suggested change
# Some tests fail, but Travis ensures a proper build
doCheck = false;

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Oct 10, 2020

@colemickens Can you please integrate the suggestions?

Edit: I have opened #100173 because the author did not answer in almost 2 months.

@SuperSandro2000 SuperSandro2000 mentioned this pull request Oct 10, 2020
10 tasks
@AndersonTorres
Copy link
Member

@AndersonTorres AndersonTorres commented Oct 12, 2020

Closed by stalling

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

6 participants