1. I'm not sure what this is for - why do the dependency files need to go through mwccgap? Obviously I'm just missing something, not trying to shoot down the PR, just don't understand it (yet 😅)
2. Could you make "mwccpsp.exe" configurable (take the value from the args), as mwccgap also supports mwccps2 😎

Drat. I ran black on the sources but didn't catch the mypi rules. Will fix.

This is to support getting dependency files generated at build time. These can be used with a build system to rebuild objects when needed and avoid rebuilding them when not (related: Xeeynamo/sotn-decomp#2792)

So the TL;DR is that mwccpsp will create dependency files as part of compilation, but because mwccgap only cares about the compiled object data (and writes the .c file to a temporary directory), the dependency files are discarded?

And then in addition to that, you post process the dependency files to fixup the paths?

Not sure if it meets your requirements, but we have a battle-tested transform_dep.py script that handles converting wine/wibo paths in MWCC depfiles: https://github.com/encounter/dtk-template/blob/main/tools/transform_dep.py (license CC0)

@mkst Yes, that's exactly the use case.

I'm not sure how you want MWCC to be able to be overridden in the unit test, but I added an environment variable check.

PATH=$PATH:../wibo MWCC=../sotn-decomp/bin/mwccpsp.exe python3 -m unittest

@encounter the use case is similar, but minimally mwccgap needs to grab the output and put it int the equivalent place outside of the tmpdir it uses. Because the mwccgap caller doesn't really "know" about windows paths, it makes sense to me that the translation would happen in mwccgap instead of an external tool.

The other issue is that we pipe our sources into mwccgap after filtering with another script. mwcc doesn't support this, but mwccgap does by creating an ephemeral temp file. That temp file shows up in the dependency file and needs to stripped to make sense to the caller.

I tried to keep path translation as close to wibo as possible, but also have a separate PR for wibo to expose path translation as well. I would like to use the tool that translates the paths to translate the paths instead of having a duplicate implementation, if possible. We use wibo, but I hadn't considered the wine use case, but wouldn't winepath work for path translation?

I hadn't realised the hardcoding was in the unit tests (on holiday and on my phone), so ignore that comment.

I think I'd prefer if there was an explicit flag to pass to mwccgap to enable this behaviour (off by default) rather than looking at the c_flags. Also feeling like the path manipulation stuff doesn't really belong in mwccgap but appreciate its possibly the easiest place to do it...

I hadn't realised the hardcoding was in the unit tests

I made the change and think making it configurable even for tests is reasonable. I didn't want to assume any version of mwcc was available for the tests to be runnable, so these are already disabled unless they can be found, but making them work with mwccps2 makes sense.

I think I'd prefer if there was an explicit flag to pass to mwccgap to enable this behaviour

I might be misinterpreting the request, but I wanted to avoid mwccgap.py and mwccgap/mwccgap.py having to know details about explicit mwcc compiler options and pass those around. Except for the make_rule, Compiler is the only place those options are evaluated and they are necessary for mwcc.

I also didn't want mwccgap specific args for wrapping mwcc args. I'd prefer not to change the option names because they mirror gcc/clang options with the same meaning. I want behavior like you would expect if mwcc was a native app and it was being used directly, so if you check mwcc --help, you'd know what to expect.

Also feeling like the path manipulation stuff doesn't really belong in mwccgap but appreciate its possibly the easiest place to do it…

As much as possible, I was trying to make the typical use case Just Work™. Having an external post processing step complicates build system logic. Just like mwccgap's handling of ASM, users get the output they expect.

I just ran into an issue this change would have addressed again and wanted to see if the issue is the feature or the implementation and how to get this merged in.

Headers not being tracked nearly cost me a great deal of time this week (and certainly has I the past). I usually use this patch in my working branch, but had forgotten that it was stashed. I was working on a header file shared between PS and PSP versions of the game and rebuilding as I went. The PSP build continued to match successfully until a clean build much later. Then I remembered I had to manually touch any of the C files that include any of the headers were used by on the PSP side to get them to rebuild.

In sotn-decomp the PS1 build graph It leverages the GCC generated dependency files (-MMD) to track header dependencies and will rebuild the appropriate things when headers change:

$> touch src/st/e_bat.h ; ninja | cat
[1/18] check config/check.hd.sha
[2/18] check config/check.pspeu.sha
[3/18] psx cc src/st/np3/e_bat.c
[4/18] psx cc src/st/dai/e_bat.c
[5/18] psx cc src/st/no3/e_bat.c
⋮
[12/18] check config/check.us.sha
⋮

However, note that those 3 C files are also compiled for PSP via mwccgap and ninja didn't rebuild them (the psp cc rule) because it doesn't have dependency files to know those files were effected.

With the change in this PR the same files get recompiled for the PSP as well:

$> touch src/st/e_bat.h ; ninja | cat
[1/33] check config/check.hd.sha
[2/33] psx cc src/st/no3/e_bat.c
[3/33] psx cc src/st/np3/e_bat.c
[4/33] psx cc src/st/dai/e_bat.c
⋮ 
[11/33] check config/check.us.sha
[12/33] psp cc src/st/dai/e_bat.c
[13/33] psp cc src/st/np3/e_bat.c
[14/33] psp cc src/st/no3/e_bat.c
⋮
[21/33] check config/check.pspeu.sha

The PS1 and PSP build graphs have around 2000 and 700 nodes, respectively. It's impractical to rebuild all files for any header change. On the GCC side the dependency output allows us to rebuild just what's necessary. On the PSP side we can't currently get those dependency files so we either selectively configure a hardcoded list of "hot" header files that will rebuild the world, or we don't track it at all.

An alternative is to rebuild everything anytime anything in the source tree changes. On the PSP side, a build that just recompiles all C files, links them, and verifies the checksum takes 13s. It rebuilds 619 nodes in the build graph. With dependency tracking for the e_bat.h file the same operation takes 0.6s, and rebuilds 16 nodes. 40x more build steps and 20x longer build.

A different approach would be to manually track header files with each c file that uses them (recursively). I'd consider that unmaintainable, error prone, and difficult to verify for correctness.

If you're ninja, make, or any other build system that can use dependency files, it is a significant boon to productivity and the development cycle. For ninja, for example, add -MMD and -gccdep to the mwccgap command line, add depfile="$out.d" and deps="gcc" to the build rule. Going forward ninja tracks all #include. Touching an included file will rebuild anything that includes it.