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: allow cross compilation under builtin flag #163

Open
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

DSchroer
Copy link
Contributor

@DSchroer DSchroer commented Jan 16, 2024

Fix for #160

occt-sys no longer builds OCCT in the build.rs file. This allows it to be re-included in the workspace and the build still only triggered when the builtin feature is enabled.

This also now passes the correct ENV variables to the build so it is built for TARGET and not HOST.

Finally added CI to test these builds work on various platforms.

Had to rename Handle_* to Handle* for MSVC.

@@ -148,7 +148,7 @@ exclude = [
# "OCCT/src/BRepCheck/*",
# "OCCT/src/MAT2d/*",
"OCCT/src/ExpToCasExe/*",
# "OCCT/src/STEPControl/*",
"OCCT/src/STEPControl/*",
Copy link
Owner

Choose a reason for hiding this comment

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

This might have implications on whether or not I can publish to crates.io, we're currently right at the 10MB crate limit. We'll see how it goes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had issues where the STEPControl.hxx headers were missing on windows when it was not included.

@bschwind
Copy link
Owner

bschwind commented Sep 6, 2024

@DSchroer I'd be interested in revisiting this - since this PR was opened I have requested a larger crate size for occt-sys which means I don't need to do as many exclude shenanigans in Cargo.toml, as well as less patching of the occt src directory.

@fidoriel
Copy link
Contributor

I am interested in a working cross compile chain. Any chance to finish this project @DSchroer? If not, I am going to try it ;)

@bschwind
Copy link
Owner

@fidoriel please do! I'll do my best to help review/test if any PRs are posted.

@fidoriel
Copy link
Contributor

To sum up:
the crates upload limit is overcome. Nevertheless, the windows thing deserves its own PR, being out of scope for this one. Hence I am not on windows, so cannot (and will not) test. And therefor extract the cross compile part.

So, a proper cleanup and squash is needed? I use currently a fork with this patch applied ;)

@bschwind
Copy link
Owner

@fidoriel yep that sounds about right!

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.

3 participants