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

pagmo2, pythonPackages.pygmo refactor fix broken packages #51399

Closed
wants to merge 3 commits into
base: master
from

Conversation

@costrouc
Copy link
Contributor

costrouc commented Dec 2, 2018

Motivation for this change

Fixed issues with #50645 for merging.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option 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/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@Mic92

This comment has been minimized.

Copy link
Contributor

Mic92 commented Dec 2, 2018

@GrahamcOfBorg build pythonPackages.pygmo

@worldofpeace worldofpeace self-assigned this Dec 6, 2018

pkgs/development/python-modules/pygmo/default.nix Outdated

buildInputs = [ cmake eigen nlopt ipopt boost pagmo2 ];
propagatedBuildInputs = [ numpy cloudpickle ipyparallel numba ];

preBuild = ''
cp -v -r $src/* .
cmake -DCMAKE_INSTALL_PREFIX=$out -DPAGMO_BUILD_TESTS=no -DCMAKE_SYSTEM_NAME=Linux -DPagmo_DIR=${pagmo2} -DPAGMO_BUILD_PYGMO=yes -DPAGMO_BUILD_PAGMO=no -DPAGMO_WITH_EIGEN3=yes -DPAGMO_WITH_NLOPT=yes -DNLOPT_LIBRARY=${nlopt}/lib/libnlopt_cxx.so -DPAGMO_WITH_IPOPT=yes -DIPOPT=${ipopt}
cmake -DCMAKE_INSTALL_PREFIX=$out -DPAGMO_BUILD_TESTS=no -DCMAKE_SYSTEM_NAME=Linux -DPagmo_DIR=${pagmo2} -DPAGMO_BUILD_PYGMO=yes -DPAGMO_BUILD_PAGMO=no -DPAGMO_WITH_EIGEN3=yes -DPAGMO_WITH_NLOPT=yes -DNLOPT_LIBRARY=${nlopt}/lib/libnlopt.so -DPAGMO_WITH_IPOPT=yes

This comment has been minimized.

@worldofpeace

worldofpeace Dec 6, 2018

Member

I'm seeing

CMake Warning:
  Manually-specified variables were not used by the project:

    PAGMO_BUILD_TESTS
    PAGMO_WITH_EIGEN3
    PAGMO_WITH_IPOPT
    PAGMO_WITH_NLOPT

So I think we can drop those for the pygmo build.

This comment has been minimized.

@Mic92

Mic92 Dec 6, 2018

Contributor

Actually python could become an build output of pagmo2.
There is a function called toPythonModule.
Do you know if that works? @dotlambda for outputs of derivation?

This comment has been minimized.

@worldofpeace

This comment has been minimized.

@dotlambda

dotlambda Dec 11, 2018

Member

Do you know if that works? @dotlambda for outputs of derivation?

It doesn't because it uses overrideAttrs.

This comment has been minimized.

@dotlambda

dotlambda Dec 11, 2018

Member

And I think doing anything to use the compilation output of pagmo will fail because we have to compile with different versions of python.

This comment has been minimized.

@Mic92

Mic92 Dec 12, 2018

Contributor

Please also split the lines like this:

cmake -DCMAKE_INSTALL_PREFIX=$out \
  -DPAGMO_BUILD_TESTS=no \
  -DCMAKE_SYSTEM_NAME=Linux
  ...

Do you really have to pass CMAKE_SYSTEM_NAME?

@worldofpeace

This comment has been minimized.

Copy link
Member

worldofpeace commented Dec 11, 2018

@worldofpeace worldofpeace force-pushed the costrouc:fix-broken-python-unstable branch to 4e3a564 Dec 12, 2018

@worldofpeace

This comment has been minimized.

Copy link
Member

worldofpeace commented Dec 12, 2018

I've addressed our feedback and resolved the conflict.

@GrahamcOfBorg build pagmo2 pythonPackages.pygmo python3Packages.pygmo python3Packages.dftfit

@dotlambda

This comment has been minimized.

Copy link
Member

dotlambda commented Dec 12, 2018

This is what I was able to come up with: dotlambda@4d0a561
I don't know if it's any better or if it's even working correctly.

@Mic92

This comment has been minimized.

Copy link
Contributor

Mic92 commented Dec 12, 2018

@dotlambda I prefer your solution because it does not use bare cmake commands and instead rely on our cmake support hooks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.