Re: [PATCH v10 00/15] Upstreaming the Scalar command

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 6055 bytes --]

Hi Junio,

On Wed, 8 Dec 2021, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > The Scalar Functional Tests were designed with Azure Repos in mind, i.e.
> > they specifically verify that the `gvfs-helper` (emulating Partial Clone
> > using the predecessor of Partial Clone, the GVFS protocol) manages to
> > access the repositories in the intended way.
> > ...
> > I do realize, though, that clarity of intention has been missing from this
> > mail thread all around, so let me ask point blank: Junio, do you want me
> > to include upstreaming `gvfs-helper` in the overall Scalar plan?
>
> Sorry, I do not follow.

In
https://lore.kernel.org/git/CABPp-BGpe9Q5k22Yu8a=1xwu=pZYSeNQoqEgf+DN07cU4EB1ew@mail.gmail.com/
(i.e. in the great great grand parent of this mail), you specifically
replied to my mentioning Scalar's Functional Test suite:

	> > One other thing is very interesting about that vfs-with-scalar
	> > branch thicket: it contains a GitHub workflow which will run
	> > Scalar's quite extensive Functional Tests suite. This test
	> > suite is quite comprehensive and caught us a lot of bugs in
	> > the past, not only in the Scalar code, but also core Git.
	>
	> From your wording it sounds like the plan might not include
	> moving these tests over.  Perhaps it doesn't make sense to move
	> them all over, but since they've caught problems in both Scalar
	> and core Git, it would be nice to see many of those tests come
	> to Git as well as part of a future follow on series.

I had mentioned a couple of times that I had no intention to move Scalar's
Function Tests into contrib/scalar/, and your wording "it would be nice to
see many of those tests come to Git as well" made it sound as if you
disagreed with that intention.

But it was not a clear "please do port them over" nor a "nah, we don't
want that test suite implemented in C# and requiring, for the most part,
access to a dedicacted Azure Repo".

Hence I was asking for a clear answer to the question whether you want me
to spend time on preparing a patch series to contribute Scalar's
Functional Tests to contrib/scalar/ as well.

I _suspect_ your clear answer, if you are willing to give it as clearly,
to be "no, we do not do integration tests here, and besides, C# is not a
language we want to add to Git's tree".

> What I was lamenting about was the lack of CI test coverage of stuff
> that is already being considered to go 'next'.  Specifically, since
> contrib/scalar/Makefile in 'seen' has a 'test' target, it would be a
> shame not to exercise it, when we should be able to do so in the CI
> fairly easily.

We do have a very different understanding of "fairly easily" in that case.
Three iterations, and three weeks time spent on implementing what you
suggest, only to see broken by the merge of the `ab/ci-updates` patch
series, suggesting a fixup for the incorrect merge, seeing that fixup
rejected, and then more discussing, all of that does not strike me as
"fairly easily". It strikes me as "a lot of time and effort was spent,
mostly stepping on toes".

Granted, if `ab/ci-updates` would not have happened, it would have been
much easier. Or if `ab/ci-updates` had waited until `js/scalar` advanced
to `next`. But the way it happened was (unnecessarily?) un-easy.

> I fail to see what gvfs-helper has to do with anything in the
> context of advancing the js/scalar topic as we have today.

Okay, okay! I was just asking about gvfs-helper because that would be
required to port over Scalar's Functional Tests. The same Functional Tests
that I heard you mentioning would be "nice to see" to "come to Git as
well".

> If "The Scalar Functional Tests" that were designed with Azure Repos in
> mind is not a good fit to come into contrib/scalar/, it is fine not to
> have it here---lack of it would not make the test target you have in
> contrib/scalar/Makefile any less valuable, I would think.

The test target won't go anywhere, no worries. Just like the test target
in contrib/subtree/ does not go anywhere.

And just like `contrib/subtree/`, it does not have to be run as part of
Git's CI build.

> Unless you are saying that "make -C contrib/scalar test" is useless,
> that is.  But I do not think that is the case.

It is as useful as `make -C contrib/subtree test`. Which, as Ævar will
readily offer, is broken, because it does not ensure that top-level `make
all` is executed and therefore in a fresh checkout will fail.

Of course, I disagree that it is "broken". It works as designed. It is in
the contrib/ part of the tree, i.e. safely in the realm of "you have to
build Git first, and then the thing in contrib/". In other words, the idea
to "fix" this kind of "broken"ness is a solution in search of a problem.

And as I have said multiple times, I still think that having Scalar's code
in contrib/ is a good spot to experiment with it. It sends the right
signal of "this is not really something we promise to maintain just yet".
It is a logical place for code that developers can build themselves, but
that is not built and installed with Git by default.

Having it in the Git tree will give interested developers a chance who
want to clone a large repository on Linux, without having to touch
anything with "Microsoft" in its repository name.

Having it in the Git tree will give interested developers a chance to
experiment with things like "let's try to let `scalar clone` _not_
clone into `<enlistment>/src/`, but instead create a bare clone in
`<enlistment>/.git` and make `<enlistment>/src/` a worktree". Things like
that.

I would find those things quite a bit more useful than to force regular
Git contributors who want to change libgit.a (even if it is just pointless
refactoring) to pay attention to contrib/scalar/ in CI, when there is
still no clear answer whether Scalar will even become a first-class Git
command eventually (which I hope it will, of course).

Ciao,
Dscho