[bug#49946] [PATCH 1/3] gnu: node: Avoid duplicating build phases.

[Pierre Langlois]

Hi Philip,

Philip McGrath <philip@philipmcgrath.com> writes:

> * gnu/packages/node.scm (node)[arguments]: Split 'patch-files phase
> into 'patch-hardcoded-program-references and
> 'delete-problematic-tests. Adapt those phases and 'configure to work
> unmodified on node-lts.
> (node, node-lts)[inputs]: Use bash-minimal rather than bash.
> (node-lts)[arguments]: Inherit 'patch-hardcoded-program-references,
> 'delete-problemating-tests, and 'configure phases from the bootstrap
> node. Remove the 'patch-files phase, keeping its remaining
> non-inherited work in a new 'replace-llhttp-sources phase.

While I agree that most of the time, factoring out common code is a good
thing, I'm not sure it applies in the case of patching tests. The list
of tests is specific to a version and it's likely for each version to
need fixes. Having a common phase that describes the tests to patch for
2 versions (3 if we add node 16) is harder to maintain than three phases
IMO, even though they'll look similar indeed. Having to change commmon
code can also cause unnessecary rebuilds.

For example, I started working updating node last weekend and saw these
test changes:

 - 14.16 -> 14.17: Delete test/parallel/test-https-agent-unref-socket.js,
                   requires networking
 - 16: Extra test needs /bin/sh patched 
test/parallel/test-stdin-from-file-spawn.js"
       A couple tests were renamed:
       test/parallel/test-cluster-master-error.js -> 
test/parallel/test-cluster-primary-error.js
       test/parallel/test-cluster-master-kill.js -> 
test/parallel/test-cluster-primary-kill.js

That being said, I definetely agree we should have a separate phase for
the replacement of the llhttp source, that's logically different from
patching tests, and is unlikely to change version to version.

Keeping the list of tests local to each packages allows to add node 16
while avoiding rebuilding the others, does this make sense? I could be
wrong here of course :-).

Thanks,
Pierre

signature.asc
Description: PGP signature