* Re: [PATCH] Proof-of-concept patch to remember what the detached HEAD was
From: Björn Steinbrink @ 2009-10-17 7:55 UTC (permalink / raw)
To: Julian Phillips
Cc: Daniel Barkalow, James Pickens, Jeff King, Junio C Hamano,
Nicolas Pitre, Jay Soffian, git
In-Reply-To: <alpine.LNX.2.00.0910161821230.30589@reaper.quantumfyre.co.uk>
On 2009.10.16 18:31:23 +0100, Julian Phillips wrote:
> On Fri, 16 Oct 2009, Bj?rn Steinbrink wrote:
> >On 2009.10.16 13:15:35 +0100, Julian Phillips wrote:
> >>How about:
> >>
> >>$ git checkout origin/master
> >>$ git fetch
> >>Refusing to fetch, as it would update a checkedout branch
> >>"git fetch -f" will force the update, but you will need to run "git
> >>reset --hard HEAD" to update your checkout to match.
> >
> >That would redefine -f (currently means "allow non-fast-forward
> >updates"), the flag that allows the checked out branch head to be
> >updated is -u, --update-head-ok, and is for internal use only.
> >
> >And suggesting "reset --hard" seems wrong, that just kills any
> >uncommitted changes.
>
> Ok, so the commands were wrong. Not important.
>
> It was the approach that I was trying to suggest rather than the
> actual commands. The point I was trying to make was how, as a user,
> I would be happy to git behave.
Your approach explicitly included "mess up the index/worktree state",
otherwise, "git fetch" would not have to tell the user that he has to do
a "git reset --hard HEAD". I honestly can't believe that you would be
happy with git messing up your work.
> So, I try to run fetch, git says "ooh, now that would be dangerous -
> you can force it happen by running "git foo", but you will then be
> in situation X, which you can then recover from by running "git
> bar", though you may need to run "git stash" to save any edits you
> have made" or something similar.
But why make "git fetch" with non-"obscure" refspecs dangerous to begin
with? If we detach but keep some extra information, there's no need to
make "git fetch" dangerous, _and_ we can still provide a command that
just fetches the most recent version of the "checked out" remote
tracking branch and checks that out. May it be another mode of operation
for "git pull" or some "git up" command or whatever.
> >And such uncommitted changes would be lost in the big "undo the fetch
> >update" diff. So you'd have to do:
> >git reset --soft HEAD@{1}
> >git checkout --merge HEAD@{1}
> >
> >to keep them, while updating to the new state of the remote tracking
> >branch. Not quite intuitive, is it?
>
> I don't care what git has to do, I'm talking about the user
> experience - if we have to write some new code to support it, that
> really isn't a terribly hard thing to do. UIs should be driven down
> from the user interaction not up from the implementation.
Those commands are those that git would have to show to you, instead of
"git reset --hard HEAD", i.e. they're what you, as the user, have to do.
And while "git reset --hard HEAD" might be remotely understandable to
the user, that command sequence is very unlikely to be understood by
most. And providing a command that does just this sequence is insane,
it's just a bandaid for git doing crap to your worktree/index state. So
let's better not start with git doing that crap at all.
My current "idea" (I don't think I'll have time to implement that any
time soon) is:
Keep some extra information in HEAD (or somewhere else) when HEAD is
detached about the ref that HEAD is "weakly" bound to. For example, "git
checkout origin/master" might create a weak binding to
refs/remotes/origin/master (for other [corner] cases, see the other
mail I wrote, in which I outlined some cases I considered interesting).
"git update" can use the branch.<name>.{remote,merge} setup, or
alternatively the "weak binding" information, to fetch from the right
remote, and checks whether a fast-forward of HEAD to the according
upstream branch head is possible. If so, it does a "git checkout --merge
<upstream>" (possibly leaving conflicts for the uncommitted changes,
just like "svn update"). If a fast-forward is not possible, it
complains, telling the user that he needs to use "git merge/rebase/pull"
instead, and might want to create a branch head, in case of a detached
HEAD.
If there's also going to be a rule that forbids commits on some kind of
detached HEAD, than the command could also tell the user (when a
fast-forward is not possible) that upstream possibly rewrote history,
and that the user might want to use "git checkout --merge <upstream>"
(or maybe "git update -f"?) to just go to the new upstream version (not
sure if that hint should be shown in addition to or instead of the "git
merge/rebase/pull" hint).
Björn
^ permalink raw reply
* Re: [PATCH] grep: do not segfault when -f is used
From: Junio C Hamano @ 2009-10-17 7:44 UTC (permalink / raw)
To: Matt Kraai; +Cc: git, gitster, Johannes Sixt
In-Reply-To: <1255702405-7050-1-git-send-email-kraai@ftbfs.org>
Matt Kraai <kraai@ftbfs.org> writes:
> "git grep" would segfault if its -f option was used because it would
> try to use an uninitialized strbuf, so initialize the strbuf.
>
> Thanks to Johannes Sixt <j.sixt@viscovery.net> for the help with the
> test cases.
>
> Signed-off-by: Matt Kraai <kraai@ftbfs.org>
Thanks, both of you.
^ permalink raw reply
* Re: [PATCH] Proof-of-concept patch to remember what the detached HEAD was
From: Junio C Hamano @ 2009-10-17 7:43 UTC (permalink / raw)
To: Christoph Bartoschek; +Cc: git, Daniel Barkalow
In-Reply-To: <m49nq6-uk5.ln1@burns.bruehl.pontohonk.de>
Christoph Bartoschek <bartoschek@gmx.de> writes:
[jc: added Daniel back to cc list; please do not cull the cc list without
good reason]
> Daniel Barkalow wrote:
>
>> The upshot of the messages should be:
>>
>> $ git checkout origin/master
>> Since you can't actually change "origin/master" yourself, you'll just
>> be sightseeing unless you create a local branch to hold new local work.
>>
>> $ git branch
>> * (not a local branch, but "origin/master")
>>
>> $ git commit
>> You've been sightseeing "origin/master". The commit can't change that
>> value, so your commit isn't held in any branch. If you want to create
>> a branch to hold it, here's how.
> ...
> But then I was not able to verify that the checkout indeed matched the
> 1.3.0-beta. "git status" and "git branch" did not help here.
This is not going to help you, but "git reflog" would have helped here.
The reason my suggesting "git reflog" now won't help you is because the
word "reflog" does not connect the question "how did I get here" unless
and until you know git already; in other words, it is not your fault that
you got lost, but it is showing a wart in the UI.
If the question you were asking was "does the files I have in my work tree
after issuing that scary checkout actually match origin/1.3.0-beta?", you
could have asked that question in a more direct way, and the command to do
so is "git diff origin/1.3.0-beta". I do not think this would be asking
the user to be doing something unreasonably unintuitive.
If the question you were asking was (and it was not, from the description
of your experience, but you could be in that situation when you "return
some weeks later") "how does the checked out history relate to 1.3.0-beta?",
then there is a way to ask the question in a very direct way, and the
command to do so is "git show-branch HEAD origin/1.3.0-beta" (or give the
same argument to "gitk").
Although it is not _so_ unreasonable to expect "git status" to show the
information, I suspect it would not be practical. After all, whenever
somebody is lost, everything is "status". For a person who is lost and
does not know where in the history he is, it might be reasonable to expect
"status" to give the relationship between your HEAD and some branch/tag,
while for another person who was hit by "git gui" complaining that he has
too many loose objects, it might be reasonable for him to expect "status"
to give the number of loose objects in the repository. IOW, "status" is
too broad a word and following the path to cram everything into "status"
so that any new person who gets lost can get necessary infor from the
command will unfortunately lead to insanity.
The second item in the Daniel's transcript above may be an improvement but
I think it is a wrong economy to record and show 'but "origin/master"'
(which cannot be correct forever and has to be invalidated once the user
starts committing or resetting) in the message. I am wondering if a
similar effect to help new users can be had by rewording the message to:
$ git branch
* (not a local branch; see "git reflog" to learn how you got here)
The user can see how he got there even after doing something else after
the checkout (see Nico's write-up in $gmane/130527). The difference is
between giving fish and teaching how to catch one himself.
^ permalink raw reply
* Re: [PATCH] Proof-of-concept patch to remember what the detached HEAD was
From: Sean Estabrooks @ 2009-10-17 7:24 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Nicolas Pitre, Jeff King, Junio C Hamano, Daniel Barkalow,
Jay Soffian, git
In-Reply-To: <alpine.DEB.1.00.0910160357370.4985@pacific.mpi-cbg.de>
On Fri, 16 Oct 2009 04:07:23 +0200 (CEST)
Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Just recently, I had a user request (a very valid one, mind you) where the
> user does not want to provide a commit message, and wants to just commit
> all the current changes. In that particular case, it is very sensible to
> ask for these things. It is something utterly simple to ask for. Yet, it
> is utterly hard with Git, especially if I have to explain it.
Hey Johannes,
It's actually easy, but maybe hard to find:
$ git commit --cleanup=verbatim -m ""
Cheers,
Sean
^ permalink raw reply
* Re: [PATCH] Proof-of-concept patch to remember what the detached HEAD was
From: Christoph Bartoschek @ 2009-10-16 22:36 UTC (permalink / raw)
To: git
In-Reply-To: <alpine.LNX.2.00.0910140037570.32515@iabervon.org>
Daniel Barkalow wrote:
> The upshot of the messages should be:
>
> $ git checkout origin/master
> Since you can't actually change "origin/master" yourself, you'll just
> be sightseeing unless you create a local branch to hold new local work.
>
> $ git branch
> * (not a local branch, but "origin/master")
>
> $ git commit
> You've been sightseeing "origin/master". The commit can't change that
> value, so your commit isn't held in any branch. If you want to create
> a branch to hold it, here's how.
I appreciate such a message for git branch as a user. Today I had the
following problem:
I wanted to compile Qt Creator 1.3 from their repository. I cloned it with
git clone and then issued git checkout origin/1.3.0-beta. First there came a
the warning and I was not sure whether the checkout succeeded at all. I had
to ask in the IRC channel whether the checkout was ok.
But then I was not able to verify that the checkout indeed matched the
1.3.0-beta. "git status" and "git branch" did not help here.
Having an improved "git branch" would help a lot, especially if one returns
some weeks later to the directory and wants to know what the checkout is.
Christoph
^ permalink raw reply
* Re: Introduction and Wikipedia and Git Blame
From: jamesmikedupont @ 2009-10-17 6:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7v3a5irkel.fsf@alter.siamese.dyndns.org>
Thank you very much for your input and advice,
I have a lot of learn about this great tool.
I am working on learning how the existing blame tool runs now.
Will report back when I have some code.
mike
On Sat, Oct 17, 2009 at 1:25 AM, Junio C Hamano <gitster@pobox.com> wrote:
> "jamesmikedupont@googlemail.com" <jamesmikedupont@googlemail.com> writes:
>
>> What do you think of my idea to create blames along a specific user
>> defined byte positions ?
>
> Overly complicated and not enough time for _review_. If you are blaming
> one-byte (or one-char) per line, wouldn't it be enough to consider the
> line number in the output as byte (or char) position when reconstituting
> the original text?
>
^ permalink raw reply
* Re: [PATCH v3 0/5] Pretty formats for reflog data
From: Jeff King @ 2009-10-17 1:18 UTC (permalink / raw)
To: Thomas Rast; +Cc: Junio C Hamano, Jef Driesen, Nanako Shiraishi, git
In-Reply-To: <cover.1255701207.git.trast@student.ethz.ch>
On Fri, Oct 16, 2009 at 04:20:32PM +0200, Thomas Rast wrote:
> I tried for some time, but all attempts at interrupting the lists
> ended up terminating it again, so that the %g family list would not
> line up with the rest of the parameters. Having the note there would
> be nice, but I think keeping the list together optically is more
> important. However, AFAICS it really is the first character that only
> works with certain options (%m makes little sense without A...B, but
> still expands to >).
Another example is "%d", but there we load decorations on the fly the
first time it is used. I don't think that's a good idea here, since
using "%gd" shouldn't automatically imply "-g", which drastically
changes the nature of the command.
The "%m" behavior does what I would expect. If you haven't done any
limiting, then there can be no "left" or "boundary" entries.
I am tempted to do a note like the one below, but maybe the behavior is
obvious enough to users that it isn't necessary.
> Looking at it did make me notice that @{1} is invalid asciidoc and
> needs to be spelled @\{1\} though :-)
Thanks. I am often very guilty of not actually building documentation
when reviewing patches, because it is so tedious to build and examine.
---
diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 6359272..43192af 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -135,6 +135,12 @@ The placeholders are:
- '%n': newline
- '%x00': print a byte from a hex code
+NOTE: Some placeholders may depend on other options given to the
+revision traversal engine. For example, the `%g*` reflog options will
+insert an empty string unless we are traversing reflog entries (e.g., by
+`git log -g`). The `%d` placeholder will use the "short" decoration
+format if `--decorate` was not already provided on the command line.
+
* 'tformat:'
+
The 'tformat:' format works exactly like 'format:', except that it
^ permalink raw reply related
* Re: [PATCH v3 0/5] Pretty formats for reflog data
From: Junio C Hamano @ 2009-10-17 0:50 UTC (permalink / raw)
To: Thomas Rast; +Cc: Jeff King, Jef Driesen, Nanako Shiraishi, git
In-Reply-To: <cover.1255701207.git.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> Next round :-)
Very nicely done; I like it.
Thanks.
^ permalink raw reply
* Re: [PATCH] am: allow some defaults to be specified via git-config
From: Junio C Hamano @ 2009-10-17 0:50 UTC (permalink / raw)
To: Sam Vilain; +Cc: git, Nigel McNie
In-Reply-To: <1255650627-17576-1-git-send-email-sam.vilain@catalyst.net.nz>
Sam Vilain <sam.vilain@catalyst.net.nz> writes:
> Some users prefer in particular '3way' to be the default, let them
> specify it via the config file - and some other boolean settings while
> we're at it.
I have to wonder how this will interact with the internal call rebase
makes into am. Would there be unexpected fallouts?
^ permalink raw reply
* Re: [PATCH RFC] git describe without refs distinguishes dirty working tree
From: Nicolas Pitre @ 2009-10-17 0:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jean Privat, Shawn O. Pearce, git
In-Reply-To: <7vvdifq6vw.fsf@alter.siamese.dyndns.org>
On Fri, 16 Oct 2009, Junio C Hamano wrote:
> 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.
OpenOCD is another project that just started using Git and copied
describe usage from Linux.
Nicolas
^ permalink raw reply
* Re: [PATCH 3/3] Add proof-of-concept %[w(width,in1,in2)<<any-string>>%] implementation
From: Junio C Hamano @ 2009-10-17 0:18 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Johannes Schindelin, git
In-Reply-To: <200910170200.03681.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> Why not
>
> %[w(-1,4,4)%s%+b]
Yeah. As you said "subject unwrapped and body indented" or something like
that, I excluded that part outside the indentation, but log output indents
everything, so placing both inside %[w] would be correct.
> (i.e. %+ is this empty line between subject and body, if it exists).
As %+ is to add LF iff the next expansion is non-empty, it must always be
followed by an expansion, i.e. %something. That means "%+%something" is
unnecessary verbose and "%+something" is enough. "%+anything" syntax,
just like "%[func()anything]" syntax, is not limited to any particular
expansion.
> The %+x seems a bit strange... but I guess implementing conditional
> expansion a la shell or rpn spec/queryformat would be out of question
> (i.e. %?s:+ )...
Why not?
I can see us doing something like %[conditional%|iftrue%|iffalse%] quite
easily, now we have the "nested" variant of the strbuf_expand()
infrastructure.
But that is not the primary focus of the two patch series I've
demonstrated so far.
^ permalink raw reply
* Re: [PATCH 3/3] Add proof-of-concept %[w(width,in1,in2)<<any-string>>%] implementation
From: Jakub Narebski @ 2009-10-17 0:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7v7huurkif.fsf@alter.siamese.dyndns.org>
On Sat, 17 Oct 2009, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>
> > But even with original w(<width>,<indent1>,<indent2>) we can get output
> > of bare "git log" using pretty format... well, almost; it would be the
> > same if there was ability to put infinite width, and there doesn't seem
> > to be specifier for the whole, unchanged commit message (subject,
> > unwrapped + separating lines + body).
>
> I think I already discussed this when I sent out %s%+b patch. You would
> need to adjust and apply both series, but essentially it would become
> something like:
>
> %s%+[w(-1,4,4)%b]
>
> I.e. a single subject line, potentially followed by a LF and body indented
> by 4-place, but the LF will be there only when the body is not empty.
Why not
%[w(-1,4,4)%s%+b]
Or is it
%[w(-1,4,4)%s%+%b]
(i.e. %+ is this empty line between subject and body, if it exists).
The %+x seems a bit strange... but I guess implementing conditional
expansion a la shell or rpn spec/queryformat would be out of question
(i.e. %?s:+ )...
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: Introduction and Wikipedia and Git Blame
From: Junio C Hamano @ 2009-10-16 23:25 UTC (permalink / raw)
To: jamesmikedupont@googlemail.com; +Cc: Junio C Hamano, Johannes Schindelin, git
In-Reply-To: <ee9cc730910161419x608f5972x705ce8088d72c94a@mail.gmail.com>
"jamesmikedupont@googlemail.com" <jamesmikedupont@googlemail.com> writes:
> What do you think of my idea to create blames along a specific user
> defined byte positions ?
Overly complicated and not enough time for _review_. If you are blaming
one-byte (or one-char) per line, wouldn't it be enough to consider the
line number in the output as byte (or char) position when reconstituting
the original text?
^ permalink raw reply
* Re: [PATCH 3/3] Add proof-of-concept %[w(width,in1,in2)<<any-string>>%] implementation
From: Junio C Hamano @ 2009-10-16 23:23 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Junio C Hamano, Johannes Schindelin, git
In-Reply-To: <200910170020.01756.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> I think better solution might be to give _string_ to use for initial and
> subsequent indent rather than number of spaces... well, at least more
> generic one, allowing one to use e.g. "\t" (TAB) character to indent,
> or indent in the following way:
I do not see there is anything _better_ or _worse_. It may simply be a
useful _addition_. I'd usually say "care to send in a patch?" here, but
given that the general framework and the "wrap like the shortlog" hasn't
been plugged-in yet, there is no need to rush.
> But even with original w(<width>,<indent1>,<indent2>) we can get output
> of bare "git log" using pretty format... well, almost; it would be the
> same if there was ability to put infinite width, and there doesn't seem
> to be specifier for the whole, unchanged commit message (subject,
> unwrapped + separating lines + body).
I think I already discussed this when I sent out %s%+b patch. You would
need to adjust and apply both series, but essentially it would become
something like:
%s%+[w(-1,4,4)%b]
I.e. a single subject line, potentially followed by a LF and body indented
by 4-place, but the LF will be there only when the body is not empty.
^ permalink raw reply
* Re: [RFC PATCH v3 00/17] Return of smart HTTP
From: Shawn O. Pearce @ 2009-10-16 23:16 UTC (permalink / raw)
To: Mark Lodato; +Cc: git
In-Reply-To: <ca433830910161604g5a6bde76n26eb2b1e8155fb36@mail.gmail.com>
Mark Lodato <lodatom@gmail.com> wrote:
> The gitweb part is just bonus. (The only thing for gitweb is the one
> ScriptAlias line.) The real challenge is getting git-http-backend to
> serve repositories out of something other than DocumentRoot - say
> DocumentRoot is /var/www/htdocs but your git repositories are in
> /pub/git - which is why I posted the configuration. If you'd like, I
> can send you a patch to add this to the documentation.
Yes, a patch would be wonderful.
> One idea to improve the situation is to first try
> $GIT_PROJECT_ROOT$PATH_INFO, falling back to $PATH_TRANSLATED if
> $GIT_PROJECT_ROOT is empty. This would make the configuration simple:
>
> -- 8< --
> SetEnv GIT_PROJECT_ROOT /pub/git
> ScriptAlias /git /usr/libexec/git-core/git-http-backend
> -- >8 --
Oooh. Perhaps a good idea. :-)
--
Shawn.
^ permalink raw reply
* Re: [RFC PATCH v3 00/17] Return of smart HTTP
From: Mark Lodato @ 2009-10-16 23:04 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20091016143154.GS10505@spearce.org>
I just realized I forgot to say something in my last email: THANK
YOU!!! I have been looking forward to this for a long time. I was
planning to one day to sit down and start thinking about how to
implement a smart protocol, and then I see a post saying that not only
has someone figured out the protocol, but he has implemented it!
Amazing! This is really crucial for corporate adoption, at least at
my job. We need to have strong authentication for many users, and the
SSH key management is just a nightmare, not to mention that not all
users can SSH due to firewalls. This will save me so much time and
frustration. Thanks!
On Fri, Oct 16, 2009 at 10:31 AM, Shawn O. Pearce <spearce@spearce.org> wrote:
> If users don't want to upgrade, or can't upgrade, then you can't
> push over HTTP. Simple.
Yeah, I realized after I wrote my previous email that it probably
doesn't matter - the current state of HTTP push is so awful that no
one probably uses it!
> Really, what it comes down to is, I don't think it matters that
> we don't have backwards compatiblity for pushing through WebDAV.
> If you think it matters, you are free to write a patch series on
> top of mine which adds the functionality. But don't wait for me
> to do it, it won't happen.
Agreed. I was just wondering if it was on the to-do list for the far
off future. Either way, it should probably be noted in the
documentation that dumb push is not supported.
>> Also, your examples use "DocumentRoot /pub/git", but I think most
>> people would want to have their main website as the DocumentRoot, have
>> the URL "/git" serve the repositories through gitweb, and have that
>> same "/git" URL be `git clone'-able.
>
> Why not have git-http-backend exec gitweb when it gets a request
> for the repository itself? Why do you have to go through such
> contortions in Apache for this? The two CGIs are shipped in the
> same software package, surely one could actually invoke the other.
>
> [snip]
>
> Yes, I'd like to have examples in the git-http-backend manpage.
> I put a couple in there already, but they don't consider gitweb
> because I assumed we'd find a way to have gitweb be invoked out
> of git-http-backend. Unfortunately that hasn't happened yet.
The gitweb part is just bonus. (The only thing for gitweb is the one
ScriptAlias line.) The real challenge is getting git-http-backend to
serve repositories out of something other than DocumentRoot - say
DocumentRoot is /var/www/htdocs but your git repositories are in
/pub/git - which is why I posted the configuration. If you'd like, I
can send you a patch to add this to the documentation.
One idea to improve the situation is to first try
$GIT_PROJECT_ROOT$PATH_INFO, falling back to $PATH_TRANSLATED if
$GIT_PROJECT_ROOT is empty. This would make the configuration simple:
-- 8< --
SetEnv GIT_PROJECT_ROOT /pub/git
ScriptAlias /git /usr/libexec/git-core/git-http-backend
-- >8 --
As far as having git-http-backend launch an external process such as
gitweb, I personally don't think this is important enough to block
this from continuing. One could configure the webserver as I have in
my previous email until such a feature is implemented. For cgit, the
solution is simple - just integrate git-http-backend into the
executable.
Regards,
Mark
^ permalink raw reply
* Re: [PATCH RFC] git describe without refs distinguishes dirty working tree
From: Junio C Hamano @ 2009-10-16 23:02 UTC (permalink / raw)
To: Jean Privat; +Cc: Shawn O. Pearce, git
In-Reply-To: <dffdbd190910161452o4ac0b426i7c48649eafa0d53@mail.gmail.com>
Jean Privat <jean@pryen.org> 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.
^ permalink raw reply
* Re: [PATCH RFC] git describe without refs distinguishes dirty working tree
From: Shawn O. Pearce @ 2009-10-16 22:37 UTC (permalink / raw)
To: Jean Privat; +Cc: Junio C Hamano, git
In-Reply-To: <dffdbd190910161452o4ac0b426i7c48649eafa0d53@mail.gmail.com>
Jean Privat <jean@pryen.org> wrote:
> > 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.
...
> 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. Thus it makes sense to me that "git describe"
> alone should describe the working tree and that "git describe HEAD"
> should describe the HEAD commit.
Yup. That's my take on it too. This default of "no argument means
describe the working tree" matches with tools like `git diff`,
`git checkout`, `git status`, `git blame` with no revision arguments.
We are being blasted by users for being inconsistent in our UI in too
many places. Here's yet another. We need to start standardizing
on a more consistent UI model. If that model means we need to
use a "--worktree" flag to mean "against the working tree" then
we should start doing that also to `git status`, `git checkout`,
`git blame`, and `git diff`.
> $ 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 ?
IMHO, that should be a fatal usage error, if we go that approach.
I would also argue `git describe --dirty HEAD` is equally fatal.
--
Shawn.
^ permalink raw reply
* Re: [PATCH 3/3] Add proof-of-concept %[w(width,in1,in2)<<any-string>>%] implementation
From: Jakub Narebski @ 2009-10-16 22:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7v3a5jupr7.fsf@alter.siamese.dyndns.org>
On Fri, 16 Oct 2009, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>
> > I don't remember what were original parameters to w(72,4,8) means...
>
> "man git-shortlog" look for -w.
Thanks. So those would be:
-w[<width>[,<indent1>[,<indent2>]]]::
Linewrap the output by wrapping each line at `width`. The first
line of each entry is indented by `indent1` spaces, and the second
and subsequent lines are indented by `indent2` spaces. `width`,
`indent1`, and `indent2` default to 76, 6 and 9 respectively.
I think better solution might be to give _string_ to use for initial and
subsequent indent rather than number of spaces... well, at least more
generic one, allowing one to use e.g. "\t" (TAB) character to indent,
or indent in the following way:
* Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod
tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim
veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea
commodo consequat. [...]
But even with original w(<width>,<indent1>,<indent2>) we can get output
of bare "git log" using pretty format... well, almost; it would be the
same if there was ability to put infinite width, and there doesn't seem
to be specifier for the whole, unchanged commit message (subject,
unwrapped + separating lines + body).
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: Q: supplying large sets of path to git commands
From: Junio C Hamano @ 2009-10-16 22:08 UTC (permalink / raw)
To: Constantine Plotnikov; +Cc: Git Mailing List
In-Reply-To: <85647ef50910160340s5b608d9cmb13679d7c6a987cd@mail.gmail.com>
Constantine Plotnikov <constantine.plotnikov@gmail.com> writes:
> Some git commands like git check-attr supports receiving paths from
> stdin.
Here is how one might implement it for diff/log family of commands that
use "setup_revisions()".
I didn't test it (of course) beyond running
./git diff --name-only HEAD | ./git diff --stdin-paths --stat -p
in order to include the patch text in this message, and running
./git diff --name-only | ./git log --stdin-paths --no-merges --stat
---
Documentation/git-rev-list.txt | 1 +
Documentation/rev-list-options.txt | 7 ++++
revision.c | 59 +++++++++++++++++++++++++++++++++--
3 files changed, 63 insertions(+), 4 deletions(-)
diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index 3341d1b..8fa276c 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -46,6 +46,7 @@ SYNOPSIS
[ \--reverse ]
[ \--walk-reflogs ]
[ \--no-walk ] [ \--do-walk ]
+ [ \--stdin-paths | \--stdin-paths-z ]
<commit>... [ \-- <paths>... ]
DESCRIPTION
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index bf66116..9960de8 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -317,6 +317,13 @@ The following options select the commits to be shown:
Commits modifying the given <paths> are selected.
+--stdin-paths::
+--stdin-paths-z::
+
+ Additionally read <paths> (see above) from the standard input,
+ one path per line terminated with linefeed (or NUL when
+ --stdin-paths-z is used).
+
--simplify-by-decoration::
Commits that are referred by some branch or tag are selected.
diff --git a/revision.c b/revision.c
index 9fc4e8d..4caccdb 100644
--- a/revision.c
+++ b/revision.c
@@ -1218,6 +1218,38 @@ void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,
ctx->argc -= n;
}
+enum append_stdin_mode {
+ APPEND_STDIN = 1,
+ APPEND_STDIN_Z = 2,
+};
+
+static const char **grab_pathspec(const char *prefix, const char **argv,
+ enum append_stdin_mode append_stdin)
+{
+ const char **nargv;
+ int cnt, alloc, term;
+ struct strbuf line = STRBUF_INIT;
+
+ if (!append_stdin)
+ return get_pathspec(prefix, argv);
+
+ for (cnt = 0; argv[cnt]; cnt++)
+ ; /* first count */
+ alloc = alloc_nr(cnt);
+ nargv = xmalloc(sizeof(*nargv) * alloc);
+ for (cnt = 0; argv[cnt]; cnt++)
+ nargv[cnt] = argv[cnt]; /* then copy */
+
+ term = (append_stdin == APPEND_STDIN_Z) ? '\0' : '\n';
+ while (strbuf_getline(&line, stdin, term) != EOF) {
+ ALLOC_GROW(nargv, cnt, alloc);
+ nargv[cnt++] = strbuf_detach(&line, NULL);
+ }
+ ALLOC_GROW(nargv, cnt, alloc);
+ nargv[cnt] = NULL;
+ return get_pathspec(prefix, nargv);
+}
+
/*
* Parse revision information, filling in the "rev_info" structure,
* and removing the used arguments from the argument list.
@@ -1228,17 +1260,26 @@ void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,
int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def)
{
int i, flags, left, seen_dashdash;
+ enum append_stdin_mode append_stdin = 0;
- /* First, search for "--" */
+ /* First, search for "--", "--stdin-paths", and "--stdin-paths-z" */
seen_dashdash = 0;
for (i = 1; i < argc; i++) {
const char *arg = argv[i];
+
+ if (!strcmp(arg, "--stdin-paths"))
+ append_stdin = APPEND_STDIN;
+ else if (!strcmp(arg, "--stdin-paths-z"))
+ append_stdin = APPEND_STDIN_Z;
+
if (strcmp(arg, "--"))
continue;
argv[i] = NULL;
argc = i;
if (argv[i + 1])
- revs->prune_data = get_pathspec(revs->prefix, argv + i + 1);
+ revs->prune_data = grab_pathspec(revs->prefix,
+ argv + i + 1,
+ append_stdin);
seen_dashdash = 1;
break;
}
@@ -1283,6 +1324,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
revs->no_walk = 0;
continue;
}
+ if (!strcmp(arg, "--stdin-paths") ||
+ !strcmp(arg, "--stdin-paths-z"))
+ continue; /* already handled */
opts = handle_revision_opt(revs, argc - i, argv + i, &left, argv);
if (opts > 0) {
@@ -1308,12 +1352,19 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
for (j = i; j < argc; j++)
verify_filename(revs->prefix, argv[j]);
- revs->prune_data = get_pathspec(revs->prefix,
- argv + i);
+ revs->prune_data = grab_pathspec(revs->prefix,
+ argv + i,
+ append_stdin);
break;
}
}
+ if (!revs->prune_data && append_stdin) {
+ const char *empty_argv[1] = { NULL };
+ revs->prune_data = grab_pathspec(revs->prefix, empty_argv,
+ append_stdin);
+ }
+
if (revs->def == NULL)
revs->def = def;
if (revs->show_merge)
^ permalink raw reply related
* Re: [PATCH RFC] git describe without refs distinguishes dirty working tree
From: Jean Privat @ 2009-10-16 21:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn O. Pearce, git
In-Reply-To: <7veip3t6rm.fsf@alter.siamese.dyndns.org>
> 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).
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. Thus it makes sense to me that "git describe"
alone should describe the working tree and that "git describe HEAD"
should describe the HEAD commit.
Moreover, since build scripts use "git describe" not to describe an
arbitrary commit but to describe the working tree, it is nice to make
what thez really want the default.
Currently, the breaking of scripts is the only reason that makes me
not entirely convinced that default should be to describe the working
tree. But I think that if "git describe" could be redesigned,
describing the working tree by default should be the right thing.
I also think that the breakage should not be so important:
* it could be fixed without requiring/checking a specific git version
* it appears only for people that build on a dirty tree
* the effect may be only to produce a strange version number
* git describe is porcelain :)
* 1.7 may break other things, so some fixing me be required anyway :).
If git people prefers that default remains HEAD, I have no real strong
objection (I still have to resend the patch because of line breaking
anyway).
> 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 ?
-J
^ permalink raw reply
* Re: Introduction and Wikipedia and Git Blame
From: jamesmikedupont @ 2009-10-16 21:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vpr8nt894.fsf@alter.siamese.dyndns.org>
What do you think of my idea to create blames along a specific user
defined byte positions ?
please review my suggestion and comment.
mike
On Fri, Oct 16, 2009 at 10:05 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> "jamesmikedupont@googlemail.com" <jamesmikedupont@googlemail.com> writes:
>>
>>>> You would need to postprocess the computed result (either by diff or
>>>> blame) to lay out the final text output in either case anyway, and making
>>>> the existing blame engine do the work for you would be a better approach,
>>>> I think.
>>>
>>> Please can you tell me what is the basic algorithm of the blame engine?
>>
>> I think this is one of the most conprehensive write-up on the algorithm:
>>
>> http://thread.gmane.org/gmane.comp.version-control.git/28826/focus=28895
>>
>> The whole thread (at least what I wrote in it) is worth reading if you
>> want to understand what the current code does. The first message in the
>> thread talks about "NEEDSWORK" label on an unimplemented part of the code,
>> and says "we could", but these gaps were since filled.
>
> Ah, nevermind. The thread is the definitive description of the blame
> algorithm, but I agree with Dscho that in this case, you either have to
> change blame itself to do this "byte-wise" comparison internally between
> versions, or re-do the blame logic yourself like Dscho suggests. Dscho is
> right in this case; an unmodifled blame engine, unless you feed a history
> that is converted to use the byte-per-line format, won't help you at all.
>
> So it would be either between rolling a custom byte-wise blame algorithm
> yourself and teaching a new byte-wise mode to existing blame engine.
> Sorry for making the task sound much easier than it would be.
>
>
>
^ permalink raw reply
* Re: [PATCH] Proof-of-concept patch to remember what the detached HEAD was
From: Nicolas Pitre @ 2009-10-16 20:19 UTC (permalink / raw)
To: Julian Phillips
Cc: Junio C Hamano, Björn Steinbrink, Daniel Barkalow,
James Pickens, Jeff King, Jay Soffian, git
In-Reply-To: <alpine.LNX.2.00.0910162029460.31673@reaper.quantumfyre.co.uk>
On Fri, 16 Oct 2009, Julian Phillips wrote:
> My interest in this thread is solely that it might provide a mechanism to find
> out which tag was checked out. So, I'm just chucking in my $0.02 as a user.
Try this:
$ git checkout v1.5.5
Note: moving to 'v1.5.5' which isn't a local branch
If you want to create a new branch from this checkout, you may do so
(now or later) by using -b with the checkout command again. Example:
git checkout -b <new_branch_name>
HEAD is now at 1d2375d... GIT 1.5.5
[look around, and then ...]
$ git checkout HEAD~2
Previous HEAD position was 1d2375d... GIT 1.5.5
HEAD is now at f61cc48... git-svn: fix following renamed paths when tracking a single path
[go out for lunch ... and forget what this was about.]
$ git reflog -3
f61cc48 HEAD@{0}: checkout: moving from 1d2375d... to HEAD~2
1d2375d HEAD@{1}: checkout: moving from master to v1.5.5
c274db7 HEAD@{2}: pull : Fast forward
Here I have all the information to see what I did, and from what state.
I even know that I did a pull on the master branch before moving away
from it. The -3 limits the log to 3 entries. With no limit you get it
all in your default pager.
So there is no need for another mechanism to find out what tag was
actually checked out -- you have it all already.
Nicolas
^ permalink raw reply
* Re: [PATCH RFC] git describe without refs distinguishes dirty working tree
From: Junio C Hamano @ 2009-10-16 20:37 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Jean Privat, git
In-Reply-To: <20091016173902.GT10505@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> Yea, 1.7 may be safer for this, but also many scripts are based
> on the GIT-VERSION-GEN in git.git which uses HEAD as an explicit
> argument. So they might actually be OK with this change.
Well, from my cursory look of its scripts/setlocalversion, even the Linux
kernel is not safe X-<.
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.
^ permalink raw reply
* Re: [PATCH] Proof-of-concept patch to remember what the detached HEAD was
From: Junio C Hamano @ 2009-10-16 20:19 UTC (permalink / raw)
To: Julian Phillips
Cc: Björn Steinbrink, Daniel Barkalow, James Pickens, Jeff King,
Nicolas Pitre, Jay Soffian, git
In-Reply-To: <alpine.LNX.2.00.0910162029460.31673@reaper.quantumfyre.co.uk>
Julian Phillips <julian@quantumfyre.co.uk> writes:
> On Fri, 16 Oct 2009, Junio C Hamano wrote:
>
>> Julian Phillips <julian@quantumfyre.co.uk> writes:
>> ...
>>> I don't care what git has to do, I'm talking about the user experience
>>
>> But Bj?rn is showing two commands the _user_ has to type, iow, the comment
>> is about the user experience.
>
> Only Currently. My point was that _if_ we wanted to support this sort
> of thing, then we can make is simpler to do by providing a simple
> command for the user.
>
> The point I wanted to make was that the decision on what to do should
> be driven by the user's experience - not by the fact that it is easier
> to implement something else.
Sorry, but I do not see in what way what you said in the thread connects
to the above three lines. Are you talking about this one from you a few
messages back?
How about:
$ git checkout origin/master
$ git fetch
Refusing to fetch, as it would update a checkedout branch
"git fetch -f" will force the update, but you will need to run "git
reset --hard HEAD" to update your checkout to match.
I am not seeing "not the implementation ease but the user experience"
drive in this suggestion. If you are driving from the user experience
point of view, I would have instead suggested:
How about:
$ git checkout origin/master
$ git fetch
and fetch happily updates the tracked branch, without affecting the
HEAD state (nor index nor the work tree, of course).
User experience here is:
* Let's checkout what the upstream has on the master. I am at least
aware that git is distributed and does not go to the network unless
I tell it to, and am aware that origin/master is the state of the
upsream _when I told git to look at it the last time_. So I'll get
the state I talked with the upstream the last time, and that is what
I want to look at.
* Hmm, there may be even more updates since I talked with the upstream
the last time. Let's tell git to fetch. I already know that this
updates origin/master to a more recent state.
Isn't it much cleaner and less confusing? And with these kind of
awareness, the user can choose to do various things from here, e.g.:
* Oh, fetch did fetched something. I want the updated state. Let's
check it out with "git checkout origin/master"; or
* Ok, what changed since I last talked with the upstream? I have one
end of the state already checked out, and I updated origin/master
with their latest, so I can say "git diff HEAD origin/master".
Notice in the latter the user can be a neophyte who hasn't learned reflogs
yet.
> My interest in this thread is solely that it might provide a mechanism
> to find out which tag was checked out.
Hmm, what is lacking in "git describe HEAD" for that? I am not
complaining that you might be asking for something that exists, but I _do_
want to know if something that exists is not a satisfactory solution and
if so how it can be improved.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox