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

Start work on excluding unnecessary CMake modules #91

Merged
merged 18 commits into from
Jul 30, 2023

Conversation

bschwind
Copy link
Owner

@bschwind bschwind commented Jul 19, 2023

Goals of this PR:

  • Reduce the size of the occt-sys crate to below 10 MB, the crates.io upload limit
  • Reduce the amount of work CMake needs to do to build opencascade

I realized we can override opencascade files with the CMake BUILD_PATCH variable. I recreated the adm/MODULES directory in our patch directory and verified that CMake picks it up and uses it instead of the file in OCCT.

Next step is to modify adm/MODULES to cut out things we don't need, as well as editing occt-sys/Cargo.toml to exclude big directories that we don't use anyway.

Here's a reference to some of the CMake stuff in occt.

Depends on #112 being merged first.

crates/occt-sys/build.rs Outdated Show resolved Hide resolved
@strohel
Copy link
Collaborator

strohel commented Jul 19, 2023

I realize that this could also be a work-around for CI caching. Once we publish occt-sys to crates.io, we can depend on it through crates.io, including it in the cache (hopefully).

@bschwind
Copy link
Owner Author

Once we publish occt-sys to crates.io, we can depend on it through crates.io, including it in the cache (hopefully).

Ah yeah, if that works well then I'd much prefer that approach.

@bschwind
Copy link
Owner Author

Trying hard to cut out all unnecessary files to keep the crate size under the 10MB limit - I think we may need a different approach of splitting up toolkits into different crates. I've gotten fairly good at controlling which "toolkits" (as occt calls them) get built, but getting them all to fit in one crate is looking very difficult and time consuming to accomplish.

@strohel
Copy link
Collaborator

strohel commented Jul 20, 2023

Issues linked to rust-lang/crates.io#195 seem to be like a good library of workarounds people do to circumvent the limit.

@bschwind bschwind mentioned this pull request Jul 25, 2023
@bschwind
Copy link
Owner Author

cc @mkovaxx

@bschwind
Copy link
Owner Author

@strohel if we specify occt-sys as a git dependency, will our cargo cache action cache it properly? I think we wouldn't be able to publish on crates.io with a git dependency, but maybe it would help with the caching situation?

@bschwind bschwind force-pushed the cmake-size-reduction branch from 05afa1e to da50dd8 Compare July 29, 2023 17:01
@bschwind bschwind marked this pull request as ready for review July 29, 2023 17:08
@bschwind bschwind force-pushed the cmake-size-reduction branch 2 times, most recently from 727dcd7 to 8cf900d Compare July 30, 2023 09:34
@bschwind bschwind changed the base branch from main to occt-7-7-1-upgrade July 30, 2023 10:59
Base automatically changed from occt-7-7-1-upgrade to main July 30, 2023 11:04
@bschwind bschwind force-pushed the cmake-size-reduction branch from 9c9009a to 91417ad Compare July 30, 2023 11:05
@bschwind
Copy link
Owner Author

Merging this for now, there can be some cleanup afterwards but I want to get this in now to get CI build times down.

@bschwind bschwind merged commit 356a689 into main Jul 30, 2023
@bschwind bschwind deleted the cmake-size-reduction branch July 30, 2023 11:08
Copy link
Collaborator

@strohel strohel left a comment

Choose a reason for hiding this comment

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

Nice!

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.

2 participants