GNU bug report logs - #49946
[PATCH 00/31] Tree-sitter, node-gyp addon support and emacs-tree-sitter

Previous Next

Full log

View this message in rfc822 format

From: Pierre Langlois <pierre.langlois <at> gmx.com>
To: Philip McGrath <philip <at> philipmcgrath.com>
Cc: Pierre Langlois <pierre.langlois <at> gmx.com>, 49946 <at> debbugs.gnu.org, Maxime Devos <maximedevos <at> telenet.be>
Subject: [bug#49946] [PATCH 1/3] gnu: node: Avoid duplicating build phases.
Date: Sat, 02 Oct 2021 12:03:13 +0100

[Message part 1 (text/plain, inline)]

Hi Philip,

Philip McGrath <philip <at> 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 (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.