[feature] Don't trick users in `conan.tools.microsoft.unix_path` with internal details like win_bash or subsystem

[SpaceIm]

• [x] I've read the CONTRIBUTING guide.

Please, do not try to be too smart and guess what users want: https://github.com/conan-io/conan/blob/0fe6058771c7d9585f9fed1935a4e0ec078d3314/conan/tools/microsoft/subsystems.py#L5-L6

When someone uses unix_path, he wants a unix_path regardless of the value of self.win_bash.

Because maybe he may want to enable win_bash in very specific parts of a conanfile (for example only in build() & package() when we may really rely on a shell, but not in generate() where we prepare files to be injected and don't call any bash script), and for such function as unix_path, it's really unexpected to not get... a unix path.

Maybe also we want to have a unix_path in package_info(), and win_bash doesn't really make sense, it can take any value, it doesn't matter.

I expect many mistakes with current logic in conan-center recipes.

According to the current logic of unix_path, it should be called native_system_path or something like this, it's misleading.

[memsharded]

I have added some changes in my PR in https://github.com/conan-io/conan/pull/12178/commits/f3d917913064a44e770d03d7e9c5b1b54ba3b1b8:

• Remove the first if conanfile.win_bash
• Then it will rely on the new logic for defining when a path/environment file/command should use or not a subsystem syntax:
  • Definition of conf tools.microsoft.bash:subsystem
  • Definition of conf tools.microsoft.bash:active to indicate you are already running inside a subsystem env
  • Definition of conanfile.win_bash or conanfile.win_bash to indicate that self.run() commands will run inside a subsystem
  • In that PR, the os.subsystem is no longer used to define the bash subsystem, only the binary compatibility

[SpaceIm]

For me this function should be totally stateless, it's called unix_path, why would it not return a unix path in some hidden reasons?

If conan internally needs this logic, it would be better to create an internal function returning a unix path conditionally. But for users, it's totally misleading, and in conan-center we need a function returning a unix path unconditonally.

[memsharded]

For me this function should be totally stateless, it's called unix_path, why would it not return a unix path in some hidden reasons?

Because it needs to know the subsystem in which it is running: msys, cygwin, etc. If it doesn't know the subsystem and it is in Windows, the best it can do is return the windows path. What is "unix_path"? Maybe we can just rename it to windows_subsystem_path, but its intention is (and it always was) to return the path for a given subsystem, but the subsystem should be known.

[SpaceIm]

I see.

• The name of this function is misleading, I was expecting the same behavior than conans.tools.unix_path which works fine for all use cases occuring in conan-center-index.
• I think it should be possible to force in unix_path function a specific "subsystem path" regardless of current value of tools.microsoft.bash:subsystem or win_bash. Indeed, current implementation doesn't take into account several use cases of this function: call unix_path in package_info() of a recipeA to define a specific variable to be used downstream in a specific subsystem, but recipeA by itself doesn't really uses a specific subsystem.

[SpaceIm]

I don't understand why https://github.com/conan-io/conan/pull/12178 fixes this issues. Does it allow to define a buildenv var in package_info() of recipe A with a unix_path(), so that if recipe B has win_bash = True AND recipeA in tool_requires, this env var is propagated in VirtualBuildEnv with this unix path?

Basically this issue is related to problems we have with migration of recipes like autoconf, automake, pkgconf etc. These recipes MUST define env vars with unix path style in package_info() when they are consumed in a build of another recipe. For example the own self.win_bash of autoconf recipe doesn't matter for package_info of autoconf, it's the self.win_bash of consuming recipe which matters. With conan v1 tools, we were always using conans.tools.unix_path in package_info() of autoconf, but there is no way to achieve the same behavior with conan.tools.microsoft.unix_path.

[memsharded]

I don't understand why https://github.com/conan-io/conan/pull/12178 fixes this issues. Does it allow to define a buildenv var in package_info() of recipe A with a unix_path(), so that if recipe B has win_bash = True AND recipeA in tool_requires, this env var is propagated in VirtualBuildEnv with this unix path?

This is not a unix_path() problem, it is a general issue with paths from packages to consumers, because when the package_info() of the package A executes, it is completely unaware and agnostic of what the consumer package B has. It doesn't matter if recipe B has win_bash=True, that cannot be used in any form in A, it is impossible (plus it might have more than one consumer, some with win_bash=True and other without it, so it is not that is not logistically possible, it shouldn't be conceptually done, as it will be broken).

The approach should be that the consumer B is the one using unix_path() if necessary to transform the path.

Actually, there is also another related problem using self.package_folder in package_info(), it is completely broken for editable packages, and the recommendation is to avoid it as much as possible, defining things relative to the "build-folder" or "source-folder" instead. I am opening a new ticket to investigate this

[memsharded]

The modern VirtualBuildEnv and VirtualRunEnv already have an internal unix_path(). It shouldn't be necessary to define the unix_path in the dependency package_info() at all, just defining it as a normal path self.buildenv_info.define_path() should work.

[SpaceIm]

The modern VirtualBuildEnv and VirtualRunEnv already have an internal unix_path(). It shouldn't be necessary to define the unix_path in the dependency package_info() at all, just defining it as a normal path self.buildenv_info.define_path() should work.

How do they know they should call unix_path? If win_bash is True when they are called? If we want to set win_bash conditionnally in a recipe, in which method we have to ensure win_bash True so that these generators work as intended?

[memsharded]

Because they define win_bash, yes. It is the consumer that runs in the windows bash, and it is the one that should convert paths to the right format. win_bash only affects in Windows, so as long as the package builds in Windows always in the subsystem bash, then no need to make it conditionally. I think this is the majority of cases, I have not seen many other cases in which the build happens conditionally in bash or not, but if you have examples, it would be good to know them.

[SpaceIm]

The modern VirtualBuildEnv and VirtualRunEnv already have an internal unix_path(). It shouldn't be necessary to define the unix_path in the dependency package_info() at all, just defining it as a normal path self.buildenv_info.define_path() should work.

In practice it doesn't seem to work properly in conan 1.53.0 (the fact that buildenv vars defined as paths & coming from tool_requires are wrapped by unix_path in VirtualBuildEnv if win_bash is True in consumer recipe).

Take a look at https://github.com/conan-io/conan-center-index/pull/14110, it's libsmacker recipe.

This recipe builds with autotools (always) and it calls autoreconf at build time, so libtool (and autoconf/automake/m4) is in tool_requires, as well as msys2 if build machine is Windows (and win_bash is enabled). autoconf recipe defines several buildenv vars as paths but without explicit unix_path conversion in package_info() since https://github.com/conan-io/conan-center-index/pull/12896. So this change broke libsmacker build on Windows because these env vars were not converted to unix_path: https://github.com/conan-io/conan-center-index/pull/14110#issuecomment-1308469125

Then we have "fixed" autoconf recipe to convert again paths of these env vars in package_info() of autoconf recipe, with legacy unix_path tool: https://github.com/conan-io/conan-center-index/pull/14118, and therefore libsmacker build worked again.

Because they define win_bash, yes. It is the consumer that runs in the windows bash, and it is the one that should convert paths to the right format. win_bash only affects in Windows, so as long as the package builds in Windows always in the subsystem bash, then no need to make it conditionally. I think this is the majority of cases, I have not seen many other cases in which the build happens conditionally in bash or not, but if you have examples, it would be good to know them.

Sure: argon2, argtable2, calceph, freexl, gdal (before 3.5.0), lcms, libcurl, libdb, libdeflate, libelf, libfdk_aac, libid3tag, libjpeg, libmad, libmicrohttpd, libmp3lame, libpq, librasterliste, librasterlite2, libsodium, libspatialite, libstudxml, libusb, libxml2, libxslt, lzma_sdk, mpdecimal, mpg123, mpir, nasm, openssl, opusfile, pthreads4w, readosm, sqlcipher, tcl, tk, xz_utils, yasm, zimg

[memsharded]

Ok, thanks. Yes, that would be something like:

        if self._is_msvc:
            self.win_bash = False
            self._build_msvc()
        else:
            self.win_bash = True
            if self._is_mingw:

I think it can be done in the configure() method.

[SpaceIm]

Ok, currently in conan-center recipes, win_bash is conditionally enabled in build_requirements() where we also conditionally add msys2 in tool_requires. It makes sense for me because it's enabled only when we build, and the logic is not scattered in different methods.

Example: https://github.com/conan-io/conan-center-index/blob/d492cbd99ea402291451452b77588beef3c7c3f7/recipes/libjpeg/all/conanfile.py#L58-L62