conan-io / conan

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

[question] Conan 2 Migration - Windows Paths #17163

Closed radonish closed 1 month ago

radonish commented 1 month ago

What is your question?

In working on migrating my company to Conan 2 I came across an interesting Windows PATH behavior. I'm trying to determine if it's a Conan bug or something with the recipe implementation. The issue is that the PATH additions that come from tool_requires() within the build_requirements() function are not delimited by semicolons. The result is something like this:

PATH1;PATH2;PATH3 is updated to PATH1;PATH2 CONAN_TOOL_PATH1 CONAN_TOOL_PATH2 PATH3

I expected PATH1;PATH2;CONAN_TOOL_PATH1;CONAN_TOOL_PATH2;PATH3

Specifically, I'm testing with openssl and the issue is it can't find nasm when attempting to execute it as a part of its build process.

I've updated the OpenSSL configuration script that claims it cannot find nasm to print the path. I see the behavior I shared above - the nasm package is indeed in the path, but missing semicolons. I'm assuming that is why it cannot find nasm there as I can do a simple command line test where I update my path and execute nasm successfully.

Recipe issue? Or Conan issue? Any help is appreciated - thank you!

Have you read the CONTRIBUTING guide?

memsharded commented 1 month ago

Hi @radonish

It is possible that you might be depending on legacy generators.

If using:

The concat that you are reporting on your comment seems to be the effect of adding variables as normal variables, not as a path variables, can you please check that?

radonish commented 1 month ago

Hello, @memsharded - thanks for your response.

The producer is indeed using self.buildenv_info.define_path() in package_info(). The consumer is using VirtualBuildEnv within its generate() method as well. So, all looks OK from that perspective. Producer's package_info():

def package_info(self):
    self.cpp_info.libdirs = []
    self.cpp_info.includedirs = []

    compiler_executables = {"asm": self._nasm}
    self.conf_info.update("tools.build:compiler_executables", compiler_executables)
    self.buildenv_info.define_path("NASM", self._nasm)
    self.buildenv_info.define_path("NDISASM", self._ndisasm)
    self.buildenv_info.define_path("AS", self._nasm)

After the consumer calls VirtualBuildEnv(self).generate() I can see that the generated conanbuildenv-release-x86.bat file has the PATH with the wrong delimiter:

set "PATH=C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.41.34120\bin\Hostx86\x86 c:\scratchnobackup\conantest\.conan2\p\b\straw2117d9311809e\p\bin c:\scratchnobackup\conantest\.conan2\p\b\nasma836e9967f321\p\bin %PATH%"
radonish commented 1 month ago

@memsharded, I think I found the culprit. Even though the producer and consumer appeared to be doing things as you noted was preferred, our Windows profile had this in it:

[buildenv]
PATH=+C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.41.34120\bin\Hostx86\x86

If I comment out the PATH update in the profile, VirtualBuildEnv generates the path with a semicolon delimiter instead of a space. Does that make sense to you? It seems like one should be able to update the path in the profile.

memsharded commented 1 month ago

yes, that makes sense.

The correct syntax is:

PATH=+(path)C:\Program Files (x86)\Microsoft Visual Studio...

Check https://docs.conan.io/2/reference/config_files/profiles.html#buildenv for the full reference

radonish commented 1 month ago

Thanks, @memsharded - glossed over your profile mention initially as I had forgotten about that update in the midst of trying different things to debug. Thanks for your help!

memsharded commented 1 month ago

Happy to help! :)