Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

depends: use -isystem instead of -I to add system includes #3470

Merged
merged 1 commit into from Oct 20, 2013

Conversation

Projects
None yet
3 participants
Member

elupus commented Oct 20, 2013

This avoids libs using already installed system headers when
building themselves.

@elupus elupus depends: use -isystem instead of -I to add system includes
This avoids libs using already installed system headers when
building themselves.
077a933
Contributor

davilla commented Oct 20, 2013

this needs heavy testing on all platforms that use depends as lib builders will typically abuse <> vs "" in includes

Member

elupus commented Oct 20, 2013

Agree it needs testing. However it does make sense. Our prefix dir is system includes.

The reason i needed it was for afpfs. There was a change in it that changed a function's prototype. A rebuild of depends then fail, since it's using the system include of afp.h instead of the one in the lib it's currently building.

jenkins build this please

Contributor

davilla commented Oct 20, 2013

Ahhh, good catch

@elupus elupus added a commit that referenced this pull request Oct 20, 2013

@elupus elupus Merge pull request #3470 from elupus/depends
depends: use -isystem instead of -I to add system includes
98bc252

@elupus elupus merged commit 98bc252 into xbmc:master Oct 20, 2013

1 check passed

default Merged build #390 succeeded in 1 hr 40 min
Details
Member

theuni commented Oct 23, 2013

This is missing Toolchain.cmake.in.

Note that I didn't use -isystem because some configures will (stupidly) try to filter out -Ifoo and -Lbar. IIRC ffmpeg was the worst offender of that kind of thing.

@davilla davilla added a commit to davilla/xbmc that referenced this pull request Oct 23, 2013

@davilla davilla fixed, include missing change to -isystem from #3470 86c68e7
Member

elupus commented Oct 24, 2013

Did it cause a problem? It does seem fine on the build bots and my system.

@davilla davilla added a commit that referenced this pull request Oct 24, 2013

@davilla davilla Merge pull request #3483 from davilla/fix-pr3470
fixed, include missing change to -isystem from #3470
692bb83
Contributor

davilla commented Oct 24, 2013

@elupus elupus deleted the elupus:depends branch Oct 24, 2013

Member

theuni commented Oct 25, 2013

Nah, just thought I'd mention in case it bit somewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment