fix(builder): Skip minify of a .js with an existing -dbg.js sibling

[matz3]

A -dbg.js next to its .js in the sources is a broken input state: the minify task owns the -dbg.js path and generates it from the .js, so a pre-existing one must not be there. The glob matched both as separate map-step keys. The .js key wrote its generated debug variant to the -dbg.js path, which the -dbg.js key also wrote - two concurrent keys writing one path, which the shared assertDistinctWrite guard rejected, so the build crashed.

To stay robust rather than crash, filter the step keys to skip both members of such a pair: a .js whose -dbg.js sibling already exists is left as it is (that -dbg.js already serves as the debug variant), the -dbg.js is passed through, and a warning is logged. The sibling paths are derived with the shared getDebugName and getNonDebugName helpers, so the special .view.js / .controller.js suffixes are handled consistently.

The integration test reproduces the crash and pins the expected skip; unit tests cover the filter and the warning.

Also add a TODO in the minify processor: a lone -dbg.js (no .js sibling) still reaches the processor and gets a -dbg-dbg.js debug variant, which is almost certainly unintended. Clarify whether such files should be skipped instead.

[codeworrior]

Maybe I forgot about an earlier decision (e.g. in the wake of the transition from Maven to CLI), but IMO the existence of a *-dbg.js file is not a broken input state. Maybe the capability to deal with it went lost somewhen, but I'm sure we used that in the past, especially for some bootable bundles (for which normally no dbg file is created, but restarting in debug mode required such a file in the past) and for files that our old tooling could not minify at that time (e.g. newer ES6 code).

I searched the SAPUI5 codebase and only found one example in sap.ushell_abap (sap/ushell_abap/thirdparty/sap-xhrlib-esm-dbg.js, an ESM file).

[matz3]

Thanks for the feedback. So is your comment only about the framing of this scenario as broken, or is it also supposed to be handled differently in the minify task?

[codeworrior]

Mhmm, good question. My first reaction was "about framing the scenario as broken only". But reading the commit message again, it might be about the warning, too. sap.ushell_abap will see a warning in each build although they can't handle it differently (as we do not support ES modules?).

[RandomByte]

I was also wondering about past cases. I believe this is not the first time we encounter this and have to fix bugs (around pre-existing source maps maybe?) because of it.

UI5 CLI v4 seems to be able to deal with such cases while not logging a warning, right?

[matz3]

UI5 CLI v4 seems to be able to deal with such cases while not logging a warning, right?

I assume that the result is at least unexpected (both files end up being minified, plus a -dbg-dbg.js file) and it might even be non-deterministic because of the parallel processing. But I need to verify this.

[matz3]

I searched the SAPUI5 codebase and only found one example in sap.ushell_abap (sap/ushell_abap/thirdparty/sap-xhrlib-esm-dbg.js, an ESM file).

In the current internal untested version I can see that the sap/ushell_abap/thirdparty/sap-xhrlib-esm-dbg.js is being minified and there is a sap/ushell_abap/thirdparty/sap-xhrlib-esm-dbg-dbg.js in the build output.

I agree, we should consider changing the log from warning to verbose, as this can be expected.
But on the other hand, we do have minification excludes that can be configured, and I would assume they are exactly for that purpose. Maybe the warning should include a hint to configure the exclude if this is an expected input for the build.

I assume that the result is at least unexpected (both files end up being minified, plus a -dbg-dbg.js file) and it might even be non-deterministic because of the parallel processing. But I need to verify this.

It's non-deterministic if the input .js and -dbg.js files are not aligned (i.e. if they are out of sync or minified differently than our task would do).

[d3xter666]

With the current architecture a new build that finds a -dbg file will read it as an input. And since it can't be a file we generated in this build - so it's either a real source file, or something a previous run left behind.

Whether a -dbg source file is valid, I don't know (the ushell ESM case says maybe it is, sometimes).

But I think the crash shows we need something more than a fix in the minify task. Right now a build that fails halfway can leave files behind that the next build picks up as input. Either:

• some recovery after a crash (cleanup or some kind of resume), or
• make cache + its generation atomic (write/output to an external, sandboxed dir until everything is ready, then commit everything/write the cache at once). It's also what Nix and Bazel do - build into a sandbox, move the output in only on success, so a failed build leaves nothing behind.

[matz3]

Right now a build that fails halfway can leave files behind that the next build picks up as input

@d3xter666 would you mind explaining what you mean here and how it relates to this issue? So far, this is about receiving -dbg.js files as input in a build (either from the sources, or (in theory) produced by a task that ran before).