Re: [PATCH] ci(linux32): make Javascript Actions work in x86 mode

[Jeff King <peff@peff.net>]

On Tue, Sep 17, 2024 at 02:20:41PM +0200, Johannes Schindelin wrote:

> Installing lib64stdc++ indeed does not rely on the implementation detail
> that `/__e/node20/` contains the Node version used to execute the Actions
> in those Docker containers.
> 
> Of course, the fact that installing lib64stdc++ (and no other 64-bit
> library) "fixes" the 64-bit Node version is also an implementation detail.

Yes, though "provide a 64-bit c/c++ runtime" does not seem all that
exotic. I'd be much more worried about the implicit library version
dependencies that exist, but that is true even on 64-bit systems. As
shown in the issues I linked earlier, lots of people are hitting a
similar problem with containers that have an older glibc.

The real solution there (IMHO) is a statically linked node in the runner
images, but I'm not sure of the reasons they haven't pursued that yet.

In the absence of that, the fact that your solution uses a node build
which (for reasons I'm still not sure I understand) seems to be OK with
an older glibc feels like a compelling reason. In fact, I find that much
more compelling than the risk of 32/64-bit confusion. I think part of
what I was responding to was the focus on that in your commit message.

> In particular given that mapping the "externals" by any other name than
> `/__e/` risks breaking existing GitHub workflows that might make use of
> exactly that directory name, I consider the chances for that name change
> to be negligible. It probably won't change, ever.

Fair enough. Going into this whole problem, I was not clear where /__e/
was coming from. I had thought at first it was something being carried
along by the Actions themselves (and thus action-specific and likely to
change). But it looks like it is part of the runner image and just
mounted into the container volume, so it is a magic keyword that Actions
and runner images both have to depend on.

> Of course, my favorite solution would be for `actions/runner` to be fixed
> so that it detects the situation and uses a 32-bit variant in that case
> [*1*].

Yes, me too (and preferably statically linked :) ).

> And yes, the idea of mixing 32-bit and 64-bit things in a container that
> was specifically used to only have 32-bit things still does not convince
> me, it still looks like a much better idea to either stick with a
> 32-bit-only container, or to just do away with the complexity of a
> container altogether if the environment does not need to be free of 64-bit
> anyway (but why did we bother with that in the first place, then?).

I think 32-bit builds directly inside a 64-bit runner image is a good
way to do the thing you were initially worried about: accidentally using
tools of the wrong type. Doing the whole build and test within the
32-bit container is something we'd want to keep.

The other alternative, which neither of us shown patches for, but which
I mentioned (courtesy of Ed) in the original thread is: do the Actions
outside the 32-bit container, run docker ourselves mounting the repo,
and then build and test inside the container. That's apparently how
libgit2 does it. It sidesteps the issue entirely, as the container never
runs anything external. And it would be applicable to all projects, no
matter what's in their containers (32-bit, old distros, even qemu of
alternate architectures).

I'm not sure how much work it would be to do, though.

Anyway, it sounds like Junio has merged my patches down to get things
moving again. I'm OK if you want to rebase your proposed fix on top of
that.

-Peff