Skip to content

perlPackages: init 3 perl packages#135667

Merged
stigtsp merged 3 commits intoNixOS:masterfrom
inkpot-monkey:3-perlPackages
Sep 8, 2021
Merged

perlPackages: init 3 perl packages#135667
stigtsp merged 3 commits intoNixOS:masterfrom
inkpot-monkey:3-perlPackages

Conversation

@inkpot-monkey
Copy link
Contributor

Motivation for this change

Dependencies for ngi-nix project

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all packages 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/)
  • 21.11 Release Notes (or backporting 21.05 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.

@inkpot-monkey inkpot-monkey requested a review from stigtsp as a code owner August 25, 2021 12:30
@inkpot-monkey
Copy link
Contributor Author

The BarcodeZBar module is sourced through the zbar pkg in nixpkgs but when building errors with:

Warning (mostly harmless): No library found for -lzbar

Contrary to this log it isnt harmless at all.

Not sure why propagatedBuildInputs and NIX_LDFLAGS are not working here but modifying the makefile.PL to include zbar.lib directly works

Processor.t requires network access

@inkpot-monkey inkpot-monkey force-pushed the 3-perlPackages branch 2 times, most recently from 93cbb05 to 6c4f8e1 Compare August 25, 2021 12:38
Copy link
Member

Choose a reason for hiding this comment

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

What's the difference with https://metacpan.org/release/SPADIX/Barcode-ZBar-0.04 ? Some glance at https://github.com/mchehab/zbar/commits/master/perl tells me there's substantial changes, but users may be confused as to the provenance of this module.

Might be worth convincing upstream to push a new version to CPAN.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The metacpan module was last updated in 2009 and uses a zbar version from that time (before it was forked onto github).

The changes in the github repo are to update the module so that it works with the current version.

Unfortunately it doesn't seem like the cpan maintainer is around anymore, not sure how cpan modules work exactly but could the current maintainer of zbar take ownership of that? Or would it be better to suggest that he makes a new cpan module (similar to graphviz and graphviz2)?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, there is a process for adopting CPAN modules outlined in http://neilb.org/2013/07/24/adopt-a-module.html that upstream can do - I'd be happy to lend a hand and talk to the CPAN admins if needed 💪

It is also possible to make a new Barcode::ZBar2 dist like what GraphViz did - the namespace changes will be necessary due to how CPAN arranges ownership of namespaces, if not taking the adoption route.

Copy link
Member

Choose a reason for hiding this comment

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

So Barcode-ZBar 0.04 on CPAN is not the same as ${zbar.src}/perl ?

NetServerSSPrefork was a similar case where the we went for an updated version from the authors repo, instead of the latest on CPAN. I suggest marking the version like "0.04pre" or "unstable-YYYY-MM-DD" if the CPAN version can't be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is correct the zbar.src is different from the CPAN version. The Perl module was forked at the same time that the main project was and has been updated since to maintain compatability.

Copy link
Member

@stigtsp stigtsp Aug 25, 2021

Choose a reason for hiding this comment

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

Works for me.

Can you add a short comment describing the reason for it?

Also, I'd prefer setting something like version = "0.04pre" since it kinda matches $Barcode::ZBar::VERSION - if that's ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure ill do that now

Copy link
Member

Choose a reason for hiding this comment

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

Just a little nitpick: Can you move the comment to i.e. above the src attribute, so it is more clear that it belongs to this derivation? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any idea why the tests are failing? Its building on my machine with
nix build .#perlPackages.BarcodeZBar

Copy link
Member

@stigtsp stigtsp Aug 25, 2021

Choose a reason for hiding this comment

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

The packages build fine for me too, however the basic evaluation checks fail with:

...
2021-08-25T14:05:55.8834116Z checking Nixpkgs on aarch64-darwin
2021-08-25T14:06:35.4812678Z error: while querying the derivation named 'perl5.32.1-Barcode-ZBar-0.04pre':
2021-08-25T14:06:35.4815914Z while evaluating the attribute '__impureHostDeps' of the derivation 'perl5.32.1-Barcode-ZBar-0.04pre' at /nix/store/jzigajkia0hjyjq1xakyfkcpisxqfdfm-source/pkgs/stdenv/generic/make-derivation.nix:205:7:
2021-08-25T14:06:35.4829281Z while evaluating anonymous function at /nix/store/jzigajkia0hjyjq1xakyfkcpisxqfdfm-source/pkgs/stdenv/generic/make-derivation.nix:188:32, called from /nix/store/jzigajkia0hjyjq1xakyfkcpisxqfdfm-source/pkgs/stdenv/generic/make-derivation.nix:188:17:
2021-08-25T14:06:35.4832733Z while evaluating 'chooseDevOutputs' at /nix/store/jzigajkia0hjyjq1xakyfkcpisxqfdfm-source/lib/attrsets.nix:500:22, called from undefined position:
2021-08-25T14:06:35.4835556Z undefined variable 'TestMore' at /nix/store/jzigajkia0hjyjq1xakyfkcpisxqfdfm-source/pkgs/top-level/perl-packages.nix:1234:20
2021-08-25T14:06:35.5536880Z builder for '/nix/store/6cy9172l569fpkylir23939kianysr7j-nixpkgs-release-checks.drv' failed with exit code 1
2021-08-25T14:06:35.5735589Z error: build of '/nix/store/6cy9172l569fpkylir23939kianysr7j-nixpkgs-release-checks.drv' failed
2021-08-25T14:06:35.5783136Z ##[error]Process completed with exit code 100.

https://github.com/NixOS/nixpkgs/pull/135667/checks?check_run_id=3422793076

I'm not sure why it fails, but could be an issue with the action or the platform it's running on. (TestMore correctly points to null as it's a part of core)

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Aug 25, 2021
@stigtsp stigtsp changed the title Init 3 perl packages perlPackages: init 3 perl packages Aug 25, 2021
@inkpot-monkey inkpot-monkey force-pushed the 3-perlPackages branch 3 times, most recently from 48f00f7 to 6e48536 Compare August 25, 2021 13:58
@stigtsp
Copy link
Member

stigtsp commented Aug 25, 2021

@GrahamcOfBorg build perlPackages.BarcodeZBar perlPackages.TestSnapshot perlPackages.DataSectionSimple

@stigtsp
Copy link
Member

stigtsp commented Aug 25, 2021

@GrahamcOfBorg eval

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
buildInputs =[ TestMore ExtUtilsMakeMaker ];
buildInputs = [ TestMore ExtUtilsMakeMaker ];

TestMore does not exist. Do you mean TestSimple?

Copy link
Member

Choose a reason for hiding this comment

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

@SuperSandro2000 TestMore is linked to null since it is in core. #135667 (comment)

(Not sure why the "Basic evaluation checks" test fail. Thought it might be related to aarch64-darwin?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I think I see why now. It is failing on aarch64-darwin as the ZBar package where we are getting the src for the module is only packaged for x86_64-linux

Copy link
Member

Choose a reason for hiding this comment

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

Might need something like

Suggested change
buildInputs =[ TestMore ExtUtilsMakeMaker ];
buildInputs = [ TestMore ExtUtilsMakeMaker ];
perlPreHook = lib.optionalString stdenv.isDarwin "export LD=$CC";

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed TestMore to side step confusion and added in the suggestion from @zakame and it now the Basic Evaluation Checks pass 🚀

What is happening here?

Copy link
Member

@zakame zakame Aug 27, 2021

Choose a reason for hiding this comment

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

Yeah, it has to do with the macOS xcode-tools ld not being picked up instead of the Nix-provided gcc (which can also do linking.) You can see this through calling nix log on the perl5XXPackages.BarcodeZBar derivation, or through ofborg, e.g.

cc -c   -fno-common -DPERL_DARWIN -mmacosx-version-min=10.12 -fno-strict-aliasing -pipe -fstack-protector-strong -I/no-such-path/include -DPERL_USE_SAFE_PUTENV -Wno-error=implicit-function-declaration -O3   -DVERSION=\"0.04\" -DXS_VERSION=\"0.04\"  "-I/nix/store/bbmc66vbfpyri3qink1mylhmk1h97a7n-perl-5.34.0/lib/perl5/5.34.0/darwin-thread-multi-2level/CORE"   ZBar.c
rm -f blib/arch/auto/Barcode/ZBar/ZBar.bundle
LD_RUN_PATH="/nix/store/1iiggnhv90xja3n3j76bvvzy0h18dx46-zbar-0.23.90-lib/lib" cc  -mmacosx-version-min=10.12 -bundle -undefined dynamic_lookup -L/no-such-path/lib -fstack-protector-strong  ZBar.o  -o blib/arch/auto/Barcode/ZBar/ZBar.bundle  \
   -L/nix/store/1iiggnhv90xja3n3j76bvvzy0h18dx46-zbar-0.23.90-lib/lib -lzbar   \

@zakame
Copy link
Member

zakame commented Aug 27, 2021

@GrahamcOfBorg build perlPackages.BarcodeZBar perlPackages.TestSnapshot perlPackages.DataSectionSimple

Copy link
Member

@stigtsp stigtsp left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the PR :)

@stigtsp stigtsp merged commit 4ee9590 into NixOS:master Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants