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

backends: Add custom target inc dirs before target inc dirs #2291

Merged
merged 2 commits into from Sep 11, 2017
Merged

backends: Add custom target inc dirs before target inc dirs #2291

merged 2 commits into from Sep 11, 2017

Conversation

nirbheek
Copy link
Member

@nirbheek nirbheek commented Sep 7, 2017

Custom target include dirs must be overridable by target-specific include dirs otherwise in case of header name collisions, the user has no way to override this behaviour. This is happening right now for gtk+ and graphene when graphene is used via a subproject.

@nirbheek nirbheek added this to the 0.42.1 milestone Sep 7, 2017
@jpakkane
Copy link
Member

jpakkane commented Sep 7, 2017

This needs a test.

@nirbheek
Copy link
Member Author

nirbheek commented Sep 7, 2017

Added.

@codecov-io
Copy link

codecov-io commented Sep 7, 2017

Codecov Report

❗ No coverage uploaded for pull request base (master@cb8b5a1). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2291   +/-   ##
=========================================
  Coverage          ?   52.71%           
=========================================
  Files             ?       74           
  Lines             ?    18449           
  Branches          ?     3855           
=========================================
  Hits              ?     9725           
  Misses            ?     7690           
  Partials          ?     1034
Impacted Files Coverage Δ
mesonbuild/backend/vs2010backend.py 89.62% <100%> (ø)
mesonbuild/backend/ninjabackend.py 70.94% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb8b5a1...b2c7b8c. Read the comment docs.

@nirbheek
Copy link
Member Author

nirbheek commented Sep 7, 2017

Linux builds are failing because pip3 doesn't exist anymore and has been replaced by versioned names: 3.3, 3.4, 3.5, 3.6, etc.

@nirbheek
Copy link
Member Author

nirbheek commented Sep 7, 2017

Linux and macOS were passing a little while ago, so the relevant change is that appveyor MSVC passes again. I think this can be merged.

@nirbheek
Copy link
Member Author

nirbheek commented Sep 7, 2017

Oh, nvm, that was when there was no test and I was thinking about #2288 for appveyor failures.

Custom target include dirs must be overridable by target-specific
include dirs otherwise in case of header name collisions, the user has
no way to override this behaviour.
@jpakkane
Copy link
Member

I'm surprised that custom target dirs are added automatically in the search path. Git blame says that the original commit message does not state this change in behaviour, only that the order changes.

Still, since we have this, it really should work properly.

@jpakkane jpakkane merged commit 77bf63d into mesonbuild:master Sep 11, 2017
@nirbheek nirbheek deleted the fix-custom-target-includes-ordering branch September 11, 2017 16:49
@nirbheek
Copy link
Member Author

git blame can be a bit wonky at times. I manually searched for the commit that added it, and it seems it was f9060a7#diff-2f566b4da6de635b323a7c47114652d8R1200 in 2015. There's still no reason as to why it was added, but we should indeed keep it around.

hubot pushed a commit to GNOME/gtk that referenced this pull request Sep 11, 2017
With these changes gtk+ builds for me using fallbacks for all libraries
with fallbacks available. Needs the following changes:

ebassi/graphene#109 (graphene)
https://bugzilla.gnome.org/show_bug.cgi?id=787414 (pango)
mesonbuild/meson#2291 (will be in meson 0.42.1)

https://bugzilla.gnome.org/show_bug.cgi?id=787416
nirbheek added a commit that referenced this pull request Sep 11, 2017
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

3 participants