meetecho / janus-gateway

Janus WebRTC Server
https://janus.conf.meetecho.com
GNU General Public License v3.0
8.17k stars 2.47k forks source link

[1.x] *** No rule to make target 'npm/dist/janus.cjs.js', needed by 'all-am'. Stop. #3416

Closed graugans closed 1 month ago

graugans commented 1 month ago

What version of Janus is this happening on? JANUS_VERSION=bad60d7032026af453a98ed7aba7fbc083385ea2

Have you tested a more recent version of Janus too? I pulled the latest version from master

Was this working before? Nope, have never tried it before

Is there a gdb or libasan trace of the issue? No, I am not able to build

Additional context When I follow the instructions given in Using janus.js as JavaScript module I do get the following error message:

*** No rule to make target 'npm/dist/janus.cjs.js', needed by 'all-am'.  Stop.

This is my command line to build:

autogen.sh   && \
./configure --enable-post-processing \
                     --prefix=/usr/local  \
                     --enable-javascript-common-js-module=yes    && \
make         && \
make install && \
make configs

Both npm and nodejs I do have installed from the Ubuntu 22.04 package feed.

atoppi commented 1 month ago

It could be a regression after https://github.com/meetecho/janus-gateway/commit/493a97bd0a5253d11f4a3633be93df80b801833c. Can you try building the module directly with npm? Head to the project root and then issue:

npm install
npm run rollup -- --o npm/dist/janus.cjs.js --f cjs

Side note: in case you need the ES module just replace the last step with

npm run rollup -- --o npm/dist/janus.es.js --f es
graugans commented 1 month ago

Yes, I had to update the nodejs version to run the npm stuff like this:

 cd /tmp  \
&& curl -sL https://deb.nodesource.com/setup_18.x -o nodesource_setup.sh \
&& bash nodesource_setup.sh \
&& apt-get -y update  \
&& apt-get install -y nodejs \
&& apt-get clean \
&& rm -rf /var/lib/apt/lists/* \
&& rm -rf /tmp/nodesource_setup.sh

I then was able to build the js file:

cd /usr/local/src/janus-gateway/npm \
&& npm install \
&& npm run rollup -- --o dist/janus.cjs.js --f cjs \
&& mkdir -p /usr/local/share/janus/javascript \
&& cp -r /usr/local/src/janus-gateway/npm/dist/janus.cjs.js /usr/local/share/janus/javascript

I have re-checked if updating the nodejs version also will fix my issue with make but this was not the case, even everything was rebuild fresh from a separate Docker layer.

atoppi commented 1 month ago

Have you tried what I suggested?

graugans commented 1 month ago

Have you tried what I suggested?

When I run the npm from the project root as suggested I do get those errors:

0.905 npm error code ENOENT
0.905 npm error syscall open
0.906 npm error path /usr/local/src/janus-gateway/package.json
0.906 npm error errno -2
0.908 npm error enoent Could not read package.json: Error: ENOENT: no such file or directory, open '/usr/local/src/janus-gateway/package.json'
0.908 npm error enoent This is related to npm not being able to find a file.
0.908 npm error enoent
0.909 
0.909 npm error A complete log of this run can be found in: /root/.npm/_logs/2024-08-21T06_24_51_783Z-debug-0.log
------

When I change to the npm sub-folder everything works fine, so it seems to be an issue with the Makefile.am

cd /usr/local/src/janus-gateway/npm \
&& npm install \
&& npm run rollup -- --o dist/janus.cjs.js --f cjs 
atoppi commented 1 month ago

The project root folder is the one containing the sources. In your case npm is unable to find the package.json, are you sure it is there?

graugans commented 1 month ago

Sorry, I have two versions I am testing with the bad60d7032026af453a98ed7aba7fbc083385ea2 does not yet contain the package.json in the project root. With the latest commit calling npm from the project root works!

graugans commented 1 month ago

This patch fixed the make call for me:

From cac4844534d805d7694f3070a05f2662416960b0 Mon Sep 17 00:00:00 2001
From: Christian Ege <ch.ege.io>
Date: Wed, 21 Aug 2024 08:56:34 +0200
Subject: [PATCH] enable js modules always

Signed-off-by: Christian Ege <ch.ege.io>
---
 Makefile.am | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 2a35c848..9a65a194 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -91,11 +91,9 @@ EXTRA_DIST += $(jsmodules_DATA)

 if ENABLE_JAVASCRIPT_MODULES

-npm/node_modules/rollup/dist/bin/rollup: npm/package.json
-   cd npm && $(NPM) install && touch node_modules/rollup/dist/bin/rollup
-
-npm/dist/janus.%.js: html/janus.js npm/node_modules/rollup/dist/bin/rollup npm/rollup.config.mjs npm/module.js
-   cd npm && $(NPM) run rollup -- --o $(patsubst npm/%,%,$@) --f $*
+npm/dist/janus.%.js:
+   $(NPM) install
+   $(NPM) run rollup -- --o $@ --f $*

 endif

Hopefully I find some time today to clean it up and create an MR.

atoppi commented 1 month ago

Thanks @graugans I'll push a similar fix asap.

atoppi commented 1 month ago

It should be fixed now.