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

pkgconfig: Allow passing Dependency objects to library(_private) #2757

Merged
merged 7 commits into from Dec 31, 2017

Conversation

Projects
None yet
4 participants
@xclaesse
Contributor

xclaesse commented Dec 10, 2017

Special case ThreadDependency by taking compiler's flags and
PkgConfigDependency by adding them in requires(.private) instead. For
other Dependency objects just take their link_args.

Closes #2725

@xclaesse xclaesse changed the title from pkgconfig: Add support for Dependency library(_private) to pkgconfig: Allow passing Dependency objects to library(_private) Dec 12, 2017

@nirbheek

This comment has been minimized.

Show comment
Hide comment
@nirbheek

nirbheek Dec 12, 2017

Member

This requires a test case, preferably something that tests with as many dependency types as possible. f.ex., this is definitely broken with dependency types that don't yield -lfoo in link_args. For those, you'll have to split it into -Lbar -lfoo manually.

Member

nirbheek commented Dec 12, 2017

This requires a test case, preferably something that tests with as many dependency types as possible. f.ex., this is definitely broken with dependency types that don't yield -lfoo in link_args. For those, you'll have to split it into -Lbar -lfoo manually.

@xclaesse

This comment has been minimized.

Show comment
Hide comment
@xclaesse

xclaesse Dec 12, 2017

Contributor

dependency types that don't yield -lfoo

Any example?

Contributor

xclaesse commented Dec 12, 2017

dependency types that don't yield -lfoo

Any example?

@nirbheek

This comment has been minimized.

Show comment
Hide comment
@nirbheek

nirbheek Dec 12, 2017

Member

dependency types that don't yield -lfoo

Any example?

BoostDependency when not using pkg-config files will do this.

... actually, now that I think of it, a lot of dependency files use PkgConfigDependency internally. F.ex: GLDependency, BoostDependency, SDL2Dependency, etc. This is a bit problematic.

I wonder if we can set self in these cases and replace the actual object or something like that.

Member

nirbheek commented Dec 12, 2017

dependency types that don't yield -lfoo

Any example?

BoostDependency when not using pkg-config files will do this.

... actually, now that I think of it, a lot of dependency files use PkgConfigDependency internally. F.ex: GLDependency, BoostDependency, SDL2Dependency, etc. This is a bit problematic.

I wonder if we can set self in these cases and replace the actual object or something like that.

@xclaesse

This comment has been minimized.

Show comment
Hide comment
@xclaesse

xclaesse Dec 12, 2017

Contributor

I see, they should set self.pcdep so we can use hasattr() on it.

Contributor

xclaesse commented Dec 12, 2017

I see, they should set self.pcdep so we can use hasattr() on it.

@jpakkane

This comment has been minimized.

Show comment
Hide comment
@jpakkane

jpakkane Dec 16, 2017

Member

Please move the test to run_unittests.py. That is the correct place for any tests that do complex system checking. Project tests are designed for simple run & go tests. Furthermore we try to keep tests running pkg-config away from common tests so that they can be run everywhere without thinking (there may be some others, we should fix them).

Member

jpakkane commented Dec 16, 2017

Please move the test to run_unittests.py. That is the correct place for any tests that do complex system checking. Project tests are designed for simple run & go tests. Furthermore we try to keep tests running pkg-config away from common tests so that they can be run everywhere without thinking (there may be some others, we should fix them).

@nirbheek

This comment has been minimized.

Show comment
Hide comment
@nirbheek

nirbheek Dec 16, 2017

Member

Furthermore we try to keep tests running pkg-config away from common tests so that they can be run everywhere without thinking (there may be some others, we should fix them).

If you move this to unit tests, please put it in AllPlatformTests and skip the test if pkg-config is not found. We already do this for other tests, so you can copy that code from there.

We should ideally move all project tests that use pkg-config from linuxlike to common tests and skip them if pkg-config is not found because even though we shouldn't require pkg-config for tests on Windows and macOS, we still use pkg-config on those platforms if it's available. Also our CI has pkg-config on all platforms.

Member

nirbheek commented Dec 16, 2017

Furthermore we try to keep tests running pkg-config away from common tests so that they can be run everywhere without thinking (there may be some others, we should fix them).

If you move this to unit tests, please put it in AllPlatformTests and skip the test if pkg-config is not found. We already do this for other tests, so you can copy that code from there.

We should ideally move all project tests that use pkg-config from linuxlike to common tests and skip them if pkg-config is not found because even though we shouldn't require pkg-config for tests on Windows and macOS, we still use pkg-config on those platforms if it's available. Also our CI has pkg-config on all platforms.

Xavier Claessens added some commits Dec 10, 2017

@jpakkane jpakkane merged commit dd3f49a into mesonbuild:master Dec 31, 2017

3 checks passed

ci/sideci No issues left; 5 issues fixed.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@xclaesse xclaesse deleted the xclaesse:pkgconfig branch Apr 9, 2018

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