From: Junio C Hamano <gitster@pobox.com>
To: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH] ci(dockerized): do show the result of failing tests again
Date: Mon, 17 Nov 2025 10:28:17 -0800 [thread overview]
Message-ID: <xmqqpl9gike6.fsf@gitster.g> (raw)
In-Reply-To: <pull.2003.git.1763399064983.gitgitgadget@gmail.com> (Johannes Schindelin via GitGitGadget's message of "Mon, 17 Nov 2025 17:04:24 +0000")
"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:
> From: Johannes Schindelin <johannes.schindelin@gmx.de>
>
> The quality of tests/test suites does not show as much when there are no
> breakages as in the amount of time required after bugs trigger test
> failures before the bugs can be identified, analyzed and resolved.
>
> As such, it is an unfortunate side effect of 2a21098b98a (github: adapt
> containerized jobs to be rootless, 2025-01-10) that the output of failed
> test cases, which was shown before that change directly in the build
> logs, is now no longer shown at all.
>
> The reason is a side effect of trying to run the build and the tests
> with permissions other than the `root` user, but without providing the
> prerequisite permissions to signal what tests failed and whose output
> hence needs to be included in the logs.
>
> The way this signaling works is for the workflow to write into
> special-purpose files whose path is specific to the current workflow
> step and which can be accessed via the `$GITHUB_ENV` environment
> variable, which differs between workflow steps. It is this file that is
> missing write permission for the `builder` user that was introduced in
> above-mentioned commit.
>
> The solution is simple: make the file world-writable.
I expected to see a+w not o+w from this statement; as long as it
works I have no strong objections, but if I saw o+w without the
above explanation I would probably have wondered who are in the
group that we do not want this file touched by.
> Technically, this write permission should be removed after the step has
> completed, if proper security practices were to be upheld, but since
> nothing uses that file again, it does not matter, and the fix is more
> succinct this way.
>
> This commit is best viewed with `--color-words`.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2003%2Fdscho%2Ffix-failure-reporting-in-dockerized-ci-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2003/dscho/fix-failure-reporting-in-dockerized-ci-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/2003
>
> .github/workflows/main.yml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> index 816d5a34c4..ca7cc2984f 100644
> --- a/.github/workflows/main.yml
> +++ b/.github/workflows/main.yml
> @@ -433,7 +433,7 @@ jobs:
> - run: ci/install-dependencies.sh
> - run: useradd builder --create-home
> - run: chown -R builder .
> - - run: sudo --preserve-env --set-home --user=builder ci/run-build-and-tests.sh
> + - run: chmod o+w $GITHUB_ENV && sudo --preserve-env --set-home --user=builder ci/run-build-and-tests.sh
> - name: print test failures
> if: failure() && env.FAILED_TEST_ARTIFACTS != ''
> run: sudo --preserve-env --set-home --user=builder ci/print-test-failures.sh
Thanks. Will apply.
next prev parent reply other threads:[~2025-11-17 18:28 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-17 17:04 [PATCH] ci(dockerized): do show the result of failing tests again Johannes Schindelin via GitGitGadget
2025-11-17 18:28 ` Junio C Hamano [this message]
2025-11-23 2:41 ` Junio C Hamano
2025-11-18 9:36 ` Jeff King
2025-11-25 6:15 ` Elijah Newren
2025-11-25 14:32 ` Junio C Hamano
2025-11-25 17:40 ` Johannes Schindelin
2025-11-25 19:27 ` Junio C Hamano
2025-11-29 18:31 ` Johannes Schindelin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqpl9gike6.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=johannes.schindelin@gmx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).