Skip to content

pngpp: darwin support#33756

Merged
LnL7 merged 1 commit intoNixOS:masterfrom
acowley:pngpp-darwin
Jan 11, 2018
Merged

pngpp: darwin support#33756
LnL7 merged 1 commit intoNixOS:masterfrom
acowley:pngpp-darwin

Conversation

@acowley
Copy link
Contributor

@acowley acowley commented Jan 11, 2018

  1. Building with clang is addressed by navigating a minor #if in some
    of the code.

  2. I noticed that even when things were building correctly, passing
    ${out} as a variable assignment to make was actually not working:
    there were compiler warnings about missing include directories whose
    bogus paths involved the literal string out. I ended up fixing this
    by performing the assignment to the make variable PREFIX in the
    Makefile itself.

Motivation for this change

Make pngpp build on darwin.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@acowley
pngpp: darwin support
ed559bf 
1) Building with clang is addressed by navigating a minor #if in some
of the code.

2) I noticed that even when things were building correctly, passing
`${out}` as a variable assignment to `make` was actually not working:
there were compiler warnings about missing include directories whose
bogus paths involved the literal string `out`. I ended up fixing this
by performing the assignment to the make variable `PREFIX` in the
`Makefile` itself.
@GrahamcOfBorg GrahamcOfBorg added 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 Jan 11, 2018
@LnL7
Copy link
Member

LnL7 commented Jan 11, 2018

@GrahamcOfBorg build pngpp

GrahamcOfBorg
GrahamcOfBorg reviewed Jan 11, 2018
View reviewed changes
Copy link

@GrahamcOfBorg GrahamcOfBorg left a comment

Choose a reason for hiding this comment

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

Failure for system: x86_64-darwin

build flags: -j1 -l1 SHELL=/nix/store/hgsbnqhg7zr5vbjazb250ac4wnj3cfhy-bash-4.4-p12/bin/bash docs
VERSION=0.2.9 doxygen
dyld: Library not loaded: /System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices
  Referenced from: /nix/store/lqmhaf173dw3kzc46cgvgcc9n8pacma5-doxygen-1.8.11/bin/doxygen
  Reason: no suitable image found.  Did find:
	/System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices: file system sandbox blocked stat()
	/System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices: file system sandbox blocked stat()
make: *** [Makefile:100: docs] Abort trap: 6
builder for '/nix/store/y7cacm6qlwqlyvpb5km26zg2addffayz-pngpp-0.2.9.drv' failed with exit code 2
error: build of '/nix/store/y7cacm6qlwqlyvpb5km26zg2addffayz-pngpp-0.2.9.drv' failed

GrahamcOfBorg
GrahamcOfBorg reviewed Jan 11, 2018
View reviewed changes
Copy link

@GrahamcOfBorg GrahamcOfBorg left a comment

Choose a reason for hiding this comment

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

Success for system: x86_64-linux

finalizing index lists...
writing tag file...
lookup cache used 1277/65536 hits=5893 misses=1327
finished...
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/4jcmd904wnkx24klr8cq6l6kwq3iaj98-pngpp-0.2.9
strip is /nix/store/wxn5gn8amxm1w0ikcx4gbs8a17wvss4j-binutils-2.28.1/bin/strip
patching script interpreter paths in /nix/store/4jcmd904wnkx24klr8cq6l6kwq3iaj98-pngpp-0.2.9
checking for references to /tmp/nix-build-pngpp-0.2.9.drv-0 in /nix/store/4jcmd904wnkx24klr8cq6l6kwq3iaj98-pngpp-0.2.9...
/nix/store/4jcmd904wnkx24klr8cq6l6kwq3iaj98-pngpp-0.2.9

GrahamcOfBorg
GrahamcOfBorg reviewed Jan 11, 2018
View reviewed changes
Copy link

@GrahamcOfBorg GrahamcOfBorg left a comment

Choose a reason for hiding this comment

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

Success for system: aarch64-linux

finalizing index lists...
writing tag file...
lookup cache used 1277/65536 hits=5893 misses=1327
finished...
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/yrhp9ag3m666qrf8d6phl1yffg8h7mx2-pngpp-0.2.9
strip is /nix/store/c6qj0j45xizkrx58i65j75a5ysmqhgrs-binutils-2.28.1/bin/strip
patching script interpreter paths in /nix/store/yrhp9ag3m666qrf8d6phl1yffg8h7mx2-pngpp-0.2.9
checking for references to /build in /nix/store/yrhp9ag3m666qrf8d6phl1yffg8h7mx2-pngpp-0.2.9...
/nix/store/yrhp9ag3m666qrf8d6phl1yffg8h7mx2-pngpp-0.2.9

@LnL7
Copy link
Member

LnL7 commented Jan 11, 2018

@GrahamcOfBorg build pngpp

GrahamcOfBorg
GrahamcOfBorg reviewed Jan 11, 2018
View reviewed changes
Copy link

@GrahamcOfBorg GrahamcOfBorg left a comment

Choose a reason for hiding this comment

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

Success for system: aarch64-linux

/nix/store/yrhp9ag3m666qrf8d6phl1yffg8h7mx2-pngpp-0.2.9

GrahamcOfBorg
GrahamcOfBorg reviewed Jan 11, 2018
View reviewed changes
Copy link

@GrahamcOfBorg GrahamcOfBorg left a comment

Choose a reason for hiding this comment

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

Success for system: x86_64-linux

finalizing index lists...
writing tag file...
lookup cache used 1277/65536 hits=5893 misses=1327
finished...
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/4jcmd904wnkx24klr8cq6l6kwq3iaj98-pngpp-0.2.9
strip is /nix/store/wxn5gn8amxm1w0ikcx4gbs8a17wvss4j-binutils-2.28.1/bin/strip
patching script interpreter paths in /nix/store/4jcmd904wnkx24klr8cq6l6kwq3iaj98-pngpp-0.2.9
checking for references to /tmp/nix-build-pngpp-0.2.9.drv-0 in /nix/store/4jcmd904wnkx24klr8cq6l6kwq3iaj98-pngpp-0.2.9...
/nix/store/4jcmd904wnkx24klr8cq6l6kwq3iaj98-pngpp-0.2.9

GrahamcOfBorg
GrahamcOfBorg reviewed Jan 11, 2018
View reviewed changes
Copy link

@GrahamcOfBorg GrahamcOfBorg left a comment

Choose a reason for hiding this comment

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

Success for system: x86_64-darwin

Generating file member index...
Generating example index...
finalizing index lists...
writing tag file...
lookup cache used 1277/65536 hits=5893 misses=1327
finished...
post-installation fixup
strip is /nix/store/bhfkh37smh9lc5bngdha6zx98k6w6cgd-cctools-binutils-darwin/bin/strip
patching script interpreter paths in /nix/store/wgvyk3356y5kk10hj5j26yrlqfxk4wqj-pngpp-0.2.9
/nix/store/wgvyk3356y5kk10hj5j26yrlqfxk4wqj-pngpp-0.2.9

@LnL7
Copy link
Member

LnL7 commented Jan 11, 2018

We disabled sandboxing on the builders, seems like there's a some kind of problem with propagating impure dependencies.

LnL7
LnL7 reviewed Jan 11, 2018
View reviewed changes
pkgs/development/libraries/png++/default.nix
preConfigure = stdenv.lib.optionalString stdenv.isDarwin ''
substituteInPlace error.hpp --replace "#if (_POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && !_GNU_SOURCE" "#if (__clang__ || _POSIX_C_SOURCE >= 200112L || _XOPEN_SOURCE >= 600) && !_GNU_SOURCE"
'' + ''
sed "s|\(PNGPP := .\)|PREFIX := ''${out}\n\\1|" -i Makefile
Copy link
Member

@LnL7 LnL7 Jan 11, 2018

Choose a reason for hiding this comment

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

makeFlags are quoted, did you try this? makeFlags = [ "PREFIX=$(out)" ];

Copy link
Contributor Author

@acowley acowley Jan 11, 2018

Choose a reason for hiding this comment

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

I did.

Copy link
Member

@LnL7 LnL7 Jan 11, 2018

Choose a reason for hiding this comment

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

Weird, that usually works.

Copy link
Contributor Author

@acowley acowley Jan 11, 2018

Choose a reason for hiding this comment

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

To expand, I tried everything I could think of, $(out) in particular because it’s given as an example in the nixpkgs manual. It failed when running make test. I’d be interested if anybody can figure it out.

@LnL7 LnL7 merged commit 10c6439 into NixOS:master Jan 11, 2018
@LnL7 LnL7 mentioned this pull request Jan 26, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@LnL7 LnL7 LnL7 left review comments

@GrahamcOfBorg GrahamcOfBorg GrahamcOfBorg left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@acowley @LnL7 @GrahamcOfBorg