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

Ensure our include directories get added in the correct order #2354

Merged
merged 2 commits into from
Mar 3, 2019
Merged

Ensure our include directories get added in the correct order #2354

merged 2 commits into from
Mar 3, 2019

Conversation

asmaloney
Copy link
Contributor

If you have assimp installed already and in the include path (e.g. I have it via homebrew), it can pick up the wrong headers.

This forces the include order so our local ones are found first when building assimp.

If you have assimp installed already and in the include path (e.g. I have it via homebrew), it can pick up the wrong headers.

This forces the include order so our local ones are found first when building assimp.
@kimkulling
Copy link
Member

Thanks a lot for the pull request. I din't know that keyword BEFORE. I will merge it!

@kimkulling kimkulling closed this Mar 2, 2019
@kimkulling kimkulling reopened this Mar 2, 2019
@asmaloney
Copy link
Contributor Author

Thanks Kim. I wasn't aware of BEFORE before either. I just found it when trying to solve my issue :-)

My use-case is that I'm including assimp as a git submodule and adding it in cmake using add_subdirectory().

One suggestion to make this cleaner would be to not add certain options (building tests and tools for example) if you are using it this way. i.e. check if we are "included in another project" or "building ourselves" and only show those options in the latter case.

If that sounds reasonable I can make the changes and another PR.

(I'm also a bit confused by the merge above: Merge branch 'master' into include_fix. What was merged where? :-) The PR wasn't merged into master.)

@kimkulling kimkulling merged commit e00d92e into assimp:master Mar 3, 2019
@kimkulling
Copy link
Member

Hi,

at first I merged the latest master into your branch. Afterwards I merged your branch back to master. So now all is on the master. Thanks a lot for the contribution.

Kim

@asmaloney
Copy link
Contributor Author

Ah - I didn't know you could push to my branch... I've never seen anyone do it that way.

Any thoughts on my other suggestion?

@asmaloney asmaloney deleted the include_fix branch March 3, 2019 19:48
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.

None yet

2 participants