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

Disable cross OS compilation #31919

Closed
wants to merge 2 commits into from
Closed

Conversation

sdmaclea
Copy link
Contributor

@sdmaclea sdmaclea commented Feb 7, 2020

Disable compilation and instsallation of miscellaneous executables during cross
compilation.

The add_executable_clr and install_clr add checks to see if we are doing cross compilation, and whether the component is desired for the type of cross compilation we are doing. Only if desisered call the underlying add_executable and install functions respectively.

Disable compilation and instsallation of miscellaneous executables during cross
compilation.
@sdmaclea sdmaclea self-assigned this Feb 7, 2020
Configure disable components to build except during cross OS build
@sdmaclea
Copy link
Contributor Author

sdmaclea commented Feb 7, 2020

@hoyosjs Yes. This is simply a build optimization. I was annoyed waiting for these to build. I thought no one will ever use these cross OS built things so lets disable them and save a few CPU cycles every build. I had been hoping they would magically fix my regression.... No such luck..... Disabling the dbi was a bigger win. It meant I had to fix fewer lines of code to get the compile to finish w/o errors.

@hoyosjs
Copy link
Member

hoyosjs commented Feb 7, 2020

There's an issue around

install(EXPORT dactabletools DESTINATION dactabletools)
Looks like using install_clr is making the export dactabletools never be defined and then we end up trying to do an install an empty never defined export. I have to accept Jeremy is more familiar with this.

@jkoritzinsky
Copy link
Member

Yeah the install_clr() function doesn't have quite all the same features as install(). We'd have to add EXPORT support to install_clr() (which would really just be forwarding it down to the call to install()).

@sdmaclea
Copy link
Contributor Author

sdmaclea commented Feb 8, 2020

Closing this for now. This a nice to have, but the infrastructure does not make this easy.

@sdmaclea sdmaclea closed this Feb 8, 2020
@sdmaclea
Copy link
Contributor Author

sdmaclea commented Feb 8, 2020

Looks like this line means I don't have to change the difficult bits

if (CLR_CMAKE_TARGET_WIN32 AND NOT CLR_CMAKE_CROSS_ARCH)

Before #31659 this was WIN32 which was the host so these sub directories used to be included. Now it is target so that resolved my issue with them.

@ghost ghost added the will_lock_this label Dec 6, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Jan 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants