For reference #34779 is loosely related to the sys.platform =="win32" issue¹, and orthogonal to the change here.

I wanted the check to be abstracted into a method in case multiple things would be "equivalently-Windows" from a Spack point of view. No such case exists yet (although the thread you linked is an example of treating "cygwin" and "win32" the same), but that could be handled later. However, when it comes to that, we run into the problem again.

I don't think it's a major problem here but I'm generally concerned about mypy considerations negatively affecting the API. Is there some way to give mypy extra information about this?

Side note: my concern in #34779 (comment) does not apply to this PR.

There is apparently no way to get mypy to understand anything but the checks documented in PEP 484, which are intentionally "simple". The mypy developers don't seem particularly enthused about covering more cases, and someone would have to decide which ones to support, etc. See python/mypy#8547 (comment).

One issue in the current code is that is_windows is defined in lots of places, and it's not even used in some files, but that's not caught because it's assigned to a module-scope variable. So, when making this, I actually removed several unneeded instances of is_windows = sys.platform == "win32". They probably became unused with refactoring.

IMO this means we need to standardize on the sys.platform == "win32" idiom for now -- it's honestly pretty readable and sys.platform != "win32" might even be more readable than not is_windows (at least to me).

I'd say let's switch to using this, then think about whether more stuff can be consolidated in a checker-friendly way in #34779.

@tgamblin would mypy better handle this case if there was a singular definition for is_windows somewhere and each module that actually needed it just imported it? IK that would at least throw an unused import if refactoring removed the need for the is_windows statement.
If that would solve the problem, I have a branch implementing something along those lines that's been languishing as I focus on other efforts, but I could spin up on that and PR pretty quickly. Personally I find a lot of sys.platform == "win32" clutters up a stanza of code pretty easily/quickly and I've gotten feedback from other community members to that effect as well.

@johnwparent: unfortunately no, there's no way to make mypy smarter about this.

I also find that sys.platform == "win32" is ugly, but OTOH I think that does a good job of making you want to find a way to not have to write it :). I don't think is_windows or an is_windows() method is necessarily the best way to hide this (and does it really hide it?). I think it would be better if we instead came up with some abstractions to use that worked well in both places, and buried the if inside those.

So, really, I think we have to do it this way unless mypy changes -- I don't see where making the relatively small number of lines here slightly prettier is worth giving up the ability to mypy check the whole codebase.

@scheibelp @johnwparent any additional thoughts?

From what you're saying about mypy it doesn't really seem like there's another option, so not much of a surface to have thoughts on LOL.
Seems like our hands are tied, so LGTM.
I'll start mocking out ways to abstract logic like that in a way mypy can understand, do you have a good way to test what mypy can handle/docs? FWIW I think implementing the pathlib refactor in full will resolve a lot of these anyway.

I think it's really just the stuff in PEP 484 above, with no smart data flow analysis to track the value of the expression. I suspect it's bc mypy is mostly tracking types -- not values -- and they don't want to implement what you'd need to statically analyze the assignment to is_windows.