{"thread":{"id":"62107","subject":"[PATCH] ci(linux32): make Javascript Actions work in x86 mode","startedAt":"2024-09-14T00:42:43Z","lastAt":"2024-09-19T07:11:53Z","messageCount":8,"participants":["Johannes Schindelin via GitGitGadget","Jeff King","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"502785","messageId":"pull.1790.git.1726274559928.gitgitgadget@gmail.com","threadId":"62107","inReplyTo":null,"subject":"[PATCH] ci(linux32): make Javascript Actions work in x86 mode","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-09-14T00:42:39Z","receivedAt":"2024-09-14T00:42:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nIn February 2023, older `actions/upload-artifact` were deprecated:\nhttps://github.blog/changelog/2024-02-13-deprecation-notice-v1-and-v2-of-the-artifact-actions/\nThis was recently followed by brown-outs.\n\nHowever, the `linux32` job relied on those, as there are well-documented\nproblems (see https://github.com/actions/runner/issues/2115 for example)\nrunning modern, Javascript-based Actions in 32-bit only containers.\n\nTo get the CI builds to work again, a work-around was implemented in\nhttps://lore.kernel.org/git/20240912094841.GD589828@coredump.intra.peff.net\nto let the 32-bit container make use of the 64-bit node 20 provided by\nthe Actions runner.\n\nThis, however, runs the risk of using 64-bit executables when we\npurposefully chose a Docker image that only contains 32-bit bits and\npieces so that accidental use of 64-bit libraries or executables would\nnot happen.\n\nLet's go about this the other way round instead, by overriding the amd64\nversion of node 20 the Actions runner provides with an x86 one (which is\n\"officially unofficial\" by virtue of being hosted on\nunofficial-builds.nodejs.org).\n\nThis allows us to stop using the now-deprecated versions of\n`actions/checkout` and `actions/upload-artifact` before these Actions\nbecame Javascript-based Actions.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n    ci(linux32): do make Javascript Actions work\n    \n    I propose this alternative to 9c261856c91 (ci: use regular action\n    versions for linux32 job, 2024-09-12), keeping the updates to Javascript\n    Actions.\n    \n    The benefit is that it keeps the 32-bit container used in the linux32\n    job clean of any 64-bit libraries so that we don't accidentally end up\n    testing 64-bit stuff without wanting it.\n    \n    For good measure, I also reported this problem with that deprecation at\n    https://github.com/actions/upload-artifact/issues/616 even though I know\n    that the GitHub Actions team saw a headcount-losing reorg recently and\n    therefore I do not really expect that they have any bandwidth to help\n    with this. So this work-around is the best we can do for now, I believe.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1790%2Fdscho%2Fuse-x86-node-in-linux32-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1790/dscho/use-x86-node-in-linux32-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1790\n\n .github/workflows/main.yml | 18 +++++++++++++-----\n 1 file changed, 13 insertions(+), 5 deletions(-)\n\ndiff --git a/.github/workflows/main.yml b/.github/workflows/main.yml\nindex 97f9b063109..25a5f5f0e29 100644\n--- a/.github/workflows/main.yml\n+++ b/.github/workflows/main.yml\n@@ -348,19 +348,27 @@ jobs:\n       jobname: ${{matrix.vector.jobname}}\n       distro: ${{matrix.vector.distro}}\n     runs-on: ubuntu-latest\n-    container: ${{matrix.vector.image}}\n+    container:\n+      image: ${{matrix.vector.image}}\n+      volumes:\n+        # override /__e/node20 on 32-bit because it is 64-bit\n+        - /tmp:/__e${{matrix.vector.jobname != 'linux32' && '-x86' || ''}}/node20\n     steps:\n-    - uses: actions/checkout@v4\n-      if: matrix.vector.jobname != 'linux32'\n-    - uses: actions/checkout@v1 # cannot be upgraded because Node.js Actions aren't supported in this container\n+    - name: prepare x86 variant of node20\n       if: matrix.vector.jobname == 'linux32'\n+      run: |\n+        apt -q update && apt -q -y install curl &&\n+        NODE_URL=https://unofficial-builds.nodejs.org/download/release/v20.17.0/node-v20.17.0-linux-x86.tar.gz &&\n+        curl -Lo /tmp/node.tar.gz $NODE_URL &&\n+        tar -C /__e/node20 -x --strip-components=1 -f /tmp/node.tar.gz\n+    - uses: actions/checkout@v4\n     - run: ci/install-dependencies.sh\n     - run: ci/run-build-and-tests.sh\n     - name: print test failures\n       if: failure() && env.FAILED_TEST_ARTIFACTS != ''\n       run: ci/print-test-failures.sh\n     - name: Upload failed tests' directories\n-      if: failure() && env.FAILED_TEST_ARTIFACTS != '' && matrix.vector.jobname != 'linux32'\n+      if: failure() && env.FAILED_TEST_ARTIFACTS != ''\n       uses: actions/upload-artifact@v4\n       with:\n         name: failed-tests-${{matrix.vector.jobname}}\n\nbase-commit: 87dc3914693da5febad427161fc6bdfeed3426c7\n-- \ngitgitgadget\n"},{"id":"502787","messageId":"20240914072932.GB1284567@coredump.intra.peff.net","threadId":"62107","inReplyTo":"pull.1790.git.1726274559928.gitgitgadget@gmail.com","subject":"Re: [PATCH] ci(linux32): make Javascript Actions work in x86 mode","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-09-14T07:29:32Z","receivedAt":"2024-09-14T07:29:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Sep 14, 2024 at 12:42:39AM +0000, Johannes Schindelin via GitGitGadget wrote:\n\n> To get the CI builds to work again, a work-around was implemented in\n> https://lore.kernel.org/git/20240912094841.GD589828@coredump.intra.peff.net\n> to let the 32-bit container make use of the 64-bit node 20 provided by\n> the Actions runner.\n> \n> This, however, runs the risk of using 64-bit executables when we\n> purposefully chose a Docker image that only contains 32-bit bits and\n> pieces so that accidental use of 64-bit libraries or executables would\n> not happen.\n\nHow big a risk is this? In my experience with multiarch systems, the\ndifficulty is the opposite: convincing the non-base toolchain to work at\nall, rather than using it accidentally. Especially as the solution in my\npatch is not configuring apt for multiarch support, where you could\naccidentally install gcc:amd64 versus gcc:i386. Even if we were to\naccidentally bring in the cross-platform compiler via the\ngcc-10-x86-64-linux-gnu package, you'd have to point $(CC) at it\nexplicitly.\n\nBut maybe others have had a different experience?\n\n> Let's go about this the other way round instead, by overriding the amd64\n> version of node 20 the Actions runner provides with an x86 one (which is\n> \"officially unofficial\" by virtue of being hosted on\n> unofficial-builds.nodejs.org).\n\nI'm not totally opposed to this direction, but I'm a little concerned\nabout the stability/maintenance of the solution. In particular:\n\n> +        NODE_URL=https://unofficial-builds.nodejs.org/download/release/v20.17.0/node-v20.17.0-linux-x86.tar.gz &&\n\nWill this URL work forever? Looking at the release/ directory, it looks\nlike it should hang around. They have entries going back to 2019 (which\nis not all that old, but I suspect that's when they started the build\nrepository).\n\nThe flip side is: will node20 be sufficient for Actions forever? Node16\nwas already deprecated in Actions in 2023, and it was released in 2021\n(looks like Node releases have a 2-year lifespan in general). So node20\ntakes us to April 2026 or so.\n\nOf course my solution has similar problems. Probably node24 or whatever\ncomes next will need another glibc bump.\n\nI was mildly surprised that the build you reference didn't run into that\nproblem, actually. But I double-checked and it appears to run OK in\nXenial; it needs 2.17, according to some \"objdump -T\" sleuthing. I'm not\nsure why that's so different from the official 64-bit builds.\n\n> +        curl -Lo /tmp/node.tar.gz $NODE_URL &&\n> +        tar -C /__e/node20 -x --strip-components=1 -f /tmp/node.tar.gz\n\nThis is pretty intimate with how Actions work (both the node20 version\nand the \"/__e/\" magic). It's hard to say if/when that would bite us.\n\n>     For good measure, I also reported this problem with that deprecation at\n>     https://github.com/actions/upload-artifact/issues/616 even though I know\n>     that the GitHub Actions team saw a headcount-losing reorg recently and\n>     therefore I do not really expect that they have any bandwidth to help\n>     with this. So this work-around is the best we can do for now, I believe.\n\nI think it's reasonably well known there already. Here are some relevant\nissues I came across:\n\n  - https://github.com/actions/checkout/issues/334\n  - https://github.com/actions/checkout/issues/1590\n  - https://github.com/actions/setup-node/issues/922\n  - https://github.com/actions/runner/issues/2906\n\nMost folks are hitting the glibc issue rather than the i386 one, but the\ncore of the problem is the same.\n\nIMHO the ultimate solution is a statically-linked node binary (you'd\nstill be relying on the kernel, but that has a very good track record of\nuserspace compatibility).\n\n-Peff\n"},{"id":"502792","messageId":"xmqq34m2tasj.fsf@gitster.g","threadId":"62107","inReplyTo":"20240914072932.GB1284567@coredump.intra.peff.net","subject":"Re: [PATCH] ci(linux32): make Javascript Actions work in x86 mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-09-14T17:17:16Z","receivedAt":"2024-09-14T17:17:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I'm not totally opposed to this direction, but I'm a little concerned\n> about the stability/maintenance of the solution. In particular:\n>\n>> +        NODE_URL=https://unofficial-builds.nodejs.org/download/release/v20.17.0/node-v20.17.0-linux-x86.tar.gz &&\n> ...\n>> +        curl -Lo /tmp/node.tar.gz $NODE_URL &&\n>> +        tar -C /__e/node20 -x --strip-components=1 -f /tmp/node.tar.gz\n>\n> This is pretty intimate with how Actions work (both the node20 version\n> and the \"/__e/\" magic). It's hard to say if/when that would bite us.\n\nThanks for clearly expressing the uneasiness I felt, which I could\nnot turn into concrete words, when I saw the patch first time.\n\nEach of these approaches may have its pros and cons, but I somehow\ndo not see that the newly proposed alternative is 10x better than\nwhat was reviewed and queued already to be worth the effort to\nreplace it.\n\nThanks, both.\n\n"},{"id":"502800","messageId":"20240915110706.GA2017642@coredump.intra.peff.net","threadId":"62107","inReplyTo":"xmqq34m2tasj.fsf@gitster.g","subject":"Re: [PATCH] ci(linux32): make Javascript Actions work in x86 mode","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-09-15T11:07:06Z","receivedAt":"2024-09-15T11:07:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Sep 14, 2024 at 10:17:16AM -0700, Junio C Hamano wrote:\n\n> Each of these approaches may have its pros and cons, but I somehow\n> do not see that the newly proposed alternative is 10x better than\n> what was reviewed and queued already to be worth the effort to\n> replace it.\n\nThat's my feeling, too, but I'd reserve final judgement to see Dscho's\nresponse; it's possible I am under-estimating the 32/64-bit confusion\nrisk.\n\nI'd also note that his patch does not require bumping the distro\nversion, which would let us continue testing that old version in GitHub\nActions. That might be worth considering.\n\n-Peff\n"},{"id":"502812","messageId":"xmqqr09krijc.fsf@gitster.g","threadId":"62107","inReplyTo":"20240915110706.GA2017642@coredump.intra.peff.net","subject":"Re: [PATCH] ci(linux32): make Javascript Actions work in x86 mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-09-15T16:25:11Z","receivedAt":"2024-09-15T16:25:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Sat, Sep 14, 2024 at 10:17:16AM -0700, Junio C Hamano wrote:\n>\n>> Each of these approaches may have its pros and cons, but I somehow\n>> do not see that the newly proposed alternative is 10x better than\n>> what was reviewed and queued already to be worth the effort to\n>> replace it.\n>\n> That's my feeling, too, but I'd reserve final judgement to see Dscho's\n> response; it's possible I am under-estimating the 32/64-bit confusion\n> risk.\n\nFWIW, what you said matches my recollection from years ago ;-) back\nwhen I had to deal with that.\n\n> I'd also note that his patch does not require bumping the distro\n> version, which would let us continue testing that old version in GitHub\n> Actions. That might be worth considering.\n\nYes, that is true.\n\nConsidering that 16.04 has passed its expiration date for standard\nsupport a few years ago, I am not sure how many more years of\npractical/unsupported use and testing we would be getting by giving\ncycles for the release in CI, though.\n\nThanks.\n"},{"id":"502950","messageId":"d8b15f7e-7847-f6ff-cf8f-02aee254b070@gmx.de","threadId":"62107","inReplyTo":"xmqqr09krijc.fsf@gitster.g","subject":"Re: [PATCH] ci(linux32): make Javascript Actions work in x86 mode","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2024-09-17T12:20:41Z","receivedAt":"2024-09-17T12:20:54Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 15 Sep 2024, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n>\n> > On Sat, Sep 14, 2024 at 10:17:16AM -0700, Junio C Hamano wrote:\n> >\n> >> Each of these approaches may have its pros and cons, but I somehow\n> >> do not see that the newly proposed alternative is 10x better than\n> >> what was reviewed and queued already to be worth the effort to\n> >> replace it.\n\nInstalling lib64stdc++ indeed does not rely on the implementation detail\nthat `/__e/node20/` contains the Node version used to execute the Actions\nin those Docker containers.\n\nOf course, the fact that installing lib64stdc++ (and no other 64-bit\nlibrary) \"fixes\" the 64-bit Node version is also an implementation detail.\n\nBetween GitHub Actions' and Node's development speed, I personally would\nexpect the latter implementation detail to be changed many times over\nbefore the former implementation detail would be changed.\n\nIn particular given that mapping the \"externals\" by any other name than\n`/__e/` risks breaking existing GitHub workflows that might make use of\nexactly that directory name, I consider the chances for that name change\nto be negligible. It probably won't change, ever.\n\nOf course, my favorite solution would be for `actions/runner` to be fixed\nso that it detects the situation and uses a 32-bit variant in that case\n[*1*].\n\nBusiness forces work against this wish, though, so I don't see this\nhappening.\n\nThe next best thing, in my mind, is to come up with a solution that is\ngeneral enough that other projects could follow this example, which is\nwhat I did.\n\nAnd yes, the idea of mixing 32-bit and 64-bit things in a container that\nwas specifically used to only have 32-bit things still does not convince\nme, it still looks like a much better idea to either stick with a\n32-bit-only container, or to just do away with the complexity of a\ncontainer altogether if the environment does not need to be free of 64-bit\nanyway (but why did we bother with that in the first place, then?).\n\nCiao,\nJohannes\n\nFootnote *1*: I have added quite a few details about this here:\nhttps://github.com/actions/upload-artifact/issues/616#issuecomment-2350667347\n"},{"id":"502956","messageId":"xmqqikuul3xz.fsf@gitster.g","threadId":"62107","inReplyTo":"d8b15f7e-7847-f6ff-cf8f-02aee254b070@gmx.de","subject":"Re: [PATCH] ci(linux32): make Javascript Actions work in x86 mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-09-17T15:01:28Z","receivedAt":"2024-09-17T15:01:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Of course, my favorite solution would be for `actions/runner` to be fixed\n> so that it detects the situation and uses a 32-bit variant in that case\n> [*1*].\n\nYeah, that would be ideal.\n\n> The next best thing, in my mind, is to come up with a solution that is\n> general enough that other projects could follow this example, which is\n> what I did.\n\nI guess both patterns can be followed if they discover either, and\nit made me wonder if this isn't a problem already solved by others,\nbut at the end of the day, any solution that was good enough and\nunblocks us quickly was needed; the ones that were reviewed earlier\nhave been fast tracked down to 'maint' as of last night.  That does\nnot mean we won't have to further improve it, of course.  At least\nuntil 32-bit archs no longer matter, that is ;-).\n\nThanks.\n\n"},{"id":"503049","messageId":"20240919070415.GA18979@localhost","threadId":"62107","inReplyTo":"d8b15f7e-7847-f6ff-cf8f-02aee254b070@gmx.de","subject":"Re: [PATCH] ci(linux32): make Javascript Actions work in x86 mode","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-09-19T07:05:08Z","receivedAt":"2024-09-19T07:11:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 17, 2024 at 02:20:41PM +0200, Johannes Schindelin wrote:\n\n> Installing lib64stdc++ indeed does not rely on the implementation detail\n> that `/__e/node20/` contains the Node version used to execute the Actions\n> in those Docker containers.\n> \n> Of course, the fact that installing lib64stdc++ (and no other 64-bit\n> library) \"fixes\" the 64-bit Node version is also an implementation detail.\n\nYes, though \"provide a 64-bit c/c++ runtime\" does not seem all that\nexotic. I'd be much more worried about the implicit library version\ndependencies that exist, but that is true even on 64-bit systems. As\nshown in the issues I linked earlier, lots of people are hitting a\nsimilar problem with containers that have an older glibc.\n\nThe real solution there (IMHO) is a statically linked node in the runner\nimages, but I'm not sure of the reasons they haven't pursued that yet.\n\nIn the absence of that, the fact that your solution uses a node build\nwhich (for reasons I'm still not sure I understand) seems to be OK with\nan older glibc feels like a compelling reason. In fact, I find that much\nmore compelling than the risk of 32/64-bit confusion. I think part of\nwhat I was responding to was the focus on that in your commit message.\n\n> In particular given that mapping the \"externals\" by any other name than\n> `/__e/` risks breaking existing GitHub workflows that might make use of\n> exactly that directory name, I consider the chances for that name change\n> to be negligible. It probably won't change, ever.\n\nFair enough. Going into this whole problem, I was not clear where /__e/\nwas coming from. I had thought at first it was something being carried\nalong by the Actions themselves (and thus action-specific and likely to\nchange). But it looks like it is part of the runner image and just\nmounted into the container volume, so it is a magic keyword that Actions\nand runner images both have to depend on.\n\n> Of course, my favorite solution would be for `actions/runner` to be fixed\n> so that it detects the situation and uses a 32-bit variant in that case\n> [*1*].\n\nYes, me too (and preferably statically linked :) ).\n\n> And yes, the idea of mixing 32-bit and 64-bit things in a container that\n> was specifically used to only have 32-bit things still does not convince\n> me, it still looks like a much better idea to either stick with a\n> 32-bit-only container, or to just do away with the complexity of a\n> container altogether if the environment does not need to be free of 64-bit\n> anyway (but why did we bother with that in the first place, then?).\n\nI think 32-bit builds directly inside a 64-bit runner image is a good\nway to do the thing you were initially worried about: accidentally using\ntools of the wrong type. Doing the whole build and test within the\n32-bit container is something we'd want to keep.\n\nThe other alternative, which neither of us shown patches for, but which\nI mentioned (courtesy of Ed) in the original thread is: do the Actions\noutside the 32-bit container, run docker ourselves mounting the repo,\nand then build and test inside the container. That's apparently how\nlibgit2 does it. It sidesteps the issue entirely, as the container never\nruns anything external. And it would be applicable to all projects, no\nmatter what's in their containers (32-bit, old distros, even qemu of\nalternate architectures).\n\nI'm not sure how much work it would be to do, though.\n\nAnyway, it sounds like Junio has merged my patches down to get things\nmoving again. I'm OK if you want to rebase your proposed fix on top of\nthat.\n\n-Peff\n"}]}