[bug] Setting environment-variables via `[buildenv]` or `[runenv]` from profiles stores leading/trailing spaces in value

DenizThatMenace commented 3 months ago

Describe the bug

Environment details

OS:

Linux (Ubuntu 22.04 via WSL)

compiler:

Clang 18 (but irrelevant)

Conan version:

2.6

Conan profile:

[settings]
arch = x86_64
os = Linux
build_type = Debug
compiler = clang
compiler.version = 18
compiler.libcxx = libstdc++11
compiler.cppstd = 20
compiler.cstd = 17

[conf]
tools.build:compiler_executables = { 'c': 'clang-18', 'cpp': 'clang++-18', 'rc': 'llvm-rc-18' }
tools.cmake.cmaketoolchain:find_package_prefer_config = True
tools.cmake.cmaketoolchain:generator = Ninja Multi-Config

[buildenv]
AR = llvm-ar-18
LD= ld.lld-18
anyPathVar = (path)/usr/bin

[runenv]
someTool = MyTool
somePath = (path)C:\Windows

Description

Other than expected, the parsed environment-variables from the [buildenv] and [runenv] section of the profile are stored with a leading whitespace. This is obvious when looking at the output of the conan-run:

...

======== Input profiles ========
Profile host:
[settings]
arch=x86_64
build_type=Debug
compiler=clang
compiler.cppstd=20
compiler.cstd=17
compiler.libcxx=libstdc++11
compiler.version=18
os=Linux
[conf]
tools.build:compiler_executables={'c': 'clang-18', 'cpp': 'clang++-18', 'rc': 'llvm-rc-18'}
tools.cmake.cmaketoolchain:find_package_prefer_config=True
tools.cmake.cmaketoolchain:generator=Ninja Multi-Config
[buildenv]
AR= llvm-ar-18
LD= ld.lld-18
anyPathVar=(path)/usr/bin
[runenv]
someTool= MyTool
somePath=(path)C:\Windows

Profile build:
[settings]
arch=x86_64
build_type=Debug
compiler=clang
compiler.cppstd=20
compiler.cstd=17
compiler.libcxx=libstdc++11
compiler.version=18
os=Linux
[conf]
tools.build:compiler_executables={'c': 'clang-18', 'cpp': 'clang++-18', 'rc': 'llvm-rc-18'}
tools.cmake.cmaketoolchain:find_package_prefer_config=True
tools.cmake.cmaketoolchain:generator=Ninja Multi-Config
[buildenv]
AR= llvm-ar-18
LD= ld.lld-18
anyPathVar=(path)/usr/bin
[runenv]
someTool= MyTool
somePath=(path)C:\Windows
...

Interestingly, only for non-path variables the whitespaces are not split.

I understand, why it might be beneficial to also be able to store leading (and trailing) whitespaces in environment variables. But shouldn't there be some other syntax to do so? E.g. consider:

Surrounding the entire value by double-quotes (as it is done in Bash) or similar?
Or require the author of that profile to provide a (verbatim) prefix to the value (similar to the (path) prefix), so that everything following it will be stored as value.

Anyway, this disparity from how variables and values of other sections are parsed makes it surprising and error-prone. At least, this should be documented somewhere.

How to reproduce it

Simply just try to build any conan package like so:

conan create <dir-with-conanfile.py> -pr:a <conan-profile-from-above>

DenizThatMenace commented 3 months ago

This is where stripping whitespaces should be done: https://github.com/conan-io/conan/blob/f357f8169c1e4ecf2bee5ca0b5fb723fc36d82b0/conan/tools/env/environment.py#L632-L636

memsharded commented 3 months ago

Hi @DenizThatMenace

Thanks for your report. I think this was already discussed in https://github.com/conan-io/conan/issues/16479, so this would be a duplicate.

The comment in https://github.com/conan-io/conan/issues/16479#issuecomment-2180396671 explain the behavior and rationale.

DenizThatMenace commented 3 months ago

Hi @memsharded

Yes, that seems to be the exact same behavior and your linked answer explains the rationale, which is what I also guessed above in my description.

However, even if the mentioned example is not putting any spaces around the = sign, it is still "implicit" knowledge that leading spaces on the right side of that = sign are preserved. This is especially confusing as this holds only true for sections [buildenv] and [runenv].

It is "implicit" knowledge because it is nowhere explicitly mentioned in the documentation. (I hope you do not suggest that the average conan-user, although (s)he might be a developer, should search conan's python scripts in order to find that comment which states that stripping is explicitly left out.)

I would recommend to document that behavior in Conan's user documentation. It should probably be explicitly mentioned as a highlighted "note" in the documentation of [buildenv] and [runenv] here.

memsharded commented 3 months ago

I would recommend to document that behavior in Conan's user documentation. It should probably be explicitly mentioned as a highlighted "note" in the documentation of [buildenv] and [runenv] here.

Agree, moved this to docs repo, proposed https://github.com/conan-io/docs/pull/3813

conan-io / docs