conan-io / conan

Conan - The open-source C and C++ package manager
https://conan.io
MIT License
7.95k stars 951 forks source link

[bug] Spurious leading spaces in generated conanbuildenv shell scripts #16479

Closed IanCollins closed 1 week ago

IanCollins commented 2 weeks ago

Describe the bug

On Linux, using conan 1.60.1

When conan generates the conanbuildenv shell script, it prepends a space for each exported buildenv variable, thus: export CLANG_VERSION=" 18"
For most, this isn't a problem but for numeric values, this result in literally interpreted as is giving an invalid value. Forr example clang-$CLANG_VERSION becomes clang- 18 which doesn't go down too well!

How to reproduce it

Import and look at the generated files.

memsharded commented 2 weeks ago

Hi @IanCollins

Thanks for your report.

It would be necessary a bit more of detail: the exact profile file that you are using, for example, and a basic command or conanfile to reproduce. For example, it would be necessary to use the new [buildenv] in the profile, not the legacy [env]. Could you please provide these details? Thanks!

memsharded commented 1 week ago

Hi @IanCollins

Please let us know if you can provide those details. Thanks!

IanCollins commented 1 week ago

Sure, sorry I missed the first message.

Yes, the profile does use [buildenv]. One example:

default.jinja: `[conf] tools.env.virtualenv:auto_use = True

[buildenv] AR = ar AS = as RANLIB = ranlib CC = clang CXX = clang++ STRIP = strip

CFLAGS = -w CXXFLAGS = -w

[settings] os = Linux arch = x86_64 compiler = clang compiler.version = 16 compiler.cppstd = 17 compiler.libcxx = libstdc++11 build_type = Release

[options] *:shared = False`

native_linux_clang_release_static.jinja: `include(default.jinja)

[settings] build_type=Release os.platform_name="None"

[options] *:shared=False`

After running a conan install, we have a generated conanbuildenv-release-x86_64.sh with the following exports: `export CXXFLAGS=" -w"

export CFLAGS=" -w"

export STRIP=" strip"

export CXX=" clang++"

export CC=" clang"

export RANLIB=" ranlib"

export AS=" as"

export AR=" ar"`

memsharded commented 1 week ago

Thanks for the details.

I have been checking this, the code internals, etc.

Check the PR https://github.com/conan-io/conan/pull/10343 And issue: https://github.com/conan-io/conan/issues/10334

There is even a comment in code:

# values are not strip() unless they are a path, to preserve potential whitespaces
name = name.strip()

It seems that this is mostly expected. The definition of environment variables values can accept any arbitrary value, including strings with leading spaces. We could argue that users could define quotes, but it happens that some users also want strings with literal quotes for values, and that further complicate things.

Please note that in the documentation spaces are not used around "=" in profiles: https://docs.conan.io/2/reference/config_files/profiles.html

So the leading space in env-vars values is not being automatically removed, please make sure to not include that leading space, unless you explicitly want it.

IanCollins commented 1 week ago

Okay, thank you for checking.

This is somewhat counterintuitive, but I'm sure I can fight my programmer's urge to line up my variables!

memsharded commented 1 week ago

This is somewhat counterintuitive, but I'm sure I can fight my programmer's urge to line up my variables!

Yep, I know, but sometimes it is complicated to cover all edge cases, and some ergonomics might be lost there.