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

Fix FreeCAD build 19.03 #57879

Merged
merged 3 commits into from Mar 24, 2019
Merged

Fix FreeCAD build 19.03 #57879

merged 3 commits into from Mar 24, 2019

Conversation

timor
Copy link
Member

@timor timor commented Mar 18, 2019

Motivation for this change

Some dependencies of FreeCAD have not been building since a recent change to buildPythonPackage. This fixes the python modules necessary for FreeCAD, which should help with
#56826

This PR is only intended to fix the build for 19.03. There is another PRs which additionally updates a bunch of dependencies (#57121), that should be pulled in in the long run.

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.

@FRidh
Copy link
Member

FRidh commented Mar 19, 2019

Fixes should typically go to master first, and then cherry-picked (with -x flag) to a release branch.

@timor
Copy link
Member Author

timor commented Mar 19, 2019

Fixes should typically go to master first

That certainly was my intention. Is something missing?

@@ -16,8 +16,12 @@ buildPythonPackage rec {
sha256 = "18n14ha2d3j3ghg2f2aqnf2mks94nn7ma9ii7vkiwcay93zm82cf";
};

nativeBuildInputs = with pkgs; [
swig1 coin3d soqt
Copy link
Member

Choose a reason for hiding this comment

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

You're sure these should only be native? Did you actually try using pivy?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, not sure. I tested compilation and running FreeCAD. I don't know how to test, from within FreeCAD, that the coin3d library is called correctly. Should I add it to buildInputs too, to be on the safe side?

Copy link
Member

Choose a reason for hiding this comment

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

My intuition is that both, coin3d and soqt, don't belong into nativeBuildInputs but buildInputs. If that works, I guess we can leave it as is.
However, we should eventually bring pivy up to date and maybe try running the tests as in https://github.com/FreeCAD/pivy/blob/0.6.4/.travis.yml#L20.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, they were there, until b4acd97 specifically set strictDeps to true, but states that compile/runtime deps belong to buildInputs. These two things seem to be opposed to each other, though, since this has the effect of CFLAGS not
including the buildInputs dependencies anymore. Equally probable that I misunderstood the whole thing, though.

TLDR: no idea, really.

Note that there is another WIP PR (see description) which deals with bringing pivy and everything else up-to-date.

Copy link
Member

Choose a reason for hiding this comment

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

Afaik, swig is only used at build time whereas the other two are runtime dependencies. Hence coin3d and soqt should definitely be in buildInputs. After looking at this further I think it is required to also put them in nativeBuildInputs because coin-config soqt-config are called at build time.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@dotlambda
Copy link
Member

@GrahamcOfBorg build python2.pkgs.pivy python3.pkgs.pivy python2.pkgs.pyside-tools python3.pkgs.pyside-tools

@dotlambda
Copy link
Member

@GrahamcOfBorg build python2.pkgs.pivy python2.pkgs.pysideTools python3.pkgs.pysideTools

Copy link
Member

@dotlambda dotlambda left a comment

Choose a reason for hiding this comment

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

Please correct the commit messages: the attribute is pythonPackages.pysideTools.

@dotlambda dotlambda added the 9.needs: port to stable A PR needs a backport to the stable release. label Mar 24, 2019
@timor
Copy link
Member Author

timor commented Mar 24, 2019

Please correct the commit messages: the attribute is pythonPackages.pysideTools.

Right, thanks! Changed the messages.

@dotlambda
Copy link
Member

@GrahamcOfBorg build python2.pkgs.pivy python2.pkgs.pysideTools python3.pkgs.pysideTools

@dotlambda dotlambda merged commit 5bf6a13 into NixOS:master Mar 24, 2019
@dotlambda
Copy link
Member

@timor Thanks a lot!

@dotlambda
Copy link
Member

backported in 18e7ce4, 1bbc983, and da39363

@timor timor deleted the fix-freecad-build-19.03 branch March 24, 2019 19:05
@samueldr samueldr removed the 9.needs: port to stable A PR needs a backport to the stable release. label Apr 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants