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

pcl: qt4 -> qt5; 1.8.0 -> 1.8.1 #30342

Merged
merged 1 commit into from Oct 14, 2017
Merged

pcl: qt4 -> qt5; 1.8.0 -> 1.8.1 #30342

merged 1 commit into from Oct 14, 2017

Conversation

acowley
Copy link
Contributor

@acowley acowley commented Oct 12, 2017

Motivation for this change

Get pcl building on darwin.

Drop the qt4 dependency as it is rotting.

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
Copy link
Contributor Author

acowley commented Oct 12, 2017

The Travis CI build failures are due to exceeding the time limit.

@peterhoeg
Copy link
Member

The way Qt5 dependencies are specified is different from Qt4.

  1. use libsForQt5.callPackage instead of regular callPackage.
  2. Don't pass "qt59" (due to the item above), instead use the sub attributes from the qt5 attribute set such as qtbase, qtx11extras and so on.

@acowley
Copy link
Contributor Author

acowley commented Oct 13, 2017

Thanks for the great feedback, @peterhoeg! I've made the changes you suggested, and had the extremely welcome surprise of those changes not changing the hash of the package, so I don't have to wait for another multi-hour build! 🙌


src = fetchFromGitHub {
owner = "PointCloudLibrary";
repo = "pcl";
rev = name;
sha256 = "1pki4y7mc2dryxc8wa7rs4hg74qab80rpy90jnw3j8fzf09kxcll";
sha256 = "05wvqqi2fyk5innw4mg356r71c1hmc9alc7xkf4g81ds3b3867xq";
};

enableParallelBuilding = true;

nativeBuildInputs = [ pkgconfig ];
Copy link
Member

Choose a reason for hiding this comment

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

cmake should also go into nativeBuildInputs.

@peterhoeg
Copy link
Member

so I don't have to wait for another multi-hour build!

I will strongly recommend working against nixos-unstable instead of master so that all your dependencies are pre-built. That dramatically cuts down on the build time.

@acowley
Copy link
Contributor Author

acowley commented Oct 13, 2017

I work off of channels/nixpkgs-unstable, so pcl is the only thing being built. It just takes ages. Which I will now experience because moving cmake does change the hash. I'll report back tomorrow.

@acowley
Copy link
Contributor Author

acowley commented Oct 13, 2017

Still works for me, so I hope we're good to go.

@peterhoeg
Copy link
Member

I'll try building it on NixOS.

@acowley
Copy link
Contributor Author

acowley commented Oct 13, 2017

If you'd like to try building something that uses PCL to ensure that the GUI components work, here is a test application they provide in their docs that I have packaged for nix for my own testing.

@peterhoeg peterhoeg merged commit 2fdfefa into NixOS:master Oct 14, 2017
@peterhoeg
Copy link
Member

Thanks!

@acowley acowley deleted the pcl-qt5 branch November 1, 2017 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants