[question] Windows bash handling in conancenter recipes

[SpaceIm]

What is your question?

Currently in conan v2 recipes, we have this logic in build requirements when a bash is needed and build machine is Windows:

    @property
    def _settings_build(self):
        return getattr(self, "settings_build", self.settings)

    def build_requirements(self):
        if self._settings_build.os == "Windows":
            self.win_bash = True
            if not self.conf.get("tools.microsoft.bash:path", check_type=str):
                self.tool_requires("msys2/cci.latest")

I just realized that this logic doesn't properly handle consumers running conan from a bash terminal on Windows and setting tools.microsoft.bash:active=True (this conf didn't exist AFAIK when we have implemented this logic in conancenter recipes) and tools.microsoft.bash:subsystem. Indeed in this case tools.microsoft.bash:path is useless and not used at all in conan v2 client, so there is no reason for consumers to set this conf. But if tools.microsoft.bash:path is not set, current conancenter recipes will download msys2 recipe and inject it in build context (and override conf of users?) while it's useless and could even lead to conflicts and weird behavior.

So I guess we could replace this logic by:

    @property
    def _settings_build(self):
        return getattr(self, "settings_build", self.settings)

    def build_requirements(self):
        if self._settings_build.os == "Windows":
            self.win_bash = True
            if not self.conf.get("tools.microsoft.bash:subsystem", check_type=str):
                self.tool_requires("msys2/cci.latest")

@memsharded what do you think?

or maybe more explicit?

    @property
    def _settings_build(self):
        return getattr(self, "settings_build", self.settings)

    def build_requirements(self):
        if self._settings_build.os == "Windows":
            self.win_bash = True
            if not (self.conf.get("tools.microsoft.bash:subsystem", check_type=str) and \
                    (self.conf.get("tools.microsoft.bash:active", default=False, check_type=bool) or \
                     self.conf.get("tools.microsoft.bash:path", check_type=str))):
                self.tool_requires("msys2/cci.latest")

[SpaceIm]

@memsharded what is your opinion (I've remembered this issue while answering to https://github.com/conan-io/conan/issues/16963)?