freecad: build with netgen enabled (fixes #382847)#408577
freecad: build with netgen enabled (fixes #382847)#408577eteq wants to merge 2 commits intoNixOS:masterfrom
Conversation
NixOSInfra
added
the
12.first-time contribution
github-actions
bot
added
10.rebuild-darwin: 0
|
Thanks for the credit! It's not necessary to use the commit, though, it's not formatted right for nixpkgs anyway. Plus it's clear from the followups that I missed several things. Thanks for following up on this bug! Most of this will still be necessary with an update to 1.0.1. I'm not a nixpkgs developer either, but I think the title of this PR should be a bit different. Something like "freecad: build with netgen enabled (#382847)" would likely be appropriate. I'd probably squash all the commits into one commit with the same title, too. Take a look at some recent commits to the repo to see the general idea. |
LordGrimmauld
left a comment
LordGrimmauld
left a comment
There was a problem hiding this comment.
Couple notes:
First of, welcome to nixpkgs 👋
Second, make sure nixpkgs isn't broken in intermediary commits. This ensures people later bisecting through nixpkgs don't get dropped into a broken state between your changes. For this, it might make sense to do two commits:
- freecad: update python (contains the python312Packages bump)
- freecad: enable netgen support (with a commit message body that points to the issue, and a short explanation why.)
Adding "-DBUILD_NETGEN=ON" in a commit without adding netgen to inputs would be broken. Remember that commits need to follow the pattern <package>: <change>. Similarly, the PR title should probably be freecad: add netgen support. I am not a committer, i can't force-edit your title.
Further: The python changes are fine. While technically python3Packages is preferred over python312Packages, we will soon bump python to point to 3.13 by default, which would break shiboken2 (see #407932). This means pinning to 3.12 until we drop Qt5 from freecad is a good idea, you did everything right.
|
|
netgen and the FEM plugin's use of it wasn't being built, causing the FEM plugin to fail (NixOS#382847). This commit explicitly turns on them on and adds netgen as a dependency. Co-authored-by: Michael Leuchtenburg <michael@slashhome.org>
the freecad python deps are based on the python nix package, which was updated to 3.12 but freecad's python deps were still 3.11. This commit updates the dependent packages to be for 3.12.
eteq
changed the title
|
Alright I squashed to two commits along the lines of your suggestion, @LordGrimmauld, thanks for the review! And in the netgen one I changed your status to co-author, @dyfrgi, which I think rightly reflects the work you did setting me down the path to finish the job. Do I need to ping someone with merge rights too or just sit tight until they can get to this? |
|
I adopted freecad in #408873, so technically i am package maintainer now. I rely on freecad for some aspects in my thesis, so i already noticed whenever it broke. |
|
|
No other log output. |
|
Ah no, it is netgen, i can't read |
|
strace says: Might it be missing |
Previously, FreeCAD attempted to write at apths in `/`, such as `/tmpNETGEN_3825524_190303824.out`. This was discovered while packaging netgen support for FreeCAD in nixpkgs [1]. The fix is trivial: Just add to `/tmp/` to actually be in the tmp directory. [1] NixOS/nixpkgs#408577 (comment)
Previously, FreeCAD attempted to write at apths in `/`, such as `/tmpNETGEN_3825524_190303824.out`. This was discovered while packaging netgen support for FreeCAD in nixpkgs [1]. The fix is trivial: Just add to `/tmp/` to actually be in the tmp directory. [1] NixOS/nixpkgs#408577 (comment)
|
I am currently testing this change - if it does work, you might want to cherry-pick LordGrimmauld@0a0a656 into here. |
|
Nice find, @LordGrimmauld. That code hasn't changed in 9 years. I wonder why we're triggering the bug now and weren't before? There must be something in the way that code is called that changed without breaking any tests (are there tests for this?) and without anyone noticing. In the original repo, that function looks different than it does here. In fact, it looked different to begin with - it calls a helper function from the SALOME kernel to get the base tmpdir. I wonder if that function was not being called before? |
I suspect people run freecad in containers a lot. Writing to / isn't much of an issue in flatpak i believe, fuse just hides that away? But indeed, this is a bit surprising. It might also just be noone was running netgen to start with... |
|
Also, patching to |
|
Okay, i give up for now. Just a bunch of crashes and no sane way to debug... |
|
Curiously, in my own testing I'm seeing a segfault at the same spot now (I could swear I wasn't initially, but I might be mis-remembering getting it to work on a non-nix install...), but I don't see any attempts to access are you seeing a different stack trace, @LordGrimmauld, or were you not able to get debug symbols to get this in the first place (that is, I'm trying to figure out if segfault is separate from the |
its the same stack trace. And while i initially suspected the version comparison for smesh was broken (comparing strings against bitshifted numbers), fixing that also did not help. At which point i gave up. |
wegank
added
the
2.status: merge conflict
nixpkgs-ci
bot
added
the
2.status: stale
8 participants
This is a fix to the freecad package.nix that I think closes #382847 - note that the first step that sent me down this path was from @dyfrgi
in #382847 (comment), so I added their commit from that comment as the first commit in this PR (credit where credit is due!).
Now the more iffy parts:
python312instead ofpython? But that might cause more mucking up of the dependency tree.Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.