From: Junio C Hamano <hidden> Date: 2021-09-14 17:29:27
Johannes Schindelin [off-list ref] writes:
Okay, let's try an analogy.
Imagine that a person is asking for directions to the train station. And
the other person is replying by asking "did you know that this train
station was built in 1878? It is actually quite interesting a story...
[and then goes on to describe the history and what excites them about
it]". Now, the first person tries again to ask for directions, again does
not get an answer to that question, and is slowly starting to look at
their watch. The second person, being completely oblivious to all of this,
goes on with their wonderful story about the train station and its
cultural heritage. So the first person walks a bit further to ask a third
person, but the second person is not done yet and says "but you haven't
heard me out! That's disrespectful!".
Just imagine for a minute how you would feel if you were the first person.
And that is how I feel asking for reviews about the Scalar patch series
and then being forcefully dragged into that tangent about the build
process.
At least to me, how this Makefile for Scalar should interact with
the overall build process does not mesh well with the story about
hwo direction to and history of the station are unrelated. If we
plan to start from contrib/ and eventually want to make it a part
of the core Git (i.e. "git scalar <subcmd> ..." becomes just like
"git bisect <subcmd> ..."), we would eventually need to see the
recipe needed for including "bisect" and "scalar" work the same
way, no?
I am getting the impression that such a unified build process is
Ævar wants to see at the end, I am not even sure if you do from
the above "analogy". Cool down a bit, perhaps?
The following assumes that you share the goal of making "git
scalar" just like "git bisect"---another first class citizen of
Git toolbox, the user can choose to use it or the user may not
have a need to interact with it, but it exists there by default
and is not an opt-in add-on component.
I would understand it if your plan is to convert to a unified
build procedure at the very end of the upstreaming process, and
not while you populate contrib/ with more and more scalar stuff,
because the Makefile bits for the entire scalar, while not yet
upstreamed, has already been written as a separate procedure and
having to convert the whole thing upfront before you can start
trickle parts would mean you need to (re)start the process. And
I would even be sympathetic if you felt it like a distraction.
But at least I view it as a step that needs to happen sometime
between now and at the end. I do not yet have an opinion on
which one is more pleasant, between (1) having to deal with a
single Makefile that needs to be aware of two different locations
*.[ch] lives in, and (2) having to deal with two Makefiles that
duplicates definitions and risks them needlessly diverging.
I also would understand it if the reason why you want to keep the
top-level Makefile as intact as possible because you sense a high
probability that scalar will stay in contrib/ and even turn out
to be a failure. Keeping the build procedure separated certainly
will keep it easier to yank it out later. But I do not think
such a case is quite likely.
Thanks.
Okay, let's try an analogy.
Imagine that a person is asking for directions to the train station. And
the other person is replying by asking "did you know that this train
station was built in 1878? It is actually quite interesting a story...
[and then goes on to describe the history and what excites them about
it]". Now, the first person tries again to ask for directions, again does
not get an answer to that question, and is slowly starting to look at
their watch. The second person, being completely oblivious to all of this,
goes on with their wonderful story about the train station and its
cultural heritage. So the first person walks a bit further to ask a third
person, but the second person is not done yet and says "but you haven't
heard me out! That's disrespectful!".
Just imagine for a minute how you would feel if you were the first person.
And that is how I feel asking for reviews about the Scalar patch series
and then being forcefully dragged into that tangent about the build
process.
At least to me, how this Makefile for Scalar should interact with
the overall build process does not mesh well with the story about
hwo direction to and history of the station are unrelated. If we
plan to start from contrib/ and eventually want to make it a part
of the core Git (i.e. "git scalar <subcmd> ..." becomes just like
"git bisect <subcmd> ..."), we would eventually need to see the
recipe needed for including "bisect" and "scalar" work the same
way, no?
I am getting the impression that such a unified build process is
Ævar wants to see at the end, I am not even sure if you do from
the above "analogy". Cool down a bit, perhaps?
The following assumes that you share the goal of making "git
scalar" just like "git bisect"---another first class citizen of
Git toolbox, the user can choose to use it or the user may not
have a need to interact with it, but it exists there by default
and is not an opt-in add-on component.
I would understand it if your plan is to convert to a unified
build procedure at the very end of the upstreaming process, and
not while you populate contrib/ with more and more scalar stuff,
because the Makefile bits for the entire scalar, while not yet
upstreamed, has already been written as a separate procedure and
having to convert the whole thing upfront before you can start
trickle parts would mean you need to (re)start the process. And
I would even be sympathetic if you felt it like a distraction.
But at least I view it as a step that needs to happen sometime
between now and at the end. I do not yet have an opinion on
which one is more pleasant, between (1) having to deal with a
single Makefile that needs to be aware of two different locations
*.[ch] lives in, and (2) having to deal with two Makefiles that
duplicates definitions and risks them needlessly diverging.
For what it's worth what I had on top of this is not (1) or (2), but a
(0): I.e. there isn't a contrib/scalar anymore, I moved:
contrib/scalar/scalar.c -> scalar
contrib/scalar/scalar.txt -> Documentation/scalar.txt
contrib/scalar/t9099-scalar.sh -> t/t9099-scalar.sh
We build, test, and otherwise check (e.g. "make check-docs") it by
default, what we don't do is install it unless you ask. You need to run:
# Or any other install* target
make install install-doc INSTALL_SCALAR=YesPlease
It could be be kept in contrib/scalar/ even with that sort of approach,
and it would still be simpler than the two-Makefile approach.
But just moving the code, tests and documentation where everything else
lives cuts down an all sorts of special cases, file globs in various
places (e.g. doc lints) will just work and won't need adjustment.
I also would understand it if the reason why you want to keep the
top-level Makefile as intact as possible because you sense a high
probability that scalar will stay in contrib/ and even turn out
to be a failure. Keeping the build procedure separated certainly
will keep it easier to yank it out later. But I do not think
such a case is quite likely.
For what it's worth the WIP patch(es) I have on top of it will probably
make such a thing even easier, not that removing it from the tree would
be much of a problem in either case. It's mostly a few lines added to
lists in various places in the Makfile.
If I were to clean this up properly most of the changes would be
teaching the Makefile that it can build N number of named top-level
"special" commands that get dropped into bin/, not just the "git" we
hardcode now.
If you're interested I could try to clean that up and send something on
top, but given the tense-ness of the discussion & unrelated but relevant
patch queue I've got outstanding wouldn't do so otherwise...
On 9/14/2021 2:09 PM, Ævar Arnfjörð Bjarmason wrote:
On Tue, Sep 14 2021, Junio C Hamano wrote:
quoted
Johannes Schindelin [off-list ref] writes:
At least to me, how this Makefile for Scalar should interact with
the overall build process does not mesh well with the story about
hwo direction to and history of the station are unrelated. If we
plan to start from contrib/ and eventually want to make it a part
of the core Git (i.e. "git scalar <subcmd> ..." becomes just like
"git bisect <subcmd> ..."), we would eventually need to see the
recipe needed for including "bisect" and "scalar" work the same
way, no?
We should definitely work to find a better way to describe our
vision for how _the ideas in Scalar_ can be adopted into Git proper.
Before this series, we were adding functionality to Git that allowed
Scalar to simplify to just a CLI that configures Git features. This
submission allows that CLI to be available via an opt-in compile flag.
This should allow more users to try out the ideas and perhaps we find
the things that really work for people (and almost more importantly,
the ideas that are _too_ opinionated).
But the way I see Scalar being fully incorporated into Git is not as
a "git scalar <foo>" command or even having "scalar" be included by
default. Instead, perhaps a new builtin would need to be created and
its CLI would need to be presented and reviewed with significant
attention to long-term support in the Git project. Having Scalar as
a testing ground for these ideas seems like a positive way forward.
This is a big reason why we think that contrib/ is a good place for
it to exist.
quoted
I am getting the impression that such a unified build process is
Ævar wants to see at the end, I am not even sure if you do from
the above "analogy". Cool down a bit, perhaps?
I agree that the temperature of this thread has gotten a bit
heated. I think there is something valuable to be gained from
each perspective, but not in a way that either has presented it.
quoted
The following assumes that you share the goal of making "git
scalar" just like "git bisect"---another first class citizen of
Git toolbox, the user can choose to use it or the user may not
have a need to interact with it, but it exists there by default
and is not an opt-in add-on component.
I would understand it if your plan is to convert to a unified
build procedure at the very end of the upstreaming process, and
not while you populate contrib/ with more and more scalar stuff,
because the Makefile bits for the entire scalar, while not yet
upstreamed, has already been written as a separate procedure and
having to convert the whole thing upfront before you can start
trickle parts would mean you need to (re)start the process. And
I would even be sympathetic if you felt it like a distraction.
But at least I view it as a step that needs to happen sometime
between now and at the end. I do not yet have an opinion on
which one is more pleasant, between (1) having to deal with a
single Makefile that needs to be aware of two different locations
*.[ch] lives in, and (2) having to deal with two Makefiles that
duplicates definitions and risks them needlessly diverging.
Since we already need to modify the root Makefile, I think having
the root Makefile add the files from contrib/scalar from an
optional flag is a great way to reduce duplication across multiple
Makefiles while also maintaining the Scalar is compiled optionally.
One big goal is to minimize how often we need to update Scalar. I
can see things like adjusting the recommended config once per
release cycle based on which new features are available. I don't
really want to be spending time updating the Makefile to match a
contribution that was already carefully reviewed and tested. I
also don't want to put the burden of updating contrib/scalar upon
those contributors.
For what it's worth what I had on top of this is not (1) or (2), but a
(0): I.e. there isn't a contrib/scalar anymore, I moved:
contrib/scalar/scalar.c -> scalar> contrib/scalar/scalar.txt -> Documentation/scalar.txt
contrib/scalar/t9099-scalar.sh -> t/t9099-scalar.sh
We build, test, and otherwise check (e.g. "make check-docs") it by
default, what we don't do is install it unless you ask. You need to run:
# Or any other install* target
make install install-doc INSTALL_SCALAR=YesPlease
It could be be kept in contrib/scalar/ even with that sort of approach,
and it would still be simpler than the two-Makefile approach.
I think keeping it in contrib/scalar is best for now. But I do
agree that a single Makefile has benefits.
One early suggestion from a while back was to modify git.c to
handle the "scalar" executable as well as the "git" executable,
specifically to reduce duplication handling options such as
-c config.key=value
-C worktree
--exec-path
and similar commands. While our duplication of the "-c" option
does add similar code in a second place, these other options
are less critical for Scalar, especially in its current version.
I think refactoring the code in git.c to cater to the "scalar"
executable is at least premature. If we want to pursue these
other options in the future, then that refactoring could happen
as a separate discussion after the rest of the build system and
CLI have been figured out.
_Perhaps_ Johannes still had that level of integration in his
head when responding to the single-Makefile recommendations.
But just moving the code, tests and documentation where everything else
lives cuts down an all sorts of special cases, file globs in various
places (e.g. doc lints) will just work and won't need adjustment.
quoted
I also would understand it if the reason why you want to keep the
top-level Makefile as intact as possible because you sense a high
probability that scalar will stay in contrib/ and even turn out
to be a failure. Keeping the build procedure separated certainly
will keep it easier to yank it out later. But I do not think
such a case is quite likely.
For what it's worth the WIP patch(es) I have on top of it will probably
make such a thing even easier, not that removing it from the tree would
be much of a problem in either case. It's mostly a few lines added to
lists in various places in the Makfile.
Do you have a version of these patches available for adaptation
into this series? I'd like to take a look and see what it would
look like to squash them into this series. Forgive me if I just
missed the link. (I see the diff you posted earlier in this thread.)
If I were to clean this up properly most of the changes would be
teaching the Makefile that it can build N number of named top-level
"special" commands that get dropped into bin/, not just the "git" we
hardcode now.
This is an interesting idea for revamping how adjacent tools are
compiled and shipped with Git from contrib/ (or possibly elsewhere
if we decided to start including more things as "blessed helpers".
As a complete aside: I'm interested in using the sparse-checkout
feature as I work on the Git codebase, just to make sure I hit
pain points before any other user.
This is the best that I could do for my purposes:
$ git sparse-checkout list
.github
Documentation
builtin
compat
contrib/scalar
ewah
git-gui
gitk-git
gitk-gui
gitweb
mergetools
negotiator
perl
po
refs
sha1dc
sha256
t
templates
trace2
xdiff
And 'git status' reports that this includes 97% of the tracked
files. Perhaps there are ways to make this be smaller by having
make skip building things like git-gui if the directory doesn't
exist. Another idea would be to skip any logic around translating
messages if the 'po' directory is missing.
The reason I bring this up is that I'm interested in finding
ways to make our build system be streamlined a bit using the
presence of directories as a way to opt in/out of certain build
outputs. Since Scalar is being added as a new component, this is
a good opportunity to establish a pattern that works for this
effort, too.
Thanks,
-Stolee
From: Junio C Hamano <hidden> Date: 2021-09-14 21:49:21
Junio C Hamano [off-list ref] writes:
But at least I view it as a step that needs to happen sometime
between now and at the end. I do not yet have an opinion on
which one is more pleasant, between (1) having to deal with a
single Makefile that needs to be aware of two different locations
*.[ch] lives in, and (2) having to deal with two Makefiles that
duplicates definitions and risks them needlessly diverging.
FWIW, I am leaning towards the latter, with the assumption that this
may take more than a cycle to cook in contrib/.
Adding the Makefile bits to the top-level and keeping the topic in
'next' means all the topics that pass the pipeline will need to be
written in such a way that its Makefile change works well with and
without the unified Makefile bits from this topic, an additional
burden on other topics this topic would impose.
So it is understandable to want to keep the changes to the top-level
Makefile to the minimum, even if it may mean that it requires more
effort in the end to clean things up when the topic graduates.
An alternative would be to bypass the contrib/ phase and start as a
new subcommand that is first-class citizen from day one and let it
spend as much time as it needs to mature. It would burden the
topics that pass the pipeline while this is cooking the same way as
having a unified build procedure in the top-level Makefile, of
course, though.
On Tue, Sep 14, 2021 at 04:35:42PM -0400, Derrick Stolee wrote:
We should definitely work to find a better way to describe our
vision for how _the ideas in Scalar_ can be adopted into Git proper.
..
But the way I see Scalar being fully incorporated into Git is not as
a "git scalar <foo>" command or even having "scalar" be included by
default.
I agree 1000%. It may be convenient for existing scalar users to have
a way they can stick with the CLI that they are used to, but before we
add this functionality to git proper, let's please make sure git users
get a CLI which makes sense and not dictated by history.
We already have enough people who complain that the git interface
design is hard to understand but which can't be changed because we
bias our UI design in favor of existing users as opposed new users.
Given that I'm an existing user, I'm actually not complaining, but I
do recognize the validity of the complaints that for example, git
reset does three different, mostly unrelated things.
Given that the existing scalar users are used to "scalar <foo>" and
not "git scalar <foo">, I'd gently suggest that it's better that there
be an existing compatibility program which translates "scalar <foo>"
to whatever git command(s) makes sense, as opposed to optimizing for
the simplicity of said program so that all forms of "scalar <foo>"
should get translated to "git scalar <foo>".
So for example, if we are inside a mono repro, it would seem to me
that "git commit" should automatically do the right thing, as opposed
to imposing cognitive load on the user to know when they are supposed
to type "git commit ..." versus "git scalar commit ..." versus "scalar
commit".
Please, let's design the UI for scalar integration into git with deep
sympathy for the users, and *not* the convenience of the compatibility
script for the existing scalar CLI users.
- Ted
On 9/14/2021 2:09 PM, Ævar Arnfjörð Bjarmason wrote:
quoted
On Tue, Sep 14 2021, Junio C Hamano wrote:
quoted
Johannes Schindelin [off-list ref] writes:
At least to me, how this Makefile for Scalar should interact with
the overall build process does not mesh well with the story about
hwo direction to and history of the station are unrelated. If we
plan to start from contrib/ and eventually want to make it a part
of the core Git (i.e. "git scalar <subcmd> ..." becomes just like
"git bisect <subcmd> ..."), we would eventually need to see the
recipe needed for including "bisect" and "scalar" work the same
way, no?
We should definitely work to find a better way to describe our
vision for how _the ideas in Scalar_ can be adopted into Git proper.
Before this series, we were adding functionality to Git that allowed
Scalar to simplify to just a CLI that configures Git features. This
submission allows that CLI to be available via an opt-in compile flag.
This should allow more users to try out the ideas and perhaps we find
the things that really work for people (and almost more importantly,
the ideas that are _too_ opinionated).
Yeah that makes sense. I think it looks like a useful command & it's
already useful to users.
I haven't been suggesting any changes to what gets installed here, FWIW
I think we could be even more aggressive on that front, e.g. shipping it
unconditionally in libexec, maybe with an optional switch for
/usr/bin/scalar, or to ship "scalar" symlinked to "git" and have it
route to the top-level scalar command depending on argv.
I dabbled in that a bit locally, FWIW it seems if anything even easier
to do than the approaches we've discussed so far, but I wanted to focus
on providing the same behavior in terms of build system maintenance.
But the way I see Scalar being fully incorporated into Git is not as
a "git scalar <foo>" command or even having "scalar" be included by
default. Instead, perhaps a new builtin would need to be created and
its CLI would need to be presented and reviewed with significant
attention to long-term support in the Git project. Having Scalar as
a testing ground for these ideas seems like a positive way forward.
*Nod*
This is a big reason why we think that contrib/ is a good place for
it to exist.
Here's where you and Johannes lose me. There's some rationale in your
minds for why sticking it in contrib is the obvious way to go. So far
you've been describing how it'll look to users etc, how it's arranged in
our source tree only matters to git.git developers.
I think the actual reason is to carve in advance some subjective
ownership/apartness or whatever for this thing, if that's the case I
think documentation/commit messages would also work.
I really don't care much if something lives in contrib or not in the
abstract, but various integration around builds in Makefile makes that
much easier in practice, and if "make install" looks the same...
quoted
quoted
I am getting the impression that such a unified build process is
Ævar wants to see at the end, I am not even sure if you do from
the above "analogy". Cool down a bit, perhaps?
I agree that the temperature of this thread has gotten a bit
heated. I think there is something valuable to be gained from
each perspective, but not in a way that either has presented it.
Thanks, hopefully we can keep it jovial going forward. If you've got any
(either on-list or off-list) feedback about how I can improve my side of
that it would be most welcome.
quoted
quoted
The following assumes that you share the goal of making "git
scalar" just like "git bisect"---another first class citizen of
Git toolbox, the user can choose to use it or the user may not
have a need to interact with it, but it exists there by default
and is not an opt-in add-on component.
I would understand it if your plan is to convert to a unified
build procedure at the very end of the upstreaming process, and
not while you populate contrib/ with more and more scalar stuff,
because the Makefile bits for the entire scalar, while not yet
upstreamed, has already been written as a separate procedure and
having to convert the whole thing upfront before you can start
trickle parts would mean you need to (re)start the process. And
I would even be sympathetic if you felt it like a distraction.
But at least I view it as a step that needs to happen sometime
between now and at the end. I do not yet have an opinion on
which one is more pleasant, between (1) having to deal with a
single Makefile that needs to be aware of two different locations
*.[ch] lives in, and (2) having to deal with two Makefiles that
duplicates definitions and risks them needlessly diverging.
Since we already need to modify the root Makefile, I think having
the root Makefile add the files from contrib/scalar from an
optional flag is a great way to reduce duplication across multiple
Makefiles while also maintaining the Scalar is compiled optionally.
One big goal is to minimize how often we need to update Scalar. I
can see things like adjusting the recommended config once per
release cycle based on which new features are available. I don't
really want to be spending time updating the Makefile to match a
contribution that was already carefully reviewed and tested. I
also don't want to put the burden of updating contrib/scalar upon
those contributors.
I'd think not having large parts of t/Makefile & Makefile should ease
that maintenance burden for you & others.
quoted
For what it's worth what I had on top of this is not (1) or (2), but a
(0): I.e. there isn't a contrib/scalar anymore, I moved:
contrib/scalar/scalar.c -> scalar> contrib/scalar/scalar.txt -> Documentation/scalar.txt
contrib/scalar/t9099-scalar.sh -> t/t9099-scalar.sh
We build, test, and otherwise check (e.g. "make check-docs") it by
default, what we don't do is install it unless you ask. You need to run:
# Or any other install* target
make install install-doc INSTALL_SCALAR=YesPlease
It could be be kept in contrib/scalar/ even with that sort of approach,
and it would still be simpler than the two-Makefile approach.
I think keeping it in contrib/scalar is best for now. But I do
agree that a single Makefile has benefits.
I noted the "why contrib" above.
One early suggestion from a while back was to modify git.c to
handle the "scalar" executable as well as the "git" executable,
specifically to reduce duplication handling options such as
-c config.key=value
-C worktree
--exec-path
and similar commands. While our duplication of the "-c" option
does add similar code in a second place, these other options
are less critical for Scalar, especially in its current version.
I think refactoring the code in git.c to cater to the "scalar"
executable is at least premature. If we want to pursue these
other options in the future, then that refactoring could happen
as a separate discussion after the rest of the build system and
CLI have been figured out.
As noted above that seems like a sensible way forward, I hadn't noticed
how much of git.c's setup was copied into scalar.c.
It seems to me that it wouldn't be that hard, on the order of the
existing setup code or less. I.e. just make "git.c" learn that it may be
running some arbitrary command name, and do some options parsing, but
and finally dispatch to a cmd_scalar(). IOW mostly like a built-in.
_Perhaps_ Johannes still had that level of integration in his
head when responding to the single-Makefile recommendations.
quoted
But just moving the code, tests and documentation where everything else
lives cuts down an all sorts of special cases, file globs in various
places (e.g. doc lints) will just work and won't need adjustment.
quoted
I also would understand it if the reason why you want to keep the
top-level Makefile as intact as possible because you sense a high
probability that scalar will stay in contrib/ and even turn out
to be a failure. Keeping the build procedure separated certainly
will keep it easier to yank it out later. But I do not think
such a case is quite likely.
For what it's worth the WIP patch(es) I have on top of it will probably
make such a thing even easier, not that removing it from the tree would
be much of a problem in either case. It's mostly a few lines added to
lists in various places in the Makfile.
Do you have a version of these patches available for adaptation
into this series? I'd like to take a look and see what it would
look like to squash them into this series. Forgive me if I just
missed the link. (I see the diff you posted earlier in this thread.)
I've got it at
https://github.com/avar/git/tree/avar-dscho/scalar-the-beginning-normalize-Makefile
Not very ML-ready, and soft-depends on some other Makefile cleanups I
thought I'd do & still haven't untangled and submitted. Soft-depends as
in this can easily be done on master, but some of the variable names
etc. are quite confusing there.
But you should be able to check it out, it'll build, test and install if
you run "install" with "INSTALL_SCALAR=Y".
You may run into on everly eager new (but unrelated to this, I just
merged it on top) Makefile assertion I'm experimenting with, just
comment out the relevant line in the Makefile if that happens,
i.e. something like this error:
Makefile:3608: *** "please sort and de-duplicate BUILT_INS_EXTRA!". Stop.
quoted
If I were to clean this up properly most of the changes would be
teaching the Makefile that it can build N number of named top-level
"special" commands that get dropped into bin/, not just the "git" we
hardcode now.
This is an interesting idea for revamping how adjacent tools are
compiled and shipped with Git from contrib/ (or possibly elsewhere
if we decided to start including more things as "blessed helpers".
As a complete aside: I'm interested in using the sparse-checkout
feature as I work on the Git codebase, just to make sure I hit
pain points before any other user.
This is the best that I could do for my purposes:
$ git sparse-checkout list
.github
Documentation
builtin
compat
contrib/scalar
ewah
git-gui
gitk-git
gitk-gui
gitweb
mergetools
negotiator
perl
po
refs
sha1dc
sha256
t
templates
trace2
xdiff
And 'git status' reports that this includes 97% of the tracked
files. Perhaps there are ways to make this be smaller by having
make skip building things like git-gui if the directory doesn't
exist. Another idea would be to skip any logic around translating
messages if the 'po' directory is missing.
For git-gui in particular NO_TCLTK=Y should do it.
The reason I bring this up is that I'm interested in finding
ways to make our build system be streamlined a bit using the
presence of directories as a way to opt in/out of certain build
outputs. Since Scalar is being added as a new component, this is
a good opportunity to establish a pattern that works for this
effort, too.
Sure, the hard part isn't that you can't grep out nonexisting files or
directories when building where we now use a glob.
It's that everything downstream of that, i.e. tests, installation
etc. is going to have to work properly in the face of arbitrary parts of
what the developer who tested the code expected going missing.
Which is why we've generally carved out very specific things, usually
along the boundaries of installed dependencies.
From: Johannes Schindelin <hidden> Date: 2021-10-06 20:10:18
Hi Junio,
On Tue, 14 Sep 2021, Junio C Hamano wrote:
An alternative would be to bypass the contrib/ phase and start as a
new subcommand that is first-class citizen from day one and let it
spend as much time as it needs to mature.
I don't think that there is a lot of sense in that. The main benefits of
`scalar` are in the `register` and the `clone` part, and the most natural
end game would hence be for `git init` and `git clone` to sprout new
options to support Scalar's features, in a Git-native way.
As I have explained earlier, the `scalar` command has existing users, and
therefore its command-line interface is not up for discussion (for
example, turning `scalar` into `git scalar` would be a usability
disaster). Scalar's _functionality_, however, should make it into Git
proper. Into existing built-ins, that is.
So I don't think that the contrib/ phase can be by-passed. It would not
make sense to port Scalar to a new builtin. To the contrary,
contrib/scalar/ should be the final destination for the `scalar` command.
And you can't bypass a final destination. That simply makes no sense.
So why bother with contrib/ at all? you may ask. The reason is that it
makes it substantially easier for me to move the features into core Git,
as I can incrementally implement those new options for Git's built-ins,
use them in `contrib/scalar/` instead of duplicating the functionality,
and then make use of Scalar's Functional Test suite for a much more
comprehensive testing (which has served us already really well in the
past). It also doesn't hurt that this way, my day job will be very happy
because Scalar users directly benefit from that work.
Of course, these suggestions to integrate Scalar more into the core part
of Git (missing the point that the final destination for the functionality
is not a new built-in, but rather new options for existing built-ins) made
everything much more cumbersome for me instead, for no gain that would be
apparent to me, impeding on aforementioned ease to move the features into
core Git (which has not happened yet, as a consequence), but hopefully
this will soon be a thing of the past.
So I would like to request that we close the discussion about the question
whether to integrate Scalar more into the top-level Makefile or into
git.c, and instead go ahead with keeping the `scalar` command in
contrib/scalar/. The freed-up time can then be used to focus on the much
more rewarding project of upstreaming Scalar's functionality such as
teaching `git clone` a short-and-sweet option that Just Makes Sense for
large monorepos (i.e. that imitates at least a large part of what `scalar
clone` does right now).
Ciao,
Dscho
An alternative would be to bypass the contrib/ phase and start as a
new subcommand that is first-class citizen from day one and let it
spend as much time as it needs to mature.
I don't think that there is a lot of sense in that. The main benefits of
`scalar` are in the `register` and the `clone` part, and the most natural
end game would hence be for `git init` and `git clone` to sprout new
options to support Scalar's features, in a Git-native way.
As I have explained earlier, the `scalar` command has existing users,
and therefore its command-line interface is not up for discussion (for
example, turning `scalar` into `git scalar` would be a usability
disaster). Scalar's _functionality_, however, should make it into Git
proper. Into existing built-ins, that is.
Given the sub-thread this seems like it's trying to be an indirect reply
to me, but I haven't been advocating changing the UX of "scalar" in any
way, or that we should have a "git scalar".
I have in fact been advocating exactly what you're advocating here. So
we're in violent agreement. Yes the CLI UI shouldn't change, that's the
whole point of having a "scalar" in git.git, not a "git scalar" or
whatever.
I've been suggesting we can simplify the *build system* git.git uses,
something no user will ever see.
So I don't think that the contrib/ phase can be by-passed. It would not
make sense to port Scalar to a new builtin. To the contrary,
contrib/scalar/ should be the final destination for the `scalar` command.
And you can't bypass a final destination. That simply makes no sense.
I'm right now using a "scalar" installed on my system based on the diff
at the end of this E-Mail that changes the *build system* without any
user-facing changes (see
https://lore.kernel.org/git/87k0jhn0p9.fsf@evledraar.gmail.com/) for a
previous reference.
$ which scalar
/home/avar/local/bin/scalar
$ scalar -h 2>&1 | head -n 1
usage: scalar [-C <directory>] [-c <key>=<value>] <command> [<options>]
$ man scalar
[...]
NAME
scalar - an opinionated repository management tool
$ git help scalar
No manual entry for gitscalar
Except that I think your patches as they stand (correct me if I'm wrong)
don't have any way to install it or its documentation, just to build it
in-place.
So why bother with contrib/ at all? you may ask. The reason is that it
makes it substantially easier for me to move the features into core Git,
as I can incrementally implement those new options for Git's built-ins,
use them in `contrib/scalar/` instead of duplicating the functionality,
and then make use of Scalar's Functional Test suite for a much more
comprehensive testing (which has served us already really well in the
past). It also doesn't hurt that this way, my day job will be very happy
because Scalar users directly benefit from that work.
I think all of that summarizes "why have this live in git.git", which I
100% agree with. It's not a counter-argument to a working solution for
simplifying your proposed build system integration.
Of course, these suggestions to integrate Scalar more into the core part
of Git (missing the point that the final destination for the functionality
is not a new built-in, but rather new options for existing built-ins) made
everything much more cumbersome for me instead, for no gain that would be
apparent to me, impeding on aforementioned ease to move the features into
core Git (which has not happened yet, as a consequence), but hopefully
this will soon be a thing of the past.
The diff below doesn't make scalar a built-in.
So I would like to request that we close the discussion about the question
whether to integrate Scalar more into the top-level Makefile or into
git.c, and instead go ahead with keeping the `scalar` command in
contrib/scalar/. The freed-up time can then be used to focus on the much
more rewarding project of upstreaming Scalar's functionality such as
teaching `git clone` a short-and-sweet option that Just Makes Sense for
large monorepos (i.e. that imitates at least a large part of what `scalar
clone` does right now).
The reason I care about this is because duplicating this as a one-off
may be easier for you now, but it creates a lot of maintenance burden
for others down the line.
For example, I've been fixing memory leaks recently and have a
linux-leaks CI job that's now running on GitHub. Scalar would benefit
from having a t/t9099-scalar.sh run as part of that.
Yes you can special-case it somehow and teach ci/, or even "make test"
to do the same thing. But there's a lot of little things both current
and future that are going to be like that. Here's another one:
$ make -C contrib/scalar scalar
$ echo bad >cache.h
$ make -C contrib/scalar scalar
make: 'scalar' is up to date.
I.e. because it's using its own Makefile it's not getting the very
basics of the dependency graph right. My version?
$ make scalar
$ touch cache.h
$ make scalar
[... Works exactly as well as doing the same with "git"...]
Yes we could fix that and other things etc, but why not just get all
that for free in around 1/4 lines of Makefile boilerplate (and less than
that in complexity)?
.gitignore | 1 +
Documentation/Makefile | 3 ++
{contrib/scalar => Documentation}/scalar.txt | 4 +-
Makefile | 44 +++++++++++-----
contrib/scalar/.gitignore | 5 --
contrib/scalar/Makefile | 57 --------------------
contrib/scalar/t/Makefile | 78 ----------------------------
contrib/scalar/scalar.c => scalar.c | 0
{contrib/scalar/t => t}/t9099-scalar.sh | 8 +--
9 files changed, 37 insertions(+), 163 deletions(-)
[Note that this probably doesn't fully apply to your series/master, not
because of any great textual/semantic conflict, but just because I find
it convenient to stack my WIP/unsubmitted serieses in related areas on
top of one another. This is on top of other Makefile changes I've got
unsubmitted, but it would be easy/trivial to re-apply on master+your
series].
@@ -18,6 +18,9 @@ MAN1_TXT += $(filter-out \MAN1_TXT+=git.txtMAN1_TXT+=gitk.txtMAN1_TXT+=gitweb.txt+ifndef NO_INSTALL_SCALAR_DOC+MAN1_TXT+=scalar.txt+endif# man5 / man7 guides (note: new guides should also be added to command-list.txt)MAN5_TXT+=gitattributes.txt
diff --git a/contrib/scalar/scalar.txt b/Documentation/scalar.txtsimilarity index 99%rename from contrib/scalar/scalar.txtrename to Documentation/scalar.txtindex 3a80f829edc..f287ab18c31 100644--- a/contrib/scalar/scalar.txt+++ b/Documentation/scalar.txt
@@ -149,6 +149,6 @@ SEE ALSO -------- linkgit:git-clone[1], linkgit:git-maintenance[1].-Scalar+GIT ----Associated with the linkgit:git[1] suite+Part of the linkgit:git[1] suite
@@ -807,6 +808,7 @@ BUILT_INS += $(BUILT_INS_EXTRA)# what 'all' will build but not install in gitexecdirOTHER_PROGRAMS=git$X+OTHER_PROGRAMS+=scalar$XARTIFACTS_TAR+=$(OTHER_PROGRAMS)# what test wrappers are needed and 'install' will install, in bindir
@@ -818,12 +820,18 @@ BINDIR_PROGRAMS_NEED_X += git-upload-packBINDIR_PROGRAMS_NO_X+=git-cvsserver+ifdef INSTALL_SCALAR+BINDIR_PROGRAMS_NEED_X+=scalar+endifINSTALL_BINDIR_XPROGRAMS+=$(patsubst%,%$X,$(BINDIR_PROGRAMS_NEED_X))INSTALL_BINDIR_PROGRAMS+=$(INSTALL_BINDIR_XPROGRAMS)$(BINDIR_PROGRAMS_NO_X)# We have bin-wrappers for programs that we don't installTEST_BINDIR_PROGRAMS_NEED_X+=$(BINDIR_PROGRAMS_NEED_X)TEST_BINDIR_PROGRAMS_NEED_X+=$(TEST_PROGRAMS_NEED_X)+ifndef INSTALL_SCALAR+TEST_BINDIR_PROGRAMS_NEED_X+=scalar$X+endifTEST_BINDIR_PROGRAMS+=$(TEST_BINDIR_PROGRAMS_NEED_X)TEST_BINDIR_PROGRAMS+=$(BINDIR_PROGRAMS_NO_X)
@@ -2230,7 +2238,7 @@ please_set_SHELL_PATH_to_a_more_modern_shell:shell_compatibility_test:please_set_SHELL_PATH_to_a_more_modern_shell-strip:$(BIN_PROGRAMS)git$X+strip:$(BIN_PROGRAMS)git$Xscalar$X$(STRIP)$(STRIP_OPTS)$^### Flags affecting all rules
diff --git a/contrib/scalar/scalar.c b/scalar.csimilarity index 100%rename from contrib/scalar/scalar.crename to scalar.cdiff --git a/contrib/scalar/t/t9099-scalar.sh b/t/t9099-scalar.shsimilarity index 94%rename from contrib/scalar/t/t9099-scalar.shrename to t/t9099-scalar.shindex 7e8771d0eff..f686ba5d84c 100755--- a/contrib/scalar/t/t9099-scalar.sh+++ b/t/t9099-scalar.sh
@@ -2,13 +2,7 @@test_description='test the `scalar` command'-TEST_DIRECTORY=$PWD/../../../t-exportTEST_DIRECTORY--# Make it work with --no-bin-wrappers-PATH=$PWD/..:$PATH--.../../../t/test-lib.sh+../test-lib.shGIT_TEST_MAINT_SCHEDULER="crontab:test-tool crontab ../cron.txt"exportGIT_TEST_MAINT_SCHEDULER