Re: [PATCH v2 10/13] bash prompt: combine 'git rev-parse' executions

2013-06-18 Thread Jeff King
On Tue, Jun 18, 2013 at 04:17:03AM +0200, SZEDER Gábor wrote:

 The whole series speeds up the bash prompt on Windows/MSysGit
 considerably.  Here are some timing results in two scenarios, repeated
 10 times:
 
 At the top of the work tree, before:
 
 $ time for i in {0..9} ; do prompt=$(__git_ps1) ; done
 
 real0m1.716s
 user0m0.301s
 sys 0m0.772s
 
   After:
 
 real0m0.686s
 user0m0.075s
 sys 0m0.287s
 
 In a subdirectory, during rebase, stash status indicator enabled,
 before:
 
 real0m3.557s
 user0m0.495s
 sys 0m1.767s
 
   After:
 
 real0m0.702s
 user0m0.045s
 sys 0m0.409s

Very nice speedup (or perhaps it is a testament to how bad fork() is on
msys). Reading patches 8 and 9, I can't help but feel that git status
is letting us down a little by making us parse all of this data
ourselves. In theory, __git_ps1() could just be something like:

  eval $(git status --shell)
  printf ...

and the heavy lifting could be done in a single C process which does not
have to worry about fork overhead. But that is quite far from where we
are now, so while it might be an interesting place to go in the future,
I do not think such dreams would want to hold up current work.

-Peff
--
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] builtin/checkout.c: don't leak memory in check_tracking_name

2013-06-18 Thread Jeff King
On Mon, Jun 17, 2013 at 06:40:49PM -0700, Brandon Casey wrote:

 From: Brandon Casey draf...@gmail.com
 
 remote_find_tracking() populates the query struct with an allocated
 string in the dst member.  So, we do not need to xstrdup() the string,
 since we can transfer ownership from the query struct (which will go
 out of scope at the end of this function) to our callback struct, but
 we must free the string if it will not be used so we will not leak
 memory.
 
 Let's do so.

Thanks, looks obviously correct. I wonder if other callers of
remote_find_tracking make the same mistake. It looks like
check_tracking_branch does. And add_branch_for_removal. and
append_ref_to_tracked_list. Yeesh.

-Peff
--
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-add--interactive.perl: Permit word-based diff

2013-06-18 Thread Jeff King
On Sat, Jun 15, 2013 at 04:01:04PM +0200, Mark Abraham wrote:

 Controlled by new configuration option
 color.word-diff-in-interactive-add. There is no existing support for
 git add to pass a command-line option like --word-diff=color to
 git-add--interactive.perl, so a configuration option is the only
 lightweight solution.
 
 With this feature, the added or deleted form of a hunk can be empty,
 so made some necessary checks for $_ being defined.

Hmm. We can't apply a word-diff, so presumably your permit here is
just for the display, replacing the colorized bits. And that looks like
what your patch does.

Note that the number of lines in your --word-diff=color hunk and the
actual diff will not necessarily be the same.  What happens if I split a
hunk with your patch?

-Peff
--
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] http.c: don't rewrite the user:passwd string multiple times

2013-06-18 Thread Daniel Stenberg

On Tue, 18 Jun 2013, Jeff King wrote:

TL;DR: I'm just confirming what's said here! =)


My understanding of curl's pointer requirements are:

 1. Older versions of curl (and I do not recall which version off-hand,
but it is not important) stored just the pointer. Calling code was
required to manage the string lifetime itself.

 2. Newer versions of curl will strdup the string in curl_easy_setopt.


That's correct. This new behavior in (2) was introduced in libcurl 7.17.0 - 
released in September 2007 and should thus be fairly rare by now.


I mention this primarily because I think it should be noted that there will 
probably be very little testing by users with such old libcurl versions. It 
may increase the time between a committed change and people notice brekages 
caused by it. Even Debian old-stable has a much newer version.


For older versions, if we were to grow the strbuf, we might free() the 
pointer provided to an earlier call to curl_easy_setopt. But since we are 
about to call curl_easy_setopt with the new value, I would assume that curl 
will never actually look at the old one (i.e., when replacing an old 
pointer, it would not dereference it, but simply overwrite it with the new 
value).


Another accurate description.

--

 / daniel.haxx.se
--
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 2/6] test-lib: refactor $GIT_SKIP_TESTS matching

2013-06-18 Thread Johannes Sixt
Am 6/17/2013 11:18, schrieb Thomas Rast:
 +match_pattern_list () {
 + arg=$1
 + shift
 + test -z $*  return 1
 + for pat
 + do
 + case $arg in
 + $pat)
 + return 0
 + esac
 + done
 + return 1
 +}

Watch this failing test case:

   GIT_SKIP_TESTS=t950[012] ./t3006-ls-files-long.sh

'pat' is too short and too sweet to be used as a shell variable name in a
library function. ;)

-- Hannes
--
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] sha1_name: stop hard-coding 40-character hex checks

2013-06-18 Thread Ramkumar Ramachandra
Phil Hord wrote:
 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.

Please read the cover-letter: I was just toying around to see if this
was a good idea, and Junio points out that it's not.
--
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 v3 1/2] am: handle stray $dotest directory

2013-06-18 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 Will replace what has been queued on 'pu'.

 ... after fixing an indentation error, that is.

Where did the error occur?  I thought I taught my Emacs to always
indent properly.
--
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] config doc: rewrite push.default section

2013-06-18 Thread Ramkumar Ramachandra
Philip Oakley wrote:
 A sentence, in the Documentation/config.txt, is needed to clarify the
 Central workflow and any distinction with the  non-central workflow(s). We
 cannot assume the new reader has the same world view of that concept (they
 may be thinking it means we do a centralised VCS, not a DVCS with a chosen
 central primary repo - assuming I have understood it correctly).

 It took a while to bottom out the issues, so it is worth summarising the key
 point(s) in the documentation to avoid having to repeat the disussions ;-)

I'm not sure where to put it: Documentation/gitworkflows.txt perhaps?

 [...]

Agreed with the rest.  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] GIT-VERSION-GEN: support non-standard $GIT_DIR path

2013-06-18 Thread Dennis Kaarsemaker
On ma, 2013-06-17 at 13:09 -0700, Junio C Hamano wrote:
 Junio C Hamano gits...@pobox.com writes:
 
  Dennis Kaarsemaker den...@kaarsemaker.net writes:
 
   I'm doing daily builds of git, using many workers and a shared git.git,
   with per-worker checkouts
 
  OK, so GIT_DIR is explicitly specified in these workers.

Yes, both GIT_DIR and GIT_WORK_TREE are set and the use of .git/HEAD and
anything relying on it is shunned, so workers can run checkout as they
please.

  Makes sense.

 Actually it does not.  What if GIT_DIR is an empty string or not set
 at all?  The patch breaks the build for everybody else, doesn't it?

It does indeed, I only tested in my setup and not with a normal make
test. Apologies.

 Perhaps like this instead?

  GIT-VERSION-GEN | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
 index 2908204..91ec831 100755
 --- a/GIT-VERSION-GEN
 +++ b/GIT-VERSION-GEN
 @@ -11,7 +11,7 @@ LF='
  if test -f version
  then
   VN=$(cat version) || VN=$DEF_VER
 -elif test -d .git -o -f .git 
 +elif test -d ${GIT_DIR:-.git} -o -f .git 
   VN=$(git describe --match v[0-9]* --abbrev=7 HEAD 2/dev/null) 
   case $VN in
   *$LF*) (exit 1) ;;

Yes, that makes a lot more sense and actually works in normal make test
and with a detached .git. Do you want me to send an updated patch?

-- 
Dennis Kaarsemaker
www.kaarsemaker.net

--
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-add--interactive.perl: Permit word-based diff

2013-06-18 Thread Thomas Rast
[I don't seem to have received a copy of the original mail, so I can
only guess...]

Jeff King p...@peff.net writes:

 On Sat, Jun 15, 2013 at 04:01:04PM +0200, Mark Abraham wrote:

 Controlled by new configuration option
 color.word-diff-in-interactive-add. There is no existing support for
 git add to pass a command-line option like --word-diff=color to
 git-add--interactive.perl, so a configuration option is the only
 lightweight solution.
 
 With this feature, the added or deleted form of a hunk can be empty,
 so made some necessary checks for $_ being defined.

 Hmm. We can't apply a word-diff, so presumably your permit here is
 just for the display, replacing the colorized bits. And that looks like
 what your patch does.

 Note that the number of lines in your --word-diff=color hunk and the
 actual diff will not necessarily be the same.  What happens if I split a
 hunk with your patch?

If it's actually what you hint at, there's another problem: the word
diff might not even have the same number of hunks.  For example, a
long-standing bug (or feature, depending on POV) of word-diff is that it
does not take opportunities to completely drop hunks that did not make
any word-level changes.

Consider this:

  diff --git a/foo b/foo
  index 5eaddaa..089eeac 100644
  --- a/foo
  +++ b/foo
  @@ -1,2 +1,2 @@
  -foo bar  
 
  -baz  
 
  +foo  
 
  +bar baz

In word-diff mode that's

  diff --git a/foo b/foo
 
  index 5eaddaa..089eeac 100644 
 
  --- a/foo 
 
  +++ b/foo 
 
  @@ -1,2 +1,2 @@   
 
  foo   
 
  bar baz

Arguably word-diff should be fixed to drop the hunk.  But if 'add -p'
depends on exact hunk correspondence, it will break.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
--
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 mz/rebase-tests] rebase topology tests: fix commit names on case-insensitive file systems

2013-06-18 Thread Johannes Sixt
From: Johannes Sixt j...@kdbg.org

The recently introduced tests used uppercase letters to denote
cherry-picks of commits having the corresponding lowercase letter names.
The helper functions also set up tags with the names of the commits.

But this constellation fails on case-insensitive file systems because
there cannot be distinct tags with names that differ only in case.

Use a less subtle convention for the names of cherry-picked commits.

Signed-off-by: Johannes Sixt j...@kdbg.org
---
 Knowing that the tests would take their time to complete on Windows,
 today was the first time I ran them, and there were some unexpected
 issues. Fixed by this patch.

 t/t3421-rebase-topology-linear.sh | 20 ++--
 t/t3425-rebase-topology-merges.sh | 16 
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/t/t3421-rebase-topology-linear.sh 
b/t/t3421-rebase-topology-linear.sh
index e67add6..9c55cba 100755
--- a/t/t3421-rebase-topology-linear.sh
+++ b/t/t3421-rebase-topology-linear.sh
@@ -79,9 +79,9 @@ test_run_rebase success -p
 #  /
 # a---b---c---g---h
 #  \
-#   d---G---i
+#   d---gp--i
 #
-# uppercase = cherry-picked
+# gp = cherry-picked g
 # h = reverted g
 #
 # Reverted patches are there for tests to be able to check if a commit
@@ -94,7 +94,7 @@ test_expect_success 'setup of linear history for range 
selection tests' '
test_commit g 
revert h g 
git checkout d 
-   cherry_pick G g 
+   cherry_pick gp g 
test_commit i 
git checkout b 
test_commit f
@@ -120,7 +120,7 @@ test_run_rebase () {
shift
test_expect_$result rebase $* can drop last patch if in upstream 
reset_rebase 
-   git rebase $* h G 
+   git rebase $* h gp 
test_cmp_rev h HEAD^ 
test_linear_range 'd' h..

@@ -152,7 +152,7 @@ test_run_rebase () {
reset_rebase 
git rebase $* --onto h f i 
test_cmp_rev h HEAD~3 
-   test_linear_range 'd G i' h..
+   test_linear_range 'd gp i' h..

 }
 test_run_rebase success ''
@@ -222,9 +222,9 @@ test_run_rebase failure -p
 #  /
 # a---b---c---g
 #
-# x---y---B
+# x---y---bp
 #
-# uppercase = cherry-picked
+# bp = cherry-picked b
 # m = reverted b
 #
 # Reverted patches are there for tests to be able to check if a commit
@@ -239,7 +239,7 @@ test_expect_success 'setup of linear history for test 
involving root' '
git rm -rf . 
test_commit x 
test_commit y 
-   cherry_pick B b
+   cherry_pick bp b
 '
 
 test_run_rebase () {
@@ -277,7 +277,7 @@ test_run_rebase () {
shift
test_expect_$result rebase $* --onto --root drops patch in onto 
reset_rebase 
-   git rebase $* --onto m --root B 
+   git rebase $* --onto m --root bp 
test_cmp_rev m HEAD~2 
test_linear_range 'x y' m..

@@ -308,7 +308,7 @@ test_run_rebase () {
shift
test_expect_$result rebase $* without --onto --root with disjoint 
history drops patch in onto 
reset_rebase 
-   git rebase $* m B 
+   git rebase $* m bp 
test_cmp_rev m HEAD~2 
test_linear_range 'x y' m..

diff --git a/t/t3425-rebase-topology-merges.sh 
b/t/t3425-rebase-topology-merges.sh
index 5400a05..1d195fb 100755
--- a/t/t3425-rebase-topology-merges.sh
+++ b/t/t3425-rebase-topology-merges.sh
@@ -30,7 +30,7 @@ test_expect_success 'setup of non-linear-history' '
test_commit g 
revert h g 
git checkout d 
-   cherry_pick G g 
+   cherry_pick gp g 
test_commit i 
git checkout b 
test_commit f
@@ -154,11 +154,11 @@ test_expect_success rebase -p can re-create two branches 
on onto 
 #  /
 # a---b---c---g---h
 #  \
-#   d---G---i
+#   d---gp--i
 #\   \
 # e---u
 #
-# uppercase = cherry-picked
+# gp = cherry-picked g
 # h = reverted g
 test_expect_success 'setup of non-linear-history for patch-equivalence tests' '
git checkout e 
@@ -186,24 +186,24 @@ test_expect_success rebase -p --onto in merged history 
does not drop patches in
git rebase -p --onto h f u 
test_cmp_rev h HEAD~3 
test_cmp_rev HEAD^2~2 HEAD~2 
-   test_revision_subjects 'd G i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD
+   test_revision_subjects 'd gp i e u' HEAD~2 HEAD^2^ HEAD^2 HEAD^ HEAD
 
 
 # a---b---c---g---h
 #  \
-#   d---G---s
+#   d---gp--s
 #\   \ /
 # \   X
 #  \ / \
 #   e---t
 #
-# uppercase = cherry-picked
+# gp = cherry-picked g
 # h = reverted g
 test_expect_success 'setup of non-linear-history for dropping whole side' '
-   git checkout G 
+   git checkout gp 
test_merge s e 
git checkout e 
-   test_merge t G
+ 

Re: [PATCH] config doc: rewrite push.default section

2013-06-18 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 I do no think the remainder (snipped) belongs to the log message.

Oh, it was never intended to be a proper commit message.  I'll write a
proper one when I send it in with the implementation.

  - To understand if central, works as upstream, otherwise works as
current, the readers need to know if their workflow is 'central'
or not, so we need to say how that is decided upfront (probably
immediately before Possible values are: in the introductory
paragraph for push.default;

Good idea.

  - For each of these choices, what it means is more important to the
readers than how the implementation achieves that semantics
(e.g. current uses refs/heads/foo:refs/heads/foo when you are on
foo branch). I think the ideal is a description of the meaning
that is clear enough not to require any implementation detail.

I'd definitely like to include the push refspec, to educate users on
how to achieve a one-off matching-push even when push.default is set
to current, for example.

 As this is an attempt to _define_ the semantics of what should
 happen in triangular workflow, I think it would be healthy to wait
 for a week or so in order for others (not just two of us) can see if
 we have defined a sane semantics we would want to go forward with.

Agreed.

 I am reasonably sure 'current' would be the best default for
 triangular (and that is why I suggested 'simple' to fall back to
 it), but I do not think others had a chance to see what design our
 discussion settled, think if that makes sense, and think of a better
 alternative.

You didn't realize one thing, because my wording was terrible:
`simple` is just a safe version of `current` :)

* `simple` - behaves exactly like `current`, with one safety feature:
in central workflows, it errors out if branch.$branch.merge is set and
is not equal to $branch (to make sure that the `push` and `pull`
aren't asymmetrical).

In other words, it's `current` with the safety feature of `upstream`.
There's no point in erroring out if no upstream is set, or mentioning
its relationship with `upstream`.

 +* `nothing` - error out unless a refspec is explicitly given.

 I do not think 'error out' is the primary effect of this mode.
 Wouldn't do not push anything be much better?

Do not push anything is misleading because it implies an exit status
of zero; what is really does is error out, no?

 +* `current` - push the refspec $HEAD.  HEAD is resolved early to a
 +  branch name (referred to as $HEAD).  In other words, push the
 +  current branch to update a branch with the same name on the pushing
 +  side.

 As already pointed out, dropping everything before and including In
 other words,  would be better.

I'd like to put it in the end, like Matthieu suggested.

 Also push the current branch is
 talking about the branch on the pushing side (you, the one who is
 running git push), and the side that is getting updated is at the
 receiving end, not pushing side.

receiving end is better, yes.

 I think listing 'simple' at the end would be easier to read.  The
 above already swaps the order to make sure current and upstream are
 described before it, which is good.

As I pointed out, `simple` is just a composition of `current` and
`upstream`.  That's why it comes after them.  Also, I thought
`nothing`, `current`, `upstream` were fundamental: which is why they
come first.

 But I do not see a reason to move 'matching' which was next to
 'nothing' here.

It's probably a personal bias, but I don't like matching at all :P

 +* `matching` - push the refspec :.  In other words, push all
 +  branches having the same name in both ends, even if it means
 +  non-fast-forward updates.  This is for those who prepare all the
 +  branches into a publishable shape and then push them out with a
 +  single command.  Dangerous, and inappropriate unless you are the
 +  only person updating your push destination.

 It was already pointed out that unnecessary negativity needs to be
 fixed, but more importantly the above Dangerous is not even
 correct.

Okay, I'll explain my bias:

I have _never_ lost data with git: the reflog is my close companion.
There is one recent instance where I *lost* data, and I was very
unhappy about it.  I work on multiple machines, and not all local
branches are always synced with the upstream branches: when I switch
to a local branch to work on the topic, I notice my prompt and get it
to '=' before starting work.  In this one instance, I was developing
@{push} and toying around with various push.default settings.  I'd
forgotten that I'd set push.default to matching and issued my usual
push to update my branch.  This resulted in a huge number of my
branches getting rewound, and I could do nothing about it!

We've mentioned that pull --rebase is _dangerous_ (with the underline
for effect) in our documentation, when it's trivial to recover from
it: git reset --hard @{1}.  Yet, when I mention matching being
dangerous, I'm accused of being 

Re: [PATCH] Documentation/git-push.txt: explain better cases where --force is dangerous

2013-06-18 Thread Matthieu Moy
Junio C Hamano gits...@pobox.com writes:


 After I re-read the one, I found that override somewhat a strange
 expression.  There is nothing that overrides or to be overriden.

Right, I actually meant overwrite.

 How about putting it like this?

I'm not sure push out refs other than the current branch is strong
enough. Once you are used to push = fast-forward = can't loose data,
push out a ref is not very scary.

I'd do s/push out/overwrite/, but I'm fine with your version too.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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] config doc: rewrite push.default section

2013-06-18 Thread Ramkumar Ramachandra
Matthieu Moy wrote:
 I'd put it the other way around: the intuitive explanation first, and
 the technical one after. For people not totally familiar with Git, the
 first part does not make much sense (and when I learn a new tool, I
 really don't like when the doc assumes I already know too much about
 it).

Good.

 Also, this $HEAD Vs HEAD doesn't seem very clear to me. I don't have a
 really good proposal for a better wording, but maybe replacing $HEAD
 with $branch would make a bit more sense, as having $HEAD != HEAD is
 weird.

Good.

 +* `simple` - in central workflows, behaves like `upstream`, except
 +  that it errors out unless branch.$HEAD.merge is equal to $HEAD.

 ... except that it errors out if branch.$HEAD.merge is not equal to
 $HEAD. ?

Good.

 +  single command.  Dangerous, and inappropriate unless you are the
 +  only person updating your push destination.

 Here also, I'd have said Dangerous, and inappropriate if you are
 not 

I might have overplayed the danger a bit, as Junio points out.  I'll
have a look at your --force documentation patch.
--
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


Aw: Re: [PATCH] Remove pdf target from Makefiles

2013-06-18 Thread Thomas Ackermann
 
 I don't understand.  Do you mean that you want to change the rules
 that generate user-manual.xml?  Would generating different XML files
 for the PDF and for other purposes (with different names) work as a
 way to achieve that without losing the printable manual?
 

This would be even worse when we have to create different xml depending on the
wanted output. The problem here is in the xml-pdf/html step: I wanted to change
the formatting of links in the pdf from Section number to section_name 
to
make it the same as in the html. Thereby I noticed that the definition for this 
is
in files from /etc/asciidoc/dblatex. So to change it we have to introduce our 
own files
in ./Documentation and work on them to bring the formatting of user-manual.html
and user-manual.pdf closer together.


---
Thomas
--
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 2/6] test-lib: refactor $GIT_SKIP_TESTS matching

2013-06-18 Thread Thomas Rast
Johannes Sixt j.s...@viscovery.net writes:

 Am 6/17/2013 11:18, schrieb Thomas Rast:
 +match_pattern_list () {
 +arg=$1
 +shift
 +test -z $*  return 1
 +for pat
 +do
 +case $arg in
 +$pat)
 +return 0
 +esac
 +done
 +return 1
 +}

 Watch this failing test case:

GIT_SKIP_TESTS=t950[012] ./t3006-ls-files-long.sh

 'pat' is too short and too sweet to be used as a shell variable name in a
 library function. ;)

Gah!  Serves me right for not testing test-lib.  Thanks for noticing.

/me goes stand in the corner for a while

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
--
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] Documentation/git-merge.txt: weaken warning about uncommited changes

2013-06-18 Thread Matthieu Moy
Commit 35d2fffd introduced 'git merge --abort' as a synonym to 'git reset
--merge', and added some failing tests in t7611-merge-abort.sh (search
'###' in this file) showing that 'git merge --abort' could not always
recover the pre-merge state.

Still, in many cases, 'git merge --abort' just works, and it is usually
considered that the ability to start a merge with uncommited changes is
an important property of Git.

Weaken the warning by discouraging only merge with /non-trivial/
uncommited changes.

Signed-off-by: Matthieu Moy matthieu@imag.fr
---
This is a followup to an old discussion:

http://thread.gmane.org/gmane.comp.version-control.git/221069

I don't think documenting all the failure cases in the doc would be a
good idea, so I've left the in some cases part.

 Documentation/git-merge.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
index 67ca99c..8c7f2f6 100644
--- a/Documentation/git-merge.txt
+++ b/Documentation/git-merge.txt
@@ -56,8 +56,8 @@ especially if those changes were further modified after the 
merge
 was started), 'git merge --abort' will in some cases be unable to
 reconstruct the original (pre-merge) changes. Therefore:
 
-*Warning*: Running 'git merge' with uncommitted changes is
-discouraged: while possible, it leaves you in a state that is hard to
+*Warning*: Running 'git merge' with non-trivial uncommitted changes is
+discouraged: while possible, it may leave you in a state that is hard to
 back out of in the case of a conflict.
 
 
-- 
1.8.3.1.495.g13f33cf.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 v2 3/6] test-lib: verbose mode for only tests matching a pattern

2013-06-18 Thread Thomas Rast
Jeff King p...@peff.net writes:

 On Mon, Jun 17, 2013 at 11:18:48AM +0200, Thomas Rast wrote:

 As suggested by Jeff King, this takes care to wrap the entire test_expect_*
 block, but nothing else, in the verbose toggling.  To that end we use
 a new pair of hook functions.  The placement is a bit weird because we
 need to wait until the beginning of test_skip for $test_count to be
 incremented.
[...]
   test_start ;# increment number, run setup hooks
   if ! test_skip
   then
 ...
   fi
   test_finish ;# teardown hooks

 Then it is a bit easier to see that each start has a finish (whereas in
 the current version, the setups in test_skip are matched by individual
 teardowns in each caller). I did not look too hard at it, though, so I
 wouldn't be surprised if there is some other hidden order dependency
 that makes that not work. :)

No, I think that's actually very reasonable.  I'll do it that way in v3.

 But then what is this hunk doing:

  test_eval_ () {
  # This is a separate function because some tests use
  # return to end a test_expect_success block early.
 @@ -358,9 +399,7 @@ test_run_ () {
  
  if test -z $immediate || test $eval_ret = 0 || test -n 
 $expecting_failure
  then
 -setup_malloc_check
  test_eval_ $test_cleanup
 -teardown_malloc_check
  fi
  if test $verbose = t  test -n $HARNESS_ACTIVE
  then

Thanks for catching this -- it's just a mis-edit that would effectively
revert 1/6.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
--
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] Documentation/git-merge.txt: weaken warning about uncommited changes

2013-06-18 Thread Ramkumar Ramachandra
Matthieu Moy wrote:
 Weaken the warning by discouraging only merge with /non-trivial/
 uncommited changes.

 I don't think documenting all the failure cases in the doc would be a
 good idea, so I've left the in some cases part.

It's already documented in the pre-merge checks section, as Jonathan
pointed out [1].  We should update the documentation to point to it: I
do not think non-trivial is much of an improvement.

[1]: http://thread.gmane.org/gmane.comp.version-control.git/221069/focus=221366
--
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] Documentation/git-merge.txt: weaken warning about uncommited changes

2013-06-18 Thread Matthieu Moy
Ramkumar Ramachandra artag...@gmail.com writes:

 Matthieu Moy wrote:
 Weaken the warning by discouraging only merge with /non-trivial/
 uncommited changes.

 I don't think documenting all the failure cases in the doc would be a
 good idea, so I've left the in some cases part.

 It's already documented in the pre-merge checks section, as Jonathan
 pointed out [1]. 

I had missed this one. But that's not the only case, you may also have
problems with renames. The complete list would be really long to have
here, and won't bring much to the user.

 We should update the documentation to point to it: I do not think
 non-trivial is much of an improvement.

Actually, I think it essentially says it all. If your changes are
important enough to deserve a real backup, you should stash or commit.
If you're ready to take the risk of losing it (the risk is small, but
does exist), it's OK to run git merge blindly.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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 V3 1/4] git-mw: Introduction of GitMediawiki.pm

2013-06-18 Thread Benoît Person
On 17 June 2013 09:12, Matthieu Moy matthieu@grenoble-inp.fr wrote:
 Also, it seems to be only part of the solution. With your change, from
 contrib/mw-to-git/ and after running only make,

 ./git-mw takes the installed version of GitMediawiki.pm in priority

 ../../bin-wrappers/git takes the installed version of git-mw only (i.e.
 does not know git mw if make install hasn't been ran).
 Same thing as the documentation point, I think I am a bit lost in that
 whole thing. I will re-look into it for the next version :/ .

 In short, the include path should contain both the *.pm file and the
 git-foo ones.
The fact is, for now, is there a way to test changes in
git-remote-mediawiki.perl without 'make install'-ing it ? I could not find one

So maybe in the build-perl-script of the toplevel Makefile we could add
something copying the script at the toplevel ?

And in GitMediawiki's Makefile, we let everything stay as is : copying *.pm
into /perl/blib/lib when building and copying it in installdir when installing ?

 I think you removed a newline from the end of the file. It's usually
 considered good practice to have this trailing newline (e.g. so that
 cat file in a terminal doesn't put your prompt after the last line).
 IIRC, it's actually required to call the file a text file according to
 POSIX.
That catch oO, thanks for the explanations.
From my point of view, this could definitely be improved from:
 - perlcritic -2 *.perl
 + perlcritic -2 *.perl
 \ No newline at end of file
to something like that:
 - perlcritic -2 *.perl
 + perlcritic -2 *.perl
 \ removed newline at end of file
which gives more insights into why the line is considered edited.

Benoit Person
--
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 0/6] Fix checkout-dash to work with rebase

2013-06-18 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 I actually tried to reorder the patches and they seem to work OK as
 expected.  And I think it makes sense to order them the way I've
 been suggesting, so I'll tentatively queue the result of reordering
 on 'rr/rebase-checkout-reflog' and push it out as a part of 'pu'.

 Please check to see if I introduced a new bug while doing so.

Thanks for the reorder and commit message tweaks.  I'm working on the
series you put up on `pu` now.

 For example, the one in git reabse does this:

 GIT_REFLOG_ACTION=$GIT_REFLOG_ACTION: checkout $onto_name
 git checkout -q $onto^0 || die could not detach HEAD
 git update-ref ORIG_HEAD $orig_head
 ...
 run_specific_rebase

 But the specific rebase, e.g. git-rebase--interactive, does this:

 case $head_name in
 refs/*)
 message=$GIT_REFLOG_ACTION: $head_name onto $onto 
 git update-ref -m $message $head_name $newhead $orig_head 
 git symbolic-ref \
   -m $GIT_REFLOG_ACTION: returning to $head_name \
   HEAD $head_name
 ;;
 esac  {

 I think the message you added to git reabse is only meant for that
 specific checkout $onto, but it is set globally.  Wouldn't it
 affect later use, which expected it to be rebase and nothing else?

Both rebase.sh and rebase--interactive.sh set a sane GIT_REFLOG_ACTION
right on top (using set_reflog_action), so no worries.  I'll just
double-check to make sure that no bogus/ incorrect messages are
printed.

 Perhaps something like this on top of the entire series may be
 sufficient (which will be queued as SQUASH??? at the tip).

I think this takes the wrong approach to the problem.  In my opinion,
the correct approach is to actually overshadow die() with a function
that clears GIT_REFLOG_ACTION before calling die().

 git grep -C2 'git checkout' -- git-rebase\*.sh

Ugh.  I'll check all the codepaths thoroughly before submitting a re-roll.

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 v2 10/13] bash prompt: combine 'git rev-parse' executions

2013-06-18 Thread SZEDER Gábor
On Tue, Jun 18, 2013 at 02:05:35AM -0400, Jeff King wrote:
 On Tue, Jun 18, 2013 at 04:17:03AM +0200, SZEDER Gábor wrote:
 
  The whole series speeds up the bash prompt on Windows/MSysGit
  considerably.  Here are some timing results in two scenarios, repeated
  10 times:
  
  At the top of the work tree, before:
  
  $ time for i in {0..9} ; do prompt=$(__git_ps1) ; done
  
  real0m1.716s
  user0m0.301s
  sys 0m0.772s
  
After:
  
  real0m0.686s
  user0m0.075s
  sys 0m0.287s
  
  In a subdirectory, during rebase, stash status indicator enabled,
  before:
  
  real0m3.557s
  user0m0.495s
  sys 0m1.767s
  
After:
  
  real0m0.702s
  user0m0.045s
  sys 0m0.409s
 
 Very nice speedup (or perhaps it is a testament to how bad fork() is on
 msys).

Well, it seems it's not just fork()  friends.  The latter case on
Linux, before:

  $ time for i in {0..99}; do prompt=$(__git_ps1) ; done
  
real0m2.819s
user0m0.180s
sys 0m0.272s

  After:
  
real0m0.787s
user0m0.000s
sys 0m0.044s

If you look solely at speedup (Win/MSys: 80%, Linux: 72%), Linux isn't
that much better either, but overall it's about an order of magnitude
faster to begin with (100 repetitions vs. 10).

Btw, it could still be a bit faster in you would care to change your
prompt to run from $PROMPT_COMMAND, because it would avoid that final
$(__git_ps1) command sunstitution, too.  But I didn't measured that
because I find the interface awful ;)  (Hmm, speaking of which, the
patch reading HEAD might break setups using $PROMPT_COMMAND, because
it might do a simple return without updating $PS1...)

 Reading patches 8 and 9, I can't help but feel that git status
 is letting us down a little by making us parse all of this data
 ourselves. In theory, __git_ps1() could just be something like:
 
   eval $(git status --shell)
   printf ...
 
 and the heavy lifting could be done in a single C process which does not
 have to worry about fork overhead. But that is quite far from where we
 are now, so while it might be an interesting place to go in the future,
 I do not think such dreams would want to hold up current work.

Yeah, that would be one way to go.  It would have the added benefit
that it could support 'core.abbrev' in the non-describable detached
HEAD case without noticable overhead.

OTOH it still requires a command substitution and a git command, so
it's less than ideal.  The previous version of this series more than a
year ago did some tricky things to create a prompt without a single
fork(), let alone exec(), e.g. looking for .git directory with bash
builtins, comparing pwd's prefix with path to .git dir to find out
whether inside .git dir, etc.  As a result even that
subdir+rebase+stash case could be done in 10ms on Windows.  Too bad
that that inside .git dir check could misfire on case insensitive
file systems, so we have to do that check with '$(git rev-parse)'.

  http://thread.gmane.org/gmane.comp.version-control.git/197432/focus=197450


Gábor

--
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] send-email: allow use of basic email list in --cc --to and --bcc

2013-06-18 Thread Jorge-Juan . Garcia-Garcia
From: Jorge Juan Garcia Garcia jorge-juan.garcia-gar...@ensimag.imag.fr

Make it so that we can use a list of email in flags
instead of having to use one flag per email address.

The format of email list handled is pretty basic for now:
$ git send-email --to='Foo f...@example.com, b...@example.com'
We thought it would be nice to have a first-step version which works
before handling more complex ones such as:
$ git send-email --to='Foo, Bar foo...@example.com'

Signed-off-by: Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr
Signed-off-by: Jorge Juan Garcia Garcia 
jorge-juan.garcia-gar...@ensimag.imag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---

Changes in the patch:
 -Update documentation
 -Removal of no-longer needed user input verification
 -New function that splits email list into seperate email addresses
 -New test to make sure it behaves the way intended

 Documentation/git-send-email.txt |   21 +++--
 git-send-email.perl  |   38 --
 t/t9001-send-email.sh|   37 -
 3 files changed, 75 insertions(+), 21 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 40a9a9a..e3444cf 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -50,16 +50,22 @@ Composing
'sendemail.multiedit'.
 
 --bcc=address::
+--bcc=[address,...]::
Specify a Bcc: value for each email. Default is the value of
'sendemail.bcc'.
-+
-The --bcc option must be repeated for each user you want on the bcc list.
+   The format supported for email list is the following:
+   Foo f...@example.com, b...@example.com.
+   Please notice that the email list does not handle commas in
+   email names such as Foo, Bar foo...@example.com.
 
 --cc=address::
+--cc=[address,...]::
Specify a starting Cc: value for each email.
Default is the value of 'sendemail.cc'.
-+
-The --cc option must be repeated for each user you want on the cc list.
+   The format supported for email list is the following:
+   Foo f...@example.com, b...@example.com.
+   Please notice that the email list does not handle commas in
+   email names such as Foo, Bar foo...@example.com.
 
 --compose::
Invoke a text editor (see GIT_EDITOR in linkgit:git-var[1])
@@ -111,12 +117,15 @@ is not set, this will be prompted for.
is not set, this will be prompted for.
 
 --to=address::
+--to=[address,...]::
Specify the primary recipient of the emails generated. Generally, this
will be the upstream maintainer of the project involved. Default is the
value of the 'sendemail.to' configuration value; if that is unspecified,
and --to-cmd is not specified, this will be prompted for.
-+
-The --to option must be repeated for each user you want on the to list.
+   The format supported for email list is the following:
+   Foo f...@example.com, b...@example.com.
+   Please notice that the email list does not handle commas in
+   email names such as Foo, Bar foo...@example.com.
 
 --8bit-encoding=encoding::
When encountering a non-ASCII message or subject that does not
diff --git a/git-send-email.perl b/git-send-email.perl
index 671762b..d7e4887 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -426,20 +426,6 @@ my ($repoauthor, $repocommitter);
 ($repoauthor) = Git::ident_person(@repo, 'author');
 ($repocommitter) = Git::ident_person(@repo, 'committer');
 
-# Verify the user input
-
-foreach my $entry (@initial_to) {
-   die Comma in --to entry: $entry'\n unless $entry !~ m/,/;
-}
-
-foreach my $entry (@initial_cc) {
-   die Comma in --cc entry: $entry'\n unless $entry !~ m/,/;
-}
-
-foreach my $entry (@bcclist) {
-   die Comma in --bcclist entry: $entry'\n unless $entry !~ m/,/;
-}
-
 sub parse_address_line {
if ($have_mail_address) {
return map { $_-format } Mail::Address-parse($_[0]);
@@ -1079,6 +1065,27 @@ sub smtp_auth_maybe {
return $auth;
 }
 
+sub split_email_list {
+my(@list) = @_;
+my @tmp;
+my @emails;
+   for (my $i = 0; $i = $#list; $i++) {
+   if ($list[$i] =~ /,/) {
+   @emails = split(/,/, $list[$i]);
+   } else {
+   @emails = $list[$i];
+   }
+   # Removal of unwanted spaces
+   for (my $j = 0; $j = $#emails; $j++) {
+   $emails[$j] =~ s/^\s+//;
+   $emails[$j] =~ s/\s+$//;
+   }
+   @tmp = (@tmp, @emails);
+   }
+return(@tmp);
+}
+
+
 # Returns 1 if the message was sent, and 0 otherwise.
 # In actuality, the whole program dies when there
 # is an error sending a message.
@@ -1089,6 +1096,9 @@ sub send_message {
  not grep { $cc eq $_ || $_ =~ /\Q${cc}\E$/ } @recipients
}
   @cc);
+  

Re: [PATCH v2 0/6] Fix checkout-dash to work with rebase

2013-06-18 Thread Ramkumar Ramachandra
Ramkumar Ramachandra wrote:
 Thanks for the reorder and commit message tweaks.  I'm working on the
 series you put up on `pu` now.

That said, I do not agree with one of your commit message updates:

checkout: respect GIT_REFLOG_ACTION

GIT_REFLOG_ACTION is an environment variable specifying the reflog
message to write after an action is completed.  Several other commands
including merge, reset, and commit respect it.

Fix the failing tests in t/checkout-last by making checkout respect it
too.  You can now expect

  $ git checkout -

to work as expected after any operation that internally uses checkout
as its implementation detail, e.g. rebase.

After the patch you _cannot_ expect checkout-dash to work after any
operation that internally uses checkout; it's limited to rebase.  Many
other scripts (eg. bisect) do _not_ set GIT_REFLOG_ACTION at all.
--
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] fixup! bash prompt: use bash builtins to find out current branch

2013-06-18 Thread SZEDER Gábor
From: SZEDER Gábor sze...@ira.uka.de

---
On Tue, Jun 18, 2013 at 11:49:31AM +0200, SZEDER Gábor wrote:
 (Hmm, speaking of which, the
 patch reading HEAD might break setups using $PROMPT_COMMAND, because
 it might do a simple return without updating $PS1...)

Indeed.  Here's the fixup.

 contrib/completion/git-prompt.sh | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 4e5c8efa..3c8c7b0d 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -362,7 +362,12 @@ __git_ps1 ()
b=$(git symbolic-ref HEAD 2/dev/null)
else
local head=
-   read head 2/dev/null $g/HEAD || return
+   if ! read head 2/dev/null $g/HEAD; then
+   if [ $pcmode = yes ]; then
+   PS1=$ps1pc_start$ps1pc_end
+   fi
+   return
+   fi
# is it a symbolic ref?
b=${head#ref: }
if [ $head = $b ]; then
-- 
1.8.3.1.487.g8f4672d

--
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 V3 1/4] git-mw: Introduction of GitMediawiki.pm

2013-06-18 Thread Matthieu Moy
Benoît Person benoit.per...@ensimag.fr writes:

From my point of view, this could definitely be improved from:
 - perlcritic -2 *.perl
 + perlcritic -2 *.perl
 \ No newline at end of file
 to something like that:
 - perlcritic -2 *.perl
 + perlcritic -2 *.perl
 \ removed newline at end of file
 which gives more insights into why the line is considered edited.

Too late ;-). This is a diff/patch thing, and Git is compatible with
them.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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] status: display the SHA1 of the commit being currently processed

2013-06-18 Thread Mathieu Liénard--Mayor

Le 2013-06-17 20:37, Junio C Hamano a écrit :

Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr
writes:


When in the middle of a rebase, it can be annoying to go in .git
in order to find the SHA1 of the commit where the rebase stopped.

git-status now includes this information in its default output.
With this new information, the message is now shorter, to avoid
too long lines.

The new message looks like:
$ git status
 HEAD detached from 33e516f
 Editing c346c87 while rebasing branch 'rebase_i_edit' on 'f90e540'.


Hmph.  It only looks into rebase-merge and not rebase-apply; is this
patch complete, or just to show a Work-In-Progress?

It's a complete patch, at least we considered it as one.
We didn't want to change the output too much, so when the old message 
was
too vague (ie. saying ...ing a commit) we replaced a commit by the 
SHA1.


I do not think you need to introduce a new stopped-sha file (if you
need it, call that with sha-1).  git rebase [-i/-m] knows where
it stopped and what the next step is without having to have such an
extra file.  Why should you need one?

I'm not following. At what point are we introducing a new file ?
What we meant to do was:
- if the user removed the file .git/.../stopped_sha for some reason,
  go back to the old too vague output
- otherwise, use the content of the file to display the SHA1 in the 
output


It seems that wt_status_get_state() tries to read in-progress state
for various operations, and I think the logic to _detect_ what to
show (i.e. what is the next commit to be replayed?  how many more
remains to be replayed?, etc.) would mix well with that function.

This patch is meant to be a first-step.
The only modification it's supposed to bring is the SHA1 where we 
stopped.
Display the list of what's left to be done isn't the purpose of this 
particular patch.

Extend wt_status_state structure to hold the necessary info, query
the state from the filesystem in that function, and display the info
(but not collect info) in show_rebase_in_progress(), to keep the
clean division of labor between these two places.

Do you mean that we should include the stopped_SHA in wt_status_state ?


Also, please pay closer attention to topics that are under
discussion in other threads.  I think Ram's Fix 'checkout -' after

Will do.

'rebase' finishes topic cf.

  
http://thread.gmane.org/gmane.comp.version-control.git/227994/focus=228092


makes the output reasonably better and consistent (please check what
I'll be pushing out on 'pu' later today after fixing some of them
up).  I suspect that this patch will conflict with it, so either you
would need to wait, or work together with that branch (i.e. rebase
on top of it as necessary), or something.
We have several modifications to make, so in the end we'll rebase on 
top of it.


In the longer term to address issues discussed in this thread cf.

  
http://thread.gmane.org/gmane.comp.version-control.git/227432/focus=227471


I think the right direction is *NOT* to keep the first HEAD
detached at line and to add more cruft to the status output as
additional lines, when various sequencer-like operations that
tentatively take you to detached HEAD state to give control back to
you in the middle.  git status knows what operation is in
progress, and I think we should start our output _without_ that
HEAD detached at line.

Thanks.


--
Mathieu Liénard--Mayor,
2nd year at Grenoble INP - ENSIMAG
(+33)6 80 56 30 02
--
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] send-email: allow use of basic email list in --cc --to and --bcc

2013-06-18 Thread Ramkumar Ramachandra
jorge-juan.garcia-gar...@ensimag.imag.fr wrote:
 The format of email list handled is pretty basic for now:
 $ git send-email --to='Foo f...@example.com, b...@example.com'
 We thought it would be nice to have a first-step version which works
 before handling more complex ones such as:
 $ git send-email --to='Foo, Bar foo...@example.com'

Is this a regression?  I can't send emails to a recipient whose name
contains a comma?
--
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] send-email: allow use of basic email list in --cc --to and --bcc

2013-06-18 Thread Mathieu Liénard--Mayor

Le 2013-06-18 12:12, Ramkumar Ramachandra a écrit :

jorge-juan.garcia-gar...@ensimag.imag.fr wrote:

The format of email list handled is pretty basic for now:
$ git send-email --to='Foo f...@example.com, 
b...@example.com'
We thought it would be nice to have a first-step version which 
works

before handling more complex ones such as:
$ git send-email --to='Foo, Bar foo...@example.com'


Is this a regression?  I can't send emails to a recipient whose name
contains a comma?

It is not. Previously the input would be considered incorrect:

-# Verify the user input
-
-foreach my $entry (@initial_to) {
-   die Comma in --to entry: $entry'\n unless $entry !~ m/,/;
-}
--
Mathieu Liénard--Mayor,
2nd year at Grenoble INP - ENSIMAG
(+33)6 80 56 30 02
--
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] send-email: allow use of basic email list in --cc --to and --bcc

2013-06-18 Thread Ramkumar Ramachandra
Mathieu Liénard--Mayor wrote:
 Is this a regression?  I can't send emails to a recipient whose name
 contains a comma?

 It is not. Previously the input would be considered incorrect:

Right.  It dies with

  Comma in --to entry: ...

This artificial limitation is imposed by 79ee555b (Check and document
the options to prevent mistakes, 2006-06-21).

Perhaps include this information in the commit message?
--
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] send-email: allow use of basic email list in --cc --to and --bcc

2013-06-18 Thread benoît person
 +sub split_email_list {
 +my(@list) = @_;
 +my @tmp;
 +my @emails;
 +   for (my $i = 0; $i = $#list; $i++) {
 +   if ($list[$i] =~ /,/) {
 +   @emails = split(/,/, $list[$i]);
 +   } else {
 +   @emails = $list[$i];
 +   }
 +   # Removal of unwanted spaces
 +   for (my $j = 0; $j = $#emails; $j++) {
 +   $emails[$j] =~ s/^\s+//;
 +   $emails[$j] =~ s/\s+$//;
 +   }
 +   @tmp = (@tmp, @emails);
 +   }
 +return(@tmp);
 +}
Why two regex ? You could do something like :
$emails[$j] =~ s/^\s+|\s+$//g;
to remove leading and trailing whitespaces at the same time.

I think it's better to use the builin 'push' function to concatenate
your two arrays:
push(@tmp, @emails);

Benoit Person
--
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 0/6] Fix checkout-dash to work with rebase

2013-06-18 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
 index a58406d..c175ef1 100755
 --- a/t/t3404-rebase-interactive.sh
 +++ b/t/t3404-rebase-interactive.sh
 @@ -934,6 +934,21 @@ test_expect_success 'rebase --edit-todo can be used to 
 modify todo' '
 test L = $(git cat-file commit HEAD | sed -ne \$p)
  '

 +test_expect_success 'rebase -i produces readable reflog' '

I don't know if this test is a good idea at all.  We never directly
check the messages written by a command to the reflog, and I suspect
that there is a good reason for this: the format of .git/logs is not
guaranteed to be stable, and the messages written by various commands
are not guaranteed to be stable either; the only machine-parsing of
reflogs we do is very minimal: interpret_nth_prior_checkout() and
grab_1st_switch().

A quick

  $ git grep .git/logs -- t

shows that I'm mostly right about this.

Why make an exception in the case of rebase -i?  In the worst case,
the patch atleast needs to be discussed as an independent patch: it's
certainly not obvious enough to sneak into this series.  I'll submit a
re-roll without this hunk.
--
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] send-email: allow use of basic email list in --cc --to and --bcc

2013-06-18 Thread Ramkumar Ramachandra
jorge-juan.garcia-gar...@ensimag.imag.fr wrote:
 diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
 index 9f46f22..87641bc 100755
 --- a/t/t9001-send-email.sh
 +++ b/t/t9001-send-email.sh
 @@ -1349,4 +1349,39 @@ test_expect_success $PREREQ 
 'sendemail.aliasfile=~/.mailrc' '
 grep ^!someone@example\.org!$ commandline1
  '

 -test_done
 +test_expect_success $PREREQ 'setup expected-list' '
 [...]
 +test_expect_success $PREREQ 'use email list in --cc --to and --bcc' '

What is the meaning of this test?  It looks like you've run git
send-email twice in exactly the same way, and compared their outputs
(after smudging the unstable headers).
--
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] send-email: allow use of basic email list in --cc --to and --bcc

2013-06-18 Thread Mathieu Liénard--Mayor

Le 2013-06-18 12:47, Ramkumar Ramachandra a écrit :

jorge-juan.garcia-gar...@ensimag.imag.fr wrote:

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 9f46f22..87641bc 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1349,4 +1349,39 @@ test_expect_success $PREREQ 
'sendemail.aliasfile=~/.mailrc' '

grep ^!someone@example\.org!$ commandline1
 '

-test_done
+test_expect_success $PREREQ 'setup expected-list' '
[...]
+test_expect_success $PREREQ 'use email list in --cc --to and --bcc' 
'


What is the meaning of this test?  It looks like you've run git
send-email twice in exactly the same way, and compared their outputs
(after smudging the unstable headers).
The first one uses one flag per email address, just like we had to do 
so far.
The second one uses one email-list per flag, which is the new feature 
we're introducing.
Then we compare the output of the two, and expect it to be exactly the 
same.


Shouldn't

$ git send-email --cc 'f...@example.com' --cc 'b...@example.com'

and

$ git send-email --cc 'f...@example.com, b...@example.com'

have the exact same effect ?
--
Mathieu Liénard--Mayor,
2nd year at Grenoble INP - ENSIMAG
(+33)6 80 56 30 02
--
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 V3 1/4] git-mw: Introduction of GitMediawiki.pm

2013-06-18 Thread Matthieu Moy
Benoît Person benoit.per...@ensimag.fr writes:

 On 17 June 2013 09:12, Matthieu Moy matthieu@grenoble-inp.fr wrote:
 Also, it seems to be only part of the solution. With your change, from
 contrib/mw-to-git/ and after running only make,

 ./git-mw takes the installed version of GitMediawiki.pm in priority

 ../../bin-wrappers/git takes the installed version of git-mw only (i.e.
 does not know git mw if make install hasn't been ran).
 Same thing as the documentation point, I think I am a bit lost in that
 whole thing. I will re-look into it for the next version :/ .

 In short, the include path should contain both the *.pm file and the
 git-foo ones.
 The fact is, for now, is there a way to test changes in
 git-remote-mediawiki.perl without 'make install'-ing it ? I could not find one

You can put contrib/mw-to-git/ in your $PATH, and then run git normally.

 So maybe in the build-perl-script of the toplevel Makefile we could add
 something copying the script at the toplevel ?

I find this a bit dirty, as it polutes the toplevel with untracked files
(that are not in the .gitignore file). But that's what the testsuite
already does (IIRC, it does a symlink), so I'd say it's OK for now.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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] send-email: allow use of basic email list in --cc --to and --bcc

2013-06-18 Thread Ramkumar Ramachandra
jorge-juan.garcia-gar...@ensimag.imag.fr wrote:
 +sub split_email_list {
 +my(@list) = @_;
 +my @tmp;
 +my @emails;
 +   for (my $i = 0; $i = $#list; $i++) {
 +   if ($list[$i] =~ /,/) {
 +   @emails = split(/,/, $list[$i]);
 +   } else {
 +   @emails = $list[$i];
 +   }

Perhaps use map like in sanitize_address_list and
validate_address_list to prettify 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: [PATCH] send-email: allow use of basic email list in --cc --to and --bcc

2013-06-18 Thread Ramkumar Ramachandra
Mathieu Liénard--Mayor wrote:
 Shouldn't

 $ git send-email --cc 'f...@example.com' --cc 'b...@example.com'

 and

 $ git send-email --cc 'f...@example.com, b...@example.com'

 have the exact same effect ?

Ah.  Perhaps it would be clearer to check the headers directly like in
the other tests?
--
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] send-email: allow use of basic email list in --cc --to and --bcc

2013-06-18 Thread Matthieu Moy
Ramkumar Ramachandra artag...@gmail.com writes:

 Mathieu Liénard--Mayor wrote:
 Shouldn't

 $ git send-email --cc 'f...@example.com' --cc 'b...@example.com'

 and

 $ git send-email --cc 'f...@example.com, b...@example.com'

 have the exact same effect ?

 Ah.  Perhaps it would be clearer to check the headers directly like in
 the other tests?

Actually, I find it more elegant like this: it doesn't rely on the
particular layout of headers, so the tests would still pass if something
else is changed in the headers.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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


send-email adds redundant From: lines to message body

2013-06-18 Thread SZEDER Gábor
Hi,

'git send-email' recently started to add redundant From: lines to my
messages, see e.g.

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

Bisect points to:

commit da18759e86bb1a7ee718c79a0c6cb15fbcbdf3c2
Author: Michael S. Tsirkin m...@redhat.com
Date:   2013-06-05 21:11:00 +0300

send-email: make --suppress-cc=self sanitize input

--suppress-cc=self fails to filter sender address in many cases where it
needs to be sanitized in some way, for example quoted:
A U. Thor aut...@example.com
To fix, make send-email sanitize both sender and the address it is
compared against.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
Signed-off-by: Junio C Hamano gits...@pobox.com

The exact commands I run were:

  git format-patch --cover-letter --subject-prefix='PATCH v2' 
2847cae835fa70f00e6e2286fbfa5595cb2247d0..bash-prompt-speedup
  vim -cover-letter.patch
  git send-email --to=git@vger.kernel.org 00*

i.e. no '--suppress-cc=self' option at all.  Values of related config
variables are:

  git config --get-regexp '(sendemail|user)\..*'
  sendemail.smtpserver /usr/local/bin/putmail
  sendemail.confirm auto
  user.name SZEDER Gábor
  user.email sze...@ira.uka.de

The 'á' in my first name is apparently significant: creating a commit
and sending it as a patch with that accent removed doesn't trigger a
redundant From: line.


Thanks,
Gábor

--
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


Undeliverable Mail

2013-06-18 Thread Postmaster
No message body: areatrain...@iberotelmakadibeach.com


Original message follows.

--
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: send-email adds redundant From: lines to message body

2013-06-18 Thread Michael S. Tsirkin
On Tue, Jun 18, 2013 at 01:09:04PM +0200, SZEDER Gábor wrote:
 Hi,
 
 'git send-email' recently started to add redundant From: lines to my
 messages, see e.g.
 
   http://article.gmane.org/gmane.comp.version-control.git/228132

Can you please show what do commits point to?
E.g. push to some public tree?

 Bisect points to:
 
 commit da18759e86bb1a7ee718c79a0c6cb15fbcbdf3c2
 Author: Michael S. Tsirkin m...@redhat.com
 Date:   2013-06-05 21:11:00 +0300
 
 send-email: make --suppress-cc=self sanitize input
 
 --suppress-cc=self fails to filter sender address in many cases where it
 needs to be sanitized in some way, for example quoted:
 A U. Thor aut...@example.com
 To fix, make send-email sanitize both sender and the address it is
 compared against.
 
 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 Signed-off-by: Junio C Hamano gits...@pobox.com
 
 The exact commands I run were:
 
   git format-patch --cover-letter --subject-prefix='PATCH v2' 
 2847cae835fa70f00e6e2286fbfa5595cb2247d0..bash-prompt-speedup
   vim -cover-letter.patch
   git send-email --to=git@vger.kernel.org 00*
 
 i.e. no '--suppress-cc=self' option at all.  Values of related config
 variables are:
 
   git config --get-regexp '(sendemail|user)\..*'
   sendemail.smtpserver /usr/local/bin/putmail
   sendemail.confirm auto
   user.name SZEDER Gábor
   user.email sze...@ira.uka.de
 
 The 'á' in my first name is apparently significant: creating a commit
 and sending it as a patch with that accent removed doesn't trigger a
 redundant From: line.
 
 
 Thanks,
 Gábor
--
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: send-email adds redundant From: lines to message body

2013-06-18 Thread SZEDER Gábor
On Tue, Jun 18, 2013 at 02:42:07PM +0300, Michael S. Tsirkin wrote:
 On Tue, Jun 18, 2013 at 01:09:04PM +0200, SZEDER Gábor wrote:
  Hi,
  
  'git send-email' recently started to add redundant From: lines to my
  messages, see e.g.
  
http://article.gmane.org/gmane.comp.version-control.git/228132
 
 Can you please show what do commits point to?
 E.g. push to some public tree?

Sure.

  https://github.com/szeder/git.git bash-prompt-speedup

--
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] name-rev: Allow to omit refs/tags/ part in --refs option when --tags used

2013-06-18 Thread Namhyung Kim

Hi Junio,

2013-06-18 AM 12:27, Junio C Hamano wrote:

Namhyung Kim namhyung@lge.com writes:


In its current form, when an user wants to filter specific ref using
--refs option, she needs to give something like --refs=refs/tags/v1.*.

This is not intuitive as users might think it's enough to give just
actual tag name part like --refs=v1.*.


I do not think Users might think is not particularly a good
justification, but I agree that it would be useful to allow
--refs=v1.\* to match refs/heads/v1.4-maint and refs/tags/v1.4.0; it
is easy for the users to disambiguate with longer prefix if they
wanted to.


Right.  I just failed to find right words. :)




It applies to refs other than
just tags too.  Change it for users to be able to use --refs=sth or
--refs=remotes/sth.

Also remove the leading 'tags/' part in the output when --tags option
was given since the option restricts to work with tags only.


This part is questionable, as it changes the output people's scripts
have been reading from the command since eternity ago.


True.



If the pattern asks to match with v1.* (not tags/v1.* or
refs/tags/v1.*) and you find refs/tags/v1.*, it might be acceptable
to strip refs/tags/ part.  Existing users are _expected_ to feed a
pattern with full refname starting with refs/, so they will not be
negatively affected by such a usability enhancement on the output
side.


This is what I wanted to do exactly. :)




diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 6238247..446743b 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -97,7 +97,8 @@ static int name_ref(const char *path, const unsigned char 
*sha1, int flags, void
if (data-tags_only  prefixcmp(path, refs/tags/))
return 0;

-   if (data-ref_filter  fnmatch(data-ref_filter, path, 0))
+   if (data-ref_filter  !prefixcmp(data-ref_filter, refs/)
+fnmatch(data-ref_filter, path, 0))
return 0;


What does this mean?  When --refs is specified, if it begins with
refs/ then do not show unmatching path, but let any path be subject
to the following if --refs does not begin with refs/ sounds like a
broken logic, unless you add another fnmatch() later in the codepath
to compensate.  And you indeed do so, but then at that point, do we
still need this if(...) return 0 at all?

I think it can and should be improved here, and then the one in the
main logic you added can be removed.

Wouldn't it make more sense to see if the given pattern matches a
tail substring of the ref, instead of using the hardcoded strip
refs/heads/, refs/tags or refs/, and then match once logic?  That
way, --refs=origin/* can find refs/remotes/origin/master by running
fnmatch of origin/* against its substrings, i.e.

refs/remotes/origin/master
 remotes/origin/master
 origin/master

and find that the pattern matches it.

Perhaps it is just the matter of adding something like:

static int subpath_matches(const char *path, const char *filter)
{
const char *subpath = path;
while (subpath) {
if (!fnmatch(data-ref_filter, subpath, 0))
return subpath - path;
subpath = strchr(path, '/');

 subpath


 if (subpath)
subpath++;
}
return -1;
}

and then at the beginning of name_ref() do this:

int can_abbreviate_output = data-name_only;

if (data-tags_only  prefixcmp(path, refs/tags/))
return 0;
if (data-ref_filter) {
switch (subpath_matches(path, data-ref_filter)) {
case -1: /* did not match */
return 0;
default: /* matched subpath */
can_abbreviate_output = 1;
break;
case 0: /* matched fully */
break;
}
}

The logic before calling name_rev() will be kept as only decide how
the output looks like, without mixing the unrelated decide if we
want to use it logic in.


Looks good to me with the little change above!

I'll resend v2 with changes in this and your other reply.

Thanks,
Namhyung
--
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 0/8] Re-roll rr/rebase-checkout-reflog

2013-06-18 Thread Ramkumar Ramachandra
This is a re-roll for rr/rebase-checkout-reflog, which is queued in
`pu`.

I've reviewed the changes carefully, and I believe that they are all
correct.  I've added a [6/8], because I think it is the correct way to
unset GIT_REFLOG_ACTION.

Junio: there is no need to re-export GIT_REFLOG_ACTION every time we
set it.  I've also fixed up some of your commit messages.  Let me know
if any further tweaks are necessary.

Thanks.

Junio C Hamano (1):
  t/status-help: test HEAD detached from

Ramkumar Ramachandra (7):
  wt-status: remove unused field in grab_1st_switch_cbdata
  t/checkout-last: test checkout - after a rebase
  status: do not depend on rebase reflog messages
  checkout: respect GIT_REFLOG_ACTION
  sh-setup: make die_with_status clear GIT_REFLOG_ACTION
  rebase: write better reflog messages
  rebase -i: write better reflog messages

 builtin/checkout.c | 11 ---
 git-rebase--am.sh  |  2 ++
 git-rebase--interactive.sh |  3 +++
 git-rebase.sh  |  8 +++-
 git-sh-setup.sh|  1 +
 t/t2012-checkout-last.sh   | 34 +
 t/t7512-status-help.sh | 47 +-
 wt-status.c|  7 ---
 8 files changed, 85 insertions(+), 28 deletions(-)

-- 
1.8.3.1.456.gb7f4cb6

--
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 4/8] status: do not depend on rebase reflog messages

2013-06-18 Thread Ramkumar Ramachandra
b397ea4 (status: show more info than currently not on any branch,
2013-03-13) attempted to make the output of 'git status' richer in
the case of a detached HEAD.  Before this patch, with a detached
HEAD, we saw:

  $ git status
  # Not currently on any branch.

But after the patch, we see:

  $ git checkout v1.8.2
  $ git status
  # HEAD detached at v1.8.2

It works by digging the reflog for the most recent message of the
form checkout: moving from  to .  It then asserts that
HEAD and  are the same, and displays this message.  When they
aren't equal, it displays:

  $ git status
  # HEAD detached from fe11db

so that the user can see where the HEAD was first detached.

In case of a rebase [-i] operation in progress, this message depends on
the implementation of rebase writing checkout:  messages to the
reflog, but that is an implementation detail of rebase.  To remove
this dependency so that rebase can be updated to write better reflog
messages, replace this HEAD detached from message with:

  # rebase in progress; onto $ONTO

Changes to the commit object name in the expected output for some of the
tests shows that what the test expected status to show during rebase
-i was not consistent with the output during a vanilla rebase, which
showed on top of what commit the series is being replayed.  Now we
consistently show something meaningful to the end user.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
 t/t7512-status-help.sh | 37 +
 wt-status.c|  5 -
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index bafa5e7..d6c66d7 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -77,7 +77,7 @@ test_expect_success 'status when rebase in progress before 
resolving conflicts'
ONTO=$(git rev-parse --short HEAD^^) 
test_must_fail git rebase HEAD^ --onto HEAD^^ 
cat expected -EOF 
-   # HEAD detached at $ONTO
+   # rebase in progress; onto $ONTO
# You are currently rebasing branch '\''rebase_conflicts'\'' on 
'\''$ONTO'\''.
#   (fix conflicts and then run git rebase --continue)
#   (use git rebase --skip to skip this patch)
@@ -104,7 +104,7 @@ test_expect_success 'status when rebase in progress before 
rebase --continue' '
echo three main.txt 
git add main.txt 
cat expected -EOF 
-   # HEAD detached at $ONTO
+   # rebase in progress; onto $ONTO
# You are currently rebasing branch '\''rebase_conflicts'\'' on 
'\''$ONTO'\''.
#   (all conflicts fixed: run git rebase --continue)
#
@@ -136,7 +136,7 @@ test_expect_success 'status during rebase -i when conflicts 
unresolved' '
ONTO=$(git rev-parse --short rebase_i_conflicts) 
test_must_fail git rebase -i rebase_i_conflicts 
cat expected -EOF 
-   # HEAD detached at $ONTO
+   # rebase in progress; onto $ONTO
# You are currently rebasing branch '\''rebase_i_conflicts_second'\'' 
on '\''$ONTO'\''.
#   (fix conflicts and then run git rebase --continue)
#   (use git rebase --skip to skip this patch)
@@ -162,7 +162,7 @@ test_expect_success 'status during rebase -i after 
resolving conflicts' '
test_must_fail git rebase -i rebase_i_conflicts 
git add main.txt 
cat expected -EOF 
-   # HEAD detached at $ONTO
+   # rebase in progress; onto $ONTO
# You are currently rebasing branch '\''rebase_i_conflicts_second'\'' 
on '\''$ONTO'\''.
#   (all conflicts fixed: run git rebase --continue)
#
@@ -188,10 +188,9 @@ test_expect_success 'status when rebasing -i in edit mode' 
'
export FAKE_LINES 
test_when_finished git rebase --abort 
ONTO=$(git rev-parse --short HEAD~2) 
-   TGT=$(git rev-parse --short two_rebase_i) 
git rebase -i HEAD~2 
cat expected -EOF 
-   # HEAD detached from $TGT
+   # rebase in progress; onto $ONTO
# You are currently editing a commit while rebasing branch 
'\''rebase_i_edit'\'' on '\''$ONTO'\''.
#   (use git commit --amend to amend the current commit)
#   (use git rebase --continue once you are satisfied with your 
changes)
@@ -216,9 +215,8 @@ test_expect_success 'status when splitting a commit' '
ONTO=$(git rev-parse --short HEAD~3) 
git rebase -i HEAD~3 
git reset HEAD^ 
-   TGT=$(git rev-parse --short HEAD) 
cat expected -EOF 
-   # HEAD detached at $TGT
+   # rebase in progress; onto $ONTO
# You are currently splitting a commit while rebasing branch 
'\''split_commit'\'' on '\''$ONTO'\''.
#   (Once your working directory is clean, run git rebase --continue)
#
@@ -246,11 +244,10 @@ test_expect_success 'status after editing the last commit 
with --amend during a
export FAKE_LINES 

[PATCH 3/8] t/checkout-last: test checkout - after a rebase

2013-06-18 Thread Ramkumar Ramachandra
  $ git checkout -

does not work as expected after a rebase.  This is because the
reflog records checkout made by rebase as its implementation
detail the same way as end-user initiated checkout, and makes it
count as the branch that was previously checked out.

Add four failing tests documenting this bug: two for a normal rebase,
and another two for an interactive rebase.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
 t/t2012-checkout-last.sh | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/t/t2012-checkout-last.sh b/t/t2012-checkout-last.sh
index b44de9d..6ad6edf 100755
--- a/t/t2012-checkout-last.sh
+++ b/t/t2012-checkout-last.sh
@@ -116,4 +116,38 @@ test_expect_success 'master...' '
test z$(git rev-parse --verify HEAD) = z$(git rev-parse --verify 
master^)
 '
 
+test_expect_failure 'checkout - works after a rebase A' '
+   git checkout master 
+   git checkout other 
+   git rebase master 
+   git checkout - 
+   test z$(git symbolic-ref HEAD) = zrefs/heads/master
+'
+
+test_expect_failure 'checkout - works after a rebase A B' '
+   git branch moodle master~1 
+   git checkout master 
+   git checkout other 
+   git rebase master moodle 
+   git checkout - 
+   test z$(git symbolic-ref HEAD) = zrefs/heads/master
+'
+
+test_expect_failure 'checkout - works after a rebase -i A' '
+   git checkout master 
+   git checkout other 
+   git rebase -i master 
+   git checkout - 
+   test z$(git symbolic-ref HEAD) = zrefs/heads/master
+'
+
+test_expect_failure 'checkout - works after a rebase -i A B' '
+   git branch foodle master~1 
+   git checkout master 
+   git checkout other 
+   git rebase master foodle 
+   git checkout - 
+   test z$(git symbolic-ref HEAD) = zrefs/heads/master
+'
+
 test_done
-- 
1.8.3.1.456.gb7f4cb6

--
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 1/8] t/status-help: test HEAD detached from

2013-06-18 Thread Ramkumar Ramachandra
From: Junio C Hamano gits...@pobox.com

b397ea (status: show more info than currently not on any branch,
2013-03-13) wanted to make sure that after a checkout to detach HEAD,
the user can see where the HEAD was originally detached from.  The last
test added by that commit to t/status-help shows one example,
immediately after HEAD is detached via a checkout.  Enhance that test to
test the HEAD detached from message is displayed when the user further
resets to another commit.

Signed-off-by: Junio C Hamano gits...@pobox.com
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 t/t7512-status-help.sh | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index bf08d4e..bafa5e7 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -667,7 +667,7 @@ test_expect_success 'status when cherry-picking after 
resolving conflicts' '
test_i18ncmp expected actual
 '
 
-test_expect_success 'status showing detached from a tag' '
+test_expect_success 'status showing detached at and from a tag' '
test_commit atag tagging 
git checkout atag 
cat expected -\EOF
@@ -675,6 +675,14 @@ test_expect_success 'status showing detached from a tag' '
nothing to commit (use -u to show untracked files)
EOF
git status --untracked-files=no actual 
+   test_i18ncmp expected actual 
+
+   git reset --hard HEAD^ 
+   cat expected -\EOF
+   # HEAD detached from atag
+   nothing to commit (use -u to show untracked files)
+   EOF
+   git status --untracked-files=no actual 
test_i18ncmp expected actual
 '
 
-- 
1.8.3.1.456.gb7f4cb6

--
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 7/8] rebase: write better reflog messages

2013-06-18 Thread Ramkumar Ramachandra
Now that the checkout invoked internally from rebase knows to honor
GIT_REFLOG_ACTION, we can start to use it to write a better reflog
message when rebase anotherbranch, rebase --onto branch,
etc. internally checks out the new fork point.  We will write:

  rebase: checkout master

instead of the old

  rebase

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
 git-rebase--am.sh | 2 ++
 git-rebase.sh | 8 +++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index 34e3102..d4bade8 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -40,9 +40,11 @@ else
rm -f $GIT_DIR/rebased-patches
case $head_name in
refs/heads/*)
+   GIT_REFLOG_ACTION=$GIT_REFLOG_ACTION: checkout 
$head_name
git checkout -q $head_name
;;
*)
+   GIT_REFLOG_ACTION=$GIT_REFLOG_ACTION: checkout 
$orig_head
git checkout -q $orig_head
;;
esac
diff --git a/git-rebase.sh b/git-rebase.sh
index d0c11a9..b43600d 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -543,7 +543,11 @@ then
if test -z $force_rebase
then
# Lazily switch to the target branch if needed...
-   test -z $switch_to || git checkout $switch_to --
+   if test -n $switch_to
+   then
+   GIT_REFLOG_ACTION=$GIT_REFLOG_ACTION: checkout 
$switch_to
+   git checkout $switch_to --
+   fi
say $(eval_gettext Current branch \$branch_name is up to 
date.)
exit 0
else
@@ -568,6 +572,8 @@ test $type = interactive  run_specific_rebase
 
 # Detach HEAD and reset the tree
 say $(gettext First, rewinding head to replay your work on top of it...)
+
+GIT_REFLOG_ACTION=$GIT_REFLOG_ACTION: checkout $onto_name
 git checkout -q $onto^0 || die could not detach HEAD
 git update-ref ORIG_HEAD $orig_head
 
-- 
1.8.3.1.456.gb7f4cb6

--
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 5/8] checkout: respect GIT_REFLOG_ACTION

2013-06-18 Thread Ramkumar Ramachandra
GIT_REFLOG_ACTION is an environment variable specifying the reflog
message to write after an action is completed.  Several other commands
including merge, reset, and commit respect it.

Fix the failing tests in t/checkout-last by making checkout respect it
too.  You can now expect

  $ git checkout -

to work as expected after any a rebase [-i].  It will also work with any
other scripts provided they set an appropriate GIT_REFLOG_ACTION if they
internally use git checkout.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
 builtin/checkout.c   | 11 ---
 t/t2012-checkout-last.sh |  8 
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index f5b50e5..1e2af85 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -587,7 +587,7 @@ static void update_refs_for_switch(const struct 
checkout_opts *opts,
   struct branch_info *new)
 {
struct strbuf msg = STRBUF_INIT;
-   const char *old_desc;
+   const char *old_desc, *reflog_msg;
if (opts-new_branch) {
if (opts-new_orphan_branch) {
if (opts-new_branch_log  !log_all_ref_updates) {
@@ -620,8 +620,13 @@ static void update_refs_for_switch(const struct 
checkout_opts *opts,
old_desc = old-name;
if (!old_desc  old-commit)
old_desc = sha1_to_hex(old-commit-object.sha1);
-   strbuf_addf(msg, checkout: moving from %s to %s,
-   old_desc ? old_desc : (invalid), new-name);
+
+   reflog_msg = getenv(GIT_REFLOG_ACTION);
+   if (!reflog_msg)
+   strbuf_addf(msg, checkout: moving from %s to %s,
+   old_desc ? old_desc : (invalid), new-name);
+   else
+   strbuf_insert(msg, 0, reflog_msg, strlen(reflog_msg));
 
if (!strcmp(new-name, HEAD)  !new-path  !opts-force_detach) {
/* Nothing to do. */
diff --git a/t/t2012-checkout-last.sh b/t/t2012-checkout-last.sh
index 6ad6edf..e7ba8c5 100755
--- a/t/t2012-checkout-last.sh
+++ b/t/t2012-checkout-last.sh
@@ -116,7 +116,7 @@ test_expect_success 'master...' '
test z$(git rev-parse --verify HEAD) = z$(git rev-parse --verify 
master^)
 '
 
-test_expect_failure 'checkout - works after a rebase A' '
+test_expect_success 'checkout - works after a rebase A' '
git checkout master 
git checkout other 
git rebase master 
@@ -124,7 +124,7 @@ test_expect_failure 'checkout - works after a rebase A' '
test z$(git symbolic-ref HEAD) = zrefs/heads/master
 '
 
-test_expect_failure 'checkout - works after a rebase A B' '
+test_expect_success 'checkout - works after a rebase A B' '
git branch moodle master~1 
git checkout master 
git checkout other 
@@ -133,7 +133,7 @@ test_expect_failure 'checkout - works after a rebase A B' 
'
test z$(git symbolic-ref HEAD) = zrefs/heads/master
 '
 
-test_expect_failure 'checkout - works after a rebase -i A' '
+test_expect_success 'checkout - works after a rebase -i A' '
git checkout master 
git checkout other 
git rebase -i master 
@@ -141,7 +141,7 @@ test_expect_failure 'checkout - works after a rebase -i 
A' '
test z$(git symbolic-ref HEAD) = zrefs/heads/master
 '
 
-test_expect_failure 'checkout - works after a rebase -i A B' '
+test_expect_success 'checkout - works after a rebase -i A B' '
git branch foodle master~1 
git checkout master 
git checkout other 
-- 
1.8.3.1.456.gb7f4cb6

--
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 2/8] wt-status: remove unused field in grab_1st_switch_cbdata

2013-06-18 Thread Ramkumar Ramachandra
The struct grab_1st_switch_cbdata has the field found, which is
set in grab_1st_switch() when a match is found.  This information is
redundant and unused by any code.  The return value of the function
serves to communicate this information anyway.

Remove the field.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
 wt-status.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index bf84a86..2511270 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1035,7 +1035,6 @@ got_nothing:
 }
 
 struct grab_1st_switch_cbdata {
-   int found;
struct strbuf buf;
unsigned char nsha1[20];
 };
@@ -1059,7 +1058,6 @@ static int grab_1st_switch(unsigned char *osha1, unsigned 
char *nsha1,
for (end = target; *end  *end != '\n'; end++)
;
strbuf_add(cb-buf, target, end - target);
-   cb-found = 1;
return 1;
 }
 
-- 
1.8.3.1.456.gb7f4cb6

--
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 8/8] rebase -i: write better reflog messages

2013-06-18 Thread Ramkumar Ramachandra
Now that the checkout invoked internally from rebase -i knows to
honor GIT_REFLOG_ACTION, we can start to use it to write a better reflog
message when rebase -i anotherbranch, rebase -i --onto branch,
etc. internally checks out the new fork point.  We will write:

  rebase -i (start): checkout master

instead of the old

  rebase -i (start)

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
 git-rebase--interactive.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index f953d8d..5179500 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -326,6 +326,7 @@ pick_one_preserving_merges () {
if [ $1 != -n ]
then
# detach HEAD to current parent
+   GIT_REFLOG_ACTION=$GIT_REFLOG_ACTION: checkout 
$first_parent
output git checkout $first_parent 2 /dev/null ||
die Cannot move HEAD to $first_parent
fi
@@ -838,6 +839,7 @@ comment_for_reflog start
 
 if test ! -z $switch_to
 then
+   GIT_REFLOG_ACTION=$GIT_REFLOG_ACTION: checkout $switch_to
output git checkout $switch_to -- ||
die Could not checkout $switch_to
 fi
@@ -981,6 +983,7 @@ has_action $todo ||
 
 test -d $rewritten || test -n $force_rebase || skip_unnecessary_picks
 
+GIT_REFLOG_ACTION=$GIT_REFLOG_ACTION: checkout $onto_name
 output git checkout $onto || die_abort could not detach HEAD
 git update-ref ORIG_HEAD $orig_head
 do_rest
-- 
1.8.3.1.456.gb7f4cb6

--
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 6/8] sh-setup: make die_with_status clear GIT_REFLOG_ACTION

2013-06-18 Thread Ramkumar Ramachandra
Several callers set GIT_REFLOG_ACTION via set_reflog_action(), but
nobody unsets it, leaving a potentially stray variable in the
environment.  Fix this by making die_with_status() unset it.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 git-sh-setup.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 2f78359..3297103 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -51,6 +51,7 @@ die () {
 }
 
 die_with_status () {
+   export GIT_REFLOG_ACTION=
status=$1
shift
echo 2 $*
-- 
1.8.3.1.456.gb7f4cb6

--
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] name-rev: Allow to omit refs/tags/ part in --refs option when --tags used

2013-06-18 Thread Namhyung Kim

2013-06-18 AM 12:53, Junio C Hamano wrote:

Junio C Hamano gits...@pobox.com writes:


Wouldn't it make more sense to see if the given pattern matches a
tail substring of the ref, instead of using the hardcoded strip
refs/heads/, refs/tags or refs/, and then match once logic?  That
way, --refs=origin/* can find refs/remotes/origin/master by running
fnmatch of origin/* against its substrings, i.e.

refs/remotes/origin/master
 remotes/origin/master
 origin/master

and find that the pattern matches it.

Perhaps it is just the matter of adding something like:
...
and then at the beginning of name_ref() do this:

int can_abbreviate_output = data-name_only;

if (data-tags_only  prefixcmp(path, refs/tags/))
return 0;
if (data-ref_filter) {
switch (subpath_matches(path, data-ref_filter)) {
case -1: /* did not match */
return 0;
default: /* matched subpath */
can_abbreviate_output = 1;
break;
case 0: /* matched fully */
break;
}
}

The logic before calling name_rev() will be kept as only decide how
the output looks like, without mixing the unrelated decide if we
want to use it logic in.


... which may make the call name_rev with this abbreviated path
logic look something like this:

if (o  o-type == OBJ_COMMIT) {
if (can_abbreviate_output)
path = shorten_unambiguous_ref(path, 0);
else if (!prefixcmp(path, refs/heads/))
path = path + 11;
else if (data-tags_only
 data-name_only
 !prefixcmp(path, refs/tags/))
path = path + 10;
else if (!prefixcmp(path, refs/))
path = path + 5;

name_rev((struct commit *) o, xstrdup(path), 0, 0, deref);
}




Hmm.. I thought about it twice.

This will affects the output of `--name-only 
--refs=refs/remotes/origin/*` case.  (AFAIK it only affected to tags so 
far)  As the name_only always sets can_abbreviate_output, it'll shorten 
the name of remote ref even if it's fully matched.


 $ ./git name-rev --refs=refs/remotes/origin/* a2055c2
 a2055c2 remotes/origin/maint~642

 $ ./git name-rev --refs=refs/remotes/origin/* --name-only a2055c2
 origin/maint~642

I think it should be 'data-tags_only  data-name_only' for 
compatibility reason.


I also see that the 3rd condition of 'tags_only  name_only' turned out 
to be useless for the similar reason.  When name_only set, it'll take 
the first case so 3rd case cannot be reached. When it's not set it 
cannot take the third case too obviously.  So I'll just remove it.


Thanks,
Namhyung
--
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 v3 2/8] test-lib: refactor $GIT_SKIP_TESTS matching

2013-06-18 Thread Thomas Rast
It's already used twice, and we will have more of the same kind of
matching in a minute.

Helped-by: Johannes Sixt j...@kdbg.org
Signed-off-by: Thomas Rast tr...@inf.ethz.ch
---
 t/test-lib.sh | 41 -
 1 file changed, 24 insertions(+), 17 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 35da859..4fa141a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -328,6 +328,20 @@ test_debug () {
test $debug =  || eval $1
 }
 
+match_pattern_list () {
+   arg=$1
+   shift
+   test -z $*  return 1
+   for pattern_
+   do
+   case $arg in
+   $pattern_)
+   return 0
+   esac
+   done
+   return 1
+}
+
 test_eval_ () {
# This is a separate function because some tests use
# return to end a test_expect_success block early.
@@ -358,14 +372,10 @@ test_run_ () {
 test_skip () {
test_count=$(($test_count+1))
to_skip=
-   for skp in $GIT_SKIP_TESTS
-   do
-   case $this_test.$test_count in
-   $skp)
-   to_skip=t
-   break
-   esac
-   done
+   if match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS
+   then
+   to_skip=t
+   fi
if test -z $to_skip  test -n $test_prereq 
   ! test_have_prereq $test_prereq
then
@@ -630,15 +640,12 @@ cd -P $TRASH_DIRECTORY || exit 1
 
 this_test=${0##*/}
 this_test=${this_test%%-*}
-for skp in $GIT_SKIP_TESTS
-do
-   case $this_test in
-   $skp)
-   say_color info 3 skipping test $this_test altogether
-   skip_all=skip all tests in $this_test
-   test_done
-   esac
-done
+if match_pattern_list $this_test $GIT_SKIP_TESTS
+then
+   say_color info 3 skipping test $this_test altogether
+   skip_all=skip all tests in $this_test
+   test_done
+fi
 
 # Provide an implementation of the 'yes' utility
 yes () {
-- 
1.8.3.1.530.g6f90e57

--
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 v3 3/8] test-lib: rearrange start/end of test_expect_* and test_skip

2013-06-18 Thread Thomas Rast
This moves

* the early setup part from test_skip to a new function test_start_

* the final common parts of test_expect_* to a new function
  test_finish_

to make the next commit more obvious.

Signed-off-by: Thomas Rast tr...@inf.ethz.ch
---
 t/test-lib-functions.sh | 6 --
 t/test-lib.sh   | 9 -
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 5251009..ad6d60a 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -343,6 +343,7 @@ test_declared_prereq () {
 }
 
 test_expect_failure () {
+   test_start_
test $# = 3  { test_prereq=$1; shift; } || test_prereq=
test $# = 2 ||
error bug in the test script: not 2 or 3 parameters to 
test-expect-failure
@@ -357,10 +358,11 @@ test_expect_failure () {
test_known_broken_failure_ $1
fi
fi
-   echo 3 
+   test_finish_
 }
 
 test_expect_success () {
+   test_start_
test $# = 3  { test_prereq=$1; shift; } || test_prereq=
test $# = 2 ||
error bug in the test script: not 2 or 3 parameters to 
test-expect-success
@@ -375,7 +377,7 @@ test_expect_success () {
test_failure_ $@
fi
fi
-   echo 3 
+   test_finish_
 }
 
 # test_external runs external test scripts that provide continuous
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 4fa141a..e99b0ea 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -369,8 +369,15 @@ test_run_ () {
return $eval_ret
 }
 
-test_skip () {
+test_start_ () {
test_count=$(($test_count+1))
+}
+
+test_finish_ () {
+   echo 3 
+}
+
+test_skip () {
to_skip=
if match_pattern_list $this_test.$test_count $GIT_SKIP_TESTS
then
-- 
1.8.3.1.530.g6f90e57

--
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 v3 0/8] --valgrind improvements

2013-06-18 Thread Thomas Rast
Changes from v2:

- $pat renamed to $pattern_ to avoid collisions (thanks j6t)

- New patch 3 that tests --verbose operation, and additions to patch
  (now) 4 that test --verbose-only.  (Adding similar tests for
  --valgrind[-only] and associated options would be nice, but is much
  harder because the user may not have valgrind.)

- Rearranged the hooking in (now) 4, 6 and 8 to make it more obvious
  what is going on, as per Peff's comments

- Fixed a misspelled variable that prevented the valgrind setup only
  once logic from working

Thomas Rast (8):
  test-lib: enable MALLOC_* for the actual tests
  test-lib: refactor $GIT_SKIP_TESTS matching
  test-lib: rearrange start/end of test_expect_* and test_skip
  test-lib: self-test that --verbose works
  test-lib: verbose mode for only tests matching a pattern
  test-lib: valgrind for only tests matching a pattern
  test-lib: allow prefixing a custom string before ok N etc.
  test-lib: support running tests under valgrind in parallel

 t/README|  10 ++
 t/t-basic.sh|  54 ++-
 t/test-lib-functions.sh |   6 +-
 t/test-lib.sh   | 244 ++--
 t/valgrind/valgrind.sh  |   3 +
 5 files changed, 265 insertions(+), 52 deletions(-)

-- 
1.8.3.1.530.g6f90e57

--
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 v3 4/8] test-lib: self-test that --verbose works

2013-06-18 Thread Thomas Rast
t contains some light self-tests of test-lib.sh, but --verbose was
not covered.  Add a test.

The only catch is that the presence of a test harness influences the
output (specifically, the presence of some empty lines).  The easiest
fix is to unset TEST_HARNESS for the sub-test scripts.  This means
that we no longer check whether test-lib.sh works under the harness;
however, since running everything under 'prove' seems far more common
than the esoteric --verbose-only feature introduced in the next
commit, this seems the smaller risk.

Signed-off-by: Thomas Rast tr...@inf.ethz.ch
---
 t/t-basic.sh | 28 +++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/t/t-basic.sh b/t/t-basic.sh
index cefe33d..b568c06 100755
--- a/t/t-basic.sh
+++ b/t/t-basic.sh
@@ -47,8 +47,10 @@ test_expect_failure 'pretend we have a known breakage' '
 
 run_sub_test_lib_test () {
name=$1 descr=$2 # stdin is the body of the test code
+   shift 2
mkdir $name 
(
+   unset HARNESS_ACTIVE
cd $name 
cat $name.sh -EOF 
#!$SHELL_PATH
@@ -65,7 +67,7 @@ run_sub_test_lib_test () {
cat $name.sh 
chmod +x $name.sh 
export TEST_DIRECTORY 
-   ./$name.sh out 2err
+   ./$name.sh $@ out 2err
)
 }
 
@@ -215,6 +217,30 @@ test_expect_success 'pretend we have a mix of all possible 
results' 
EOF
 
 
+test_expect_success 'test --verbose' '
+   test_must_fail run_sub_test_lib_test \
+   test-verbose test verbose --verbose -\EOF 
+   test_expect_success passing test true
+   test_expect_success test with output echo foo
+   test_expect_success failing test false
+   test_done
+   EOF
+   check_sub_test_lib_test test-verbose -\EOF
+expecting success: true
+ok 1 - passing test
+   
+expecting success: echo foo
+foo
+   
+ok 2 - test with output
+expecting success: false
+not ok 3 - failing test
+# false
+# failed 1 among 3 test(s)
+1..3
+   EOF
+'
+
 test_set_prereq HAVEIT
 haveit=no
 test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '
-- 
1.8.3.1.530.g6f90e57

--
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 v3 5/8] test-lib: verbose mode for only tests matching a pattern

2013-06-18 Thread Thomas Rast
With the new --verbose-only=pattern option, one can enable --verbose
at a per-test granularity.  The pattern is matched against the test
number, e.g.

  ./t-basic.sh --verbose-only='2[0-2]'

to see only the full output of test 20-22, while showing the rest in the
one-liner format.

As suggested by Jeff King, this takes care to wrap the entire
test_expect_* block, but nothing else, in the verbose toggling.  We
can use the test_start/end functions from the previous commit for the
purpose.

This is arguably not *too* useful on its own, but makes the next patch
easier to follow.

Helped-by: Jeff King p...@peff.net
Signed-off-by: Thomas Rast tr...@inf.ethz.ch
---
 t/README |  5 +
 t/t-basic.sh | 30 --
 t/test-lib.sh| 31 +++
 3 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/t/README b/t/README
index 35b3c5c..f4e6299 100644
--- a/t/README
+++ b/t/README
@@ -76,6 +76,11 @@ appropriately before running make.
command being run and their output if any are also
output.
 
+--verbose-only=pattern::
+   Like --verbose, but the effect is limited to tests with
+   numbers matching pattern.  The number matched against is
+   simply the running count of the test within the file.
+
 --debug::
This may help the person who is developing a new test.
It causes the command defined with test_debug to run.
diff --git a/t/t-basic.sh b/t/t-basic.sh
index b568c06..0d86039 100755
--- a/t/t-basic.sh
+++ b/t/t-basic.sh
@@ -225,17 +225,43 @@ test_expect_success 'test --verbose' '
test_expect_success failing test false
test_done
EOF
+   mv test-verbose/out test-verbose/out+
+   grep -v ^Initialized empty test-verbose/out+ test-verbose/out 
check_sub_test_lib_test test-verbose -\EOF
 expecting success: true
 ok 1 - passing test
-   
+Z
 expecting success: echo foo
 foo
-   
 ok 2 - test with output
+Z
 expecting success: false
 not ok 3 - failing test
 # false
+Z
+# failed 1 among 3 test(s)
+1..3
+   EOF
+'
+
+test_expect_success 'test --verbose-only' '
+   test_must_fail run_sub_test_lib_test \
+   test-verbose-only-2 test verbose-only=2 \
+   --verbose-only=2 -\EOF 
+   test_expect_success passing test true
+   test_expect_success test with output echo foo
+   test_expect_success failing test false
+   test_done
+   EOF
+   check_sub_test_lib_test test-verbose-only-2 -\EOF
+ok 1 - passing test
+Z
+expecting success: echo foo
+foo
+ok 2 - test with output
+Z
+not ok 3 - failing test
+# false
 # failed 1 among 3 test(s)
 1..3
EOF
diff --git a/t/test-lib.sh b/t/test-lib.sh
index e99b0ea..2bceb92 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -184,6 +184,9 @@ do
help=t; shift ;;
-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
verbose=t; shift ;;
+   --verbose-only=*)
+   verbose_only=$(expr z$1 : 'z[^=]*=\(.*\)')
+   shift ;;
-q|--q|--qu|--qui|--quie|--quiet)
# Ignore --quiet under a TAP::Harness. Saying how many tests
# passed without the ok/not ok details is always an error.
@@ -342,6 +345,32 @@ match_pattern_list () {
return 1
 }
 
+maybe_teardown_verbose () {
+   test -z $verbose_only  return
+   exec 4/dev/null 3/dev/null
+   verbose=
+}
+
+last_verbose=t
+maybe_setup_verbose () {
+   test -z $verbose_only  return
+   if match_pattern_list $test_count $verbose_only
+   then
+   exec 42 31
+   # Emit a delimiting blank line when going from
+   # non-verbose to verbose.  Within verbose mode the
+   # delimiter is printed by test_expect_*.  The choice
+   # of the initial $last_verbose is such that before
+   # test 1, we do not print it.
+   test -z $last_verbose  echo 3 
+   verbose=t
+   else
+   exec 4/dev/null 3/dev/null
+   verbose=
+   fi
+   last_verbose=$verbose
+}
+
 test_eval_ () {
# This is a separate function because some tests use
# return to end a test_expect_success block early.
@@ -371,10 +400,12 @@ test_run_ () {
 
 test_start_ () {
test_count=$(($test_count+1))
+   maybe_setup_verbose
 }
 
 test_finish_ () {
echo 3 
+   maybe_teardown_verbose
 }
 
 test_skip () {
-- 
1.8.3.1.530.g6f90e57

--
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 v3 8/8] test-lib: support running tests under valgrind in parallel

2013-06-18 Thread Thomas Rast
With the new --valgrind-parallel=n option, we support running the
tests in a single test script under valgrind in parallel using 'n'
processes.

This really follows the dumbest approach possible, as follows:

* We spawn the test script 'n' times, using a throw-away
  TEST_OUTPUT_DIRECTORY.  Each of the instances is given options that
  ensures that it only runs every n-th test under valgrind, but
  together they cover the entire range.

* We add up the numbers from the individual tests, and provide the
  usual output.

This is really a gross hack at this point, and should be improved.  In
particular we should keep the actual outputs somewhere more easily
discoverable, and summarize them to the user.

Nevertheless, this is already workable and gives a speedup of more
than 2 on a dual-core (hyperthreaded) machine, using n=4.  This is
expected since the overhead of valgrind is so big (on the order of 20x
under good conditions, and a large startup overhead at every git
invocation) that redundantly running the non-valgrind tests in between
is not that expensive.

Helped-by: Jeff King p...@peff.net
Signed-off-by: Thomas Rast tr...@inf.ethz.ch
---
 t/test-lib.sh | 106 ++
 1 file changed, 84 insertions(+), 22 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index f2abff9..acc7f2a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -204,6 +204,15 @@ do
--valgrind-only=*)
valgrind_only=$(expr z$1 : 'z[^=]*=\(.*\)')
shift ;;
+   --valgrind-parallel=*)
+   valgrind_parallel=$(expr z$1 : 'z[^=]*=\(.*\)')
+   shift ;;
+   --valgrind-only-stride=*)
+   valgrind_only_stride=$(expr z$1 : 'z[^=]*=\(.*\)')
+   shift ;;
+   --valgrind-only-offset=*)
+   valgrind_only_offset=$(expr z$1 : 'z[^=]*=\(.*\)')
+   shift ;;
--tee)
shift ;; # was handled already
--root=*)
@@ -217,7 +226,7 @@ do
esac
 done
 
-if test -n $valgrind_only
+if test -n $valgrind_only || test -n $valgrind_only_stride
 then
test -z $valgrind  valgrind=memcheck
test -z $verbose  verbose_only=$valgrind_only
@@ -367,7 +376,9 @@ maybe_teardown_verbose () {
 last_verbose=t
 maybe_setup_verbose () {
test -z $verbose_only  return
-   if match_pattern_list $test_count $verbose_only
+   if match_pattern_list $test_count $verbose_only ||
+   { test -n $valgrind_only_stride 
+   expr $test_count % $valgrind_only_stride - 
$valgrind_only_offset = 0 /dev/null; }
then
exec 42 31
# Emit a delimiting blank line when going from
@@ -391,7 +402,7 @@ maybe_teardown_valgrind () {
 
 maybe_setup_valgrind () {
test -z $GIT_VALGRIND  return
-   if test -z $valgrind_only
+   if test -z $valgrind_only  test -z $valgrind_only_stride
then
GIT_VALGRIND_ENABLED=t
return
@@ -400,6 +411,10 @@ maybe_setup_valgrind () {
if match_pattern_list $test_count $valgrind_only
then
GIT_VALGRIND_ENABLED=t
+   elif test -n $valgrind_only_stride 
+   expr $test_count % $valgrind_only_stride - 
$valgrind_only_offset = 0 /dev/null
+   then
+   GIT_VALGRIND_ENABLED=t
fi
 }
 
@@ -550,6 +565,9 @@ test_done () {
esac
 }
 
+
+# Set up a directory that we can put in PATH which redirects all git
+# calls to 'valgrind git ...'.
 if test -n $valgrind
 then
make_symlink () {
@@ -597,33 +615,42 @@ then
make_symlink $symlink_target $GIT_VALGRIND/bin/$base || exit
}
 
-   # override all git executables in TEST_DIRECTORY/..
-   GIT_VALGRIND=$TEST_DIRECTORY/valgrind
-   mkdir -p $GIT_VALGRIND/bin
-   for file in $GIT_BUILD_DIR/git* $GIT_BUILD_DIR/test-*
-   do
-   make_valgrind_symlink $file
-   done
-   # special-case the mergetools loadables
-   make_symlink $GIT_BUILD_DIR/mergetools $GIT_VALGRIND/bin/mergetools
-   OLDIFS=$IFS
-   IFS=:
-   for path in $PATH
-   do
-   ls $path/git-* 2 /dev/null |
-   while read file
+   # In the case of --valgrind-parallel, we only need to do the
+   # wrapping once, in the main script.  The worker children all
+   # have $valgrind_only_stride set, so we can skip based on that.
+   if test -z $valgrind_only_stride
+   then
+   # override all git executables in TEST_DIRECTORY/..
+   GIT_VALGRIND=$TEST_DIRECTORY/valgrind
+   mkdir -p $GIT_VALGRIND/bin
+   for file in $GIT_BUILD_DIR/git* $GIT_BUILD_DIR/test-*
do
-   make_valgrind_symlink $file
+   make_valgrind_symlink $file
done
-   done
-   IFS=$OLDIFS
+   # special-case the mergetools loadables
+   

[PATCH v3 6/8] test-lib: valgrind for only tests matching a pattern

2013-06-18 Thread Thomas Rast
With the new --valgrind-only=pattern option, one can enable
--valgrind at a per-test granularity, exactly analogous to
--verbose-only from the previous commit.

The options are wired such that --valgrind implies --verbose (as
before), but --valgrind-only=pattern implies
--verbose-only=pattern unless --verbose is also in effect.

Signed-off-by: Thomas Rast tr...@inf.ethz.ch
---
 t/README   |  5 +
 t/test-lib.sh  | 36 +++-
 t/valgrind/valgrind.sh |  3 +++
 3 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/t/README b/t/README
index f4e6299..abe991f 100644
--- a/t/README
+++ b/t/README
@@ -126,6 +126,11 @@ appropriately before running make.
the 't/valgrind/' directory and use the commands under
't/valgrind/bin/'.
 
+--valgrind-only=pattern::
+   Like --valgrind, but the effect is limited to tests with
+   numbers matching pattern.  The number matched against is
+   simply the running count of the test within the file.
+
 --tee::
In addition to printing the test output to the terminal,
write it to files named 't/test-results/$TEST_NAME.out'.
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 2bceb92..817ab43 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -201,6 +201,9 @@ do
--valgrind=*)
valgrind=$(expr z$1 : 'z[^=]*=\(.*\)')
shift ;;
+   --valgrind-only=*)
+   valgrind_only=$(expr z$1 : 'z[^=]*=\(.*\)')
+   shift ;;
--tee)
shift ;; # was handled already
--root=*)
@@ -211,7 +214,14 @@ do
esac
 done
 
-test -n $valgrind  verbose=t
+if test -n $valgrind_only
+then
+   test -z $valgrind  valgrind=memcheck
+   test -z $verbose  verbose_only=$valgrind_only
+elif test -n $valgrind
+then
+   verbose=t
+fi
 
 if test -n $color
 then
@@ -371,6 +381,25 @@ maybe_setup_verbose () {
last_verbose=$verbose
 }
 
+maybe_teardown_valgrind () {
+   test -z $GIT_VALGRIND  return
+   GIT_VALGRIND_ENABLED=
+}
+
+maybe_setup_valgrind () {
+   test -z $GIT_VALGRIND  return
+   if test -z $valgrind_only
+   then
+   GIT_VALGRIND_ENABLED=t
+   return
+   fi
+   GIT_VALGRIND_ENABLED=
+   if match_pattern_list $test_count $valgrind_only
+   then
+   GIT_VALGRIND_ENABLED=t
+   fi
+}
+
 test_eval_ () {
# This is a separate function because some tests use
# return to end a test_expect_success block early.
@@ -401,10 +430,12 @@ test_run_ () {
 test_start_ () {
test_count=$(($test_count+1))
maybe_setup_verbose
+   maybe_setup_valgrind
 }
 
 test_finish_ () {
echo 3 
+   maybe_teardown_valgrind
maybe_teardown_verbose
 }
 
@@ -588,6 +619,9 @@ then
export GIT_VALGRIND
GIT_VALGRIND_MODE=$valgrind
export GIT_VALGRIND_MODE
+   GIT_VALGRIND_ENABLED=t
+   test -n $valgrind_only  GIT_VALGRIND_ENABLED=
+   export GIT_VALGRIND_ENABLED
 elif test -n $GIT_TEST_INSTALLED
 then
GIT_EXEC_PATH=$($GIT_TEST_INSTALLED/git --exec-path)  ||
diff --git a/t/valgrind/valgrind.sh b/t/valgrind/valgrind.sh
index 6b87c91..4215303 100755
--- a/t/valgrind/valgrind.sh
+++ b/t/valgrind/valgrind.sh
@@ -4,6 +4,9 @@ base=$(basename $0)
 
 TOOL_OPTIONS='--leak-check=no'
 
+test -z $GIT_VALGRIND_ENABLED 
+exec $GIT_VALGRIND/../../$base $@
+
 case $GIT_VALGRIND_MODE in
 memcheck-fast)
;;
-- 
1.8.3.1.530.g6f90e57

--
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 v3 1/8] test-lib: enable MALLOC_* for the actual tests

2013-06-18 Thread Thomas Rast
1b3185f (MALLOC_CHECK: various clean-ups, 2012-09-14) moved around the
MALLOC_CHECK_ and MALLOC_PERTURB_ assignments, intending to limit
their effect to only the test runs.  However, they were actually
enabled only during test cleanup.  Call setup/teardown_malloc_check
also around the evaluation of the actual test snippet.

Signed-off-by: Thomas Rast tr...@inf.ethz.ch
---
 t/test-lib.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index eff3a65..35da859 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -337,8 +337,10 @@ test_eval_ () {
 test_run_ () {
test_cleanup=:
expecting_failure=$2
+   setup_malloc_check
test_eval_ $1
eval_ret=$?
+   teardown_malloc_check
 
if test -z $immediate || test $eval_ret = 0 || test -n 
$expecting_failure
then
-- 
1.8.3.1.530.g6f90e57

--
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 v3 7/8] test-lib: allow prefixing a custom string before ok N etc.

2013-06-18 Thread Thomas Rast
This is not really meant for external use, and thus not documented. It
allows the next commit to neatly distinguish between sub-tests and the
main run.

The format is intentionally not valid TAP.  The use in the next commit
would not result in anything valid either way, and it seems better to
make it obvious.

Signed-off-by: Thomas Rast tr...@inf.ethz.ch
---
 t/test-lib.sh | 27 +++
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 817ab43..f2abff9 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -209,6 +209,9 @@ do
--root=*)
root=$(expr z$1 : 'z[^=]*=\(.*\)')
shift ;;
+   --statusprefix=*)
+   statusprefix=$(expr z$1 : 'z[^=]*=\(.*\)')
+   shift ;;
*)
echo error: unknown test option '$1' 2; exit 1 ;;
esac
@@ -316,12 +319,12 @@ trap 'die' EXIT
 
 test_ok_ () {
test_success=$(($test_success + 1))
-   say_color  ok $test_count - $@
+   say_color  ${statusprefix}ok $test_count - $@
 }
 
 test_failure_ () {
test_failure=$(($test_failure + 1))
-   say_color error not ok $test_count - $1
+   say_color error ${statusprefix}not ok $test_count - $1
shift
echo $@ | sed -e 's/^/#   /'
test $immediate =  || { GIT_EXIT_OK=t; exit 1; }
@@ -329,12 +332,12 @@ test_failure_ () {
 
 test_known_broken_ok_ () {
test_fixed=$(($test_fixed+1))
-   say_color error ok $test_count - $@ # TODO known breakage vanished
+   say_color error ${statusprefix}ok $test_count - $@ # TODO known 
breakage vanished
 }
 
 test_known_broken_failure_ () {
test_broken=$(($test_broken+1))
-   say_color warn not ok $test_count - $@ # TODO known breakage
+   say_color warn ${statusprefix}not ok $test_count - $@ # TODO known 
breakage
 }
 
 test_debug () {
@@ -458,8 +461,8 @@ test_skip () {
of_prereq= of $test_prereq
fi
 
-   say_color skip 3 skipping test: $@
-   say_color skip ok $test_count # skip $1 (missing 
$missing_prereq${of_prereq})
+   say_color skip 3 ${statusprefix}skipping test: $@
+   say_color skip ${statusprefix}ok $test_count # skip $1 
(missing $missing_prereq${of_prereq})
: true
;;
*)
@@ -495,11 +498,11 @@ test_done () {
 
if test $test_fixed != 0
then
-   say_color error # $test_fixed known breakage(s) vanished; 
please update test(s)
+   say_color error ${statusprefix}# $test_fixed known breakage(s) 
vanished; please update test(s)
fi
if test $test_broken != 0
then
-   say_color warn # still have $test_broken known breakage(s)
+   say_color warn ${statusprefix}# still have $test_broken known 
breakage(s)
fi
if test $test_broken != 0 || test $test_fixed != 0
then
@@ -522,9 +525,9 @@ test_done () {
then
if test $test_remaining -gt 0
then
-   say_color pass # passed all $msg
+   say_color pass ${statusprefix}# passed all 
$msg
fi
-   say 1..$test_count$skip_all
+   say ${statusprefix}1..$test_count$skip_all
fi
 
test -d $remove_trash 
@@ -538,8 +541,8 @@ test_done () {
*)
if test $test_external_has_tap -eq 0
then
-   say_color error # failed $test_failure among $msg
-   say 1..$test_count
+   say_color error ${statusprefix}# failed $test_failure 
among $msg
+   say ${statusprefix}1..$test_count
fi
 
exit 1 ;;
-- 
1.8.3.1.530.g6f90e57

--
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 v2] name-rev: Allow to specify a subpath for --refs option

2013-06-18 Thread Namhyung Kim
From: Namhyung Kim namhyung@lge.com

In its current form, when an user wants to filter specific ref using
 --refs option, she needs to give something like --refs=refs/tags/v1.*.

It'd be convenient providing a way to specify a subpath of ref pattern.
For example, --refs=origin/* can find refs/remotes/origin/master by
searching the pattern against its substrings in turn:

  refs/remotes/origin/master
  remotes/origin/master
  origin/master

If it finds a match in a subpath, unambigous part of the ref path will
be removed in the output.

Many thanks to Junio C. Hamano for suggesting better logic and code.

Signed-off-by: Namhyung Kim namhyung@lge.com
---
 Documentation/git-name-rev.txt |  3 ++-
 builtin/name-rev.c | 36 +---
 2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-name-rev.txt b/Documentation/git-name-rev.txt
index ad1d146..6b0f1ba 100644
--- a/Documentation/git-name-rev.txt
+++ b/Documentation/git-name-rev.txt
@@ -25,7 +25,8 @@ OPTIONS
Do not use branch names, but only tags to name the commits
 
 --refs=pattern::
-   Only use refs whose names match a given shell pattern.
+   Only use refs whose names match a given shell pattern.  The pattern
+   can be one of branch name, tag name or fully qualified ref name.
 
 --all::
List all commits reachable from all refs
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 6238247..87d4854 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -82,6 +82,20 @@ copy_data:
}
 }
 
+static int subpath_matches(const char *path, const char *filter)
+{
+   const char *subpath = path;
+
+   while (subpath) {
+   if (!fnmatch(filter, subpath, 0))
+   return subpath - path;
+   subpath = strchr(subpath, '/');
+   if (subpath)
+   subpath++;
+   }
+   return -1;
+}
+
 struct name_ref_data {
int tags_only;
int name_only;
@@ -92,13 +106,23 @@ static int name_ref(const char *path, const unsigned char 
*sha1, int flags, void
 {
struct object *o = parse_object(sha1);
struct name_ref_data *data = cb_data;
+   int can_abbreviate_output = data-tags_only  data-name_only;
int deref = 0;
 
if (data-tags_only  prefixcmp(path, refs/tags/))
return 0;
 
-   if (data-ref_filter  fnmatch(data-ref_filter, path, 0))
-   return 0;
+   if (data-ref_filter) {
+   switch (subpath_matches(path, data-ref_filter)) {
+   case -1: /* did not match */
+   return 0;
+   case 0:  /* matched fully */
+   break;
+   default: /* matched subpath */
+   can_abbreviate_output = 1;
+   break;
+   }
+   }
 
while (o  o-type == OBJ_TAG) {
struct tag *t = (struct tag *) o;
@@ -110,12 +134,10 @@ static int name_ref(const char *path, const unsigned char 
*sha1, int flags, void
if (o  o-type == OBJ_COMMIT) {
struct commit *commit = (struct commit *)o;
 
-   if (!prefixcmp(path, refs/heads/))
+   if (can_abbreviate_output)
+   path = shorten_unambiguous_ref(path, 0);
+   else if (!prefixcmp(path, refs/heads/))
path = path + 11;
-   else if (data-tags_only
-data-name_only
-!prefixcmp(path, refs/tags/))
-   path = path + 10;
else if (!prefixcmp(path, refs/))
path = path + 5;
 
-- 
1.7.11.7

--
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: send-email adds redundant From: lines to message body

2013-06-18 Thread Michael S. Tsirkin
On Tue, Jun 18, 2013 at 01:48:00PM +0200, SZEDER Gábor wrote:
 On Tue, Jun 18, 2013 at 02:42:07PM +0300, Michael S. Tsirkin wrote:
  On Tue, Jun 18, 2013 at 01:09:04PM +0200, SZEDER Gábor wrote:
   Hi,
   
   'git send-email' recently started to add redundant From: lines to my
   messages, see e.g.
   
 http://article.gmane.org/gmane.comp.version-control.git/228132
  
  Can you please show what do commits point to?
  E.g. push to some public tree?
 
 Sure.
 
   https://github.com/szeder/git.git bash-prompt-speedup

I see. The From line looks like this:
From: =?UTF-8?q?SZEDER=20G=C3=A1bor?= sze...@ira.uka.de
which is why it didn't match.

I added a test like this and sure enough, it fails.
Will look into a fix now.

---
send-email: add test for duplicate utf8 name

Verify that author name is not duplicated if it matches
sender, even if it is in utf8.

Signed-off-by: Michael S. Tsirkin m...@redhat.com

---

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 9f46f22..cef884f 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -956,6 +956,19 @@ test_expect_success $PREREQ 'utf8 author is correctly 
passed on' '
grep ^From: Füñný Nâmé odd_?=m...@example.com msgtxt1
 '
 
+test_expect_success $PREREQ 'utf8 sender is not duplicated' '
+   clean_fake_sendmail 
+   test_commit weird_sender 
+   test_when_finished git reset --hard HEAD^ 
+   git commit --amend --author Füñný Nâmé odd_?=m...@example.com 
+   git format-patch --stdout -1 funny_name.patch 
+   git send-email --from=Füñný Nâmé odd_?=m...@example.com \
+ --to=nob...@example.com \
+ --smtp-server=$(pwd)/fake.sendmail \
+ funny_name.patch 
+   test `grep ^From: msgtxt1|wc -l` -eq 1
+'
+
 test_expect_success $PREREQ 'sendemail.composeencoding works' '
clean_fake_sendmail 
git config sendemail.composeencoding iso-8859-1 
--
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 4/8] status: do not depend on rebase reflog messages

2013-06-18 Thread Peter Krefting

Ramkumar Ramachandra:


+   on_what = _(rebase in progress; onto );


Could you please add a

  /* TRANSLATORS: Followed by branch name. */

or something similar here (and possibly to the other on_whats in the 
function)?


Ideally, the %s for the branch name should be inside that 
on_what string, but I guess that can be difficult since it is output 
in a different colour?


--
\\// Peter - http://www.softwolves.pp.se/
--
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 2/2] send-email: add test for duplicate utf8 name

2013-06-18 Thread Michael S. Tsirkin
Verify that author name is not duplicated if it matches
sender, even if it is in utf8.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 t/t9001-send-email.sh | 13 +
 1 file changed, 13 insertions(+)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 9f46f22..cef884f 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -956,6 +956,19 @@ test_expect_success $PREREQ 'utf8 author is correctly 
passed on' '
grep ^From: Füñný Nâmé odd_?=m...@example.com msgtxt1
 '
 
+test_expect_success $PREREQ 'utf8 sender is not duplicated' '
+   clean_fake_sendmail 
+   test_commit weird_sender 
+   test_when_finished git reset --hard HEAD^ 
+   git commit --amend --author Füñný Nâmé odd_?=m...@example.com 
+   git format-patch --stdout -1 funny_name.patch 
+   git send-email --from=Füñný Nâmé odd_?=m...@example.com \
+ --to=nob...@example.com \
+ --smtp-server=$(pwd)/fake.sendmail \
+ funny_name.patch 
+   test `grep ^From: msgtxt1|wc -l` -eq 1
+'
+
 test_expect_success $PREREQ 'sendemail.composeencoding works' '
clean_fake_sendmail 
git config sendemail.composeencoding iso-8859-1 
-- 
MST
--
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: send-email adds redundant From: lines to message body

2013-06-18 Thread Michael S. Tsirkin
On Tue, Jun 18, 2013 at 01:48:00PM +0200, SZEDER Gábor wrote:
 On Tue, Jun 18, 2013 at 02:42:07PM +0300, Michael S. Tsirkin wrote:
  On Tue, Jun 18, 2013 at 01:09:04PM +0200, SZEDER Gábor wrote:
   Hi,
   
   'git send-email' recently started to add redundant From: lines to my
   messages, see e.g.
   
 http://article.gmane.org/gmane.comp.version-control.git/228132
  
  Can you please show what do commits point to?
  E.g. push to some public tree?
 
 Sure.
 
   https://github.com/szeder/git.git bash-prompt-speedup

Okay, missed sanitizing author in one more place.
I sent a patch that should fix it for you,
tested-by reports appreciated.

-- 
MST
--
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 1/2] send-email: sanitize author when writing From line

2013-06-18 Thread Michael S. Tsirkin
sender is now sanitized, but we didn't sanitize
author when checking whether From: line is needed
in the message body.
As a result git started writing duplicate From lines
when author matched sender and has utf8 characters.

Reported-by: SZEDER Gábor sze...@ira.uka.de
Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 git-send-email.perl | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 671762b..ecbf56f 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1259,6 +1259,7 @@ foreach my $t (@files) {
open my $fh, , $t or die can't open file $t;
 
my $author = undef;
+   my $sauthor = undef;
my $author_encoding;
my $has_content_type;
my $body_encoding;
@@ -1297,7 +1298,7 @@ foreach my $t (@files) {
}
elsif (/^From:\s+(.*)$/i) {
($author, $author_encoding) = 
unquote_rfc2047($1);
-   my $sauthor = sanitize_address($author);
+   $sauthor = sanitize_address($author);
next if $suppress_cc{'author'};
next if $suppress_cc{'self'} and $sauthor eq 
$sender;
printf((mbox) Adding cc: %s from line '%s'\n,
@@ -1393,7 +1394,7 @@ foreach my $t (@files) {
$subject = quote_subject($subject, $auto_8bit_encoding);
}
 
-   if (defined $author and $author ne $sender) {
+   if (defined $sauthor and $sauthor ne $sender) {
$message = From: $author\n\n$message;
if (defined $author_encoding) {
if ($has_content_type) {
-- 
MST
--
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


[Request] Git reset should be able to ignore file permissions

2013-06-18 Thread Alexander Nestorov
Recently I had to write some automation scripts and I found
that git reset --hard actually restores each file's permissions.

That is causing both the created and the last-modified dates
of the file to get changed to the time of the git reset.

This behavior is easy to demonstrate:

echo test  myfile
chmod 777 myfile
git add myfile  git commit -m Test  git push
chmod 775 myfile
git reset --hard origin/master

After the git reset --hard command, the entire file was
checkout-ed. Instead, git should be able to check if the
content of the file changed and only if it did, check it out.

I do realize that checking the content of each file in a big
repo could result in a slow operation, but there should be a
switch/argument/option to make git reset actually check the
content of each file instead of blindly replacing it.

After reading man a few times I didn't saw any option
that'd let me do this; the only solution I'm able to think about
is actually restoring the permissions of each file to the ones
git thinks they should have before doing the git reset.

Maybe I'm wrong and there is a way for doing what I want, if
so, please correct me.
But if there isn't, should this be implemented? Are there any
reasons for not doing it?


Thank you for your attention
Regards

--
alexandernst
--
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: [Request] Git reset should be able to ignore file permissions

2013-06-18 Thread Matthieu Moy
Alexander Nestorov alexander...@gmail.com writes:

 echo test  myfile
 chmod 777 myfile
 git add myfile  git commit -m Test  git push
 chmod 775 myfile
 git reset --hard origin/master

This doesn't tell what the permissions are in origin/master.

If the last line was git reset --hard HEAD, then it wouldn't touch
myfile (it's executable in the worktree and in HEAD, so Git doesn't need
to change it). Neither the x bit, nor the ctime or mtime.

If you reset the file to a point where it was not executable, then Git
changes its executable bit, and I don't see why it would do otherwise:
Git tracks the executable bit, so when you say reset the file to how it
was in this revision, this includes the content and executability.

Reading your message, I don't understand why you need to be able to
ignore the x bit.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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] t/t9802: explicitly name the upstream branch to use as a base

2013-06-18 Thread Pete Wyckoff
bca...@nvidia.com wrote on Mon, 17 Jun 2013 18:40 -0700:
 From: Brandon Casey draf...@gmail.com
 
 Prior to commit fa83a33b, the 'git checkout' DWIMery would create a
 new local branch if the specified branch name did not exist and it
 matched exactly one ref in the remotes namespace.  It searched
 the remotes namespace for matching refs using a simple comparison
 of the trailing portion of the remote ref names.  This approach
 could sometimes produce false positives or negatives.
 
 Since fa83a33b, the DWIMery more strictly excludes the remote name
 from the ref comparison by iterating through the remotes that are
 configured in the .gitconfig file.  This has the side-effect that
 any refs that exist in the remotes namespace, but do not match
 the destination side of any remote refspec, will not be used by
 the DWIMery.
 
 This change in behavior breaks the tests in t9802 which relied on
 the old behavior of searching all refs in the remotes namespace,
 since the git-p4 script does not configure any remotes in the
 .gitconfig.  Let's work around this in these tests by explicitly
 naming the upstream branch to base the new local branch on when
 calling 'git checkout'.

Thanks for finding and fixing this.  Great explanation.  I
tested it locally too.

Acked-by: Pete Wyckoff p...@padd.com

-- Pete
--
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: [Request] Git reset should be able to ignore file permissions

2013-06-18 Thread Alexander Nestorov
Git does preserve file permissions, that is, git is aware of the
permissions you can set with chmod.

I'm not trying to ignore the x bit, what I'm trying to do is make
git reset checkout only the files that actually changed instead
of checking out all the files with different permissions than the
ones git thinks they should have.

Said with other word: when you run git reset, git does a status
and checkouts all the files that showed up from the status.
That's exactly what I'm trying to avoid, as status is aware of both
content changes and permissions changes.
--
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: Exact format of tree objets

2013-06-18 Thread Chico Sokol
Thanks!

By the way, where can I find this kind of specification? I couldn't
find the spec of tree objects here:
https://github.com/git/git/tree/master/Documentation


--
Chico Sokol


On Wed, Jun 12, 2013 at 11:06 AM, Jakub Narebski jna...@gmail.com wrote:
 Junio C Hamano gitster at pobox.com writes:
 Chico Sokol chico.sokol at gmail.com writes:

  Is there any official documentation of tree objets format? Are tree
  objects encoded specially in some way? How can I parse the inflated
  contents of a tree object?
 
  We're suspecting that there is some kind of special format or
  encoding, because the command git cat-file -p sha show me ...
  While git cat-file tree sha generate ...

 cat-file -p is meant to be human-readable form.  The latter gives
 the exact byte contents read_sha1_file() sees, which is a binary
 format.  Essentially, it is a sequence of:

  - mode of the entry encoded in octal, without any leading '0' pad;
  - pathname component of the entry, terminated with NUL;
  - 20-byte SHA-1 object name.

 I always wondered why this is the sole object format where SHA-1 is in 20-
 byte binary format and not 40-chars hexadecimal string format...

 --
 Jakub Narębski




 --
 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 2/2] send-email: add test for duplicate utf8 name

2013-06-18 Thread SZEDER Gábor
Hi Michael,


thanks for the quick turnaround.

On Tue, Jun 18, 2013 at 03:49:29PM +0300, Michael S. Tsirkin wrote:
 Verify that author name is not duplicated if it matches
 sender, even if it is in utf8.
 
 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 ---
  t/t9001-send-email.sh | 13 +
  1 file changed, 13 insertions(+)
 
 diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
 index 9f46f22..cef884f 100755
 --- a/t/t9001-send-email.sh
 +++ b/t/t9001-send-email.sh
 @@ -956,6 +956,19 @@ test_expect_success $PREREQ 'utf8 author is correctly 
 passed on' '
   grep ^From: Füñný Nâmé odd_?=m...@example.com msgtxt1
  '
  
 +test_expect_success $PREREQ 'utf8 sender is not duplicated' '
 + clean_fake_sendmail 
 + test_commit weird_sender 
 + test_when_finished git reset --hard HEAD^ 
 + git commit --amend --author Füñný Nâmé odd_?=m...@example.com 
 + git format-patch --stdout -1 funny_name.patch 
 + git send-email --from=Füñný Nâmé odd_?=m...@example.com \
 +   --to=nob...@example.com \
 +   --smtp-server=$(pwd)/fake.sendmail \
 +   funny_name.patch 
 + test `grep ^From: msgtxt1|wc -l` -eq 1

Perhaps you could use here

  grep ^From: msgtxt1 from 
  test_line_count = 1 from

instead, to make it more debugging-friendly.

 +'
 +
  test_expect_success $PREREQ 'sendemail.composeencoding works' '
   clean_fake_sendmail 
   git config sendemail.composeencoding iso-8859-1 
 -- 
 MST

I couldn't apply this patch with git am/apply on top of today's master
for some reason, although the context lines seem to match exactly...
in the end had to apply it manually.

Anyway, 1/2 fixes my problem, so:

Tested-by: SZEDER Gábor sze...@ira.uka.de


Thanks,
Gábor

--
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: [Request] Git reset should be able to ignore file permissions

2013-06-18 Thread John Keeping
On Tue, Jun 18, 2013 at 03:25:22PM +0200, Alexander Nestorov wrote:
 Recently I had to write some automation scripts and I found
 that git reset --hard actually restores each file's permissions.
 
 That is causing both the created and the last-modified dates
 of the file to get changed to the time of the git reset.
 
 This behavior is easy to demonstrate:
 
 echo test  myfile
 chmod 777 myfile
 git add myfile  git commit -m Test  git push
 chmod 775 myfile
 git reset --hard origin/master
 
 After the git reset --hard command, the entire file was
 checkout-ed. Instead, git should be able to check if the
 content of the file changed and only if it did, check it out.

Does git reset --keep behave in the same way?  I would expect it to
leave permissions as they were.
--
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: [Request] Git reset should be able to ignore file permissions

2013-06-18 Thread Matthieu Moy
Alexander Nestorov alexander...@gmail.com writes:

 I'm not trying to ignore the x bit, what I'm trying to do is make
 git reset checkout only the files that actually changed instead
 of checking out all the files with different permissions than the
 ones git thinks they should have.

Ah, OK, you want git reset --hard to just do a chmod, which would not
touch the file's mtime (but only the ctime).

Then, it's even easier to demonstrate: just touch instead of chmod.
Indeed:

$ touch myfile; sleep 2
$ strace -f git reset --hard 21 | grep myfile
lstat(myfile, {st_mode=S_IFREG|0755, st_size=5, ...}) = 0
lstat(myfile, {st_mode=S_IFREG|0755, st_size=5, ...}) = 0
unlink(myfile)= 0
open(myfile, O_WRONLY|O_CREAT|O_EXCL, 0777) = 4

(sleep 2 is needed in the demonstration to avoid the racy git
safeties, but it's not really important)

Git doesn't even try to read the file content: once it detected that the
stat information changed, it rewrite the file without looking at its
content. It's faster this way for files that actually changed.

 Said with other word: when you run git reset, git does a status
 and checkouts all the files that showed up from the status.

No, it's indeed the opposite: status re-checks the content of changed
files, and update the stat-cache in the index accordingly if the content
actually didn't change.

Runing git status before git reset --hard should solve your problem.
The part of git status of interest is git update-index --refresh:

$ touch myfile; sleep 2
$ git update-index --refresh
$ strace -f git reset --hard 21 | grep myfile
lstat(myfile, {st_mode=S_IFREG|0755, st_size=5, ...}) = 0
$

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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: [Request] Git reset should be able to ignore file permissions

2013-06-18 Thread Alexander Nestorov
Git reset --keep is not an option as it will abort the operation if
there are local changes,
which is exactly what I want to do: replace files with local changes
but leave file
permissions as they are.
--
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: [Request] Git reset should be able to ignore file permissions

2013-06-18 Thread Alexander Nestorov
Indeed, git update-index --refresh before git reset did the trick :)
Anyways, what about the proposal? Should it be implemented?

Thank you
--
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] sha1_name: stop hard-coding 40-character hex checks

2013-06-18 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes:

 Phil Hord wrote:
 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.

 Please read the cover-letter: 

Which was...

 [1/2] is important.  [2/2] is a minor prettification, that wouldn't
 have been possible without [1/2].

...and I do not find how the above is connected with

 I was just toying around to see if this
 was a good idea,...

...this in any way.  Puzzled.
--
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 v3 1/2] am: handle stray $dotest directory

2013-06-18 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes:

 Junio C Hamano wrote:
 Will replace what has been queued on 'pu'.

 ... after fixing an indentation error, that is.

 Where did the error occur?

I think you can compare what is in 'pu' and what you sent out
without asking.

--
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 v3 4/5] stash: introduce 'git stash store'

2013-06-18 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes:

 Junio C Hamano wrote:
 + test $# == 1 ||

 This is broken under POSIX shells.  No need to resend (will locally
 patch up).

 Ugh, my C habit.  Thanks for catching it.

You're welcome---that is what the reviews are for.
--
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] Documentation/git-push.txt: explain better cases where --force is dangerous

2013-06-18 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes:

 Junio C Hamano gits...@pobox.com writes:


 After I re-read the one, I found that override somewhat a strange
 expression.  There is nothing that overrides or to be overriden.

 Right, I actually meant overwrite.

 How about putting it like this?

 I'm not sure push out refs other than the current branch is strong
 enough. Once you are used to push = fast-forward = can't loose data,
 push out a ref is not very scary.

 I'd do s/push out/overwrite/, but I'm fine with your version too.

OK, I agree that it makes sense to do that substitution.
Will queue.
--
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-VERSION-GEN: support non-standard $GIT_DIR path

2013-06-18 Thread Junio C Hamano
Dennis Kaarsemaker den...@kaarsemaker.net writes:

 On ma, 2013-06-17 at 13:09 -0700, Junio C Hamano wrote:
 Junio C Hamano gits...@pobox.com writes:
 
  Dennis Kaarsemaker den...@kaarsemaker.net writes:
 
   I'm doing daily builds of git, using many workers and a shared git.git,
   with per-worker checkouts
 
  OK, so GIT_DIR is explicitly specified in these workers.

 Yes, both GIT_DIR and GIT_WORK_TREE are set and the use of .git/HEAD and
 anything relying on it is shunned, so workers can run checkout as they
 please.

  Makes sense.

 Actually it does not.  What if GIT_DIR is an empty string or not set
 at all?  The patch breaks the build for everybody else, doesn't it?

 It does indeed, I only tested in my setup and not with a normal make
 test. Apologies.

 Perhaps like this instead?

  GIT-VERSION-GEN | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN
 index 2908204..91ec831 100755
 --- a/GIT-VERSION-GEN
 +++ b/GIT-VERSION-GEN
 @@ -11,7 +11,7 @@ LF='
  if test -f version
  then
  VN=$(cat version) || VN=$DEF_VER
 -elif test -d .git -o -f .git 
 +elif test -d ${GIT_DIR:-.git} -o -f .git 
  VN=$(git describe --match v[0-9]* --abbrev=7 HEAD 2/dev/null) 
  case $VN in
  *$LF*) (exit 1) ;;

 Yes, that makes a lot more sense and actually works in normal make test
 and with a detached .git. Do you want me to send an updated patch?

I think I've locally amended it already before queueing it to 'pu',
so no need to resend.

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: [Request] Git reset should be able to ignore file permissions

2013-06-18 Thread Alexander Nestorov
Sorry for not keeping everyone Cced, I wasn't aware of the rules.

Yes, writing about that in the docs seems more reasonable than patching reset,
as as you said, that'd just run update-index before the reset.
Let me get at home and I'll try to push a change :)

Regards
--
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 v3 1/2] am: handle stray $dotest directory

2013-06-18 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
 Where did the error occur?

 I think you can compare what is in 'pu' and what you sent out
 without asking.

Ah, I sent out the series before I taught Emacs to indent properly.

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] Documentation/git-merge.txt: weaken warning about uncommited changes

2013-06-18 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes:

 Ramkumar Ramachandra artag...@gmail.com writes:

 Matthieu Moy wrote:
 Weaken the warning by discouraging only merge with /non-trivial/
 uncommited changes.

 I don't think documenting all the failure cases in the doc would be a
 good idea, so I've left the in some cases part.

 It's already documented in the pre-merge checks section, as Jonathan
 pointed out [1]. 

 I had missed this one. But that's not the only case, you may also have
 problems with renames. The complete list would be really long to have
 here, and won't bring much to the user.

True.

After having re-read the thread up to the message from Jonathan, I
suspect that a merge half of pull --autosquash series (which had
to be dropped) was based on a misunderstanding and we didn't have to
have that discussion if the documentation were a little less
discouraging about merging in a dirty working tree?

 We should update the documentation to point to it: I do not think
 non-trivial is much of an improvement.

 Actually, I think it essentially says it all. If your changes are
 important enough to deserve a real backup, you should stash or commit.
 If you're ready to take the risk of losing it (the risk is small, but
 does exist), it's OK to run git merge blindly.

Your documentation update makes sure that we are less discouraging,
I think.  It does not have to be the only phrasing (hinting others
to try to come up with a beeter version if they are so inclined),
but it is going in the right direction.
--
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] send-email: allow use of basic email list in --cc --to and --bcc

2013-06-18 Thread Junio C Hamano
jorge-juan.garcia-gar...@ensimag.imag.fr writes:

 From: Jorge Juan Garcia Garcia jorge-juan.garcia-gar...@ensimag.imag.fr

 Make it so that we can use a list of email in flags
 instead of having to use one flag per email address.

 The format of email list handled is pretty basic for now:
   $ git send-email --to='Foo f...@example.com, b...@example.com'
 We thought it would be nice to have a first-step version which works
 before handling more complex ones such as:
   $ git send-email --to='Foo, Bar foo...@example.com'

Doesn't

git send-email --to='Foo f...@example.com' --to='b...@example.com'

work?  If it does, I do not see much point of this change.  If you
are starting from two pieces of information, why combine it into
one, only have the program to split it again, risking to be bitten
by bugs, and changing the code to do so, risking to add new bugs?


--
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 6/8] sh-setup: make die_with_status clear GIT_REFLOG_ACTION

2013-06-18 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes:

 Several callers set GIT_REFLOG_ACTION via set_reflog_action(), but
 nobody unsets it, leaving a potentially stray variable in the
 environment.  Fix this by making die_with_status() unset it.

I am totally lost.

 - You can set your own environment variables, and they will affect
   your child processes' environment, but only _before_ you spawn
   them.  After you spawn them, changes to your environment will not
   affect your child processes' environment.

 - There is no mechanism for you to affect environment of your
   parent process.

 - And we are exiting the process by calling die_with_status.

Whose environment are you trying to affect (or not affect) with this
change?  This will not affect your children, this will not affect
your parent, and this will not help you, either.

Puzzled.


 Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
 ---
  git-sh-setup.sh | 1 +
  1 file changed, 1 insertion(+)

 diff --git a/git-sh-setup.sh b/git-sh-setup.sh
 index 2f78359..3297103 100644
 --- a/git-sh-setup.sh
 +++ b/git-sh-setup.sh
 @@ -51,6 +51,7 @@ die () {
  }
  
  die_with_status () {
 + export GIT_REFLOG_ACTION=
   status=$1
   shift
   echo 2 $*
--
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] send-email: allow use of basic email list in --cc --to and --bcc

2013-06-18 Thread Matthieu Moy
Junio C Hamano gits...@pobox.com writes:

 Doesn't

   git send-email --to='Foo f...@example.com' --to='b...@example.com'

 work?  If it does, I do not see much point of this change.  If you
 are starting from two pieces of information, why combine it into
 one, only have the program to split it again, risking to be bitten
 by bugs, and changing the code to do so, risking to add new bugs?

The obvious use-case is to copy-paste a list of addresses from an email.
Currently, the Cc: list of the email I'm sending looks like

Cc: jorge-juan.garcia-gar...@ensimag.imag.fr,  git@vger.kernel.org,  Mathieu 
Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr

If I were to use git send-email on it, I'd have to cut the list
myself.

This could be mentionned in the commit message.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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] Documentation: Move git diff blob blob

2013-06-18 Thread Kevin Bracey
The section describing git diff blob blob had been placed in a
position that disrupted the statement This is synonymous to the
previous form.

Reorder to place this form after all the commit-using forms, and the
note applying to them. Also mention this form in the initial description
paragraph.

Signed-off-by: Kevin Bracey ke...@bracey.fi
---
 Documentation/git-diff.txt | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
index a7b4620..78d6d50 100644
--- a/Documentation/git-diff.txt
+++ b/Documentation/git-diff.txt
@@ -18,8 +18,8 @@ SYNOPSIS
 DESCRIPTION
 ---
 Show changes between the working tree and the index or a tree, changes
-between the index and a tree, changes between two trees, or changes
-between two files on disk.
+between the index and a tree, changes between two trees, changes between
+two blob objects, or changes between two files on disk.
 
 'git diff' [--options] [--] [path...]::
 
@@ -56,11 +56,6 @@ directories. This behavior can be forced by --no-index.
This is to view the changes between two arbitrary
commit.
 
-'git diff' [options] blob blob::
-
-   This form is to view the differences between the raw
-   contents of two blob objects.
-
 'git diff' [--options] commit..commit [--] [path...]::
 
This is synonymous to the previous form.  If commit on
@@ -87,6 +82,11 @@ and the range notations (commit..commit and
 commit\...commit) do not mean a range as defined in the
 SPECIFYING RANGES section in linkgit:gitrevisions[7].
 
+'git diff' [options] blob blob::
+
+   This form is to view the differences between the raw
+   contents of two blob objects.
+
 OPTIONS
 ---
 :git-diff: 1
-- 
1.8.3.rc0.28.g4b02ef5

--
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: Exact format of tree objets

2013-06-18 Thread Chico Sokol
What is the encoding of the filename?


--
Chico Sokol


On Tue, Jun 11, 2013 at 3:26 PM, Ilari Liusvaara
ilari.liusva...@elisanet.fi wrote:
 On Tue, Jun 11, 2013 at 01:25:14PM -0300, Chico Sokol wrote:
 Is there any official documentation of tree objets format? Are tree
 objects encoded specially in some way? How can I parse the inflated
 contents of a tree object?

 Tree object consists of entries, each concatenation of:
 - Octal mode (using ASCII digits 0-7).
 - Single SPACE (0x20)
 - Filename
 - Single NUL (0x00)
 - 20-byte binary SHA-1 of referenced object.

 At least following octal modes are known:
 4: Directory (tree).
 100644: Regular file (blob).
 100755: Executable file (blob).
 12: Symbolic link (blob).
 16: Submodule (commit).

 The entries are always sorted in (bytewise) lexicographical order,
 except directories sort like there was impiled '/' at the end.

 So e.g.:
 !  0  9  a  a-  a- (directory)  a (directory)  a0  ab  b  z.


 The idea of sorting directories specially is that if one recurses
 upon hitting a directory and uses '/' as path separator, then the
 full filenames are in bytewise lexicographical order.

 -Ilari
--
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] http.c: don't rewrite the user:passwd string multiple times

2013-06-18 Thread Junio C Hamano
Daniel Stenberg dan...@haxx.se writes:

 On Tue, 18 Jun 2013, Jeff King wrote:

 TL;DR: I'm just confirming what's said here! =)

Thanks.  We are very fortunate to have you as the cURL guru who
gives prompt responses and sanity checks to us.
--
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 6/8] sh-setup: make die_with_status clear GIT_REFLOG_ACTION

2013-06-18 Thread Ramkumar Ramachandra
Junio C Hamano wrote:
  - There is no mechanism for you to affect environment of your
parent process.

Please excuse my stupidity and drop this patch.  I got mislead by your
SQUASH??? patch which took care to set the environment variable and
call checkout in a subshell.
--
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: [Request] Git reset should be able to ignore file permissions

2013-06-18 Thread Alexander Nestorov
I'm home, 
https://github.com/alexandernst/git/commit/61f0a7d558e3cbae308fabdff66bd87569d6aa18
Is that good? Should I PR?
--
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 mz/rebase-tests] rebase topology tests: fix commit names on case-insensitive file systems

2013-06-18 Thread Martin von Zweigbergk
On Tue, Jun 18, 2013 at 12:28 AM, Johannes Sixt j.s...@viscovery.net wrote:

 The recently introduced tests used uppercase letters to denote
 cherry-picks of commits having the corresponding lowercase letter names.
 The helper functions also set up tags with the names of the commits.

 But this constellation fails on case-insensitive file systems because
 there cannot be distinct tags with names that differ only in case.

 Use a less subtle convention for the names of cherry-picked commits.

Makes sense, and the patch looks good. Thanks.

  Knowing that the tests would take their time to complete on Windows,

I'm curious just how slow it is. Are we talking minutes?
--
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: [Request] Git reset should be able to ignore file permissions

2013-06-18 Thread Matthieu Moy
Alexander Nestorov alexander...@gmail.com writes:

 I'm home, 
 https://github.com/alexandernst/git/commit/61f0a7d558e3cbae308fabdff66bd87569d6aa18
 Is that good?

Please, post your patches inline, it eases review. More generally, read
Documentation/SubmittingPatches.

+Ignore file permissions::

It's not only about permissions, and it does not ignore them, it just
notices when there's actually no change although the mtime has changed.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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/*

2013-06-18 Thread 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 

Re: [PATCH v2 0/6] Fix checkout-dash to work with rebase

2013-06-18 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes:

 Junio C Hamano wrote:
 diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
 index a58406d..c175ef1 100755
 --- a/t/t3404-rebase-interactive.sh
 +++ b/t/t3404-rebase-interactive.sh
 @@ -934,6 +934,21 @@ test_expect_success 'rebase --edit-todo can be used to 
 modify todo' '
 test L = $(git cat-file commit HEAD | sed -ne \$p)
  '

 +test_expect_success 'rebase -i produces readable reflog' '

 I don't know if this test is a good idea at all.

It is.

This catches a change to GIT_REFLOG_ACTION that is meant to only
apply to checkout done internally in git-rebase leaks to the later
codepath and affects the reflog message left by rebase--interactive.

Apply the test change without the do not leak part in the fix-up
(queued as a single SQUASH??? commit on 'pu') to what you posted
earlier and see how it breaks.

# rr/rebase-checkout-reflog with fix-up
$ git checkout 33b1cdeb 
# revert the fix but keep the test
$ git checkout HEAD^ -- git-rebase--interactive.sh git-rebase.sh
# run the test
$ make  cd t  sh ./t3404-*.sh -v -i

The test fails with this:

--- expect  2013-06-18 16:09:21.0 +
+++ actual  2013-06-18 16:09:21.0 +
@@ -1,4 +1,4 @@
-rebase -i (start): checkout I
+rebase -i (start): checkout branch-reflog-test: checkout I
 rebase -i (pick): G
 rebase -i (pick): H
 rebase -i (finish): returning to refs/heads/branch-reflog-test

checkout branch-reflog-test part is leaking from your setting of
GIT_REFLOG_ACTION for git checkout in git rebase; it is only
necessary to affect that checkout and it should not affect later
commands that write reflog entries, but we see the breakage.

And that is why I did the fix-up to make sure your changes only
apply to git checkout.

If you apply the test part of that fixup to what you posted today,
what would you see?  I didn't look at the patches very closely, but
I wouldn't be surprised if they are still leaking the change meant
only to checkout to the later stages and breaking that test (the
reason why I would not be surprised is because the impression I am
getting from reading your responses is that you do not understand
why it is bad not to limit the setting only to checkout).

 Why make an exception in the case of rebase -i?

Because the whole point of last two patches are word reflog
messages better for rebase, the improvements made by these two
patches are better protected from future changes; it also makes sure
that deliberate improvements will show up as changes to the tests.
--
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] t/t9802: explicitly name the upstream branch to use as a base

2013-06-18 Thread Junio C Hamano
Pete Wyckoff p...@padd.com writes:

 Thanks for finding and fixing this.  Great explanation.  I
 tested it locally too.

 Acked-by: Pete Wyckoff p...@padd.com

Thanks, both.  Queued.

--
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


  1   2   >