Make compiler.include_dirs expose all paths correct again for MSVC

[kxrob]

Fixup etc. for changes in 9f9a3e5 / c84d3e5 which crashed the pywin32 CI builds recently - setuptools is currently frozen there <=63.2.0 until clarified.
The problem happened at
https://github.com/mhammond/pywin32/blob/fc69a1ecc150a13433a7a71d61bac52a5f3df42b/setup.py#L695
/ mhammond/pywin32@fc69a1e) ( @zooba )

Besides the unnecessary compatibility break the MSVC paths should probably not be hidden generally I think.
(Otherwise pywin32 would need to dig into distutils internals compiler.__class__.include_dirs in a strange manner, a class variable which is only available after instance.initialize() ...)

[jaraco]

Sorry, but I can't accept this change. It essentially undoes the changes that I intentionally made in #153, including breaking the regression test that captures the expectation we're trying to retain.

The reason that test was added was because if set_include_dirs is invoked after initialization, it erases the values previously added. The compiler still needs to be able to access the system paths in that case.

Please file a bug report describing what the issue is and what you expected to happen differently and we can discuss ways to address it.

[kxrob]

It essentially undoes the changes that I intentionally made in #153

Not really: It accomplishes the (original) goal of that PR fully ( --include-dirs and --library-dirs options) in a different way by changing the class var method into "adding dirs in the right order" and should solve all actual problems. Without the serious incompatible side effects, from which pywin32 and likely others suffer. And without effectively hiding the MSVC & SDK dirs used by distutils - which complex projects like pywin32 need to inspect.

Just set_include_dirs() can again replace everything as the name "set" indicates (unlike add_include_dir()) .
But set_include_dirs() is probably rarely used from outside - and if, then in the meaning of "set = replaces" so far and then #153 will so far will also breaks things incompatibly.

because if set_include_dirs is invoked after initialization, it erases the values previously added

I think usually that is the meaning of "set".
And I thinks that is not about the actual real motive(s) of #153 or actual error reports as far as I understood ?

Or which actual problems does #153 solve so far, which this PR here doesn't solve?

When this "API" break & limitation would remain as is (without this or another solution), then something like this kind of strange workaround would need to stay in pywin32:
https://github.com/mhammond/pywin32/blob/b52884ae5417fd2f857007ae1c74d4f753591d36/setup.py#L688
mhammond/pywin32@b52884a
mhammond/pywin32#1936
The MSVC & SDK dirs are required for several reasons in setup.py.

I have already tested the complex pywin32 build (inserting includes in various ways) with this PR - also with --include-dirs.

The altered and extended tests check that SDK includes are not erased by build_ext.include_dirs (carries --include-dirs) and ext.include_dirs (as it was before). Which I think was (well known) the issue - and which e.g. required a workaround in pywin32 setup.py in _fixup_sdk_dirs() ( (mhammond/pywin32#1936 can now make that obsolete finally.)

[jaraco]

I think usually that is the meaning of "set".

Perhaps, except it's explicitly documented that set shouldn't affect compiler defaults.

[jaraco]

Or which actual problems does #153 solve so far, which this PR here doesn't solve?

I'm trying to work toward a more layered approach, with better abstractions around state, so that there's not a lot of entangled concerns. Right now, the compiler is very complicated by the _fix methods and initialize and the fact that add_*_dirs exists but isn't used anywhere. I'd like to work toward interfaces that meet the user's needs and not much more, with tested interfaces that protect the expectations.

[kxrob]

except it's explicitly documented that set shouldn't affect compiler defaults.

This does not affect any list of standard include directories that the compiler may search by default.

I think this 23 years old doc just refers, as a rather trivial hint, to the standard directories which are potentially built internally into a compiler binary and not manageable by distutils due to low level reasons. E.g. into gcc specifically for a certain distribution (gcc -xc -E -v -). I guess those default dirs are searched last though. MSVC probably would just search the cwd when no environment INCLUDE or -I 's are given.

set_include_dirs() is used inside distutils only in few similar cases right after compiler instantiation. If anybody used set_include_dirs / .include_dirs from outside so far the last 23+ years - its in in the old and usual simple sense "everything that's not hard-built into MSVC / gcc ... and goes out as -I option or env." . When problems with missing / false libs occur, one first looks at the command line executed ...
Maybe just the doc needs clarifications towards what was in effect all the time so far and what a "set" usually does in a simple manner.

The SDK dirs on Windows are not bound or somehow internal to the compiler (several SDK kits are usually installed - 4 here currently), they are kind of extra libraries just often needed on Windows and provided as defaults.
And there are reasons to be even able to change the used SDK paths / version in certain cases in a setup script - by modifying compiler.include_dirs.

The compatibility issue here would less be about what set_include_dirs() does (it is and was not used e.g. in pywin32), but about what the (non-private var/property) compiler.include_dirs shows for inspection and potentially accepts hands-on (the old fixup in pywin32 modified it by extending / +=, as does the new workaround so far.)

[zooba]

On one hand, I do prefer the behaviour of this patch over what's currently in there for MSVC (I didn't notice just how much Jason(?) rewrote my original PR - I was using private instance attributes, not class variables... can see that breaking under certain [mis]uses...). Though calling set_... multiple times should really replace anything that had been previously set.

On the other hand, it would break the behaviour for gcc, where set_include_dirs really should clear out anything previously added by the user. gcc finds its own default headers while running, whereas MSVC needs them to be passed from the environment, which is why MSVC was broken for so long (code written by gcc users 😉) and why the behaviour has to be subtly different for each OS.

It's a shame that distutils was never designed properly in the first place. I'm sure the intent was for the command line interface to be the only "public" API, and it would have saved a lot of angst if everyone who was monkey patching it knew that they'd be responsible to fix their issues later on. Alas, we do things carefully instead. However, I'd argue that include_dirs was never meant for introspecting the default system include directories, so relying on that was a calculated risk (I also warned you that it would break after the patch here was merged).