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

fix: quote template CMake variables #17156

Merged
merged 3 commits into from
Oct 14, 2024
Merged

Conversation

skycaptain
Copy link
Contributor

@skycaptain skycaptain commented Oct 13, 2024

Changelog: Bugfix: Fix cppstd/cstd variable_watch when they are not defined.
Changelog: Bugfix: Fix cstd error reporting when a recipe does not support the required version.
Docs: Omit

This fix addresses an issue where "cppstd" is undefined for pure C recipes. Consequently, "set" sometimes leaves the
"conan_watched_std_variable" unset, leading to an error in the subsequent string comparison:

CMake Error at .../conan_toolchain.cmake:64 (if):
  if given arguments:

    "READ_ACCESS" "STREQUAL" "MODIFIED_ACCESS" "AND" "NOT" "11" "STREQUAL"

  Unknown arguments specified
  • Refer to the issue that supports this Pull Request.
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

@CLAassistant
Copy link

CLAassistant commented Oct 13, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@AbrilRBS AbrilRBS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch! I wish I had figured this would be a possibility when adding it!

Maybe adding a simple test for this would be great, let us know if you need help with that :)

This fix addresses an issue where "cppstd" is undefined for pure C
recipes. Consequently, "set" sometimes leaves the
"conan_watched_std_variable" unset, leading to an error in the
subsequent string comparison:

```
CMake Error at .../conan_toolchain.cmake:64 (if):
  if given arguments:

    "READ_ACCESS" "STREQUAL" "MODIFIED_ACCESS" "AND" "NOT" "11" "STREQUAL"

  Unknown arguments specified
```
@skycaptain
Copy link
Contributor Author

Great catch! I wish I had figured this would be a possibility when adding it!

Maybe adding a simple test for this would be great, let us know if you need help with that :)

Done.

@AbrilRBS
Copy link
Member

AbrilRBS commented Oct 14, 2024

Thanks @skycaptain - The test also uncovered a bug in the cstd error reporting, so that's 2 for 1!

Either way, I'm proposing #17157 to fix the cstd error while removing some slight duplication

@AbrilRBS AbrilRBS added this to the 2.9.0 milestone Oct 14, 2024
@AbrilRBS
Copy link
Member

The current test error is due to a missconfiguration for functional tests on our side:

The provided compiler.cstd=11 requires at least msvc>=192 but version 191 provided pinging @memsharded who implemented the feature in the first place and might better know where to bump the necessary values :)

Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks for these fixes @skycaptain !

I'll fix the test.

@memsharded memsharded self-assigned this Oct 14, 2024
@memsharded memsharded merged commit e85aaa1 into conan-io:develop2 Oct 14, 2024
2 checks passed
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.

4 participants