all: drop NO_NPM mode

This provided a nice workaround for the pain of running `npm install`,
but we are about to solve this problem in a different way.

Let's start by removing this workaround.

.github/workflows/unit-tests-refresh.yml

@@ -31,7 +31,7 @@ jobs:
         run: sudo containers/unit-tests/start CC=clang distcheck
 
       - name: Run i386 gcc distcheck test
-        run: sudo containers/unit-tests/start :i386 NO_NPM=1 distcheck
+        run: sudo containers/unit-tests/start :i386 distcheck
 
       - name: Log into container registry
         run: sudo podman login -u cockpituous -p ${{ secrets.COCKPITUOUS_GHCR_TOKEN }} ghcr.io

.github/workflows/unit-tests.yml

@@ -9,7 +9,7 @@ jobs:
          - check-memory
          - CC=clang :i386 check-memory
          - CC=clang distcheck
-         - :i386 NO_NPM=1 distcheck
+         - :i386 distcheck
       fail-fast: false
     timeout-minutes: 60
     steps:

HACKING.md

@@ -80,14 +80,6 @@ to build everything.  Cockpit has a single non-recursive Makefile.  You can
 only run `make` from the top-level and it will always rebuild the whole
 project.
 
-When building from git, you can optionally set the `NO_NPM` environment
-variable to `1` to prevent `autogen.sh` from invoking `npm`.  As a result,
-there will be no `node_modules` directory created.  In case this directory is
-missing, the build will avoid building the parts of cockpit which are
-dependent on node, which can be useful for working on the C parts.  You can
-also use `NO_NPM=0` or `NO_NPM=1` as an environment variable or flag to `make`
-to force building (or force not building) the node-related parts.
-
 You can run unit tests of the current checkout:
 
     $ make check

Makefile.am

@@ -73,32 +73,6 @@ V_TAR = $(V_TAR_$(V))
 V_TAR_ = $(V_TAR_$(AM_DEFAULT_VERBOSITY))
 V_TAR_0 = @echo "  TAR     " $@;
 
-# We set SKIP_NODE to 1 if we should avoid buidling anything that
-# depends on node_modules/ having been created.  This allows doing
-# variable expansions like x_DATA = $(FILES$(SKIP_NODE)) which will
-# expand to $(FILES) in the normal case, and to the empty variable
-# $(FILES1) in the case SKIP_NODE is set.  This is similar to the tricks
-# used by automake for verbosity control (ie: V=1 et al).
-#
-# We give the user explicit control over this process via the NO_NPM
-# make variable.  As a user you should use `make NO_NPM=1` or `make
-# NO_NPM=0` to force the value one way or the other.  Otherwise, we
-# guess, depending on the presence of the `node_modules` directory.
-SKIP_NODE := $(shell "$(top_srcdir)/tools/should-skip-node" "$(NO_NPM)")
-SKIP_NODE_ERROR = $(error Couldn't decide if we should depend on node_modules or not)
-SKIP_NODE_SHELLSTATUS_0 = ok
-SKIP_NODE_CHECK_SHELLSTATUS := $(SKIP_NODE_ERROR$(SKIP_NODE_SHELLSTATUS_$(.SHELLSTATUS)))
-
-# Make sure this fails early, and with a good message
-distdir: $(BUILT_SOURCES)
-	@if test -n "$(SKIP_NODE)"; then \
-	  echo; \
-	  echo "Can't dist cockpit without node_modules.  Try running tools/npm-install."; \
-	  echo; \
-	  exit 1; \
-	fi
-	$(MAKE) $(AM_MAKEFLAGS) distdir-am
-
 MV = mv -f
 
 COPY_RULE = \
@@ -189,7 +163,7 @@ WEBPACK_CONFIG = $(srcdir)/webpack.config.js
 WEBPACK_INPUTS =
 WEBPACK_OUTPUTS =
 WEBPACK_INSTALL =
-WEBPACK_DEPS = $(WEBPACK_PACKAGES$(SKIP_NODE):%=$(top_srcdir)/dist/%/Makefile.deps)
+WEBPACK_DEPS = $(WEBPACK_PACKAGES:%=$(top_srcdir)/dist/%/Makefile.deps)
 
 # required for running unit and integration tests; commander and ws are deps of chrome-remote-interface
 WEBPACK_TEST_DEPS = \
@@ -199,7 +173,7 @@ WEBPACK_TEST_DEPS = \
 	node_modules/ws \
 	$(NULL)
 
-noinst_SCRIPTS += $(MANIFESTS$(SKIP_NODE))
+noinst_SCRIPTS += $(MANIFESTS)
 EXTRA_DIST += $(MANIFESTS) $(WEBPACK_DEPS) webpack.config.js
 
 # Nothing generates this directly, but it's included as a dependency in

autogen.sh

@@ -39,7 +39,7 @@ if [ -z "$(which glib-gettextize)" ]; then
     exit 1
 fi
 
-test "${NO_NPM:-}" != "1" && tools/npm-install
+tools/npm-install
 
 rm -rf autom4te.cache
 

containers/unit-tests/README.md

@@ -32,8 +32,6 @@ to modify its behaviour:
 
  - `CC=othercc` to set the `CC` environment variable inside the container (ie:
    to build with a different compiler)
- - `NO_NPM=1` to do the initial build without `node_modules`.  Some scenarios
-   (ie: distcheck) may run tools/npm-install anyway, after the initial build.
  - `:tag` to specify a different tag to use for the `cockpit/unit-tests` image
    (eg: `i386`)
 

containers/unit-tests/run.sh

@@ -37,7 +37,7 @@ if [ ! -e /source/.git ]; then
     exit 1
 fi
 git clone /source /tmp/source
-if [ "${NO_NPM:-}" != "1" -a -d /source/node_modules ]; then
+if [ -d /source/node_modules ]; then
     cp -r /source/node_modules /tmp/source/
 fi
 cd /tmp/source

containers/unit-tests/scenario-dist.sh

@@ -1,17 +1,5 @@
 #!/bin/sh -eux
 
-if [ "${NO_NPM:-}" = "1" ]; then
-    # We can't 'make dist' with NO_NPM, so unset it and finish the build
-    unset NO_NPM
-    test ! -d /tmp/source/node_modules # this shouldn't be here
-    if [ -d /source/node_modules ]; then
-        cp -r /source/node_modules /tmp/source
-    fi
-    tools/npm-install
-    test -d /tmp/source/node_modules # this must surely be here now
-    make
-fi
-
 make NO_DIST_CACHE=1 dist
 
 # container has a writable /results/ in the "dist" scenario, but not in others; copy dist tarball there

containers/unit-tests/start

@@ -13,7 +13,7 @@ while test $# -gt 0; do
   case "$1" in
     --docker) docker="docker";;
     --podman) docker="podman";;
-    NO_NPM=1) no_npm='--env=NO_NPM=1';;
+    NO_NPM=1) no_npm='--env=NO_NPM=1';; # noop
     CC=*) ccarg="--env=$1";;
     shell) flags=-it; cmd=/bin/bash;;
     :*) image=ghcr.io/cockpit-project/unit-tests"$1";;

tools/should-skip-node

@@ -1,30 +0,0 @@
-#!/bin/sh -eu
-
-# Print '1' if the building of node/npm-dependent components should be skipped
-
-srcdir=`dirname $0`
-test -z "$srcdir" && srcdir=.
-top_srcdir="${srcdir}/.."
-
-(test -f "${top_srcdir}/src/ws/cockpit.service.in") || {
-    echo "**Error**: Directory '${top_srcdir}' does not look like the top-level Cockpit directory"
-    exit 1
-}
-
-case "${1:-}" in
-  1)
-    echo 1;
-    ;;
-
-  0)
-    ;;
-
-  '')
-    test -d "${top_srcdir}/node_modules" || echo 1
-    ;;
-
-  *)
-    echo "Invalid value for NO_NPM: $1" >&2
-    exit 1
-    ;;
-esac