{"thread":{"id":"56955","subject":"[PATCH] fixup??? Merge branch 'ab/ci-updates' into seen","startedAt":"2021-11-22T11:02:14Z","lastAt":"2021-11-23T17:58:57Z","messageCount":7,"participants":["Johannes Schindelin via GitGitGadget","Ævar Arnfjörð Bjarmason","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"441925","messageId":"pull.1081.git.1637578930464.gitgitgadget@gmail.com","threadId":"56955","inReplyTo":null,"subject":"[PATCH] fixup??? Merge branch 'ab/ci-updates' into seen","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-11-22T11:02:10Z","receivedAt":"2021-11-22T11:02:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin \n\nThe merge of the `ab/ci-updates` patches needs this fix-up to avoid\nbreaking the Scalar tests.\n\nSigned-off-by: Johannes Schindelin \n---\n fixup??? Merge branch 'ab/ci-updates' into seen\n \n This fixes the CI breakage introduced with ab/ci-updates. It is based on\n the current version of seen: 89513b853be (Merge branch 'en/keep-cwd'\n into seen, 2021-11-21).\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1081%2Fdscho%2Ffix-ci-updates-merge-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1081/dscho/fix-ci-updates-merge-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1081\n\n ci/run-build-and-tests.sh | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\nindex c0bae709b3b..c508c18ad44 100755\n--- a/ci/run-build-and-tests.sh\n+++ b/ci/run-build-and-tests.sh\n@@ -45,9 +45,8 @@ linux-gcc-4.8)\n \texport MAKE_TARGETS=all\n \t;;\n esac\n-make -C contrib/scalar test\n-\n make $MAKE_TARGETS\n+make -C contrib/scalar test\n \n check_unignored_build_artifacts\n \n\nbase-commit: 89513b853be400c439e19e479ed14e9e8de44722\n-- \ngitgitgadget\n"},{"id":"441963","messageId":"211122.86ee78yxts.gmgdl@evledraar.gmail.com","threadId":"56955","inReplyTo":"pull.1081.git.1637578930464.gitgitgadget@gmail.com","subject":"Re: [PATCH] fixup??? Merge branch 'ab/ci-updates' into seen","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-22T15:57:39Z","receivedAt":"2021-11-22T16:11:50Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Nov 22 2021, Johannes Schindelin via GitGitGadget wrote:\n\n> From: Johannes Schindelin \n>\n> The merge of the `ab/ci-updates` patches needs this fix-up to avoid\n> breaking the Scalar tests.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> fixup??? Merge branch 'ab/ci-updates' into seen\n> \n> This fixes the CI breakage introduced with ab/ci-updates. It is based on\n> the current version of seen: 89513b853be (Merge branch 'en/keep-cwd'\n> into seen, 2021-11-21).\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1081%2Fdscho%2Ffix-ci-updates-merge-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1081/dscho/fix-ci-updates-merge-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1081\n>\n> ci/run-build-and-tests.sh | 3 +--\n> 1 file changed, 1 insertion(+), 2 deletions(-)\n>\n> diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\n> index c0bae709b3b..c508c18ad44 100755\n> --- a/ci/run-build-and-tests.sh\n> +++ b/ci/run-build-and-tests.sh\n> @@ -45,9 +45,8 @@ linux-gcc-4.8)\n> \texport MAKE_TARGETS=all\n> \t;;\n> esac\n> -make -C contrib/scalar test\n> -\n> make $MAKE_TARGETS\n> +make -C contrib/scalar test\n> \n> check_unignored_build_artifacts\n\nThe CI breakage was introduced with the merger with ab/ci-updates, but\nthe combination of the two just reveals an existing breakage in\njs/scalar.\n\nWhich is that on js/scalar this won't work:\n\n make clean\n make -C contrib/scalar test\n\nSince its dependency graph is broken. It tries to run before \"make all\"\nhas been run, but fails.\n\nIt would be one thing if it just refused to run because the sources\nhaven't been compiled, but instead it tries in a way that suggests\nthat's meant to work, but it doesn't work.\n\nI didn't spot this myself because I was testing on my locally patched\nversion of the scalar topic which doesn't have this breakage, which is a\npatch you haven't reviwed/looked at/replied to yet, as noted in [1].\n\nBut there's also an existing breakage in js/scalar and this fix-up,\nwhich is that yes, CI passes with this change. But what are meant to be\ncompile-only CI targets which don't run \"make test\" are now running no\ntests ... except for this one scalar test.\n\nFixing that issue both in js/scalar & in this fix-up would be rather\neasy, but it's all just a work-arond for the discrepancy that \"make\ntest\" isn't running the scalar tests by default.\n\nIf we fixed that more general problem this fix-up wouldn't be needed.\n\nBut at this point I'm just starting to repeat points I've made in\npatches[2] I've sent you on top of js/scalar, and which you seem to be\nignoring.\n\n1. https://lore.kernel.org/git/211118.86zgq14jp1.gmgdl@evledraar.gmail.com/\n2. https://lore.kernel.org/git/patch-1.1-86fb8d56307-20211028T185016Z-avarab@gmail.com/\n"},{"id":"442029","messageId":"nycvar.QRO.7.76.6.2111222257430.63@tvgsbejvaqbjf.bet","threadId":"56955","inReplyTo":"211122.86ee78yxts.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] fixup??? Merge branch 'ab/ci-updates' into seen","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-11-22T21:58:59Z","receivedAt":"2021-11-22T21:59:16Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ævar,\n\nOn Mon, 22 Nov 2021, Ævar Arnfjörð Bjarmason wrote:\n\n> On Mon, Nov 22 2021, Johannes Schindelin via GitGitGadget wrote:\n>\n> > diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\n> > index c0bae709b3b..c508c18ad44 100755\n> > --- a/ci/run-build-and-tests.sh\n> > +++ b/ci/run-build-and-tests.sh\n> > @@ -45,9 +45,8 @@ linux-gcc-4.8)\n> > \texport MAKE_TARGETS=all\n> > \t;;\n> > esac\n> > -make -C contrib/scalar test\n> > -\n> > make $MAKE_TARGETS\n> > +make -C contrib/scalar test\n> >\n> > check_unignored_build_artifacts\n>\n> The CI breakage was introduced with the merger with ab/ci-updates, but\n> the combination of the two just reveals an existing breakage in\n> js/scalar.\n\nWhich shows that I was wrong to pander to your repeated demand to include\nScalar in the CI builds already at this early stage.\n\nWill fix,\nDscho\n"},{"id":"442063","messageId":"211123.8635nnydmm.gmgdl@evledraar.gmail.com","threadId":"56955","inReplyTo":"nycvar.QRO.7.76.6.2111222257430.63@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] fixup??? Merge branch 'ab/ci-updates' into seen","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-22T23:12:24Z","receivedAt":"2021-11-22T23:28:05Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Nov 22 2021, Johannes Schindelin wrote:\n\n> Hi Ævar,\n>\n> On Mon, 22 Nov 2021, Ævar Arnfjörð Bjarmason wrote:\n>\n>> On Mon, Nov 22 2021, Johannes Schindelin via GitGitGadget wrote:\n>>\n>> > diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\n>> > index c0bae709b3b..c508c18ad44 100755\n>> > --- a/ci/run-build-and-tests.sh\n>> > +++ b/ci/run-build-and-tests.sh\n>> > @@ -45,9 +45,8 @@ linux-gcc-4.8)\n>> > \texport MAKE_TARGETS=all\n>> > \t;;\n>> > esac\n>> > -make -C contrib/scalar test\n>> > -\n>> > make $MAKE_TARGETS\n>> > +make -C contrib/scalar test\n>> >\n>> > check_unignored_build_artifacts\n>>\n>> The CI breakage was introduced with the merger with ab/ci-updates, but\n>> the combination of the two just reveals an existing breakage in\n>> js/scalar.\n>\n> Which shows that I was wrong to pander to your repeated demand to include\n> Scalar in the CI builds already at this early stage.\n\nUs finding an a bug in a topic that's happening outside of CI means we\nshouldn't have added it to CI in the first place? Isn't spotting these\nissues a good thing?\n\nWhat I'm pointing out is that this isn't an issue that happened because\nof the merger with ab/ci-updates, but merely turned into a CI failure\nbecause of it.\n\nBefore that in your initial patch to integrate it into CI[1] the\nrelevant part of the patch was, with extra context added by me:\n\n [...]\n linux-gcc-4.8|pedantic)\n # Don't run the tests; we only care about whether Git can be\n # built with GCC 4.8 or with pedantic\n ;;\n *)\n make test\n ;;\n esac\n make test\n ;;\n esac\n +make -C contrib/scalar test\n \n check_unignored_build_artifacts\n\nI.e. it added scalar testing to the linux-gcc-4.8 & \"pedantic\" jobs,\nwhich are meant to be compile-only.\n\nThe other issue is that the \"test\" Makefile target in\ncontrib/scalar/Makefile attempts to build the top-level from scratch,\nbut fails (which is how it turned into a CI failure). The same thing\nhappens when running it outside fo CI.\n\nI don't think I've been demanding that you do anything. I have been\nasking if there's a good reason for why we wouldn't test this code we've\ngot in-tree. Your commit[1] states:\n\n Since Scalar depends on `libgit.a`, it makes sense to ensure in the CI\n and the PR builds that it does not get broken in case of industrious\n refactorings of the core Git code.\n\nWhich is rationale that I entirely agree with, the only extent to which\nI don't is that I don't think it goes far enough, i.e. shouldn't we also\nadd this to \"make test\" and not just CI? Why shouldn't I see failures\nlocally, and only when I push to CI (unless I go out of my way to run\nthe tests-like-CI-would)?\n\nIn any case, both of these breakages are present in your version of the\nversion of the patches, but not the change I've been proposing on-top to\nadd it to CI and \"make test\". You've also refused to talk about why you\ninsist on that particular approach, which is shown to be more fragile\nhere. So it seems rather odd to blame my suggestions (or \"demands\") for\nthem.\n\n1. https://lore.kernel.org/git/1b0328fa236a35c2427b82f53c32944e513580d3.1637158762.git.gitgitgadget@gmail.com/\n\n"},{"id":"442129","messageId":"nycvar.QRO.7.76.6.2111231215020.63@tvgsbejvaqbjf.bet","threadId":"56955","inReplyTo":"211123.8635nnydmm.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] fixup??? Merge branch 'ab/ci-updates' into seen","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-11-23T12:02:48Z","receivedAt":"2021-11-23T12:02:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Ævar,\n\nOn Tue, 23 Nov 2021, Ævar Arnfjörð Bjarmason wrote:\n\n> On Mon, Nov 22 2021, Johannes Schindelin wrote:\n>\n> > On Mon, 22 Nov 2021, Ævar Arnfjörð Bjarmason wrote:\n> >\n> >> On Mon, Nov 22 2021, Johannes Schindelin via GitGitGadget wrote:\n> >>\n> >> > diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\n> >> > index c0bae709b3b..c508c18ad44 100755\n> >> > --- a/ci/run-build-and-tests.sh\n> >> > +++ b/ci/run-build-and-tests.sh\n> >> > @@ -45,9 +45,8 @@ linux-gcc-4.8)\n> >> > \texport MAKE_TARGETS=all\n> >> > \t;;\n> >> > esac\n> >> > -make -C contrib/scalar test\n> >> > -\n> >> > make $MAKE_TARGETS\n> >> > +make -C contrib/scalar test\n> >> >\n> >> > check_unignored_build_artifacts\n> >>\n> >> The CI breakage was introduced with the merger with ab/ci-updates, but\n> >> the combination of the two just reveals an existing breakage in\n> >> js/scalar.\n> >\n> > Which shows that I was wrong to pander to your repeated demand to include\n> > Scalar in the CI builds already at this early stage.\n>\n> Us finding an a bug in a topic that's happening outside of CI means we\n> shouldn't have added it to CI in the first place? Isn't spotting these\n> issues a good thing?\n\nLet's analyze \"these issues\".\n\nBefore your patch series, Scalar's `make -C contrib/scalar test` came\nafter the `make test` which ensured that Git was built. As designed.\n\nAfter merging your patch series, the `make test` was magically moved\n_after_ `make -C contrib/scalar test` (which is wrong for more reasons\nthan just that Git was not built yet).\n\nSo the \"issue\" is a simple mis-merge, and I provided a fix.\n\nCiao,\nJohannes\n\nP.S.: Of course, this could have been easily avoided by holding off\npatches that intentionally touch the very same code as other patch series\nthat are already in flight. This kind of conflict seems to happen more\noften than usual as of late. It happened with the FSMonitor patches and\nrepo-settings, with the hooks patches, the pager patch yesterday, etc.\n"},{"id":"442144","messageId":"211123.86ilwjujmd.gmgdl@evledraar.gmail.com","threadId":"56955","inReplyTo":"nycvar.QRO.7.76.6.2111231215020.63@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] fixup??? Merge branch 'ab/ci-updates' into seen","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-23T12:18:09Z","receivedAt":"2021-11-23T12:44:31Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Nov 23 2021, Johannes Schindelin wrote:\n\n> Hi Ævar,\n>\n> On Tue, 23 Nov 2021, Ævar Arnfjörð Bjarmason wrote:\n>\n>> On Mon, Nov 22 2021, Johannes Schindelin wrote:\n>>\n>> > On Mon, 22 Nov 2021, Ævar Arnfjörð Bjarmason wrote:\n>> >\n>> >> On Mon, Nov 22 2021, Johannes Schindelin via GitGitGadget wrote:\n>> >>\n>> >> > diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\n>> >> > index c0bae709b3b..c508c18ad44 100755\n>> >> > --- a/ci/run-build-and-tests.sh\n>> >> > +++ b/ci/run-build-and-tests.sh\n>> >> > @@ -45,9 +45,8 @@ linux-gcc-4.8)\n>> >> > \texport MAKE_TARGETS=all\n>> >> > \t;;\n>> >> > esac\n>> >> > -make -C contrib/scalar test\n>> >> > -\n>> >> > make $MAKE_TARGETS\n>> >> > +make -C contrib/scalar test\n>> >> >\n>> >> > check_unignored_build_artifacts\n>> >>\n>> >> The CI breakage was introduced with the merger with ab/ci-updates, but\n>> >> the combination of the two just reveals an existing breakage in\n>> >> js/scalar.\n>> >\n>> > Which shows that I was wrong to pander to your repeated demand to include\n>> > Scalar in the CI builds already at this early stage.\n>>\n>> Us finding an a bug in a topic that's happening outside of CI means we\n>> shouldn't have added it to CI in the first place? Isn't spotting these\n>> issues a good thing?\n>\n> Let's analyze \"these issues\".\n>\n> Before your patch series, Scalar's `make -C contrib/scalar test` came\n> after the `make test` which ensured that Git was built. As designed.\n>\n> After merging your patch series, the `make test` was magically moved\n> _after_ `make -C contrib/scalar test` (which is wrong for more reasons\n> than just that Git was not built yet).\n>\n> So the \"issue\" is a simple mis-merge, and I provided a fix.\n\nI'm pointing out that your patch to \"master\" has a logic error where you\nadded the scalar tests after that case/esac, but on \"master\" any new\n\"make new-test\" needs to be added thusly:\n\n diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\n index cc62616d806..37df8e2397a 100755\n --- a/ci/run-build-and-tests.sh\n +++ b/ci/run-build-and-tests.sh\n @@ -34,21 +34,28 @@ linux-gcc)\n export GIT_TEST_WRITE_REV_INDEX=1\n export GIT_TEST_CHECKOUT_WORKERS=2\n make test\n + make new-test\n ;;\n linux-clang)\n export GIT_TEST_DEFAULT_HASH=sha1\n make test\n + make new-test\n export GIT_TEST_DEFAULT_HASH=sha256\n make test\n + make new-test\n ;;\n linux-gcc-4.8|pedantic)\n # Don't run the tests; we only care about whether Git can be\n # built with GCC 4.8 or with pedantic\n + \"make new test does not go here\"\n ;;\n *)\n make test\n + make new-test\n ;;\n esac\n +\"and not here, as it would run under pedantic\"\n \n check_unignored_build_artifacts\n \nAs a result if you look at your own CI run's \"pedantic\" job at\nhttps://github.com/gitgitgadget/git/runs/4292915519?check_suite_focus=true\nyou'll see that it runs the scalar test, which is not the\nintention. That job should be compile-only job with the -pedantic flag.\n\nI also notice now that it doesn't behave the same way with the\nsha1/sha256 test. We do end up testing both, but it's not tested like\nthe rest (but in any case that job is split up in my CI topic).\n\nThe other issue is that even if you put a \"make test\" before a \"make\"\netc. it should still work, as \"test\" knows to depend on the compilation\nit needs for \"test\", but that's not the case with \"make -C\ncontrib/scalar test\".\n\nShouldn't it either know how to build git from scratch, or better yet\nbehave like \"make -C t\" which doesn't, and just punts out entirely? The\nin-between seems strange.\n\nIn any case, I see js/scalar got ejected out of \"seen\" with Junio's last\npush-out. I think that any re-roll based on \"master\" should be adding\nthe \"make new-test\" like the above diff. At that point we'll only have a\ntextual conflict, not a semantic one.\n\n> P.S.: Of course, this could have been easily avoided by holding off\n> patches that intentionally touch the very same code as other patch series\n> that are already in flight. This kind of conflict seems to happen more\n> often than usual as of late. It happened with the FSMonitor patches and\n> repo-settings, with the hooks patches, the pager patch yesterday, etc.\n\nSure, without going into the details of some of those (which I think\nyou've got the wrong impression of) it's also the responsibility of us\nto try to avoid these conflicts when possible.\n\nYou've picked an approach with js/scalar that'll require changing every\nplace where we invoke \"make test\" in CI to also invoke a \"make -C\ncontrib/scalar test\", instead of just adding it to \"make test\"\nitself. The latter is clearly possible, I've sent you a patch to do it\nthat way, and it works.\n\nSo why can't we discuss why that more invasive and textually conflicting\napproach is necessary? The scalar topic is clearly a much bigger bite\nfor git.git than any relatively recent and tiny ci/ topic of mine, and\nwill be longer in-flight. So anything that reduces such conflicts would\nbe a good thing.\n\nMy current diffstat on top of your \"vanilla\" topic showing no\nspecial-casing in .github/*, ci/* etc. is needed with that approach, and\nthus any potential for conflicts there doesn't exist anymore[1]:\n\n .github/workflows/main.yml | 15 -----\n .gitignore | 1 +\n Documentation/Makefile | 4 ++\n Documentation/cmd-list.perl | 2 +-\n Documentation/git.txt | 12 ++++\n {contrib/scalar => Documentation}/scalar.txt | 4 +-\n Makefile | 96 +++++++++++++++++++---------\n ci/run-build-and-tests.sh | 1 -\n ci/run-test-slice.sh | 5 --\n command-list.txt | 2 +\n contrib/scalar/.gitignore | 2 -\n contrib/scalar/Makefile | 45 -------------\n contrib/scalar/t/Makefile | 78 ----------------------\n contrib/scalar/scalar.c => scalar.c | 0\n {contrib/scalar/t => t}/t9099-scalar.sh | 42 +++++++-----\n 15 files changed, 115 insertions(+), 194 deletions(-)\n\n1. http://github.com/avar/git/commit/6df9fc2812d (commit message not\n entirely up-to-date)\n"},{"id":"442182","messageId":"xmqqo86a92jm.fsf@gitster.g","threadId":"56955","inReplyTo":"211123.86ilwjujmd.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] fixup??? Merge branch 'ab/ci-updates' into seen","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-23T17:58:53Z","receivedAt":"2021-11-23T17:58:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> I'm pointing out that your patch to \"master\" has a logic error where you\n> added the scalar tests after that case/esac, but on \"master\" any new\n> \"make new-test\" needs to be added thusly:\n>\n> diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\n> index cc62616d806..37df8e2397a 100755\n> --- a/ci/run-build-and-tests.sh\n> +++ b/ci/run-build-and-tests.sh\n> @@ -34,21 +34,28 @@ linux-gcc)\n> ...\n> *)\n> make test\n> + make new-test\n> ;;\n> esac\n> +\"and not here, as it would run under pedantic\"\n> \n> check_unignored_build_artifacts\n> \n> As a result if you look at your own CI run's \"pedantic\" job at\n> https://github.com/gitgitgadget/git/runs/4292915519?check_suite_focus=true\n> you'll see that it runs the scalar test, which is not the\n> intention. That job should be compile-only job with the -pedantic flag.\n\nI think the above shows that it is a bug in the topic itself, but\nthe presense of the ab/ci-updates topic makes the issue harder to\nsee. It makes the problem manifest in quite a different way. The\nband-aid we saw from Dscho does \"fix\" the manifestation after two\ntopics get merged (i.e. scalar build or test must follow the primary\nbuild and cannot be done by itself), without correcting the original\nbug (i.e. scalar test is done in a wrong CI job).\n\nAlso, when writing recipes for CI, if you know that scalar build or\ntest must be preceded by primary build, I wonder if it is with more\ngood manners to write\n\n\tmake test\n-\tmake -C scalar test\n+\tmake && make -C scalar test\n\nto make the dendency clear? In addition, it would be courteous to\nthe fellow developers to make the public entry points like \"all\" and\n\"test\" self contained. The Makefile of scalar knows as well as, or\nbetter than, the developers that going up to the top-level of the\nworking tree and running \"make\" is required before \"all\" target can\nbe built, so automating that would help everybody from the trouble,\nand the silly ugliness of \"make && make -C there\" I suggested above.\n\nI do not, for example, mind at all if something silly like this was\ndone in contrib/scalar/Makefile:\n\n all: ../../git\n ../../git:\n\t$(MAKE) -C ../..\n test: all\n\t...\n\nwhich is with clearly broken dependencies (e.g. if you edit\nrevision.c, scalar will not be rebuilt or the change would not\naffect scalar's tests), but for the purpose of \"letting CI build and\ntest from scratch to smoke out problems early\", it is good enough.\n\nPerhaps Ævar's suggestions were a lot more perfectionist than what\npragmatic me would say, and didn't mesh well with the test of Dscho\nwho is even more pragmatic than me? In a separate message, Dscho\ntalks about weeks of delay, but it does not look to me that it is\nsolely Ævar's fault.\n\nWe know we hope to be able to make scalar as part of the top-level\noffering from the project someday, but before that, we should make\nsure it is as good as it has been advertiesed so far, and by not\neven being able to easily integrate \"correctly\" (i.e. in line with\nthe design of the surrounding code) with the CI, we are stumbling at\nthe first step.\n\nThanks, both.\n"}]}