Skip to content

freecad: add ifc support #330190

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 18, 2025
Merged

freecad: add ifc support #330190

merged 1 commit into from
Jan 18, 2025

Conversation

autra
Copy link
Contributor

@autra autra commented Jul 26, 2024 •
edited
Loading

Description of changes

This PR adds optional ifc support with ifcopenshell.

It depends on #312381 (fix ifcopenshell build)

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added the 6.topic: python Python is a high-level, general-purpose programming language. label Jul 26, 2024
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jul 26, 2024
@luzpaz
Copy link
Contributor

luzpaz commented Sep 24, 2024

Just a heads up

@autra
Copy link
Contributor Author

autra commented Sep 24, 2024

Thanks, we shouldn't step too much into each other shoes hopefully ;-)

@autra autra force-pushed the freecad_ifc_support branch from bad8c88 to 2014cd1 Compare October 6, 2024 18:05
@github-actions github-actions bot removed the 6.topic: python Python is a high-level, general-purpose programming language. label Oct 6, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. and removed 2.status: merge conflict This PR has merge conflicts with the target branch labels Oct 6, 2024
@autra autra force-pushed the freecad_ifc_support branch from 2014cd1 to 5d8f6e6 Compare October 11, 2024 12:00
@autra autra marked this pull request as ready for review October 11, 2024 12:00
@autra autra force-pushed the freecad_ifc_support branch 2 times, most recently from 4259d7b to 09d03a4 Compare October 23, 2024 15:01
@autra
Copy link
Contributor Author

autra commented Oct 23, 2024

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 330190

x86_64-linux

@autra autra force-pushed the freecad_ifc_support branch from 09d03a4 to c48fb6a Compare October 24, 2024 21:48
@autra
Copy link
Contributor Author

autra commented Oct 24, 2024

This is now ready. I took the opportunity to document the different options so that they appear in https://search.nixos.org/ (I hope it's the correct way, if it is, I think all packages should just do that).

@AndersonTorres
Copy link
Member

I think all packages should just do that

All packages can't do that. Indeed no package can do that.

This "configuration via input parameters" is too unstable and prone to errors.
#324199

Atemu is experimenting with the module system in #312432 to provide a better experience.

AndersonTorres
AndersonTorres suggested changes Oct 25, 2024
View reviewed changes
@autra autra force-pushed the freecad_ifc_support branch from c48fb6a to d1626fb Compare October 30, 2024 15:06
@autra
Copy link
Contributor Author

autra commented Oct 30, 2024

@AndersonTorres I added a README. For the option, I see no alternative right now :-)

@autra autra requested a review from AndersonTorres December 20, 2024 09:47
@autra
Copy link
Contributor Author

autra commented Dec 20, 2024

@gebner @AndersonTorres what you guys think about this now?

@AndersonTorres
Copy link
Member

Rebase.

@autra autra force-pushed the freecad_ifc_support branch from d1626fb to 79d0fea Compare December 20, 2024 13:46
@autra
Copy link
Contributor Author

autra commented Dec 20, 2024

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 330190

@autra
Copy link
Contributor Author

autra commented Dec 20, 2024

Rebase.

Done.

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one person. label Dec 21, 2024
GaetanLepage
GaetanLepage approved these changes Jan 16, 2025
View reviewed changes
Copy link
Contributor

@GaetanLepage GaetanLepage left a comment

Choose a reason for hiding this comment

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

LGTM

@GaetanLepage
Copy link
Contributor

Maybe add

passthru = {
  tests = {
    withIfcSupport = freecad.override { ifcSupport = true; };
  };
};

@autra
Copy link
Contributor Author

autra commented Jan 17, 2025

Maybe add

passthru = {
  tests = {
    withIfcSupport = freecad.override { ifcSupport = true; };
  };
};

The point is just to compile it once with ifc support during the test phase?

@GaetanLepage
Copy link
Contributor

The point is just to compile it once with ifc support during the test phase?

Exactly

@autra autra force-pushed the freecad_ifc_support branch from aa7161c to 621ff02 Compare January 18, 2025 16:46
@autra
Copy link
Contributor Author

autra commented Jan 18, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 330190

x86_64-linux

@autra
Copy link
Contributor Author

autra commented Jan 18, 2025

Updated @GaetanLepage @AndersonTorres

GaetanLepage
GaetanLepage reviewed Jan 18, 2025
View reviewed changes
Copy link
Contributor

@GaetanLepage GaetanLepage left a comment

Choose a reason for hiding this comment

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

Changes look good. Can you please squash everything in a single commit though ?

@autra autra force-pushed the freecad_ifc_support branch from 621ff02 to 64477a8 Compare January 18, 2025 22:03
@autra
Copy link
Contributor Author

autra commented Jan 18, 2025

Changes look good. Can you please squash everything in a single commit though ?

oups, forgot to squash my fixups indeed. Done now.

@GaetanLepage
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 330190

GaetanLepage
GaetanLepage approved these changes Jan 18, 2025
View reviewed changes
Copy link
Contributor

@GaetanLepage GaetanLepage left a comment

Choose a reason for hiding this comment

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

All good, thanks @autra !

@GaetanLepage GaetanLepage merged commit c6b7c70 into NixOS:master Jan 18, 2025
25 of 27 checks passed
@autra
Copy link
Contributor Author

autra commented Jan 18, 2025

Thanks for the reviews @GaetanLepage @AndersonTorres :-)

@autra autra deleted the freecad_ifc_support branch January 18, 2025 22:18
@LordGrimmauld
Copy link
Contributor

Why was ifc support default-disabled? According to #370121 this is functionality not just required by obscure plugins but by core functionality. This should probably be default-enabled.

LordGrimmauld added a commit to LordGrimmauld/nixpkgs that referenced this pull request Jul 5, 2025
@LordGrimmauld
freecad: enable ifc unconditionally
fa28145 
closes NixOS#370121

follow-up to NixOS#330190

The BIM workspace depends on ifc, there is little reason
why it should be conditional to a flag that causes a
from-source rebuild. It should just be always enabled.
@LordGrimmauld LordGrimmauld mentioned this pull request Jul 5, 2025
Merged
13 tasks
@LordGrimmauld
Copy link
Contributor

I propose making it unconditional: #422675

LordGrimmauld added a commit to LordGrimmauld/nixpkgs that referenced this pull request Jul 5, 2025
@LordGrimmauld
freecad: enable ifc unconditionally
1956444 
closes NixOS#370121

follow-up to NixOS#330190

The BIM workspace depends on ifc, there is little reason
why it should be conditional to a flag that causes a
from-source rebuild. It should just be always enabled.
@luzpaz
Copy link
Contributor

luzpaz commented Jul 5, 2025

Thank you!

LordGrimmauld added a commit to LordGrimmauld/nixpkgs that referenced this pull request Jul 6, 2025
@LordGrimmauld
freecad: enable ifc unconditionally
f772e38 
closes NixOS#370121

follow-up to NixOS#330190

The BIM workspace depends on ifc, there is little reason
why it should be conditional to a flag that causes a
from-source rebuild. It should just be always enabled.
@autra
Copy link
Contributor Author

autra commented Jul 16, 2025

I propose making it unconditional: #422675

@LordGrimmauld last time I checked upstream, it was optional (but indeed required as soon as you want to use BIM features), and I kept it optional just for the sake of keeping things as light as possible.

That being said, I'm only using freecad for opening ifc files so I'm all for enable it by default, thanks for having done that!

mdaniels5757 pushed a commit to mdaniels5757/nixpkgs that referenced this pull request Aug 16, 2025
@LordGrimmauld @mdaniels5757
freecad: enable ifc unconditionally
35b6372 
closes NixOS#370121

follow-up to NixOS#330190

The BIM workspace depends on ifc, there is little reason
why it should be conditional to a flag that causes a
from-source rebuild. It should just be always enabled.

(cherry picked from commit f772e38)
mdaniels5757 pushed a commit to mdaniels5757/nixpkgs that referenced this pull request Aug 16, 2025
@LordGrimmauld @mdaniels5757
freecad: enable ifc unconditionally
c7b5cca 
closes NixOS#370121

follow-up to NixOS#330190

The BIM workspace depends on ifc, there is little reason
why it should be conditional to a flag that causes a
from-source rebuild. It should just be always enabled.

(cherry picked from commit f772e38)
mdaniels5757 pushed a commit to mdaniels5757/nixpkgs that referenced this pull request Aug 16, 2025
@LordGrimmauld @mdaniels5757
freecad: enable ifc unconditionally
8ed6ce5 
closes NixOS#370121

follow-up to NixOS#330190

The BIM workspace depends on ifc, there is little reason
why it should be conditional to a flag that causes a
from-source rebuild. It should just be always enabled.

(cherry picked from commit f772e38)
mdaniels5757 pushed a commit to mdaniels5757/nixpkgs that referenced this pull request Aug 17, 2025
@LordGrimmauld @mdaniels5757
freecad: enable ifc unconditionally
2c1a96d 
closes NixOS#370121

follow-up to NixOS#330190

The BIM workspace depends on ifc, there is little reason
why it should be conditional to a flag that causes a
from-source rebuild. It should just be always enabled.

(cherry picked from commit f772e38)
mdaniels5757 pushed a commit to mdaniels5757/nixpkgs that referenced this pull request Aug 17, 2025
@LordGrimmauld @mdaniels5757
freecad: enable ifc unconditionally
e4732a8 
closes NixOS#370121

follow-up to NixOS#330190

The BIM workspace depends on ifc, there is little reason
why it should be conditional to a flag that causes a
from-source rebuild. It should just be always enabled.

(cherry picked from commit f772e38)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GaetanLepage GaetanLepage GaetanLepage approved these changes

@gebner gebner Awaiting requested review from gebner

+1 more reviewer

@AndersonTorres AndersonTorres AndersonTorres approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 12.approvals: 2 This PR was reviewed and approved by two persons.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@autra @luzpaz @AndersonTorres @GaetanLepage @LordGrimmauld @wegank