On Thu, Oct 21 2021, Johannes Schindelin wrote:
> Hi Ævar,
>
> tl;dr it isn't worth your nor my time for you to focus on the build
> process in contrib/scalar/ at this moment.
>
> Having said that, I do appreciate your interest in this patch series, and
> I have suggestions at the end of this mail how we could collaborate on it
> in a more fruitful manner.
>
> On Wed, 20 Oct 2021, Ævar Arnfjörð Bjarmason wrote:
>
>> On Tue, Oct 19 2021, Johannes Schindelin wrote:
>>
>> > On Mon, 18 Oct 2021, Junio C Hamano wrote:
>> >
>> >> * js/scalar (2021-10-07) 15 commits
>> >> - scalar: accept -C and -c options before the subcommand
>> >> - scalar: implement the `version` command
>> >> - scalar: implement the `delete` command
>> >> - scalar: teach 'reconfigure' to optionally handle all registered enlistments
>> >> - scalar: allow reconfiguring an existing enlistment
>> >> - scalar: implement the `run` command
>> >> - scalar: teach 'clone' to support the --single-branch option
>> >> - scalar: implement the `clone` subcommand
>> >> - scalar: implement 'scalar list'
>> >> - scalar: let 'unregister' handle a deleted enlistment directory gracefully
>> >> - scalar: 'unregister' stops background maintenance
>> >> - scalar: 'register' sets recommended config and starts maintenance
>> >> - scalar: create test infrastructure
>> >> - scalar: start documenting the command
>> >> - scalar: create a rudimentary executable
>> >>
>> >> Add pieces from "scalar" to contrib/.
>> >>
>> >> What's the status of this thing?
>> >
>> > As far as I am concerned, the current status is: We agreed that this
>> > thing _can_ live in contrib/, and that the `scalar` command itself
>> > should not be integrated deeply into Git because the end game is for
>> > `git clone` (and maybe `git maintenance`) to learn Scalar's tricks.
>> >
>> > A concern was raised that `make -C contrib/scalar` does not rebuild
>> > `libgit.a` whenever any of `libgit.a`'s source files were modified. In
>> > light of the previous paragraph, I believe that my time would be
>> > better spent on designing a new `git clone` option, though, than to
>> > spend time on a build process that will be soon obsolete (except if I
>> > allow myself to be distracted by things like teaching `make -C
>> > contrib/scalar` to know about `libgit.a`'s prerequisites). In other
>> > words, it is a technical debt I firmly believe is worth accruing.
>> >
>> > Other than that, I heard no objections, therefore I believe that this
>> > is ready for `next`, to be cooked in the v2.34 cycle.
>>
>> Your patches as they stand don't implement a "make install{-doc,-man}"
>> for the new command.
>
> Right. They are just a start.
>
> I would like to direct your attention to
> https://github.com/microsoft/git/compare/vfs-2.33.0...dscho:vfs-with-scalar,
> which has a more complete picture of what I have.
Okey, seems to be general scalar features from a quick glance...
> It is a thicket of 8 patch series. I would like to hold your horses until
> it is time for me to submit `include-scalar-in-build`, in particular
> https://github.com/dscho/git/commit/473ca8ae673.
...the patches I've got on top of your series are:
9 files changed, 36 insertions(+), 162 deletions(-)
Which already do the installation step and fix dependency issues you
have etc. Then this linked patch above is:
Showing with 51 additions and 0 deletions.
So we're ~200 lines in with Makefile logic that we could demonstrably
get in a much easier way, and this still doesn't address some of the
integration bugs.
>> I'm happy to help you make that work, but I don't think framing it as
>> some abstract objection about whether something lives in contrib or not
>> is accurate.
>
> Scalar is headed to contrib/ _specifically_ to make it clear that it is
> _not_ a core part of Git. I laid out the reasons before: the user
> interface of the Scalar executable is already in use, and hence
> non-negotiable.
This invariant that something must be in a given directory so that we
communicate that it's not a "core part" is, I think, something you or
Stolee came up with. My reading of Junio's messages on the matter is
that he'd be OK with it either way.
In any case, what I've also mentioned to you is that moving it it out of
contrib isn't necessary for the changes I'm proposing.
E.g. the build process could with a couple of more rules just symlink
those for the duration of the build, or all the wildcards involved could
be adjusted. It would still be simpler than your implementation.
But since all of that would be *indistinguishable to anyone using git*,
since the difference would go away with "make install" I really don't
see why you think it's necessary over say a comment at the top of each
file, naming the files contrib-scalar.c etc.
I.e. we're *only* talking about a difference seen by people who
contribute to git, which is a handful of people reading this ML for the
purposes of this discussion.
If those people know that scalar's integration & maintenance status is
so-and-so why does an implementation detail of the build system matter?
And if one way of doing that is demonstrably much simpler, why not go
for that?
Who are these people that will see some files in the top-level directory
as opposed to contrib/* and become confused?
> Since it won't be part of core Git, I fail to see the urgency to integrate
> its build process more closely into Git's build process, _right now_.
I think the diffstats make it evident that you're wrong about what
constitutes closer integration.
Just because two parts of a program live in different files or different
places on a filesystem doesn't mean that they're not more logically
entangled.
You already have a hard dependency on git's make process and dependency
logic for this subdirectory, the question is whether that needs to be
this complex. It doesn't.
Having to have the two communicate over 2+ Makefiles makes that
integration closer and more complex, not more detached.
>> For example in your just-sent[2] you say:
>>
>> I would like to add a plug for Scalar here. Maybe we can link to this
>> "opinionated tool based on Git" here? I wouldn't ask if I didn't _know_
>> that it helps monorepo users out there.
>>
>> I agree that would be useful, but currently our documentation build
>> would fail if you linked to the scalar from other git documentation.
>> Since we lint it and check if the linkgit:* crosslinks would be 404'd.
>
> The link I was implying was
> https://github.com/microsoft/git/blob/HEAD/contrib/scalar/docs/index.md.
> Sorry for not spelling it out clearly.
Yes, but that's dodging the issue. Will we never want to have a
"linkgit:scalar[1]" anywhere else in git's documentation referring to
it? Or to refer from scalar's documentation to git's?
You do have the latter already in your patches, it's just not lint'ed in
your version of the patches.
Just like the sources aren't run against "make coccicheck". Any such
problem is small in isolation, but we've got various "make" integrations
like that.
>> I don't see why wouldn't consider an up-front solution to that technical
>> debt, or why you're seemingly ignoring comments about aspects of your
>> patches that are broken or will cause that unnecessary technical debt.
>
> One of the many reasons why one might want to accrue technical debt, and
> it is my reason in this context, is the expectation that it won't need to
> be paid down at all because we will eventually just delete contrib/scalar/
> in a couple of years, when it has served its purpose.
That sounds good, and in my version of the Makefile integration that'll
be removing a line or two referring to "scalar" somewhere.
In yours it'll be a lot more lines, not counting any shimmying we accrue
in the meantime to e.g. make doc lints and the like work for this
special-case.
>> It would be a hassle to deal with when it comes to various build-system
>> integration we already have, or which I have WIP work to implement. I'm
>> also offering to fix it for you, so it wouldn't be much of a distraction
>> to your efforts.
>
> Unfortunately, it is very much a distraction of my efforts. Not only would
> your patches force me to spend more time to rework and adapt my patches to
> your changes, as they _specifically_ touch the same code that I am working
> on. It is also forcing me to spend time on reading all your mails, and on
> writing mails like this one.
Well, you and me both. The reason I hacked up this alternate
implementation for you is because your proposed patches semantically
conflicted with Makefile improvements I've got locally.
Aside from any problems I'd have with that it seems to me that you'd
benefit more from the version of the integration I hacked up,
specifically we'll always build and test scalar, we just won't install
unless asked. So you'll get CI etc. integration from day one.
I think (but I'm not sure as I write this) that in some earlier version
of your patches scalar was truly optional, but even in your v1 it's
added to "OBJECTS", but commit message of the 1st patch still says:
The idea here is that you can (optionally) build Scalar via
make -C contrib/scalar/Makefile
This will build the `scalar` executable and put it into the
contrib/scalar/ subdirectory.
Now, I think we should just build/test the thing unconditionally, and
make installation optional, regardless of any question of where it lives
in-tree.
But aside from that this integration of building it but not testing it
seemsl like the worst of both worlds. I.e. it's not "truly contrib" in
that anyone working in-tree can ignore it, and "upstream" needs to deal
with any changes, but if you break it you might not notice for a while,
as we don't run the tests.
