Re: [PATCH RFC] git describe without refs distinguishes dirty working tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jean Privat <jean@xxxxxxxxx> writes:

>> I still haven't heard anything that helps me to decide which way the
>> default should be.  The only concrete thing I have heard against the
>> change of the default is that it will break existing setup, but I haven't
>> heard anything concrete for the change yet.
>
> As a said in the comment part of the initial message, I initially
> planed to add a --worktree option that means "I want you to describe
> my working tree". Knowing that from my naive point of view, the
> description of the working tree is what build script wanted :
> description of HEAD (on which is based the working tree) + saying that
> if the working tree is dirty or not (done manually by scripts).
> Moreover, in my naive view with the "--worktree" option, no refs where
> allowed (i.e. describe the working tree xor describe some commit
> references).

I do not think it is naive at all.  "git describe --worktree HEAD^" won't
make any sense in that world view; by definition the work tree is not
derived from anything but HEAD.

> Then, I realized that for some other git commands that can work both
> on the working tree and on an arbitrary commit reference, the default
> was to work on the working tree and require an explicit HEAD to work
> on the HEAD commit.

But the thing is, "git describe" without arguments already works on HEAD
and describes it, and people depend on the behaviour.

I originally thought this _might_ break and hoped it won't be a big issue,
but now I've seen that even the kernel would break (it runs the command
without saying "HEAD"), I do not want to risk breaking other projects I
may not even heard of.  Some people might have copied our GIT-VERSION-GEN
(that says "HEAD"), but I would not bet against that many many more people
would have copied the use of "git describe" from the kernel build tree
than from us.  After all, they are more famous and established than we
are.  [*1*]

One line of argument that I would have found reasonable to defend the
change of the default is that "The --worktree (or --dirty) option is
cumbersome to type.  And in _this_ workflow (that I haven't imagined, but
still would be *very* valid and useful one---so anybody who argues along
this line needs to fill in the blank here), it is the best solution to ask
the "git describe" command about the state of the work tree from the
command line very often, and here is why (here is another blank to fill
in), because there is no other way to get at the information, and/or the
info given by other existing commands are suboptimal for these reasons
(here is another blank to fill in)."

Then we would need to weigh benefits (for the interactive use of a very
useful and often used form of the command) against downsides (for people
having to update their existing scripts).

For use by scripts, the argument against having to give an extra option to
get the new behaviour becomes much weaker than for interactive use, even
if the new behaviour may be more common.  The point of scripting is that
you can write it once and forget about it; if it requires "--worktree" in
order to omit an extra "diff --index HEAD", it is not such a big deal.

And it is important to realize that "can write it once and forget about
it" cuts both ways.  It should apply not only to the people who write
their scripts after this patch of yours is applied to git, but should
equally apply to the people who wrote their scripts long time ago,
expecting that they can safely forget about them once they wrote them,
relying on the existing behaviour of the command.  Changing the defaults
for them means we lied to them and they have to update their scripts.  In
other words, they cannot script and forget about it anymore.

An attitude to change the default lightly like that, be it in 1.7.0 or
not, will then later come back and haunt your scripts you write while
assuming that the behaviour your patch will bring in will stay forever.
Allowing such a casual attitude will lead to other people changing the
default equally casually in the future.

It is my job to say no, even when I 100% agree that the new behaviour
would have been the default one if we were inventing "git describe" from
scratch and there were no existing users [*2*].  We do not live in an
ideal world, and 1.7.0 is not a blank check to change everything in sight.

>> How about "describe --dirty" and "describe --dirty=-mod" (the latter
>> creates v1.6.5-15-gc274db7-mod")
>
> May be better than "--wortree" (especially because of the value part),
> but what happen with
>  $ git describe --dirty v1.2.1
> should it show an error, output "v1.2.1" anyway, or output
> "v1.2.1-dirty" if the working tree is different from v1.2.1 ?

I am actually fine with your --worktree option, especially after seeing
your much more clear (i.e. "the state of my work tree") explanation in the
beginning of your response.

But I think "git describe" working in this "describe the version in the
work tree in the point in history" mode should reject any explicit
revision argument; by definition the work tree is not derived from
anything but HEAD.


[Footnote]

*1* In addition to the "it would break the kernel" issue, there are some
projects that expect users to tweak files they ship before building
(e.g. makefiles and config.h and the like).  For these people, any and all
builds would be -dirty.  I expect these projects, when migrating to git,
would either update their build procedures (moving config.h to
config.h-sample or something), but another solution for them would be to
use "git describe" without the -dirty bits.  So they are another class of
people who may not want -dirty by default.

*2* It personally hurts in a case like this, but that is what maintainers
have to do.  The maintainer's job is not to be loved, but is largely to
protect existing users from the second system syndrome.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]