Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 46.05%. Comparing base (3d9129d) to head (ea2ea28).

@@           Coverage Diff           @@
##             main    #1196   +/-   ##
=======================================
  Coverage   46.05%   46.05%           
=======================================
  Files          11       11           
  Lines        4942     4942           
  Branches     1345     1345           
=======================================
  Hits         2276     2276           
  Misses       2666     2666           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AFOliveira thanks

This doesn't seem like a bug that should be fixed in this repository. Do all other node.js repos have this workaround? What provokes it, and are we doing something wrong that leads to these ownership states? Is it because we're pretending to be root within the container?

As @ThinkOpenly mentions, I think you are trying to solve a container problem here. If that's the case, we should probably figure out a solution that does something with the Dockerfile rather than try to adjust settings out of context.

@ThinkOpenly , I encountered this problem when unprivileged shell gitlab-runner tried to remove node_modules/ content so I suppose it's better to fix in upstream project because someone else might encounter it. I'm not sure that it's a common npm issue because not all content of modules have unexpected owner. Most modules do not have this problem.

@ThinkOpenly ,

Do all other node.js repos have this workaround?
Yes, I see quite a lot of similar solutions: https://github.com/search?q=chown+-R+%24%28whoami%29+%22node_modules%2F%22&type=code

@ThinkOpenly,

What provokes it, and are we doing something wrong that leads to these ownership states? Is it because we're pretending to be root within the container?

I think it depends on how provided package was deployed. If we compare a couple of yauzl versions which is used at the project then we can notice ownership changes:

$ git clone https://github.com/riscv-software-src/riscv-unified-db.git
$ docker run -it -v $PWD:$PWD -w "$PWD" riscvintl/udb:0.9 bash

$ wget https://registry.npmjs.org/yauzl/-/yauzl-2.10.0.tgz
$ tar --same-owner -xvzf yauzl-2.10.0.tgz
$ ls -l package/
total 84
-rw-rw-r-- 1 ubuntu ubuntu  1077 Apr 23  2018 LICENSE
-rw-rw-r-- 1 ubuntu ubuntu 31177 Jul  3  2018 README.md
-rw-r--r-- 1 root   root    7872 Oct 26  1985 fd-slicer.js
-rw-rw-r-- 1 ubuntu ubuntu 33069 Jul  3  2018 index.js
-rw-rw-r-- 1 ubuntu ubuntu   881 Jul  3  2018 package.json
# OWNER IS UBUNTU

$ wget https://registry.npmjs.org/yauzl/-/yauzl-3.0.0.tgz
$ tar --same-owner -xvzf yauzl-3.0.0.tgz
$ ls -l package/
total 88
-rw-r--r-- 1 root root  1077 Oct 26  1985 LICENSE
-rw-r--r-- 1 root root 33238 Oct 26  1985 README.md
-rw-r--r-- 1 root root  7872 Oct 26  1985 fd-slicer.js
-rw-r--r-- 1 root root 33427 Oct 26  1985 index.js
-rw-r--r-- 1 root root   737 Oct 26  1985 package.json
# OWNER IS ROOT

$ cat /etc/passwd
root:x:0:0:root:/root:/bin/bash
ubuntu:x:1000:1000:Ubuntu:/home/ubuntu:/bin/bash

Looks like npm extracts packages and stores files ownership if it's possible. Perhaps, npm packages updating may help as well. But it will be better to work inside one container with globally preinstalled npm dependencies.

$ npm i --verbose
...
npm http fetch GET 200 https://registry.npmjs.org/yauzl/-/yauzl-2.10.0.tgz 808ms (cache miss)
...
$ ll node_modules/yauzl/
total 92
drwxr-xr-x   2 root   root    4096 Oct 28 18:01 ./
drwxr-xr-x 359 root   root   12288 Oct 28 18:01 ../
-rw-r--r--   1 ubuntu ubuntu  1077 Oct 28 18:01 LICENSE
-rw-r--r--   1 ubuntu ubuntu 31177 Oct 28 18:01 README.md
-rw-r--r--   1 ubuntu ubuntu 33069 Oct 28 18:01 index.js
-rw-r--r--   1 ubuntu ubuntu   881 Oct 28 18:01 package.jso
# OWNER IS UBUNTU

$ npm i yauzl # v3.2.0 is the last version
$ ll node_modules/yauzl/
total 124
drwxr-xr-x   2 root root  4096 Oct 28 18:17 ./
drwxr-xr-x 345 root root 12288 Oct 28 18:17 ../
-rw-r--r--   1 root root  1077 Oct 28 18:17 LICENSE
-rw-r--r--   1 root root 46785 Oct 28 18:17 README.md
-rw-r--r--   1 root root  8677 Oct 28 18:17 fd-slicer.js
-rw-r--r--   1 root root 38952 Oct 28 18:17 index.js
-rw-r--r--   1 root root   776 Oct 28 18:17 package.json
# OWNER IS ROOT

@dhower-qc , We can install packages during to image build with -g option as alternative solution. This way won't keep npm packages in mounted workspace. But it makes sense to do when all commads will execute inside one container.

We've talked about moving to a model where everything gets installed in the container (e.g., #973). At this point in the project, I'm all for it. @qwersem, if that is something you have bandwidth to tackle (maybe just starting with the NodeJS dependencies), go for it.

I was working on it for other reasons, but I think #1220 will resolve this.

@jordancarlin, agree with you, Global npm installation should help.