Re: [PATCH] fixup??? Merge branch 'ab/ci-updates' into seen
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-11-22 16:11:50
On Mon, Nov 22 2021, Johannes Schindelin via GitGitGadget wrote:
quoted hunk ↗ jump to hunk
From: Johannes Schindelin <redacted> The merge of the `ab/ci-updates` patches needs this fix-up to avoid breaking the Scalar tests. Signed-off-by: Johannes Schindelin <redacted> --- fixup??? Merge branch 'ab/ci-updates' into seen This fixes the CI breakage introduced with ab/ci-updates. It is based on the current version of seen: 89513b853be (Merge branch 'en/keep-cwd' into seen, 2021-11-21). Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1081%2Fdscho%2Ffix-ci-updates-merge-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1081/dscho/fix-ci-updates-merge-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/1081 ci/run-build-and-tests.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh index c0bae709b3b..c508c18ad44 100755 --- a/ci/run-build-and-tests.sh +++ b/ci/run-build-and-tests.sh@@ -45,9 +45,8 @@ linux-gcc-4.8) export MAKE_TARGETS=all ;; esac -make -C contrib/scalar test - make $MAKE_TARGETS +make -C contrib/scalar test check_unignored_build_artifacts
The CI breakage was introduced with the merger with ab/ci-updates, but
the combination of the two just reveals an existing breakage in
js/scalar.
Which is that on js/scalar this won't work:
make clean
make -C contrib/scalar test
Since its dependency graph is broken. It tries to run before "make all"
has been run, but fails.
It would be one thing if it just refused to run because the sources
haven't been compiled, but instead it tries in a way that suggests
that's meant to work, but it doesn't work.
I didn't spot this myself because I was testing on my locally patched
version of the scalar topic which doesn't have this breakage, which is a
patch you haven't reviwed/looked at/replied to yet, as noted in [1].
But there's also an existing breakage in js/scalar and this fix-up,
which is that yes, CI passes with this change. But what are meant to be
compile-only CI targets which don't run "make test" are now running no
tests ... except for this one scalar test.
Fixing that issue both in js/scalar & in this fix-up would be rather
easy, but it's all just a work-arond for the discrepancy that "make
test" isn't running the scalar tests by default.
If we fixed that more general problem this fix-up wouldn't be needed.
But at this point I'm just starting to repeat points I've made in
patches[2] I've sent you on top of js/scalar, and which you seem to be
ignoring.
1. https://lore.kernel.org/git/211118.86zgq14jp1.gmgdl@evledraar.gmail.com/ (local)
2. https://lore.kernel.org/git/patch-1.1-86fb8d56307-20211028T185016Z-avarab@gmail.com/ (local)