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

[bug] unix_path() uses WSL path flavor on Windows #8883

Closed
srnwk opened this issue Apr 27, 2021 · 14 comments · Fixed by #9194
Closed

[bug] unix_path() uses WSL path flavor on Windows #8883

srnwk opened this issue Apr 27, 2021 · 14 comments · Fixed by #9194

Comments

@srnwk
Copy link

srnwk commented Apr 27, 2021

When conans.client.tools.win.unix_path() is called and the path_flavor is left to be determined automatically,
it will call OSInfo.detect_windows_subsystem().
When this happens on a Windows 10 system with WSL installed, this will find C:\Windows\System32\bash.exe,
run uname -a inside WSL and identify the subsystem as WSL, and choose the WSL path flavor.
Example of the generated conanbuildinfo.txt:

[ENV_xapian-core]
AUTOMAKE_CONAN_INCLUDES=["/mnt/d/conan_data/xapian-core/1.4.16/_/_/package/d140711d95cc16a85766a8fc3a551dfafe84cf63/bin/share/aclocal"]
PATH=["D:\conan_data\xapian-core\1.4.16\_\_\package\d140711d95cc16a85766a8fc3a551dfafe84cf63\bin"]

This also makes conan install run slower than it needs to when it has to wait for WSL startup before misidentifying the subsystem.

Environment Details (include every applicable attribute)

  • Operating System+version: Windows 10 19042.928
  • Compiler+version: Visual Studio 2019 16
  • Conan version: 1.35.2 - 1.36 develop
  • Python version: 3.9.1 x64

Steps to reproduce (Include if Applicable)

conan install xapian-core/1.4.16

@memsharded
Copy link
Member

Hi @srnwk

I am afraid I don't understand the issue completely, I would need a bit more of context:

  • The command to reproduce: conan install xapian-core/1.4.16@ (it fails without @), doesn't produce any local conanbuildinfo.txt.
  • How the unix_path() is being called.
  • What is the expected behavior, to have a native windows build?

@srnwk
Copy link
Author

srnwk commented Apr 27, 2021

Sorry, choose the cmake generator for example, conan install xapian-core/1.4.16@ -g cmake -if .

unix_path() is being called by the xapian-core recipe in this case: https://github.com/conan-io/conan-center-index/blob/master/recipes/xapian-core/all/conanfile.py#L147

As for the expected behavior - I wouldn't expect unix_path() to generate a WSL style /mnt/d/... path that only works inside WSL when running outside of WSL.

@memsharded
Copy link
Member

Ok, thanks, now it is clear. I have been able to reproduce it.

This is very unfortunate, the detection is being done with bash -c uname under the hood, which in Windows, happens to return "Linux" both when you are running inside the WSL environment, but also when you are running in your Windows native cmd prompt 😓

I have no idea how this could be fixed, seems challenging without breaking, lets investigate a bit further:

  • The detect_windows_subsystem() is returning WSL even when not running inside the WSL, but in native Windows. This could be considered a bug, and needs to be fixed.

@sourcedelica
Copy link

If you are running in WSL you will have a number of environment variables set like WSL_DISTRO_NAME.

@srnwk
Copy link
Author

srnwk commented Apr 29, 2021

detect_windows_subsystem() is also used by run_in_windows_bash(), where detecting WSL from "outside" appears to be the desired outcome.

@memsharded
Copy link
Member

detect_windows_subsystem() is also used by run_in_windows_bash(), where detecting WSL from "outside" appears to be the desired outcome.

I see, good point. I think the run_in_windows_bash() is probably incorrectly relying on the subsystem = subsystem or OSInfo.detect_windows_subsystem() to decide if it has a working bash to be used, and probably it would be necessary to fix it too.

My biggest concern is that this is too messy and fragile at this moment, so it is very likely that changing anything there will be breaking to many people, even if we are apparently fixing some bugs.

@SSE4
Copy link
Contributor

SSE4 commented May 6, 2021

unfortunately, I don't see a safe and easy way to fix it without breakage. probably, declaring explicit os.subsystem in profile is the way to avoid problem (or, specifying CONAN_BASH_PATH to the wanted bash).

@memsharded memsharded modified the milestones: 1.37, 1.38 May 30, 2021
@lieser
Copy link

lieser commented May 31, 2021

Stumbled on the same problem then I tried to build the autoconf package with conan create (test package fails)

Interesting is that the a OSInfo.detect_windows_subsystem() inside the package_info() function of the autoconf package returns wsl (i.e. the env_info is populated with WSL style paths), but a call to OSInfo.detect_windows_subsystem() inside the build() function of the TestPackageConan package returns msys2 (i.e. the commands run inside the test package with self.run(..., win_bash=tools.os_info.is_windows)) are run inside msys2).

Note that I also found #8161, which seems to be about the same issue.

@memsharded
Copy link
Member

Thanks for the feedback @lieser

Yes, it might be an issue of path priorities, because test_package/conanfile.py might have some less environment definitions for example, or might inject msys2 dependency to Conan package or something.

The whole management of the run_in_windows_bash is a bit of a mess right now, even if we wanted to improve it for 1.37, it has not been possible, but it will be a priority for 1.38. It will evolve in the sense of less auto-detection, and more explicit configuration via profiles.

@robindegen
Copy link

robindegen commented Jun 25, 2021

I appear to be also hit by this. I am unable to build autoconf, as the added /mnt/ fails the package test. I disabled the test, but then automake also fails because it sets the environment variable wrong. I'd be happy with any kind of workaround at this point

@robindegen
Copy link

I'm not sure if the change fixed it; since I still have the same issue in 1.50.0

@memsharded
Copy link
Member

Hi @robindegen

This issue was closed by the introduction of a new mechanism, defining win_bash=True as a class or object attribute, not passing it in the method argument. If you have a case that we can use to reproduce, please file a new issue with details, so we can have a look. Thanks!

@robindegen
Copy link

robindegen commented Jul 21, 2022

I'm still having issues like this when trying to build autoconf and automake to start using swig. I'm still investigating deeper where exactly it is going wrong. From what I currently understand it seems to already go wrong in the autoconf package. That one builds just fine with the proper msys paths, but then it sets all the environment vars (using tools.unix_path) like "AUTOCONF" to /mnt/. And then when building automake it ofcourse uses those environment variables to find the autoconf executable. I tried modifying the conanfiles to use win_bash=True and removing the parameter in the affected packages.

@memsharded
Copy link
Member

If you are talking about the autoconf and automake packages in ConanCenter, it is true that these packages haven't been upgraded yet to the new approach that fixes it and using the new build system integrations. This work has already started in ConanCenter, but it will still take some time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants