On Mon, Nov 22 2021, Johannes Schindelin wrote: > On Mon, 22 Nov 2021, Ævar Arnfjörð Bjarmason wrote: > >> On Mon, Nov 22 2021, Johannes Schindelin wrote: >> >> > On Sat, 20 Nov 2021, Elijah Newren wrote: >> > >> >> On Fri, Nov 19, 2021 at 3:03 PM Johannes Schindelin via GitGitGadget >> >> <gitgitgadget@xxxxxxxxx> wrote: >> >> > >> >> > tl;dr: This series contributes the core part of the Scalar command to >> >> > the Git project. This command provides an opinionated way to create >> >> > and configure Git repositories with a focus on very large >> >> > repositories. >> >> >> >> I thought after >> >> https://lore.kernel.org/git/nycvar.QRO.7.76.6.2110062241150.395@xxxxxxxxxxxxxxxxx/ >> >> that you'd update merge.renames to true on what is now patch 7. Did >> >> you end up changing your mind, or was this overlooked? >> > >> > Oops! Thank you so much for the reminder. >> > >> > Will fix. I do not plan on sending out a new iteration for a few more days >> > because I do not want to send lots of patches to the list right now, >> > reviewer bandwidth seems to be stretched quite a bit already. >> >> Bandwidth which is further stretched by continuing to send updates to >> this topic while ignoring outstanding feedback. > > The feedback you are referring to is probably the repeated demand to > integrate Scalar deeply into Git's build process. > > As I have tired of replying, it is not the time for that yet. > > Repeating that demand does not make it more sensible, nor does it > magically make it the right time. I'm not repeating that demand. I clearly also think the approach you're insisting picking isn't a good one, but let's leave that aside. What I'm referring to with "I've also been pointing out" in the context you elided is that if you: 1. Check out your topic 2. Apply my proposed patch on top (a newer version than on-list is in avar/scalar-move-build-from-contrib-2 in my GH fork) 3. Push both to CI 4. Diff the two logs of the runs (or just manually click through and inspect them) You'd have seen before sending your version of the CI integration that the difference in behavior that started with your version of the topic was particular to the contrib/scalar/ integration, but not the top-level Makefile integration. I.e. adding the scalar tests to the previously build-only jobs. I've been noting that as clearly as I'm able to in numerous past exchanges. You've either ignored those reports, or like here, selectivtely replied only to parts of what I've told you. I.e. something like "I'm not going 100% for the approach you suggest". Sure, I'm not saying you have to. But I also noted that the patch with that suggested approach can be considered a bug report against your series. The reason that patch isn't split into two things, one fixing all the issues I noticed, and another implementing some "alternate build" approach is that I found that to be impossible to do. Those issues are all particular to emergent effects of the build integration you're choosing to go with. E.g. in the case of "seen" being broken the CI simply runs "make test" as it did before, and scalar integrates into that, and you can run that target without having built anything already. > Nor is it credible to call the build "broken" when it does what it is > supposed to do, thank you very much. Here's specific commands showing that it's broken. On your version (I've got v7 locally, but the same is true of v8): $ make clean; make -C contrib/scalar test [...] CC hook.o CC version.o CC help.o AR libgit.a make[1]: Leaving directory '/home/avar/g/git' SUBDIR ../.. make[1]: Entering directory '/home/avar/g/git' * new link flags CC contrib/scalar/scalar.o LINK contrib/scalar/scalar make[1]: Leaving directory '/home/avar/g/git' make -C t make[1]: Entering directory '/home/avar/g/git/contrib/scalar/t' *** prove *** error: GIT-BUILD-OPTIONS missing (has Git been built?). t9099-scalar.sh .. Dubious, test returned 1 (wstat 256, 0x100) No subtests run Test Summary Report ------------------- t9099-scalar.sh (Wstat: 256 Tests: 0 Failed: 0) Non-zero exit status: 1 Parse errors: No plan found in TAP output On the patch I proposed to apply on top: $ make clean; make test T=t9099-scalar.sh [...] CC t/helper/test-xml-encode.o GEN bin-wrappers/git GEN bin-wrappers/git-receive-pack GEN bin-wrappers/git-shell GEN bin-wrappers/git-upload-archive GEN bin-wrappers/git-upload-pack GEN bin-wrappers/scalar GEN bin-wrappers/git-cvsserver GEN bin-wrappers/test-fake-ssh GEN bin-wrappers/test-tool LINK t/helper/test-fake-ssh LINK t/helper/test-tool make -C t/ all make[1]: Entering directory '/home/avar/g/git/t' rm -f -r 'test-results' *** prove *** t9099-scalar.sh .. ok All tests successful. You might correctly note that this doesn't work either on that version (or for any other existing test in t/): $ make clean >/dev/null; make -C t T=t9099-scalar.sh GIT_VERSION = 2.34.GIT-dev make: Entering directory '/home/avar/g/git/t' rm -f -r 'test-results' *** prove *** error: GIT-BUILD-OPTIONS missing (has Git been built?). t9099-scalar.sh .. Dubious, test returned 1 (wstat 256, 0x100) No subtests run Test Summary Report ------------------- t9099-scalar.sh (Wstat: 256 Tests: 0 Failed: 0) Non-zero exit status: 1 Parse errors: No plan found in TAP output Which is true, but that's not broken because it's not attempting to build the top-level via some incomplete integration, but your version is.
