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

Tests: add (broken) override_options test case #8553

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MathieuDuponchelle
Copy link
Contributor

This reproduces a case I encountered in a project, where a pkg-config
dependency (aws-cpp-sdk-s3) has -std=c++11 in its cflags, but the
project uses c++17 features and passes -std=c++17 through
override_options.

The final compile command has c++17 before c++11, causing compilation
to fail.

This reproduces a case I encountered in a project, where a pkg-config
dependency (aws-cpp-sdk-s3) has -std=c++11 in its cflags, but the
project uses c++17 features and passes -std=c++17 through
override_options.

The final compile command has c++17 before c++11, causing compilation
to fail.
@jpakkane
Copy link
Member

I would call this a bug in the pc file. It should not define the version to be used. The C++ standard libraries guarantee (as far as I can tell) that old and new code can be compiled and linked together and the result will work. If the code can not be used with an older version of C++ then its headers should have an #ifdef check for the version and error out if it is too old.

Still, that being said, we should probably detect this and either remove all -std flags coming from pc files or, at the very least, print an error.

@MathieuDuponchelle
Copy link
Contributor Author

MathieuDuponchelle commented Mar 22, 2021

Still, that being said, we should probably detect this and either remove all -std flags coming from pc files or, at the very least, print an error.

Sounds fair to me

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