Hi Peff,

On Fri, 10 Dec 2021, Jeff King wrote:

> On Fri, Dec 10, 2021 at 03:38:53AM +0100, Ævar Arnfjörð Bjarmason wrote:
>
> > I just don't think it makes any sense that I edit say refs.[ch], run
> > "make test" locally, but only see that something broke in scalar's
> > specific use of libgit.a later when I look at GitHub CI.
>
> I'm definitely sympathetic to this. Having been surprised by CI failure
> on something that worked locally is annoying at best, and downright
> frustrating when you can't easily reproduce the problem.

I feel your frustration. Same here.

> But isn't that already true for most of the value that CI provides?
> While part of its purpose may be a back-stop for folks who don't run
> "make test" locally, I think the biggest value is that it covers a much
> wider variety of platforms and scenarios that you don't get out of "make
> test" already.
>
> In some of those cases you can reproduce the problem locally by tweaking
> build or test knobs. But in others it can be quite a bit more
> challenging (e.g., something that segfaults only on Windows). At least
> in the proposed change here you'd only be a "make test-all" away from
> reproducing the problem locally.
>
> I dunno. I don't feel that strongly either way about whether scalar
> tests should be part of "make test". Mostly just observing that this is
> not exactly a new case.

It isn't a new case.

What is new is that we are talking about CI for patches targeting contrib/
specifically to introduce something cautiously that still has a chance of
not ending up in Git proper (for whatever reasons), as Junio seems to
be anxious to not give any premature "go" to integrate Scalar fully.

In that light, I am somewhat surprised that we are still discussing
putting a burden on contributors having to adapt contrib/scalar/ to
their changes, when Junio still endeavors the option of not accepting
that to-be-adapted code into core Git, after all.

I fully expected everybody to be on board with leaving the responsibility
to keep contrib/scalar/ building and passing the tests to _me_, until the
day Scalar is accepted as a full Git command (which might not happen).

> > If I'm preparing patches for submission I'll need to get CI passing, so
> > I'll need to fix those tests & behavior either way as it's
> > in-tree. Knowing about the failures later-not-sooner wastes more time,
> > not less.
>
> I think there's probably a tradeoff here. How often you get a "late"
> notification of a bug (and how much of your time that wastes) versus how
> much time you spend locally running tests that you don't care about.
>
> I do agree that CI presents a bit of a conundrum for stuff at the edge
> of the project. It's become a de facto requirement for it to pass. In
> general that's good. But it means that features which were introduced
> under the notion of "the people who care about this area will tend to
> its maintenance" slowly become _everybody's_ problem as soon as they
> have any CI coverage. Another example here is the cmake stuff. Or the
> recent discussion about "-x" and bash.
>
> I wonder if there's a good way to make some CI results informational,
> rather than "failing". I.e., run scalar tests via CI, but if you're not
> working on scalar, you don't have to care. Folks who are interested in
> the area would keep tabs on those results and make sure that Junio's
> tree stays passing.
>
> That view disagrees with the final paragraph here, though:
>
> > The reason we do that with the completion is because some changes to
> > e.g. tweak getopts will need to have a corresponding change to the
> > completion.
> >
> > The reason we've not done that with contrib/{subtree,mw-to-git}/ is
> > because those are thoroughly in the category of only incidentally being
> > in-tree.
> > [...]
> > Scalar is thoroughly on the "completion" side of that divide, not
> > "subtree".
>
> I haven't followed the discussion closely, but in my mind "scalar" was
> still in the "it may live in-tree for convenience, but people who aren't
> working on it don't necessarily need to care about it" camp. Maybe
> that's not the plan, though.

I had hoped for a clearer answer from Junio where he sees Scalar in the
long term, for now he seems to be undecided.

As a consequence, I kept targeting contrib/scalar/ with this first patch
series, to leave the door open for keeping it in contrib/ as a "not
maintained by Junio!" part of the tree.

That is independent, of course, of my intention to keep maintaining
Scalar's code (once we get a few steps further, that is, because we're
still quite stuck here, the Scalar patch series has not seen any concerns
in the last half dozen iterations about its design nor about its actual
code). I intend to keep maintainig the Scalar code no matter whether it
lives in contrib/ or whether it will be turned into a first-class command
whose source code lives in the top-level directory.

So yes, from my side I do not understand at all where this notion comes
from that contrib/scalar/ should be treated any different than
contrib/subtree/ for now. At least until contrib/scalar/ is
feature-complete, that won't change.

But of course, we can keep discussing back and forth the build process of
Scalar, whether it should be tested in CI or not, whether it should be in
contrib/ or in the top-level directory or not in Git at all, without
getting the Scalar patches anywhere, for the next few years, in which case
the outcome of that discussion will be completely moot because the Scalar
patches would still be as stuck as they are right now. In which case it
would be super annoying for any contributor who had to adapt the code in
contrib/scalar/ to code changes in libgit.a, for no value in return
whatsoever. So far, that contributor has been me.

I sincerely hope that it won't come to that, and that we can move forward
with this here patch series, with the next ones I have lined up to make
Scalar feature-complete, and _then_ discuss the merits of making Scalar a
first-class Git command or not. At that point we will automatically have
the answer whether to build Scalar and run its tests as part of Git's CI.

Ciao,
Dscho