Skip to content

Conversation

@targos
Copy link
Member

@targos targos commented Sep 25, 2025

Refs: #4144

@targos
Copy link
Member Author

targos commented Sep 25, 2025

These are copies of the ubuntu2204 files with changes in:

  • FROM ubuntu:24.04 / FROM arm32v7/ubuntu:24.04
  • apt-get: clang-19, gcc and g++ (default version is 13), openjdk-21-jre-headless

@targos
Copy link
Member Author

targos commented Sep 25, 2025

@targos
Copy link
Member Author

targos commented Sep 25, 2025

#6 [ 3/14] RUN pip3 install tap2junit==0.2.0
#6 0.755 error: externally-managed-environment
#6 0.755 
#6 0.755 × This environment is externally managed
#6 0.755 ╰─> To install Python packages system-wide, try apt install
#6 0.755     python3-xyz, where xyz is the package you are trying to
#6 0.755     install.
#6 0.755     
#6 0.755     If you wish to install a non-Debian-packaged Python package,
#6 0.755     create a virtual environment using python3 -m venv path/to/venv.
#6 0.755     Then use path/to/venv/bin/python and path/to/venv/bin/pip. Make
#6 0.755     sure you have python3-full installed.
#6 0.755     
#6 0.755     If you wish to install a non-Debian packaged Python application,
#6 0.755     it may be easiest to use pipx install xyz, which will manage a
#6 0.755     virtual environment for you. Make sure you have pipx installed.
#6 0.755     
#6 0.755     See /usr/share/doc/python3.12/README.venv for more information.
#6 0.755 
#6 0.755 note: If you believe this is a mistake, please contact your Python installation or OS distribution provider. You can override this, at the risk of breaking your Python installation or OS, by passing --break-system-packages.
#6 0.755 hint: See PEP 668 for the detailed specification.
#6 ERROR: process \"/bin/sh -c pip3 install tap2junit==0.2.0\" did not complete successfully: exit code: 1
------
 > [ 3/14] RUN pip3 install tap2junit==0.2.0:
0.755     sure you have python3-full installed.
0.755     
0.755     If you wish to install a non-Debian packaged Python application,
0.755     it may be easiest to use pipx install xyz, which will manage a
0.755     virtual environment for you. Make sure you have pipx installed.
0.755     
0.755     See /usr/share/doc/python3.12/README.venv for more information.
0.755 
0.755 note: If you believe this is a mistake, please contact your Python installation or OS distribution provider. You can override this, at the risk of breaking your Python installation or OS, by passing --break-system-packages.
0.755 hint: See PEP 668 for the detailed specification.
------
Dockerfile:33
--------------------
  31 |           automake
  32 |     
  33 | >>> RUN pip3 install tap2junit==0.2.0
  34 |     
  35 |     RUN addgroup --gid 1000 iojs

Not sure what's the best way to manage the virtual environment in the Docker images.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Non-blocking comment)
I think it's worth updating to keep this in step with the other platforms, but just noting that we don't currently use these on Node.js >= 24 as it doesn't build on 32-bit ARM so there's less of a need to update for clang here.

@richardlau
Copy link
Member

Not sure what's the best way to manage the virtual environment in the Docker images.

We're already doing this on Alpine:

ENV PATH /usr/local/venv/bin:/usr/lib/ccache/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
to add the venv bin to the path and
RUN python3 -m venv /usr/local/venv
RUN pip3 install tap2junit=={{ tap2junit_version }}

@targos
Copy link
Member Author

targos commented Sep 25, 2025

Ah right, I forgot. Thanks!

@richardlau
Copy link
Member

No problem, I forget all kinds of stuff in our Ansible set up 😅.

@targos
Copy link
Member Author

targos commented Sep 25, 2025

Next error:

#8 [ 5/15] RUN addgroup --gid 1000 iojs
#8 0.645 fatal: The GID `1000' is already in use.
#8 ERROR: process \"/bin/sh -c addgroup --gid 1000 iojs\" did not complete successfully: exit code: 21

@richardlau
Copy link
Member

Next error:

#8 [ 5/15] RUN addgroup --gid 1000 iojs
#8 0.645 fatal: The GID `1000' is already in use.
#8 ERROR: process \"/bin/sh -c addgroup --gid 1000 iojs\" did not complete successfully: exit code: 21

It looks like Ubuntu 24.04 images have added a non-root ubuntu user with ID 1000:

Workarounds seem to be to delete or reassign the user.

@targos
Copy link
Member Author

targos commented Sep 25, 2025

Yes, I found that and pushed 76db7cf just before your message.
I'll try to deploy when I'm back home.

@richardlau
Copy link
Member

richardlau commented Sep 25, 2025

I don't think it matters to us what the user is actually called (though it'll be confusing if it's inconsistent with elsewhere), but it is important to keep the UID/GID to preserve permissions for the volume mount

ExecStart=/usr/bin/docker run --init --rm -v /home/{{ server_user }}:/home/{{ server_user }} --name node-ci-{{ item.name }} --sysctl net.ipv4.ip_unprivileged_port_start=1024 node-ci:{{ item.name }}

@targos
Copy link
Member Author

targos commented Sep 25, 2025

Images are built and running.

I started two random test builds: https://ci.nodejs.org/job/targos-node-test-commit-linux-containered/2/

@targos
Copy link
Member Author

targos commented Sep 25, 2025

@richardlau
Copy link
Member

richardlau commented Sep 25, 2025

There was one failure: https://ci.nodejs.org/job/targos-node-test-commit-linux-containered/2/nodes=ubuntu2404_sharedlibs_openssl111_x64/testReport/junit/(root)/parallel/test_process_execve_no_args/

I'm surprised this test isn't failing on the existing sharedlib configurations. It executes nop which was added in nodejs/node#58412 to node.gyp. I think because it's there it ends up being dynamically linked to OpenSSL (the one we define in the container, in this case OpenSSL 1.1.1) but to run that we'd need LD_LIBRARY_PATH set but that test isn't passing the environment on when calling process.execve(). Unless older Ubuntu containers kept libcrypto.so.1.1 around for compatibility and we were picking that up instead of the one we compile for our containers and it just happened to work before?

FWIW nop doesn't need to be linked to OpenSSL, but I think it is as a side effect of being in node.gyp.

@targos
Copy link
Member Author

targos commented Sep 26, 2025

I don't know what to do here.

@richardlau
Copy link
Member

Okay I'm more confused now. There is clearly a difference in the way nop has been linked:

On test-ibm-ubuntu2204-docker-x64-1:

In node-ci-test-ibm-ubuntu2204_sharedlibs_container-x64-1:

iojs@18b37e409244:~/build/workspace/node-test-commit-linux-containered$ git status
HEAD detached from 3a4f7025bfd
Untracked files:
  (use "git add <file>..." to include in what will be committed)
        select-compiler.sh

nothing added to commit but untracked files present (use "git add" to track)
iojs@18b37e409244:~/build/workspace/node-test-commit-linux-containered$ ldd ./out/Release/node
        linux-vdso.so.1 (0x00007ffed0fb6000)
        libcrypto.so.1.1 => not found
        libssl.so.1.1 => not found
        libstdc++.so.6 => /lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007fe44b5d4000)
        libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fe44b4ed000)
        libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007fe452153000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fe44b2c4000)
        /lib64/ld-linux-x86-64.so.2 (0x00007fe452179000)
iojs@18b37e409244:~/build/workspace/node-test-commit-linux-containered$ ldd ./out/Release/nop
        linux-vdso.so.1 (0x00007fff21bdc000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f471fb59000)
        /lib64/ld-linux-x86-64.so.2 (0x00007f471fd8d000)
iojs@18b37e409244:~/build/workspace/node-test-commit-linux-containered$

i.e. nop is not linked to libcrypto, but in node-ci-test-ibm-ubuntu2404_sharedlibs_container-x64-2:

iojs@af2514bf5164:~/build/workspace/targos-node-test-commit-linux-containered$ git status
HEAD detached at d3ee5d9b2bc
Untracked files:
  (use "git add <file>..." to include in what will be committed)
        select-compiler.sh

nothing added to commit but untracked files present (use "git add" to track)
iojs@af2514bf5164:~/build/workspace/targos-node-test-commit-linux-containered$ ldd ./out/Release/node
        linux-vdso.so.1 (0x00007ffdaa57d000)
        libcrypto.so.1.1 => not found
        libssl.so.1.1 => not found
        libatomic.so.1 => /lib/x86_64-linux-gnu/libatomic.so.1 (0x00007f9437da3000)
        libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007f9431517000)
        libstdc++.so.6 => /lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007f9431299000)
        libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007f9437d73000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f9431087000)
        /lib64/ld-linux-x86-64.so.2 (0x00007f9437db4000)
iojs@af2514bf5164:~/build/workspace/targos-node-test-commit-linux-containered$ ldd ./out/Release/nop
        linux-vdso.so.1 (0x00007ffdbc6c5000)
        libcrypto.so.1.1 => not found
        libssl.so.1.1 => not found
        libatomic.so.1 => /lib/x86_64-linux-gnu/libatomic.so.1 (0x00007fb7ee28f000)
        libstdc++.so.6 => /lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007fb7ee011000)
        libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fb7edf28000)
        libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007fb7edef8000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fb7edce6000)
        /lib64/ld-linux-x86-64.so.2 (0x00007fb7ee2a5000)
iojs@af2514bf5164:~/build/workspace/targos-node-test-commit-linux-containered$

we can see that nop has been linked to the same libraries as node, and the not found libraries are because LD_LIBRARY_PATH isn't set. The question is, why has nop been linked to all of those extra libraries (compared to in the Ubuntu 22.04 container)? 🤔

@richardlau
Copy link
Member

The question is, why has nop been linked to all of those extra libraries (compared to in the Ubuntu 22.04 container)? 🤔

clang vs gcc?
I think this is the job that last ran on test-ibm-ubuntu2204_sharedlibs_container-x64-1:
https://ci.nodejs.org/job/node-test-commit-linux-containered/nodes=ubuntu2204_sharedlibs_openssl111_x64/52896/consoleFull

11:13:09   ccache g++-12 -o /home/iojs/build/workspace/node-test-commit-linux-containered/out/Release/nop -pthread -rdynamic -m64  -Wl,--start-group /home/iojs/build/workspace/node-test-commit-linux-containered/out/Release/obj.target/nop/test/nop/nop.o -L/opt/openssl-1.1.1w/lib/ -lcrypto -lssl -Wl,--end-group

and this is from https://ci.nodejs.org/job/targos-node-test-commit-linux-containered/2/nodes=ubuntu2404_sharedlibs_openssl111_x64/consoleFull:

18:20:46   ccache clang++-19 -o /home/iojs/build/workspace/targos-node-test-commit-linux-containered/out/Release/nop -pthread -rdynamic -m64  -Wl,--start-group /home/iojs/build/workspace/targos-node-test-commit-linux-containered/out/Release/obj.target/nop/test/nop/nop.o -L/opt/openssl-1.1.1w/lib/ -lcrypto -lssl -latomic -Wl,--end-group

and they look to have passed the same command line arguments with the only difference being g++-12 in the first case and clang++-19 in the second.

@targos
Copy link
Member Author

targos commented Sep 26, 2025

I guess -lcrypto means that it should link against libcrypto? So Clang is actually doing what we ask?

@richardlau
Copy link
Member

I guess -lcrypto means that it should link against libcrypto? So Clang is actually doing what we ask?

Yes, it would appear to be doing something reasonable based on the command line. I'm doing a check in a local Ubuntu 24.04 container with gcc 14 (I tried the default gcc 13 but that fails to build V8) to compare with using clang to build.

@richardlau
Copy link
Member

richardlau commented Sep 26, 2025

So I think we've run into https://stackoverflow.com/questions/79405331/why-does-ubuntus-linker-use-as-needed-by-default. Apparently Ubuntu's version of gcc defaults to linking with --as-needed, which will not link in any of the libraries, e.g. -lcrypto, if they are not needed. gcc on other Linux distributions, such as Fedora, or clang do not do this by default.

Passing --as-needed through to the linker (-Wl,--as-needed) with clang prevents the additional crypto and ssl libraries on the command line from being linked:

iojs@f90592b2fea9:~/node$ ccache clang++-19 -o /home/iojs/node/out/Release/nop -pthread -rdynamic -m64 -Wl,--start-group /home/iojs/node/out/Release/obj.target/nop/test/nop/nop.o -L/opt/openssl-1.1.1w/lib/ -lc
rypto -lssl -Wl,--end-group
iojs@f90592b2fea9:~/node$ ldd out/Release/nop
        linux-vdso.so.1 (0x00007fff69c62000)
        libcrypto.so.1.1 => /opt/openssl-1.1.1w/lib/libcrypto.so.1.1 (0x00007fc746ad5000)
        libssl.so.1.1 => /opt/openssl-1.1.1w/lib/libssl.so.1.1 (0x00007fc746a3d000)
        libstdc++.so.6 => /lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007fc7467bb000)
        libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fc7466d2000)
        libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007fc7466a4000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fc746490000)
        /lib64/ld-linux-x86-64.so.2 (0x00007fc746dcb000)
iojs@f90592b2fea9:~/node$ ccache clang++-19 -o /home/iojs/node/out/Release/nop -pthread -rdynamic -m64 -Wl,--as-needed -Wl,--start-group /home/iojs/node/out/Release/obj.target/nop/test/nop/nop.o -L/opt/openssl-1.1.1w/lib/ -lcrypto -lssl -Wl,--end-group
iojs@f90592b2fea9:~/node$ ldd out/Release/nop
        linux-vdso.so.1 (0x00007fffdcfc7000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007f0b13d79000)
        /lib64/ld-linux-x86-64.so.2 (0x00007f0b13f96000)
iojs@f90592b2fea9:~/node$

It would be better not to pass through the -L/-l options where they are not needed, but I don't think that's going to be easy to do with the shared library configurations. I'll try added -Wl,--as-needed just for the nop target in node.gyp, although I have a suspicion the option is specific to the GNU linker and may be rejected on some platforms (I'll run a CI to check).

@richardlau
Copy link
Member

richardlau commented Sep 26, 2025

--as-needed doesn't work with the linker used on AIX and SmartOS and possibly other linkers: https://ci.nodejs.org/job/node-test-commit/82944/

I've opted for a simpler test modification: nodejs/node#60027
Update This might be a bug in process.execve, which is documented to default the env to process.env so we should not have to set it. Investigating...

@richardlau
Copy link
Member

Update This might be a bug in process.execve, which is documented to default the env to process.env so we should not have to set it. Investigating...

Yes, it's a bug. Opened nodejs/node#60029

nodejs-github-bot pushed a commit to nodejs/node that referenced this pull request Oct 1, 2025
The `env` parameter for `process.execve` is documented to default
to `process.env`.

PR-URL: #60029
Refs: nodejs/build#4156
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@targos
Copy link
Member Author

targos commented Oct 1, 2025

Rebuild with all configurations: https://ci.nodejs.org/job/targos-node-test-commit-linux-containered/4/

@targos
Copy link
Member Author

targos commented Oct 1, 2025

All green!

@targos targos merged commit dec8e7d into nodejs:main Oct 1, 2025
1 check passed
@targos targos deleted the ubuntu2404-docker branch October 1, 2025 18:44
targos pushed a commit to nodejs/node that referenced this pull request Oct 6, 2025
The `env` parameter for `process.execve` is documented to default
to `process.env`.

PR-URL: #60029
Refs: nodejs/build#4156
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
aduh95 pushed a commit to nodejs/node that referenced this pull request Oct 22, 2025
The `env` parameter for `process.execve` is documented to default
to `process.env`.

PR-URL: #60029
Refs: nodejs/build#4156
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants