Re: [PATCH] defer expensive load_ref_decorations until needed

[Phil Hord]

On Wed, Nov 22, 2017 at 1:27 PM, Jeff King <p...@peff.net> wrote:
> On Tue, Nov 21, 2017 at 03:43:36PM -0800, Phil Hord wrote:
>
>> With many thousands of references, a simple `git rev-parse HEAD` may take
>> more than a second to return because it first loads all the refs into
>> memory even though it will never use them.
>
> The overall goal of lazy-loading seems reasonable, but I'm slightly
> confused: how and why does "git rev-parse HEAD" load ref decorations?
>
> Grepping around I find that we mostly load them only when appropriate
> (when the "log" family sees a decorate option, when we see %d/%D in a
> pretty format, or with --simplify-by-decoration in a traversal). And
> poking at "rev-parse HEAD" in gdb seems to confirm that it does not hit
> that function.

Hm. I think I was confused.

I wrote v1 of this patch a few months ago. Clearly I was wrong about
rev-parse being afflicted.  We have a script that was suffering and it
uses both "git log --format=%h" and "git rev-parse" to get hashes; I
remember testing both, but I can't find it in my $zsh_history; my
memory and my commit-message must be faulty.

However, "git log" does not need any --decorate option to trigger this lag.

$ git for-each-ref| wc -l
24172
$ time git log --format=%h -1
git log --format=%h -1   0.47s user 0.04s system 99% cpu 0.509 total

I grepped the code just now, too, and I see the same as you, though;
it seems to hold off unless !!decoration_style.  Nevertheless, gdb
shows me decoration_style=1 with this command:

GIT_CONFIG=/dev/null cgdb --args git log -1 --format="%h"

Here are timing tests on this repo without this change:

git log --format=%h -1 0.54s user 0.05s system 99% cpu
0.597 total
git log --format=%h -1 --decorate  0.54s user 0.04s system 98% cpu
0.590 total
git log --format=%h%d -1   0.53s user 0.05s system 99% cpu
0.578 total

And the same commands with this change:

git log --format=%h -1  0.01s user 0.01s system 71%
cpu 0.017 total
git log --format=%h -1 --decorate   0.00s user 0.01s system 92%
cpu 0.009 total
git log --format=%h%d -10.53s user 0.09s system 88%
cpu 0.699 total

> I have definitely seen "rev-parse HEAD" be O(# of refs), but that is
> mostly attributable to having all the refs packed (and until v2.15.0,
> the packed-refs code would read the whole file into memory).

Hm.  Could this be why rev-parse was slow for me?  My original problem
showed up on v1.9 (build machine) and I patched it on v2.14.0-rc1.
But, no; testing on 1.9, 2.11 and 2.14 still doesn't show me the lag
in rev-parse.  I remain befuddled.

> I've also
> seen unnecessary ref lookups due to replace refs (we load al of the
> packed refs to find out that no, there's nothing in refs/replace).

I haven't seen this in the code, but I have had refs/replace hacks in
the past. Is that enough to wake this up?

Phil

[PATCH v2 2/2] stash-store: add failing test for same-ref

[Phil Hord]

stash-store cannot create a new stash with the same ref as stash@{0}. No
error is returned even though no new stash log is created. Add a failing
test to track.

Signed-off-by: Phil Hord <phil.h...@gmail.com>
---
 t/t3903-stash.sh | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 279e31717..7d511afd3 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -813,6 +813,17 @@ test_expect_success 'push -m also works without space' '
test_cmp expect actual
 '
 
+test_expect_failure 'store same ref twice' '
+   >foo &&
+   git add foo &&
+   STASH_ID=$(git stash create) &&
+   git stash store -m "original message" $STASH_ID &&
+   git stash store -m "custom message" $STASH_ID &&
+   echo "stash@{0}: custom message" >expect &&
+   git stash list -1 >actual &&
+   test_cmp expect actual
+'
+
 test_expect_success 'store -m foo shows right message' '
>foo &&
git add foo &&
-- 
2.15.0.471.g17a719cfe.dirty

[PATCH v2] Teach stash to parse -m/--message like commit does

[Phil Hord]

`git stash push -m foo` uses "foo" as the message for the stash. But
`git stash push -m"foo"` does not parse successfully.  Similarly
`git stash push --message="My stash message"` also fails.  The stash
documentation doesn't suggest this syntax should work, but gitcli
does and my fingers have learned this pattern long ago for `commit`.

Teach `git stash` and `git store` to parse -mFoo and --message=Foo
the same as `git commit` would do.  Even though it's an internal
function, add similar support to create_stash() for consistency.

Reviewd-by: Junio C Hamano <gits...@pobox.com>
Signed-off-by: Phil Hord <phil.h...@gmail.com>
---

Added tests for 'stash push' and 'stash store'.
Added a note that create_stash is included but unnecessary.

 git-stash.sh | 18 +++
 t/t3903-stash.sh | 93 
 2 files changed, 111 insertions(+)

diff --git a/git-stash.sh b/git-stash.sh
index 4b7495144..1114005ce 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -76,6 +76,12 @@ create_stash () {
shift
stash_msg=${1?"BUG: create_stash () -m requires an 
argument"}
;;
+   -m*)
+   stash_msg=${1#-m}
+   ;;
+   --message=*)
+   stash_msg=${1#--message=}
+   ;;
-u|--include-untracked)
shift
untracked=${1?"BUG: create_stash () -u requires an 
argument"}
@@ -193,6 +199,12 @@ store_stash () {
shift
stash_msg="$1"
;;
+   -m*)
+   stash_msg=${1#-m}
+   ;;
+   --message=*)
+   stash_msg=${1#--message=}
+   ;;
-q|--quiet)
quiet=t
;;
@@ -251,6 +263,12 @@ push_stash () {
test -z ${1+x} && usage
stash_msg=$1
;;
+   -m*)
+   stash_msg=${1#-m}
+   ;;
+   --message=*)
+   stash_msg=${1#--message=}
+   ;;
--help)
show_help
;;
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 3b1ac1971..39c7f2ebd 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -804,6 +804,99 @@ test_expect_success 'push -m shows right message' '
test_cmp expect actual
 '
 
+test_expect_success 'push -m also works without space' '
+   >foo &&
+   git add foo &&
+   git stash push -m"unspaced test message" &&
+   echo "stash@{0}: On master: unspaced test message" >expect &&
+   git stash list -1 >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'store -m foo shows right message' '
+   git stash clear &&
+   git reset --hard &&
+   echo quux >bazzy &&
+   git add bazzy &&
+   STASH_ID=$(git stash create) &&
+   git stash store -m "store m" $STASH_ID &&
+   echo "stash@{0}: store m" >expect &&
+   git stash list -1 >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'store -mfoo shows right message' '
+   git stash clear &&
+   git reset --hard &&
+   echo quux >bazzy &&
+   git add bazzy &&
+   STASH_ID=$(git stash create) &&
+   git stash store -m"store mfoo" $STASH_ID &&
+   echo "stash@{0}: store mfoo" >expect &&
+   git stash list -1 >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'store --message=foo shows right message' '
+   git stash clear &&
+   git reset --hard &&
+   echo quux >bazzy &&
+   git add bazzy &&
+   STASH_ID=$(git stash create) &&
+   git stash store --message="store message=foo" $STASH_ID &&
+   echo "stash@{0}: store message=foo" >expect &&
+   git stash list -1 >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'store --message foo shows right message' '
+   git stash clear &&
+   git reset --hard &&
+   echo quux >bazzy &&
+   git add bazzy &&
+   STASH_ID=$(git stash create) &&
+   git stash store --message "store message foo" $STASH_ID &&
+   echo "stash@{0}: store message foo" >expect &&
+   git stash list -1 >actual &&
+   test_cmp expe

Re: [PATCH] defer expensive load_ref_decorations until needed

[Phil Hord]

On Tue, Nov 21, 2017, Junio C Hamano  wrote:
> Junio C Hamano  writes:
>
> I am not sure if "maybe_" is a good name here, though.  If anything,
> you are making the semantics of "load_ref_decorations()" to "maybe"
> (but I do not suggest renaming that one).
>
> How about calling it to load_ref_decorations_lazily() or something?

I groped about for something conventional, but "..._gently" didn't fit
the bill, so I went with "maybe".  I like "lazily" better for this
case. I will change it for v2.

>> Other than that, I like what this patch attempts to do.  A nicely
>> identified low-hanging fruit ;-).
>
> Having said that, this will have a bad interaction with another
> topic in flight: <20171121213341.13939-1-rafa.al...@gmail.com>
>
> Perhaps this should wait until the other topic lands and stabilizes.
> We'd need to rethink if the approach taken by this patch, i.e. to
> still pass the info to load() but holding onto it until the time
> lazy_load() actually uses it, is a sensible way forward, or we would
> want to change the calling convention to help making it easier to
> implement the lazy loading.

I noticed that after just after cleaning this one up, but didn't look
closely yet.  I'll hold this in my local queue until ra lands.

P

Re: doc: prefer 'stash push' over 'stash save'

[Phil Hord]

You probably already noticed this was my fault for filtering the patch
through Gmail's GUI.  I did also push a replacement which hopefully
does apply.

On Tue, Nov 21, 2017 at 8:39 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Jonathan Nieder <jrnie...@gmail.com> writes:
>
>> Phil Hord wrote:
>>
>>> Although `git stash save` was deprecated recently, some parts of the
>>> documentation still refer to it instead of `push`.
>>>
>>> Signed-off-by: Phil Hord <phil.h...@gmail.com>
>>> ---
>>>  Documentation/git-stash.txt | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> Reviewed-by: Jonathan Nieder <jrnie...@gmail.com>
>> Thanks.
>
> Heh, this does not even apply to 8be661007 that it claims to apply
> on top of, which is contained in fd2ebf14 ("stash: mark "git stash
> save" deprecated in the man page", 2017-10-22).
>
> I've wiggled it in, so there is no need to resend, but next time
> please be careful when sending the patch and also when sending a
> reviewed-by.

[PATCH] defer expensive load_ref_decorations until needed

[Phil Hord]

With many thousands of references, a simple `git rev-parse HEAD` may take
more than a second to return because it first loads all the refs into
memory even though it will never use them.

Defer loading any references until we actually need them.

Signed-off-by: Phil Hord <phil.h...@gmail.com>
---
 log-tree.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index 3b904f037..c1509f8b9 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -84,8 +84,10 @@ void add_name_decoration(enum decoration_type type, const 
char *name, struct obj
res->next = add_decoration(_decoration, obj, res);
 }
 
+static void maybe_load_ref_decorations();
 const struct name_decoration *get_name_decoration(const struct object *obj)
 {
+   maybe_load_ref_decorations();
return lookup_decoration(_decoration, obj);
 }
 
@@ -150,10 +152,13 @@ static int add_graft_decoration(const struct commit_graft 
*graft, void *cb_data)
 
 void load_ref_decorations(int flags)
 {
-   if (!decoration_loaded) {
+   decoration_flags = flags;
+}
 
+static void maybe_load_ref_decorations()
+{
+   if (!decoration_loaded) {
decoration_loaded = 1;
-   decoration_flags = flags;
for_each_ref(add_ref_decoration, NULL);
head_ref(add_ref_decoration, NULL);
for_each_commit_graft(add_graft_decoration, NULL);
-- 
2.15.0.471.g17a719cfe.dirty

[PATCH] doc: prefer 'stash push' instead of 'stash save'

[Phil Hord]

Although `git stash save` was deprecated recently, some parts of the
documentation still refer to it.

Signed-off-by: Phil Hord <phil.h...@gmail.com>
---
 Documentation/git-stash.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 8be661007..056dfb866 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -175,14 +175,14 @@ create::
return its object name, without storing it anywhere in the ref
namespace.
This is intended to be useful for scripts.  It is probably not
-   the command you want to use; see "save" above.
+   the command you want to use; see "push" above.
 
 store::
 
Store a given stash created via 'git stash create' (which is a
dangling merge commit) in the stash ref, updating the stash
reflog.  This is intended to be useful for scripts.  It is
-   probably not the command you want to use; see "save" above.
+   probably not the command you want to use; see "push" above.
 
 DISCUSSION
 --
-- 
2.15.0.471.g17a719cfe.dirty

[PATCH] stash: Learn to parse -m/--message like commit does

[Phil Hord]

`git stash push -m foo` uses "foo" as the message for the stash. But
`git stash push -m"foo"` does not parse successfully.  Similarly
`git stash push --message="My stash message"` also fails.  Nothing
in the documentation suggests this syntax should work, but it does
work for `git commit`, and my fingers have learned this pattern long
ago.

Teach `git stash` to parse -mFoo and --message=Foo the same as
`git commit` would do.

Signed-off-by: Phil Hord <phil.h...@gmail.com>
---
 git-stash.sh | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/git-stash.sh b/git-stash.sh
index 4b7495144..1114005ce 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -76,6 +76,12 @@ create_stash () {
shift
stash_msg=${1?"BUG: create_stash () -m requires an 
argument"}
;;
+   -m*)
+   stash_msg=${1#-m}
+   ;;
+   --message=*)
+   stash_msg=${1#--message=}
+   ;;
-u|--include-untracked)
shift
untracked=${1?"BUG: create_stash () -u requires an 
argument"}
@@ -193,6 +199,12 @@ store_stash () {
shift
stash_msg="$1"
;;
+   -m*)
+   stash_msg=${1#-m}
+   ;;
+   --message=*)
+   stash_msg=${1#--message=}
+   ;;
-q|--quiet)
quiet=t
;;
@@ -251,6 +263,12 @@ push_stash () {
test -z ${1+x} && usage
stash_msg=$1
;;
+   -m*)
+   stash_msg=${1#-m}
+   ;;
+   --message=*)
+   stash_msg=${1#--message=}
+   ;;
--help)
show_help
;;
-- 
2.15.0.471.g17a719cfe.dirty

Re: stash: learn to parse -m/--message like commit does

[Phil Hord]

Hm..  Sorry about the formatting here.  It's been a while.  I'll try again.

On Tue, Nov 21, 2017 at 3:07 PM, Phil Hord <phil.h...@gmail.com> wrote:
> `git stash push -m foo` uses "foo" as the message for the stash. But
> `git stash push -m"foo"` does not parse successfully.  Similarly
> `git stash push --message="My stash message"` also fails.  Nothing
> in the documentation suggests this syntax should work, but it does
> work for `git commit`, and my fingers have learned this pattern long
> ago.
>
> Teach `git stash` to parse -mFoo and --message=Foo the same as
> `git commit` would do.
>
> Signed-off-by: Phil Hord <phil.h...@gmail.com>
> ---
>  git-stash.sh | 18 ++
>  1 file changed, 18 insertions(+)
>
> diff --git a/git-stash.sh b/git-stash.sh
> index 4b7495144..1114005ce 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -76,6 +76,12 @@ create_stash () {
>   shift
>   stash_msg=${1?"BUG: create_stash () -m requires an argument"}
>   ;;
> + -m*)
> + stash_msg=${1#-m}
> + ;;
> + --message=*)
> + stash_msg=${1#--message=}
> + ;;
>   -u|--include-untracked)
>   shift
>   untracked=${1?"BUG: create_stash () -u requires an argument"}
> @@ -193,6 +199,12 @@ store_stash () {
>   shift
>   stash_msg="$1"
>   ;;
> + -m*)
> + stash_msg=${1#-m}
> + ;;
> + --message=*)
> + stash_msg=${1#--message=}
> + ;;
>   -q|--quiet)
>   quiet=t
>   ;;
> @@ -251,6 +263,12 @@ push_stash () {
>   test -z ${1+x} && usage
>   stash_msg=$1
>   ;;
> + -m*)
> + stash_msg=${1#-m}
> + ;;
> + --message=*)
> + stash_msg=${1#--message=}
> + ;;
>   --help)
>   show_help
>   ;;
> --
> 2.15.0.471.g17a719cfe.dirty

stash: learn to parse -m/--message like commit does

[Phil Hord]

`git stash push -m foo` uses "foo" as the message for the stash. But
`git stash push -m"foo"` does not parse successfully.  Similarly
`git stash push --message="My stash message"` also fails.  Nothing
in the documentation suggests this syntax should work, but it does
work for `git commit`, and my fingers have learned this pattern long
ago.

Teach `git stash` to parse -mFoo and --message=Foo the same as
`git commit` would do.

Signed-off-by: Phil Hord <phil.h...@gmail.com>
---
 git-stash.sh | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/git-stash.sh b/git-stash.sh
index 4b7495144..1114005ce 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -76,6 +76,12 @@ create_stash () {
  shift
  stash_msg=${1?"BUG: create_stash () -m requires an argument"}
  ;;
+ -m*)
+ stash_msg=${1#-m}
+ ;;
+ --message=*)
+ stash_msg=${1#--message=}
+ ;;
  -u|--include-untracked)
  shift
  untracked=${1?"BUG: create_stash () -u requires an argument"}
@@ -193,6 +199,12 @@ store_stash () {
  shift
  stash_msg="$1"
  ;;
+ -m*)
+ stash_msg=${1#-m}
+ ;;
+ --message=*)
+ stash_msg=${1#--message=}
+ ;;
  -q|--quiet)
  quiet=t
  ;;
@@ -251,6 +263,12 @@ push_stash () {
  test -z ${1+x} && usage
  stash_msg=$1
  ;;
+ -m*)
+ stash_msg=${1#-m}
+ ;;
+ --message=*)
+ stash_msg=${1#--message=}
+ ;;
  --help)
  show_help
  ;;
-- 
2.15.0.471.g17a719cfe.dirty

doc: prefer 'stash push' over 'stash save'

[Phil Hord]

Although `git stash save` was deprecated recently, some parts of the
documentation still refer to it instead of `push`.

Signed-off-by: Phil Hord <phil.h...@gmail.com>
---
 Documentation/git-stash.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 8be661007..056dfb866 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -175,14 +175,14 @@ create::
  return its object name, without storing it anywhere in the ref
  namespace.
  This is intended to be useful for scripts.  It is probably not
- the command you want to use; see "save" above.
+ the command you want to use; see "push" above.

 store::

  Store a given stash created via 'git stash create' (which is a
  dangling merge commit) in the stash ref, updating the stash
  reflog.  This is intended to be useful for scripts.  It is
- probably not the command you want to use; see "save" above.
+ probably not the command you want to use; see "push" above.

 DISCUSSION
 --
-- 
2.15.0.471.g17a719cfe.dirty

Re: git-push branch confusion caused by user mistake

[Phil Hord]

On Mon, Mar 13, 2017 at 1:55 AM Jacob Keller <jacob.kel...@gmail.com> wrote:
> On Fri, Mar 10, 2017 at 2:13 PM, Junio C Hamano <gits...@pobox.com> wrote:
> > Phil Hord <phil.h...@gmail.com> writes:
> >> I think git should be smarter about deducing the dest ref from the
> >> source ref if the source ref is in refs/remotes, but I'm not sure how
> >> far to take it.
> >
> > My knee-jerk reaction is "Don't take it anywhere".
> >
> > Giving a refspec from the command line is an established way to
> > defeat the default behaviour when you do not give any and only the
> > remote, and making it do things behind user's back, you would be
> > robbing the escape hatch from people.
>
> It might be worth having some warning or something happen here? I've
> had several  co-workers at $DAYJOB get confused by this sort of thing.

On one very active project at $work, we have 380,000 commits, 4600
branches in refs/heads and 96 branches in refs/remotes.  About half of
the refs/remotes (43) are obviously user errors.  The other half it's
not possible for me to know.

I suggested to our admins to block attempts to push to
'refs/remotes/*' so in the future users don't lose track of commits
they think they pushed.  But I don't know if that will really happen.

Thanks for the counterexample feedback.

git-push branch confusion caused by user mistake

[Phil Hord]

This week a user accidentally did this:

$ git push origin origin/master
Total 0 (delta 0), reused 0 (delta 0)
To parent.git
 * [new branch]  origin/master -> origin/master

He saw his mistake when the "new branch" message appeared, but he was
confused about how to fix it and worried he broke something.

It seems reasonable that git expanded the original args into this one:

git push origin refs/remotes/origin/master

However, since the dest ref was not provided, it was assumed to be the
same as the source ref, so it worked as if he typed this:

git push origin refs/remotes/origin/master:refs/remotes/origin/master

Indeed, git ls-remote origin shows the result:

$ git ls-remote origin
d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e HEAD
d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e refs/heads/master
d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e refs/remotes/origin/master

Also, I verified that this (otherwise valid) command has similar
unexpected results:
$ git remote add other foo.git && git fetch other && git push
origin other/topic
$ git ls-remote origin
d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e HEAD
d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e refs/heads/master
d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e refs/remotes/origin/master
d1ff1c9224ae5e58a7656fb9ecc95865d42ed71e refs/remotes/other/topic

I think git should be smarter about deducing the dest ref from the
source ref if the source ref is in refs/remotes, but I'm not sure how
far to take it.  It feels like we should translate refspecs something
like this for push:

origin/master
=> refs/remotes/origin/master:refs/heads/master

refs/remotes/origin/master
 => refs/remotes/origin/master:refs/heads/master

origin/master:origin/master
 => refs/remotes/origin/master:refs/heads/origin/master

master:refs/remotes/origin/master
 => refs/heads/master:refs/remotes/origin/master

That is, we should not infer a remote refspec of "refs/remotes/*"; we
should only get there if "refs/remotes" was given explicitly by the
user.

Does this seem reasonable?  I can try to work up a patch if so.

Re: Request re git status

[Phil Hord]

On Mon, Feb 6, 2017 at 3:36 PM Ron Pero  wrote:
> I almost got bit by git: I knew there were changes on the remote
> server, but git status said I was uptodate with the remote.
>

Do you mean you almost pushed some changed history with "--force"
which would have lost others' changes?  Use of this option is
discouraged on shared branches for this very reason.  But if you do
use it, the remote will tell you the hash of the old branch so you can
undo the damage.

But if you did not use --force, then you were not in danger of being
bit.  Git would have prevented the push in that case.


> Why ... not design it to [optionally] DO a fetch and THEN declare
> whether it is up to date?

It's because `git status` does not talk to the remote server, by
design.  The only Git commands that do talk to the remote are push,
pull and fetch.  All the rest work off-line and they do so
consistently.

Imagine `git status` did what you requested; that is, it first did a
fetch and then reported the status.  Suppose someone pushed a commit
to the remote immediately after your fetch completed.  Now git will
still report "up to date" but it will be wrong as soon as the remote
finishes adding the new push.  Yet the "up to date" message will
remain on your console, lying to you.  If you leave and come back in
two days, the message will remain there even if it is no longer
correct.

So you should accept that `git status` tells you the status with
respect to your most recent fetch, and that you are responsible for
the timing of the most recent fetch.  To have git try to do otherwise
would be misleading.

> Or change the message to tell what it really
> did, e.g. "Your branch was up-to-date with 'origin/master' when last
> checked at {timestamp}"? Or even just say, "Do a fetch to find out
> whether your branch is up to date"?

These are reasonable suggestions, but i don't think the extra wording
adds anything for most users.  Adding a timestamp seems generally
useful, but it could get us into other trouble since we have to depend
on outside sources for timestamps.  :-\

diff --color-words breaks spacing

[Phil Hord]

I noticed some weird spacing when comparing files with git diff
--color-words.  The space before a colored word disappears sometimes.

$ git --version
git version 2.11.0.485.g4e59582ff

echo "FOO foo; foo = bar" > a
echo "FOO foo = baz" > b
git diff --color-words --no-index a b
FOOfoo; foo = barbaz

There should be a space after FOO in the diff, even if git doesn't
think "foo" and "foo;" are the same word.

If I remove the semicolon, it looks better, but in fact it only moves
the error later. The missing space is now between the two "foo" words.

echo "FOO foo foo = bar" > a
echo "FOO foo = baz" > b
git diff --color-words --no-index a b
FOO foofoo = barbaz

Here's the same with the color codes changed to text for purposes of this email:

echo "FOO foo; foo = bar" > a
echo "FOO foo = baz" > b
git diff --color-words --no-index a b | tr  \\033 E
FOOE[31mfoo;E[m foo = E[31mbarE[mE[32mbazE[m

echo "FOO foo foo = bar" > a
echo "FOO foo = baz" > b
git diff --color-words --no-index a b | tr  \\033 E
FOO fooE[31mfooE[m = E[31mbarE[mE[32mbazE[m

Re: [RFC 2/2] grep: use '/' delimiter for paths

[Phil Hord]

On Tue, Jan 24, 2017 at 1:54 AM Stefan Hajnoczi  wrote:
> > The use of "git show" you are demonstrating is still about showing
> > the commit object, whose behaviour is defined to show the log
> > message and the diff relative to its sole parent, limited to the
> > paths that match the pathspec.
> >
> > It is perfectly fine and desirable that "git show :"
> > and "git show  -- " behaves differently.  These are
> > two completely different features.
>
> Thanks for explaining guys.  It all makes logical sense.  I just hope I
> remember the distinctions in that table for everyday git use.


Familiar itch; previous discussion here:

https://public-inbox.org/git/1377528372-31206-1-git-send-email-ho...@cisco.com/

Phil

Re: [PATCH v5 1/4] implement submodule config API for lookup of .gitmodules values

[Phil Hord]

On Mon, Jun 15, 2015 at 5:06 PM, Heiko Voigt hvo...@hvoigt.net wrote:
 In a superproject some commands need to interact with submodules. They
 need to query values from the .gitmodules file either from the worktree
 of from certain revisions. At the moment this is quite hard since a
 caller would need to read the .gitmodules file from the history and then
 parse the values. We want to provide an API for this so we have one
 place to get values from .gitmodules from any revision (including the
 worktree).

 The API is realized as a cache which allows us to lazily read
 .gitmodules configurations by commit into a runtime cache which can then
 be used to easily lookup values from it. Currently only the values for
 path or name are stored but it can be extended for any value needed.

 It is expected that .gitmodules files do not change often between
 commits. Thats why we lookup the .gitmodules sha1 from a commit and then
 either lookup an already parsed configuration or parse and cache an
 unknown one for each sha1. The cache is lazily build on demand for each
 requested commit.

 This cache can be used for all purposes which need knowledge about
 submodule configurations. Example use cases are:

  * Recursive submodule checkout needs to lookup a submodule name from
its path when a submodule first appears. This needs be done before
this configuration exists in the worktree.

  * The implementation of submodule support for 'git archive' needs to
lookup the submodule name to generate the archive when given a
revision that is not checked out.

  * 'git fetch' when given the --recurse-submodules=on-demand option (or
configuration) needs to lookup submodule names by path from the
database rather than reading from the worktree. For new submodule it
needs to lookup the name from its path to allow cloning new
submodules into the .git folder so they can be checked out without
any network interaction when the user does a checkout of that
revision.

 Signed-off-by: Heiko Voigt hvo...@hvoigt.net
 ---
  .gitignore   |   1 +
  Documentation/technical/api-submodule-config.txt |  46 +++
  Makefile |   2 +
  submodule-config.c   | 445 
 +++
  submodule-config.h   |  27 ++
  submodule.c  |   1 +
  submodule.h  |   1 +
  t/t7411-submodule-config.sh  |  85 +
  test-submodule-config.c  |  66 
  9 files changed, 674 insertions(+)
  create mode 100644 Documentation/technical/api-submodule-config.txt
  create mode 100644 submodule-config.c
  create mode 100644 submodule-config.h
  create mode 100755 t/t7411-submodule-config.sh
  create mode 100644 test-submodule-config.c


Instead of test-submodule-config.c to test this new module, it could
be useful to implement these as extensions to rev-parse:

git rev-parse --submodule-name [ref:]path
git rev-parse --submodule-path [ref:]name
git rev-parse --submodule-url [ref:]name
git rev-parse --submodule-ignore [ref:]name
git rev-parse --submodule-recurse [ref:]name

Has this already been considered and rejected for some reason?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Submodules as first class citizens (was Re: Moving to subtrees for plugins?)

[Phil Hord]

On Tue, Jun 9, 2015 at 2:40 PM, Jens Lehmann jens.lehm...@web.de wrote:
 Am 07.06.2015 um 08:26 schrieb Stefan Beller:

 On 06.06.2015 12:53, Luca Milanesio wrote:

 On 6 Jun 2015, at 18:49, Phil Hord phil.h...@gmail.com wrote:
 On Fri, Jun 5, 2015, 2:58 AM lucamilanesio luca.milane...@gmail.com
 wrote:

 Ideally, as a git clone --recursive already exists, I would like to
 see a git diff --recursive that goes through the submodules as well
 :-)

 Something possibly to propose to the Git mailing list?


 Such an option makes lots of sense to me (though --recurse-submodules
 should be its name for consistency reasons). This could be an alias for
 --submodule=full, as the --submodule option controls the format of
 submodule diffs.

To me, --recurse-submodules means submodules are still not first-class
citizens.  But let's put that aside for a moment; I don't care about
the switch name too much as long as I can configure
'diff.recurse-submodules = true'.

[The following is rather long.  I'm sorry for that.  Feel free to look
away when it gets too vague.]

Let me set up a submodule like so:

  $ git init /tmp/Super  cd /tmp/Super
  Super$ git submodule add https://github.com/gitster/git.git Foo

I wish to be able to grep from Super and find matches in all my submodules.

  Super$ git grep --recurse-submodules base--int
  Foo/.gitignore:/git-rebase--interactive
  Foo/Makefile:SCRIPT_LIB += git-rebase--interactive

But I want this to work naturally across git-module boundaries, so I
want this also to work (grepping a super-project from within a
submodule):

  Super$ cd Foo
  Foo$ git grep --recurse-submodules base--int ..
  .gitignore:/git-rebase--interactive
  Makefile:SCRIPT_LIB += git-rebase--interactive

I expect some groans from the audience here, because I think if the
syntax above worked, then so would this:

  $ cd /tmp
  tmp$ git grep base--int /tmp/Super/Foo
  /tmp/Super/Foo/.gitignore:/git-rebase--interactive
  /tmp/Super/Foo/Makefile:SCRIPT_LIB += git-rebase--interactive

This usage has nothing to do with submodules, really, except that it
allows git commands to reach into foreign git directories by virtue of
the path supplied as some argument instead of via $GITDIR, and in
doing so it helps solve some git submodules use cases of mine.

But if that did not turn your stomach, try this one:

  $ cd /tmp/Super
  Super$ printf Some submodule dataFoo/data.txt
  Super$ git add Foo/data.txt
  fatal: Pathspec 'Foo/data.txt' is in submodule 'Foo'
  Super$ git add --recurse-submodules Foo/data.txt

Some notes on this usage:

1. --recurse-submodules seems like a reasonable name for this switch,
especially when you consider the 'git add --recurse-submodules .' use
case.

2. This recursive 'git add' seems dangerous to me unless git-status
also shows all the changed/untracked files in submodules as well if
the --recurse-submodules switch is included.  This would support the
expectation that 'git add .' is going to add the files shown by 'git
status .'

3. Configuring --recurse-submodules as the default mode for 'git add'
but not for 'git status' seems reckless enough that I think there
should not be separate options for these two commands.  There are
probably many other cross-command scenarios with similar coupling.

Moving on, as we have :/ to mean 'workdir root', I wonder how you
would spell super-project workdir root.  Maybe it would be ::/

I realize the kinds of features I'm talking about require extensive
code changes in Git.  For example, consider the meaning of this:

  Super$ git diff --recurse-submodules origin/next origin/master

Since I created Super just a few minutes ago and it has no remote
named 'origin', this command seems meaningless to me.  But suppose
that origin/next and origin/master did exist in my Super project.
Then, I would expect in my wishlist Git, that

A.  Super$ git diff --recurse-submodules origin/next origin/master
This would include differences in Foo between origin/master:Foo and
origin/next:Foo; that is, the commits referenced from those gitlinks
in Super.

B.  Super$ git diff --recurse-submodules origin/next HEAD
This would include differences in Foo between origin/master:Foo and
HEAD:Foo; that is, the commits referenced from those gitlinks in
Super.

C.  Super$ git diff --recurse-submodules origin/next
This would include differences in Foo between origin/master:Foo and
the current Foo workdir.

D.  Super$ cd Foo  git diff origin/next
This would include differences in Foo between the Foo submodule's
origin/master and the current Foo workdir.

Now, C and D seem confusingly similar to me and technically very
different.  I could understand the results, but I could easily be led
astray, especially if I am writing a script.  But I still think it is
reasonable and correct.

I think this could have dire consequences for some commands like 'git
apply'. But I think it is reasonable for git apply to reject such
cross-project diffs, at least in the beginning.  :-)

While I am thinking

Submodules as first class citizens (was Re: Moving to subtrees for plugins?)

[Phil Hord]

On Fri, Jun 5, 2015, 2:58 AM lucamilanesio luca.milane...@gmail.com wrote:

 Some devs of my Team complained that with submodules it is
 difficult to see the “full picture” of the difference
 between two SHA1 on the root project, as the submodules
 would just show as different SHA1s. When you Google
 “subtree submodules” you find other opinions as well:

 Just to mention a few:
 -
 https://codingkilledthecat.wordpress.com/2012/04/28/why-y
 our-company-shouldnt-use-git-submodules/ -
 http://blogs.atlassian.com/2013/05/alternatives-to-git-su
 bmodule-git-subtree/

 To be honest with you, I am absolutely fine with
 submodules as I can easily leave with the “extra pain” of
 diffing by hand recursively on submodules. But it is true
 that it may happen to either forget to do a git submodule
 update or otherwise forget you are in a detached branch
 and start committing “on the air” without a branch.

...

 Ideally, as a git clone --recursive already exists, I would like to
 see a git diff --recursive that goes through the submodules as well :-)

 Something possibly to propose to the Git mailing list?


I've worked on git diff --recursive a bit myself, along with some
simpler use cases (git ls-tree --recursive) as POCs. I think some of
the needs there begin to have ui implications which could be
high-friction. I really want to finish it someday, but I've been too
busy lately at $job, and now my experiments are all rather stale.

It would be a good discussion to have over at the git list (copied).
Heiko and Jens have laid some new groundwork in this area and it may
be a good time to revisit it.  Or maybe they've even moved deeper than
that; I have been distracted for well over a year now.

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] rebase -i: redo tasks that die during cherry-pick

[Phil Hord]

On Wed, Apr 29, 2015 at 1:15 PM, Junio C Hamano gits...@pobox.com wrote:

 Thanks, will queue.

 Aside from the much more invasive possibility, the patch makes me
 wonder if it would have been a better design to have a static todo
 with a current pointer as two state files.  Then reschedule would
 have been just the matter of decrementing the number in current,
 instead of grab the last line of one file and prepend to the other
 file, and then lose the last line.

That's an interesting idea.  Changing it now would impact anyone who
now depends on the current todo/done behavior, and I imagine there
are many.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Regular Rebase Failure

[Phil Hord]

 On Mon, Apr 27, 2015 at 10:07 AM, Adam Steel adamgst...@gmail.com wrote:
 Stefan,

 So I switched git versions.

 $ git --version
 git version 2.3.1

 I'm still getting the same regular rebase failures.

 ---

 fatal: Unable to create
 '/Users/asteel/Repositories/rails-teespring/.git/index.lock': File
 exists.

Is the repository located on a mounted network share, or could other
users be accessing it via a network mount?  We had a similar problem
recently on a new Jenkins VM instance which had only NFS-mounted
storage available. I don't remember if it was Git that was failing on
there, and I wasn't directly involved in solving the problem.  But
while researching the issue I found ominous warnings about the dangers
of file-locking on remote shares [1]. Which is to say, I don't know
much, but I heard a rumor...  :-)

Perhaps this is old news and already well covered in Git.  But I am curious...


[1] http://0pointer.de/blog/projects/locking.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] rebase -i: redo tasks that die during cherry-pick

[Phil Hord]

When rebase--interactive processes a task, it removes the item from
the todo list and appends it to another list of executed tasks. If a
pick (this includes squash and fixup) fails before the index has
recorded the changes, take the corresponding item and put it on the todo
list again. Otherwise, the changes introduced by the scheduled commit
would be lost.

That kind of decision is possible since the cherry-pick command
signals why it failed to apply the changes of the given commit. Either
the changes are recorded in the index using a conflict (return value 1)
and rebase does not continue until they are resolved or the changes
are not recorded in the index (return value neither 0 nor 1) and
rebase has to try again with the same task.

Add a test cases for regression testing to the rebase-interactive
test suite.

Signed-off-by: Fabian Ruch baf...@gmail.com
Signed-off-by: Phil Hord ho...@cisco.com
---

Notes:
Last year in ${gmane}/250126 Fabian Ruch helpfully provided a patch
to fix a rebase bug I complained about. I have simplified it a bit
and merged in the tests which had been in a separate commit.

It has bitten me twice since the original discussion and has also
been reported by others, though I haven't found those emails to
add them to the CC list yet.

CC: Michael Haggerty mhag...@alum.mit.edu

 git-rebase--interactive.sh| 16 +++
 t/t3404-rebase-interactive.sh | 47 +++
 2 files changed, 63 insertions(+)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 08e5d86..bab0dcc 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -132,6 +132,16 @@ mark_action_done () {
fi
 }
 
+# Put the last action marked done at the beginning of the todo list
+# again. If there has not been an action marked done yet, leave the list of
+# items on the todo list unchanged.
+reschedule_last_action () {
+   tail -n 1 $done | cat - $todo $todo.new
+   sed -e \$d $done $done.new
+   mv -f $todo.new $todo
+   mv -f $done.new $done
+}
+
 append_todo_help () {
git stripspace --comment-lines $todo \EOF
 
@@ -252,6 +262,12 @@ pick_one () {
output eval git cherry-pick \
${gpg_sign_opt:+$(git rev-parse --sq-quote 
$gpg_sign_opt)} \
$strategy_args $empty_args $ff $@
+
+   # If cherry-pick dies it leaves the to-be-picked commit unrecorded. 
Reschedule
+   # previous task so this commit is not lost.
+   ret=$?
+   case $ret in [01]) ;; *) reschedule_last_action ;; esac
+   return $ret
 }
 
 pick_one_preserving_merges () {
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index eed76cc..ac429a0 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1055,4 +1055,51 @@ test_expect_success 'todo count' '
grep ^# Rebase ..* onto ..* ([0-9] actual
 '
 
+test_expect_success 'rebase -i commits that overwrite untracked files (pick)' '
+   git checkout --force branch2 
+   git clean -f 
+   set_fake_editor 
+   FAKE_LINES=edit 1 2 git rebase -i A 
+   test_cmp_rev HEAD F 
+   test_path_is_missing file6 
+   file6 
+   test_must_fail git rebase --continue 
+   test_cmp_rev HEAD F 
+   rm file6 
+   git rebase --continue 
+   test_cmp_rev HEAD I
+'
+
+test_expect_success 'rebase -i commits that overwrite untracked files 
(squash)' '
+   git checkout --force branch2 
+   git clean -f 
+   git tag original-branch2 
+   set_fake_editor 
+   FAKE_LINES=edit 1 squash 2 git rebase -i A 
+   test_cmp_rev HEAD F 
+   test_path_is_missing file6 
+   file6 
+   test_must_fail git rebase --continue 
+   test_cmp_rev HEAD F 
+   rm file6 
+   git rebase --continue 
+   test $(git cat-file commit HEAD | sed -ne \$p) = I 
+   git reset --hard original-branch2
+'
+
+test_expect_success 'rebase -i commits that overwrite untracked files (no ff)' 
'
+   git checkout --force branch2 
+   git clean -f 
+   set_fake_editor 
+   FAKE_LINES=edit 1 2 git rebase -i --no-ff A 
+   test $(git cat-file commit HEAD | sed -ne \$p) = F 
+   test_path_is_missing file6 
+   file6 
+   test_must_fail git rebase --continue 
+   test $(git cat-file commit HEAD | sed -ne \$p) = F 
+   rm file6 
+   git rebase --continue 
+   test $(git cat-file commit HEAD | sed -ne \$p) = I
+'
+
 test_done
-- 
2.4.0.rc3.329.gd1f7d3b

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] git-push.txt: clean up force-with-lease wording

[Phil Hord]

The help text for the --force-with-lease option to git-push
does not parse cleanly.  Clean up the wording and syntax to
be more sensible.  Also remove redundant information in the
--force-with-lease alone description.

Signed-off-by: Phil Hord ho...@cisco.com
---
 Documentation/git-push.txt | 14 ++
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 5171086..863c30c 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -157,9 +157,8 @@ already exists on the remote side.
Usually, git push refuses to update a remote ref that is
not an ancestor of the local ref used to overwrite it.
 +
-This option bypasses the check, but instead requires that the
-current value of the ref to be the expected value.  git push
-fails otherwise.
+This option overrides this restriction if the current value of the
+remote ref is the expected value.  git push fails otherwise.
 +
 Imagine that you have to rebase what you have already published.
 You will have to bypass the must fast-forward rule in order to
@@ -171,15 +170,14 @@ commit, and blindly pushing with `--force` will lose her 
work.
 This option allows you to say that you expect the history you are
 updating is what you rebased and want to replace. If the remote ref
 still points at the commit you specified, you can be sure that no
-other people did anything to the ref (it is like taking a lease on
-the ref without explicitly locking it, and you update the ref while
-making sure that your earlier lease is still valid).
+other people did anything to the ref. It is like taking a lease on
+the ref without explicitly locking it, and the remote ref is updated
+only if the lease is still valid.
 +
 `--force-with-lease` alone, without specifying the details, will protect
 all remote refs that are going to be updated by requiring their
 current value to be the same as the remote-tracking branch we have
-for them, unless specified with a `--force-with-lease=refname:expect`
-option that explicitly states what the expected value is.
+for them.
 +
 `--force-with-lease=refname`, without specifying the expected value, will
 protect the named ref (alone), if it is going to be updated, by
-- 
2.3.3.377.gdac1145

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Checkout --force without complete overwrite?

[Phil Hord (hordp)]

I have a repo whose workdir tends to get pretty dirty as I jump around from 
branch to branch tending weeds and whatnot.   Sometimes when I try to switch 
branches git refuses because of local file changes.

  git checkout otherbranch
  error: Your local changes to the following files would be overwritten by 
checkout:
foo.txt
bar.c
  Please, commit your changes or stash them before you can switch branches.
  Aborting

I resolve this by examining my local changes and deciding if I want to keep 
them or not.  I keep the changes in the conflicting files that I want, and then 
I discard the rest.

One way to discard the rest is to say 
   git checkout HEAD -- foo.txt bar.c  git checkout otherbranch

But sometimes the list of local-change conflicts I want to discard is too long 
and this recipe seems like a good alternative to me:
   git checkout -f otherbranch

But this is disastrous, because I have been focused on examining the 
conflicting local changes in foo.txt and bar.c, but I forgot about my 
non-conflicting changes to baz.c, lost as it is in a sea of untracked files 
making up the majority of my workdir local changes.  So all my untracked files 
make the leap unscathed, but my precious forgotten changes in baz.c get wiped 
out by the checkout --force, even though the baz.c in index and in otherbranch 
are the same.

I've read the documentation for 'git checkout --force' several times and I have 
a hard time deciding what it means to do.  But I'm sure it's doing what it's 
designed to do and what the man page says it will.  (English is my first 
language, btw.)

What I am seeking is some way to checkout the other branch and replace my 
conflicted local changes while ignoring my non-conflicting local changes in 
tracked files.  Something like --force-gently, maybe.  Does such an option 
exist?

I could script something, but it comes up only often enough to bite me, so I'm 
sure I'd forget I had scripted it.

Thanks,
Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Bug in log for path in case of identical commit

[Phil Hord]

On Fri, Oct 31, 2014 at 4:40 AM, Alexandre Garnier zigarn+...@gmail.com wrote:
 When merging 2 branches with the same modifications on the both sides,
 depending the merge side, one branch disappear from the file history.

 To be more clear, there is a script in attachment to reproduce, but
 here is the result :
 $ git log --graph --oneline --all --decorate --name-status
 *   63c807f (HEAD, master) Merge branch 'branch' into 'master'
 |\
 | * 5dc8785 (branch) Change line 15 on branch
 | | M   file
 | * d9cd3ce Change line 25 on branch
 | | M   file
 * | 7220d52 Change line 15 on master
 |/
 |   M   file
 * 7566672 Initial commit
   A file

 $ git log --graph --oneline --all --decorate --name-status -- file
 * 5dc8785 (branch) Change line 15 on branch
 | M file
 * d9cd3ce Change line 25 on branch
 | M file
 * 7566672 Initial commit
   A file

 = The commit 7220d52 modified the file but is not shown in file
 history anymore.
 The expected result would be:
 * 5dc8785 (branch) Change line 15 on branch
 | M file
 * d9cd3ce Change line 25 on branch
 | M file
 | * 7220d52 Change line 15 on master
 |/
 |   M   file
 * 7566672 Initial commit
   A file

 The order between the 2 commits on the branch is not important.
 If you do a 'cherry-pick 7220d52' or a 'merge --squash master' instead
 of applying the same modification for commit 5dc8785, you get the same
 result (cherry-picking was my initial use-case).
 If you do not create the commit d9cd3ce, then the file history show all 
 commits.
 If you merge 'master' into 'branch', then the file history show all commits.

This last line was confusing to me.  But I think you've misinterpreted
the results a bit.  There is no difference between merge master into
branch and merge branch into master in this case.  The real reason
the extra commit is shown in the former case is that you used
'--all' (include all refs as commandline arguments) and the commit
which was being omitted was directly referenced by a ref, 'master'.

When I remove the --all from your test script, I get consistent logs
for the two merges.

Maybe this has misled your other tests as well.  Read the History
Simplification section of git help log.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git reflog --date

[Phil Hord]

On Tue, Oct 21, 2014 at 1:24 PM, Junio C Hamano gits...@pobox.com wrote:
 John Tapsell johnf...@gmail.com writes:

 Hi all,

   Could we add a default to --date so that:

 git reflog --date

 just works?  (Currently you need to do:   git reflog --date=iso)  It
 should probably obey the default in log.date?

 Hmph.  --date=style is not the way to choose between timed and
 counted output in the first place, though.

Of course not.  I always use --relative-date for this.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: squash commits deep down in history

[Phil Hord]

On Thu, Oct 23, 2014 at 8:34 AM, Henning Moll newssc...@gmx.de wrote:
 Hi,

 i need to squash several commits into a single one in a automated way. I know 
 that there is interactive rebase, which can also be automated using 
 GIT_SEQUENCE_EDITOR. Unfortunately my history is very large and i need to 
 squash deep down in the history several times. So using interactive rebase 
 seems not to be the right tool.

 I wonder if i can solve this task using filter-branch? I have a file that 
 list the SHA1s for each squash operation per line. All SHA1s of a line are in 
 chronological order (youngest to oldest), or in other words: the first SHA1 
 is the child of the second, and so on.

 | ab4423e 3432343 3234252
 | 2324342 5232343
 | ...

 Lets say there are N lines in that file. Each line means: squash all SHA1s of 
 this line into the first (or last) SHA1 of this line.

I've often felt there should be some simple commands to do these kinds
of edits.  For example, it is easy to amend HEAD, but an
order-of-magnitude more difficult to amend HEAD^.   I imagine commands
like this:

   git rebase --reword HEAD^
   git rebase --edit some-old-commit
   git rebase --squash ab4423e 3432343 3234252

But each time I think of implementing one of these I am daunted by the
many exceptional cases.


 Performing this task with rebase would require N rewritings of the history. 
 So e.g. HEAD (but many others too) would be rewritten N times even if it is 
 not directly part of a line. My thinking is, that a filter-branch can do this 
 in a single rewrite and therefore would be much more performant.

 How can i solve this? Any ideas?

I know you solved this already with filter-branch, but that seems like
a complicated solution to me.  I think the --interactive rebase would
have been useful for you.  You should keep in mind that you do not
need to repeat the command multiple times for your multiple squashes.
For example, if your to-do list looks like this simple example:

pick 000
pick ab4423e
pick 3432343
pick 3234252
pick 001
pick 002
pick 2324342
pick 5232343
pick 003

I think you could get the desired effect by changing it to this:

pick 000
pick ab4423e
squash 3432343
squash 3234252
pick 001
pick 002
pick 2324342
squash 5232343
pick 003

And then running that all in one interactive rebase.  Does that make sense?

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 02/23] rebase -i: allow squashing empty commits without complaints

[Phil Hord]

Thanks for working on this.

On Wed, Aug 6, 2014 at 7:59 PM, Fabian Ruch baf...@gmail.com wrote:
 The to-do list commands `squash` and `fixup` apply the changes
 introduced by the named commit to the tree but instead of creating
 a new commit on top of the current head it replaces the previous
 commit with a new commit that records the updated tree. If the
 result is an empty commit git-rebase stops with the error message

You asked to amend the most recent commit, but doing so would make
it empty. You can repeat your command with --allow-empty, or you can
remove the commit entirely with git reset HEAD^.

 This message is not very helpful because neither does git-rebase
 support an option `--allow-empty` nor does the messages say how to
 resume the rebase. Firstly, change the error message to

The squash result is empty and --keep-empty was not specified.

You can remove the squash commit now with

  git reset HEAD^

Once you are down, run

s/down/done


  git rebase --continue


I like the direction of this rewording, but it seems to hide the
instructions for the user who wants to keep the empty commit.

I'm not sure how to improve it.  Maybe this:

The squash result is empty and --keep-empty was not specified.

You may remove the squash commit now with

  git reset HEAD^

or keep it by doing nothing extra.

Continue the rebase with

  git rebase --continue

 If the user wishes to squash a sequence of commits into one
 commit, f. i.

pick A
squash Revert A
squash A'

 , it does not matter for the end result that the first squash
 result, or any sub-sequence in general, is going to be empty. The
 squash message is not affected at all by which commits are created
 and only the commit created by the last line in the sequence will
 end up in the final history. Secondly, print the error message
 only if the whole squash sequence produced an empty commit.

 Lastly, since an empty squash commit is not a failure to rewrite
 the history as planned, issue the message above as a mere warning
 and interrupt the rebase with the return value zero. The
 interruption should be considered as a notification with the
 chance to undo it on the spot. Specifying the `--keep-empty`
 option tells git-rebase to keep empty squash commits in the
 rebased history without notification.

 Add tests.

 Reported-by: Peter Krefting pe...@softwolves.pp.se
 Signed-off-by: Fabian Ruch baf...@gmail.com
 ---
 Hi,

 Peter Krefting is cc'd as the author of the bug report Confusing
 error message in rebase when commit becomes empty discussed on the
 mailing list in June. Phil Hord and Jeff King both participated in
 the problem discussion which ended with two proposals by Jeff.

 Jeff King writes:
   1. Always keep such empty commits. A user who is surprised by them
  being empty can then revisit them. Or drop them by doing another
  rebase without --keep-empty.

   2. Notice ourselves that the end-result of the whole squash is an
  empty commit, and stop to let the user deal with it.

 This patch chooses the second alternative. Either way seems OK. The
 crucial consensus of the discussion was to silently throw away empty
 interim commits.

This patch does _not_ silently throw away empty commits. I wonder if
such behavior could be triggered with something like --no-keep-empty.
Maybe that belongs in another patch.


Fabian

  git-rebase--interactive.sh| 20 +++---
  t/t3404-rebase-interactive.sh | 62 
 +++
  2 files changed, 79 insertions(+), 3 deletions(-)

 diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
 index 3222bf6..8820eac 100644
 --- a/git-rebase--interactive.sh
 +++ b/git-rebase--interactive.sh
 @@ -549,7 +549,7 @@ do_next () {
 squash|s|fixup|f)
 # This is an intermediate commit; its message will 
 only be
 # used in case of trouble.  So use the long version:
 -   do_with_author output git commit 
 --allow-empty-message \
 +   do_with_author output git commit 
 --allow-empty-message --allow-empty \
 --amend --no-verify -F $squash_msg \
 ${gpg_sign_opt:+$gpg_sign_opt} ||
 die_failed_squash $sha1 $rest
 @@ -558,18 +558,32 @@ do_next () {
 # This is the final command of this squash/fixup group
 if test -f $fixup_msg
 then
 -   do_with_author git commit 
 --allow-empty-message \
 +   do_with_author git commit 
 --allow-empty-message --allow-empty \
 --amend --no-verify -F $fixup_msg \
 ${gpg_sign_opt:+$gpg_sign_opt} ||
 die_failed_squash $sha1

Re: Trouble merging renamed but identical files - CONFLICT (rename/rename)

[Phil Hord]

On Fri, Jun 27, 2014 at 6:39 PM, Jason Pyeron jpye...@pdinc.us wrote:
 On Fri, Jun 27, 2014 at 4:47 PM, Jason Pyeron
 jpye...@pdinc.us wrote:
  There are two identical files from the same original
 parent, but both were
  renamed in their own branches. One branch moved the file to
 a new folder, the
  other renamed the file in the same folder.

 You have not stated what you think the issue is.  You have only stated
 the setup.

 Thanks, I could have said it better.

 I think that git should understand that I have moved a file in path only (the
 tree object containing the file's entry change, but not the entry it self) and
 that the branch from which I want to merge back (with common ancestry) has
 renamed the file in the same path ( the tree object is unchanged, but the 
 entry
 is) such that the object is re-parented and renamed in that path.

I think Git's perspective is that you have moved the file in both
contexts. The name of the file includes the path and filename.  The
fact is that you renamed the file in both branches.  If you had
renamed the file in only one branch, Git would have had a better
chance of figuring out the right thing to do.  Git tries not to do
something potentially dangerous without getting your involvement.

That said, Git's rename handling is stupid sometimes and could stand
to be improved.

 How can this be done in git or if it cannot what are the chalenges to patching
 git for this issue.

I do not know a better thing for git to do here.  I can imagine cases
where either choice is the wrong one.  If git silently makes the
choice and continues, say, during a rebase, you might not notice until
things have horribly awry.


 ...
 Maybe you meant to move the renamed file to the same folder where it
 exists in the merge target.  I do not get a conflict when I do that.

 Are you saying I should git mv src/TrueCrypt.sln CipherShed.sln ?

 Then it will be in the wrong path as intended.


git reset --hard b60070f4d0879e277f44d174a163bbb292325fea
git mv src/TrueCrypt.sln CipherShed.sln
git commit -m 'renamed to be congruent with a0c84ff'
git merge a0c84ff28f356bcb8b872a9c65a2e9bff97b3f68

 No conflict (on that file, anyway).

 Agreed, but not the desired end state.

Git's magic still has limits.

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Trouble merging renamed but identical files - CONFLICT (rename/rename)

[Phil Hord]

On Fri, Jun 27, 2014 at 8:42 PM, Jason Pyeron jpye...@pdinc.us wrote:
 Sorry for the http://pastebin.com/1R68v6jt (changes the merge to
 1ca13ed2271d60ba93d40bcc8db17ced8545f172, and manually reconciles the merge),
 but it was too long to be readable in the email.

 git blame HEAD -- src/Main/Forms/CipherShed.fbp | cut -c 1-8 | sort -u

 Gives:
 ac812aa3
 b50a2fb1

 git blame b60070f4d0879e277f44d174a163bbb292325fea --
 src/Main/Forms/TrueCrypt.fbp | cut -c 1-8 | sort -u

 Gives:
 07b2176f
 0eb8b4fa
 12c94add
 a17c95a3
 a757b4d4
 cac6cd14
 d0a9dfa8
 d94128a9
 e6b1437a
 f1bb489c

 If I use cherry pick (vs merge), I can maintain the big history in b60070f, 
 but
 loose the small history in 1ca13ed

   [test]
   / \
  /   \
 [b60070f] [1ca13ed]
 | |
 | |
 [65efd37] |
 |\|
 | \   |
 [d8da778] [39ebb06]

 How do I maintain all the history including the (line) changes in 1ca13ed?

I see the results, but my brain is not able to make sense of your goal
yet.  I'll try again later when I've had my coffee.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Trouble merging renamed but identical files - CONFLICT (rename/rename)

[Phil Hord]

On Sun, Jun 29, 2014 at 11:31 AM, Phil Hord phil.h...@gmail.com wrote:
 On Fri, Jun 27, 2014 at 8:42 PM, Jason Pyeron jpye...@pdinc.us wrote:
 Sorry for the http://pastebin.com/1R68v6jt (changes the merge to
 1ca13ed2271d60ba93d40bcc8db17ced8545f172, and manually reconciles the merge),
 but it was too long to be readable in the email.

Ok, I think I understand the issue you are trying to solve now.

Git (rather famously[1]) does not record renames or copies.  It is
expected instead that renames and copies will be detected when it is
important after the fact. This allows us to ignore rename detection
and resolution when creating project history; in the future, better
rename/copy detection will just work on existing repositories and
the repos themselves will not need to be adjusted.

What you are encountering now seems to be a shortcoming of Git's
current rename/copy detection.  But you are trying to overcome today's
shortcoming by adjusting your project history to accommodate it.
Instead you should just do the merge like you normally would without
regard to how 'git blame' shows the result.

Maybe there is a bug here still, but it is probably in git-blame.

[1] 
https://git.wiki.kernel.org/index.php/GitFaq#Why_does_Git_not_.22track.22_renames.3F
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Trouble merging renamed but identical files - CONFLICT (rename/rename)

[Phil Hord]

On Sun, Jun 29, 2014 at 4:20 PM, Jason Pyeron jpye...@pdinc.us wrote:
 -Original Message-
 From: Phil Hord
 Sent: Sunday, June 29, 2014 16:09

 On Sun, Jun 29, 2014 at 11:31 AM, Phil Hord
 phil.h...@gmail.com wrote:
  On Fri, Jun 27, 2014 at 8:42 PM, Jason Pyeron
 jpye...@pdinc.us wrote:
  Sorry for the http://pastebin.com/1R68v6jt (changes the merge to
  1ca13ed2271d60ba93d40bcc8db17ced8545f172, and manually
 reconciles the merge),
  but it was too long to be readable in the email.

 Ok, I think I understand the issue you are trying to solve now.

 Git (rather famously[1]) does not record renames or copies.  It is
 expected instead that renames and copies will be detected when it is
 important after the fact. This allows us to ignore rename detection
 and resolution when creating project history; in the future, better
 rename/copy detection will just work on existing repositories and
 the repos themselves will not need to be adjusted.

 Looking at http://pastebin.com/1R68v6jt , I have a work around.

 In summary, 7.git cherry-pick -x HEAD..rebranding , then

 git merge $(echo 'Merge of 1ca13ed2271d60ba93d40bcc8db17ced8545f172 branch -
 rebranding' |\
 git commit-tree -p HEAD -p rebranding \
  $(git cat-file -p HEAD | grep ^tree | sed -e 's/^tree //') )

 Now it is perfect in the blame and log --graph.

Yes, but your workaround unnecessarily duplicates commits and
complicates the history of your project. You are munging your project
to compensate for git's current shortcomings.

But it's your project.  Your choice.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Trouble merging renamed but identical files - CONFLICT (rename/rename)

[Phil Hord]

On Sun, Jun 29, 2014 at 5:13 PM, Jason Pyeron jpye...@pdinc.us wrote:
 -Original Message-
 From: Phil Hord
 Sent: Sunday, June 29, 2014 16:27

 On Sun, Jun 29, 2014 at 4:20 PM, Jason Pyeron
 jpye...@pdinc.us wrote:
  -Original Message-
  From: Phil Hord
  Sent: Sunday, June 29, 2014 16:09
 
  On Sun, Jun 29, 2014 at 11:31 AM, Phil Hord
  phil.h...@gmail.com wrote:
   On Fri, Jun 27, 2014 at 8:42 PM, Jason Pyeron
  jpye...@pdinc.us wrote:
   Sorry for the http://pastebin.com/1R68v6jt (changes the merge to
   1ca13ed2271d60ba93d40bcc8db17ced8545f172, and manually
  reconciles the merge),
   but it was too long to be readable in the email.
 
  Ok, I think I understand the issue you are trying to solve now.
 
  Git (rather famously[1]) does not record renames or copies.  It is
  expected instead that renames and copies will be detected
 when it is
  important after the fact. This allows us to ignore rename detection
  and resolution when creating project history; in the future, better
  rename/copy detection will just work on existing repositories and
  the repos themselves will not need to be adjusted.
 
  Looking at http://pastebin.com/1R68v6jt , I have a work around.
 
  In summary, 7.git cherry-pick -x HEAD..rebranding , then
 
  git merge $(echo 'Merge of
 1ca13ed2271d60ba93d40bcc8db17ced8545f172 branch -
  rebranding' |\
  git commit-tree -p HEAD -p rebranding \
   $(git cat-file -p HEAD | grep ^tree | sed -e
 's/^tree //') )
 
  Now it is perfect in the blame and log --graph.

 Yes, but your workaround unnecessarily duplicates commits and
 complicates the history of your project. You are munging your project

 But I want to avoid thet complicating, while still showing that line 42 was
 modified by X. Should this be possible with a merge, without using 
 cherry-pick?

I think it should.  But there are other complications in your project
which may be getting in the way. You are merging two branches with no
common ancestor.  When git walks down either path looking for the
source commit for each line, it finds two sources for it.  For
example, it reaches commit 39ebb06 which appears to be the origin of
all lines in that file since it has no parent. I imagine this could
act as a short-circuit to further searching.  Furthermore, I'm not
sure how git should know any better.

It seems you already have a merge point for these two branches, but I
haven't looked deeply into how that merge was done.  But I think the
multiple disconnected branch histories may be the cause of the
confusion.

 Btw I am not able to pull up https://git.wiki.kernel.org/ or
 http://git.wiki.kernel.org/


That is strange.  It works for me here, and I am just a user like you.

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Trouble merging renamed but identical files - CONFLICT (rename/rename)

[Phil Hord]

On Fri, Jun 27, 2014 at 4:47 PM, Jason Pyeron jpye...@pdinc.us wrote:
 There are two identical files from the same original parent, but both were
 renamed in their own branches. One branch moved the file to a new folder, the
 other renamed the file in the same folder.

You have not stated what you think the issue is.  You have only stated
the setup.


I suppose you want Git to merge without conflict in the end, though,
based on your script.  Is that right?


 Steps to reproduce the issue:
 git init
 git fetch https://github.com/pdinc-oss/CipherShed.git
 git fetch https://github.com/srguglielmo/CipherShed.git
 git checkout -b test b60070f4d0879e277f44d174a163bbb292325fea
 git merge a0c84ff28f356bcb8b872a9c65a2e9bff97b3f68

 CONFLICT (rename/rename): Rename TrueCrypt.sln-src/TrueCrypt.sln in 
 branch
 HEAD rename TrueCrypt.sln-CipherShed.sln in
 a0c84ff28f356bcb8b872a9c65a2e9bff97b3f68

Git seems to be doing the correct thing here.


 git reset --hard b60070f4d0879e277f44d174a163bbb292325fea
 git mv src/TrueCrypt.sln src/CipherShed.sln
 git commit -m 'renamed to be congruent with a0c84ff'
 git merge a0c84ff28f356bcb8b872a9c65a2e9bff97b3f68

 Sill get a CONFLICT (rename/rename): Rename
 TrueCrypt.sln-src/CipherShed.sln in branch HEAD rename
 TrueCrypt.sln-CipherShed.sln in 
 a0c84ff28f356bcb8b872a9c65a2e9bff97b3f68

Git seems to be doing the correct thing here, too.

 I will have many more to come, any suggestions?

Maybe you meant to move the renamed file to the same folder where it
exists in the merge target.  I do not get a conflict when I do that.

   git reset --hard b60070f4d0879e277f44d174a163bbb292325fea
   git mv src/TrueCrypt.sln CipherShed.sln
   git commit -m 'renamed to be congruent with a0c84ff'
   git merge a0c84ff28f356bcb8b872a9c65a2e9bff97b3f68

No conflict (on that file, anyway).

Regards,
Phil

 This message is copyright PD Inc, subject to license 20080407P00.

p.s. Maybe you should remove this copyright threat in the future when
you are writing to an open source community seeking help.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: files deleted with no record?

[Phil Hord]

On Mon, Jun 23, 2014 at 9:15 PM, Jeremy Scott jer...@great-scotts.org wrote:
 I just encountered a situation where a merge was made, with no
 apparent changes in files (ie no log), but the result was that some
 files were deleted.

 person A adds some files
 person B adds some files from the same point

You mean from the same point in history, right?  Not files with the
same filename or path?

 person B commits and pushes.
 person A commits -- now merge is required
 a few more commits on top of person B's commit

I don't understand this step.  Who adds a few more commits on top of B and why?

 person A merges - this removes the files that B added. There is no log
 of the files being deleted

This sounds like an evil merge (see man gitglossary, and google),
but it's not clear to me how you arrived here.

When I tried to reproduce your issue with this script, it did not
remove any files unexpectedly:
---8---
#!/bin/sh

set -e
mkdir foo  cd foo  git init
echo foo  foo  git add foo  git commit -mfoo

git checkout -b PersonA master
echo bar  bar  git add bar
git commit -mPersonA: bar

git checkout -b PersonB master
echo baz  baz  git add baz
git commit -mPersonB: baz

echo foo-conflict  foo  git add foo
git commit -mPersonB: foo

git checkout PersonA
echo foo-different  foo  git add foo
git commit -mPersonA: foo (conflicts with PersonB)

git log --oneline --all --stat

if ! git merge PersonB ; then
  git checkout PersonA foo
  git commit --no-edit
fi
git log --oneline --graph --stat
---8---

A sneaky issue with merges is that they do not have a clear way to
show patch information -- the diff between the commit and its ancestor
-- because they have multiple ancestors.  You can show diffs for a
merge relative to each of its parents, but it is not usually done for
you automatically.  This is why making changes in a merge which are
not actually required for the merge is bad (evil).

 Clearly this is possible - was this a user error?

Probably, but we'd need to see how the user got there.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Confusing error message in rebase when commit becomes empty

[Phil Hord]

On Wed, Jun 11, 2014 at 8:49 AM, Peter Krefting pe...@softwolves.pp.se wrote:
 I am rebasing a branch to combine a couple of commits. One is a revert of a 
 previous commit. Since there are commits in-between, I do squash to make 
 sure I get everything, and then add the actual change on top of that. The 
 problem is that rebase stops with a confusing error message (from commit, 
 presumably):

   $ git rebase --interactive
   [...]
   You asked to amend the most recent commit, but doing so would make
   it empty. You can repeat your command with --allow-empty, or you can
   remove the commit entirely with git reset HEAD^.
   rebase in progress; onto 342b22f
   You are currently rebasing branch 'mybranch' on '342b22f'.

   No changes

   Could not apply 4682a1f20f6ac29546536921bc6ea0386441e23e... Revert 
 something

 OK, so I should retry the command with --allow-empty, then:

   $ git rebase --interactive --allow-empty
   error: unknown option `allow-empty'

 Nope, that's not quite right.


The correct switch for rebase is --keep-empty, but it is too late to
choose it once the interactive rebase is underway.  I think the
correct advice might be something like this:

  You asked to squash this commit and its parent, but doing so would make
  it empty. You can drop this empty commit with git reset HEAD^ , or you can
  keep it with git commit --amend --allow-empty.

But I have not tested this.


 Running git rebase --continue does work as expected, but perhaps it just 
 shouldn't stop in this case?


What does it mean when you say it worked as expected?  Did it leave
the empty commit, omit the empty commit, or leave some un-squashed
commit?  It's not clear to me what --continue _should_ do in this
case, but it does seem like the two options here should be

 1. keep the empty commit
 2. drop the empty commit

I would expect git rebase --skip to drop the empty commit, but maybe
it will skip the squash instead.  I don't know.  Better advice here
is certainly needed.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Confusing error message in rebase when commit becomes empty

[Phil Hord]

On Wed, Jun 11, 2014 at 1:57 PM, Peter Krefting pe...@softwolves.pp.se wrote:
 Phil Hord:


 What does it mean when you say it worked as expected?  Did it leave
 the empty commit, omit the empty commit, or leave some un-squashed
 commit?


 Actually, it did not work as expected I noted afterward, it just dropped the
 reversion commit, and did not squash the next commit into it as I had asked,
 so from three commits, change, revert, new-change I had two, change,
 new-change with the end result being the same (i.e., instead of squashing
 all three into one new-change, I had change and revert +
 new-change).

Did you have a series of three commits being squashed in your to-do
list?  I mean, did you have a list like this:

   pick ...  do foo
   squash ...  revert do foo
   squash ...  What I really meant to do.

I suppose the rebase stopped after the first squash failed due to the
emptiness of the proposed result.  Then rebase --continue proceeded,
having decided that you were finished with the 'revert' commit.  Then
... I would expect the next commit would actually be squashed, but I
can only speculate at the reasons it might have decided not to after
your continue.

This actually sounds like another case of a bug I reported a few weeks
ago[1] and which Fabian Ruch was kindly investigating[2] and trying to
fix.  I don't think his fix would have helped in this case, but I do
think it is worthy of consideration for that same patch series.

 It's not clear to me what --continue _should_ do in this case, but it does
 seem like the two options here should be

 I sort of expect a squashed commit of change + revert to be an empty
 commit, and of change + revert + new-change to be a commit of
 new-change.

Yes, but empty commits are discouraged on some projects.  If you want
your change + revert = empty commit to appear after the squash, I
would expect you would want to use --keep-empty on your inital rebase
command.  But I'm not sure that will do what you expected either; it
may only keep previously-empty commits during the rebase.

[1] http://article.gmane.org/gmane.comp.version-control.git/245688
[2] http://www.mail-archive.com/git%40vger.kernel.org/msg51703.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 1/3] sequencer: Signal failed ff as an aborted, not a conflicted merge

[Phil Hord]

On 06/10/2014 01:56 PM, Junio C Hamano wrote:
 Fabian Ruch baf...@gmail.com writes:

 On 05/27/2014 08:42 PM, Junio C Hamano wrote:
 Fabian Ruch baf...@gmail.com writes:
 [..]

 In order to signal the three possible situations (not only success and
 failure to complete) after a pick through porcelain commands such as
 `cherry-pick`, exit with a return value that is neither 0 nor 1. -1 was
 chosen in line with the other situations in which the sequencer
 encounters an error.
 Hmph... do we still pass negative to exit() anywhere in our codebase?
 No, but I thought `cmd_cherry_pick` would just forward the `return -1` from 
 the
 sequencer to the shell. I had another look and found that `cmd_cherry_pick`
 calls `die` instead. Now, the commit inserts 128 as exit status in
 `fast_forward_to`.

 Would it be appropriate to mention the choice of exit status in the coding
 guidelines? I didn't know that the int-argument to exit(3) gets truncated and
 that this is why it is a general rule to only use values in the range from 0 
 to
 255 with exit(3).
 I personally do not think of a reason why it is necessary to mention
 how the argument to exit(3) is expected to be used by the system, but
 if it is common not to know it, I guess it would not hurt to have a
 single paragraph with at most two lines.

 In any case, I agree that exiting with 1 that signals failed with
 conflict can be confusing to the caller.  Can we have a test to
 demonstrate when this fix matters?

I think you are asking for a test and not for clarification.  But a test
was provided in 3/3 in this series.  Was it not related directly enough?

For clarification, this tri-state return value matters when the caller
is planning to do some cleanup and needs to handle the fallout
correctly.  Maybe changing this return value is not the correct way
forward, though.  It might be better if the caller could examine the
result after-the-fact instead.  This would require some reliable state
functions which I recall were somewhat scattered last time I looked. 
Also I cannot think of a reliable test for the previous cherry-pick
failed during pre-condition checks and I'm not sure anything should
exist to track this in .git outside of the return value for this function. 

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 2/3] rebase -i: Reschedule tasks that failed before the index was touched

[Phil Hord]

Hi Fabian,

Thanks for looking into this.

On 05/27/2014 07:56 AM, Michael Haggerty wrote:
 +reschedule_last_action () {
 +tail -n 1 $done | cat - $todo $todo.new
 +sed -e \$d $done $done.new
 +mv -f $todo.new $todo
 +mv -f $done.new $done
 +}
 +
  append_todo_help () {
  git stripspace --comment-lines $todo \EOF
  
 @@ -470,11 +480,15 @@ do_pick () {
 --no-post-rewrite -n -q -C $1 
  pick_one -n $1 
  git commit --allow-empty --allow-empty-message \
 -   --amend --no-post-rewrite -n -q -C $1 ||
 -die_with_patch $1 Could not apply $1... $2
 +   --amend --no-post-rewrite -n -q -C $1
 git cherry-pick indicates its error status specifically as 1 or some
 other value.  But here it could be that pick_one succeeds but git
 commit fails; in that case ret is set to the return code of git
 commit.  So, if git commit fails with a retcode different than 1,
 then reschedule_last_action will be called a few lines later.  This
 seems incorrect to me.

I agree.  I was thinking that pick_one should get this new behavior
instead of do_pick, but some callers may not appreciate the new behavior
(i.e. squash/fixup), though I expect those callers have similar problems
as this commit is trying to fix.

I think adding a pick_one_with_reschedule function (to be called in both
places from do_pick) could solve the issue Michael mentioned without
breaking other pick_one callers.

I have not tested doing so, but I think it would look like this:

===

diff --git i/git-rebase--interactive.sh w/git-rebase--interactive.sh
index fe813ba..ae5603a 100644
--- i/git-rebase--interactive.sh
+++ w/git-rebase--interactive.sh
@@ -235,6 +235,17 @@ git_sequence_editor () {
 eval $GIT_SEQUENCE_EDITOR '$@'
 }
 
+pick_one_with_reschedule () {
+pick_one $1
+ret=$?
+case $ret in
+0) ;;
+1) ;;
+*) reschedule_last_action ;;
+esac
+return $ret
+}
+
 pick_one () {
 ff=--ff
 
@@ -474,13 +485,13 @@ do_pick () {
 # rebase --continue.
 git commit --allow-empty --allow-empty-message --amend \
--no-post-rewrite -n -q -C $1 
-pick_one -n $1 
+pick_one_with_reschedule -n $1 
 git commit --allow-empty --allow-empty-message \
--amend --no-post-rewrite -n -q -C $1 \
${gpg_sign_opt:+$gpg_sign_opt} ||
 die_with_patch $1 Could not apply $1... $2
 else
-pick_one $1 ||
+pick_one_with_reschedule $1 ||
 die_with_patch $1 Could not apply $1... $2
 fi
 }

===

I don't much like the name 'pick_one_with_reschedule', but I didn't like
my other choices, either.

I'll try to look at this and your patches more closely when I have a bit
more time.

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Zsh submodule name completion borked

[Phil Hord]

On Thu, May 1, 2014 at 6:35 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 Phil Hord wrote:
 When I use zsh tab-completion to complete the submodule name in 'git
 submodule init', I get more than I expected.

 From the gerrit repository (which has plugins):
   $ git submodule init plugins/TAB
   plugins/commit-message-length-validator\ \(v1.0-rc1-9-g545000b\)
   plugins/reviewnotes\ \(v1.0-rc1-8-ge984300\)
   plugins/replication\ \(v1.1-rc0-13-g4c3f4c9\)

 It works ok in bash.  I tried to bisect the trouble, but it still
 fails in 1.8.3, so I'm beginning to think it's me.  Does this happen
 to anyone else?  Is it something in my local configuration causing
 this?


It seems to be something local.  I thought the issue persisted with no
local .zshrc config, but it looks like I only turned off my local
config and not the global settings.  The recent Ubuntu update is a
likely culprit.  I'll investigate locally and turn my reports up to
Ubuntu/Debian/Zshell.

 Define 'works' in bash. From what I can see from the bash completion,
 it's not doing anything special, so the completion you see are simply
 files.

To clarify my description in case anyone else sees it or is
interested, before I load /etc/zsh/zshrc, tab gives me simple filename
expansion.

After I load /etc/zsh/zshrc, tab expands only submodules in HEAD.  But
for some reason it gets the wrong kind of results in the expansion,
returning not just submodule paths, but submodule paths with tag info
appended.

Sample session:
  $ zsh --norcs
  % git submodule init plugins/TAB
   commit-message-length-validator/
   README
   reviewnotes/
   replication/
  ^C
  % source /etc/zsh/zshrc
  % git submodule init plugins/TAB
  plugins/commit-message-length-validator\ \(v1.0-rc1-9-g545000b\)
  plugins/reviewnotes\ \(v1.0-rc1-8-ge984300\)
  plugins/replication\ \(v1.1-rc0-13-g4c3f4c9\)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Zsh submodule name completion borked

[Phil Hord]

When I use zsh tab-completion to complete the submodule name in 'git
submodule init', I get more than I expected.

From the gerrit repository (which has plugins):
  $ git submodule init plugins/TAB
  plugins/commit-message-length-validator\ \(v1.0-rc1-9-g545000b\)
  plugins/reviewnotes\ \(v1.0-rc1-8-ge984300\)
  plugins/replication\ \(v1.1-rc0-13-g4c3f4c9\)

It works ok in bash.  I tried to bisect the trouble, but it still
fails in 1.8.3, so I'm beginning to think it's me.  Does this happen
to anyone else?  Is it something in my local configuration causing
this?

Thanks,
Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

git-rebase-todo gets popped prematurely

[Phil Hord]

During a 'git rebase --continue', I got an error about having left a
file in place which the next commit intended to add as new.  Stupid me.

So I rm'ed the file and tried again.  This time, git rebase --continue
succeeded.  But it accidentally left out the next commit in my rebase-todo.

I looked in the code and it seems that when the pick returns an error,
rebase--interactive stops and lets the user clean up.  But it assumes
the index  already tracks a conflicted merge, and so it removes the
commit from the todo list.  In this case, however, the conflicted merge
was avoided by detecting it in advance.  The result is that the would
be overwritten conflict evicts the entire commit from the rebase action.

I think the code needs to detect the difference between merge failed;
conflicted index and merge failed; no change.  I think I can do this
with 'git-status -s -uno', maybe.  But I haven't tried it yet and it
feels like I'm missing a case or two also.

I tried to bisect this to some specific change, but it fails the same
way as far back as 1.6.5. 

Test script follows in case anyone has a better idea how to approach
this and wants to understand it better.

#!/bin/sh

set -x
git --version
rm -rf baz
git init baz  cd baz
echo initialinitial  git add initial  git commit -minitial
echo foofoo  git add foo  git commit -mfoo
echo barbar  git add bar  git commit -mbar
git log --oneline

GIT_EDITOR='sed -i -e s/^pick/edit/ -e /^#/d' git rebase -i HEAD^^
touch bar
git rebase --continue
rm bar
git rebase --continue
git log --oneline


And the tail of the output (note the missing bar commit even though
Successfully rebased):

+ git log --oneline
fcc9b6e bar
8121f15 foo
a521fa1 initial
+ GIT_EDITOR='sed -i -e s/^pick/edit/ -e /^#/d'
+ git rebase -i 'HEAD^^'
Stopped at 8121f1593ea5c66dc7e9af7719100c1fcf4ab721... foo
You can amend the commit now, with

git commit --amend

Once you are satisfied with your changes, run

git rebase --continue

+ touch bar
+ git rebase --continue
error: The following untracked working tree files would be
overwritten by merge:
bar
Please move or remove them before you can merge.
Aborting
Could not apply fcc9b6ef2941e870f88362edbe0f1078cebb20e5... bar
+ rm bar
+ git rebase --continue
Successfully rebased and updated refs/heads/master.
+ git log --oneline
8121f15 foo
a521fa1 initial


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Borrowing objects from nearby repositories

[Phil Hord]

On Tue, Mar 11, 2014 at 11:37 PM, Andrew Keller and...@kellerfarm.com wrote:
 I am considering developing a new feature, and I'd like to poll the group for 
 opinions.

 Background: A couple years ago, I wrote a set of scripts that speed up 
 cloning of frequently used repositories.  The scripts utilize a bare Git 
 repository located at a known location, and automate providing a --reference 
 parameter to `git clone` and `git submodule update`.  Recently, some 
 coworkers of mine expressed an interest in using the scripts, so I published 
 the current version of my scripts, called `git repocache`, described at the 
 bottom of https://github.com/andrewkeller/ak-git-tools.

 Slowly, it has occurred to me that this feature, or something similar to it, 
 may be worth adding to Git, so I've been thinking about the best approach.  
 Here's my best idea so far:

 1)  Introduce '--borrow' to `git-fetch`.  This would behave similarly to 
 '--reference', except that it operates on a temporary basis, and does not 
 assume that the reference repository will exist after the operation 
 completes, so any used objects are copied into the local objects database.  
 In theory, this mechanism would be distinct from '--reference', so if both 
 are used, some objects would be copied, and some objects would be accessible 
 via a reference repository referenced by the alternates file.

Interesting.  I do something similar on my CI Server to reduce
workload on Gerrit. Having a built-in to support submodules would be
nice.  Currently my script does this:

MIRROR=/path/to/local/mirror
NEW=ssh://gerrit-server
git clone ${MIRROR}/project  cd project

#-- Init/update submodules from our local mirror if possible
git submodule update --recursive --init

#-- Switch to the remote server URL
git config remote.origin.url $(git config remote.origin.url|sed -e
s|^${MIRROR}|${NEW}|)
git submodule sync #--recursive ; recursive not supported :-[

#-- Checkout remote updates
git pull --ff-only --recurse-submodules origin ${BRANCH}
git submodule update --recursive --init


Is that about the same as you are aiming for?


 2)  Teach `git fetch` to read 'repocache.path' (or a better-named 
 configuration), and use it to automatically activate borrowing.

Seems like this could be trouble if a local repo is coincidentally
named the same as some unrelated repo you want to clone.  But I can
see the value.

What about something similar to url.insteadOf?   Maybe
'url.${SERVER}.autoBorrow = ${MIRROR}', with replacement semantics
similar to insteadOf.

 3)  For consistency, `git clone`, `git pull`, and `git submodule update` 
 should probably all learn '--borrow', and forward it to `git fetch`.

 4)  In some scenarios, it may be necessary to temporarily not automatically 
 borrow, so `git fetch`, and everything that calls it may need an argument to 
 do that.

--no-borrow

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Git Config pushInsteadOf is not working

[Phil Hord]

I thought you had the URLs backwards, but that doesn't seem to be the
problem, assuming I am reading your transcription correctly. Maybe the
'insteadOf' is being applied in addition to (and cancelling out) the
pushInsteadOf.  Does it work as expected if you remove one or the
other?

In any case, it sounds like a Git issue, not a Gerrit one. You should
ask on git@vger.kernel.org, which I have cc'ed here.

Phil

On Tue, Mar 11, 2014 at 4:44 AM, Raf rafeah.rahi...@gmail.com wrote:
 Hi All,

 I have been searching high and low for this issue, but somehow I do not see
 anyone encountering the same issue as me.

 Here is the scenario:

 I have created a local mirror for my group of developers to download the
 AOSP code from an external gerrit server.
 So the developers will download the code from the mirror but push to the
 external gerrit server.

 Hence, I have edited my /home/user/.gitconfig file to add the following:
 #To download from
 [url ssh://localMirror]
 insteadOf=ssh://gerritServer
 #to push
 [url ssh://gerritServer]
 pushInsteadOf = ssh://localMirror


 Some how, the pushInsteadOf does not work, when i tried to push the changes
 to the external gerrit server, it still pushes to the local mirror server.

 Also, when I tried to manually add the remote to the repository: git remote
 add gerrit_origin ssh://gerritServer
 I tried to push to the gerrit_origin, it still pushes to the local mirror
 server. Which is strange..

 Please help. I have spent whole day looking for this solution to no avail.

 Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC] Introduce git submodule add|update --attach

[Phil Hord]

On Sun, Dec 29, 2013 at 8:49 PM, Francesco Pretto cez...@gmail.com wrote:

 by default git submodule performs its add or update operations on a detached
 HEAD. This works well when using an existing full-fledged/indipendent project 
 as
 the submodule, as there's less frequent need to update it or commit back
 changes. When the submodule is actually a large portion of shareable code
 between  different projects, and the superproject needs to track very closely
 the evolution of the submodule (or the other way around), I feel more 
 confortable
 to reattach the HEAD of the submodule with an existing branch. This can be as
 simple as having a superproject project1 in branch master with a submodule
 common attached to the branch master-project1 or, in a more development
 workflow, project1 in branch featureA with the same submodule common
 attached to a similarly named branch featureA. Doing this in git requires me
 the following:

 # Maintainer
 $ git submodule add --branch master-project1 repository common
 $ git commit -m Added submodule
 $ git config -f .gitmodules submodule.common.ignore all
 $ git push
 $ cd path
 $ git checkout master-project1

 # Developer
 $ git pull
 $ git submodule init
 $ git submodule update --remote
 $ cd path
 $ branch=$(git config -f ..\.gitmodules submodule.common.branch); git 
 checkout $branch

 While the burden for the repository maitainer/administrator is acceptable, in
 the developer point of view there are two problems:
 1) Checking out an attached HEAD of a specified branch as when using 
 --remote
 is not really simple as it could be and could require lauching of scrips or
 reading some repository specific documentation. Also in Windows platform the
 syntax for inline shell evaluation of commands is less known between users;
 2) There's no way to store a similar default behaviour in the repository 
 except
 by using scripts. Also recently submodule.modulename.update custom !commands
 in no more supported when stored in .gitmodules [1].

 The attached patch tries to solve these problems by introducing an --attach
 switch to the add and update submodule commands and a --detach switch 
 just
 for the update command. It also add the support for an 
 'submodule.name.attach'
 property when updating. Using the --attach switch when adding a submodule 
 does:
 - create the submodule checking out an attached HEAD;
 - set the 'submodule.name.attach' property to 'true';
 - set the 'submodule.name.ignore' property to 'all' (this is useful as
 attaching to the branch doesn't require tracking of revision sha1).

 The rationale of setting 'attach' and 'ignore' properties when adding a
 submodule with the --attach switch is to give a convenient default 
 behaviour.
 No other properties are set: the repository responsible will still be required
 to configure a different 'submodule.name.update' behaviour separetely, if he
 wants that.

 When updating, using the '--attach' switch or operating in a repository with
 'submodule.name.attach' set to 'true' will:
 - checkout a branch with an attached HEAD if the repository was just cloned;
 - perform a fast-forward only merge of changes if it's a 'checkout' update,
 keeping the HEAD attached;
 - reattach the HEAD prior performing a 'merge', 'rebase' or '!command' update
 operation if the HEAD was found detached.

I need to understand this reattach the HEAD case better. Can you
give some examples of the expected behavior when merge, rebase or
!command is encountered?


 '--attach' or 'submodule.name.attach' set to true also implies '--remote', 
 as
 it's needed the origin HEAD sha1 to verify the current HEAD state.

 A '--detach' switch is also available. Using the '--detach' switch or
 operating in a repository with 'submodule.name.attach' set to 'false' during
 update will:
 - checkout a detached HEAD if the repository was just cloned (same behaviour 
 as
 before);
 - detach the HEAD prior performing a 'merge', 'rebase' or '!command' update
 operation if the HEAD was found attached.

 'submodule.name.attach' works the same way as 'submodule.name.update'
 property: git copies the values found in .gitmodules in .git/config when
 performing an init command. update looks for the values in .git/config
 only.

 '--attach' and '--detach' switches override an opposite behaviour of 
 'submodule.name.attach'
 properties.

 The patch is small (touches only git-submodule.sh) and 100% additive with
 respect to currently documented behaviour: when using add and update
 commands without the introduced switches and properties, git shall operate as
 before. As a bonus (but this was done to ease conditionals and keep the code
 clean) it also clarifies and validates the content of 
 'submodule.name.update'
 during 'update' command, warning the user if it's not one of the supported
 values 'checkout', 'merge', 'rebase' and 'none'. Please note that 'checkout'
 update command was documented in upstream Documentation/gitmodules.txt [2] 
 as
 a valid 

Re: Finding the repository

[Phil Hord]

On Thu, Oct 24, 2013 at 9:46 AM, Duy Nguyen pclo...@gmail.com wrote:
 On Thu, Oct 24, 2013 at 2:49 PM, Perry Hutchison per...@pluto.rain.com 
 wrote:
 Duy Nguyen pclo...@gmail.com wrote:

 ... it's not easy to determine ambiguity here, especially when the
 repo finding code does not know anything about bar/barz.c (is it
 a pathname or an argument to an option?).
...
 There are more cases to consider, like what if you do
 git rm bar/baz.c and rab/zab.c where bar and rab are
 two different repositories..

 So we remove baz.c from bar and zab.c from rab.  It's not clear
 to me that there's anything wrong with that -- it's exactly what
 I would expect to have happen (and also what the hackish script
 I posted will do).

 For git rm, maybe. Many other commands need repo information and it
 would not make sense to have paths from two different repositories.
 For example, commit, rev-list or log. And it may break more things as
 most of current commands are designed to work on one repo from a to z.
 Some may support multi-repo operations if they're part of submodule
 support.

I've done some preliminary work on extending this sort of behavior to
submodule commands.  For example,

  git grep --recurse-submodules foo

which would look in the current project path and also any submodules
encountered.  This usage also begs for this extension:

  git grep --recurse-submodules foo path/to/sub/bar.c

Where 'path/to/sub' is a submodule, and therefore a foreign git repo
to this one.  Solving this is a little bit easier than your case
because git is already running inside a repo. Extending the reach to
submodules only requires more odb's than our first one to be
considered.  Along the way, I have considered your case, but I haven't
focused on it. Lately I haven't had time to focus on my case either,
though.  :-\

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] for-each-ref: introduce %C(...) for color

[Phil Hord]

On Fri, Sep 27, 2013 at 8:10 AM, Ramkumar Ramachandra
artag...@gmail.com wrote:
 Enhance 'git for-each-ref' with color formatting options.  You can now
 use the following format in for-each-ref:

   %C(green)%(refname:short)%C(reset)

 Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
 ---
  Documentation/git-for-each-ref.txt |  4 +++-
  builtin/for-each-ref.c | 23 +++
  2 files changed, 22 insertions(+), 5 deletions(-)

 diff --git a/Documentation/git-for-each-ref.txt 
 b/Documentation/git-for-each-ref.txt
 index f2e08d1..078a116 100644
 --- a/Documentation/git-for-each-ref.txt
 +++ b/Documentation/git-for-each-ref.txt
 @@ -45,7 +45,9 @@ OPTIONS
 It also interpolates `%%` to `%`, and `%xx` where `xx`
 are hex digits interpolates to character with hex code
 `xx`; for example `%00` interpolates to `\0` (NUL),
 -   `%09` to `\t` (TAB) and `%0a` to `\n` (LF).
 +   `%09` to `\t` (TAB) and `%0a` to `\n` (LF). Additionally,
 +   colors can be specified using `%C(colorname)`. Use
 +   `%C(reset)` to reset the color.

Reduce the color explanation here and refer to the config page.
Something like pretty-formats does:

'%C(...)': color specification, as described in color.branch.*
config option;


  pattern...::
 If one or more patterns are given, only refs are shown that
 diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
 index 1d4083c..a1ca186 100644
 --- a/builtin/for-each-ref.c
 +++ b/builtin/for-each-ref.c
 @@ -9,6 +9,7 @@
  #include quote.h
  #include parse-options.h
  #include remote.h
 +#include color.h

  /* Quoting styles */
  #define QUOTE_NONE 0
 @@ -155,10 +156,13 @@ static const char *find_next(const char *cp)
 while (*cp) {
 if (*cp == '%') {
 /*
 +* %C( is the start of a color;
  * %( is the start of an atom;
  * %% is a quoted per-cent.
  */
 -   if (cp[1] == '(')
 +   if (cp[1] == 'C'  cp[2] == '(')
 +   return cp;
 +   else if (cp[1] == '(')
 return cp;
 else if (cp[1] == '%')
 cp++; /* skip over two % */
 @@ -180,8 +184,11 @@ static int verify_format(const char *format)
 const char *ep = strchr(sp, ')');
 if (!ep)
 return error(malformed format string %s, sp);
 -   /* sp points at %( and ep points at the closing ) */
 -   parse_atom(sp + 2, ep);
 +   /* Ignore color specifications: %C(
 +* sp points at %( and ep points at the closing )
 +*/
 +   if (prefixcmp(sp, %C())
 +   parse_atom(sp + 2, ep);
 cp = ep + 1;
 }
 return 0;
 @@ -933,12 +940,20 @@ static void emit(const char *cp, const char *ep)
  static void show_ref(struct refinfo *info, const char *format, int 
 quote_style)
  {
 const char *cp, *sp, *ep;
 +   char color[COLOR_MAXLEN];

 for (cp = format; *cp  (sp = find_next(cp)); cp = ep + 1) {
 ep = strchr(sp, ')');
 if (cp  sp)
 emit(cp, sp);
 -   print_value(info, parse_atom(sp + 2, ep), quote_style);
 +
 +   /* Do we have a color specification? */
 +   if (!prefixcmp(sp, %C())
 +   color_parse_mem(sp + 3, ep - sp - 3, --format, 
 color);
 +   else {
 +   printf(%s, color);

'color' used uninitialized here?

 +   print_value(info, parse_atom(sp + 2, ep), 
 quote_style);
 +   }
 }
 if (*cp) {
 sp = cp + strlen(cp);
 --
 1.8.4.478.g55109e3

 --
 To unsubscribe from this list: send the line unsubscribe git in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] for-each-ref: introduce %(upstream:track[short])

[Phil Hord]

On Fri, Sep 27, 2013 at 8:10 AM, Ramkumar Ramachandra
artag...@gmail.com wrote:
 Introduce %(upstream:track) to display [ahead M, behind N] and
 %(upstream:trackshort) to display =, , , or 
 appropriately (inspired by contrib/completion/git-prompt.sh).

 Now you can use the following format in for-each-ref:

   %C(green)%(refname:short)%C(reset)%(upstream:trackshort)

 to display refs with terse tracking information.

Thanks.  I like this.


 Note that :track and :trackshort only work with upstream, and error
 out when used with anything else.

I think I would like to use %(refname:track) myself, but this does not
detract from this change.


 Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
 ---
  Documentation/git-for-each-ref.txt |  6 +-
  builtin/for-each-ref.c | 44 
 --
  2 files changed, 47 insertions(+), 3 deletions(-)

 diff --git a/Documentation/git-for-each-ref.txt 
 b/Documentation/git-for-each-ref.txt
 index f1d4e9e..682eaa8 100644
 --- a/Documentation/git-for-each-ref.txt
 +++ b/Documentation/git-for-each-ref.txt
 @@ -93,7 +93,11 @@ objectname::
  upstream::
 The name of a local ref which can be considered ``upstream''
 from the displayed ref. Respects `:short` in the same way as
 -   `refname` above.
 +   `refname` above.  Additionally respects `:track` to show
 +   [ahead N, behind M] and `:trackshort` to show the terse
 +   version (like the prompt) , , , or =.  Has no
 +   effect if the ref does not have tracking information
 +   associated with it.

  HEAD::
 Used to indicate the currently checked out branch.  Is '*' if
 diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
 index e54b5d8..10843bb 100644
 --- a/builtin/for-each-ref.c
 +++ b/builtin/for-each-ref.c
 @@ -631,6 +631,7 @@ static void populate_value(struct refinfo *ref)
 int eaten, i;
 unsigned long size;
 const unsigned char *tagged;
 +   int upstream_present = 0;

This flag is out of place.  It should be in the same scope as 'branch'
since the code which depends on this flag also depends on '!!branch'.

However, I don't think it is even necessary.  The only way to reach
the places where this flag is tested is when (name=upstream) and
(upstream exists).  In all other cases, the parser loops before
reaching the track/trackshort code or else it doesn't enter it.


 ref-value = xcalloc(sizeof(struct atom_value), used_atom_cnt);

 @@ -648,6 +649,7 @@ static void populate_value(struct refinfo *ref)
 int deref = 0;
 const char *refname;
 const char *formatp;
 +   struct branch *branch;

 if (*name == '*') {
 deref = 1;
 @@ -659,7 +661,6 @@ static void populate_value(struct refinfo *ref)
 else if (!prefixcmp(name, symref))
 refname = ref-symref ? ref-symref : ;
 else if (!prefixcmp(name, upstream)) {
 -   struct branch *branch;
 /* only local branches may have an upstream */
 if (prefixcmp(ref-refname, refs/heads/))
 continue;
 @@ -669,6 +670,7 @@ static void populate_value(struct refinfo *ref)
 !branch-merge[0]-dst)
 continue;
 refname = branch-merge[0]-dst;
 +   upstream_present = 1;
 }
 else if (!strcmp(name, flag)) {
 char buf[256], *cp = buf;
 @@ -686,6 +688,7 @@ static void populate_value(struct refinfo *ref)
 } else if (!strcmp(name, HEAD)) {
 const char *head;
 unsigned char sha1[20];
 +
 head = resolve_ref_unsafe(HEAD, sha1, 1, NULL);
 if (!strcmp(ref-refname, head))
 v-s = *;
 @@ -698,11 +701,48 @@ static void populate_value(struct refinfo *ref)
 formatp = strchr(name, ':');
 /* look for short refname format */
 if (formatp) {
 +   int num_ours, num_theirs;
 +
 formatp++;
 if (!strcmp(formatp, short))
 refname = shorten_unambiguous_ref(refname,
   warn_ambiguous_refs);
 -   else
 +   else if (!strcmp(formatp, track) 
 +   !prefixcmp(name, upstream)) {
 +   char buf[40];
 +
 +   if (!upstream_present)
 +   continue;
 +   stat_tracking_info(branch, num_ours, 
 num_theirs);
 +   if (!num_ours  !num_theirs)
 +

Re: Bug: [hostname:port]:repo.git notation no longer works (for ssh)

[Phil Hord]

On Fri, Sep 27, 2013 at 4:07 AM, Morten Stenshorne msten...@opera.com wrote:
 If I don't go via the ssh tunnel (I finally have some VPN stuff these
 days, so I don't really need the tunnel thing anymore, but that's going
 to be a lot of remotes to update, so I'd prefer it just worked like it
 used to):

 -url = [localhost:2223]:blink.git
 +url = git:blink.git

 ... it works fine.


Until you get a proper fix, I wonder if this will help:

  git config --global --add url.git:.insteadOf  [localhost:2223]:

See git help config for details on the insteadOf config setting.

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Git tag output order is incorrect (IMHO)

[Phil Hord]

Someone at $work asked me this week how to find the current and
previous tags on his branch so he could generate release notes.  I
just need last two tags on head in topo-order. I was surprised by
how complicated this turned out to be. I ended up with this:

  git log --decorate=full --pretty=format:'%d' HEAD |
sed -n -e 's-^.* refs/tags/\(.*\)[ )].*$-\1-p' |
head -2

Surely there's a cleaner way, right?

Phil



On Sun, Sep 8, 2013 at 6:49 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 On Thu, Jul 18, 2013 at 10:27 AM, Rahul Bansal rahul.ban...@rtcamp.com 
 wrote:
 I am posting here first time, so please excuse me if this is not right place 
 to send something like this.

 Please check - 
 http://stackoverflow.com/questions/6091306/can-i-make-git-print-x-y-z-style-tag-names-in-a-sensible-order

 And also - https://github.com/gitlabhq/gitlabhq/issues/4565

 IMHO git tag is expected to show tag-list ordered by versions.

 It may be case, that people do not follow same version numbering convention. 
 Most people after x.9.x increment major version (that is why they may not be 
 affected because of this)

 Another option like git tag --date-asc can be added which will print tags 
 by creation date. (As long as people do not create backdated tag, this will 
 work).

 I completely agree, and there was a proposal to an option like this a
 long time ago:

 http://article.gmane.org/gmane.comp.version-control.git/111032

 --
 Felipe Contreras
 --
 To unsubscribe from this list: send the line unsubscribe git in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] git-gui: Modify push dialog to support Gerrit review

[Phil Hord]

On Fri, Sep 6, 2013 at 6:30 AM, Joergen Edelbo j...@napatech.com wrote:
 Problem: It is not possible to push for Gerrit review as you will
 always try to push to refs/heads/... on the remote.

 Changes done:

 Add an option to select Gerrit review and a corresponding entry
 for a branch name. If this option is selected, push the changes to
 refs/for/gerrit-branch/local branch. In this way the local branch
 names will be used as topic branches on Gerrit.


In my gerrit repos, I have this configuration

  $  git config remote.origin.push HEAD:refs/for/master

And so I can simply 'git push' and git does what I mean.

My main complaint with git-gui's push is that it ignores my
configuration. Can you teach git-gui to honor this setting, instead?

I would like for this remote.name.push option to be smarter and figure
out which $branch I mean when I am not on master, but that is a different
discussion. Having said that, I see that your change to git-gui attempts to
make that automatic.  Kudos for that, but I don't think this will work for
me as I am often on a detached branch and so refs/heads/$b is meaningless.

Can you think of a sane way to separate the from and the to branches in
the GUI?  I mean, I would like to push HEAD and I would like it to
go to refs/for/frotz-2.0.

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] Fix path prefixing in grep_object

[Phil Hord]

On Tue, Aug 27, 2013 at 12:07 AM, Junio C Hamano gits...@pobox.com wrote:
 Junio C Hamano gits...@pobox.com writes:

 Not necessarily.  If the user is asking the question in a more
 natural way (I want to see where in 'next' branch's tip commit hits
 appear, by the way, I know I am only interested in builtin/ so I'd
 give pathspec as well when I am asking this question), the output
 does give commit colon path, so it is more than coincidence.

 This part needs to be qualified.  Natural is of course in the eyes
 of beholder.  If we assume that your #1 holds true (i.e. the tuple
 in which tree object are we reporting, what path we saw hits is
 important and merging them into one in what blob we saw hits lose
 information),

My #1 is really what I inferred from your statements.  I did not
think the tuple was important, but I agree that may be my
shortsightedness.  If the tuple is important, then it seems to me that
the --null behavior is a bug (the colon is left intact); but changing
it now seems senseless or harmful.

 then it is natural to ask inside origin/master tree,
 where do I have hits?  By the way, I am only interested in builtin/
 and get origin/master:builtin/pack-objects.c as an answer (this is
 from your earlier example), than asking inside origin/master:builtin
 tree, where do I have hits?

 If we do not consider #1 is false and the tree information can be
 discarded, then it does not matter if we converted the colon after
 origin/master to slash when we answer the latter question, and the
 latter question stops being unnatural.

 ...
 but it might be a good change to allow A:B:C to be parsed as a
 proper extended SHA-1 expression and make it yield

   git rev-parse $(git rev-parse $(git rev-parse A):B):C

 Right now, B:C is used as a path inside tree-ish A, but I think
 we can safely fall back, when path B:C does not appear in tree-ish
 A, to see if path B appears in it and is a tree, and then turn it
 into a look-up of path C in that tree A:B.

 And if we want to keep the tree, path tuple, but still want to
 make it easier to work with the output, allowing A:B:C to be parsed
 as an extended SHA-1 expression would be a reasonable solution, not
 a work-around. The answer is given to the question asked in either
 way (either in origin/master, but limited to these pathspecs or
 in the tree origin/master:builtin/) without losing information,
 but the issue you had is that the answer to the latter form of
 question is not understood by the object name parser, which I
 personally think is a bug.

I am beginning to agree, though we discovered some other weird output
from grep which also does not parse. (origin/master:../relative/path).

It seems that fixing this bug could be very confusing for
get_sha1_with_context unless the context was rewritten to match the
traditional syntax.

P
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] Fix path prefixing in grep_object

[Phil Hord]

On Mon, Aug 26, 2013 at 3:14 AM, Junio C Hamano gits...@pobox.com wrote:
 Junio C Hamano gits...@pobox.com writes:

 Jonathan Nieder jrnie...@gmail.com writes:

 I think Phil meant that when git grep is asked to search within
 HEAD:some/path, filenames tacked on at the end should be appended
 with a '/' separator instead of the usual ':' (e.g.,
 HEAD:some/path/inner/path.c, not HEAD:some/path:inner/path.c).

 Ah, OK.

 I am not sure if we have a proper hint in the revision parser
 machinery, but it may not be too hard to teach rev-cmdline interface
 to keep that kind of information (i.e. This tree object name is a
 result of parsing 'tree-ish:path' syntax).

 Actually, having thought about this a bit more, I am no longer sure
 if this is even a good idea to begin with.

 An output line that begins with HEAD:some/path:inner/path.c from
 git grep is an answer to this question: find the given pattern in
 a tree-ish specified with HEAD:some/path.

 On the other hand, HEAD:some/path/inner/path.c is an answer to a
 different question: find the pattern in a tree-ish specified with
 HEAD.  The question may optionally be further qualified with but
 limit the search to some/path.  Both the question and its answer
 are more intuitive than the former one.

I disagree.  The man page says that git grep lists the filenames of
matching files.  And it usually does.  When told to search in a
different branch, the filename is helpfully shown in a form that other
git commands recognize, namely $branch:$path. This is useful for
scripts that want to do something with the resulting file names.

But when a path included in the query with the branch, the output is
useless to my scripts or finger memory without some cleanup first.
The aim of this patch is to fix that so the cleanup is not necessary.

   $ git grep -l setup_check HEAD Documentation
   HEAD:Documentation/technical/api-gitattributes.txt

   $ git grep -l setup_check HEAD:Documentation
   HEAD:Documentation:technical/api-gitattributes.txt

The path in the first example is meaningful.  The path in the second
example is erroneous.


 And we have a nice way to ask that question directly, i.e.

 $ git grep -e pattern HEAD some/path

 which can be extended naturally to more than one path, e.g.

 $ git grep -e pattern HEAD some/path another/hierarchy

 without having to repeat HEAD: part again for each path.

Yes, but that's not always what I want. Sometimes I want to search on
different trees. When doing so, why should I be crippled with broken
output?

$ git grep -e pattern origin/master:some/path origin/next:another/hierarchy
origin/master:some/path:sub/dir/foo.txt
origin/next:another/hierarchy:path/frotz.c

I would prefer to have real paths I can pass to 'git show', ones with
just one meaningful colon rather than two vague ones:

origin/master:some/path/sub/dir/foo.txt
origin/next:another/hierarchy/path/frotz.c

 If the user asked the question of the former form, i.e.

 $ git grep -e pattern HEAD:some/path

 there may be a reason why the user wanted to ask it in that
 (somewhat strange) way.  I am not 100% sure if it is a good idea to
 give an answer to a question different from what the user asked by
 internally rewriting the question to

 $ git grep -e pattern HEAD -- some/path

We are not rewriting the question at all.

The current code assumes the user gave only an object name and is
trying to help by prefixing that name on the matched path using the
colon as a separator, as would be the norm.  But that is the wrong
separator in some cases, specifically when the tree reference includes
a path.

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2] grep: use slash for path delimiter, not colon

[Phil Hord]

When a commit is grepped and matching filenames are printed, grep-objects
creates the filename by prefixing the original cmdline argument to the
matched path separated by a colon.  Normally this forms a valid blob
reference to the filename, like this:

  git grep -l foo HEAD
  HEAD:some/path/to/foo.txt
  ^

But a tree path may be given to grep instead; in this case the colon is
not a valid delimiter to use since it is placed inside a path.

  git grep -l foo HEAD:some
  HEAD:some:path/to/foo.txt
   ^

The slash path delimiter should be used instead.  Fix git grep to
discern the correct delimiter so it can report valid object names.

  git grep -l foo HEAD:some
  HEAD:some/path/to/foo.txt
   ^

Also, prevent the delimiter being added twice, as happens now in these
examples:

  git grep -l foo HEAD:
  HEAD::some/path/to/foo.txt
   ^
  git grep -l foo HEAD:some/
  HEAD:some/:path/to/foo.txt
^

Add a test to confirm correct path forming.
---
This version is a bit more deterministic and also adds a test.

It accepts the expense of examining the path argument again to 
determine if it is a tree-ish + path rather than just a tree (commit).
The get_sha1 call occurs one extra time for each tree-ish argument,
so it's not expensive. We avoid mucking with the object_array API this
way, and also do not rely on the object-type to tell us anything about
the way the object name was spelled.

This one also adds a check to avoid duplicating an extant delimiter.

 builtin/grep.c  |  9 -
 t/t7810-grep.sh | 15 +++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 03bc442..6fc418f 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -480,8 +480,15 @@ static int grep_object(struct grep_opt *opt, const struct 
pathspec *pathspec,
len = name ? strlen(name) : 0;
strbuf_init(base, PATH_MAX + len + 1);
if (len) {
+   struct object_context ctx;
+   unsigned char sha1[20];
+   char delimiter = ':';
+   if (!get_sha1_with_context(name, 0, sha1, ctx) 
+   ctx.path[0]!=0)
+   delimiter='/';
strbuf_add(base, name, len);
-   strbuf_addch(base, ':');
+   if (name[len-1] != delimiter)
+   strbuf_addch(base, delimiter);
}
init_tree_desc(tree, data, size);
hit = grep_tree(opt, pathspec, tree, base, base.len,
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index f698001..2494bfc 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -886,6 +886,21 @@ test_expect_success 'grep -e -- -- path' '
 '
 
 cat expected EOF
+HEAD:t/a/v:1:vvv
+HEAD:t/v:1:vvv
+EOF
+
+test_expect_success grep HEAD -- path/ '
+   git grep -n -e vvv HEAD -- t/ actual 
+   test_cmp expected actual
+'
+
+test_expect_success grep HEAD:path '
+   git grep -n -e vvv HEAD:t/ actual 
+   test_cmp expected actual
+'
+
+cat expected EOF
 hello.c:int main(int argc, const char **argv)
 hello.c:   printf(Hello world.\n);
 EOF
-- 
1.8.4.557.g34b3a2e

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] Fix path prefixing in grep_object

[Phil Hord]

On Mon, Aug 26, 2013 at 12:23 PM, Junio C Hamano gits...@pobox.com wrote:
 Phil Hord phil.h...@gmail.com writes:

 On Mon, Aug 26, 2013 at 3:14 AM, Junio C Hamano gits...@pobox.com wrote:
 Junio C Hamano gits...@pobox.com writes:

 Jonathan Nieder jrnie...@gmail.com writes:

 I think Phil meant that when git grep is asked to search within
 HEAD:some/path, filenames tacked on at the end should be appended
 with a '/' separator instead of the usual ':' (e.g.,
 HEAD:some/path/inner/path.c, not HEAD:some/path:inner/path.c).

 Ah, OK.

 I am not sure if we have a proper hint in the revision parser
 machinery, but it may not be too hard to teach rev-cmdline interface
 to keep that kind of information (i.e. This tree object name is a
 result of parsing 'tree-ish:path' syntax).

 Actually, having thought about this a bit more, I am no longer sure
 if this is even a good idea to begin with.

 An output line that begins with HEAD:some/path:inner/path.c from
 git grep is an answer to this question: find the given pattern in
 a tree-ish specified with HEAD:some/path.

 On the other hand, HEAD:some/path/inner/path.c is an answer to a
 different question: find the pattern in a tree-ish specified with
 HEAD.  The question may optionally be further qualified with but
 limit the search to some/path.  Both the question and its answer
 are more intuitive than the former one.

 I disagree.  The man page says that git grep lists the filenames of
 matching files.

 But you need to realize that the manual page gives a white lie in
 order to stay short enough to be readable.  It does give filenames
 when reading from the working tree, the index or a top-level tree
 object.  When given arbitrary tree object that is not top-level, it
 does give filenames relative to the given tree object, which is the
 answer HEAD:some/path:inner/path.c to the question where in the
 tree HEAD:some/path do we have hits?.

 If the user asked the question of the former form, i.e.

 $ git grep -e pattern HEAD:some/path

 there may be a reason why the user wanted to ask it in that
 (somewhat strange) way.  I am not 100% sure if it is a good idea to
 give an answer to a question different from what the user asked by
 internally rewriting the question to

 $ git grep -e pattern HEAD -- some/path

 We are not rewriting the question at all.

 That statement is trapped in a narrow implementor mind. I know you
 are not rewriting the question in your implementation, but what do
 the end users see? It gives an answer to a question different from
 they asked; the observed behaviour is as if the question was
 rewritten before the machinery went to work.

 If your justification were above says 'there may be a readon why
 the user wanted to ask it in that way', i.e. 'find in this tree
 object HEAD:some/path and report where hits appear', but the reason
 can only be from laziness and/or broken script and the user always
 wants the answer from within the top-level tree-ish, then that
 argument may make some sense. You need to justify why it is OK to
 lose information in the answer by rewriting the colon that separates
 the question (in this tree object) and the answer (at this path
 relative to the tree object given).

 Whether you rewrite the input or the output is not important; you
 are trying to give an answer to a different question, which is what
 I find questionable.

Ok, so if I can summarize what I am inferring from your objection:

 1. The (tree-path, found-path) pair is useful information to get back
from git-grep.

 2. A colon is used to delimit these pieces of information, just as a
colon is used to delimit the filename from the matched-line results.

 3. The fact that the colon is also the separator used in object refs
is mere coincidence; the colon was _not_ chosen because it
conveniently turns the results list into valid object references.  A
comma could have been instead, or even a \t.

Have I got that right?

If so, then I would like to point out to you the convenience I
accidentally encountered using this tool.  Perhaps you didn't realize
how helpful it was when you chose to use a colon there.

On the other hand, perhaps a colon is an unfortunate syntactic
collision which should be corrected in the future.

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] Fix path prefixing in grep_object

[Phil Hord]

On Mon, Aug 26, 2013 at 12:49 PM, Phil Hord phil.h...@gmail.com wrote:
 If so, then I would like to point out to you the convenience I
 accidentally encountered using this tool.  Perhaps you didn't realize
 how helpful it was when you chose to use a colon there.

My itch comes from a case where I am looking for references in some
other branch and I want to narrow my search.

  $ git grep object origin/master
  origin/master:.gitignore:/git-count-objects
  origin/master:.gitignore:/git-fsck-objects
  origin/master:.gitignore:/git-hash-object
  8600 lines more deleted

I find hits in the region that interests me and then I try to zoom in
there.  Conveniently, the path I want to search is right there on my
screen.  So I copy the part of the object reference I want to limit my
search to, and I try again:

   $ git grep object origin/master:builtin/

Now, I find the file that has the code I wanted.  But I want to do
something fancier to it than grep, so this time I copy-and-paste the
filename into my command line after typing 'git show':

   $ git show origin/master:builtin/pack-objects.c | vim -

But this doesn't work if the delimiter used was a colon.

   $ git show origin/master:builtin:pack-objects.c | vim -

I have to edit my copy-and-pasted text, which means I have think about
it a bit, and it interrupts all the other things I was thinking about
already.  Eventually I might learn to do this edit automatically,
except it is not needed most of the time.  It is only needed when I
provide a tree-path instead of a branch space path. In the latter
case, the full path is spelled out for me correctly.  And in all other
cases the filenames provided by git-grep are valid filenames or object
names.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] Fix path prefixing in grep_object

[Phil Hord]

On Mon, Aug 26, 2013 at 1:03 PM, Junio C Hamano gits...@pobox.com wrote:
 Junio C Hamano gits...@pobox.com writes:

 If your justification were above says 'there may be a readon why
 the user wanted to ask it in that way', i.e. 'find in this tree
 object HEAD:some/path and report where hits appear', but the reason
 can only be from laziness and/or broken script and the user always
 wants the answer from within the top-level tree-ish, then that
 argument may make some sense. You need to justify why it is OK to
 lose information in the answer by rewriting the colon that separates
 the question (in this tree object) and the answer (at this path
 relative to the tree object given).

 Whether you rewrite the input or the output is not important; you
 are trying to give an answer to a different question, which is what
 I find questionable.

 For example, one of the cases the proposed change will break that I
 am worried about is a script that wants to take N trees and a
 pattern, and report where in the given trees hits appear, something
 like:

 git grep -c -e $pattern $@ |
 perl -e '
 my @trees = @ARGV;
 my %found = ();
 while () {
 my $line = $_;
 for (@trees) {
 my $tree_prefix = $_ . :;
 my $len = len($tree_prefix);
 if (substr($line, 0, $len) eq $tree_prefix) {
 my ($path_count) = substr($line, $len);
 my ($path, $count) = $path_count =~ 
 /^(.*):(\d+)$/
 $found{$tree} = [$path, $count];
 }
 }
 }
 # Do stats on %found
 ' $@


I do understand there is potential breakage when a script is parsing
the output.  I did not consider that this was a feature someone may
want; it really only looks like a breakage to me, for the reasons I've
already given.

It's surprising just how broken it looks to me (given that I now know
it is not) since all the other filenames output from 'git-grep -l' are
valid filenames or object references.  There is only this one
tree-path instance which does not.  I suppose I will learn to live
with it.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] Fix path prefixing in grep_object

[Phil Hord]

On Mon, Aug 26, 2013 at 1:26 PM, Junio C Hamano gits...@pobox.com wrote:
 Phil Hord phil.h...@gmail.com writes:

 If your justification were above says 'there may be a readon why
 the user wanted to ask it in that way', i.e. 'find in this tree
 object HEAD:some/path and report where hits appear', but the reason
 can only be from laziness and/or broken script and the user always
 wants the answer from within the top-level tree-ish, then that
 argument may make some sense. You need to justify why it is OK to
 lose information in the answer by rewriting the colon that separates
 the question (in this tree object) and the answer (at this path
 relative to the tree object given).

 Whether you rewrite the input or the output is not important; you
 are trying to give an answer to a different question, which is what
 I find questionable.

 Ok, so if I can summarize what I am inferring from your objection:

  1. The (tree-path, found-path) pair is useful information to get back
 from git-grep.

 At least that was the intent. I can be persuaded that your change
 will not break anybody if you successfully argue that it is not a
 useful information, though.

Anyone who is interested in the matched trees from the original
command-line only needs to compare the matched paths' prefixes with
the original args to see which one is responsible for each hit.  But
this is not convincing to me because they may not know the original
args to the grep command.

I do not know if this a good reason to keep supporting this mode,
though.  I can just as easily see some script being confused by four
colons in

origin/master:some/:path/file.c:1:int main()

instead of only three that he is used to because someone passed in a
longer tree-path than expected.

I don't know if I can make that argument.

I think I _can_ argue that the colon separator here is historically
just a part of the filename because it is not affected by --null.

  2. A colon is used to delimit these pieces of information, just as a
 colon is used to delimit the filename from the matched-line results.

  3. The fact that the colon is also the separator used in object refs
 is mere coincidence; the colon was _not_ chosen because it
 conveniently turns the results list into valid object references.  A
 comma could have been instead, or even a \t.

 Not necessarily.  If the user is asking the question in a more
 natural way (I want to see where in 'next' branch's tip commit hits
 appear, by the way, I know I am only interested in builtin/ so I'd
 give pathspec as well when I am asking this question), the output
 does give commit colon path, so it is more than coincidence.

 I do not think it is worth doing only for this particular use case,
 but it might be a good change to allow A:B:C to be parsed as a
 proper extended SHA-1 expression and make it yield

 git rev-parse $(git rev-parse $(git rev-parse A):B):C

 Right now, B:C is used as a path inside tree-ish A, but I think
 we can safely fall back, when path B:C does not appear in tree-ish
 A, to see if path B appears in it and is a tree, and then turn it
 into a look-up of path C in that tree A:B.

That would also solve this problem, usually.  But I don't like it
nearly as much as my patch, and I agree it seems extreme for this one
corner-case.

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] grep: use slash for path delimiter, not colon

[Phil Hord]

On Mon, Aug 26, 2013 at 4:13 PM, Johannes Sixt j...@kdbg.org wrote:
 Am 26.08.2013 21:56, schrieb Jeff King:
 Also, prevent the delimiter being added twice, as happens now in these
 examples:

   git grep -l foo HEAD:
   HEAD::some/path/to/foo.txt
^

 Which one of these two does it print then?

 HEAD:/some/path/to/foo.txt
 HEAD:some/path/to/foo.txt


With my patch it prints the latter.

This is because get_sha1_with_context(HEAD:...) returns an empty
'path' string.  The code decides to use ':' as the delimiter in that
case, but it sees there already is one at the end of HEAD:.

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] grep: use slash for path delimiter, not colon

[Phil Hord]

On Mon, Aug 26, 2013 at 4:52 PM, Jeff King p...@peff.net wrote:
 On Mon, Aug 26, 2013 at 10:13:14PM +0200, Johannes Sixt wrote:

 Am 26.08.2013 21:56, schrieb Jeff King:
  Also, prevent the delimiter being added twice, as happens now in these
  examples:
 
git grep -l foo HEAD:
HEAD::some/path/to/foo.txt
 ^

 Which one of these two does it print then?

 HEAD:/some/path/to/foo.txt
 HEAD:some/path/to/foo.txt

 It should (and does) print the latter.

 But I do note that our pathspec handling for subdirectories seems buggy.
 If you do:

   $ cd Documentation
   $ git grep -l foo | head -1
   RelNotes/1.5.1.5.txt

 that's fine; we limit to the current directory. But then if you do:

   $ git grep -l foo HEAD | head -1
   HEAD:RelNotes/1.5.1.5.txt

 we still limit to the current directory, but the output does not note
 this (it should be HEAD:./RelNotes/1.5.1.5.txt). I think this bug is
 orthogonal to Phil's patch, though.

Maybe not.  My path completes the assumption that the L:R value
returned by grep is an object ref; but Junio still thought it wasn't.
I think this is another case where his view was correct.

There's more bad news on this front.

$ cd Documentation
$ git grep -l foo HEAD .. | head -1
HEAD:../.gitignore

That's not a valid ref, either (though maybe it could be).

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC/PATCH] Fix path prefixing in grep_object

[Phil Hord]

When the pathspec given to grep includes a tree name, the full
name of matched files is assembled using colon as a separator.
If the pathspec includes a tree name, it should use a slash
instead.

Check if the pathspec already names a tree and ref (including
a colon) and use a slash if so.
---

I'm not sure about the detection I used here.  It works, but it is
not terribly robust.  Is there a better way to handle this?  Maybe
something like 'prefix_pathspec(name,);'.

 builtin/grep.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 03bc442..d0deae4 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -480,8 +480,9 @@ static int grep_object(struct grep_opt *opt, const struct 
pathspec *pathspec,
len = name ? strlen(name) : 0;
strbuf_init(base, PATH_MAX + len + 1);
if (len) {
+   int has_colon = !!strchr(name,':');
strbuf_add(base, name, len);
-   strbuf_addch(base, ':');
+   strbuf_addch(base, has_colon?'/':':');
}
init_tree_desc(tree, data, size);
hit = grep_tree(opt, pathspec, tree, base, base.len,
-- 
1.8.4.557.g34b3a2e

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] Fix path prefixing in grep_object

[Phil Hord]

On Sat, Aug 24, 2013 at 9:35 PM, Phil Hord ho...@cisco.com wrote:

 When the pathspec given to grep includes a tree name, the full
 name of matched files is assembled using colon as a separator.
 If the pathspec includes a tree name, it should use a slash
 instead.

 Check if the pathspec already names a tree and ref (including
 a colon) and use a slash if so.

I think I used lots of wrong terminology there.  What do I call these
things?

HEAD:path is a tree.

HEAD is a commit name.

Maybe like this?

  When a tree is given to grep, the full name of matched files
  is assembled using colon as a separator.

  If the tree name includes an object name, as in
  HEAD:some/path, it should use a slash instead.

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Bug] git stash generates a different diff then other commands (diff, add, etc) resulting in merge conflicts!

[Phil Hord]

On Tue, Aug 13, 2013 at 1:31 AM, Luke San Antonio
lukesananto...@gmail.com wrote:

 So I found an isolated case, it's very strange...

 Here's a script!


deleted

Thanks for that.  It was hard to read, but it demonstrates the problem well.


 ... Copy and paste that into a terminal and you should have a recreated
 version of my repository there! Now that the file is partly stashed and
 partly in the index, check out the difference in diffs:

 try:
   git diff --staged
 then try:
   git stash show -p

This one is pretty sneaky.  It is not limited to git-stash.  I can
demonstrate the problem now using 'git merge-file'.

But I can only make this problem show itself when:
   1. The collisions are separated by just one line of common code, and
   2. One of the lines of common code is duplicated in one of the
collisions, and
   3. The first two lines of the file are duplicated, and
   4. One of the first two lines is deleted on one side but not the other.

I have managed to boil the test down to this script:

#-
cat base base
1 duplicate
1 duplicate
3 unchanged
4 will change
5 gets deleted
7 duplicated
8 will change
base

cat left left
1 duplicate
1 duplicate
3 unchanged
4 changed
7 duplicated
6 new line
7 duplicated
8 changed
left

sed -e 1d left  right

git merge-file -p left base right

#-


The result looks like this, showing the duplicate collision:

  1 duplicate
  3 unchanged
  4 changed
   left
  7 duplicated
  6 new line
  7 duplicated
  ===
  7 duplicated
  6 new line
  7 duplicated
   right
  8 changed


But it should look like this instead:

  1 duplicate
  3 unchanged
  4 changed
  7 duplicated
  6 new line
  7 duplicated
  8 changed


A similar (but different) stupidity shows up if you remove line 3
from all three files.

I tested this in 1.6.5 and the same thing occurs there, so this is NOT
recent regression.

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Repo with only one file

[Phil Hord]

On Fri, Aug 9, 2013 at 6:03 AM, shawn wilson ag4ve...@gmail.com wrote:
 On Fri, Aug 9, 2013 at 2:50 AM, Johannes Sixt j.s...@viscovery.net wrote:
 Let's check: After running your command above to remove other files, does
 the command

git filter-branch -f HEAD webban.pl


 Ahha, no but:
 git filter-branch -f HEAD -- webban.pl
 did

 Thanks.

 The question still stands though - why is that unassociated commit left there?

Maybe you need  --prune-empty.

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] t/t7407: fix two typos in submodule tests

[Phil Hord]

In t/t7407-submodule-foreach.sh there is a typo in one of the
path names given for a test step.  The correct path is
nested1/nested2/.git, but nested1/nested1/nested2/.git is
given instead.  The typo is hidden because this line also
accidentally omits the  chain operator.  The omitted chain
also means the return values of all the previous commands in
this test are also being ignored.

Fix the path and add the chain operator so the entire test
sequence can be properly validated.

Signed-off-by: Phil Hord ho...@cisco.com
---
 t/t7407-submodule-foreach.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
index 91d4fd1..be93f10 100755
--- a/t/t7407-submodule-foreach.sh
+++ b/t/t7407-submodule-foreach.sh
@@ -145,7 +145,7 @@ test_expect_success 'use submodule foreach to checkout 
2nd level submodule' '
git rev-parse --resolve-git-dir nested1/.git 
test_must_fail git rev-parse --resolve-git-dir 
nested1/nested2/.git 
git submodule foreach git submodule update --init 
-   git rev-parse --resolve-git-dir nested1/nested1/nested2/.git
+   git rev-parse --resolve-git-dir nested1/nested2/.git 
test_must_fail git rev-parse --resolve-git-dir 
nested1/nested2/nested3/.git
)
 '
-- 
1.8.4.rc1.433.g415b943

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Bug] git stash generates a different diff then other commands (diff, add, etc) resulting in merge conflicts!

[Phil Hord]

On Thu, Aug 8, 2013 at 3:07 AM, Luke San Antonio
lukesananto...@gmail.com wrote:

 Hi, my name's Luke!

 Today, I had a problem merging a stash after immediately creating it.
 This is exactly what I did!

 git stash save --keep-index
 git stash pop

 And BAM! Merge conflict! This was especially weird because my file had
 this in it (taken directly from my code!)


Luke,

I think the issue is that your working directory receives your cached
file when you say 'git stash --keep-index'.  When you restore the
stash, your previous working directory now conflicts with your new
working directory, but neither is the same as HEAD.

Here's a test script to demonstrate the issue, I think.  Did I get
this right, Luke?

 # cd /tmp  rm -rf foo
 git init foo  cd foo
 echo foo  bar   git add bar  git commit -mfoo
 echo bar  bar   git add bar
 echo baz  bar
 echo Before stash  bar: $(cat bar)
 git stash --keep-index
 echo After stash  bar: $(cat bar)
 git stash apply


The output looks like this:

$  git init foo  cd foo
Initialized empty Git repository in /tmp/foo/.git/
$ git commit --allow-empty -mInitialCommit
[master (root-commit) b5ecc7e] InitialCommit
$ echo Bar  bar   git add bar  git commit -mBar
[master 16d708b] Bar
 1 file changed, 1 insertion(+)
 create mode 100644 bar
$ echo bar  bar   git add bar
$  echo baz  bar
$  echo Before stash  bar: $(cat bar)
Before stash  bar: baz
$  git stash --keep-index
Saved working directory and index state WIP on master: 16d708b Bar
HEAD is now at 16d708b Bar
$  echo After stash  bar: $(cat bar)
After stash  bar: bar
$  git stash apply
Auto-merging bar
CONFLICT (content): Merge conflict in bar
Recorded preimage for 'bar'
$ cat bar
 Updated upstream
bar
===
baz
 Stashed changes



Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Repo with only one file

[Phil Hord]

On Wed, Aug 7, 2013 at 5:07 PM, shawn wilson ag4ve...@gmail.com wrote:
 On Wed, Aug 7, 2013 at 6:43 AM, Johannes Sixt j.s...@viscovery.net wrote:
 Am 8/7/2013 8:24, schrieb shawn wilson: ... create a repo for one of
 these scripts and I'd like to keep the commit history.

 Ok, so:
 % find -type f ! -iname webban.pl | while read f; do git
 filter-branch -f --index-filter git rm --cached --ignore-unmatch $f
 HEAD ; done

 Which basically did it. But, I've got this one commit that seems to be
 orphaned - it doesn't change any files.

 Try this:

   git filter-branch HEAD -- webban.pl


  % git filter-branch HEAD -- webban.pl
 Cannot create a new backup.
 A previous backup already exists in refs/original/
 Force overwriting the backup with -f
  % git filter-branch -f HEAD -- webban.pl
 Rewrite 1e04b18c256c996312f167be808733bcc755f1e3 (9/9)
 WARNING: Ref 'refs/heads/master' is unchanged

I think you can ignore the warning.  Maybe you want to create a new
branch which only has this file in it now.

   $ git checkout -b webban

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git clone -b

[Phil Hord]

It would be nice to support more generic specs for the --branch
switch. But it is complicated because the refs have not been fetched
yet during the clone, and so normal refs operations -- which expect to
work on a local repository -- do not work.  So, the ref is looked up
locally from a list in expected locations after fetching the remote
refs but before the clone occurs.  The remote refs which are fetched
is not configurable during clone, and so only 'refs/heads/*' is
fetched for non-mirrors.

I was able to tweak git-clone to fetch the remote ref when I hacked
builtin/clone.c to check in 'refs' and also to extend the refspec to
something more broad (+refs/*:refs/remotes/origin/*), but this is
not a workable solution.  But there probably is a more correct way
than the hack I tried.

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Splitting a rev list into 2 sets

[Phil Hord]

On Thu, Jun 20, 2013 at 6:14 AM, Francis Moreau francis.m...@gmail.com wrote:
 I'd like to write a script that would parse commits in one of my repo.
 Ideally this script should accept any revision ranges that
 git-rev-list would accept.

 This script should consider commits in master differently than the
 ones in others branches.

 To get the commit set which can't be reached by master (ie commits
 which are specific to branches other than master) I would do:

   # $@ is the range spec passed to the script
   git rev-list $@ ^master | check_other_commit

 But I don't know if it's possible to use a different git-rev-list
 command to get the rest of the commits, ie the ones that are reachable
 by the specified range and master.

 One way to do that is to record the first commit set got by the first
 rev-list command and check that the ones returned by git rev-list $@
 are not in the record.

 But I'm wondering if someone can see another solution more elegant ?

I do not know if I would call this elegant, but I think this
codification of your One way to do that is at least small and mostly
readable:

   git rev-list $@ |grep -v -f (git rev-list $@ ^master)

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] fix builtin-* references to be builtin/*

[Phil Hord]

Documentation and some comments still refer to files in builtin/
as 'builtin-*.[cho]'.  Update these to show the correct location.

Signed-off-by: Phil Hord ho...@cisco.com
---
 Documentation/git-log.txt |  4 ++--
 Documentation/technical/api-builtin.txt   |  2 +-
 Documentation/technical/api-parse-options.txt | 12 ++--
 Documentation/user-manual.txt | 10 +-
 builtin/help.c|  2 +-
 builtin/notes.c   |  2 +-
 builtin/replace.c |  2 +-
 transport.c   |  2 +-
 transport.h   |  2 +-
 9 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 4687fe8..2ea79ba 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -128,9 +128,9 @@ Examples
in the release branch, along with the list of paths
each commit modifies.
 
-`git log --follow builtin-rev-list.c`::
+`git log --follow builtin/rev-list.c`::
 
-   Shows the commits that changed builtin-rev-list.c, including
+   Shows the commits that changed builtin/rev-list.c, including
those commits that occurred before the file was given its
present name.
 
diff --git a/Documentation/technical/api-builtin.txt 
b/Documentation/technical/api-builtin.txt
index 4a4228b..f3c1357 100644
--- a/Documentation/technical/api-builtin.txt
+++ b/Documentation/technical/api-builtin.txt
@@ -39,7 +39,7 @@ where options is the bitwise-or of:
on bare repositories.
This only makes sense when `RUN_SETUP` is also set.
 
-. Add `builtin-foo.o` to `BUILTIN_OBJS` in `Makefile`.
+. Add `builtin/foo.o` to `BUILTIN_OBJS` in `Makefile`.
 
 Additionally, if `foo` is a new command, there are 3 more things to do:
 
diff --git a/Documentation/technical/api-parse-options.txt 
b/Documentation/technical/api-parse-options.txt
index 1317db4..0be2b51 100644
--- a/Documentation/technical/api-parse-options.txt
+++ b/Documentation/technical/api-parse-options.txt
@@ -275,10 +275,10 @@ Examples
 
 
 See `test-parse-options.c` and
-`builtin-add.c`,
-`builtin-clone.c`,
-`builtin-commit.c`,
-`builtin-fetch.c`,
-`builtin-fsck.c`,
-`builtin-rm.c`
+`builtin/add.c`,
+`builtin/clone.c`,
+`builtin/commit.c`,
+`builtin/fetch.c`,
+`builtin/fsck.c`,
+`builtin/rm.c`
 for real-world examples.
diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index e831cc2..2483700 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -4256,7 +4256,7 @@ no longer need to call `setup_pager()` directly).
 Nowadays, `git log` is a builtin, which means that it is _contained_ in the
 command `git`.  The source side of a builtin is
 
-- a function called `cmd_bla`, typically defined in `builtin-bla.c`,
+- a function called `cmd_bla`, typically defined in `builtin/bla.c`,
   and declared in `builtin.h`,
 
 - an entry in the `commands[]` array in `git.c`, and
@@ -4264,7 +4264,7 @@ command `git`.  The source side of a builtin is
 - an entry in `BUILTIN_OBJECTS` in the `Makefile`.
 
 Sometimes, more than one builtin is contained in one source file.  For
-example, `cmd_whatchanged()` and `cmd_log()` both reside in `builtin-log.c`,
+example, `cmd_whatchanged()` and `cmd_log()` both reside in `builtin/log.c`,
 since they share quite a bit of code.  In that case, the commands which are
 _not_ named like the `.c` file in which they live have to be listed in
 `BUILT_INS` in the `Makefile`.
@@ -4287,10 +4287,10 @@ For the sake of clarity, let's stay with `git 
cat-file`, because it
 - is plumbing, and
 
 - was around even in the initial commit (it literally went only through
-  some 20 revisions as `cat-file.c`, was renamed to `builtin-cat-file.c`
+  some 20 revisions as `cat-file.c`, was renamed to `builtin/cat-file.c`
   when made a builtin, and then saw less than 10 versions).
 
-So, look into `builtin-cat-file.c`, search for `cmd_cat_file()` and look what
+So, look into `builtin/cat-file.c`, search for `cmd_cat_file()` and look what
 it does.
 
 --
@@ -4366,7 +4366,7 @@ Another example: Find out what to do in order to make 
some script a
 builtin:
 
 -
-$ git log --no-merges --diff-filter=A builtin-*.c
+$ git log --no-merges --diff-filter=A builtin/*.c
 -
 
 You see, Git is actually the best tool to find out about the source of Git
diff --git a/builtin/help.c b/builtin/help.c
index 062957f..ce7b889 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -1,5 +1,5 @@
 /*
- * builtin-help.c
+ * builtin/help.c
  *
  * Builtin help command
  */
diff --git a/builtin/notes.c b/builtin/notes.c
index 57748a6..d9a67d9 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -4,7 +4,7 @@
  * Copyright (c) 2010 Johan

[PATCHv2] fix builtin-* references to be builtin/*

[Phil Hord]

Documentation and some comments still refer to files in builtin/
as 'builtin-*.[cho]'.  Update these to show the correct location.

Signed-off-by: Phil Hord ho...@cisco.com
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Assisted-by: Junio C Hamano gits...@pobox.com
---
 Documentation/git-log.txt |  4 ++--
 Documentation/technical/api-builtin.txt   |  2 +-
 Documentation/technical/api-parse-options.txt | 12 ++--
 Documentation/user-manual.txt | 13 +++--
 builtin/help.c|  2 --
 builtin/notes.c   |  2 +-
 builtin/replace.c |  2 +-
 transport.c   |  2 +-
 transport.h   |  2 +-
 9 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 4687fe8..2ea79ba 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -128,9 +128,9 @@ Examples
in the release branch, along with the list of paths
each commit modifies.
 
-`git log --follow builtin-rev-list.c`::
+`git log --follow builtin/rev-list.c`::
 
-   Shows the commits that changed builtin-rev-list.c, including
+   Shows the commits that changed builtin/rev-list.c, including
those commits that occurred before the file was given its
present name.
 
diff --git a/Documentation/technical/api-builtin.txt 
b/Documentation/technical/api-builtin.txt
index 4a4228b..f3c1357 100644
--- a/Documentation/technical/api-builtin.txt
+++ b/Documentation/technical/api-builtin.txt
@@ -39,7 +39,7 @@ where options is the bitwise-or of:
on bare repositories.
This only makes sense when `RUN_SETUP` is also set.
 
-. Add `builtin-foo.o` to `BUILTIN_OBJS` in `Makefile`.
+. Add `builtin/foo.o` to `BUILTIN_OBJS` in `Makefile`.
 
 Additionally, if `foo` is a new command, there are 3 more things to do:
 
diff --git a/Documentation/technical/api-parse-options.txt 
b/Documentation/technical/api-parse-options.txt
index 1317db4..0be2b51 100644
--- a/Documentation/technical/api-parse-options.txt
+++ b/Documentation/technical/api-parse-options.txt
@@ -275,10 +275,10 @@ Examples
 
 
 See `test-parse-options.c` and
-`builtin-add.c`,
-`builtin-clone.c`,
-`builtin-commit.c`,
-`builtin-fetch.c`,
-`builtin-fsck.c`,
-`builtin-rm.c`
+`builtin/add.c`,
+`builtin/clone.c`,
+`builtin/commit.c`,
+`builtin/fetch.c`,
+`builtin/fsck.c`,
+`builtin/rm.c`
 for real-world examples.
diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index e831cc2..2436124 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -4256,15 +4256,16 @@ no longer need to call `setup_pager()` directly).
 Nowadays, `git log` is a builtin, which means that it is _contained_ in the
 command `git`.  The source side of a builtin is
 
-- a function called `cmd_bla`, typically defined in `builtin-bla.c`,
-  and declared in `builtin.h`,
+- a function called `cmd_bla`, typically defined in `builtin/bla.c`
+  (note that older versions of Git used to have it in `builtin-bla.c`
+  instead), and declared in `builtin.h`.
 
 - an entry in the `commands[]` array in `git.c`, and
 
 - an entry in `BUILTIN_OBJECTS` in the `Makefile`.
 
 Sometimes, more than one builtin is contained in one source file.  For
-example, `cmd_whatchanged()` and `cmd_log()` both reside in `builtin-log.c`,
+example, `cmd_whatchanged()` and `cmd_log()` both reside in `builtin/log.c`,
 since they share quite a bit of code.  In that case, the commands which are
 _not_ named like the `.c` file in which they live have to be listed in
 `BUILT_INS` in the `Makefile`.
@@ -4287,10 +4288,10 @@ For the sake of clarity, let's stay with `git 
cat-file`, because it
 - is plumbing, and
 
 - was around even in the initial commit (it literally went only through
-  some 20 revisions as `cat-file.c`, was renamed to `builtin-cat-file.c`
+  some 20 revisions as `cat-file.c`, was renamed to `builtin/cat-file.c`
   when made a builtin, and then saw less than 10 versions).
 
-So, look into `builtin-cat-file.c`, search for `cmd_cat_file()` and look what
+So, look into `builtin/cat-file.c`, search for `cmd_cat_file()` and look what
 it does.
 
 --
@@ -4366,7 +4367,7 @@ Another example: Find out what to do in order to make 
some script a
 builtin:
 
 -
-$ git log --no-merges --diff-filter=A builtin-*.c
+$ git log --no-merges --diff-filter=A builtin/*.c
 -
 
 You see, Git is actually the best tool to find out about the source of Git
diff --git a/builtin/help.c b/builtin/help.c
index 062957f..f1e236b 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -1,6 +1,4 @@
 /*
- * builtin-help.c
- *
  * Builtin help command
  */
 #include cache.h
diff --git a/builtin

Re: [PATCH 1/2] sha1_name: stop hard-coding 40-character hex checks

[Phil Hord]

On Sat, Jun 15, 2013 at 1:38 PM, Ramkumar Ramachandra
artag...@gmail.com wrote:
 In two places, get_sha1_basic() assumes that strings are possibly sha1
 hexes if they are 40 characters long, and calls get_sha1_hex() in these
 two cases.  This 40-character check is ugly and wrong: there is nothing
 preventing a revision or branch name from being exactly 40 characters.
 Replace it with a call to the more robust get_short_sha1().

I share your disdain for the bare '40's in the code.  But I think this
code is less clear than the previous version with the magic number.


 Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
 ---
  sha1_name.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

 diff --git a/sha1_name.c b/sha1_name.c
 index 90419ef..d862af3 100644
 --- a/sha1_name.c
 +++ b/sha1_name.c
 @@ -451,7 +451,7 @@ static int get_sha1_basic(const char *str, int len, 
 unsigned char *sha1)
 int refs_found = 0;
 int at, reflog_len, nth_prior = 0;

 -   if (len == 40  !get_sha1_hex(str, sha1)) {
 +   if (!get_short_sha1(str, strlen(str), sha1, GET_SHA1_QUIETLY)) {

Use len instead of strlen(str) here.  It's faster and more correct.

But also get_short_sha1 is much heavier than get_sha1_hex and does not
seem appropriate here.

 refs_found = dwim_ref(str, len, tmp_sha1, real_ref);
 if (refs_found  0  warn_ambiguous_refs) {
 warning(warn_msg, len, str);
 @@ -492,9 +492,9 @@ static int get_sha1_basic(const char *str, int len, 
 unsigned char *sha1)
 int detached;

 if (interpret_nth_prior_checkout(str, buf)  0) {
 -   detached = (buf.len == 40  !get_sha1_hex(buf.buf, 
 sha1));
 +   detached = get_short_sha1(buf.buf, buf.len, sha1, 
 GET_SHA1_QUIETLY);
 strbuf_release(buf);
 -   if (detached)
 +   if (detached != SHORT_NAME_NOT_FOUND)

The semantic meaning of 'detached' seems less clear now if you have to
compare against an enumerated constant to determine the result.  But
also, I do not see why you have to test '!= SHORT_NAME_NOT_FOUND' here
but you did not have to in the other instance.

I think it would be improved if you did this comparison in the
assignment of detached so 'detached' could keep its original boolean
meaning.

But anyway, having looked inside get_short_sha1, it really does seem
to do much more than you want here.

 return 0;
 }
 }
 --
 1.8.3.1.438.g96d34e8

 --
 To unsubscribe from this list: send the line unsubscribe git in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] pull: respect rebase.autostash

[Phil Hord]

On Fri, Jun 14, 2013 at 6:41 AM, Ramkumar Ramachandra
artag...@gmail.com wrote:
 Matthieu Moy wrote:
 It would be nice to have an --autostash command-line option too,

 I thought it would be a bit ugly, since it's already overloaded with
 options to pass to merge.

Eventually I think a switch will be needed.  At least there is
   git -c rebase.autostash=false

But we will probably need to also have --no-autostash for scripts and users.


Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] pull: respect rebase.autostash

[Phil Hord]

On Fri, Jun 14, 2013 at 4:56 AM, Ramkumar Ramachandra
artag...@gmail.com wrote:
 If a rebasing pull is requested, pull unconditionally runs
 require_clean_worktree() resulting in:

   # dirty worktree or index
   $ git pull
   Cannot pull with rebase: Your index contains uncommitted changes.
   Please commit or stash them.

 It does this to inform the user early on that a rebase cannot be run on
 a dirty worktree, and that a stash is required.  However,
 rr/rebase-autostash lifts this limitation on rebase by providing a way
 to automatically stash using the rebase.autostash configuration
 variable.  Read this variable in pull, and take advantage of this
 feature.

This commit message does not tell me what this commit does.  It mostly
describes the current situation.  Then it refers to something called
rr/rebase-autostash which will lose meaning in the future when this
commit is no longer current on the list.  A better way to refer to
this commit is to say this commit.  However, even this is not the
norm for this project.  The norm here is to avoid such noise by
speaking in the imperative mood.  That is, do not tell me what this
commit does; instead, tell the code what to do.  See
Documentation/SubmittingPatches:

  Describe your changes in imperative mood, e.g. make xyzzy do frotz
  instead of [This patch] makes xyzzy do frotz or [I] changed xyzzy
  to do frotz, as if you are giving orders to the codebase to change
  its behaviour.  Try to make sure your explanation can be understood
  without external resources. Instead of giving a URL to a mailing list
  archive, summarize the relevant points of the discussion.




 Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
 ---
  git-pull.sh |  2 ++
  t/t5520-pull.sh | 11 +++
  2 files changed, 13 insertions(+)

 diff --git a/git-pull.sh b/git-pull.sh
 index 638aabb..fb01763 100755
 --- a/git-pull.sh
 +++ b/git-pull.sh
 @@ -44,6 +44,7 @@ merge_args= edit=
  curr_branch=$(git symbolic-ref -q HEAD)
  curr_branch_short=${curr_branch#refs/heads/}
  rebase=$(git config --bool branch.$curr_branch_short.rebase)
 +autostash=$(git config --bool rebase.autostash)
  if test -z $rebase
  then
 rebase=$(git config --bool pull.rebase)
 @@ -203,6 +204,7 @@ test true = $rebase  {
 die $(gettext updating an unborn branch with 
 changes added to the index)
 fi
 else
 +   test true = $autostash ||
 require_clean_work_tree pull with rebase Please commit or 
 stash them.
 fi

This commit does not seem useful on its own.  All it does is prevent
the safety check for a clean work tree when the autostash flag is set.
 I understand that this is necessary for the rest of the change to be
useful, but I do not see any reason for it to be split into two
commits like this.  I think it would be more understandable if this
were squashed together with the rest of the change, both now for
reviews and in the future when someone tries to understand this change
in retrospect.

In particular, the commit message suggests that this commit will
perform the autostash if this variable is set, but it does not do that
yet.  I think if you squash these two together it will be more concise
and understandable.

Thanks,
Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] pull: respect rebase.autostash

[Phil Hord]

On Fri, Jun 14, 2013 at 8:12 AM, Phil Hord phil.h...@gmail.com wrote:
 On Fri, Jun 14, 2013 at 4:56 AM, Ramkumar Ramachandra
 artag...@gmail.com wrote:
 If a rebasing pull is requested, pull unconditionally runs
 require_clean_worktree() resulting in:

   # dirty worktree or index
   $ git pull
   Cannot pull with rebase: Your index contains uncommitted changes.
   Please commit or stash them.

 It does this to inform the user early on that a rebase cannot be run on
 a dirty worktree, and that a stash is required.  However,
 rr/rebase-autostash lifts this limitation on rebase by providing a way
 to automatically stash using the rebase.autostash configuration
 variable.  Read this variable in pull, and take advantage of this
 feature.

 This commit message does not tell me what this commit does.  It mostly
 describes the current situation.  Then it refers to something called
 rr/rebase-autostash which will lose meaning in the future when this
 commit is no longer current on the list.  A better way to refer to
 this commit is to say this commit.  However, even this is not the
 norm for this project.  The norm here is to avoid such noise by
 speaking in the imperative mood.  That is, do not tell me what this
 commit does; instead, tell the code what to do.  See
 Documentation/SubmittingPatches:

   Describe your changes in imperative mood, e.g. make xyzzy do frotz
   instead of [This patch] makes xyzzy do frotz or [I] changed xyzzy
   to do frotz, as if you are giving orders to the codebase to change
   its behaviour.  Try to make sure your explanation can be understood
   without external resources. Instead of giving a URL to a mailing list
   archive, summarize the relevant points of the discussion.




 Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
 ---
  git-pull.sh |  2 ++
  t/t5520-pull.sh | 11 +++
  2 files changed, 13 insertions(+)

 diff --git a/git-pull.sh b/git-pull.sh
 index 638aabb..fb01763 100755
 --- a/git-pull.sh
 +++ b/git-pull.sh
 @@ -44,6 +44,7 @@ merge_args= edit=
  curr_branch=$(git symbolic-ref -q HEAD)
  curr_branch_short=${curr_branch#refs/heads/}
  rebase=$(git config --bool branch.$curr_branch_short.rebase)
 +autostash=$(git config --bool rebase.autostash)
  if test -z $rebase
  then
 rebase=$(git config --bool pull.rebase)
 @@ -203,6 +204,7 @@ test true = $rebase  {
 die $(gettext updating an unborn branch with 
 changes added to the index)
 fi
 else
 +   test true = $autostash ||
 require_clean_work_tree pull with rebase Please commit or 
 stash them.
 fi

 This commit does not seem useful on its own.  All it does is prevent
 the safety check for a clean work tree when the autostash flag is set.
  I understand that this is necessary for the rest of the change to be
 useful, but I do not see any reason for it to be split into two
 commits like this.  I think it would be more understandable if this
 were squashed together with the rest of the change, both now for
 reviews and in the future when someone tries to understand this change
 in retrospect.

 In particular, the commit message suggests that this commit will
 perform the autostash if this variable is set, but it does not do that
 yet.  I think if you squash these two together it will be more concise
 and understandable.

 Thanks,
 Phil

Having said all that, I see now that 2/2 in this series is really
unrelated to this commit and is not the rest of the autostash
implementation, so I am more confused than before.  Was the rest of
'autostash' already implemented in some other series?  I guess that's
what you meant by rr/rebase-autostash which is NOT this commit
after all.  I am sorry for my confusion, though a clearer commit
message would have helped me in the first place.

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] stash doc: add a warning about using create

[Phil Hord]

On Fri, Jun 14, 2013 at 6:32 AM, Ramkumar Ramachandra
artag...@gmail.com wrote:
 Add a note saying that the user probably wants save in the create
 description.  While at it, document that it can optionally take a
 message in the synopsis.

 Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
 ---
  Documentation/git-stash.txt | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

 diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
 index 711ffe1..83bb3db 100644
 --- a/Documentation/git-stash.txt
 +++ b/Documentation/git-stash.txt
 @@ -16,7 +16,7 @@ SYNOPSIS
  'git stash' [save [--patch] [-k|--[no-]keep-index] [-q|--quiet]
  [-u|--include-untracked] [-a|--all] [message]]
  'git stash' clear
 -'git stash' create
 +'git stash' create [message]

  DESCRIPTION
  ---
 @@ -151,6 +151,7 @@ create::

 Create a stash (which is a regular commit object) and return its
 object name, without storing it anywhere in the ref namespace.
 +   This is probably not what you want to use; see save above.

Thanks, this is helpful.

Maybe you can also hint why this command exists.

  +   This is intended to be useful for scripts.  It is probably not the
  +   command you want to use; see save above.



  DISCUSSION
 --
 1.8.3.1.383.g0d5ad6b

 --
 To unsubscribe from this list: send the line unsubscribe git in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] [submodule] handle multibyte characters in name

[Phil Hord]

On Tue, Jun 11, 2013 at 7:04 PM, Fredrik Gustafsson iv...@iveqy.com wrote:
 Bugg reported here:
 http://thread.gmane.org/gmane.comp.version-control.git/218922/focus=226791

 Note that newline (\n) is still not supported and will not be until the
 sh-script is replaced by something in an other language. This however
 let us to use mostly all other strange characters.

Please explain the commit better so the reader can understand what the
commit does and why.  I can see what's going on by reading the commit
and the original thread, but I should not have to.

Thanks,
Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 00/36] Massive improvents to rebase and cherry-pick

[Phil Hord]

On Sun, Jun 9, 2013 at 3:37 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:

 On Sun, Jun 9, 2013 at 2:24 PM, Felipe Contreras
 felipe.contre...@gmail.com wrote:
  Same as before, but:

 Also, remove the patches from Martin von Zweigbergk, because
 apparently some people have trouble understanding that they were not
 part of this series.


Please try not to sound disgruntled. This attitude is toxic. You have
turned this change into a complaint: that some people have trouble
understanding which shows a genuine lack of understanding and
compassion on your part.  Instead you can phrase your change notes
more helpfully if you make changes only when you yourself actually
believe the change should be made.  If you cannot do this, perhaps you
can pretend.

  Also, remove the patches from Martin von Zweigbergk, which
  are not a part of this series.

Or even this:

  Also, remove the patches from Martin von Zweigbergk to avoid
  confusing reviewers.

Thanks,
Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v5 00/36] Massive improvents to rebase and cherry-pick

[Phil Hord]

On Mon, Jun 10, 2013 at 7:43 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 On Mon, Jun 10, 2013 at 5:55 PM, Phil Hord phil.h...@gmail.com wrote:
 On Sun, Jun 9, 2013 at 3:37 PM, Felipe Contreras
 felipe.contre...@gmail.com wrote:

 On Sun, Jun 9, 2013 at 2:24 PM, Felipe Contreras
 felipe.contre...@gmail.com wrote:
  Same as before, but:

 Also, remove the patches from Martin von Zweigbergk, because
 apparently some people have trouble understanding that they were not
 part of this series.

 Please try not to sound disgruntled. This attitude is toxic. You have
 turned this change into a complaint: that some people have trouble
 understanding which shows a genuine lack of understanding and
 compassion on your part.  Instead you can phrase your change notes
 more helpfully if you make changes only when you yourself actually
 believe the change should be made.  If you cannot do this, perhaps you
 can pretend.

 That would be dishonest. Moreover, there wasn't a good reason to
 remove these patches, I made it clear I added those patches only to
 make sure the real patches of this series worked correctly. Also, I
 clarified that to Thomas Rast[1], only to receive a totally
 unconstructive comment[2].

 Why don't you ask Thomas Rast to be more constructive[2]?

 Then Johan Herland uses that as an example of a constructive
 comment[3]. Why don't you correct Johan Herland?

I do not see what their comments have to do with your attitude.
Aren't your own man with cogent self-will and personal responsibility?
 Why should I also have to consider these other emails which I have
not bothered to read yet?

 No, you pick the easy target: me.

You seem to have mistaken me for someone else.  Moreover, you seem to
have mistaken you for someone else.  You are the least easy target I
know of on this list.  Everyone else seems open to community
standards.


 I already dd more than my fair share by carrying these 36 patches
 through several iterations, yet you ask *more* of me. Why don't you
 ask more of the people that just hit reply on their MUA?

 Thomas' task was easy; he simply had to say Oh, these aren't meant to
 be applied, got it.

 [1] http://article.gmane.org/gmane.comp.version-control.git/227039
 [2] http://article.gmane.org/gmane.comp.version-control.git/227040
 [3] http://article.gmane.org/gmane.comp.version-control.git/227102

I did not comment on their posts because they did not catch my eye.
Rebase and cherry-pick improvements are interesting to me, so I read
your post.  I will try not to make this mistake again.

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Re: Re: What's cooking in git.git (May 2013, #09; Wed, 29)

[Phil Hord]

On Tue, Jun 4, 2013 at 8:48 AM, John Keeping j...@keeping.me.uk wrote:
 On Tue, Jun 04, 2013 at 09:17:17PM +1000, Heiko Voigt wrote:
 On Tue, Jun 04, 2013 at 09:10:45AM +0100, John Keeping wrote:
  On Tue, Jun 04, 2013 at 03:29:51PM +1000, Heiko Voigt wrote:
   On Mon, Jun 03, 2013 at 11:23:41PM +0100, John Keeping wrote:
 Sorry, I should have been more specific here. I saw that you did some
 changes to make submodule add do the right thing with relative 
 paths,
 but the following change to t7406 does not work like I believe it
 should but instead makes the test fail:
 ---8-
 diff --git a/t/t7406-submodule-update.sh 
 b/t/t7406-submodule-update.sh
 index a4ffea0..9766b9e 100755
 --- a/t/t7406-submodule-update.sh
 +++ b/t/t7406-submodule-update.sh
 @@ -559,7 +559,9 @@ test_expect_success 'add different submodules to 
 the same pa
  test_expect_success 'submodule add places git-dir in superprojects 
 git-dir' '
 (cd super 
  mkdir deeper 
 -git submodule add ../submodule deeper/submodule 
 +(cd deeper 
 + git submodule add ../../submodule submodule
 +) 
  (cd deeper/submodule 
   git log  ../../expected
  ) 
 ---8-
   
Ah, ok.  I think this case is problematic because the repository
argument is either relative to remote.origin.url or to the top of the
working tree if there is no origin remote.  I wonder if we should 
just
die when a relative path is given for the repository and we're not at
the top of the working tree.
  
   Why not behave as if we are at the top of the working tree for relative
   paths? If there is an origin remote thats fine. If there is no origin
   remote you could warn that the path used is taken relative from the root
   of the superproject during add. What do you think?
 
  That's what the patch currently queued on pu does, which Jens wants to
  change, isn't it?

 True I did not realize this when reading it the first time. But I think
 we should still not die when in a subdirectory. After all this series is
 trying to archive that the submodule command works in subdirectories
 seamlessly right? So you probably want to translate a relative path
 without origin remote given from a subdirectory to the superproject
 level and use that. Then you do not have to die.

 The problem is that sometimes you do want to adjust the path and
 sometimes you don't.  Reading git-submodule(1), it says:

  This may be either an absolute URL, or (if it begins with ./ or
  ../), the location relative to the superproject’s origin
  repository.
  [snip]
  If the superproject doesn’t have an origin configured the
  superproject is its own authoritative upstream and the current
  working directory is used instead.

 So I think it's quite reasonable to have a server layout that looks like
 this:

 project
 |- libs
 |  |- libA
 |  `- libB
 |- core.git

 and with only core.git on your local system do:

 cd core/libs
 git submodule add ../libs/libB

 expecting that to point to libB.  But if we adjust the path then the
 user has to do:

 git submodule add ../../libs/libB

 However, it is also perfectly reasonable to have no remote configured
 and the library next to the repository itself.  In which case we do want
 to specify the additional ../ so that shell completion works in the
 natural way.

In submodule add, the leading '../' prefix on the repository url has
always meant that the url is relative to the url of the current repo.
The given repo-url is precisely what ends up in .gitmodules'
submodule.$name.url.  It works this way whether there is a remote
configured or not.

It does seem like we need to be cautious around this change.

 The only way I can see to resolve the ambiguity is to die when we hit
 this particular case.  This should be acceptable because people
 shouldn't be adding new submodules anywhere near as often as they
 perform other submodule operations and it doesn't affect absolute URLs.

I don't think I like that.  But I don't know if I like anything I
dreamed up, either.

P
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] trivial: Add missing period in documentation

[Phil Hord]

---
 Documentation/diff-options.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 104579d..b8a9b86 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -480,7 +480,7 @@ endif::git-format-patch[]
 
 --ignore-submodules[=when]::
Ignore changes to submodules in the diff generation. when can be
-   either none, untracked, dirty or all, which is the default
+   either none, untracked, dirty or all, which is the default.
Using none will consider the submodule modified when it either 
contains
untracked or modified files or its HEAD differs from the commit recorded
in the superproject and can be used to override any settings of the
-- 
1.8.3.426.gea353ce.dirty

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] diffcore-pickaxe doc: document -S and -G properly

[Phil Hord]

On Fri, May 24, 2013 at 5:37 AM, Ramkumar Ramachandra
artag...@gmail.com wrote:
 Junio C Hamano wrote:
 [...]

 I agree with the other comments, and have made suitable changes.
 Let's review your block now.

 This transformation is used to find filepairs that represent
 two kinds of changes, and is controlled by the -S, -G and
 --pickaxe-all options.

 Why do you call this a transformation?  Is git log --author=foo a
 transformation on the git-log output?  Then how is git log -Sfoo a
 transformation?

 Two kinds of changes controlled by three different options?  Isn't the
 original much clearer?

They are all three filters.  They transform the output by limiting it
to commits which meet specific conditions.  Transformation is used in
the network-graphs sense of the word.  It fits the beginning of the
document where it says this:

  The diffcore mechanism is fed a list of such comparison results
  (each of which is called filepair, although at this point each
  of them talks about a single file), and transforms such a list
  into another list.  There are currently 5 such transformations:

  - diffcore-break
  - diffcore-rename
  - diffcore-merge-broken
  - diffcore-pickaxe
  - diffcore-order
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git stash deletes/drops changes of

[Phil Hord]

On Thu, May 23, 2013 at 7:20 PM, Junio C Hamano gits...@pobox.com wrote:
 Thomas Rast tr...@inf.ethz.ch writes:

 What are the workflows that are helped if we had such a bit?  If
 we need to support them, I think you need a real --ignore-changes
 bit, not an abuse of --assume-unchanged.

 I gather -- from #git -- that it's mostly used for config files, which
 have an annoying habit of being different from the repository.

 Which is wrong, really.  But we still claim that --assume-unchanged is a
 solution to it in git-update-index(1).

 That is doubly wrong, then ;-)

 How would we want to proceed from here?  The obvious first step
 would be to fix the documentation, but then what is next?

 Thinking aloud, ignoring that Which is wrong, really part in your
 message and assuming that we do want to support --ignore-changes


The wording of --ignore-changes suffers the same lack of clarity that
--assume-unchanged does.

  --assume-unchanged : These changes are ephemeral

  --ignore-changes : These changes are precious but not to be committed

What's better?  --sequester is probably too obscure.  Maybe --hold.
Or --silence.  Or --shut-up.

Does this mean a new class of files for git-status?  Added, changed,
untracked, ignored and held?

I wonder if there is a use case for such a switch to be applied to
content rather than files.  Suppose I want to --hold these changes,
but further changes to the same files should be fair game?  That's
probably an insane situation for some future itch and not this one.
Anyway, it sounds like it would involve the index or maybe a 2nd
index.

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/6] test-lib: allow prefixing a custom string before ok N etc.

[Phil Hord]

On Fri, May 17, 2013 at 4:00 AM, Thomas Rast tr...@inf.ethz.ch wrote:
 Phil Hord phil.h...@gmail.com writes:

 On Thu, May 16, 2013 at 4:50 PM, Thomas Rast tr...@inf.ethz.ch wrote:
 This is not really meant for external use, but allows the next commit
 to neatly distinguish between sub-tests and the main run.

 Maybe we do not care about standards for this library or for your
 use-case, but placing this prefix before the {ok,not ok} breaks the
 TAProtocol.
 http://podwiki.hexten.net/TAP/TAP.html?page=TAP

 Maybe you can put the prefix _after_ the {ok, not ok} and test number.

 Actually that was half on purpose.  You will notice I did not document
 that option, as it is intended only to be used to distinguish between
 the parallel runs implemented in [6/6].

 Those parallel runs look something like

 [4] ok 1 - plain
 [4] ok 2 - plain nested in bare
 [...snip until othes catch up...]
 [4] ok 33 - re-init to update git link
 [4] ok 34 - re-init to move gitdir
 [3] ok 1 - plain
 [2] ok 1 - plain
 [4] ok 35 - re-init to move gitdir symlink
 [4] # still have 2 known breakage(s)
 [4] # passed all remaining 33 test(s)
 [4] 1..35
 [3] ok 2 - plain nested in bare

 It's invalid TAP no matter what: there are N plans and the ok/not ok
 lines from N runs all intermingled.  So I'd rather not even pretend that
 it is valid in any way.

Yes, I guessed this might have been the goal.  Maybe you can mention
it in the commit message.

I hope some future change might even unwind these back into a valid
continuous TAP stream.  But at least for now, if someone needs such a
stream, she can unwind it herself.

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] branch: show me the hot branches

[Phil Hord]

On Tue, May 14, 2013 at 7:34 PM, Junio C Hamano gits...@pobox.com wrote:
 Phil Hord phil.h...@gmail.com writes:

 I imagine it with --date-order and whatnot.

 Perhaps modeled after this one.

 git for-each-ref \
 --format='%(refname:short) %(subject)'
 --sort='-committerdate' refs/heads/


Nice.  I had no idea about for-each-ref.  I knew it was out there, but
not what it could do.  Another tool in the swiss army knife that is
git.

I think Duy was right about his patch's merits.

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] diffcore-pickaxe doc: document -S and -G properly

[Phil Hord]

On Tue, May 14, 2013 at 10:12 AM, Ramkumar Ramachandra
artag...@gmail.com wrote:
 The documentation of -S and -G is very sketchy.  Completely rewrite the
 sections in Documentation/diff-options.txt and
 Documentation/gitdiffcore.txt.

 References:
 52e9578 ([PATCH] Introducing software archaeologist's tool pickaxe.)
 f506b8e (git log/diff: add -Gregexp that greps in the patch text)

 Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
 ---
  I spent some time reading the code and history to figure out what -S
  and -G really do.  I hope I've done justice.

  Documentation/diff-options.txt | 35 +++---
  Documentation/gitdiffcore.txt  | 43 
 +++---
  2 files changed, 52 insertions(+), 26 deletions(-)

 diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
 index 104579d..765abc5 100644
 --- a/Documentation/diff-options.txt
 +++ b/Documentation/diff-options.txt
 @@ -383,14 +383,35 @@ ifndef::git-format-patch[]
 that matches other criteria, nothing is selected.

  -Sstring::
 -   Look for differences that introduce or remove an instance of
 -   string. Note that this is different than the string simply
 -   appearing in diff output; see the 'pickaxe' entry in
 -   linkgit:gitdiffcore[7] for more details.
 +   Look for commits where the specified string was added or
 +   removed.  More precisely, find commits that change the number
 +   of occurrences of the specified string.
 ++
 +It is often useful when you're looking for an exact string (like a
 +function prototype), and want to know the history of that string since
 +it first came into being.

  -Gregex::

It looks like you have deleted the -S and -G references here.  Am I
reading this right?

 -   Look for differences whose added or removed line matches
 -   the given regex.
 +   Grep through the patch text of commits for added/removed lines
 +   that match regex.  `--pickaxe-regex` is implied in this
 +   mode.
 ++
 +To illustrate the difference between `-Sregex --pickaxe-regex` and
 +`-Gregex`, consider a commit with the following diff in the same
 +file:
 ++
 +
 ++return !regexec(regexp, two-ptr, 1, regmatch, 0);
 +...
 +-hit = !regexec(regexp, mf2.ptr, 1, regmatch, 0);
 +
 ++
 +While `git log -Gregexec\(regexp` will show this commit, `git log
 +-Sregexec\(regexp --pickaxe-regex` will not (because the number of
 +occurrences of that string didn't change).

References to git-log seem out of place to me here in git-diffcore.  I
know it's only an example, but it seems that Git normally describes
these 'reference selectors' more generically.  The generic description
may be more confusing to new users, but this patch is not the place to
consider whether it should change.

 ++
 +See the 'pickaxe' entry in linkgit:gitdiffcore[7] for more
 +information.

  --pickaxe-all::
 When `-S` or `-G` finds a change, show all the changes in that
 @@ -399,7 +420,7 @@ ifndef::git-format-patch[]

  --pickaxe-regex::
 Make the string not a plain string but an extended POSIX
 -   regex to match.
 +   regex to match.  Implied when using `-G`.
  endif::git-format-patch[]

  -Oorderfile::
 diff --git a/Documentation/gitdiffcore.txt b/Documentation/gitdiffcore.txt
 index 568d757..39b9c51 100644
 --- a/Documentation/gitdiffcore.txt
 +++ b/Documentation/gitdiffcore.txt
 @@ -222,25 +222,30 @@ version prefixed with '+'.
  diffcore-pickaxe: For Detecting Addition/Deletion of Specified String
  -

 -This transformation is used to find filepairs that represent
 -changes that touch a specified string, and is controlled by the
 --S option and the `--pickaxe-all` option to the 'git diff-*'
 -commands.
 -
 -When diffcore-pickaxe is in use, it checks if there are
 -filepairs whose result side and whose origin side have
 -different number of specified string.  Such a filepair represents
 -the string appeared in this changeset.  It also checks for the
 -opposite case that loses the specified string.
 -
 -When `--pickaxe-all` is not in effect, diffcore-pickaxe leaves
 -only such filepairs that touch the specified string in its
 -output.  When `--pickaxe-all` is used, diffcore-pickaxe leaves all
 -filepairs intact if there is such a filepair, or makes the
 -output empty otherwise.  The latter behaviour is designed to
 -make reviewing of the changes in the context of the whole
 -changeset easier.
 -
 +There are two kinds of pickaxe: the S kind (corresponding to 'git log
 +-S') and the G kind (corresponding to 'git log -G').

While the switches are called -S and -G, I do not think it is helpful
to name the two pickaxe options as the S kind and the G kind.

 +
 +The S kind detects filepairs whose result side and origin side
 +have different number of occurrences of specified string.  While
 +rename detection works as usual, 'git log -S' cannot omit 

Re: [PATCH] diffcore-pickaxe doc: document -S and -G properly

[Phil Hord]

On Tue, May 14, 2013 at 1:44 PM, Phil Hord phil.h...@gmail.com wrote:
 On Tue, May 14, 2013 at 10:12 AM, Ramkumar Ramachandra
 artag...@gmail.com wrote:

  -Sstring::
 -   Look for differences that introduce or remove an instance of
 -   string. Note that this is different than the string simply
 -   appearing in diff output; see the 'pickaxe' entry in
 -   linkgit:gitdiffcore[7] for more details.
 +   Look for commits where the specified string was added or
 +   removed.  More precisely, find commits that change the number
 +   of occurrences of the specified string.
 ++
 +It is often useful when you're looking for an exact string (like a
 +function prototype), and want to know the history of that string since
 +it first came into being.

  -Gregex::

 It looks like you have deleted the -S and -G references here.  Am I
 reading this right?

Doy!  Yes, I was reading it wrong.  Sorry for the noise there.

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] diffcore-pickaxe doc: document -S and -G properly

[Phil Hord]

On Tue, May 14, 2013 at 2:44 PM, Ramkumar Ramachandra
artag...@gmail.com wrote:
 Phil Hord wrote:
 References to git-log seem out of place to me here in git-diffcore.  I
 know it's only an example, but it seems that Git normally describes
 these 'reference selectors' more generically.  The generic description
 may be more confusing to new users, but this patch is not the place to
 consider whether it should change.

 It's not for new users at all.  The most useful application of -S and
 -G is in log.  The translation from a log -G to a diffcore -G is not
 obvious at all, and warrants an explanation.

 Oh, and for the user.  No user is going to type out `man gitdiffcore`
 out of the blue: she's most probably led there from log, and we're
 connecting the dots for her.

She may have been led here by some other help topic, too.  Maybe the
git log examples belong in the git-log documentation.

 While the switches are called -S and -G, I do not think it is helpful
 to name the two pickaxe options as the S kind and the G kind.

 How do you describe something precisely without loss of meaning?  You
 stop abstracting unnecessarily.  Read the sources: you will literally
 see DIFF_PICKAXE_KIND_G there.

S and G are abstractions.

DIFF_PICKAXE_KIND_G is an implementer's distinction.

I agree this is a tricky subject.  When writing technical
documentation you must be precise and clear.  It is easy to forget
both.  It is common to forget to be either clear or precise.  It is
very difficult to be both clear and precise.

So, my suggestion is to use some meaningful names for the action of -S
and -G.  Relating these names to -S and -G in a clear way is likely to
be difficult.

 +
 +The S kind detects filepairs whose result side and origin side
 +have different number of occurrences of specified string.  While
 +rename detection works as usual, 'git log -S' cannot omit commits

 The cannot omit feels like a confusing double-negative.  How about
 includes instead?

 Intended.  Omission is expected.

This is precision at the expense of clarity.  Omission is not expected
for the user who wants to ask this question:  Where is that commit
that added 'foo'?  To Git she wants to ask Show me the commit that
added or removed 'foo'.  You and I both know that Git does this in
reverse.  Git translates this question into Show all the commits, but
hide the ones which do not add or remove 'foo'.

And so we say Git 'cannot omit' commits



 Is it worth mentioning that something in the documentation is worth
 mentioning?

 You don't have to nitpick style.  We can allow this much creative
 freedom in documentation.

I think I do.  The concepts in here are complicated enough without
loading the language with excess verbiage.  I am chronically afflicted
with this disease where I am always adding extra decorations to my
sentences.  I work hard to combat that, especially when dealing with
technical writings.  This makes it easy for me to recognize in other
technical writing.

I didn't look closely, but these unnecessary introductory clause
appeared to be the only change in this paragraph.  I do not think it
adds anything, which is why I mentioned it.

I could have been more clear and less flippant about it, though.  I'm
sorry if my manner was offensive. This is another chronic problem I
seem to have.


 +in effect, diffcore-pickaxe leaves only such filepairs that touch the
 +specified string in its output.  When in effect, diffcore-pickaxe
 +leaves all filepairs intact if there is such a filepair, or makes the
 +output empty otherwise.  The latter behavior is designed to make
 +reviewing of the changes in the context of the whole changeset easier.

 I find this description (from the original code, not from this commit)
 somewhat confusing.  It is written from the implementer's POV.

 I explained the entire -S and -G thing in terms of filepairs (and yes,
 that's implementation detail).  Why would I want to explain this in
 any other terms?

Clarity.

What is a 'filepair' when I am getting a short-log?  Internally there
was a diff, and the diff involves pairs of files.  But it's a tedious
detail to the user which might send her off needlessly trying to
understand the meaning of the term.

But you are right; gitdiffcore.txt is about precision and
implementation.  The goal is to address the technical user who is
trying to understand these operations in general, not just for log.
'filepairs' is a useful concept to lean on.

The clearer description probably belongs in git-log.

 Does
 this seem clearer to you?
 [...]

 From diff-options.txt (the more end-user side):

 When -S or -G finds a change, show all the changes in that changeset,
 not just the files that contain the change in string.

 Not clear enough?

Yes, it was clear enough to me in git-log.  Perhaps it is not worth
mentioning here.

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http

Re: [PATCH] diffcore-pickaxe doc: document -S and -G properly

[Phil Hord]

On Tue, May 14, 2013 at 2:57 PM, Ramkumar Ramachandra
artag...@gmail.com wrote:
 Junio C Hamano wrote:
 I do not use zsh but with bash+readline the old tradition lnext can
 be used (see stty -a output and it typically is set to ^V), i.e.
 \C-v followed by \C-i should give you a literal HT.

 Just looked it up: zsh has quoted-insert (^V) after which I can TAB.
 Still doesn't solve my problem though: I don't type out structs; I
 paste them the X clipboard (Emacs).  And that doesn't work either on
 bash or zsh.

 What can we do to improve the interface?

Don't use tabs in your code?

P
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC/PATCH] branch: show me the hot branches

[Phil Hord]

On Mon, May 13, 2013 at 4:02 PM, Ramkumar Ramachandra
artag...@gmail.com wrote:
 Uses commit-date to sort displayed refs.

 Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
 ---

I dig it.

I imagine it with --date-order and whatnot.  But I might like it even
better if it were reverse-sorted.  Maybe it needs -rt for that. ;-)

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Itches with the current rev spec

[Phil Hord]

On Thu, Apr 25, 2013 at 1:07 AM, Ramkumar Ramachandra
artag...@gmail.com wrote:
 2. git rebase -i master fails unless I've rebased my branch on top of
 master.  I always wished I could do the equivalent of 'git rebase -i
 master..', but I can't.

In what way does it fail?   It seems to work ok for me.  Do you mean
that it chooses extra commits you did not want?

Maybe you expect rebase--interactive will only result in changes to
commits you touch in the todo list and it will not actually rebase
anything.   Is that the goal?

I have been thinking of adding a targeted rebase -i extension.  I
often use rebase -i to change one commit in recent history or to
squash some fixup into place.  The trip through $EDITOR to do this
seems disruptive to my thinking.   So I would like to be able to do
this:

   git rebase --edit $REF

which should act the same as

  GIT_EDITOR='sed -i s/^pick $REF/edit $REF/' \
  git rebase -i $REF^

Except that $REF could be any ref and not just the exact
SHA1-abbreviation given in todo.

The change I imagine allows --fixup, --reword,  --squash, etc.  It
might even allow multiple instances of each.

I haven't thought through how to handle the case where there are
merges in the way, but I do already suppose that the command will
simply fail if a ref is not an ancestor of HEAD.

Maybe this is too simple for your workflow, though. As I said, I did
not understand your itch.

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [BUG] Highly inconsistent diff UI

[Phil Hord]

On Wed, Apr 24, 2013 at 3:00 PM, Ramkumar Ramachandra
artag...@gmail.com wrote:
 Ramkumar Ramachandra wrote:
 I'm also considering making the first
 argument optional (just git log ~rebase.autostash), and defaulting to
 mean [nearest fork point].

 Actually both can be optional.  In A~B, A defaults to [nearest fork
 point] and B defaults to HEAD.

 git log ~

 Isn't it beautiful?

Sure, but we'll forever be explaining to users that they will need to
quote the tilde to avoid shell expansion.

$ echo ~ ~ ~
~ /home/hordp ~

P
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/7] rebase: implement --[no-]autostash and rebase.autostash

[Phil Hord]

On Tue, Apr 23, 2013 at 10:02 AM, Ramkumar Ramachandra
artag...@gmail.com wrote:
 This new feature allows a rebase to be executed on a dirty worktree.
 It works by creating a temporary stash and storing it in
 $state_dir/autostash before the operation, and applying it after a
 successful operation.  It will be removed along with the $state_dir if
 the operation is aborted.

 The feature creates a special stash that does not affect the normal
 stash's reflogs, and will therefore be invisible to the end user.
 This special stash is essentially a dangling merge commit which has
 reasonable lifetime specified by gc.pruneexpire (default 2 weeks).

 Most significantly, this feature means that a caller like pull (with
 pull.rebase set to true) can easily be patched to remove the
 require_clean_work_tree restriction.

 Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
 ---
  Documentation/config.txt |  8 
  Documentation/git-rebase.txt | 10 ++
  git-rebase.sh| 38 +++---
  3 files changed, 53 insertions(+), 3 deletions(-)

 diff --git a/Documentation/config.txt b/Documentation/config.txt
 index c67038b..03ad701 100644
 --- a/Documentation/config.txt
 +++ b/Documentation/config.txt
 @@ -1867,6 +1867,14 @@ rebase.stat::
  rebase.autosquash::
 If set to true enable '--autosquash' option by default.

 +rebase.autostash::
 +   When set to true, automatically create a temporary stash
 +   before the operation begins, and apply it after the operation
 +   ends.  This means that you can run rebase on a dirty worktree.
 +   However, use with care: the final stash application after a
 +   successful rebase might result in non-trivial conflicts.
 +   Defaults to false.
 +
  receive.autogc::
 By default, git-receive-pack will run git-gc --auto after
 receiving data from git-push and updating refs.  You can stop
 diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
 index aca8405..c84854a 100644
 --- a/Documentation/git-rebase.txt
 +++ b/Documentation/git-rebase.txt
 @@ -208,6 +208,9 @@ rebase.stat::
  rebase.autosquash::
 If set to true enable '--autosquash' option by default.

 +rebase.autostash::
 +   If set to true enable '--autostash' option by default.
 +
  OPTIONS
  ---
  --onto newbase::
 @@ -394,6 +397,13 @@ If the '--autosquash' option is enabled by default using 
 the
  configuration variable `rebase.autosquash`, this option can be
  used to override and disable this setting.

 +--[no-]autostash::
 +   Automatically create a temporary stash before the operation
 +   begins, and apply it after the operation ends.  This means
 +   that you can run rebase on a dirty worktree.  However, use
 +   with care: the final stash application after a successful
 +   rebase might result in non-trivial conflicts.
 +
  --no-ff::
 With --interactive, cherry-pick all rebased commits instead of
 fast-forwarding over the unchanged ones.  This ensures that the
 diff --git a/git-rebase.sh b/git-rebase.sh
 index 8412d81..c8fddfe 100755
 --- a/git-rebase.sh
 +++ b/git-rebase.sh
 @@ -13,6 +13,7 @@ git-rebase --continue | --abort | --skip | --edit-todo
   Available options are
  v,verbose! display a diffstat of what changed upstream
  q,quiet!   be quiet. implies --no-stat
 +autostash! automatically stash/stash pop before and after
  onto=! rebase onto given branch instead of upstream
  p,preserve-merges! try to recreate merges instead of ignoring them
  s,strategy=!   use the given merge strategy
 @@ -64,6 +65,7 @@ apply_dir=$GIT_DIR/rebase-apply
  verbose=
  diffstat=
  test $(git config --bool rebase.stat) = true  diffstat=t
 +autostash=$(git config --bool rebase.autostash || echo false)
  git_am_opt=
  rebase_root=
  force_rebase=
 @@ -143,6 +145,24 @@ move_to_original_branch () {
 esac
  }

 +apply_autostash () {
 +   if test -f $state_dir/autostash
 +   then
 +   stash_sha1=$(cat $state_dir/autostash)
 +   git stash apply $stash_sha1 21 /dev/null ||
 +   die 
 +$(eval_gettext 'Applying autostash resulted in conflicts.
 +Either fix the conflicts now, or run
 +   git reset --hard
 +and apply the stash on your desired branch:
 +   git stash apply $stash_sha1
 +at any time.') 
 +   echo Applied autostash
 +   fi
 +   git gc --auto 
 +   rm -rf $state_dir
 +}

Because I am in a git-rebase which has apparently failed, I would
expect 'git rebase --abort' would save me here.  But it does not and
you have given me some unique instructions to try to recover.  I
suppose rebase--abort cannot be made to recover in this case because
this is a rebase-wrapper and all of my rebase-state is already
discarded.  But I would much prefer to have the normal undo-ability
of git-rebase here, once I realize I have made a mistake or
encountered 

[PATCH] rebase: find orig_head unambiguously

[Phil Hord]

When we 'git rebase $upstream', git uses 'rev-parse --verify
$current_branch' to find ORIG_HEAD.  But if $current_branch
is ambiguous, 'rev-parse --verify' emits a warning and returns
a SHA1 anyway.  When the wrong ambiguous choice is used,
git-rebase fails non-gracefully:  it emits a warning about
failing to lock $current_branch, an error about being unable to
checkout $current_branch again, and it might even decide the
rebase is a fast-forward when it is not.

In the 'rebase $upstream' case, we already know the unambiguous
spelling of $current_branch is HEAD.  Fix git-rebase to find
$orig_head unambiguously.

Add a test in t3400-rebase.sh which creates an ambiguous branch
name and rebases it implicitly with 'git rebase $other'.

Signed-off-by: Phil Hord ho...@cisco.com
---
 git-rebase.sh | 2 +-
 t/t3400-rebase.sh | 7 +++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index b2f1c76..2c692c3 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -473,7 +473,7 @@ case $# in
head_name=detached HEAD
branch_name=HEAD ;# detached
fi
-   orig_head=$(git rev-parse --verify ${branch_name}^0) || exit
+   orig_head=$(git rev-parse --verify HEAD) || exit
;;
 *)
die BUG: unexpected number of arguments left to parse
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index f6cc102..b58fa1a 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -101,7 +101,14 @@ test_expect_success 'HEAD was detached during rebase' '
test $(git rev-parse HEAD@{1}) != $(git rev-parse my-topic-branch@{1})
 '
 
+test_expect_success 'rebase from ambiguous branch name' '
+   git checkout -b topic side 
+   git rebase master
+'
+
 test_expect_success 'rebase after merge master' '
+   git checkout --detach refs/tags/topic 
+   git branch -D topic 
git reset --hard topic 
git merge master 
git rebase master 
-- 
1.8.2.1.854.g46dda50.dirty

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)

[Phil Hord]

On Thu, Apr 18, 2013 at 7:48 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 On Thu, Apr 18, 2013 at 3:06 PM, Phil Hord phil.h...@gmail.com wrote:
 On Wed, Apr 17, 2013 at 2:50 PM, Felipe Contreras

 Yes please. Show me one of the instances where you hit a bisect with
 any of the remote-hg commits mentioned above by Thomas Rast.

 I made no such claim.  In fact, I have never bisected to any
 remote-hg-related commit.  I fail to see the relevance of this
 qualifier, though.

 Here, this is what you said:

 You:
 Me:
 [skipping irrelevant comments]

 I'm sorry, did you actually hit an issue that required to look at the
 commit message to understand where the issue came from? No? Then I
 won't bother with hypotheticals.

 If you want to waste your time, by all means, rewrite all my commit
 messages with essays that nobody will ever read. I'm not going to do
 that for some hypothetical case that will never happen. I'm not going
 to waste my time.

 This is not a hypothetical.

 If something is not hypothetical, it's real, which means it actually
 happened, but then you said you never made the claim that it did. So
 what is it?

My claim:

   I bisected to a commit whose commit message helped me deduce its entirety.


Your fanciful interpretation, which I denied:

  Show me one of the instances where you hit a bisect with
  any of the remote-hg commits mentioned above by Thomas Rast.


I have never bisected to any commit related to remote-hg, and neither
did I ever claim to.  I do not know where you got such a ridiculous
qualifier as this to append to my statement.


 Either it did happen, or it didn't;

It did.  Where it is my actual claim, that I bisected to a commit
whose commit message helped me deduce its entirety.

But also, it didn't, where it is your preposterous interpretation of
my interest and/or experiences with remote-hg commits.

You seem only to want to argue, Felipe.  I have neither time nor
interest in pig-wrestling, myself.

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)

[Phil Hord]

On Wed, Apr 17, 2013 at 2:50 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 On Tue, Apr 16, 2013 at 5:45 PM, Phil Hord phil.h...@gmail.com wrote:
 On Tue, Apr 16, 2013 at 3:04 PM, Felipe Contreras

 If you want to waste your time, by all means, rewrite all my commit
 messages with essays that nobody will ever read. I'm not going to do
 that for some hypothetical case that will never happen. I'm not going
 to waste my time.

 This is not a hypothetical.  Almost every time I bisect a regression
 in git.git, I find the commit message tells me exactly why the commit
 did what it did and what the expected result was.  I find this to be
 amazingly useful.  Do I need to show you real instances of that
 happening? No.  I promise it did, though.

 Yes please. Show me one of the instances where you hit a bisect with
 any of the remote-hg commits mentioned above by Thomas Rast.

I made no such claim.  In fact, I have never bisected to any
remote-hg-related commit.  I fail to see the relevance of this
qualifier, though.

P
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] submodule deinit: clarify work tree removal message

[Phil Hord]

On Mon, Apr 1, 2013 at 3:02 PM, Jens Lehmann jens.lehm...@web.de wrote:
 Okay, so here is the patch for that. If someone could point out
 a portable and efficient way to check if a directory is already
 empty I would be happy to use that to silence the Cleaned
 directory message currently printed also when deinit is run on
 an already empty directory.

   isemptydir() {
test -d $(find $1 -maxdepth 0 -empty)
   }

Sorry for the late reply.  I see this patch is already in master
(which is fine with me).

Thanks,
Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] pull: introduce --[no-]autostash and pull.autostash

[Phil Hord]

On Tue, Apr 16, 2013 at 5:20 AM, Ramkumar Ramachandra
artag...@gmail.com wrote:
 Matthieu Moy wrote:
 No.  Ultimately, the entry point of all these invocations is
 git-rebase.sh.  The plan is to refactor calls from git-rebase.sh to
 git-rebase--*.sh scripts so that those scripts return control to
 git-rebase.sh, which will be the final exit point.  The logic is very
 simple: On the very first invocation of rebase (ie. no existing rebase
 in progress), stash.  If the return statement from the specific rebase
 script is 1 (which means that there are conflicts to be resolved),
 exit as usual.  If it is 0 (which means that the rebase completely
 successfully), pop the stash before exiting as usual.

 What's so complicated about that?  I'm against leaking the autostash
 implementation detail into specific rebases, because I value a clean
 and pleasant implementation over everything else.

It can be more complex than you realize.

   $ git pull --rebase --stash

It seems that there is already a .git/rebase-apply directory, and
I wonder if you are in the middle of another rebase.  If that is the
case, please try
git rebase (--continue | --abort | --skip)
If that is not the case, please
rm -fr .git/rebase-apply
and run me again.  I am stopping in case you still have something
valuable there.

If I follow the latter advice about 'rm -rf', who will remember that
'rebase' had something stashed, and what will they do with it when
they do?

What if it is weeks or months later?  I would be surprised to see
long-forgotten wip show up in my workspace all of a sudden.

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)

[Phil Hord]

On Tue, Apr 16, 2013 at 3:48 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 Here it goes. The remote helper ref is going to be used to tell
 fast-export which refs to negate (e.g. ^refs/testgit/origin/master),
 so that extra commits are not generated, which the remote helper
 should ignore anyway, because it should already have marks for those.
 So doing two consecutive pushes, would push the commits twice.

 It's worth noting this is the first time anybody asks what is the
 negative effect of this not getting fixed.

Yes, but what is noteworthy about it is that you did not include that
in your commit message to begin with.  This is the commit message
request from Documentation/SubmittingPatches:

The body should provide a meaningful commit message, which:

  . explains the problem the change tries to solve, iow, what is wrong
with the current code without the change.

  . justifies the way the change solves the problem, iow, why the
result with the change is better.

  . alternate solutions considered but discarded, if any.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)

[Phil Hord]

On Tue, Apr 16, 2013 at 3:04 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 On Tue, Apr 16, 2013 at 4:59 AM, Thomas Rast tr...@inf.ethz.ch wrote:
 A cursory look^W^Wreview of the messages in fc/remote-hg:

 [skipping irrelevant comments]

 I'm sorry, did you actually hit an issue that required to look at the
 commit message to understand where the issue came from? No? Then I
 won't bother with hypotheticals.

 If you want to waste your time, by all means, rewrite all my commit
 messages with essays that nobody will ever read. I'm not going to do
 that for some hypothetical case that will never happen. I'm not going
 to waste my time.

This is not a hypothetical.  Almost every time I bisect a regression
in git.git, I find the commit message tells me exactly why the commit
did what it did and what the expected result was.  I find this to be
amazingly useful.  Do I need to show you real instances of that
happening? No.  I promise it did, though.

Of course, 99% of the commit messages may never be useful to me or
anyone else.  But we do not eschew them altogether.  The 1% I have to
rely on are nearly always helpful and clear, and that is the part I
care about.

If you will not waste your time to write a decent commit message, why
do you waste our time asking us to review and accept ill-defined
patches?  Here, of course, I use the royal us as I do not review
your patches.  I do not know why that is; I suppose you patch things
outside of my interests, but it may also be that your patches are
simply incomprehensible by design.

Phil
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html