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 for #16882: Generated CMake build when using tools.build.linker_scripts #16883

Merged
merged 7 commits into from
Aug 28, 2024

Conversation

photex
Copy link
Contributor

@photex photex commented Aug 27, 2024

The generated build otherwise appends the "-Tfoo.ld" argument directly without a preceding space before which can generate an invalid command line.

Changelog: Bugfix: Avoid CMakeToolchain error when both architecture flags and tools.build:linker_scripts are defined, due to missing space.
Docs: Omit

Close #16882

  • 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.

photex added 2 commits August 27, 2024 13:59
The generated build otherwise appends the "-Tfoo.ld" argument directly
without a preceding space before which can generate an invalid command
line.
```
CMake Warning (dev) at /home/chipc/.conan2/p/b/mypkgbc25b469ad9ce/b/build/generators/conan_toolchain.cmake:54:
  Syntax Warning in cmake code at column 43

  Argument not separated from preceding token by whitespace.
```
@photex photex force-pushed the dev/chip/cmake_linker_scripts branch from 2c531e1 to 09d3616 Compare August 27, 2024 13:34
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.

Thanks very much for your contribution @photex

You are right, there is a bug there, and the extra space is necessary.
It would be necessary to add some unit or integration test for this, to make sure this is covered and don't break again in the future. I can also try to help with this.

conan/tools/cmake/toolchain/blocks.py Outdated Show resolved Hide resolved
@memsharded
Copy link
Member

I have pushed some changes, fixing the test (that test will cover the change already), and trying to keep the original quotes. Can you please test this branch with your build to see if it works? Thanks!

@photex
Copy link
Contributor Author

photex commented Aug 28, 2024

@memsharded you're update fixed the cmake warnings I was seeing as well. :D

@memsharded memsharded added this to the 2.7.0 milestone Aug 28, 2024
@memsharded memsharded merged commit eba4745 into conan-io:develop2 Aug 28, 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.

[bug] Specifying tools.build.linker_scripts in my profile results in an invalid command line when linking.
3 participants