Re: [PUB]corrupt repos does not return error with `git fsck`

2015-05-21 Thread Matthieu Moy
Faheem Mitha fah...@faheem.info writes:

 I was going by the answer (by CodeWizard) in
 http://stackoverflow.com/q/30348615/350713

OK, so the hash you got comes from a superproject which references it.
My guess is that the superproject did a private commit in a submodule,
added this submodule to the superproject, and forgot to push the
submodule.

If so, it's a user error (that could arguably have been avoided with a
better command-line interface, so Git is partly guilty), but not a
repository corruption.

 If I just give a random hash to `git show` in that repos, I get

 fatal: ambiguous argument '...': unknown revision or path not in the 
 working tree.

Not a random hash, but a random abreviated hash. Look:

Changing the last digit:

$ git show 280c12ab49223c64c6f914944287a7d049cf4d23
fatal: bad object 280c12ab49223c64c6f914944287a7d049cf4d23
$ git show 280c12ab49223c64c6f914944287a7d049cf4d24
fatal: bad object 280c12ab49223c64c6f914944287a7d049cf4d24
$ git show 280c12ab49223c64c6f914944287a7d049cf4d25
fatal: bad object 280c12ab49223c64c6f914944287a7d049cf4d25
$ git show 280c12ab49223c64c6f914944287a7d049cf4d26
fatal: bad object 280c12ab49223c64c6f914944287a7d049cf4d26

Removing the last digit:

$ git show 280c12ab49223c64c6f914944287a7d049cf4d2 
fatal: ambiguous argument '280c12ab49223c64c6f914944287a7d049cf4d2': unknown 
revision or path not in the working tree.
Use '--' to separate paths from revisions, like this:
'git command [revision...] -- [file...]'

-- 
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] pull: use git-rev-parse --parseopt for option parsing

2015-05-21 Thread Paul Tan
Hi,

On Mon, May 18, 2015 at 10:43 PM, Johannes Schindelin
johannes.schinde...@gmx.de wrote:
 On 2015-05-18 15:54, Paul Tan wrote:

 diff --git a/git-pull.sh b/git-pull.sh
 index 633c385..67f825c 100755
 --- a/git-pull.sh
 +++ b/git-pull.sh
 @@ -4,13 +4,53 @@
  #
  # Fetch one or more remote refs and merge it/them into the current HEAD.

 -USAGE='[-n | --no-stat] [--[no-]commit] [--[no-]squash]
 [--[no-]ff|--ff-only] [--[no-]rebase|--rebase=preserve] [-s
 strategy]... [fetch-options] repo head...'
 -LONG_USAGE='Fetch one or more remote refs and integrate it/them with
 the current HEAD.'
  SUBDIRECTORY_OK=Yes
 -OPTIONS_SPEC=
 +OPTIONS_KEEPDASHDASH=Yes

 I have to admit that I was puzzled by this at first. But it seems that the 
 intention is to handle a dashdash argument as an error, therefore we have to 
 keep it. Is my understanding correct?

Ugh, I thought we had to keep the dashdash so that we could
disambiguate the arguments from the options, but it turns out that
rev-parse would add its own dashdash. So, no, we don't need to keep
the dashdash.

Thanks for catching this. Fixed.

Regards,
Paul
--
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 v4 8/8] t5520: check reflog action in fast-forward merge

2015-05-21 Thread Paul Tan
Hi,

On Mon, May 18, 2015 at 11:20 PM, Johannes Schindelin
johannes.schinde...@gmx.de wrote:
 Hi Paul,

 On 2015-05-18 15:32, Paul Tan wrote:

 @@ -95,7 +94,11 @@ test_expect_success 'test . as a remote' '
   git checkout copy 
   test $(cat file) = file 
   git pull 
 - test $(cat file) = updated
 + test $(cat file) = updated 
 + git reflog -1 reflog.actual 
 + sed s/$_x05[0-9a-f]*/OBJID/g reflog.actual reflog.fuzzy 

 Actually, let's use s/^[0-9a-f]*/OBJID/ instead: you only want to replace 
 the first few characters.

Did you mean s/^$_x05[0-9a-f]*/OBJID/? (with $_x05 expanding to
'[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]' from test-lib.sh). If not,
then it would match even if there was no SHA1 hash.

But yes, without the ^ there will very likely be false positives.
Thanks for catching.

 @@ -106,7 +109,11 @@ test_expect_success 'the default remote . should
 not break explicit pull' '
   git reset --hard HEAD^ 
   test $(cat file) = file 
   git pull . second 
 - test $(cat file) = modified
 + test $(cat file) = modified 
 + git reflog -1 reflog.actual 
 + sed s/$_x05[0-9a-f]*/OBJID/g reflog.actual reflog.fuzzy 

 Same here.

Regards,
Paul
--
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/4] ref-filter: add ref-filter API

2015-05-21 Thread Matthieu Moy
Karthik Nayak karthik@gmail.com writes:

 +static int match_name_as_path(const char **pattern, const char *refname)

I would have appreciated a short docstring. The full doc would probably
be as long as the code, but a few examples of what matches and what
doesn't can help the reader.

 +static struct ref_filter_item *new_ref_filter_item(const char *refname,
 +const unsigned char *sha1,
 +int flag)
 +{
 + struct ref_filter_item *ref =  xcalloc(1, sizeof(struct 
 ref_filter_item));

double-space after =.

 +++ b/ref-filter.h
 @@ -0,0 +1,47 @@
 +#ifndef REF_FILTER_H
 +#define REF_FILTER_H
 +
 +#include sha1-array.h
 +#include refs.h
 +#include commit.h
 +
 +/*
 + * ref-filter is meant to act as a common provider of API's for
 + * 'tag -l', 'branch -l' and 'for-each-ref'. ref-filter is the attempt

Don't be shy: attempt at unification - unification. This message may be
an attempt, but we'll polish it until it is more than that.

 + * at unification of these three commands so that they ay benefit from

they *may*?

 + * the functionality of each other.
 + */

I miss a high-level description of what the code is doing. Essentially,
there's the complete repository list of refs, and you want to filter
only some of them, right?

From the name, I would guess that ref_filter is the structure describing
how you are filtering, but from the code it seems to be the list you're
filtering, not the filter.

 +/* An atom is a valid field atom used for sorting and formatting of refs.*/

used for is very vague. Be more precise, say how it will be involved
in sorting  formatting.

 +/*  ref_filter will hold data pertaining to a list of refs. */

This is the answer to the what? question, which is not very hard to
infer from the code. That's not anwsering what for? or why?, which
are much harder to infer for the reader.

(plus you have a double-space after /*)

-- 
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 v4 3/8] t5520: test for failure if index has unresolved entries

2015-05-21 Thread Paul Tan
Hi,

On Mon, May 18, 2015 at 11:13 PM, Johannes Schindelin
johannes.schinde...@gmx.de wrote:
 Hi Paul,

 On 2015-05-18 15:32, Paul Tan wrote:

 diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
 index 4a2c0a1..3bc0594 100755
 --- a/t/t5520-pull.sh
 +++ b/t/t5520-pull.sh
 @@ -164,6 +164,26 @@ test_expect_success 'fail if upstream branch does
 not exist' '
   test $(cat file) = file
  '

 +test_expect_success 'fail if the index has unresolved entries' '
 + git checkout -b third second^ 
 + test_when_finished git checkout -f copy  git branch -D third 

 If you agree with my argument in 2/8, it would be nice to drop this line, too.

As mentioned by Junio in 2/8 the cleanup functions will not run under
--immediate mode.

Besides, the use of test_when_finished is to send a clear signal that
the third branch is temporary and is not meant to be used by other
test cases. Furthermore, subsequent tests assume that the current
branch is copy, so it's best to preserve that.

 + test $(cat file) = file 
 + echo modified2 file 
 + git commit -a -m modified2 

 These two lines could be combined into test_commit modified2 file.

Fixed, thanks.

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


Re: [PATCH 1/2] pull: handle git-fetch's options as well

2015-05-21 Thread Paul Tan
Hi,

On Mon, May 18, 2015 at 10:37 PM, Johannes Schindelin
johannes.schinde...@gmx.de wrote:
 On 2015-05-18 15:30, Paul Tan wrote:
 t5520: failing test for pull --all with no configured upstream
 t5521: test pull --all --dry-run does not make any changes

 error_on_no_merge_candidates() does not consider the case where $#
 includes command-line flags that are passed to git-fetch.

 As such, when the current branch has no configured upstream, and there
 are no merge candidates because of that, git-pull --all erroneously reports
 that we are pulling from --all, as it believes that the first argument
 is the remote name.

 Add a failing test that shows this case.

 Signed-off-by: Paul Tan pyoka...@gmail.com
 ---
  git-pull.sh | 34 --
  t/t5520-pull.sh | 20 
  t/t5521-pull-options.sh | 14 ++
  3 files changed, 66 insertions(+), 2 deletions(-)

 diff --git a/git-pull.sh b/git-pull.sh
 index 9ed01fd..28d49ab 100755
 --- a/git-pull.sh
 +++ b/git-pull.sh
 @@ -163,11 +163,39 @@ do
   --d|--dr|--dry|--dry-|--dry-r|--dry-ru|--dry-run)
   dry_run=--dry-run
   ;;
 + --all|--no-all)
 + all=$1 ;;

 I *think* you also want to add a corresponding all= line just below the 
 dry_run= line, to ensure that all=blablabla git pull does not interfere 
 with this command-line setting.

Fixed, thanks.

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


Re: [PATCH 1/2] pull: handle git-fetch's options as well

2015-05-21 Thread Paul Tan
Hi,

On Mon, May 18, 2015 at 10:37 PM, Johannes Schindelin
johannes.schinde...@gmx.de wrote:
 On 2015-05-18 15:30, Paul Tan wrote:
 t5520: failing test for pull --all with no configured upstream
 t5521: test pull --all --dry-run does not make any changes

 error_on_no_merge_candidates() does not consider the case where $#
 includes command-line flags that are passed to git-fetch.

 As such, when the current branch has no configured upstream, and there
 are no merge candidates because of that, git-pull --all erroneously reports
 that we are pulling from --all, as it believes that the first argument
 is the remote name.

 Add a failing test that shows this case.

 Signed-off-by: Paul Tan pyoka...@gmail.com
 ---
  git-pull.sh | 34 --
  t/t5520-pull.sh | 20 
  t/t5521-pull-options.sh | 14 ++
  3 files changed, 66 insertions(+), 2 deletions(-)

 diff --git a/git-pull.sh b/git-pull.sh
 index 9ed01fd..28d49ab 100755
 --- a/git-pull.sh
 +++ b/git-pull.sh
 @@ -163,11 +163,39 @@ do
   --d|--dr|--dry|--dry-|--dry-r|--dry-ru|--dry-run)
   dry_run=--dry-run
   ;;
 + --all|--no-all)
 + all=$1 ;;

 I *think* you also want to add a corresponding all= line just below the 
 dry_run= line, to ensure that all=blablabla git pull does not interfere 
 with this command-line setting.

Fixed, thanks.

Regards,
Paul
--
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/4] for-each-ref: rename refinfo members to match similar structures

2015-05-21 Thread karthik nayak



On 05/20/2015 10:27 PM, Matthieu Moy wrote:

Karthik Nayak karthik@gmail.com writes:


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


This means that git am will consider Peff as the author ...


Written-by: Jeff King p...@peff.net


... hence this is not needed: in the final history, it will appear as if
Peff wrote this Written-by: himself, which would be weird.

If it is the case, you should add in the commit message that there's no
actual changs, and perhaps which renames were done. This makes the
review straightforward.



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


Re: identical hashes on two branches, but holes in git log

2015-05-21 Thread Philippe De Muyter
On Tue, May 19, 2015 at 03:12:31PM -0700, Junio C Hamano wrote:
 Philippe De Muyter p...@macq.eu writes:
 
  On Tue, May 19, 2015 at 09:01:10AM -0700, Junio C Hamano wrote:
  Philippe De Muyter p...@macq.eu writes:
  
   Trying to understand, I have eventually done git log on my branch and
   on v3.15 with the following commands :
  
   git log v3.15 --full-history --decorate=short | grep '^commit'  
   /tmp/3.15.commits
   git log --full-history --decorate=short | grep '^commit'  
   /tmp/mybranch.commits
  
  Either
  
  git log --oneline v3.15..HEAD ;# show what I have not in theirs
  
  or
  
  gitk v3.15...HEAD ;# show our differences graphically
 
  This shows the commits in my branch starting from the most recent common 
  point,
  thus my commits, but I see differences in the files not explained by my 
  commits,
  but by the fact that many older commits (between v3.13 and v3.14) are 
  missing on
  my branch, but still in both branches I have a commit called v3.14 with the
  same hash.  Is that normal ?
 
 Sorry, cannot parse.  Neither of the above would show files, so just
 about the place where you start talking about I see differences in
 the files, you lost me.

Look at the other part of the thread, with the discussion with Jeff and John

The light has come, and what I understand is:

 don't trust the default (ordering) mode of 'git log' :(

I surmise this happens only when 'git merge' has been used.

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


Occasional wrong behavior of rev-walking (rev-list, log, etc.)

2015-05-21 Thread Mike Hommey
Hi,

I noticed that in some weird cases, git rev-list and friends would
appear to not do their work as intended. As I wasn't entirely sure at
the time I saw previous occurrence (which involved lots of refs and
big history) , I kept that on the side to look back later, but today,
it happened again on a somewhat smaller testcase, albeit, on a big
repository.

I was able to reproduce it with the following:

$ git clone https://github.com/mozilla/gecko-dev
$ cd gecko-dev
$ git checkout -b a 4d3f25020072867e19ad6d840a73c8d77ea040bd
$ git commit --allow-empty -m a
$ git checkout -b b e90de3e5e045428e8f2b732882736dc93ce05f14
$ git commit --allow-empty -m b
$ git merge a -m merge

Now here's the problem:

$ git rev-list e90de3e5e045428e8f2b732882736dc93ce05f14..b | wc -l
86

But:
$ git rev-list e90de3e5e045428e8f2b732882736dc93ce05f14 | grep 
4d3f25020072867e19ad6d840a73c8d77ea040bd
4d3f25020072867e19ad6d840a73c8d77ea040bd

So 4d3f25020072867e19ad6d840a73c8d77ea040bd is in
e90de3e5e045428e8f2b732882736dc93ce05f14's history, which means the only
thing that's on top of e90de3e5e045428e8f2b732882736dc93ce05f14 in b is
the 3 commits created above.

My guess is that rev-walking is tripping on the fact that this repository
has commit dates in random order.

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


Re: [PATCH 3/4] for-each-ref: convert to ref-filter

2015-05-21 Thread karthik nayak



On 05/21/2015 05:20 AM, Junio C Hamano wrote:

Karthik Nayak karthik@gmail.com writes:


convert 'for-each-ref' to use the common API provided by 'ref-filter'.


Start a sentence with capital?

More importantly, the above is misleading, as if you invented a new
ref-filter API and made for-each-ref build on that new
infrastructure.

This series is in a form that is very unfriendly to reviewers.  The
previous step did not introduce any callers to ref-filter, so for
the purpose of review, it needs to be read together with this step
anyway.

And when reading these patches that way, what this half is really
doing is to move the code from for-each-ref to ref-filter, but it
does unnecessary or unrelated renaming of a handful of symbols.  It
makes it even harder to compare and contrast the original code that
was in the original for-each-ref and moved code that ends up in the
new ref-filter.  Don't do that.

You would probably want to organize them in these two steps instead:

  * Rename symbols as necessary while all the code is still in
for-each-ref. Do not create ref-filter in this step. Justify it
along the lines of some symbol names were fine while they were
file scope static implementation detail of for-each-ref, but we
will make the machinery available from other commands by moving
it to a library-ish place, so rename X to foo_X to clarify that
this is about foo (which is now necessary as it is not specific
to for-each-ref...

  * If you want to do other tweaks like wrapping refs  num_refs into
a single structure, do so while the code is still in
for-each-ref.  You can do that in the same patch as the above
(i.e. it's just part of preparatory step for a move).

  * Create ref-filter by _moving_ code from for-each-ref. Do not
touch these moved lines in this step. You would need to add
include at the top of for-each-ref and ref-filter, of course.

Thanks for the suggestion's Junio, will follow with the path you've 
mentioned.



-   for_each_rawref(grab_single_ref, cbdata);
-   refs = cbdata.grab_array;
-   num_refs = cbdata.grab_cnt;
+   refs.name_patterns = argv;
+   for_each_rawref(ref_filter_add, refs);


I think ref_filter_add() may be misnamed as a public API function.
grab_single_ref() was OK only because it was an implementation
dtail, but if you are making it public, the name should make it
clear that it is meant to be used as a for_each_*ref() callback
function.  Otherwise people may be tempted to add random parameter
to it in the future, but the signature of that function is dictated
by for_each_*ref() API.



Looking through the current each_ref_f() names, I thought I could use
'ref_filter_handler()' instead of 'ref_filter_add()', as per your 
suggestion. What do you think?

--
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: identical hashes on two branches, but holes in git log

2015-05-21 Thread Philippe De Muyter
On Wed, May 20, 2015 at 12:18:15PM -0400, Jeff King wrote:
 On Wed, May 20, 2015 at 04:12:38PM +0200, Philippe De Muyter wrote:
 
  After reading the man page of 'git log', should --topo-order not be the
  default log order ?
 
 The problem with --topo-order is that it has to traverse all of the
 commits before starting output. So:
 
   $ time git log | head -1
   commit 64fb1d0e975e92e012802d371e417266d6531676
 
   real0m0.038s
   user0m0.032s
   sys 0m0.008s
 
   $ time git log --topo-order | head -1
   commit 64fb1d0e975e92e012802d371e417266d6531676
 
   real0m4.247s
   user0m4.140s
   sys 0m0.108s
 
 -Peff

So we trade correctness for speed :(

Is there a way to set topo-order as the default log order via git config ?

Is topo-order already implemented as starting with the default order followed
by an ancestor check or does it switch immediately to topological sort ?

Philippe
--
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] doc: fix inconsistent spelling of packfile

2015-05-21 Thread Patrick Steinhardt
Fix remaining instances where pack-file is used instead of
packfile.

Signed-off-by: Patrick Steinhardt p...@pks.im
---
This patch now also fixes instances where we refer to EBNF-style
command line parameters, as discussed by Junio and Peff.

 Documentation/git-index-pack.txt  | 10 +-
 Documentation/git-unpack-objects.txt  |  2 +-
 Documentation/technical/pack-protocol.txt |  4 ++--
 Documentation/user-manual.txt |  2 +-
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-index-pack.txt b/Documentation/git-index-pack.txt
index 7a4e055..49621da 100644
--- a/Documentation/git-index-pack.txt
+++ b/Documentation/git-index-pack.txt
@@ -9,9 +9,9 @@ git-index-pack - Build pack index file for an existing packed 
archive
 SYNOPSIS
 
 [verse]
-'git index-pack' [-v] [-o index-file] pack-file
+'git index-pack' [-v] [-o index-file] packfile
 'git index-pack' --stdin [--fix-thin] [--keep] [-v] [-o index-file]
- [pack-file]
+ [packfile]
 
 
 DESCRIPTION
@@ -37,11 +37,11 @@ OPTIONS
 
 --stdin::
When this flag is provided, the pack is read from stdin
-   instead and a copy is then written to pack-file. If
-   pack-file is not specified, the pack is written to
+   instead and a copy is then written to packfile. If
+   packfile is not specified, the pack is written to
objects/pack/ directory of the current Git repository with
a default name determined from the pack content.  If
-   pack-file is not specified consider using --keep to
+   packfile is not specified consider using --keep to
prevent a race condition between this process and
'git repack'.
 
diff --git a/Documentation/git-unpack-objects.txt 
b/Documentation/git-unpack-objects.txt
index 894d20b..07d4329 100644
--- a/Documentation/git-unpack-objects.txt
+++ b/Documentation/git-unpack-objects.txt
@@ -9,7 +9,7 @@ git-unpack-objects - Unpack objects from a packed archive
 SYNOPSIS
 
 [verse]
-'git unpack-objects' [-n] [-q] [-r] [--strict]  pack-file
+'git unpack-objects' [-n] [-q] [-r] [--strict]  packfile
 
 
 DESCRIPTION
diff --git a/Documentation/technical/pack-protocol.txt 
b/Documentation/technical/pack-protocol.txt
index 812d857..fc09c63 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -465,7 +465,7 @@ contain all the objects that the server will need to 
complete the new
 references.
 
 
-  update-request=  *shallow ( command-list | push-cert ) [pack-file]
+  update-request=  *shallow ( command-list | push-cert ) [packfile]
 
   shallow   =  PKT-LINE(shallow SP obj-id LF)
 
@@ -491,7 +491,7 @@ references.
  *PKT-LINE(gpg-signature-lines LF)
  PKT-LINE(push-cert-end LF)
 
-  pack-file = PACK 28*(OCTET)
+  packfile  = PACK 28*(OCTET)
 
 
 If the receiving end does not support delete-refs, the sending end MUST
diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index 68978f5..7147519 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -3164,7 +3164,7 @@ objects.  (Note that linkgit:git-tag[1] can also be used 
to create
 lightweight tags, which are not tag objects at all, but just simple
 references whose names begin with `refs/tags/`).
 
-[[pack-files]]
+[[packfiles]]
 How Git stores objects efficiently: pack files
 ~~
 
-- 
2.4.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 v4 6/8] t5520: test --rebase failure on unborn branch with index

2015-05-21 Thread Paul Tan
Hi,

On Tue, May 19, 2015 at 2:00 AM, Stefan Beller sbel...@google.com wrote:
 On Mon, May 18, 2015 at 6:32 AM, Paul Tan pyoka...@gmail.com wrote:
 diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
 index f991439..4d649a5 100755
 --- a/t/t5520-pull.sh
 +++ b/t/t5520-pull.sh
 @@ -413,6 +413,21 @@ test_expect_success 'pull --rebase works on branch yet 
 to be born' '
 test_cmp expect actual
  '

 +test_expect_success 'pull --rebase fails on unborn branch with staged 
 changes' '
 +   test_when_finished rm -rf empty_repo2 
 +   git init empty_repo2 
 +   (
 +   cd empty_repo2 
 +   echo staged-file staged-file 
 +   git add staged-file 
 +   test $(git ls-files) = staged-file 
 +   test_must_fail git pull --rebase .. master 2../err 
 +   test $(git ls-files) = staged-file 
 +   test $(git show :staged-file) = staged-file
 +   ) 
 +   test_i18ngrep unborn branch with changes added to the index err

 So when seeing this line outside the parenthesis section, I immediately 
 thought
 there must be a reason you put it outside. The reason is not obvious
 to me though.
 So I'd suggest to move the test_i18ngrep inside the section above.

Right. Fixed.

Regards,
Paul
--
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 v11 0/5] group common commands by theme

2015-05-21 Thread Sébastien Guimmara
Just a minor change, the modification of new-command.txt was squashed to 
2/5 instead of 1/5.

Eric Sunshine (2):
  command-list: prepare machinery for upcoming common groups section
  generate-cmdlist: parse common group commands

Sébastien Guimmara (3):
  command-list.txt: add the common groups block
  command-list.txt: drop the common tag
  help: respect new common command grouping

 Documentation/cmd-list.perl |  4 +++
 Documentation/howto/new-command.txt |  4 ++-
 Makefile|  9 ---
 command-list.txt| 53 ++---
 generate-cmdlist.perl   | 50 ++
 generate-cmdlist.sh | 23 
 help.c  | 24 -
 7 files changed, 117 insertions(+), 50 deletions(-)
 create mode 100755 generate-cmdlist.perl
 delete mode 100755 generate-cmdlist.sh

-- 
2.4.0.GIT

--
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/4] ref-filter: add ref-filter API

2015-05-21 Thread karthik nayak



I miss a high-level description of what the code is doing. Essentially,
there's the complete repository list of refs, and you want to filter
only some of them, right?

 From the name, I would guess that ref_filter is the structure describing
how you are filtering, but from the code it seems to be the list you're
filtering, not the filter.


Reading this again, A bit confused by what you're trying to imply. Could 
you rephrase please?

--
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 v11 2/5] command-list.txt: add the common groups block

2015-05-21 Thread Sébastien Guimmara
The ultimate goal is for git help to display common commands in
groups rather than alphabetically. As a first step, define the
groups in a new block, and then assign a group to each
common command.

Add a block at the beginning of command-list.txt:

init start a working area (see also: git help tutorial)
worktree work on the current change (see also:[...]
info examine the history and state (see also: git [...]
history  grow, mark and tweak your history
remote   collaborate (see also: git help workflows)

storing information about common commands group, then map each common
command to a group:

git-add  mainporcelaincommon worktree

Helped-by: Eric Sunshine sunsh...@sunshineco.com
Helped-by: Junio C Hamano gits...@pobox.com
Helped-by:  Emma Jane Hogbin Westby emma.wes...@gmail.com
Signed-off-by: Sébastien Guimmara sebastien.guimm...@gmail.com
---
 Documentation/howto/new-command.txt |  4 ++-
 command-list.txt| 51 ++---
 2 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/Documentation/howto/new-command.txt 
b/Documentation/howto/new-command.txt
index d7de5a3..6d772bd 100644
--- a/Documentation/howto/new-command.txt
+++ b/Documentation/howto/new-command.txt
@@ -95,7 +95,9 @@ your language, document it in the INSTALL file.
 that categorizes commands by type, so they can be listed in appropriate
 subsections in the documentation's summary command list.  Add an entry
 for yours.  To understand the categories, look at git-commands.txt
-in the main directory.
+in the main directory.  If the new command is part of the typical Git
+workflow and you believe it common enough to be mentioned in 'git help',
+map this command to a common group in the column [common].
 
 7. Give the maintainer one paragraph to include in the RelNotes file
 to describe the new feature; a good place to do so is in the cover
diff --git a/command-list.txt b/command-list.txt
index 181a9c2..32ddab3 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -1,3 +1,14 @@
+# common commands are grouped by themes
+# these groups are output by 'git help' in the order declared here.
+# map each common command in the command list to one of these groups.
+### common groups (do not change this line)
+init start a working area (see also: git help tutorial)
+worktree work on the current change (see also: git help everyday)
+info examine the history and state (see also: git help revisions)
+history  grow, mark and tweak your common history
+remote   collaborate (see also: git help workflows)
+
+# List of known git commands.
 ### command list (do not change this line)
 # command name  category [deprecated] [common]
 git-add mainporcelain common
@@ -6,24 +17,24 @@ git-annotate
ancillaryinterrogators
 git-apply   plumbingmanipulators
 git-archimport  foreignscminterface
 git-archive mainporcelain
-git-bisect  mainporcelain common
+git-bisect  mainporcelain   common info
 git-blame   ancillaryinterrogators
-git-branch  mainporcelain common
+git-branch  mainporcelain   common history
 git-bundle  mainporcelain
 git-cat-fileplumbinginterrogators
 git-check-attr  purehelpers
 git-check-ignorepurehelpers
 git-check-mailmap   purehelpers
-git-checkoutmainporcelain common
+git-checkoutmainporcelain   common history
 git-checkout-index  plumbingmanipulators
 git-check-ref-formatpurehelpers
 git-cherry  ancillaryinterrogators
 git-cherry-pick mainporcelain
 git-citool  mainporcelain
 git-clean   mainporcelain
-git-clone   mainporcelain common
+git-clone   mainporcelain   common init
 git-column  purehelpers
-git-commit  mainporcelain common
+git-commit  mainporcelain   common history
 git-commit-tree plumbingmanipulators
 git-config  ancillarymanipulators
 git-count-objects   ancillaryinterrogators
@@ -35,14 +46,14 @@ git-cvsimport   foreignscminterface
 git-cvsserver   foreignscminterface
 git-daemon  synchingrepositories
 git-describe

[PATCH v11 4/5] command-list.txt: drop the common tag

2015-05-21 Thread Sébastien Guimmara
command-list.sh, retired in the previous patch, was the only
consumer of the common tag, so drop this now-unnecessary
attribute.

before:
git-add  mainporcelaincommon worktree

after:
git-add  mainporcelainworktree

Helped-by: Eric Sunshine sunsh...@sunshineco.com
Signed-off-by: Sébastien Guimmara sebastien.guimm...@gmail.com
---
 command-list.txt | 42 +-
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/command-list.txt b/command-list.txt
index 32ddab3..9a98752 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -11,30 +11,30 @@ remote   collaborate (see also: git help workflows)
 # List of known git commands.
 ### command list (do not change this line)
 # command name  category [deprecated] [common]
-git-add mainporcelain common
+git-add mainporcelain   worktree
 git-am  mainporcelain
 git-annotateancillaryinterrogators
 git-apply   plumbingmanipulators
 git-archimport  foreignscminterface
 git-archive mainporcelain
-git-bisect  mainporcelain   common info
+git-bisect  mainporcelain   info
 git-blame   ancillaryinterrogators
-git-branch  mainporcelain   common history
+git-branch  mainporcelain   history
 git-bundle  mainporcelain
 git-cat-fileplumbinginterrogators
 git-check-attr  purehelpers
 git-check-ignorepurehelpers
 git-check-mailmap   purehelpers
-git-checkoutmainporcelain   common history
+git-checkoutmainporcelain   history
 git-checkout-index  plumbingmanipulators
 git-check-ref-formatpurehelpers
 git-cherry  ancillaryinterrogators
 git-cherry-pick mainporcelain
 git-citool  mainporcelain
 git-clean   mainporcelain
-git-clone   mainporcelain   common init
+git-clone   mainporcelain   init
 git-column  purehelpers
-git-commit  mainporcelain   common history
+git-commit  mainporcelain   history
 git-commit-tree plumbingmanipulators
 git-config  ancillarymanipulators
 git-count-objects   ancillaryinterrogators
@@ -46,14 +46,14 @@ git-cvsimport   foreignscminterface
 git-cvsserver   foreignscminterface
 git-daemon  synchingrepositories
 git-describemainporcelain
-git-diffmainporcelain   common history
+git-diffmainporcelain   history
 git-diff-files  plumbinginterrogators
 git-diff-index  plumbinginterrogators
 git-diff-tree   plumbinginterrogators
 git-difftoolancillaryinterrogators
 git-fast-export ancillarymanipulators
 git-fast-import ancillarymanipulators
-git-fetch   mainporcelain   common remote
+git-fetch   mainporcelain   remote
 git-fetch-pack  synchingrepositories
 git-filter-branch   ancillarymanipulators
 git-fmt-merge-msg   purehelpers
@@ -62,7 +62,7 @@ git-format-patchmainporcelain
 git-fsckancillaryinterrogators
 git-gc  mainporcelain
 git-get-tar-commit-id   ancillaryinterrogators
-git-grepmainporcelain   common info
+git-grepmainporcelain   info
 git-gui mainporcelain
 git-hash-object plumbingmanipulators
 git-helpancillaryinterrogators
@@ -71,17 +71,17 @@ git-http-fetch  synchelpers
 git-http-push   synchelpers
 git-imap-send   foreignscminterface
 git-index-pack  plumbingmanipulators
-git-initmainporcelain   common init
+git-init

Re: [PATCH v11 0/5] group common commands by theme

2015-05-21 Thread Eric Sunshine
On Thu, May 21, 2015 at 1:39 PM, Sébastien Guimmara
sebastien.guimm...@gmail.com wrote:
 Just a minor change, the modification of new-command.txt was squashed to
 2/5 instead of 1/5.

Thanks. With or without addressing the two very minor nits I pointed
out in patches 2/5 and 5/5, this entire patch series (v11) is:

Reviewed-by: Eric Sunshine sunsh...@sunshineco.com

 Eric Sunshine (2):
   command-list: prepare machinery for upcoming common groups section
   generate-cmdlist: parse common group commands

 Sébastien Guimmara (3):
   command-list.txt: add the common groups block
   command-list.txt: drop the common tag
   help: respect new common command grouping

  Documentation/cmd-list.perl |  4 +++
  Documentation/howto/new-command.txt |  4 ++-
  Makefile|  9 ---
  command-list.txt| 53 
 ++---
  generate-cmdlist.perl   | 50 ++
  generate-cmdlist.sh | 23 
  help.c  | 24 -
  7 files changed, 117 insertions(+), 50 deletions(-)
  create mode 100755 generate-cmdlist.perl
  delete mode 100755 generate-cmdlist.sh
--
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 0/14] implement @{push} shorthand

2015-05-21 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Thu, May 21, 2015 at 12:44:29AM -0400, Jeff King wrote:

 This is a re-roll of the series at:
 
   http://thread.gmane.org/gmane.comp.version-control.git/268185
 
 The only changes here are the addition of patches 2 and 6, which are
 both cleanups that help make the other patches more readable/sensible.
 They are the same as what was posted during review of the thread linked
 above.  So there's nothing new here, but of course fresh reviews are
 welcome.

 Actually, that's not quite true; I forgot to mention one change:

   [03/14]: remote.c: drop remote pointer from struct branch

 In this version, this one remembers to also update the API
 documentation.

 -Peff

Thanks for a pleasant read.
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


Hello dear

2015-05-21 Thread Roseanna Martins
Hello dear
I am Mrs  Roseanna Martins from Sierra-Leone,a widow suffering from
long time (neck cancer) illness,I sent you letter a month ago, but I'm
not sure you received it, what his email fast, because I have not
heard from you, this is the reason why I repeat.

I have decided to hand over all my inheritance hence I am diagnosed of
cancer and the doctors say that I will live more days I need your
reply so that I hand you over the inheritance before it is late Remain
blessed.
Mrs. Roseanna.
--
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 0/4] submodule config lookup API

2015-05-21 Thread René Scharfe

Am 21.05.2015 um 19:06 schrieb Heiko Voigt:

diff --git a/submodule-config.h b/submodule-config.h
index 9061e4e..58afc83 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -24,6 +24,6 @@ const struct submodule *submodule_from_name(const unsigned 
char *commit_sha1,
const char *name);
  const struct submodule *submodule_from_path(const unsigned char *commit_sha1,
const char *path);
-void submodule_free(void);
+void submodule_free();


Why this change?  With void it matches the function's definition.

René

--
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 0/4] submodule config lookup API

2015-05-21 Thread Junio C Hamano
Heiko Voigt hvo...@hvoigt.net writes:

 This is finally the next iteration of the submodule config api. The last
 iteration can be found here:

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

 This iteration fixes the lookup of submodules by name
 (submodule_from_name()) where one needed to pass in the gitmodule sha1
 by mistake. To keep it simple for the user and behave as documented we
 should take the commit sha1 which is now fixed here. We now also test
 the lookup by name in the api tests.

 This should be ready for inclusion.

 Cheers Heiko

 Here is the interdiff to the last iteration:

 diff --git a/submodule-config.c b/submodule-config.c
 index 96623ad..177767d 100644
 --- a/submodule-config.c
 +++ b/submodule-config.c
 @@ -25,6 +25,11 @@ struct submodule_entry {
   struct submodule *config;
  };
  
 +enum lookup_type {
 + lookup_name,
 + lookup_path,
 +};

Please lose the comma after the last element in enum.  Some
compilers do not like it, I was told.

 + switch (lookup_type) {
 + case lookup_name:
 + submodule = cache_lookup_name(cache, sha1, key);
 + break;
 + case lookup_path:
 + submodule = cache_lookup_path(cache, sha1, key);
 + break;

Is this too deeply indented?
--
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 08/14] remote.c: report specific errors from branch_get_upstream

2015-05-21 Thread Jeff King
On Thu, May 21, 2015 at 11:33:58AM -0700, Junio C Hamano wrote:

  +static const char *error_buf(struct strbuf *err, const char *fmt, ...)
   {
  -   if (!branch || !branch-merge || !branch-merge[0])
  -   return NULL;
  +   if (err) {
  +   va_list ap;
  +   va_start(ap, fmt);
  +   strbuf_vaddf(err, fmt, ap);
  +   va_end(ap);
  +   }
  +   return NULL;
  +}
 
 Many of our functions return -1 to signal an error, and that is why
 it makes sense for our error() helper to return -1 to save code in
 the caller, but only because the callers of this private helper use
 a NULL to signal an error, this also returns NULL.  If we were to
 use the callers can opt into detailed message by passing strbuf
 pattern more widely, we would want a variant of the above that
 returns -1, too.  And such a helper would do the same thing as
 above, with only difference from the above is to return -1.

Yeah, this is not really a good general-purpose purpose function in that
sense. I have often wanted an error() that returns NULL, but avoided
adding just because it seemed like needless proliferation.

The real reason to include the return value at all in error() is to let
you turn two-liners into one-liners. We could drop the return value from
this helper entirely (or make it -1, but of course no callers would use
it) and write it out long-hand in the callers:

  if (!branch-merge) {
error_buf(err, ...);
return NULL;
  }

That is really not so bad, as there are only a handful of callers.

  +const char *branch_get_upstream(struct branch *branch, struct strbuf *err)
  +{
  +   if (!branch)
  +   return error_buf(err, _(HEAD does not point to a branch));
  +   if (!branch-merge || !branch-merge[0] || !branch-merge[0]-dst) {
  +   if (!ref_exists(branch-refname))
  +   return error_buf(err, _(no such branch: '%s'),
  +branch-name);
  +   if (!branch-merge)
  +   return error_buf(err,
  +_(no upstream configured for branch 
  '%s'),
  +branch-name);
  +   return error_buf(err,
  +_(upstream branch '%s' not stored as a 
  remote-tracking branch),
  +branch-merge[0]-src);
  +   }
  +
  return branch-merge[0]-dst;
   }
 
 This is a faithful conversion of what the get_upstream_branch() used
 to do, but that ref_exists() check and the error checking there look
 somewhat out of place.
 
 It makes the reader wonder what should happen when branch-refname
 does not exist as a ref, but branch-merge[0]-dst can be fully
 dereferenced.  Should it be an error, or if it is OK, the reason why
 it is OK is...?

Yeah, my goal here was to faithfully keep the same logic, but I had a
similar head-scratching moment. What would make more sense to me is:

  if (!branch)
return error(HEAD does not point to a branch);

  if (!branch-merge || !branch-merge[0]) {
/*
 * no merge config; is it because the user didn't define any, or
 * because it is not a real branch, and get_branch auto-vivified
 * it?
 */
if (!ref_exists(branch-refname))
return error(no such branch);
return error(no upstream configured for branch);
  }

  if (!branch-merge[0]-dst)
return error(upstream branch not stored as a remote-tracking branch);

  return branch-merge[0]-dst;


Hopefully the comment there answers your question; it is not that we
truly care whether the ref exists, but only that we are trying to tell
the difference between a real branch and a struct that is an artifact
of our internal code.

Note also that the original may dereference branch-merge[0] even if it
is NULL. I think that can't actually happen in practice (we only
allocate branch-merge if we have at least one item to put in it, and
all of the checks of branch-merge[0] are actually being over-careful).
But the code I just wrote above does not have that problem.

-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 v3 02/14] remote.c: refactor setup of branch-merge list

2015-05-21 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 When we call branch_get() to lookup or create a struct
 branch, we make sure the merge field is filled in so that
 callers can access it. But the conditions under which we do
 so are a little confusing, and can lead to two funny
 situations:
 ...
 In addition to those two fixes, this patch pushes the do we
 need to setup merge? logic down into set_merge, where it is
 a bit easier to follow.

Nicely done.  Thanks.


 Signed-off-by: Jeff King p...@peff.net
 ---
  remote.c | 19 +++
  1 file changed, 15 insertions(+), 4 deletions(-)

 diff --git a/remote.c b/remote.c
 index bec8b31..ac17e66 100644
 --- a/remote.c
 +++ b/remote.c
 @@ -1636,6 +1636,19 @@ static void set_merge(struct branch *ret)
   unsigned char sha1[20];
   int i;
  
 + if (!ret)
 + return; /* no branch */
 + if (ret-merge)
 + return; /* already run */
 + if (!ret-remote_name || !ret-merge_nr) {
 + /*
 +  * no merge config; let's make sure we don't confuse callers
 +  * with a non-zero merge_nr but a NULL merge
 +  */
 + ret-merge_nr = 0;
 + return;
 + }
 +
   ret-merge = xcalloc(ret-merge_nr, sizeof(*ret-merge));
   for (i = 0; i  ret-merge_nr; i++) {
   ret-merge[i] = xcalloc(1, sizeof(**ret-merge));
 @@ -1660,11 +1673,9 @@ struct branch *branch_get(const char *name)
   ret = current_branch;
   else
   ret = make_branch(name, 0);
 - if (ret  ret-remote_name) {
 + if (ret  ret-remote_name)
   ret-remote = remote_get(ret-remote_name);
 - if (ret-merge_nr)
 - set_merge(ret);
 - }
 + set_merge(ret);
   return ret;
  }
--
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 07/14] remote.c: introduce branch_get_upstream helper

2015-05-21 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 All of the information needed to find the @{upstream} of a
 branch is included in the branch struct, but callers have to
 navigate a series of possible-NULL values to get there.
 Let's wrap that logic up in an easy-to-read helper.

 Signed-off-by: Jeff King p...@peff.net

This step in the whole series is a gem.  I cannot believe that we
were content having to repeat that branch-merge[0]-dst if we can
dereference down to that level this many times.  Nice clean-up.

 diff --git a/builtin/branch.c b/builtin/branch.c
 index 258fe2f..1eb6215 100644
 --- a/builtin/branch.c
 +++ b/builtin/branch.c
 @@ -123,14 +123,12 @@ static int branch_merged(int kind, const char *name,
  
   if (kind == REF_LOCAL_BRANCH) {
   struct branch *branch = branch_get(name);
 + const char *upstream = branch_get_upstream(branch);
   unsigned char sha1[20];
  
 - if (branch 
 - branch-merge 
 - branch-merge[0] 
 - branch-merge[0]-dst 
 + if (upstream 
   (reference_name = reference_name_to_free =
 -  resolve_refdup(branch-merge[0]-dst, RESOLVE_REF_READING,
 +  resolve_refdup(upstream, RESOLVE_REF_READING,
   sha1, NULL)) != NULL)
   reference_rev = lookup_commit_reference(sha1);
--
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/2] fix post-rewrite hook with 'git rebase -i --exec'

2015-05-21 Thread Matthieu Moy
Matthieu Moy (2):
  rebase -i: demonstrate incorrect behavior of post-rewrite hook with
exec
  rebase -i: fix post-rewrite hook with failed exec command

 git-rebase--interactive.sh   | 10 +-
 t/t5407-post-rewrite-hook.sh | 17 +
 2 files changed, 22 insertions(+), 5 deletions(-)

-- 
2.4.1.171.g060e6ae.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


[PATCH 1/2] rebase -i: demonstrate incorrect behavior of post-rewrite hook with exec

2015-05-21 Thread Matthieu Moy
The 'exec' command is sending the current commit to stopped-sha, which is
supposed to contain the original commit (before rebase). As a result, if
an 'exec' command fails, the next 'git rebase --continue' will send the
current commit as old-sha1 to the post-rewrite hook.

The test currently fails with :

--- expected.data   2015-05-21 17:55:29.0 +
+++ [...]post-rewrite.data  2015-05-21 17:55:29.0 +
@@ -1,2 +1,3 @@
 2362ae8e1b1b865e6161e6f0e165ffb974abf018 
488028e9fac0b598b70cbeb594258a917e3f6fab
+488028e9fac0b598b70cbeb594258a917e3f6fab 
488028e9fac0b598b70cbeb594258a917e3f6fab
 babc8a4c7470895886fc129f1a015c486d05a351 
8edffcc4e69a4e696a1d4bab047df450caf99507

Signed-off-by: Matthieu Moy matthieu@imag.fr
---
 t/t5407-post-rewrite-hook.sh | 17 +
 1 file changed, 17 insertions(+)

diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh
index ea2e0d4..ecef820 100755
--- a/t/t5407-post-rewrite-hook.sh
+++ b/t/t5407-post-rewrite-hook.sh
@@ -212,4 +212,21 @@ EOF
verify_hook_input
 '
 
+test_expect_failure 'git rebase -i (exec)' '
+   git reset --hard D 
+   clear_hook_input 
+   FAKE_LINES=edit 1 exec_false 2 git rebase -i B
+   echo something bar 
+   git add bar 
+   # Fail because of exec false
+   test_must_fail git rebase --continue 
+   git rebase --continue 
+   echo rebase expected.args 
+   cat expected.data EOF 
+$(git rev-parse C) $(git rev-parse HEAD^)
+$(git rev-parse D) $(git rev-parse HEAD)
+EOF
+   verify_hook_input
+'
+
 test_done
-- 
2.4.1.171.g060e6ae.dirty

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


Re: [PATCH 2/4] ref-filter: add ref-filter API

2015-05-21 Thread Eric Sunshine
On Thu, May 21, 2015 at 1:30 PM, karthik nayak karthik@gmail.com wrote:
 On 05/21/2015 12:37 AM, Eric Sunshine wrote:
 On Wed, May 20, 2015 at 9:18 AM, Karthik Nayak karthik@gmail.com
 wrote:
   Makefile |  1 +
   ref-filter.c | 73
 
   ref-filter.h | 47 ++
   3 files changed, 121 insertions(+)
   create mode 100644 ref-filter.c
   create mode 100644 ref-filter.h

 A shortcoming of this approach is that it's not blame-friendly.
 Although those of us following this patch series know that much of the
 code in this patch was copied from for-each-ref.c, git-blame will not
 recognize this unless invoked in the very expensive git blame -C -C
 -C fashion (if I understand correctly). The most blame-friendly way
 to perform this re-organization is to have the code relocation (line
 removals and line additions) occur in one patch.

 There are multiple ways you could arrange to do so. One would be to
 first have a patch which introduces just a skeleton of the intended
 API, with do-nothing function implementations. A subsequent patch
 would then relocate the code from for-each-ref.c to ref-filter.c, and
 update for-each-ref.c to call into the new (now fleshed-out) API.

 Did you read Junio's suggestion on how I should re-order this WIP patch
 series ?
 That's somewhat on the lines of what you're suggesting. I'll probably be
 going ahead with that, not really sure about how blame works entirely so
 what do you think about that?

Yes, Junio's response did a much better job of saying what I intended.
Also, his response said something I meant to mention but forgot:
namely that, to ease the review task, code movement should be pure
movement, and not involve other changes.

Anyhow, follow Junio's advice. He knows what he's talking about. ;-)
--
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 07/14] remote.c: introduce branch_get_upstream helper

2015-05-21 Thread Jeff King
On Thu, May 21, 2015 at 02:14:29PM -0400, Jeff King wrote:

 There is a related cleanup I resisted, which is that several call-sites
 will call stat_tracking_info, then later look directly at
 branch-merge[0]-dst without a check for NULL (fill_tracking_info is
 such a site).
 
 This works because stat_tracking_info's return value tells us that we
 did indeed find an upstream to compare with. But it feels a little leaky
 to me. One solution is for stat_tracking_info to pass out the name of
 thte upstream, making the caller side something like:

So just for fun, here is what a whole patch, with the refactoring of the
return value, would look like.

diff --git a/builtin/branch.c b/builtin/branch.c
index cc55ff2..8ecabd1 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -425,25 +425,19 @@ static void fill_tracking_info(struct strbuf *stat, const 
char *branch_name,
int ours, theirs;
char *ref = NULL;
struct branch *branch = branch_get(branch_name);
+   const char *upstream;
struct strbuf fancy = STRBUF_INIT;
int upstream_is_gone = 0;
int added_decoration = 1;
 
-   switch (stat_tracking_info(branch, ours, theirs)) {
-   case 0:
-   /* no base */
-   return;
-   case -1:
-   /* with gone base */
+   if (stat_tracking_info(branch, ours, theirs, upstream)  0) {
+   if (!upstream)
+   return;
upstream_is_gone = 1;
-   break;
-   default:
-   /* with base */
-   break;
}
 
if (show_upstream_ref) {
-   ref = shorten_unambiguous_ref(branch-merge[0]-dst, 0);
+   ref = shorten_unambiguous_ref(upstream, 0);
if (want_color(branch_use_color))
strbuf_addf(fancy, %s%s%s,
branch_get_color(BRANCH_COLOR_UPSTREAM),
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 6847400..05dd23d 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -730,7 +730,7 @@ static void populate_value(struct refinfo *ref)
char buf[40];
 
if (stat_tracking_info(branch, num_ours,
-  num_theirs) != 1)
+  num_theirs, NULL))
continue;
 
if (!num_ours  !num_theirs)
@@ -753,7 +753,7 @@ static void populate_value(struct refinfo *ref)
assert(branch);
 
if (stat_tracking_info(branch, num_ours,
-   num_theirs) != 1)
+   num_theirs, NULL))
continue;
 
if (!num_ours  !num_theirs)
diff --git a/remote.c b/remote.c
index be45a39..6765051 100644
--- a/remote.c
+++ b/remote.c
@@ -2016,12 +2016,15 @@ int ref_newer(const unsigned char *new_sha1, const 
unsigned char *old_sha1)
 
 /*
  * Compare a branch with its upstream, and save their differences (number
- * of commits) in *num_ours and *num_theirs.
+ * of commits) in *num_ours and *num_theirs. The name of the upstream branch
+ * (or NULL if no upstream is defined) is returned via *upstream_name, if it
+ * is not itself NULL.
  *
- * Return 0 if branch has no upstream (no base), -1 if upstream is missing
- * (with gone base), otherwise 1 (with base).
+ * Returns -1 if num_ours and num_theirs could not be filled in (e.g., no
+ * upstream defined, or ref does not exist), 0 otherwise.
  */
-int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs)
+int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,
+  const char **upstream_name)
 {
unsigned char sha1[20];
struct commit *ours, *theirs;
@@ -2032,8 +2035,10 @@ int stat_tracking_info(struct branch *branch, int 
*num_ours, int *num_theirs)
 
/* Cannot stat unless we are marked to build on top of somebody else. */
base = branch_get_upstream(branch, NULL);
+   if (upstream_name)
+   *upstream_name = base;
if (!base)
-   return 0;
+   return -1;
 
/* Cannot stat if what we used to build on no longer exists */
if (read_ref(base, sha1))
@@ -2051,7 +2056,7 @@ int stat_tracking_info(struct branch *branch, int 
*num_ours, int *num_theirs)
/* are we the same? */
if (theirs == ours) {
*num_theirs = *num_ours = 0;
-   return 1;
+   return 0;
}
 
/* Run rev-list --left-right ours...theirs internally... */
@@ -2087,7 +2092,7 @@ int stat_tracking_info(struct branch *branch, int 
*num_ours, int *num_theirs)
/* clear object flags smudged by 

[PATCH] submodule documentation: Rewrite introductory paragraphs

2015-05-21 Thread Stefan Beller
It's better to start the man page with a description of what submodules
actually are instead of saying what they are not.

Reorder the paragraphs such that
the first short paragraph introduces the submodule concept,
the second paragraph highlights the usage of the submodule command,
the third paragraph giving background information,
and finally the fourth paragraph discusing alternatives such
as subtrees and remotes, which we don't want to be confused with.

This ordering deepens the knowledge on submodules with each paragraph.
First the basic questions like How/what will be answered, while the
underlying concepts will be taught at a later time.

Making sure it is not confused with subtrees and remotes is not really
enhancing knowledge of submodules itself, but rather painting the big
picture of git concepts, so you could also argue to have it as the second
paragraph. Personally I think this may confuse readers, specially newcomers
though.

Signed-off-by: Stefan Beller sbel...@google.com
---

I rewrote the first 2 paragraphs as well now. I think it doesn't make sense
to mention any subcommand in the description section, so I removed 
 When adding a new submodule to the tree, the 'add' subcommand is to be used.
though I kept the warning about submodules not being autmatically updated by
clone and pull. All other subcommands are gone from the description.


 Not a new problem, but does the command really manage them for
 you?  I view it more like You can use this command to manage,
 inspect and update the submodules.

 I agree.

Trying to find a better wording I looked more closely at other man pages to get
consistent with them. Most of them (e.g. add, status, rebase, revert) just 
describe
what they do in a short manner.

Now I am looking at the subtree man page and it looks as if it written in a 
similar style
to the submodules man page, the first 2 paragraphs give a short description what
subtrees do (not the command, but the concept) including an example.
The third paragraph then starts with Subtrees are not to be confused
with submodules, ... which I'd not want to see as early in a man page.
So in the subtree man page there is no such paragraph as
 
    This command can do this and that.

but rather the concept and examples are given.

 Documentation/git-submodule.txt | 50 ++---
 1 file changed, 22 insertions(+), 28 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 2c25916..97718cf 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -25,22 +25,17 @@ SYNOPSIS
 
 DESCRIPTION
 ---
-Submodules allow foreign repositories to be embedded within
-a dedicated subdirectory of the source tree, always pointed
-at a particular commit.
+This command will inspect, update and manage submodules.
 
-They are not to be confused with remotes, which are meant mainly
-for branches of the same project; submodules are meant for
-different projects you would like to make part of your source tree,
-while the history of the two projects still stays completely
-independent and you cannot modify the contents of the submodule
-from within the main project.
-If you want to merge the project histories and want to treat the
-aggregated whole as a single project from then on, you may want to
-add a remote for the other project and use the 'subtree' merge strategy,
-instead of treating the other project as a submodule. Directories
-that come from both projects can be cloned and checked out as a whole
-if you choose to go that route.
+Submodules allow you to keep another Git repository in a subdirectory
+of your repository. The other repository has its own history, which does not
+interfere with the history of the current repository. This can be used to
+have external dependencies such as libraries for example.
+
+When cloning or pulling a repository containing submodules however,
+these will not be checked out by default; the 'init' and 'update'
+subcommands will maintain submodules checked out and at
+appropriate revision in your working tree.
 
 Submodules are composed from a so-called `gitlink` tree entry
 in the main repository that refers to a particular commit object
@@ -51,19 +46,18 @@ describes the default URL the submodule shall be cloned 
from.
 The logical name can be used for overriding this URL within your
 local repository configuration (see 'submodule init').
 
-This command will manage the tree entries and contents of the
-gitmodules file for you, as well as inspect the status of your
-submodules and update them.
-When adding a new submodule to the tree, the 'add' subcommand
-is to be used.  However, when pulling a tree containing submodules,
-these will not be checked out by default;
-the 'init' and 'update' subcommands will maintain submodules
-checked out and at appropriate revision in your working tree.
-You can briefly inspect the up-to-date status of your submodules
-using the 'status' 

Re: [PATCH] submodule documentation: Reorder introductory paragraphs

2015-05-21 Thread Stefan Beller
On Thu, May 21, 2015 at 10:24 AM, Junio C Hamano gits...@pobox.com wrote:
 Stefan Beller sbel...@google.com writes:

 Reorder the paragraphs such that
 the first short paragraph introduces the submodule concept,
 the second paragraph highlights the usage of the submodule command,
 the third paragraph giving background information,
 and finally the fourth paragraph discusing alternatives such
 as subtrees and remotes, which we don't want to be confused with.

 This ordering deepens the knowledge on submodules with each paragraph.
 First the basic questions like How/what will be answered, while the
 underlying concepts will be taught at a later time.

 Sounds good.

 diff --git a/Documentation/git-submodule.txt 
 b/Documentation/git-submodule.txt
 index 2c25916..6c38c0d 100644
 --- a/Documentation/git-submodule.txt
 +++ b/Documentation/git-submodule.txt
 @@ -25,35 +25,12 @@ SYNOPSIS

  DESCRIPTION
  ---
 -Submodules allow foreign repositories to be embedded within
 -a dedicated subdirectory of the source tree, always pointed
 -at a particular commit.
 +Submodules allow other repositories to be embedded within
 +a dedicated subdirectory of the source tree pointing
 +at a particular commit in the other repository.

 Not a new problem, but I can misread this as if it requires the
 top-level superproject to have one single dedicated directory D to
 house all the foreign projects under it, D/project1, D/project2, ...

I agree, maybe we should reword the paragraphs themselves as well.

Submodules allow you to keep another Git repository in a subdirectory
of your repository. The other repository has its own history, which does not
interfere with the history of the current repository. This can be used to
have external dependencies such as libraries for example.


 -This command will manage the tree entries and contents of the
 -gitmodules file for you, as well as inspect the status of your
 -submodules and update them.
 +This command will manage the submodules for you, as well as
 +inspect the status of your submodules and update them.

 Not a new problem, but does the command really manage them for
 you?  I view it more like You can use this command to manage,
 inspect and update the submodules.

I agree.
--
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 v11 5/5] help: respect new common command grouping

2015-05-21 Thread Eric Sunshine
On Thu, May 21, 2015 at 1:39 PM, Sébastien Guimmara
sebastien.guimm...@gmail.com wrote:
 'git help' shows common commands in alphabetical order:

 The most commonly used git commands are:
addAdd file contents to the index
bisect Find by binary search the change that introduced a bug
branch List, create, or delete branches
checkout   Checkout a branch or paths to the working tree
clone  Clone a repository into a new directory
commit Record changes to the repository
[...]

 without any indication of how commands relate to high-level
 concepts or each other. Revise the output to explain their relationship
 with the typical Git workflow:

 The most commonly used git commands are:

The above line in the commit message does not match the actual output:

These are common Git commands used in various situations:

 start a working area (see also: git help tutorial)
clone  Clone a repository into a new directory
init   Create an empty Git repository or reinitialize [...]

 work on the current change (see also: git help everyday)
addAdd file contents to the index
reset  Reset current HEAD to the specified state

 examine the history and state (see also: git help revisions)
logShow commit logs
status Show the working tree status

[...]

 Helped-by: Eric Sunshine sunsh...@sunshineco.com
 Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
 Signed-off-by: Sébastien Guimmara sebastien.guimm...@gmail.com
 ---
 diff --git a/help.c b/help.c
 index 2072a87..8f72051 100644
 --- a/help.c
 +++ b/help.c
 @@ -218,17 +218,39 @@ void list_commands(unsigned int colopts,
 }
  }

 +static int cmd_group_cmp(const void *elem1, const void *elem2)
 +{
 +   const struct cmdname_help *e1 = elem1;
 +   const struct cmdname_help *e2 = elem2;
 +
 +   if (e1-group  e2-group)
 +   return -1;
 +   if (e1-group  e2-group)
 +   return 1;
 +   return strcmp(e1-name, e2-name);
 +}
 +
  void list_common_cmds_help(void)
  {
 int i, longest = 0;
 +   int current_grp = -1;

 for (i = 0; i  ARRAY_SIZE(common_cmds); i++) {
 if (longest  strlen(common_cmds[i].name))
 longest = strlen(common_cmds[i].name);
 }

 -   puts(_(The most commonly used git commands are:));
 +   qsort(common_cmds, ARRAY_SIZE(common_cmds),
 +   sizeof(common_cmds[0]), cmd_group_cmp);
 +
 +   puts(_(These are common Git commands used in various situations:));
 +
 for (i = 0; i  ARRAY_SIZE(common_cmds); i++) {
 +   if (common_cmds[i].group != current_grp) {
 +   printf(\n%s\n, 
 _(common_cmd_groups[common_cmds[i].group]));
 +   current_grp = common_cmds[i].group;
 +   }
 +
 printf(   %s   , common_cmds[i].name);
 mput_char(' ', longest - strlen(common_cmds[i].name));
 puts(_(common_cmds[i].help));
 --
 2.4.0.GIT
--
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 07/14] remote.c: introduce branch_get_upstream helper

2015-05-21 Thread Jeff King
On Thu, May 21, 2015 at 11:07:33AM -0700, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  All of the information needed to find the @{upstream} of a
  branch is included in the branch struct, but callers have to
  navigate a series of possible-NULL values to get there.
  Let's wrap that logic up in an easy-to-read helper.
 
  Signed-off-by: Jeff King p...@peff.net
 
 This step in the whole series is a gem.  I cannot believe that we
 were content having to repeat that branch-merge[0]-dst if we can
 dereference down to that level this many times.  Nice clean-up.

There is a related cleanup I resisted, which is that several call-sites
will call stat_tracking_info, then later look directly at
branch-merge[0]-dst without a check for NULL (fill_tracking_info is
such a site).

This works because stat_tracking_info's return value tells us that we
did indeed find an upstream to compare with. But it feels a little leaky
to me. One solution is for stat_tracking_info to pass out the name of
thte upstream, making the caller side something like:

diff --git a/builtin/branch.c b/builtin/branch.c
index cc55ff2..edc4deb 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -425,11 +425,12 @@ static void fill_tracking_info(struct strbuf *stat, const 
char *branch_name,
int ours, theirs;
char *ref = NULL;
struct branch *branch = branch_get(branch_name);
+   const char *upstream;
struct strbuf fancy = STRBUF_INIT;
int upstream_is_gone = 0;
int added_decoration = 1;
 
-   switch (stat_tracking_info(branch, ours, theirs)) {
+   switch (stat_tracking_info(branch, ours, theirs, upstream)) {
case 0:
/* no base */
return;
@@ -443,7 +444,7 @@ static void fill_tracking_info(struct strbuf *stat, const 
char *branch_name,
}
 
if (show_upstream_ref) {
-   ref = shorten_unambiguous_ref(branch-merge[0]-dst, 0);
+   ref = shorten_unambiguous_ref(upstream, 0);
if (want_color(branch_use_color))
strbuf_addf(fancy, %s%s%s,
branch_get_color(BRANCH_COLOR_UPSTREAM),


This is still a little error-prone, though. We assume upstream was
filled in depending on the return value of stat_tracking_info. I wonder
if we could get rid of the weird tri-state return value from
stat_tracking_info, and just have callers detect the there is no base
case by checking upstream != NULL.

I dunno. It is not buggy in any of the current callers, so it might not
be worth spending too much time on.

-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


[PATCH 2/2] rebase -i: fix post-rewrite hook with failed exec command

2015-05-21 Thread Matthieu Moy
Usually, when 'git rebase' stops before completing the rebase, it is to
give the user an opportunity to edit a commit (e.g. with the 'edit'
command). In such cases, 'git rebase' leaves the sha1 of the commit being
rewritten in $state_dir/stopped-sha, and subsequent 'git rebase
--continue' will call the post-rewrite hook with this sha1 as old-sha1
argument to the post-rewrite hook.

The case of 'git rebase' stopping because of a failed 'exec' command is
different: it gives the opportunity to the user to examine or fix the
failure, but does not stop saying here's a commit to edit, use
--continue when you're done. So, there's no reason to call the
post-rewrite hook for 'exec' commands. If the user did rewrite the
commit, it would be with 'git commit --amend' which already called the
post-rewrite hook.

Fix the behavior to leave no stopped-sha file in case of failed exec
command, and teach 'git rebase --continue' to skip record_in_rewritten if
no stopped-sha file is found.

Signed-off-by: Matthieu Moy matthieu@imag.fr
---
I'm afraid I'm the one to blame for the bug :-(.

 git-rebase--interactive.sh   | 10 +-
 t/t5407-post-rewrite-hook.sh |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 08e5d86..1c321e4 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -486,7 +486,7 @@ do_pick () {
 }
 
 do_next () {
-   rm -f $msg $author_script $amend || exit
+   rm -f $msg $author_script $amend $state_dir/stopped-sha || exit
read -r command sha1 rest  $todo
case $command in
$comment_char*|''|noop)
@@ -576,9 +576,6 @@ do_next () {
read -r command rest  $todo
mark_action_done
printf 'Executing: %s\n' $rest
-   # exec command doesn't take a sha1 in the todo-list.
-   # = can't just use $sha1 here.
-   git rev-parse --verify HEAD  $state_dir/stopped-sha
${SHELL:-@SHELL_PATH@} -c $rest # Actual execution
status=$?
# Run in subshell because require_clean_work_tree can die.
@@ -874,7 +871,10 @@ first and then run 'git rebase --continue' again.
fi
fi
 
-   record_in_rewritten $(cat $state_dir/stopped-sha)
+   if test -r $state_dir/stopped-sha
+   then
+   record_in_rewritten $(cat $state_dir/stopped-sha)
+   fi
 
require_clean_work_tree rebase
do_rest
diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh
index ecef820..24ba796 100755
--- a/t/t5407-post-rewrite-hook.sh
+++ b/t/t5407-post-rewrite-hook.sh
@@ -212,7 +212,7 @@ EOF
verify_hook_input
 '
 
-test_expect_failure 'git rebase -i (exec)' '
+test_expect_success 'git rebase -i (exec)' '
git reset --hard D 
clear_hook_input 
FAKE_LINES=edit 1 exec_false 2 git rebase -i B
-- 
2.4.1.171.g060e6ae.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 v3 08/14] remote.c: report specific errors from branch_get_upstream

2015-05-21 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 diff --git a/remote.c b/remote.c
 index dca3442..1b7051a 100644
 --- a/remote.c
 +++ b/remote.c
 @@ -1705,10 +1705,35 @@ int branch_merge_matches(struct branch *branch,
   return refname_match(branch-merge[i]-src, refname);
  }
  
 -const char *branch_get_upstream(struct branch *branch)
 +__attribute((format (printf,2,3)))
 +static const char *error_buf(struct strbuf *err, const char *fmt, ...)
  {
 - if (!branch || !branch-merge || !branch-merge[0])
 - return NULL;
 + if (err) {
 + va_list ap;
 + va_start(ap, fmt);
 + strbuf_vaddf(err, fmt, ap);
 + va_end(ap);
 + }
 + return NULL;
 +}

Many of our functions return -1 to signal an error, and that is why
it makes sense for our error() helper to return -1 to save code in
the caller, but only because the callers of this private helper use
a NULL to signal an error, this also returns NULL.  If we were to
use the callers can opt into detailed message by passing strbuf
pattern more widely, we would want a variant of the above that
returns -1, too.  And such a helper would do the same thing as
above, with only difference from the above is to return -1.

It's a shame that we have to return something from this function,
whose primary purpose is we may or may not want an error message in
a strbuf, so format the message when and only when we give you a
strbuf, but C forces us to make it always return NULL to signal an
error to the caller, and optionally format the message into a strbuf
if given.

And the name of this helper function only captures the optionally
format the message part, not the always return NULL part.

 +const char *branch_get_upstream(struct branch *branch, struct strbuf *err)
 +{
 + if (!branch)
 + return error_buf(err, _(HEAD does not point to a branch));
 + if (!branch-merge || !branch-merge[0] || !branch-merge[0]-dst) {
 + if (!ref_exists(branch-refname))
 + return error_buf(err, _(no such branch: '%s'),
 +  branch-name);
 + if (!branch-merge)
 + return error_buf(err,
 +  _(no upstream configured for branch 
 '%s'),
 +  branch-name);
 + return error_buf(err,
 +  _(upstream branch '%s' not stored as a 
 remote-tracking branch),
 +  branch-merge[0]-src);
 + }
 +
   return branch-merge[0]-dst;
  }

This is a faithful conversion of what the get_upstream_branch() used
to do, but that ref_exists() check and the error checking there look
somewhat out of place.

It makes the reader wonder what should happen when branch-refname
does not exist as a ref, but branch-merge[0]-dst can be fully
dereferenced.  Should it be an error, or if it is OK, the reason why
it is OK is...?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] submodule documentation: Reorder introductory paragraphs

2015-05-21 Thread Heiko Voigt
On Wed, May 20, 2015 at 04:11:57PM -0700, Stefan Beller wrote:
 It's better to start the man page with a description of what submodules
 actually are instead of saying what they are not.
 
 Reorder the paragraphs such that
 the first short paragraph introduces the submodule concept,
 the second paragraph highlights the usage of the submodule command,
 the third paragraph giving background information,
 and finally the fourth paragraph discusing alternatives such
 as subtrees and remotes, which we don't want to be confused with.
 
 This ordering deepens the knowledge on submodules with each paragraph.
 First the basic questions like How/what will be answered, while the
 underlying concepts will be taught at a later time.
 
 Making sure it is not confused with subtrees and remotes is not really
 enhancing knowledge of submodules itself, but rather painting the big
 picture of git concepts, so you could also argue to have it as the second
 paragraph. Personally I think this may confuse readers, specially newcomers
 though.
 
 Signed-off-by: Stefan Beller sbel...@google.com

Looks good to me.

Cheers Heiko
--
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 v10 4/5] command-list.txt: drop the common tag

2015-05-21 Thread Sébastien Guimmara
command-list.sh, retired in the previous patch, was the only
consumer of the common tag, so drop this now-unnecessary
attribute.

before:
git-add  mainporcelaincommon worktree

after:
git-add  mainporcelainworktree

Helped-by: Eric Sunshine sunsh...@sunshineco.com
Signed-off-by: Sébastien Guimmara sebastien.guimm...@gmail.com
---
 command-list.txt | 42 +-
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/command-list.txt b/command-list.txt
index 32ddab3..9a98752 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -11,30 +11,30 @@ remote   collaborate (see also: git help workflows)
 # List of known git commands.
 ### command list (do not change this line)
 # command name  category [deprecated] [common]
-git-add mainporcelain common
+git-add mainporcelain   worktree
 git-am  mainporcelain
 git-annotateancillaryinterrogators
 git-apply   plumbingmanipulators
 git-archimport  foreignscminterface
 git-archive mainporcelain
-git-bisect  mainporcelain   common info
+git-bisect  mainporcelain   info
 git-blame   ancillaryinterrogators
-git-branch  mainporcelain   common history
+git-branch  mainporcelain   history
 git-bundle  mainporcelain
 git-cat-fileplumbinginterrogators
 git-check-attr  purehelpers
 git-check-ignorepurehelpers
 git-check-mailmap   purehelpers
-git-checkoutmainporcelain   common history
+git-checkoutmainporcelain   history
 git-checkout-index  plumbingmanipulators
 git-check-ref-formatpurehelpers
 git-cherry  ancillaryinterrogators
 git-cherry-pick mainporcelain
 git-citool  mainporcelain
 git-clean   mainporcelain
-git-clone   mainporcelain   common init
+git-clone   mainporcelain   init
 git-column  purehelpers
-git-commit  mainporcelain   common history
+git-commit  mainporcelain   history
 git-commit-tree plumbingmanipulators
 git-config  ancillarymanipulators
 git-count-objects   ancillaryinterrogators
@@ -46,14 +46,14 @@ git-cvsimport   foreignscminterface
 git-cvsserver   foreignscminterface
 git-daemon  synchingrepositories
 git-describemainporcelain
-git-diffmainporcelain   common history
+git-diffmainporcelain   history
 git-diff-files  plumbinginterrogators
 git-diff-index  plumbinginterrogators
 git-diff-tree   plumbinginterrogators
 git-difftoolancillaryinterrogators
 git-fast-export ancillarymanipulators
 git-fast-import ancillarymanipulators
-git-fetch   mainporcelain   common remote
+git-fetch   mainporcelain   remote
 git-fetch-pack  synchingrepositories
 git-filter-branch   ancillarymanipulators
 git-fmt-merge-msg   purehelpers
@@ -62,7 +62,7 @@ git-format-patchmainporcelain
 git-fsckancillaryinterrogators
 git-gc  mainporcelain
 git-get-tar-commit-id   ancillaryinterrogators
-git-grepmainporcelain   common info
+git-grepmainporcelain   info
 git-gui mainporcelain
 git-hash-object plumbingmanipulators
 git-helpancillaryinterrogators
@@ -71,17 +71,17 @@ git-http-fetch  synchelpers
 git-http-push   synchelpers
 git-imap-send   foreignscminterface
 git-index-pack  plumbingmanipulators
-git-initmainporcelain   common init
+git-init

RE: recovering from unordered stage entries in index error

2015-05-21 Thread McHenry, Matt
 This message can be improved to show what entries have this problem.

Yes, that would definitely be a start.  :)

 But then I don't see any way to recover the index manually. ls-files
 will die too. Perhaps we should be gentle in this case: show warnings

Actually, ls-files succeeds on my broken index:

$ git ls-files  /dev/null
$ echo $?
0

Could I do something with 'git read-tree' to force creation of a new 
valid index?  I guess 'git clone' would work too, except that I have 'git svn' 
metadata that I'd need to preserve.

 instead of aborting the program and internally reorder the index. I
 think, unless you have multiple entries with the same stage, the
 recovered index should run well. The broken index could be renamed to
 index.broken or something for later analysis, or we forbid writing the
 reordered index to disk.
 
 Hmm?
 
  write-tree: command returned error: 128
 
  'git status' shows a few untracked files but is otherwise clean.
 
  It looks like this check was introduced in
 15999d0be8179fb7a2e6eafb931d25ed65df50aa, with the summary
 read_index_from(): catch out of order entries when reading an index file
 (first appearing in 2.2.0).
 
  Mailing list discussion looked like it implicated third-party
 tools.  I don't recall running any other tools on this repo; it doesn't do
 much day-to-day other than a long series of 'git svn fetch'es.  (But it's
 been around for a couple of years, so who knows.)
 
  At any rate, what can I do to recover from this situation?  I
 tried to locate a path with multiple index entries like this, but got no
 results:
 
  $ git ls-files -s | cut -f 2-100 | sort | uniq -c | grep -v '^[ \t]*1 '
 
  (I originally posted on SO at
 http://stackoverflow.com/questions/30264826/; I'll update that with any
 solutions that come up here, to ease future googling.)
  --
  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
 
 
 
 
 --
 Duy


[PATCH v10 0/5] group common commands by theme

2015-05-21 Thread Sébastien Guimmara
Same as v9 [1], with: 

* command-list.txt: reduce verbosity by squashing the two header lines
  into one:

### command list (do not change this line)

* include a missing update to new-command.txt.

[1] http://thread.gmane.org/gmane.comp.version-control.git/269496

Eric Sunshine (2):
  command-list: prepare machinery for upcoming common groups section
  generate-cmdlist: parse common group commands

Sébastien Guimmara (3):
  command-list.txt: add the common groups block
  command-list.txt: drop the common tag
  help: respect new common command grouping

 Documentation/cmd-list.perl |  4 +++
 Documentation/howto/new-command.txt |  4 ++-
 Makefile|  9 ---
 command-list.txt| 53 ++---
 generate-cmdlist.perl   | 50 ++
 generate-cmdlist.sh | 23 
 help.c  | 24 -
 7 files changed, 117 insertions(+), 50 deletions(-)
 create mode 100755 generate-cmdlist.perl
 delete mode 100755 generate-cmdlist.sh

-- 
2.4.0.GIT

--
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 v10 2/5] command-list.txt: add the common groups block

2015-05-21 Thread Sébastien Guimmara
The ultimate goal is for git help to display common commands in
groups rather than alphabetically. As a first step, define the
groups in a new block, and then assign a group to each
common command.

Add a block at the beginning of command-list.txt:

init start a working area (see also: git help tutorial)
worktree work on the current change (see also:[...]
info examine the history and state (see also: git [...]
history  grow, mark and tweak your history
remote   collaborate (see also: git help workflows)

storing information about common commands group, then map each common
command to a group:

git-add  mainporcelaincommon worktree

Helped-by: Eric Sunshine sunsh...@sunshineco.com
Helped-by: Junio C Hamano gits...@pobox.com
Helped-by:  Emma Jane Hogbin Westby emma.wes...@gmail.com
Signed-off-by: Sébastien Guimmara sebastien.guimm...@gmail.com
---
 command-list.txt | 51 +++
 1 file changed, 31 insertions(+), 20 deletions(-)

diff --git a/command-list.txt b/command-list.txt
index 181a9c2..32ddab3 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -1,3 +1,14 @@
+# common commands are grouped by themes
+# these groups are output by 'git help' in the order declared here.
+# map each common command in the command list to one of these groups.
+### common groups (do not change this line)
+init start a working area (see also: git help tutorial)
+worktree work on the current change (see also: git help everyday)
+info examine the history and state (see also: git help revisions)
+history  grow, mark and tweak your common history
+remote   collaborate (see also: git help workflows)
+
+# List of known git commands.
 ### command list (do not change this line)
 # command name  category [deprecated] [common]
 git-add mainporcelain common
@@ -6,24 +17,24 @@ git-annotate
ancillaryinterrogators
 git-apply   plumbingmanipulators
 git-archimport  foreignscminterface
 git-archive mainporcelain
-git-bisect  mainporcelain common
+git-bisect  mainporcelain   common info
 git-blame   ancillaryinterrogators
-git-branch  mainporcelain common
+git-branch  mainporcelain   common history
 git-bundle  mainporcelain
 git-cat-fileplumbinginterrogators
 git-check-attr  purehelpers
 git-check-ignorepurehelpers
 git-check-mailmap   purehelpers
-git-checkoutmainporcelain common
+git-checkoutmainporcelain   common history
 git-checkout-index  plumbingmanipulators
 git-check-ref-formatpurehelpers
 git-cherry  ancillaryinterrogators
 git-cherry-pick mainporcelain
 git-citool  mainporcelain
 git-clean   mainporcelain
-git-clone   mainporcelain common
+git-clone   mainporcelain   common init
 git-column  purehelpers
-git-commit  mainporcelain common
+git-commit  mainporcelain   common history
 git-commit-tree plumbingmanipulators
 git-config  ancillarymanipulators
 git-count-objects   ancillaryinterrogators
@@ -35,14 +46,14 @@ git-cvsimport   foreignscminterface
 git-cvsserver   foreignscminterface
 git-daemon  synchingrepositories
 git-describemainporcelain
-git-diffmainporcelain common
+git-diffmainporcelain   common history
 git-diff-files  plumbinginterrogators
 git-diff-index  plumbinginterrogators
 git-diff-tree   plumbinginterrogators
 git-difftoolancillaryinterrogators
 git-fast-export ancillarymanipulators
 git-fast-import ancillarymanipulators
-git-fetch   mainporcelain common
+git-fetch   mainporcelain   common remote
 git-fetch-pack  synchingrepositories
 git-filter-branch   ancillarymanipulators
 git-fmt-merge-msg   purehelpers
@@ -51,7 +62,7 @@ git-format-patch

[PATCH v10 5/5] help: respect new common command grouping

2015-05-21 Thread Sébastien Guimmara
'git help' shows common commands in alphabetical order:

The most commonly used git commands are:
   addAdd file contents to the index
   bisect Find by binary search the change that introduced a bug
   branch List, create, or delete branches
   checkout   Checkout a branch or paths to the working tree
   clone  Clone a repository into a new directory
   commit Record changes to the repository
   [...]

without any indication of how commands relate to high-level
concepts or each other. Revise the output to explain their relationship
with the typical Git workflow:

These are common Git commands used in various situations:

start a working area (see also: git help tutorial)
   clone  Clone a repository into a new directory
   init   Create an empty Git repository or reinitialize [...]

work on the current change (see also: git help everyday)
   addAdd file contents to the index
   reset  Reset current HEAD to the specified state

examine the history and state (see also: git help revisions)
   logShow commit logs
   status Show the working tree status

   [...]

Helped-by: Eric Sunshine sunsh...@sunshineco.com
Helped-by: Ramsay Jones ram...@ramsay1.demon.co.uk
Signed-off-by: Sébastien Guimmara sebastien.guimm...@gmail.com
---
 help.c | 24 +++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/help.c b/help.c
index 2072a87..8f72051 100644
--- a/help.c
+++ b/help.c
@@ -218,17 +218,39 @@ void list_commands(unsigned int colopts,
}
 }
 
+static int cmd_group_cmp(const void *elem1, const void *elem2)
+{
+   const struct cmdname_help *e1 = elem1;
+   const struct cmdname_help *e2 = elem2;
+
+   if (e1-group  e2-group)
+   return -1;
+   if (e1-group  e2-group)
+   return 1;
+   return strcmp(e1-name, e2-name);
+}
+
 void list_common_cmds_help(void)
 {
int i, longest = 0;
+   int current_grp = -1;
 
for (i = 0; i  ARRAY_SIZE(common_cmds); i++) {
if (longest  strlen(common_cmds[i].name))
longest = strlen(common_cmds[i].name);
}
 
-   puts(_(The most commonly used git commands are:));
+   qsort(common_cmds, ARRAY_SIZE(common_cmds),
+   sizeof(common_cmds[0]), cmd_group_cmp);
+
+   puts(_(These are common Git commands used in various situations:));
+
for (i = 0; i  ARRAY_SIZE(common_cmds); i++) {
+   if (common_cmds[i].group != current_grp) {
+   printf(\n%s\n, 
_(common_cmd_groups[common_cmds[i].group]));
+   current_grp = common_cmds[i].group;
+   }
+
printf(   %s   , common_cmds[i].name);
mput_char(' ', longest - strlen(common_cmds[i].name));
puts(_(common_cmds[i].help));
-- 
2.4.0.GIT

--
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: identical hashes on two branches, but holes in git log

2015-05-21 Thread Philip Oakley

From: Philippe De Muyter p...@macq.eu
To: Junio C Hamano gits...@pobox.com
Cc: git@vger.kernel.org; Jeff King p...@peff.net; John Keeping 
j...@keeping.me.uk

Sent: Thursday, May 21, 2015 8:15 AM
Subject: Re: identical hashes on two branches, but holes in git log



On Tue, May 19, 2015 at 03:12:31PM -0700, Junio C Hamano wrote:

Philippe De Muyter p...@macq.eu writes:

 On Tue, May 19, 2015 at 09:01:10AM -0700, Junio C Hamano wrote:
 Philippe De Muyter p...@macq.eu writes:

  Trying to understand, I have eventually done git log on my 
  branch and

  on v3.15 with the following commands :
 
  git log v3.15 --full-history --decorate=short | grep '^commit'  
  /tmp/3.15.commits
  git log --full-history --decorate=short | grep '^commit'  
  /tmp/mybranch.commits


 Either

 git log --oneline v3.15..HEAD ;# show what I have not in 
 theirs


 or

 gitk v3.15...HEAD ;# show our differences graphically

 This shows the commits in my branch starting from the most recent 
 common point,
 thus my commits, but I see differences in the files not explained 
 by my commits,
 but by the fact that many older commits (between v3.13 and v3.14) 
 are missing on
 my branch, but still in both branches I have a commit called v3.14 
 with the

 same hash.  Is that normal ?

Sorry, cannot parse.  Neither of the above would show files, so just
about the place where you start talking about I see differences in
the files, you lost me.


Look at the other part of the thread, with the discussion with Jeff 
and John


The light has come, and what I understand is:

don't trust the default (ordering) mode of 'git log' :(



Surely the question now should be What should the man page say that 
would have explained the default ordering mode in an understandable way, 
rather than the current misunderstanding?.


What 'ordering' were you 'trusting' (presuming) anyway? The current 
default mode doesn't actually say anything about the order anyway (as 
you've discovered).




I surmise this happens only when 'git merge' has been used.


--
Philip 


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


Re: [PATCH] submodule documentation: Reorder introductory paragraphs

2015-05-21 Thread Philip Oakley

From: Stefan Beller sbel...@google.com
It's better to start the man page with a description of what 
submodules

actually are instead of saying what they are not.

Reorder the paragraphs such that
the first short paragraph introduces the submodule concept,
the second paragraph highlights the usage of the submodule command,
the third paragraph giving background information,
and finally the fourth paragraph discusing alternatives such
as subtrees and remotes, which we don't want to be confused with.

This ordering deepens the knowledge on submodules with each paragraph.
First the basic questions like How/what will be answered, while the
underlying concepts will be taught at a later time.

Making sure it is not confused with subtrees and remotes is not really
enhancing knowledge of submodules itself, but rather painting the big
picture of git concepts, so you could also argue to have it as the 
second
paragraph. Personally I think this may confuse readers, specially 
newcomers

though.

Signed-off-by: Stefan Beller sbel...@google.com
---
Documentation/git-submodule.txt | 54 
-

1 file changed, 26 insertions(+), 28 deletions(-)

diff --git a/Documentation/git-submodule.txt 
b/Documentation/git-submodule.txt

index 2c25916..6c38c0d 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -25,35 +25,12 @@ SYNOPSIS

DESCRIPTION
---
-Submodules allow foreign repositories to be embedded within
-a dedicated subdirectory of the source tree, always pointed
-at a particular commit.
+Submodules allow other repositories to be embedded within
+a dedicated subdirectory of the source tree pointing
+at a particular commit in the other repository.

-They are not to be confused with remotes, which are meant mainly
-for branches of the same project; submodules are meant for
-different projects you would like to make part of your source tree,
-while the history of the two projects still stays completely
-independent and you cannot modify the contents of the submodule
-from within the main project.
-If you want to merge the project histories and want to treat the
-aggregated whole as a single project from then on, you may want to
-add a remote for the other project and use the 'subtree' merge 
strategy,

-instead of treating the other project as a submodule. Directories
-that come from both projects can be cloned and checked out as a whole
-if you choose to go that route.
-
-Submodules are composed from a so-called `gitlink` tree entry
-in the main repository that refers to a particular commit object
-within the inner repository that is completely separate.
-A record in the `.gitmodules` (see linkgit:gitmodules[5]) file at the
-root of the source tree assigns a logical name to the submodule and
-describes the default URL the submodule shall be cloned from.
-The logical name can be used for overriding this URL within your
-local repository configuration (see 'submodule init').
-
-This command will manage the tree entries and contents of the
-gitmodules file for you, as well as inspect the status of your
-submodules and update them.
+This command will manage the submodules for you, as well as
+inspect the status of your submodules and update them.
When adding a new submodule to the tree, the 'add' subcommand
is to be used.  However, when pulling a tree containing submodules,
these will not be checked out by default;
@@ -64,6 +41,27 @@ using the 'status' subcommand and get a detailed 
overview of the

difference between the index and checkouts using the 'summary'
subcommand.

+Submodules are composed from a so-called `gitlink` tree entry
+in the main repository that refers to a particular commit object
+within the inner repository that is completely separate.
+A record in the `.gitmodules` (see linkgit:gitmodules[5]) file at the
+root of the source tree assigns a logical name to the submodule and
+describes the default URL the submodule shall be cloned from.
+The logical name can be used for overriding this URL within your
+local repository configuration (see 'submodule init').
+
+Submodules are not to be confused with remotes, which are meant
+mainly for branches of the same project;


This use of 'branches' didn't work for me. remotes are meant mainly for 
branches of the same project ?



 submodules are meant for
+different projects you would like to make part of your source tree,
+while the history of the two projects still stays completely
+independent and you cannot modify the contents of the submodule
+from within the main project.
+If you want to merge the project histories and want to treat the
+aggregated whole as a single project from then on, you may want to
+add a remote for the other project and use the 'subtree' merge 
strategy,

+instead of treating the other project as a submodule. Directories
+that come from both projects can be cloned and checked out as a whole
+if you choose to go that route.


--
Philip 


--
To unsubscribe from this 

Re: git p4 clone - exclude file types

2015-05-21 Thread FusionX86
I thought about that, but no. The box I'm running git-p4 on has the
following specs:

CentOS 6.6 64bit
1 CPU
8GB RAM
8GB Swap

It is also on the same physical network as the Perforce server. I
remember seeing someone else complain about this, but I can't find the
article/blog now.


On Wed, May 20, 2015 at 12:49 AM, Luke Diamand l...@diamand.org wrote:
 On 19/05/15 08:38, FusionX86 wrote:

 Thanks Luke, looks like this does work for excluding files when using
 git p4. Great!

 Unrelated question...

 While using git p4 I have noticed that most of the time the clone/sync
 operations hang and I have to keep retrying. The Perforce depot I'm
 currently working with is larger than I'd like and has a lot of binary
 files which might be the cause. The point it gets to in the clone/sync
 is always random and doesn't ever stop on the same files or file
 types. Sometimes it'll die soon after starting, but other times it
 almost completes and then dies. If I keep retrying, it will eventually
 complete. I haven't been able to narrow down the cause, but I do
 notice that the git-fast-import stops right as the clone/sync dies.
 I'm wondering if git is overwhelmed and terminates. Have you ever seen
 this? Any suggestions?


 Running out of memory?

 Is this on a 32bit or 64bit system? How much virtual memory do you have?

 Luke

--
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-send-email.perl: Add sendmail aliases support

2015-05-21 Thread Allen Hubbe
On Thu, May 21, 2015 at 5:05 PM, Eric Sunshine sunsh...@sunshineco.com wrote:
 On Thu, May 21, 2015 at 4:48 PM, Allen Hubbe alle...@gmail.com wrote:
 My motivation for this patch was not really to support the sendmail
 aliases file directly.  The commit message may therefore be
 misleading.  So, I could also rewrite the commit message to say
 something like, loosely based on the sendmail aliases format, if the
 exceptions to the format in the current message are not enough.

 That probably would be a good idea. After reading the 'aliases' man
 page you cited[1], I was wondering why your parser diverged from it in
 so many ways. In addition to lack of support for folded lines, your
 format:

 * only recognizes comment lines when '#' is the first character,
 whereas [1] allows whitespace before '#'

 * only recognizes zero-length lines as empty, whereas [1] more loosely
 interprets a whitespace-only line as empty

 * doesn't support quoting the 'name' part of name: value as [1] does


Those are good points.  Maybe I shouldn't even mention sendmail at
all, not in the name of the format, and not in the commit message.
What name would be a good name for this format?

All the other formats would are based on the formats of email clients.
Adding sendmail was a departure from that.  Adding a format that is
not associated with any email program would be an even further
departure.  I don't know how other people feel about this.  Hopefully,
if this new format seems like it would be useful, the fact that it is
not an email client format can be overlooked.

 Really, I just prefer the simpler alias: email|alias syntax
 instead of the ones for mutt, elm, etc, and that is why I wrote this
 patch.

 Your patch is missing a documentation update
 (Documentation/git-send-email.txt) and new tests
 (t/t9001-send-email.sh).

Thanks for the direction.


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


Re: [PATCH] git-send-email.perl: Add sendmail aliases support

2015-05-21 Thread Junio C Hamano
Allen Hubbe alle...@gmail.com writes:

 Those are good points.  Maybe I shouldn't even mention sendmail at
 all, not in the name of the format, and not in the commit message.
 What name would be a good name for this format?

simple?

And if you are going to define such a format, then I do not think
you would even need to pretend that someday you might support
line folding (hence there is no need for eh, excuse me, you seem to
have wanted to express a long folded line here, but I do not support
it (yet) warning messages).

--
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 v5 3/3] upload-pack: optionally allow fetching reachable sha1

2015-05-21 Thread Fredrik Medley
With uploadpack.allowReachableSHA1InWant configuration option set on the
server side, git fetch can make a request with a want line that names
an object that has not been advertised (likely to have been obtained out
of band or from a submodule pointer). Only objects reachable from the
branch tips, i.e. the union of advertised branches and branches hidden by
transfer.hideRefs, will be processed. Note that there is an associated
cost of having to walk back the history to check the reachability.

This feature can be used when obtaining the content of a certain commit,
for which the sha1 is known, without the need of cloning the whole
repository, especially if a shallow fetch is used. Useful cases are e.g.
repositories containing large files in the history, fetching only the
needed data for a submodule checkout, when sharing a sha1 without telling
which exact branch it belongs to and in Gerrit, if you think in terms of
commits instead of change numbers. (The Gerrit case has already been
solved through allowTipSHA1InWant as every Gerrit change has a ref.)

Signed-off-by: Fredrik Medley fredrik.med...@gmail.com
---
 Documentation/config.txt  |  6 +++
 Documentation/technical/http-protocol.txt |  3 +-
 Documentation/technical/protocol-capabilities.txt |  7 +++
 fetch-pack.c  | 10 -
 t/t5516-fetch-push.sh | 55 +++
 upload-pack.c | 22 +++--
 6 files changed, 97 insertions(+), 6 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1311383..b8215ba 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2547,6 +2547,12 @@ uploadpack.allowTipSHA1InWant::
of a hidden ref (by default, such a request is rejected).
see also `uploadpack.hideRefs`.
 
+uploadpack.allowReachableSHA1InWant::
+   Allow `upload-pack` to accept a fetch request that asks for an
+   object that is reachable from any ref tip. However, note that
+   calculating object reachability is computationally expensive.
+   Defaults to `false`.
+
 uploadpack.keepAlive::
When `upload-pack` has started `pack-objects`, there may be a
quiet period while `pack-objects` prepares the pack. Normally
diff --git a/Documentation/technical/http-protocol.txt 
b/Documentation/technical/http-protocol.txt
index 229f845..1c561bd 100644
--- a/Documentation/technical/http-protocol.txt
+++ b/Documentation/technical/http-protocol.txt
@@ -319,7 +319,8 @@ Servers SHOULD support all capabilities defined here.
 Clients MUST send at least one want command in the request body.
 Clients MUST NOT reference an id in a want command which did not
 appear in the response obtained through ref discovery unless the
-server advertises capability `allow-tip-sha1-in-want`.
+server advertises capability `allow-tip-sha1-in-want` or
+`allow-reachable-sha1-in-want`.
 
   compute_request   =  want_list
   have_list
diff --git a/Documentation/technical/protocol-capabilities.txt 
b/Documentation/technical/protocol-capabilities.txt
index 4f8a7bf..265fcab 100644
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -260,6 +260,13 @@ If the upload-pack server advertises this capability, 
fetch-pack may
 send want lines with SHA-1s that exist at the server but are not
 advertised by upload-pack.
 
+allow-reachable-sha1-in-want
+--
+
+If the upload-pack server advertises this capability, fetch-pack may
+send want lines with SHA-1s that exist at the server but are not
+advertised by upload-pack.
+
 push-cert=nonce
 -
 
diff --git a/fetch-pack.c b/fetch-pack.c
index 699f586..0d83d47 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -46,6 +46,8 @@ static struct prio_queue rev_list = { 
compare_commits_by_commit_date };
 static int non_common_revs, multi_ack, use_sideband;
 /* Allow specifying sha1 if it is a ref tip. */
 #define ALLOW_TIP_SHA1 01
+/* Allow request of a sha1 if it is reachable from a ref (possibly hidden 
ref). */
+#define ALLOW_REACHABLE_SHA1   02
 static int allow_unadvertised_object_request;
 
 static void rev_list_push(struct commit *commit, int mark)
@@ -545,7 +547,8 @@ static void filter_refs(struct fetch_pack_args *args,
}
 
/* Append unmatched requests to the list */
-   if ((allow_unadvertised_object_request  ALLOW_TIP_SHA1)) {
+   if ((allow_unadvertised_object_request 
+   (ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1))) {
for (i = 0; i  nr_sought; i++) {
unsigned char sha1[20];
 
@@ -826,6 +829,11 @@ static struct ref *do_fetch_pack(struct fetch_pack_args 
*args,
fprintf(stderr, Server supports 
allow-tip-sha1-in-want\n);
allow_unadvertised_object_request |= ALLOW_TIP_SHA1;
}
+   if 

Re: [PATCH] git-send-email.perl: Add sendmail aliases support

2015-05-21 Thread Eric Sunshine
On Thu, May 21, 2015 at 4:48 PM, Allen Hubbe alle...@gmail.com wrote:
 My motivation for this patch was not really to support the sendmail
 aliases file directly.  The commit message may therefore be
 misleading.  So, I could also rewrite the commit message to say
 something like, loosely based on the sendmail aliases format, if the
 exceptions to the format in the current message are not enough.

That probably would be a good idea. After reading the 'aliases' man
page you cited[1], I was wondering why your parser diverged from it in
so many ways. In addition to lack of support for folded lines, your
format:

* only recognizes comment lines when '#' is the first character,
whereas [1] allows whitespace before '#'

* only recognizes zero-length lines as empty, whereas [1] more loosely
interprets a whitespace-only line as empty

* doesn't support quoting the 'name' part of name: value as [1] does

 Really, I just prefer the simpler alias: email|alias syntax
 instead of the ones for mutt, elm, etc, and that is why I wrote this
 patch.

Your patch is missing a documentation update
(Documentation/git-send-email.txt) and new tests
(t/t9001-send-email.sh).

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


Re: [PATCH 1/2] rebase -i: demonstrate incorrect behavior of post-rewrite hook with exec

2015-05-21 Thread Eric Sunshine
On Thu, May 21, 2015 at 2:13 PM, Matthieu Moy matthieu@imag.fr wrote:
 The 'exec' command is sending the current commit to stopped-sha, which is
 supposed to contain the original commit (before rebase). As a result, if
 an 'exec' command fails, the next 'git rebase --continue' will send the
 current commit as old-sha1 to the post-rewrite hook.
 [...]

 Signed-off-by: Matthieu Moy matthieu@imag.fr
 ---
 diff --git a/t/t5407-post-rewrite-hook.sh b/t/t5407-post-rewrite-hook.sh
 index ea2e0d4..ecef820 100755
 --- a/t/t5407-post-rewrite-hook.sh
 +++ b/t/t5407-post-rewrite-hook.sh
 @@ -212,4 +212,21 @@ EOF
 verify_hook_input
  '

 +test_expect_failure 'git rebase -i (exec)' '
 +   git reset --hard D 
 +   clear_hook_input 
 +   FAKE_LINES=edit 1 exec_false 2 git rebase -i B

Broken -chain.

 +   echo something bar 
 +   git add bar 
 +   # Fail because of exec false
 +   test_must_fail git rebase --continue 
 +   git rebase --continue 
 +   echo rebase expected.args 
 +   cat expected.data EOF 
 +$(git rev-parse C) $(git rev-parse HEAD^)
 +$(git rev-parse D) $(git rev-parse HEAD)
 +EOF
 +   verify_hook_input
 +'
 +
  test_done
 --
 2.4.1.171.g060e6ae.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] git-send-email.perl: Add sendmail aliases support

2015-05-21 Thread Junio C Hamano
Allen Hubbe alle...@gmail.com writes:

 diff --git a/git-send-email.perl b/git-send-email.perl
 index e1e9b14..5f2ec0d 100755
 --- a/git-send-email.perl
 +++ b/git-send-email.perl
 @@ -515,7 +515,12 @@ my %parse_alias = (
  $aliases{$alias} = [ split_addrs($addr) ];
 }
 } },
 -
 + sendmail = sub { my $fh = shift; while ($fh) {
 + next if /^$|^#|^\s/;
 + if (/^(\S+)\s*:\s*(.*?)\\?$/) {
 + my ($alias, $addr) = ($1, $2);
 + $aliases{$alias} = [ split_addrs($addr) ];
 + }}},

Let me unfold the line only to make commenting it easier.

sendmail = sub {
my $fh = shift;
while ($fh) {
next if /^$|^#|^\s/;
if (/^(\S+)\s*:\s*(.*?)\\?$/) {
my ($alias, $addr) = ($1, $2);
$aliases{$alias} = [ split_addrs($addr) ];
}
}
},

It is probably OK to omit support for folded lines, but wouldn't it
be easy enough to be a bit more helpful to give a warning when you
find such lines in the input?  Otherwise you will leave the users
wondering why some of their aliases work while others don't.

Perhaps like this (this is not even an output from diff but typed
in my MUA, so there may be typos---take it just as illustrating
ideas)?

That way, users can fold the input themselves and try again if they
wanted to.  The warning _may_ have to be squelched after a few hits
to keep the result usable, though.

sendmail = sub {
my $fh = shift;
while ($fh) {
-   next if /^$|^#|^\s/;
-   if (/^(\S+)\s*:\s*(.*?)\\?$/) {
+   next if /^$|^#/;
+   if (/^\s/ || /\\$/) {
+   print STDERR $.: $_;
+   print STDERR continuation lines in alias not 
supported\n;
+   next;
+   }
+   if (/^(\S+)\s*:\s*(.*)$/) {
my ($alias, $addr) = ($1, $2);
$aliases{$alias} = [ split_addrs($addr) ];
}
}
},

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


[PATCH v5 1/3] config.txt: clarify allowTipSHA1InWant with camelCase

2015-05-21 Thread Fredrik Medley
Most of the options in config.txt are camelCase. Improve the readability
for allowtipsha1inwant by changing to allowTipSHA1InWant.

Signed-off-by: Fredrik Medley fredrik.med...@gmail.com
---
This patch is optional. There has been work on fixing the whole
Documentation/config.txt which has not been merged yet. When adding
allowReachableSHA1InWant later, it would be good to already have changed
to allowTipSHA1InWant.

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

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 948b8b0..1311383 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2539,9 +2539,9 @@ uploadpack.hideRefs::
are under the hierarchies listed on the value of this
variable is excluded, and is hidden from `git ls-remote`,
`git fetch`, etc.  An attempt to fetch a hidden ref by `git
-   fetch` will fail.  See also `uploadpack.allowtipsha1inwant`.
+   fetch` will fail.  See also `uploadpack.allowTipSHA1InWant`.
 
-uploadpack.allowtipsha1inwant::
+uploadpack.allowTipSHA1InWant::
When `uploadpack.hideRefs` is in effect, allow `upload-pack`
to accept a fetch request that asks for an object at the tip
of a hidden ref (by default, such a request is rejected).
-- 
1.9.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 1/3] l10: de.po: grammar fix

2015-05-21 Thread Ralf Thielow
2015-05-20 10:55 GMT+02:00 Michael J Gruber g...@drmicha.warpmail.net:
 Stefan Beller venit, vidit, dixit 19.05.2015 23:46:
 On Tue, May 19, 2015 at 1:51 AM, Michael J Gruber
 g...@drmicha.warpmail.net wrote:
 Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net
 ---
  po/de.po | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/po/de.po b/po/de.po
 index 2feaec1..25258e3 100644
 --- a/po/de.po
 +++ b/po/de.po
 @@ -10478,7 +10478,7 @@ msgstr 
  #: git-am.sh:142
  msgid Using index info to reconstruct a base tree...
  msgstr 
 -Verwende Informationen aus der Staging-Area, um einen Basisverzeichnis 
 +Verwende Informationen aus der Staging-Area, um ein Basisverzeichnis 
  nachzustellen

 Waere es sinnvoll Staging-Area in Staging-Bereich umzubenennen?
 Staging ist wohl ein eher fester Term mit dem man den index/staging area
 assoziert, aber Area 'doesn't ring a bell for me'.

 Staging-Bereich would be Denglish - half German and halb Englisch.

 In any case, the term staging area is highly confusing in this context:
 It really is the index, and in this case the user has not staged
 anything for commit in the index. The index is used for storing
 information during the 3way merge. This is why we refused to replace
 index/cache by staging area in the original git documentation.

 The de l10n team decided to use Staging-Area as the translation for
 index (as far as I can see), and therefore I kept this term: It is
 important to translate the same concept in the same way consistently in
 all places. Changing the translation of index would be a major
 decision and a major patch. (I'd vote for Index.)


The initial version of the glossary of git.git's de.po has been developed in
thread [1], where Index was part of, initially. We ended up only using
Staging-Area for index, because people might understand Index
as a book's index or database's index [2].

Git itself uses the term staging area *very* rarely. index would be a
Git term we don't translate, but there's also the German word Index
and hence we might confuse users. However, I still think it may only be
confusing for absolute newbies, but give a benefit for users who knows
about git's index. I'll send a patch for this change.

Thanks

[1]
http://thread.gmane.org/gmane.comp.version-control.git/224132/
[2]
http://article.gmane.org/gmane.comp.version-control.git/224974

 Michael
--
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: identical hashes on two branches, but holes in git log

2015-05-21 Thread Philippe De Muyter
On Thu, May 21, 2015 at 08:58:35PM +0100, Philip Oakley wrote:
 From: Philippe De Muyter p...@macq.eu
 To: Junio C Hamano gits...@pobox.com
 Cc: git@vger.kernel.org; Jeff King p...@peff.net; John Keeping 
 j...@keeping.me.uk
 Sent: Thursday, May 21, 2015 8:15 AM
 Subject: Re: identical hashes on two branches, but holes in git log


 On Tue, May 19, 2015 at 03:12:31PM -0700, Junio C Hamano wrote:
 Philippe De Muyter p...@macq.eu writes:

  On Tue, May 19, 2015 at 09:01:10AM -0700, Junio C Hamano wrote:
  Philippe De Muyter p...@macq.eu writes:
 
   Trying to understand, I have eventually done git log on my   
 branch and
   on v3.15 with the following commands :
  
   git log v3.15 --full-history --decorate=short | grep '^commit'   
  /tmp/3.15.commits
   git log --full-history --decorate=short | grep '^commit'
 /tmp/mybranch.commits
 
  Either
 
  git log --oneline v3.15..HEAD ;# show what I have not in  theirs
 
  or
 
  gitk v3.15...HEAD ;# show our differences graphically
 
  This shows the commits in my branch starting from the most recent  
 common point,
  thus my commits, but I see differences in the files not explained  by 
 my commits,
  but by the fact that many older commits (between v3.13 and v3.14)  are 
 missing on
  my branch, but still in both branches I have a commit called v3.14  
 with the
  same hash.  Is that normal ?

 Sorry, cannot parse.  Neither of the above would show files, so just
 about the place where you start talking about I see differences in
 the files, you lost me.

 Look at the other part of the thread, with the discussion with Jeff and 
 John

 The light has come, and what I understand is:

 don't trust the default (ordering) mode of 'git log' :(


 Surely the question now should be What should the man page say that would 
 have explained the default ordering mode in an understandable way, rather 
 than the current misunderstanding?.

 What 'ordering' were you 'trusting' (presuming) anyway? The current default 
 mode doesn't actually say anything about the order anyway (as you've 
 discovered).

I have used 'git log' on the current 'master' branch of the linux kernel
to find at which point in the history a commit - that I know is disruptive
for my work and that I know by name because I have seen it passing on a
mailing list - had been applied.

'git log -decorate=short' showed it happening between v3.14-rc1 and v3.14-rc2,
but after

git checkout v3.14

I did not find the effects of the commit in the files that should have been
affected by the commit.

I expected at least that a commit listed between two tags on the same branch
was really applied to that branch between those two tags.

Philippe


 I surmise this happens only when 'git merge' has been used.

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


Re: git p4 clone - exclude file types

2015-05-21 Thread Luke Diamand

On 21/05/15 21:49, FusionX86 wrote:

I thought about that, but no. The box I'm running git-p4 on has the
following specs:

CentOS 6.6 64bit
1 CPU
8GB RAM
8GB Swap


Can you post the output, with -v added?

$ git-p4 clone //depot/some/dir -v

Also, what is your p4d server version?

$ p4 info

A quick test just cloning a repo with 4 files of 256MB each seems fine, 
FWIW.




It is also on the same physical network as the Perforce server. I
remember seeing someone else complain about this, but I can't find the
article/blog now.


On Wed, May 20, 2015 at 12:49 AM, Luke Diamand l...@diamand.org wrote:

On 19/05/15 08:38, FusionX86 wrote:


Thanks Luke, looks like this does work for excluding files when using
git p4. Great!

Unrelated question...

While using git p4 I have noticed that most of the time the clone/sync
operations hang and I have to keep retrying. The Perforce depot I'm
currently working with is larger than I'd like and has a lot of binary
files which might be the cause. The point it gets to in the clone/sync
is always random and doesn't ever stop on the same files or file
types. Sometimes it'll die soon after starting, but other times it
almost completes and then dies. If I keep retrying, it will eventually
complete. I haven't been able to narrow down the cause, but I do
notice that the git-fast-import stops right as the clone/sync dies.
I'm wondering if git is overwhelmed and terminates. Have you ever seen
this? Any suggestions?



Running out of memory?

Is this on a 32bit or 64bit system? How much virtual memory do you have?

Luke



--
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] pull: handle --log=n

2015-05-21 Thread Junio C Hamano
Paul Tan pyoka...@gmail.com writes:

 So, here's the re-rolled patch.

Sigh, too late.

I thought the previous round was good enough and the patch is
already on 'next'.

If the incremental change is still worth doing on top, please do so.

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


[PATCH v5 2/3] upload-pack: prepare to extend allow-tip-sha1-in-want

2015-05-21 Thread Fredrik Medley
To allow future extensions, e.g. allowing non-tip sha1, replace the
boolean allow_tip_sha1_in_want variable with the flag-style
allow_request_with_bare_object_name variable.

Signed-off-by: Fredrik Medley fredrik.med...@gmail.com
---
 fetch-pack.c  |  9 ++---
 upload-pack.c | 20 +---
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/fetch-pack.c b/fetch-pack.c
index 48526aa..699f586 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -43,7 +43,10 @@ static int marked;
 #define MAX_IN_VAIN 256
 
 static struct prio_queue rev_list = { compare_commits_by_commit_date };
-static int non_common_revs, multi_ack, use_sideband, allow_tip_sha1_in_want;
+static int non_common_revs, multi_ack, use_sideband;
+/* Allow specifying sha1 if it is a ref tip. */
+#define ALLOW_TIP_SHA1 01
+static int allow_unadvertised_object_request;
 
 static void rev_list_push(struct commit *commit, int mark)
 {
@@ -542,7 +545,7 @@ static void filter_refs(struct fetch_pack_args *args,
}
 
/* Append unmatched requests to the list */
-   if (allow_tip_sha1_in_want) {
+   if ((allow_unadvertised_object_request  ALLOW_TIP_SHA1)) {
for (i = 0; i  nr_sought; i++) {
unsigned char sha1[20];
 
@@ -821,7 +824,7 @@ static struct ref *do_fetch_pack(struct fetch_pack_args 
*args,
if (server_supports(allow-tip-sha1-in-want)) {
if (args-verbose)
fprintf(stderr, Server supports 
allow-tip-sha1-in-want\n);
-   allow_tip_sha1_in_want = 1;
+   allow_unadvertised_object_request |= ALLOW_TIP_SHA1;
}
if (!server_supports(thin-pack))
args-use_thin_pack = 0;
diff --git a/upload-pack.c b/upload-pack.c
index 745fda8..726486b 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -35,7 +35,9 @@ static int multi_ack;
 static int no_done;
 static int use_thin_pack, use_ofs_delta, use_include_tag;
 static int no_progress, daemon_mode;
-static int allow_tip_sha1_in_want;
+/* Allow specifying sha1 if it is a ref tip. */
+#define ALLOW_TIP_SHA1 01
+static int allow_unadvertised_object_request;
 static int shallow_nr;
 static struct object_array have_obj;
 static struct object_array want_obj;
@@ -442,8 +444,8 @@ static int get_common_commits(void)
 
 static int is_our_ref(struct object *o)
 {
-   return o-flags 
-   ((allow_tip_sha1_in_want ? HIDDEN_REF : 0) | OUR_REF);
+   int allow_hidden_ref = (allow_unadvertised_object_request  
ALLOW_TIP_SHA1);
+   return o-flags  ((allow_hidden_ref ? HIDDEN_REF : 0) | OUR_REF);
 }
 
 static void check_non_tip(void)
@@ -727,7 +729,8 @@ static int send_ref(const char *refname, const unsigned 
char *sha1, int flag, vo
packet_write(1, %s %s%c%s%s%s%s agent=%s\n,
 sha1_to_hex(sha1), refname_nons,
 0, capabilities,
-allow_tip_sha1_in_want ?  allow-tip-sha1-in-want 
: ,
+(allow_unadvertised_object_request  
ALLOW_TIP_SHA1) ?
+ allow-tip-sha1-in-want : ,
 stateless_rpc ?  no-done : ,
 symref_info.buf,
 git_user_agent_sanitized());
@@ -787,9 +790,12 @@ static void upload_pack(void)
 
 static int upload_pack_config(const char *var, const char *value, void *unused)
 {
-   if (!strcmp(uploadpack.allowtipsha1inwant, var))
-   allow_tip_sha1_in_want = git_config_bool(var, value);
-   else if (!strcmp(uploadpack.keepalive, var)) {
+   if (!strcmp(uploadpack.allowtipsha1inwant, var)) {
+   if (git_config_bool(var, value))
+   allow_unadvertised_object_request |= ALLOW_TIP_SHA1;
+   else
+   allow_unadvertised_object_request = ~ALLOW_TIP_SHA1;
+   } else if (!strcmp(uploadpack.keepalive, var)) {
keepalive = git_config_int(var, value);
if (!keepalive)
keepalive = -1;
-- 
1.9.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] git-send-email.perl: Add sendmail aliases support

2015-05-21 Thread Allen Hubbe
On Thu, May 21, 2015 at 4:19 PM, Junio C Hamano gits...@pobox.com wrote:
 Allen Hubbe alle...@gmail.com writes:

 diff --git a/git-send-email.perl b/git-send-email.perl
 index e1e9b14..5f2ec0d 100755
 --- a/git-send-email.perl
 +++ b/git-send-email.perl
 @@ -515,7 +515,12 @@ my %parse_alias = (
  $aliases{$alias} = [ split_addrs($addr) ];
 }
 } },
 -
 + sendmail = sub { my $fh = shift; while ($fh) {
 + next if /^$|^#|^\s/;
 + if (/^(\S+)\s*:\s*(.*?)\\?$/) {
 + my ($alias, $addr) = ($1, $2);
 + $aliases{$alias} = [ split_addrs($addr) ];
 + }}},

 Let me unfold the line only to make commenting it easier.

 sendmail = sub {
 my $fh = shift;
 while ($fh) {
 next if /^$|^#|^\s/;
 if (/^(\S+)\s*:\s*(.*?)\\?$/) {
 my ($alias, $addr) = ($1, $2);
 $aliases{$alias} = [ split_addrs($addr) ];
 }
 }
 },

 It is probably OK to omit support for folded lines, but wouldn't it
 be easy enough to be a bit more helpful to give a warning when you
 find such lines in the input?  Otherwise you will leave the users
 wondering why some of their aliases work while others don't.

The diff doesn't show enough context to include this comment:

my %parse_alias = (
# multiline formats can be supported in the future
...

I can't be sure the author's intent, but my interpretation is such.
The parsers do not support multiline, even though the format might
allow it by definition.  Another interpretation could be, no multiline
formats allowed, or, the first person to add a multiline format should
remove this comment.

I think the first interpretation is correct, because according to this
script, the mutt format also has continuation lines.  I didn't find a
more authoritative document in my quick search.

http://www.wizzu.com/mutt/checkalias.pl

I suppose at this point it is also worth mentioning that /etc/aliases
doesn't claim to support aliases of aliases, but does support
non-email-addresses like mail directories and pipes.  I don't think
most git users would try to send email directly to a pipe.

My motivation for this patch was not really to support the sendmail
aliases file directly.  The commit message may therefore be
misleading.  So, I could also rewrite the commit message to say
something like, loosely based on the sendmail aliases format, if the
exceptions to the format in the current message are not enough.
Really, I just prefer the simpler alias: email|alias syntax
instead of the ones for mutt, elm, etc, and that is why I wrote this
patch.


 Perhaps like this (this is not even an output from diff but typed
 in my MUA, so there may be typos---take it just as illustrating
 ideas)?

 That way, users can fold the input themselves and try again if they
 wanted to.  The warning _may_ have to be squelched after a few hits
 to keep the result usable, though.

 sendmail = sub {
 my $fh = shift;
 while ($fh) {
 -   next if /^$|^#|^\s/;
 -   if (/^(\S+)\s*:\s*(.*?)\\?$/) {
 +   next if /^$|^#/;
 +   if (/^\s/ || /\\$/) {
 +   print STDERR $.: $_;
 +   print STDERR continuation lines in alias not 
 supported\n;
 +   next;
 +   }
 +   if (/^(\S+)\s*:\s*(.*)$/) {
 my ($alias, $addr) = ($1, $2);
 $aliases{$alias} = [ split_addrs($addr) ];
 }
 }
 },

That's interesting.  I'd like to hear a second opinion before I add
that.  It's a good idea, but none of the other parsing routines print
out messages.


 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-send-email.perl: Add sendmail aliases support

2015-05-21 Thread Junio C Hamano
Allen Hubbe alle...@gmail.com writes:

 The diff doesn't show enough context to include this comment:

 my %parse_alias = (
 # multiline formats can be supported in the future
 ...

 I can't be sure the author's intent, but my interpretation is such.
 The parsers do not support multiline, even though the format might
 allow it by definition.  Another interpretation could be, no multiline
 formats allowed, or, the first person to add a multiline format should
 remove this comment.

I think that comment were only meant to apply to the first one, mutt.

In any case, you should read it this way:

Some formats may support multi-line; this parser back when the
comment was written did not support an alias file that uses such
a feature, but it is OK to make it support them in the future. 

After all, these subs are slurping from $fh and doing the parsing
themselves, so they are free to do multi-line if they wanted to.
It's not like there is a calling function that feeds input line by
line after splitting a logically continued line into two (and if
that were the case, supporting multi-line format may become harder
or even impossible).

And as I said, it is OK not to support ones that have folded lines.

All I was saying was that we should not SILENTLY fail or do a wrong
thing when we find something we do not support.
--
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 v6 2/2] mergetools: add winmerge as a builtin tool

2015-05-21 Thread Johannes Schindelin
Hi Junio,

On 2015-05-20 23:00, Junio C Hamano wrote:
 Sebastian Schuberth sschube...@gmail.com writes:
 
 On Wed, May 20, 2015 at 10:13 PM, Junio C Hamano gits...@pobox.com wrote:

 David Aguilar dav...@gmail.com writes:

 + for directory in $(env | grep -Ei '^PROGRAM(FILES(\(X86\))?|W6432)=' 
 |
 + cut -d '=' -f 2- | sort -u)

 Is the final sort really desired?  I am wondering if there are
 fixed precedence/preference order among variants of %PROGRAMFILES%
 environment variables that the users on the platform are expected
 to stick to, but the sort is sorting by the absolute pathnames of
 where these things are, which may not reflect that order.

 I did add the sort (and -u) by intention, to ensure that C:\Program
 Files (which is what %PROGRAMFILES% expands to by default) comes
 before C:\Program Files (x86) (which is what %PROGRAMFILES(X86)%
 expands to by default), so that programs of the OS-native bitness are
 preferred.
 
 Yuck.  So even though %PROGRAMFILES% and %PROGRAMFILES(X86)% look as
 if they are variables that can point at arbitrary places, they in
 reality don't?  Otherwise %PROGRAMFILES% may point at D:\Program
 while %PROGRAMFILES(X86)% may piont at C:\X86 and the latter would
 sort before the former, which would defeat that logic.

Well, you are correct, theoretically an administrator could set the registry 
values (which are the source of the environment variables in question) of the 
`ProgramFilesDir` key in both

HKEY_LOCAL_MACHINE\Software\Microsoft\Windows\CurrentVersion

and

HKEY_LOCAL_MACHINE\Software\WOW64\Microsoft\Windows\CurrentVersion

to wildly different locations as you outlined. However, it is not supported by 
Microsoft to change those locations via the registry:

https://support.microsoft.com/en-us/kb/933700

Ciao,
Dscho
--
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 10/14] pull: set reflog message

2015-05-21 Thread Paul Tan
On Tue, May 19, 2015 at 5:53 AM, Junio C Hamano gits...@pobox.com wrote:
 Yeah, either that, or insert separator only before adding to
 something else pattern, i.e.

 for (i = 0; i  argc; i++) {
 if (i)
 addch(msg, ' ');
 addstr(msg, argv[i]);
 }

 Both are easier to read than always append SP and trim at the end.
 Besides, trimming at the end makes readers wonder if there are cases
 where argv[argc-1] ends with whitespaces and the code on purpose
 removes them.

Yeah, this looks better and is more correct.

Thanks,
Paul
--
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] pull: handle --log=n

2015-05-21 Thread Paul Tan
On Mon, May 18, 2015 at 10:53 PM, Johannes Schindelin 
johannes.schinde...@gmx.de wrote:
 On 2015-05-18 15:39, Paul Tan wrote:
 diff --git a/t/t5524-pull-msg.sh b/t/t5524-pull-msg.sh
 index 8cccecc..eebb8c9 100755
 --- a/t/t5524-pull-msg.sh
 +++ b/t/t5524-pull-msg.sh
 @@ -17,6 +17,9 @@ test_expect_success setup '
   git commit -m add bfile
   ) 
   test_tick  test_tick 
 + echo second afile 
 + git add afile 
 + git commit -m second commit 
   echo original $dollar afile 
   git add afile 

 I completely forgot to send a mail last time where I wanted to mention the 
 test_commit function. I *think* that above invocation is equivalent to

 test_commit second commit afile second

 with the additional benefit that `test_tick` is called automatically. Maybe 
 you want to use that more concise function call?

Hmm, it says that 'second commit' is not a valid tag name ;-) Okay, I can work 
around it by making the commit message second.

On another note though, I think I need to add a test_tick for the commit just 
below as well:

echo original $dollar afile 
git add afile c
git commit -m do not clobber $dollar signs

So, here's the re-rolled patch.

--- 8 ---
Since efb779f (merge, pull: add '--(no-)log' command line option,
2008-04-06) git-pull supported the (--no-)log switch and would pass it
to git-merge.

96e9420 (merge: Make '--log' an integer option for number of shortlog
entries, 2010-09-08) implemented support for the --log=n switch, which
would explicitly set the number of shortlog entries. However, git-pull
does not recognize this option, and will instead pass it to git-fetch,
leading to unknown option errors.

Fix this by matching --log=* in addition to --log and --no-log.

Implement a test for this use case.

Signed-off-by: Paul Tan pyoka...@gmail.com
---
 git-pull.sh |  4 ++--
 t/t5524-pull-msg.sh | 18 +-
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/git-pull.sh b/git-pull.sh
index 9ed01fd..5ff4545 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -81,8 +81,8 @@ do
diffstat=--no-stat ;;
--stat|--summary)
diffstat=--stat ;;
-   --log|--no-log)
-   log_arg=$1 ;;
+   --log|--log=*|--no-log)
+   log_arg=$1 ;;
--no-c|--no-co|--no-com|--no-comm|--no-commi|--no-commit)
no_commit=--no-commit ;;
--c|--co|--com|--comm|--commi|--commit)
diff --git a/t/t5524-pull-msg.sh b/t/t5524-pull-msg.sh
index 8cccecc..9bc7821 100755
--- a/t/t5524-pull-msg.sh
+++ b/t/t5524-pull-msg.sh
@@ -16,7 +16,9 @@ test_expect_success setup '
test_tick 
git commit -m add bfile
) 
-   test_tick  test_tick 
+   test_tick 
+   test_commit second afile 
+   test_tick 
echo original $dollar afile 
git add afile 
git commit -m do not clobber $dollar signs
@@ -32,4 +34,18 @@ test_expect_success pull '
 )
 '
 
+test_expect_success '--log=1 limits shortlog length' '
+(
+   cd cloned 
+   git reset --hard HEAD^ 
+   test $(cat afile) = original 
+   test $(cat bfile) = added 
+   git pull --log=1 
+   git log -3 
+   git cat-file commit HEAD result 
+   grep Dollar result 
+   ! grep second result
+)
+'
+
 test_done
-- 
2.1.4

--
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 06/14] pull: support pull.ff config

2015-05-21 Thread Paul Tan
Hi,

On Tue, May 19, 2015 at 3:02 AM, Johannes Schindelin
johannes.schinde...@gmx.de wrote:
 Hi Paul,

 On 2015-05-18 17:06, Paul Tan wrote:

 diff --git a/builtin/pull.c b/builtin/pull.c
 index 8982fdf..b305a47 100644
 --- a/builtin/pull.c
 +++ b/builtin/pull.c
 @@ -209,6 +209,28 @@ static void argv_push_force(struct argv_array *arr)
   argv_array_push(arr, -f);
  }

 +/**
 + * If pull.ff is true, returns --ff. If pull.ff is false, returns
 + * --no-ff. If pull.ff is only, returns --ff-only. Otherwise, returns
 + * NULL.
 + */
 +static const char *config_get_ff(void)
 +{
 + const char *value;
 +
 + if (git_config_get_value(pull.ff, value))
 + return NULL;
 + switch (git_config_maybe_bool(pull.ff, value)) {
 + case 0:
 + return --no-ff;
 + case 1:
 + return --ff;
 + }
 + if (!strcmp(pull.ff, only))

 I think you want to test `!strcmp(only, value)` ;-)

Ugh .

Fixed, thanks for catching.

Regards,
Paul
--
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 02/14] pull: pass verbosity, --progress flags to fetch and merge

2015-05-21 Thread Paul Tan
Hi,

On Tue, May 19, 2015 at 1:41 AM, Johannes Schindelin
johannes.schinde...@gmx.de wrote:
 On 2015-05-18 17:05, Paul Tan wrote:
 diff --git a/builtin/pull.c b/builtin/pull.c
 index 0b771b9..a4d9c92 100644
 --- a/builtin/pull.c
 +++ b/builtin/pull.c
 @@ -11,16 +11,64 @@
  #include argv-array.h
  #include run-command.h

 +/**
 + * Given an option opt, where opt-value points to a char* and opt-defval 
 is a
 + * string, sets opt-value to the evaluation of --$defval=$arg. If `arg` 
 is
 + * NULL, then opt-value is set to --$defval. If unset is true, then
 + * opt-value is set to --no-$defval.
 + */
 +static int parse_opt_passthru(const struct option *opt, const char
 *arg, int unset)

 How about adding this to parse-options-cb.c in a separate patch? I guess the 
 description could say something like:

Yeah, I was also thinking if the callback could be generalized as well.

Specifically, I wonder if we should re-construct just the
last-provided option (the current solution), or all the options (e.g.
something like OPT_STRING_LIST). I'm currently working on the rewrite
for git-am.sh as well, and it turns out it makes heavy use of the
latter, so we probably should support both.

 /**
  * Given an option opt, recreate the command-line option, as strbuf. This is 
 useful
  * when a command needs to parse a command-line option in order to pass it to 
 yet
  * another command. This callback can be used in conjunction with the
  * PARSE_OPT_(OPTARG|NOARG|NONEG) options, but not with 
 PARSE_OPT_LASTARG_DEFAULT
  * because it expects `defval` to be the name of the command-line option to
  * reconstruct.
  *
  * The result will be stored in opt-value, which is expected to be a pointer 
 to an
  * strbuf.
  */


Heh, this docstring reads much better than mine.

 Implied by my suggested description, I also propose to put the re-recreated 
 command-line option into a strbuf instead of a char *, to make memory 
 management easier (read: avoid memory leaks).

Unfortunately, the usage of strbuf means that we lose the ability to
know if an option was not provided at all (the value is NULL). This is
important as some of these options are used to override the default
configured behavior.

 You might also want to verify that arg is `NULL` when `unset != 0`. Something 
 like this:

Hmm, would there be a situation where arg is NULL when `unset != 0`?
At first glance in the source code, it won't unless
PARSE_OPT_LASTARG_DEFAULT is set, I guess. Anyway, there's no harm in
being careful, though I think it's more likely that an invalid pointer
is passed in through arg, which we can't detect :-(.

 int parse_opt_passthru(const struct option *opt, const char *arg, int unset)
 {
 struct strbuf *buf = opt-value;

 assert(opt-defval);
 strbuf_reset(buf);
 if (unset) {
 assert(!arg);
 strbuf_addf(buf, --no-%s, opt-defval);
 }
 else {
 strbuf_addf(buf, --%s, opt-defval);
 if (arg)
 strbuf_addf(buf, =%s, arg);
 }

 return 0;
 }

  static struct option pull_options[] = {
 + /* Shared options */
 + OPT__VERBOSITY(opt_verbosity),
 + { OPTION_CALLBACK, 0, progress, opt_progress, NULL,
 +   N_(force progress reporting),
 +   PARSE_OPT_NOARG, parse_opt_passthru},

 I *think* there is a '(intptr_t) progress' missing...

Ugh. I think this shows that relying on defval is too error-prone.
Anyway, it is only required for the options -n (which maps to
--no-stat), and --summary (which maps to --stat), so we should
probably make it such that if defval is not provided, we fallback to
using long_name or short_name.

  /**
 + * Pushes -q or -v switches into arr to match the opt_verbosity level.
 + */
 +static void argv_push_verbosity(struct argv_array *arr)
 +{
 + int verbosity;
 +
 + for (verbosity = opt_verbosity; verbosity  0; verbosity--)
 + argv_array_push(arr, -v);
 +
 + for (verbosity = opt_verbosity; verbosity  0; verbosity++)
 + argv_array_push(arr, -q);
 +}

 Hmm... I guess this is *really* nit-picky, but I'd rather use i instead of 
 verbosity because the second loop is about quietness instead of verbosity 
 ;-)

Well, my thinking was that a negative verbosity *is* quietness ;-).

Thanks,
Paul
--
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: recovering from unordered stage entries in index error

2015-05-21 Thread Duy Nguyen
On Tue, May 19, 2015 at 8:48 PM, McHenry, Matt
mmche...@carnegielearning.com wrote:


 I've just upgraded my git from 2.0.5 to 2.3.6, and I'm now unable to 
 run 'git svn fetch' in one of my repositories:

 $ git svn fetch
 fatal: unordered stage entries in index

This message can be improved to show what entries have this problem.
But then I don't see any way to recover the index manually. ls-files
will die too. Perhaps we should be gentle in this case: show warnings
instead of aborting the program and internally reorder the index. I
think, unless you have multiple entries with the same stage, the
recovered index should run well. The broken index could be renamed to
index.broken or something for later analysis, or we forbid writing the
reordered index to disk.

Hmm?

 write-tree: command returned error: 128

 'git status' shows a few untracked files but is otherwise clean.

 It looks like this check was introduced in 
 15999d0be8179fb7a2e6eafb931d25ed65df50aa, with the summary 
 read_index_from(): catch out of order entries when reading an index file 
 (first appearing in 2.2.0).

 Mailing list discussion looked like it implicated third-party tools.  
 I don't recall running any other tools on this repo; it doesn't do much 
 day-to-day other than a long series of 'git svn fetch'es.  (But it's been 
 around for a couple of years, so who knows.)

 At any rate, what can I do to recover from this situation?  I tried 
 to locate a path with multiple index entries like this, but got no results:

 $ git ls-files -s | cut -f 2-100 | sort | uniq -c | grep -v '^[ \t]*1 '

 (I originally posted on SO at 
 http://stackoverflow.com/questions/30264826/; I'll update that with any 
 solutions that come up here, to ease future googling.)
 --
 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




-- 
Duy
--
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 v10 5/5] help: respect new common command grouping

2015-05-21 Thread Eric Sunshine
On Thu, May 21, 2015 at 9:13 AM, Sébastien Guimmara
sebastien.guimm...@gmail.com wrote:
 'git help' shows common commands in alphabetical order:
 [...]
 without any indication of how commands relate to high-level
 concepts or each other. Revise the output to explain their relationship
 with the typical Git workflow:
 [...]
 Signed-off-by: Sébastien Guimmara sebastien.guimm...@gmail.com
 ---
 diff --git a/help.c b/help.c
 index 2072a87..8f72051 100644
 --- a/help.c
 +++ b/help.c
 @@ -218,17 +218,39 @@ void list_commands(unsigned int colopts,
  void list_common_cmds_help(void)
  {
 int i, longest = 0;
 +   int current_grp = -1;

 for (i = 0; i  ARRAY_SIZE(common_cmds); i++) {
 if (longest  strlen(common_cmds[i].name))
 longest = strlen(common_cmds[i].name);
 }

 -   puts(_(The most commonly used git commands are:));
 +   qsort(common_cmds, ARRAY_SIZE(common_cmds),
 +   sizeof(common_cmds[0]), cmd_group_cmp);
 +
 +   puts(_(These are common Git commands used in various situations:));

The clause in various situations is quite nebulous and thus adds no
substance. If you remove it, then you're effectively left with the
original

The most commonly used git commands are:

which reads just as well or better and has the attribute of being more
concise. I'd opt to drop this change and just keep the original
wording.

Other than that minor observation, the patch looks fine.

 +
 for (i = 0; i  ARRAY_SIZE(common_cmds); i++) {
 +   if (common_cmds[i].group != current_grp) {
 +   printf(\n%s\n, 
 _(common_cmd_groups[common_cmds[i].group]));
 +   current_grp = common_cmds[i].group;
 +   }
 +
 printf(   %s   , common_cmds[i].name);
 mput_char(' ', longest - strlen(common_cmds[i].name));
 puts(_(common_cmds[i].help));
 --
 2.4.0.GIT
--
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: Troubleshoot clone issue to NFS.

2015-05-21 Thread Duy Nguyen
On Thu, May 21, 2015 at 9:31 PM, Duy Nguyen pclo...@gmail.com wrote:
 In case an object is not found pack directory
 is re-read again, which might cause some increased load on nfs.
 has_sha1_file() not finding the object should not happen often..

That last statement is probably very wrong, but I have no time to test
this now. In index-pack, there is a has_sha1_file() for file collision
test. That call on a fresh clone would fail for _every_ object in the
(new) pack and the cost of reprepare pack files could be sky high...
-- 
Duy
--
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: Troubleshoot clone issue to NFS.

2015-05-21 Thread Duy Nguyen
On Thu, May 21, 2015 at 8:13 PM,  steve.nor...@thomsonreuters.com wrote:

 So there appears to be a change in 1.8.4.2 that made this issue appear for 
 me.  Looking at the release notes the only thing that I can see that might be 
 related could be:

 * When an object is not found after checking the packfiles and then
loose object directory, read_sha1_file() re-checks the packfiles to
prevent racing with a concurrent repacker; teach the same logic to
has_sha1_file().

That would be commit 45e8a74 (has_sha1_file: re-check pack directory
before giving up - 2013-08-30). Maybe you can try the version without
that commit to confirm. In case an object is not found pack directory
is re-read again, which might cause some increased load on nfs.
has_sha1_file() not finding the object should not happen often.. Maybe
you could do an strace (following forks) on the git clone process to
see if readdir() is really called a lot?
-- 
Duy
--
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 v10 1/5] command-list: prepare machinery for upcoming common groups section

2015-05-21 Thread Eric Sunshine
On Thu, May 21, 2015 at 9:13 AM, Sébastien Guimmara
sebastien.guimm...@gmail.com wrote:
 From: Eric Sunshine sunsh...@sunshineco.com

 The ultimate goal is for git help to classify common commands by
 group. Toward this end, a subsequent patch will add a new common
 groups section to command-list.txt preceding the actual command list.
 As preparation, teach existing command-list.txt parsing machinery, which
 doesn't care about grouping, to skip over this upcoming common groups
 section.

 Signed-off-by: Eric Sunshine sunsh...@sunshineco.com
 Signed-off-by: Sébastien Guimmara sebastien.guimm...@gmail.com
 ---
 diff --git a/Documentation/cmd-list.perl b/Documentation/cmd-list.perl
 index 04f9977..5aa73cf 100755
 --- a/Documentation/cmd-list.perl
 +++ b/Documentation/cmd-list.perl
 @@ -38,6 +38,10 @@ sub format_one {
 }
  }

 +while () {
 +   last if /^### command list/;
 +}
 +
  my %cmds = ();
  for (sort ) {
 next if /^#/;
 diff --git a/Documentation/howto/new-command.txt 
 b/Documentation/howto/new-command.txt
 index d7de5a3..6d772bd 100644
 --- a/Documentation/howto/new-command.txt
 +++ b/Documentation/howto/new-command.txt
 @@ -95,7 +95,9 @@ your language, document it in the INSTALL file.
  that categorizes commands by type, so they can be listed in appropriate
  subsections in the documentation's summary command list.  Add an entry
  for yours.  To understand the categories, look at git-commands.txt
 -in the main directory.
 +in the main directory.  If the new command is part of the typical Git
 +workflow and you believe it common enough to be mentioned in 'git help',
 +map this command to a common group in the column [common].

I think you meant to squash the documentation update into patch 2/5
where the common groups block is actually introduced. It doesn't
really belong in this patch which is about updating machinery in
preparation for the new block.

Also, it's now spelled ### common groups rather than [common].

  7. Give the maintainer one paragraph to include in the RelNotes file
  to describe the new feature; a good place to do so is in the cover
 diff --git a/Makefile b/Makefile
 index 323c401..655740d 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -2455,7 +2455,7 @@ check-docs::
 esac ; \
 test -f Documentation/$$v.txt || \
 echo no doc: $$v; \
 -   sed -e '/^#/d' command-list.txt | \
 +   sed -e '1,/^### command list/d' -e '/^#/d' command-list.txt | 
 \
 grep -q ^$$v[  ] || \
 case $$v in \
 git) ;; \
 @@ -2463,7 +2463,8 @@ check-docs::
 esac ; \
 done; \
 ( \
 -   sed -e '/^#/d' \
 +   sed -e '1,/^### command list/d' \
 +   -e '/^#/d' \
 -e 's/[ ].*//' \
 -e 's/^/listed /' command-list.txt; \
 $(MAKE) -C Documentation print-man1 | \
 diff --git a/command-list.txt b/command-list.txt
 index 54d8d21..181a9c2 100644
 --- a/command-list.txt
 +++ b/command-list.txt
 @@ -1,4 +1,4 @@
 -# List of known git commands.
 +### command list (do not change this line)
  # command name  category [deprecated] [common]
  git-add mainporcelain common
  git-am  mainporcelain
 --
 2.4.0.GIT
--
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 v10 1/5] command-list: prepare machinery for upcoming common groups section

2015-05-21 Thread Sébastien Guimmara



On 05/21/2015 03:48 PM, Eric Sunshine wrote:

On Thu, May 21, 2015 at 9:13 AM, Sébastien Guimmara
sebastien.guimm...@gmail.com wrote:

From: Eric Sunshine sunsh...@sunshineco.com

The ultimate goal is for git help to classify common commands by
group. Toward this end, a subsequent patch will add a new common
groups section to command-list.txt preceding the actual command list.
As preparation, teach existing command-list.txt parsing machinery, which
doesn't care about grouping, to skip over this upcoming common groups
section.

Signed-off-by: Eric Sunshine sunsh...@sunshineco.com
Signed-off-by: Sébastien Guimmara sebastien.guimm...@gmail.com
---
diff --git a/Documentation/cmd-list.perl b/Documentation/cmd-list.perl
index 04f9977..5aa73cf 100755
--- a/Documentation/cmd-list.perl
+++ b/Documentation/cmd-list.perl
@@ -38,6 +38,10 @@ sub format_one {
 }
  }

+while () {
+   last if /^### command list/;
+}
+
  my %cmds = ();
  for (sort ) {
 next if /^#/;
diff --git a/Documentation/howto/new-command.txt 
b/Documentation/howto/new-command.txt
index d7de5a3..6d772bd 100644
--- a/Documentation/howto/new-command.txt
+++ b/Documentation/howto/new-command.txt
@@ -95,7 +95,9 @@ your language, document it in the INSTALL file.
  that categorizes commands by type, so they can be listed in appropriate
  subsections in the documentation's summary command list.  Add an entry
  for yours.  To understand the categories, look at git-commands.txt
-in the main directory.
+in the main directory.  If the new command is part of the typical Git
+workflow and you believe it common enough to be mentioned in 'git help',
+map this command to a common group in the column [common].


I think you meant to squash the documentation update into patch 2/5
where the common groups block is actually introduced. It doesn't
really belong in this patch which is about updating machinery in
preparation for the new block.


I don't mind squashing it with another commit, but in this case, wouldn't it
make more sense to squash it with 4/5, when the 'common' tag is removed and the
file is in its final form ?



Also, it's now spelled ### common groups rather than [common].



actually, this [common] is not the one I added in a previous series,
but the one that was already present:

# command name  category [deprecated] [common]


  7. Give the maintainer one paragraph to include in the RelNotes file
  to describe the new feature; a good place to do so is in the cover
diff --git a/Makefile b/Makefile
index 323c401..655740d 100644
--- a/Makefile
+++ b/Makefile
@@ -2455,7 +2455,7 @@ check-docs::
 esac ; \
 test -f Documentation/$$v.txt || \
 echo no doc: $$v; \
-   sed -e '/^#/d' command-list.txt | \
+   sed -e '1,/^### command list/d' -e '/^#/d' command-list.txt | \
 grep -q ^$$v[  ] || \
 case $$v in \
 git) ;; \
@@ -2463,7 +2463,8 @@ check-docs::
 esac ; \
 done; \
 ( \
-   sed -e '/^#/d' \
+   sed -e '1,/^### command list/d' \
+   -e '/^#/d' \
 -e 's/[ ].*//' \
 -e 's/^/listed /' command-list.txt; \
 $(MAKE) -C Documentation print-man1 | \
diff --git a/command-list.txt b/command-list.txt
index 54d8d21..181a9c2 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -1,4 +1,4 @@
-# List of known git commands.
+### command list (do not change this line)
  # command name  category [deprecated] [common]
  git-add mainporcelain common
  git-am  mainporcelain
--
2.4.0.GIT

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


Troubleshoot clone issue to NFS.

2015-05-21 Thread steve.norman
In setting up some new git servers I was trying to test the performance of some 
NFS mounted volumes and when compared to local disk (although this is a 
vitualized server so not truly local) cloning to NFS was taking a long time.

Here are some timings:

~ $ time bin/git clone https://github.com/git/git test
Cloning into 'test'...
remote: Counting objects: 185964, done.
remote: Compressing objects: 100% (276/276), done.
remote: Total 185964 (delta 203), reused 32 (delta 32), pack-reused 185656
Receiving objects: 100% (185964/185964), 61.42 MiB | 26.16 MiB/s, done.
Resolving deltas: 100% (135454/135454), done.
Checking connectivity... done.

real0m8.156s
user0m10.569s
sys 0m3.857s

~ $ time bin/git clone https://github.com/git/git /sami/test
Cloning into '/sami/test'...
remote: Counting objects: 185964, done.
remote: Compressing objects: 100% (276/276), done.
remote: Total 185964 (delta 203), reused 32 (delta 32), pack-reused 185656
Receiving objects: 100% (185964/185964), 61.42 MiB | 10.15 MiB/s, done.
Resolving deltas: 100% (135454/135454), done.
Checking connectivity... done.
Checking out files: 100% (2795/2795), done.

real0m58.685s
user0m12.153s
sys 0m7.619s

So cloning to NFS is 50 seconds slower on average (I've run this a lot over the 
last few days and it does appear to be consistent and not a network / github 
connectivity issue).  Tests creating files on the NFS with dd didn't show that 
much difference to the local disk (and were sometimes quicker).

Volume mount differences are:

/dev/mapper/rootvg-homelv on /home type ext4 (rw,nodev)
nfsserver:/vol/sami/repos on /sami type nfs 
(rw,bg,nfsvers=3,tcp,hard,nointr,timeo=600,rsize=32768,wsize=32768,addr=10.1.1.1)

And there doesn't appear to be any issue with NFS retransmissions:

/sami $ nfsstat
Client rpc stats:
calls  retransauthrefrsh
11719847   0  11720190

This morning I did some more digging as when I tried this on a newly build 
server the NFS times were slower than local disk, but only buy around 6-10 
seconds.  The new server had git 1.7.1  installed on it compared to 2.4.0 on 
the machine I've been testing on.  So I build a number of versions of git to 
test each one to try and find the point where it changed:

v1.8.0  11.363 s
v1.8.3  13.597 s
v1.8.4  13.958 s
v1.8.4.114.563 s
v1.8.4.21m 0s
v1.8.4.31m 9s
v1.8.4.51m 1s
v1.8.5  1m 0s
v1.8.5.61m 0s
v1.9.0  1m 38

v2.4.0  58s
v2.4.1  58s

So there appears to be a change in 1.8.4.2 that made this issue appear for me.  
Looking at the release notes the only thing that I can see that might be 
related could be:

* When an object is not found after checking the packfiles and then
   loose object directory, read_sha1_file() re-checks the packfiles to
   prevent racing with a concurrent repacker; teach the same logic to
   has_sha1_file().

So the questions are:

1) Should I expect a clone to NFS to be that much slower?
2) Is there anything I could do to speed it up (I've tried --bare as that is 
what the repositories will be when stored on NFS and there wasn't really a 
difference).
3) What else can I use in git to compare the performance on local to NFS to see 
if it is just clones that are affected?
4) I assume I could bisect between 1.8.4.1 and 1.8.4.2 to find exactly where 
things get worse for me?

Thanks for any help,

Steve



This e-mail is for the sole use of the intended recipient and contains 
information that may be privileged and/or confidential. If you are not an 
intended recipient, please notify the sender by return e-mail and delete this 
e-mail and any attachments. Certain required legal entity disclosures can be 
accessed on our website.http://thomsonreuters.com/prof_disclosures/
--
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: Troubleshoot clone issue to NFS.

2015-05-21 Thread Christian Couder
On Thu, May 21, 2015 at 3:13 PM,  steve.nor...@thomsonreuters.com wrote:

[...]

 So the questions are:

 1) Should I expect a clone to NFS to be that much slower?
 2) Is there anything I could do to speed it up (I've tried --bare as that is 
 what the repositories will be when stored on NFS and there wasn't really a 
 difference).
 3) What else can I use in git to compare the performance on local to NFS to 
 see if it is just clones that are affected?
 4) I assume I could bisect between 1.8.4.1 and 1.8.4.2 to find exactly where 
 things get worse for me?

Yeah, it would be very nice if you could bisect further.
You can probably do it automatically using a script such as the one
Junio used in this email:

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

Thanks,
Christian.
--
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 v10 1/5] command-list: prepare machinery for upcoming common groups section

2015-05-21 Thread Eric Sunshine
On Thu, May 21, 2015 at 9:55 AM, Sébastien Guimmara
sebastien.guimm...@gmail.com wrote:
 On 05/21/2015 03:48 PM, Eric Sunshine wrote:
 On Thu, May 21, 2015 at 9:13 AM, Sébastien Guimmara
 sebastien.guimm...@gmail.com wrote:
 The ultimate goal is for git help to classify common commands by
 group. Toward this end, a subsequent patch will add a new common
 groups section to command-list.txt preceding the actual command list.
 As preparation, teach existing command-list.txt parsing machinery, which
 doesn't care about grouping, to skip over this upcoming common groups
 section.

 Signed-off-by: Eric Sunshine sunsh...@sunshineco.com
 Signed-off-by: Sébastien Guimmara sebastien.guimm...@gmail.com
 ---
 @@ -95,7 +95,9 @@ your language, document it in the INSTALL file.
   that categorizes commands by type, so they can be listed in appropriate
   subsections in the documentation's summary command list.  Add an entry
   for yours.  To understand the categories, look at git-commands.txt
 -in the main directory.
 +in the main directory.  If the new command is part of the typical Git
 +workflow and you believe it common enough to be mentioned in 'git help',
 +map this command to a common group in the column [common].

 I think you meant to squash the documentation update into patch 2/5
 where the common groups block is actually introduced. It doesn't
 really belong in this patch which is about updating machinery in
 preparation for the new block.

 I don't mind squashing it with another commit, but in this case, wouldn't it
 make more sense to squash it with 4/5, when the 'common' tag is removed and
 the file is in its final form ?

In my mind, the most logical point at which the documentation should
start talking about the new common coups is when common groups
actually comes into existence since the new documentation is directly
related to birth of that new section of the file. The documentation
update is, at best, only very peripherally related to removal of the
old 'common' tag, so it doesn't really seem logical to tie the
documentation update to 'common' removal in 4/5. But that's just my
opinion...

 Also, it's now spelled ### common groups rather than [common].

 actually, this [common] is not the one I added in a previous series,
 but the one that was already present:

 # command name  category [deprecated] [common]

Ah, right. Thanks for clarifying.
--
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-send-email.perl: Add sendmail aliases support

2015-05-21 Thread Allen Hubbe
On Thu, May 21, 2015 at 5:38 PM, Junio C Hamano gits...@pobox.com wrote:
 Allen Hubbe alle...@gmail.com writes:

 Those are good points.  Maybe I shouldn't even mention sendmail at
 all, not in the name of the format, and not in the commit message.
 What name would be a good name for this format?

 simple?

Alright, as long as people agree that _this_ is the simple format, and
not something else.  I'll call it simple in v2, and see if anyone
complains.


 And if you are going to define such a format, then I do not think
 you would even need to pretend that someday you might support
 line folding (hence there is no need for eh, excuse me, you seem to
 have wanted to express a long folded line here, but I do not support
 it (yet) warning messages).

--
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: Occasional wrong behavior of rev-walking (rev-list, log, etc.)

2015-05-21 Thread Mike Hommey
On Thu, May 21, 2015 at 09:41:55AM -0700, Junio C Hamano wrote:
 Mike Hommey m...@glandium.org writes:
 
  My guess is that rev-walking is tripping on the fact that this repository
  has commit dates in random order.
 
 Yeah, that is well known (look for SLOP both in the code and list
 archive).

I found the recent thread about git describe --contains. (and now I
realize this is also why git describe --contains doesn't work quite well
for the same repository).

Now the question is what can we done in the short term? (short of
introducing something like generation numbers) I tried increasing
SLOP and to remove the date check (with the hope that making one or
both configurable might help). Neither fixed the particular test
case I started this thread with.

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


Re: [PATCH v5 2/3] upload-pack: prepare to extend allow-tip-sha1-in-want

2015-05-21 Thread Junio C Hamano
Fredrik Medley fredrik.med...@gmail.com writes:

 To allow future extensions, e.g. allowing non-tip sha1, replace the
 boolean allow_tip_sha1_in_want variable with the flag-style
 allow_request_with_bare_object_name variable.

 Signed-off-by: Fredrik Medley fredrik.med...@gmail.com
 ---
  fetch-pack.c  |  9 ++---
  upload-pack.c | 20 +---
  2 files changed, 19 insertions(+), 10 deletions(-)

 diff --git a/fetch-pack.c b/fetch-pack.c
 index 48526aa..699f586 100644
 --- a/fetch-pack.c
 +++ b/fetch-pack.c
 @@ -43,7 +43,10 @@ static int marked;
  #define MAX_IN_VAIN 256
  
  static struct prio_queue rev_list = { compare_commits_by_commit_date };
 -static int non_common_revs, multi_ack, use_sideband, allow_tip_sha1_in_want;
 +static int non_common_revs, multi_ack, use_sideband;
 +/* Allow specifying sha1 if it is a ref tip. */
 +#define ALLOW_TIP_SHA1   01
 +static int allow_unadvertised_object_request;

It is better to use unsigned int for these bit masks, as we are
not interested in the top-most bit getting special-cased by using a
signed type.  I'll amend this (and the one in upload-pack.c) while
applying, so no need to resend only to correct these two, unless you
have other reasons to reroll.

Thanks.

 diff --git a/upload-pack.c b/upload-pack.c
 index 745fda8..726486b 100644
 --- a/upload-pack.c
 +++ b/upload-pack.c
 @@ -35,7 +35,9 @@ static int multi_ack;
  static int no_done;
  static int use_thin_pack, use_ofs_delta, use_include_tag;
  static int no_progress, daemon_mode;
 -static int allow_tip_sha1_in_want;
 +/* Allow specifying sha1 if it is a ref tip. */
 +#define ALLOW_TIP_SHA1   01
 +static int allow_unadvertised_object_request;

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


Re: [PATCH] submodule documentation: Reorder introductory paragraphs

2015-05-21 Thread Stefan Beller
On Thu, May 21, 2015 at 1:03 PM, Philip Oakley philipoak...@iee.org wrote:
 +Submodules are not to be confused with remotes, which are meant
 +mainly for branches of the same project;


 This use of 'branches' didn't work for me. remotes are meant mainly for
 branches of the same project ?


Maybe

Submodules should not be confused with remote repositories, which are
meant to track the same repository, just at another location; ...

? Though I'm not yet completely happy with that either.
--
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: Occasional wrong behavior of rev-walking (rev-list, log, etc.)

2015-05-21 Thread Junio C Hamano
Depends on why you are running rev-list.

If you want to know if one commit is contained in another, the way
that should work the most reliably is to use merge-base, as the
traversal engine of that command was written not to trust the commit
timestamps but go with the topology alone.

(pardon top-posting and overlong lines; typed in GMail)


On Thu, May 21, 2015 at 3:49 PM, Mike Hommey m...@glandium.org wrote:
 On Thu, May 21, 2015 at 09:41:55AM -0700, Junio C Hamano wrote:
 Mike Hommey m...@glandium.org writes:

  My guess is that rev-walking is tripping on the fact that this repository
  has commit dates in random order.

 Yeah, that is well known (look for SLOP both in the code and list
 archive).

 I found the recent thread about git describe --contains. (and now I
 realize this is also why git describe --contains doesn't work quite well
 for the same repository).

 Now the question is what can we done in the short term? (short of
 introducing something like generation numbers) I tried increasing
 SLOP and to remove the date check (with the hope that making one or
 both configurable might help). Neither fixed the particular test
 case I started this thread with.

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


Re: [PATCH v5 3/3] upload-pack: optionally allow fetching reachable sha1

2015-05-21 Thread Junio C Hamano
Fredrik Medley fredrik.med...@gmail.com writes:

 --- a/Documentation/technical/protocol-capabilities.txt
 +++ b/Documentation/technical/protocol-capabilities.txt
 @@ -260,6 +260,13 @@ If the upload-pack server advertises this capability, 
 fetch-pack may
  send want lines with SHA-1s that exist at the server but are not
  advertised by upload-pack.
  
 +allow-reachable-sha1-in-want
 +--

This is an underline applied to one line prior, and their length
must match.  I'll amend while applying (attached at end), so there
is no need to resend with correction unless you have other reasons
to do so.

 diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
 index 8a5f236..fdcc114 100755
 --- a/t/t5516-fetch-push.sh
 +++ b/t/t5516-fetch-push.sh
 @@ -1120,6 +1120,61 @@ test_expect_success 'fetch exact SHA1' '
   )
  '

It looks like this new set of tests are well thought out; good job.

I spotted a few minor nits, though.  All I'll amend while applying
so there is no need to resend only to correct them.

 +for configallowtipsha1inwant in true false
 +do
 + test_expect_success shallow fetch reachable SHA1 (but not a ref), 
 allowtipsha1inwant=$configallowtipsha1inwant '
 + mk_empty testrepo 
 + (
 + cd testrepo 
 + git config uploadpack.allowtipsha1inwant 
 $configallowtipsha1inwant 
 + git commit --allow-empty -m foo 
 + git commit --allow-empty -m bar
 + ) 
 + SHA1=$(git --git-dir=testrepo/.git rev-parse HEAD^) 
 + mk_empty shallow 
 + (
 + cd shallow 
 + test_must_fail git fetch --depth=1 ../testrepo/.git 
 $SHA1 

This tries to fetch one before the tip with allowTipSHA1InWant set
to true or false; either should fail.  Good.

 + git --git-dir=../testrepo/.git config 
 uploadpack.allowreachablesha1inwant true 
 + git fetch --depth=1 ../testrepo/.git $SHA1 

And regardless of allowTip setting, with allowReachable set to true,
fetching the reachable HEAD^ would succeed.  Good.

 + git cat-file commit $SHA1 /dev/null

Minor nit; drop /dev/null, as test framework will squelch the
output by default, and when the test is run with -v option, the
output would help debugging the script.

 + )
 + '
 +
 + test_expect_success deny fetch unreachable SHA1, 
 allowtipsha1inwant=$configallowtipsha1inwant '
 + mk_empty testrepo 
 + (
 + cd testrepo 
 + git config uploadpack.allowtipsha1inwant 
 $configallowtipsha1inwant 
 + git commit --allow-empty -m foo 
 + git commit --allow-empty -m bar 
 + git commit --allow-empty -m xyz
 + )

Broken  chain

 + SHA1_1=$(git --git-dir=testrepo/.git rev-parse HEAD^^) 
 + SHA1_2=$(git --git-dir=testrepo/.git rev-parse HEAD^) 
 + SHA1_3=$(git --git-dir=testrepo/.git rev-parse HEAD) 
 + (
 + cd testrepo 
 + git reset --hard $SHA1_2 

We have one before the tip (SHA1_1), one after the tip and no longer
reachable (SHA1_3); SHA1_2 is sitting at the tip of a ref.

 + git cat-file commit $SHA1_3 /dev/null 
 + git cat-file commit $SHA1_3 /dev/null

I think one of the latter two is $SHA1_1, i.e. you make sure SHA1_{1,2,3}
are there in testrepo.

 + ) 
 + mk_empty shallow 
 + (
 + cd shallow 
 + test_must_fail git fetch ../testrepo/.git $SHA1_3 
 + test_must_fail git fetch ../testrepo/.git $SHA1_1 

With allowTip only, whether it is set to true or false, fetching _1
or _3 that are not at tip will fail.  Good.

 + git --git-dir=../testrepo/.git config 
 uploadpack.allowreachablesha1inwant true 
 + git fetch ../testrepo/.git $SHA1_1 
 + git cat-file commit $SHA1_1 /dev/null 

With allowReachable, _1 which is reachable from tip is possible,
regardless of the setting of allowTip.  Good.

 + test_must_fail git cat-file commit $SHA1_2 /dev/null 

And fetching _1 will not pull in _2, which is _1's child, that we
did not ask for.  Good (but it is probably not very relevant for the
purpose of these tests).

 + git fetch ../testrepo/.git $SHA1_2 
 + git cat-file commit $SHA1_2 /dev/null 

And of course, _2 can be fetched.

 + test_must_fail git fetch ../testrepo/.git $SHA1_3

And _3 that is not reachable cannot.

 + )
 + '
 +done

Again, very carefully covering combinations.  Good.




 Documentation/technical/protocol-capabilities.txt |  2 +-
 t/t5516-fetch-push.sh | 14 

Re: Troubleshoot clone issue to NFS.

2015-05-21 Thread Duy Nguyen
On Thu, May 21, 2015 at 10:53 PM,  steve.nor...@thomsonreuters.com wrote:
 On Thu, May 21, 2015a at 9:31 PM, Duy Nguyen [mailto:pclo...@gmail.com], did 
 scribble:
  In case an object is not found pack directory is re-read again, which
  might cause some increased load on nfs.
  has_sha1_file() not finding the object should not happen often..

 That last statement is probably very wrong, but I have no time to test this
 now. In index-pack, there is a has_sha1_file() for file collision test. That 
 call
 on a fresh clone would fail for _every_ object in the
 (new) pack and the cost of reprepare pack files could be sky high...

 Confirmed with bisect that it is that commit:

 ~/git $ git bisect bad
 45e8a7487339c0f0ea28244ef06851308d07387c is the first bad commit
 commit 45e8a7487339c0f0ea28244ef06851308d07387c
 Author: Jeff King p...@peff.net
 Date:   Fri Aug 30 15:14:13 2013 -0400

 I have an strace for that build but it is 153 megabytes so I probably 
 shouldn't attach, but the call summary is below:

 % time seconds  usecs/call callserrors syscall
 -- --- --- - - 
  93.71   39.670084 103386835  2171 futex
   3.161.338572   7190550   181 open
   1.560.658786  18 37450 3 read
   0.620.262740   2141390   pread
   0.410.171526   5 37313 9 write
   0.180.076166   0188859188835 access
   0.110.047941   0374712   getdents
   0.110.045174  11  4067  3910 lstat
   0.060.023630   0190425   close
   0.040.017995   6  2975 1 fstat
   0.020.0076681917 4   wait4
   0.010.004150   1  5065   madvise
   0.010.003548   0 16090 8 recvfrom
   0.000.001872   0  8048   select
   0.000.001870  11   173 1 mkdir
   0.000.000872   0  8055   poll
   0.000.000262  221212 readlink
   0.000.000185   0  1217  1146 stat
   0.000.000158   0   457   mprotect
   0.000.74   0   298   mmap
   0.000.69   1   109 8 rt_sigreturn
   0.000.47   0   159   brk
   0.000.21   117   getcwd
   0.000.00   042 3 lseek
   0.000.00   092   munmap
   0.000.00   035   rt_sigaction
   0.000.00   0 9   rt_sigprocmask
   0.000.00   0 8 3 ioctl
   0.000.00   011   pipe
   0.000.00   0 3   dup
   0.000.00   0 8   dup2
   0.000.00   0 6   setitimer
   0.000.00   011 1 socket
   0.000.00   0 8 7 connect
   0.000.00   0 8   sendto
   0.000.00   0 2   recvmsg
   0.000.00   0 1   bind
   0.000.00   0 1   getsockname
   0.000.00   0 3 1 getpeername
   0.000.00   0 2   setsockopt
   0.000.00   0 2   getsockopt
   0.000.00   0 8   clone
   0.000.00   0 5   execve
   0.000.00   0 3   uname
   0.000.00   0   100   fcntl
   0.000.00   0 2   fsync
   0.000.00   013   chdir
   0.000.00   014   rename
   0.000.00   0 2   link
   0.000.00   0 5   unlink
   0.000.00   0 2   symlink
   0.000.00   0 9   chmod
   0.000.00   0 6   getrlimit
   0.000.00   0 2   sysinfo
   0.000.00   0 8   getuid
   0.000.00   0 1   statfs
   0.000.00   0 5   arch_prctl
   0.000.00   0 1   gettid
   0.000.00   0 5   set_tid_address
   0.000.00   013   set_robust_list
 -- --- --- - - 
 100.00   42.333410   1594736196300 total

 Is there anything else I can provide or test?

In builtin/index-pack.c, replace the line collision_test_needed =
has_sha1_file(sha1); with 

[PATCH v2] send-email: Add simple email aliases format

2015-05-21 Thread Allen Hubbe
This format is more simple than the other alias file formats, so it may
be preferred by some users.  The format is as follows.

alias: address|alias[, address|alias...]

Aliases are specified one per line.  There is no line splitting.

Example:
alice: Alice W Land a...@example.com
bob: Robert Bobbyton b...@example.com
chloe: ch...@example.com
abgroup: alice, bob
bcgrp: bob, chloe, Other o...@example.com

Signed-off-by: Allen Hubbe alle...@gmail.com
---

Notes:
The v1 of this patch had the following subject line:
git-send-email.perl: Add sendmail aliases support

This v2 renames this email alias format to simple, because the syntax
that is actually supported by the parser differs from the format used by
sendmail.  Now, there is no mention of sendmail in the name of the
format, the documentation, or the commit message.

This v2 also adds a test case to t/t9001-send-email.sh, and updates the
list of alias file types in Documentation/git-send-email.txt.

 Documentation/git-send-email.txt |  2 +-
 git-send-email.perl  |  6 +-
 t/t9001-send-email.sh| 24 
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 804554609def..99583c4f8969 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -383,7 +383,7 @@ sendemail.aliasesFile::
 
 sendemail.aliasFileType::
Format of the file(s) specified in sendemail.aliasesFile. Must be
-   one of 'mutt', 'mailrc', 'pine', 'elm', or 'gnus'.
+   one of 'mutt', 'mailrc', 'pine', 'elm', 'gnus', or 'simple'.
 
 sendemail.multiEdit::
If true (default), a single editor instance will be spawned to edit
diff --git a/git-send-email.perl b/git-send-email.perl
index e1e9b1460ced..25d72e8db8bf 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -515,7 +515,11 @@ my %parse_alias = (
   $aliases{$alias} = [ split_addrs($addr) ];
  }
  } },
-
+   simple = sub { my $fh = shift; while ($fh) {
+   if (/^\s*(\S+)\s*:\s*(.+)$/) {
+   my ($alias, $addr) = ($1, $2);
+   $aliases{$alias} = [ split_addrs($addr) ];
+   }}},
gnus = sub { my $fh = shift; while ($fh) {
if (/\(define-mail-alias\s+(\S+?)\s+(\S+?)\)/) {
$aliases{$1} = [ $2 ];
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 7be14a4e37f7..bbb73cdf8bec 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1548,6 +1548,30 @@ test_expect_success $PREREQ 
'sendemail.aliasfile=~/.mailrc' '
2errors out 
grep ^!someone@example\.org!$ commandline1
 '
+test_expect_success $PREREQ 'sendemail.aliasfiletype=simple' '
+   clean_fake_sendmail  rm -fr outdir 
+   git format-patch -1 -o outdir 
+   {
+   echo alice: Alice W Land a...@example.com
+   echo bob: Robert Bobbyton b...@example.com
+   echo chloe: ch...@example.com
+   echo abgroup: alice, bob
+   echo bcgrp: bob, chloe, Other o...@example.com
+   } ~/.tmp-email-aliases 
+   git config --replace-all sendemail.aliasesfile \
+   $(pwd)/.tmp-email-aliases 
+   git config sendemail.aliasfiletype simple 
+   git send-email \
+   --from=Example nob...@example.com \
+   --to=alice --to=bcgrp \
+   --smtp-server=$(pwd)/fake.sendmail \
+   outdir/0001-*.patch \
+   2errors out 
+   grep ^!awol@example\.com!$ commandline1 
+   grep ^!bob@example\.com!$ commandline1 
+   grep ^!chloe@example\.com!$ commandline1 
+   grep ^!o@example\.com!$ commandline1
+'
 
 do_xmailer_test () {
expected=$1 params=$2 
-- 
2.3.4

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


git p4 rebase --branch

2015-05-21 Thread FusionX86
For my Perforce to Git migration project, I am creating an empty local
git repo and then running git p4 sync with --branch to bring code from
Perforce into a specific Git branch. I'm going this route because I
need to take two Perforce branches that are similar and put them into
different branches of the same Git repo. One will go into dev and one
in master.

This seems to be working for me so far, but I want to use rebase to
pull new changes in and just realized that --branch isn't a supported
option for rebase. Any way to use git p4 rebase and target a local git
branch other than master?
--
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 08/14] remote.c: report specific errors from branch_get_upstream

2015-05-21 Thread Jeff King
On Thu, May 21, 2015 at 12:25:57PM -0700, Junio C Hamano wrote:

  Note also that the original may dereference branch-merge[0] even if it
  is NULL. I think that can't actually happen in practice (we only
  allocate branch-merge if we have at least one item to put in it, and
  all of the checks of branch-merge[0] are actually being over-careful).
  But the code I just wrote above does not have that problem.
 
 Perhaps update the patch with this explanation in the log message,
 as a separate preparatory step?

I decided on a separate patch on top which improves the logic and
explains the issues. Here it is (it goes on top of the existing patch 8,
report specific errors from branch_get_upstream).

-- 8 --
Subject: remote.c: untangle error logic in branch_get_upstream

The error-diagnosis logic in branch_get_upstream was copied
straight from sha1_name.c in the previous commit. However,
because we check all error cases and upfront and then later
diagnose them, the logic is a bit tangled. In particular:

  - if branch-merge[0] is NULL, we may end up dereferencing
it for an error message (in practice, it should never be
NULL, so this is probably not a triggerable bug).

  - We may enter the code path because branch-merge[0]-dst
is NULL, but we then start our error diagnosis by
checking whether our local branch exists. But that is
only relevant to diagnosing missing merge config, not a
missing tracking ref; our diagnosis may hide the real
problem.

Instead, let's just use a sequence of if blocks to check
for each error type, diagnose it, and return NULL.

Signed-off-by: Jeff King p...@peff.net
---
 remote.c | 19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/remote.c b/remote.c
index 1b7051a..d2519c2 100644
--- a/remote.c
+++ b/remote.c
@@ -1721,18 +1721,25 @@ const char *branch_get_upstream(struct branch *branch, 
struct strbuf *err)
 {
if (!branch)
return error_buf(err, _(HEAD does not point to a branch));
-   if (!branch-merge || !branch-merge[0] || !branch-merge[0]-dst) {
+
+   if (!branch-merge || !branch-merge[0]) {
+   /*
+* no merge config; is it because the user didn't define any,
+* or because it is not a real branch, and get_branch
+* auto-vivified it?
+*/
if (!ref_exists(branch-refname))
return error_buf(err, _(no such branch: '%s'),
 branch-name);
-   if (!branch-merge)
-   return error_buf(err,
-_(no upstream configured for branch 
'%s'),
-branch-name);
+   return error_buf(err,
+_(no upstream configured for branch '%s'),
+branch-name);
+   }
+
+   if (!branch-merge[0]-dst)
return error_buf(err,
 _(upstream branch '%s' not stored as a 
remote-tracking branch),
 branch-merge[0]-src);
-   }
 
return branch-merge[0]-dst;
 }
-- 
2.4.1.528.g00591e3

--
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 08/14] remote.c: report specific errors from branch_get_upstream

2015-05-21 Thread Jeff King
On Thu, May 21, 2015 at 08:46:43PM -0400, Jeff King wrote:

 On Thu, May 21, 2015 at 12:25:57PM -0700, Junio C Hamano wrote:
 
   Note also that the original may dereference branch-merge[0] even if it
   is NULL. I think that can't actually happen in practice (we only
   allocate branch-merge if we have at least one item to put in it, and
   all of the checks of branch-merge[0] are actually being over-careful).
   But the code I just wrote above does not have that problem.
  
  Perhaps update the patch with this explanation in the log message,
  as a separate preparatory step?
 
 I decided on a separate patch on top which improves the logic and
 explains the issues. Here it is (it goes on top of the existing patch 8,
 report specific errors from branch_get_upstream).
 
 -- 8 --
 Subject: remote.c: untangle error logic in branch_get_upstream

And then on top of that, we can add in this cleanup I showed earlier.

Both of these should insert into the series without any trouble, but let
me know if you run into problems and I can repost the whole thing.

-- 8 --
Subject: remote.c: return upstream name from stat_tracking_info

After calling stat_tracking_info, callers often want to
print the name of the upstream branch (in addition to the
tracking count). To do this, they have to access
branch-merge-dst[0] themselves. This is not wrong, as the
return value from stat_tracking_info tells us whether we
have an upstream branch or not. But it is a bit leaky, as we
make an assumption about how it calculated the upstream
name.

Instead, let's add an out-parameter that lets the caller
know the upstream name we found.

As a bonus, we can get rid of the unusual tri-state return
from the function. We no longer need to use it to
differentiate between no tracking config and tracking ref
does not exist (since you can check the upstream_name for
that), so we can just use the usual 0/-1 convention for
success/error.

Signed-off-by: Jeff King p...@peff.net
---
 builtin/branch.c   | 16 +---
 builtin/for-each-ref.c |  4 ++--
 remote.c   | 35 +--
 remote.h   |  3 ++-
 wt-status.c| 18 ++
 5 files changed, 32 insertions(+), 44 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index cc55ff2..8ecabd1 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -425,25 +425,19 @@ static void fill_tracking_info(struct strbuf *stat, const 
char *branch_name,
int ours, theirs;
char *ref = NULL;
struct branch *branch = branch_get(branch_name);
+   const char *upstream;
struct strbuf fancy = STRBUF_INIT;
int upstream_is_gone = 0;
int added_decoration = 1;
 
-   switch (stat_tracking_info(branch, ours, theirs)) {
-   case 0:
-   /* no base */
-   return;
-   case -1:
-   /* with gone base */
+   if (stat_tracking_info(branch, ours, theirs, upstream)  0) {
+   if (!upstream)
+   return;
upstream_is_gone = 1;
-   break;
-   default:
-   /* with base */
-   break;
}
 
if (show_upstream_ref) {
-   ref = shorten_unambiguous_ref(branch-merge[0]-dst, 0);
+   ref = shorten_unambiguous_ref(upstream, 0);
if (want_color(branch_use_color))
strbuf_addf(fancy, %s%s%s,
branch_get_color(BRANCH_COLOR_UPSTREAM),
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 18d209b..92bd2b2 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -716,7 +716,7 @@ static void populate_value(struct refinfo *ref)
char buf[40];
 
if (stat_tracking_info(branch, num_ours,
-  num_theirs) != 1)
+  num_theirs, NULL))
continue;
 
if (!num_ours  !num_theirs)
@@ -738,7 +738,7 @@ static void populate_value(struct refinfo *ref)
assert(branch);
 
if (stat_tracking_info(branch, num_ours,
-   num_theirs) != 1)
+   num_theirs, NULL))
continue;
 
if (!num_ours  !num_theirs)
diff --git a/remote.c b/remote.c
index d2519c2..c884574 100644
--- a/remote.c
+++ b/remote.c
@@ -1938,12 +1938,15 @@ int ref_newer(const unsigned char *new_sha1, const 
unsigned char *old_sha1)
 
 /*
  * Compare a branch with its upstream, and save their differences (number
- * of commits) in *num_ours and *num_theirs.
+ * of commits) in *num_ours and *num_theirs. The name of the upstream branch
+ * (or NULL if 

[PATCH] test_bitmap_walk: free bitmap with bitmap_free

2015-05-21 Thread Jeff King
Commit f86a374 (pack-bitmap.c: fix a memleak, 2015-03-30)
noticed that we leak the result bitmap. But we should use
bitmap_free rather than straight free, as the former
remembers to free the bitmap array pointed to by the struct.

Signed-off-by: Jeff King p...@peff.net
---
Sorry, I should have noticed this when reviewing the original.

 pack-bitmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pack-bitmap.c b/pack-bitmap.c
index 62a98cc..d64bd94 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -987,7 +987,7 @@ void test_bitmap_walk(struct rev_info *revs)
else
fprintf(stderr, Mismatch!\n);
 
-   free(result);
+   bitmap_free(result);
 }
 
 static int rebuild_bitmap(uint32_t *reposition,
-- 
2.4.1.528.g00591e3
--
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] send-email: Add simple email aliases format

2015-05-21 Thread Eric Sunshine
On Thu, May 21, 2015 at 8:16 PM, Allen Hubbe alle...@gmail.com wrote:
 This format is more simple than the other alias file formats, so it may
 be preferred by some users.  The format is as follows.

 alias: address|alias[, address|alias...]

 Aliases are specified one per line.  There is no line splitting.

 Example:
 alice: Alice W Land a...@example.com
 bob: Robert Bobbyton b...@example.com
 chloe: ch...@example.com
 abgroup: alice, bob
 bcgrp: bob, chloe, Other o...@example.com

 Signed-off-by: Allen Hubbe alle...@gmail.com
 ---
 diff --git a/Documentation/git-send-email.txt 
 b/Documentation/git-send-email.txt
 index 804554609def..99583c4f8969 100644
 --- a/Documentation/git-send-email.txt
 +++ b/Documentation/git-send-email.txt
 @@ -383,7 +383,7 @@ sendemail.aliasesFile::

  sendemail.aliasFileType::
 Format of the file(s) specified in sendemail.aliasesFile. Must be
 -   one of 'mutt', 'mailrc', 'pine', 'elm', or 'gnus'.
 +   one of 'mutt', 'mailrc', 'pine', 'elm', 'gnus', or 'simple'.

It's perhaps somewhat unfortunate that the formats of the other alias
file types aren't described here, however, the reader can at least
look them up. But the new simple format is never described anywhere
in the documentation, so it's effectively unusable. Most users will be
unable or unwilling to consult the source code or the commit message
to figure out how to use this format. The description you wrote for
the commit message might be sufficient as proper documentation (with
proper Asciidoc formatting, of course).

  sendemail.multiEdit::
 If true (default), a single editor instance will be spawned to edit
 diff --git a/git-send-email.perl b/git-send-email.perl
 index e1e9b1460ced..25d72e8db8bf 100755
 --- a/git-send-email.perl
 +++ b/git-send-email.perl
 @@ -515,7 +515,11 @@ my %parse_alias = (
$aliases{$alias} = [ split_addrs($addr) ];
   }
   } },
 -
 +   simple = sub { my $fh = shift; while ($fh) {
 +   if (/^\s*(\S+)\s*:\s*(.+)$/) {

I imagine that users would appreciate being able to add comments to
their aliases file, and the implementation complexity to support
comment lines and blank lines (as described in the Postfix aliases
documentation you cited earlier[1]) would be so minor that I'm rather
surprised you chose not to do so.

[1]: http://www.postfix.org/aliases.5.html

 +   my ($alias, $addr) = ($1, $2);
 +   $aliases{$alias} = [ split_addrs($addr) ];
 +   }}},
 gnus = sub { my $fh = shift; while ($fh) {
 if (/\(define-mail-alias\s+(\S+?)\s+(\S+?)\)/) {
 $aliases{$1} = [ $2 ];
 diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
 index 7be14a4e37f7..bbb73cdf8bec 100755
 --- a/t/t9001-send-email.sh
 +++ b/t/t9001-send-email.sh
 @@ -1548,6 +1548,30 @@ test_expect_success $PREREQ 
 'sendemail.aliasfile=~/.mailrc' '
 2errors out 
 grep ^!someone@example\.org!$ commandline1
  '
 +test_expect_success $PREREQ 'sendemail.aliasfiletype=simple' '
 +   clean_fake_sendmail  rm -fr outdir 
 +   git format-patch -1 -o outdir 
 +   {
 +   echo alice: Alice W Land a...@example.com
 +   echo bob: Robert Bobbyton b...@example.com
 +   echo chloe: ch...@example.com
 +   echo abgroup: alice, bob
 +   echo bcgrp: bob, chloe, Other o...@example.com
 +   } ~/.tmp-email-aliases 

A here-doc would be easier to maintain and read:

cat ~/.tmp-email-aliases -\EOF 
alice: Alice W Land a...@example.com
bob: Robert Bobbyton b...@example.com
...
EOF

 +   git config --replace-all sendemail.aliasesfile \
 +   $(pwd)/.tmp-email-aliases 
 +   git config sendemail.aliasfiletype simple 
 +   git send-email \
 +   --from=Example nob...@example.com \
 +   --to=alice --to=bcgrp \
 +   --smtp-server=$(pwd)/fake.sendmail \
 +   outdir/0001-*.patch \
 +   2errors out 
 +   grep ^!awol@example\.com!$ commandline1 
 +   grep ^!bob@example\.com!$ commandline1 
 +   grep ^!chloe@example\.com!$ commandline1 
 +   grep ^!o@example\.com!$ commandline1
 +'

  do_xmailer_test () {
 expected=$1 params=$2 
 --
 2.3.4
--
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: recovering from unordered stage entries in index error

2015-05-21 Thread Duy Nguyen
On Thu, May 21, 2015 at 11:49 PM, Junio C Hamano gits...@pobox.com wrote:
 Duy Nguyen pclo...@gmail.com writes:

 This message can be improved to show what entries have this problem.
 But then I don't see any way to recover the index manually. ls-files
 will die too.

 Isn't this failure coming from git-svn that tries to write out a
 tree after it prepared whatever it wants to record in its (possibly
 temporary) index?  I have a feeling that the index held by the end
 user is not broken.

Ahh that would explain why ls-files works. Yep.
-- 
Duy
--
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: Occasional wrong behavior of rev-walking (rev-list, log, etc.)

2015-05-21 Thread Mike Hommey
On Thu, May 21, 2015 at 03:59:48PM -0700, Junio C Hamano wrote:
 Depends on why you are running rev-list.
 
 If you want to know if one commit is contained in another, the way
 that should work the most reliably is to use merge-base, as the
 traversal engine of that command was written not to trust the commit
 timestamps but go with the topology alone.

So, specifically, rev-list was just a side use-case that happened to
show the same problem (or at least appeared to) when I was looking at
what the hell was happening. I should probably have started with my
actual failure instead, sorry for that. So here it is in the form of
a testcase:

$ git clone https://github.com/mozilla/gecko-dev
$ cd gecko-dev
$ git checkout -b a 4d3f25020072867e19ad6d840a73c8d77ea040bd
$ git commit --allow-empty -m a
$ git checkout -b b e90de3e5e045428e8f2b732882736dc93ce05f14
$ git commit --allow-empty -m b
$ git merge a -m merge
$ git rebase -i e90de3e5e045428e8f2b732882736dc93ce05f14 b

This lists plenty of commits instead of just a and b, like what would
happen if there weren't timestamps skews.

I've frequently used this kind of rebase in the past. It works most of
the time because the situation with timestamps is generally not a
problem. The reason I'm doing this kind of merge+rebase business is
that I have multiple topic branches that I need to rebase on top of each
other before pushing them upstream, and it's actually simpler, command-
wise, than actually rebasing the topics on top of each other.

Mike
--
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] send-email: Add simple email aliases format

2015-05-21 Thread Allen Hubbe
This format is more simple than the other alias file formats, so it may
be preferred by some users.  The format is as follows.

alias: address|alias[, address|alias...]

Aliases are specified one per line.  There is no line splitting.
Anything on a line after and including a `#` symbol is considered a
comment, and is ignored.  Blank lines are ignored.

Example of the 'simple' format:

alice: Alice W Land a...@example.com
bob: Robert Bobbyton b...@example.com
# this is a comment
   # this is also a comment
chloe: ch...@example.com
abgroup: alice, bob # comment after an alias
bcgrp: bob, chloe, Other o...@example.com

Signed-off-by: Allen Hubbe alle...@gmail.com
---

Notes:
This v3 extends the syntax to allow blank lines, and comments.  The test
case is extended with comments added to alias file input.

The Documentation/git-send-email.txt is updated with a description of
the simple format.  A note is added for the other formats, directing
readers to check the documentation of the email clients for a
description.

 Documentation/git-send-email.txt |  2 +-
 git-send-email.perl  |  6 +-
 t/t9001-send-email.sh| 24 
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 804554609def..99583c4f8969 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -383,7 +383,7 @@ sendemail.aliasesFile::
 
 sendemail.aliasFileType::
Format of the file(s) specified in sendemail.aliasesFile. Must be
-   one of 'mutt', 'mailrc', 'pine', 'elm', or 'gnus'.
+   one of 'mutt', 'mailrc', 'pine', 'elm', 'gnus', or 'simple'.
 
 sendemail.multiEdit::
If true (default), a single editor instance will be spawned to edit
diff --git a/git-send-email.perl b/git-send-email.perl
index e1e9b1460ced..25d72e8db8bf 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -515,7 +515,11 @@ my %parse_alias = (
   $aliases{$alias} = [ split_addrs($addr) ];
  }
  } },
-
+   simple = sub { my $fh = shift; while ($fh) {
+   if (/^\s*(\S+)\s*:\s*(.+)$/) {
+   my ($alias, $addr) = ($1, $2);
+   $aliases{$alias} = [ split_addrs($addr) ];
+   }}},
gnus = sub { my $fh = shift; while ($fh) {
if (/\(define-mail-alias\s+(\S+?)\s+(\S+?)\)/) {
$aliases{$1} = [ $2 ];
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 7be14a4e37f7..bbb73cdf8bec 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1548,6 +1548,30 @@ test_expect_success $PREREQ 
'sendemail.aliasfile=~/.mailrc' '
2errors out 
grep ^!someone@example\.org!$ commandline1
 '
+test_expect_success $PREREQ 'sendemail.aliasfiletype=simple' '
+   clean_fake_sendmail  rm -fr outdir 
+   git format-patch -1 -o outdir 
+   {
+   echo alice: Alice W Land a...@example.com
+   echo bob: Robert Bobbyton b...@example.com
+   echo chloe: ch...@example.com
+   echo abgroup: alice, bob
+   echo bcgrp: bob, chloe, Other o...@example.com
+   } ~/.tmp-email-aliases 
+   git config --replace-all sendemail.aliasesfile \
+   $(pwd)/.tmp-email-aliases 
+   git config sendemail.aliasfiletype simple 
+   git send-email \
+   --from=Example nob...@example.com \
+   --to=alice --to=bcgrp \
+   --smtp-server=$(pwd)/fake.sendmail \
+   outdir/0001-*.patch \
+   2errors out 
+   grep ^!awol@example\.com!$ commandline1 
+   grep ^!bob@example\.com!$ commandline1 
+   grep ^!chloe@example\.com!$ commandline1 
+   grep ^!o@example\.com!$ commandline1
+'
 
 do_xmailer_test () {
expected=$1 params=$2 
-- 
2.3.4

--
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 v4] send-email: Add simple email aliases format

2015-05-21 Thread Allen Hubbe
This format is more simple than the other alias file formats, so it may
be preferred by some users.  The format is as follows.

alias: address|alias[, address|alias...]

Aliases are specified one per line.  There is no line splitting.
Anything on a line after and including a `#` symbol is considered a
comment, and is ignored.  Blank lines are ignored.

Example of the 'simple' format:

alice: Alice W Land a...@example.com
bob: Robert Bobbyton b...@example.com
# this is a comment
   # this is also a comment
chloe: ch...@example.com
abgroup: alice, bob # comment after an alias
bcgrp: bob, chloe, Other o...@example.com

Signed-off-by: Allen Hubbe alle...@gmail.com
---

Notes:
Note, v3 was sent in error.  This v4 presents changes since v2.

This v4 extends the syntax to allow blank lines, and comments.  The test
case is extended with comments added to alias file input.

The Documentation/git-send-email.txt is updated with a description of
the simple format.  A note is added for the other formats, directing
readers to check the documentation of the email clients for a
description.

 Documentation/git-send-email.txt | 24 +++-
 git-send-email.perl  |  8 +++-
 t/t9001-send-email.sh| 27 +++
 3 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 804554609def..38ade31e0c28 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -383,7 +383,29 @@ sendemail.aliasesFile::
 
 sendemail.aliasFileType::
Format of the file(s) specified in sendemail.aliasesFile. Must be
-   one of 'mutt', 'mailrc', 'pine', 'elm', or 'gnus'.
+   one of 'simple', 'mutt', 'mailrc', 'pine', 'elm', or 'gnus'.
++
+If the format is 'simple', then the alias file format is described below.
+Descriptions of the other file formats to the following formats can be found in
+the documentation of the email program of the same name.
++
+This 'simple' format is is as follows.
++
+   alias: address|alias[, address|alias...]
++
+Aliases are specified one per line.  There is no line splitting.  Anything on a
+line after and including a `#` symbol is considered a comment, and is ignored.
+Blank lines are ignored.
++
+Example of the 'simple' format:
++
+   alice: Alice W Land a...@example.com
+   bob: Robert Bobbyton b...@example.com
+   # this is a comment
+  # this is also a comment
+   chloe: ch...@example.com
+   abgroup: alice, bob # comment after an alias
+   bcgrp: bob, chloe, Other o...@example.com
 
 sendemail.multiEdit::
If true (default), a single editor instance will be spawned to edit
diff --git a/git-send-email.perl b/git-send-email.perl
index e1e9b1460ced..716c2bc9479a 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -515,7 +515,13 @@ my %parse_alias = (
   $aliases{$alias} = [ split_addrs($addr) ];
  }
  } },
-
+   simple = sub { my $fh = shift; while ($fh) {
+   s/#.*$//;
+   next if /^\s*$/;
+   if (/^\s*(\S+)\s*:\s*(.+)$/) {
+   my ($alias, $addr) = ($1, $2);
+   $aliases{$alias} = [ split_addrs($addr) ];
+   }}},
gnus = sub { my $fh = shift; while ($fh) {
if (/\(define-mail-alias\s+(\S+?)\s+(\S+?)\)/) {
$aliases{$1} = [ $2 ];
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 7be14a4e37f7..12c1a0c76f1d 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1549,6 +1549,33 @@ test_expect_success $PREREQ 
'sendemail.aliasfile=~/.mailrc' '
grep ^!someone@example\.org!$ commandline1
 '
 
+test_expect_success $PREREQ 'sendemail.aliasfiletype=simple' '
+   clean_fake_sendmail  rm -fr outdir 
+   git format-patch -1 -o outdir 
+   {
+   echo alice: Alice W Land a...@example.com
+   echo bob: Robert Bobbyton b...@example.com
+   echo # this is a comment
+   echo# this is also a comment
+   echo chloe: ch...@example.com
+   echo abgroup: alice, bob # comment after an alias
+   echo bcgrp: bob, chloe, Other o...@example.com
+   } ~/.tmp-email-aliases 
+   git config --replace-all sendemail.aliasesfile \
+   $(pwd)/.tmp-email-aliases 
+   git config sendemail.aliasfiletype simple 
+   git send-email \
+   --from=Example nob...@example.com \
+   --to=alice --to=bcgrp \
+   --smtp-server=$(pwd)/fake.sendmail \
+   outdir/0001-*.patch \
+   2errors out 
+   grep ^!awol@example\.com!$ commandline1 
+   grep ^!bob@example\.com!$ commandline1 
+   grep 

Re: [PATCH v4] send-email: Add simple email aliases format

2015-05-21 Thread Eric Sunshine
On Thu, May 21, 2015 at 11:40 PM, Allen Hubbe alle...@gmail.com wrote:
 This format is more simple than the other alias file formats, so it may
 be preferred by some users. [...]
 Signed-off-by: Allen Hubbe alle...@gmail.com
 ---
 diff --git a/Documentation/git-send-email.txt 
 b/Documentation/git-send-email.txt
 index 804554609def..38ade31e0c28 100644
 --- a/Documentation/git-send-email.txt
 +++ b/Documentation/git-send-email.txt
 @@ -383,7 +383,29 @@ sendemail.aliasesFile::

  sendemail.aliasFileType::
 Format of the file(s) specified in sendemail.aliasesFile. Must be
 -   one of 'mutt', 'mailrc', 'pine', 'elm', or 'gnus'.
 +   one of 'simple', 'mutt', 'mailrc', 'pine', 'elm', or 'gnus'.
 ++
 +If the format is 'simple', then the alias file format is described below.
 +Descriptions of the other file formats to the following formats can be found 
 in
 +the documentation of the email program of the same name.

The second sentence probably needs some proof-reading.

 ++
 +This 'simple' format is is as follows.
 ++
 +   alias: address|alias[, address|alias...]
 ++
 +Aliases are specified one per line.  There is no line splitting.  Anything 
 on a
 +line after and including a `#` symbol is considered a comment, and is 
 ignored.
 +Blank lines are ignored.

I'm not convinced that gratuitously diverging from the
sendmail/postfix 'aliases' format is warranted. In particular, that
format recognizes a comment line only when '#' is the first
non-whitespace character[1]; and does not consider '#' a
comment-introducer anywhere else in the line. By recognizing '#'
anywhere as a comment-introducer, you may be painting this format into
a corner rather than leaving it open for someone later to extend it to
be more sendmail/postfix-like by, for instance, supporting name
quoting and line-continuation[1].

For the same reason, I'm not convinced that simple is a good name.
sendmail may indeed be a more appropriate name, even if it means
that this early implementation documents it as (currently) a subset of
the richer sendmail/postfix 'aliases' format. By doing so, we leave
the door open so a future person can implement additional features to
bring it closer to that format.

[1]: http://www.postfix.org/aliases.5.html

 ++
 +Example of the 'simple' format:
 ++
 +   alice: Alice W Land a...@example.com
 +   bob: Robert Bobbyton b...@example.com
 +   # this is a comment
 +  # this is also a comment
 +   chloe: ch...@example.com
 +   abgroup: alice, bob # comment after an alias
 +   bcgrp: bob, chloe, Other o...@example.com
--
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] send-email: Add simple email aliases format

2015-05-21 Thread Allen Hubbe
On May 21, 2015 9:05 PM, Eric Sunshine sunsh...@sunshineco.com wrote:

 On Thu, May 21, 2015 at 8:16 PM, Allen Hubbe alle...@gmail.com wrote:
  This format is more simple than the other alias file formats, so it may
  be preferred by some users.  The format is as follows.
 
  alias: address|alias[, address|alias...]
 
  Aliases are specified one per line.  There is no line splitting.
 
  Example:
  alice: Alice W Land a...@example.com
  bob: Robert Bobbyton b...@example.com
  chloe: ch...@example.com
  abgroup: alice, bob
  bcgrp: bob, chloe, Other o...@example.com
 
  Signed-off-by: Allen Hubbe alle...@gmail.com
  ---
  diff --git a/Documentation/git-send-email.txt 
  b/Documentation/git-send-email.txt
  index 804554609def..99583c4f8969 100644
  --- a/Documentation/git-send-email.txt
  +++ b/Documentation/git-send-email.txt
  @@ -383,7 +383,7 @@ sendemail.aliasesFile::
 
   sendemail.aliasFileType::
  Format of the file(s) specified in sendemail.aliasesFile. Must be
  -   one of 'mutt', 'mailrc', 'pine', 'elm', or 'gnus'.
  +   one of 'mutt', 'mailrc', 'pine', 'elm', 'gnus', or 'simple'.

 It's perhaps somewhat unfortunate that the formats of the other alias
 file types aren't described here, however, the reader can at least
 look them up. But the new simple format is never described anywhere
 in the documentation, so it's effectively unusable. Most users will be
 unable or unwilling to consult the source code or the commit message
 to figure out how to use this format. The description you wrote for
 the commit message might be sufficient as proper documentation (with
 proper Asciidoc formatting, of course).

Ok, I will add the description in the commit, formatted, to the documentation.


   sendemail.multiEdit::
  If true (default), a single editor instance will be spawned to edit
  diff --git a/git-send-email.perl b/git-send-email.perl
  index e1e9b1460ced..25d72e8db8bf 100755
  --- a/git-send-email.perl
  +++ b/git-send-email.perl
  @@ -515,7 +515,11 @@ my %parse_alias = (
 $aliases{$alias} = [ split_addrs($addr) ];
}
} },
  -
  +   simple = sub { my $fh = shift; while ($fh) {
  +   if (/^\s*(\S+)\s*:\s*(.+)$/) {

 I imagine that users would appreciate being able to add comments to
 their aliases file, and the implementation complexity to support
 comment lines and blank lines (as described in the Postfix aliases
 documentation you cited earlier[1]) would be so minor that I'm rather
 surprised you chose not to do so.

I will add support for comments. Anything after the first '#' in any
line will be treated as a comment.


 [1]: http://www.postfix.org/aliases.5.html

  +   my ($alias, $addr) = ($1, $2);
  +   $aliases{$alias} = [ split_addrs($addr) ];
  +   }}},
  gnus = sub { my $fh = shift; while ($fh) {
  if (/\(define-mail-alias\s+(\S+?)\s+(\S+?)\)/) {
  $aliases{$1} = [ $2 ];
  diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
  index 7be14a4e37f7..bbb73cdf8bec 100755
  --- a/t/t9001-send-email.sh
  +++ b/t/t9001-send-email.sh
  @@ -1548,6 +1548,30 @@ test_expect_success $PREREQ 
  'sendemail.aliasfile=~/.mailrc' '
  2errors out 
  grep ^!someone@example\.org!$ commandline1
   '
  +test_expect_success $PREREQ 'sendemail.aliasfiletype=simple' '
  +   clean_fake_sendmail  rm -fr outdir 
  +   git format-patch -1 -o outdir 
  +   {
  +   echo alice: Alice W Land a...@example.com
  +   echo bob: Robert Bobbyton b...@example.com
  +   echo chloe: ch...@example.com
  +   echo abgroup: alice, bob
  +   echo bcgrp: bob, chloe, Other o...@example.com
  +   } ~/.tmp-email-aliases 

 A here-doc would be easier to maintain and read:

 cat ~/.tmp-email-aliases -\EOF 
 alice: Alice W Land a...@example.com
 bob: Robert Bobbyton b...@example.com
 ...
 EOF

A here-doc does not flow nicely in an indented block.  Each line in
the here-doc will also contain any indentation which may appear to the
reader to be part of the test case.  Alternatively, the here-doc could
be indented differently than the surrounding test case (all the way to
the left column), but that also has a negative impact for readability.
Finally, the EOF marker can not be indented.

With echo string, exactly string is output to the line.  The
operation is obvious to the reader.  The test case can use sane
indentation, and the resulting output will be exactly what what it
would appear to be in the test case.

Especially for something like a test case where there should be
absolutely no confusion as to exactly what is the input to the test,
clarity matters.  Any operation where the result is not immediately
obvious to the reader, does not belong here.  

Re: [PATCH v3] send-email: Add simple email aliases format

2015-05-21 Thread Allen Hubbe
Please ignore v3... this is the same as v2 for some reason.  I will
resend as v4.

On Thu, May 21, 2015 at 11:35 PM, Allen Hubbe alle...@gmail.com wrote:
 This format is more simple than the other alias file formats, so it may
 be preferred by some users.  The format is as follows.

 alias: address|alias[, address|alias...]

 Aliases are specified one per line.  There is no line splitting.
 Anything on a line after and including a `#` symbol is considered a
 comment, and is ignored.  Blank lines are ignored.

 Example of the 'simple' format:

 alice: Alice W Land a...@example.com
 bob: Robert Bobbyton b...@example.com
 # this is a comment
# this is also a comment
 chloe: ch...@example.com
 abgroup: alice, bob # comment after an alias
 bcgrp: bob, chloe, Other o...@example.com

 Signed-off-by: Allen Hubbe alle...@gmail.com
 ---

 Notes:
 This v3 extends the syntax to allow blank lines, and comments.  The test
 case is extended with comments added to alias file input.

 The Documentation/git-send-email.txt is updated with a description of
 the simple format.  A note is added for the other formats, directing
 readers to check the documentation of the email clients for a
 description.

  Documentation/git-send-email.txt |  2 +-
  git-send-email.perl  |  6 +-
  t/t9001-send-email.sh| 24 
  3 files changed, 30 insertions(+), 2 deletions(-)

 diff --git a/Documentation/git-send-email.txt 
 b/Documentation/git-send-email.txt
 index 804554609def..99583c4f8969 100644
 --- a/Documentation/git-send-email.txt
 +++ b/Documentation/git-send-email.txt
 @@ -383,7 +383,7 @@ sendemail.aliasesFile::

  sendemail.aliasFileType::
 Format of the file(s) specified in sendemail.aliasesFile. Must be
 -   one of 'mutt', 'mailrc', 'pine', 'elm', or 'gnus'.
 +   one of 'mutt', 'mailrc', 'pine', 'elm', 'gnus', or 'simple'.

  sendemail.multiEdit::
 If true (default), a single editor instance will be spawned to edit
 diff --git a/git-send-email.perl b/git-send-email.perl
 index e1e9b1460ced..25d72e8db8bf 100755
 --- a/git-send-email.perl
 +++ b/git-send-email.perl
 @@ -515,7 +515,11 @@ my %parse_alias = (
$aliases{$alias} = [ split_addrs($addr) ];
   }
   } },
 -
 +   simple = sub { my $fh = shift; while ($fh) {
 +   if (/^\s*(\S+)\s*:\s*(.+)$/) {
 +   my ($alias, $addr) = ($1, $2);
 +   $aliases{$alias} = [ split_addrs($addr) ];
 +   }}},
 gnus = sub { my $fh = shift; while ($fh) {
 if (/\(define-mail-alias\s+(\S+?)\s+(\S+?)\)/) {
 $aliases{$1} = [ $2 ];
 diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
 index 7be14a4e37f7..bbb73cdf8bec 100755
 --- a/t/t9001-send-email.sh
 +++ b/t/t9001-send-email.sh
 @@ -1548,6 +1548,30 @@ test_expect_success $PREREQ 
 'sendemail.aliasfile=~/.mailrc' '
 2errors out 
 grep ^!someone@example\.org!$ commandline1
  '
 +test_expect_success $PREREQ 'sendemail.aliasfiletype=simple' '
 +   clean_fake_sendmail  rm -fr outdir 
 +   git format-patch -1 -o outdir 
 +   {
 +   echo alice: Alice W Land a...@example.com
 +   echo bob: Robert Bobbyton b...@example.com
 +   echo chloe: ch...@example.com
 +   echo abgroup: alice, bob
 +   echo bcgrp: bob, chloe, Other o...@example.com
 +   } ~/.tmp-email-aliases 
 +   git config --replace-all sendemail.aliasesfile \
 +   $(pwd)/.tmp-email-aliases 
 +   git config sendemail.aliasfiletype simple 
 +   git send-email \
 +   --from=Example nob...@example.com \
 +   --to=alice --to=bcgrp \
 +   --smtp-server=$(pwd)/fake.sendmail \
 +   outdir/0001-*.patch \
 +   2errors out 
 +   grep ^!awol@example\.com!$ commandline1 
 +   grep ^!bob@example\.com!$ commandline1 
 +   grep ^!chloe@example\.com!$ commandline1 
 +   grep ^!o@example\.com!$ commandline1
 +'

  do_xmailer_test () {
 expected=$1 params=$2 
 --
 2.3.4

--
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] send-email: Add simple email aliases format

2015-05-21 Thread Eric Sunshine
On Thu, May 21, 2015 at 11:19 PM, Allen Hubbe alle...@gmail.com wrote:
 On May 21, 2015 9:05 PM, Eric Sunshine sunsh...@sunshineco.com wrote:
 On Thu, May 21, 2015 at 8:16 PM, Allen Hubbe alle...@gmail.com wrote:
  +test_expect_success $PREREQ 'sendemail.aliasfiletype=simple' '
  +   clean_fake_sendmail  rm -fr outdir 
  +   git format-patch -1 -o outdir 
  +   {
  +   echo alice: Alice W Land a...@example.com
  +   echo bob: Robert Bobbyton b...@example.com
  +   echo chloe: ch...@example.com
  +   echo abgroup: alice, bob
  +   echo bcgrp: bob, chloe, Other o...@example.com
  +   } ~/.tmp-email-aliases 

 A here-doc would be easier to maintain and read:

 cat ~/.tmp-email-aliases -\EOF 
 alice: Alice W Land a...@example.com
 bob: Robert Bobbyton b...@example.com
 ...
 EOF

 A here-doc does not flow nicely in an indented block.  Each line in
 the here-doc will also contain any indentation which may appear to the
 reader to be part of the test case.  Alternatively, the here-doc could
 be indented differently than the surrounding test case (all the way to
 the left column), but that also has a negative impact for readability.
 Finally, the EOF marker can not be indented.

That's true if you use EOF here-doc, but not for -EOF, as I did in
the example. With -EOF, all leading tabs are stripped from the input
lines, including from the EOF line, which is why it can be indented to
the same level as the other code in the test. The added '\' in -\EOF
from my example indicates that you don't want/expect any interpolation
inside the here-doc. The -\EOF form is used extensively throughout
the Git test suite.

 With echo string, exactly string is output to the line.  The
 operation is obvious to the reader.  The test case can use sane
 indentation, and the resulting output will be exactly what what it
 would appear to be in the test case.

Same with -\EOF; plus -\EOF content is more readable since it's
not polluted with 'echo' noise.

 Especially for something like a test case where there should be
 absolutely no confusion as to exactly what is the input to the test,
 clarity matters.  Any operation where the result is not immediately
 obvious to the reader, does not belong here.  Therefore, I will keep
 the lines in the test case as echo string.
--
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 07/14] remote.c: introduce branch_get_upstream helper

2015-05-21 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Thu, May 21, 2015 at 02:14:29PM -0400, Jeff King wrote:

 There is a related cleanup I resisted, which is that several call-sites
 will call stat_tracking_info, then later look directly at
 branch-merge[0]-dst without a check for NULL (fill_tracking_info is
 such a site).
 
 This works because stat_tracking_info's return value tells us that we
 did indeed find an upstream to compare with. But it feels a little leaky
 to me. One solution is for stat_tracking_info to pass out the name of
 thte upstream, making the caller side something like:

 So just for fun, here is what a whole patch, with the refactoring of the
 return value, would look like.

It looks much nicer and reads better, at least to me.

Nice.

 diff --git a/builtin/branch.c b/builtin/branch.c
 index cc55ff2..8ecabd1 100644
 --- a/builtin/branch.c
 +++ b/builtin/branch.c
 @@ -425,25 +425,19 @@ static void fill_tracking_info(struct strbuf *stat, 
 const char *branch_name,
   int ours, theirs;
   char *ref = NULL;
   struct branch *branch = branch_get(branch_name);
 + const char *upstream;
   struct strbuf fancy = STRBUF_INIT;
   int upstream_is_gone = 0;
   int added_decoration = 1;
  
 - switch (stat_tracking_info(branch, ours, theirs)) {
 - case 0:
 - /* no base */
 - return;
 - case -1:
 - /* with gone base */
 + if (stat_tracking_info(branch, ours, theirs, upstream)  0) {
 + if (!upstream)
 + return;
   upstream_is_gone = 1;
 - break;
 - default:
 - /* with base */
 - break;
   }
  
   if (show_upstream_ref) {
 - ref = shorten_unambiguous_ref(branch-merge[0]-dst, 0);
 + ref = shorten_unambiguous_ref(upstream, 0);
   if (want_color(branch_use_color))
   strbuf_addf(fancy, %s%s%s,
   branch_get_color(BRANCH_COLOR_UPSTREAM),
 diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
 index 6847400..05dd23d 100644
 --- a/builtin/for-each-ref.c
 +++ b/builtin/for-each-ref.c
 @@ -730,7 +730,7 @@ static void populate_value(struct refinfo *ref)
   char buf[40];
  
   if (stat_tracking_info(branch, num_ours,
 -num_theirs) != 1)
 +num_theirs, NULL))
   continue;
  
   if (!num_ours  !num_theirs)
 @@ -753,7 +753,7 @@ static void populate_value(struct refinfo *ref)
   assert(branch);
  
   if (stat_tracking_info(branch, num_ours,
 - num_theirs) != 1)
 + num_theirs, NULL))
   continue;
  
   if (!num_ours  !num_theirs)
 diff --git a/remote.c b/remote.c
 index be45a39..6765051 100644
 --- a/remote.c
 +++ b/remote.c
 @@ -2016,12 +2016,15 @@ int ref_newer(const unsigned char *new_sha1, const 
 unsigned char *old_sha1)
  
  /*
   * Compare a branch with its upstream, and save their differences (number
 - * of commits) in *num_ours and *num_theirs.
 + * of commits) in *num_ours and *num_theirs. The name of the upstream branch
 + * (or NULL if no upstream is defined) is returned via *upstream_name, if it
 + * is not itself NULL.
   *
 - * Return 0 if branch has no upstream (no base), -1 if upstream is missing
 - * (with gone base), otherwise 1 (with base).
 + * Returns -1 if num_ours and num_theirs could not be filled in (e.g., no
 + * upstream defined, or ref does not exist), 0 otherwise.
   */
 -int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs)
 +int stat_tracking_info(struct branch *branch, int *num_ours, int *num_theirs,
 +const char **upstream_name)
  {
   unsigned char sha1[20];
   struct commit *ours, *theirs;
 @@ -2032,8 +2035,10 @@ int stat_tracking_info(struct branch *branch, int 
 *num_ours, int *num_theirs)
  
   /* Cannot stat unless we are marked to build on top of somebody else. */
   base = branch_get_upstream(branch, NULL);
 + if (upstream_name)
 + *upstream_name = base;
   if (!base)
 - return 0;
 + return -1;
  
   /* Cannot stat if what we used to build on no longer exists */
   if (read_ref(base, sha1))
 @@ -2051,7 +2056,7 @@ int stat_tracking_info(struct branch *branch, int 
 *num_ours, int *num_theirs)
   /* are we the same? */
   if (theirs == ours) {
   *num_theirs = *num_ours = 0;
 - return 1;
 + return 0;
   }
  
   /* Run rev-list --left-right ours...theirs internally... */
 @@ -2087,7 +2092,7 @@ int stat_tracking_info(struct 

[PATCH v11 1/5] command-list: prepare machinery for upcoming common groups section

2015-05-21 Thread Sébastien Guimmara
From: Eric Sunshine sunsh...@sunshineco.com

The ultimate goal is for git help to classify common commands by
group. Toward this end, a subsequent patch will add a new common
groups section to command-list.txt preceding the actual command list.
As preparation, teach existing command-list.txt parsing machinery, which
doesn't care about grouping, to skip over this upcoming common groups
section.

Signed-off-by: Eric Sunshine sunsh...@sunshineco.com
Signed-off-by: Sébastien Guimmara sebastien.guimm...@gmail.com
---
 Documentation/cmd-list.perl | 4 
 Makefile| 5 +++--
 command-list.txt| 2 +-
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/Documentation/cmd-list.perl b/Documentation/cmd-list.perl
index 04f9977..5aa73cf 100755
--- a/Documentation/cmd-list.perl
+++ b/Documentation/cmd-list.perl
@@ -38,6 +38,10 @@ sub format_one {
}
 }
 
+while () {
+   last if /^### command list/;
+}
+
 my %cmds = ();
 for (sort ) {
next if /^#/;
diff --git a/Makefile b/Makefile
index 323c401..655740d 100644
--- a/Makefile
+++ b/Makefile
@@ -2455,7 +2455,7 @@ check-docs::
esac ; \
test -f Documentation/$$v.txt || \
echo no doc: $$v; \
-   sed -e '/^#/d' command-list.txt | \
+   sed -e '1,/^### command list/d' -e '/^#/d' command-list.txt | \
grep -q ^$$v[  ] || \
case $$v in \
git) ;; \
@@ -2463,7 +2463,8 @@ check-docs::
esac ; \
done; \
( \
-   sed -e '/^#/d' \
+   sed -e '1,/^### command list/d' \
+   -e '/^#/d' \
-e 's/[ ].*//' \
-e 's/^/listed /' command-list.txt; \
$(MAKE) -C Documentation print-man1 | \
diff --git a/command-list.txt b/command-list.txt
index 54d8d21..181a9c2 100644
--- a/command-list.txt
+++ b/command-list.txt
@@ -1,4 +1,4 @@
-# List of known git commands.
+### command list (do not change this line)
 # command name  category [deprecated] [common]
 git-add mainporcelain common
 git-am  mainporcelain
-- 
2.4.0.GIT

--
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 v11 5/5] help: respect new common command grouping

2015-05-21 Thread Sébastien Guimmara
'git help' shows common commands in alphabetical order:

The most commonly used git commands are:
   addAdd file contents to the index
   bisect Find by binary search the change that introduced a bug
   branch List, create, or delete branches
   checkout   Checkout a branch or paths to the working tree
   clone  Clone a repository into a new directory
   commit Record changes to the repository
   [...]

without any indication of how commands relate to high-level
concepts or each other. Revise the output to explain their relationship
with the typical Git workflow:

The most commonly used git commands are:

start a working area (see also: git help tutorial)
   clone  Clone a repository into a new directory
   init   Create an empty Git repository or reinitialize [...]

work on the current change (see also: git help everyday)
   addAdd file contents to the index
   reset  Reset current HEAD to the specified state

examine the history and state (see also: git help revisions)
   logShow commit logs
   status Show the working tree status

   [...]

Helped-by: Eric Sunshine sunsh...@sunshineco.com
Signed-off-by: Ramsay Jones ram...@ramsay1.demon.co.uk
Signed-off-by: Sébastien Guimmara sebastien.guimm...@gmail.com
---
 help.c | 24 +++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/help.c b/help.c
index 2072a87..8f72051 100644
--- a/help.c
+++ b/help.c
@@ -218,17 +218,39 @@ void list_commands(unsigned int colopts,
}
 }
 
+static int cmd_group_cmp(const void *elem1, const void *elem2)
+{
+   const struct cmdname_help *e1 = elem1;
+   const struct cmdname_help *e2 = elem2;
+
+   if (e1-group  e2-group)
+   return -1;
+   if (e1-group  e2-group)
+   return 1;
+   return strcmp(e1-name, e2-name);
+}
+
 void list_common_cmds_help(void)
 {
int i, longest = 0;
+   int current_grp = -1;
 
for (i = 0; i  ARRAY_SIZE(common_cmds); i++) {
if (longest  strlen(common_cmds[i].name))
longest = strlen(common_cmds[i].name);
}
 
-   puts(_(The most commonly used git commands are:));
+   qsort(common_cmds, ARRAY_SIZE(common_cmds),
+   sizeof(common_cmds[0]), cmd_group_cmp);
+
+   puts(_(These are common Git commands used in various situations:));
+
for (i = 0; i  ARRAY_SIZE(common_cmds); i++) {
+   if (common_cmds[i].group != current_grp) {
+   printf(\n%s\n, 
_(common_cmd_groups[common_cmds[i].group]));
+   current_grp = common_cmds[i].group;
+   }
+
printf(   %s   , common_cmds[i].name);
mput_char(' ', longest - strlen(common_cmds[i].name));
puts(_(common_cmds[i].help));
-- 
2.4.0.GIT

--
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 v11 2/5] command-list.txt: add the common groups block

2015-05-21 Thread Eric Sunshine
On Thu, May 21, 2015 at 1:39 PM, Sébastien Guimmara
sebastien.guimm...@gmail.com wrote:
 The ultimate goal is for git help to display common commands in
 groups rather than alphabetically. As a first step, define the
 groups in a new block, and then assign a group to each
 common command.

 Helped-by: Eric Sunshine sunsh...@sunshineco.com
 Helped-by: Junio C Hamano gits...@pobox.com
 Helped-by:  Emma Jane Hogbin Westby emma.wes...@gmail.com
 Signed-off-by: Sébastien Guimmara sebastien.guimm...@gmail.com
 ---
 diff --git a/command-list.txt b/command-list.txt
 index 181a9c2..32ddab3 100644
 --- a/command-list.txt
 +++ b/command-list.txt
 @@ -1,3 +1,14 @@
 +# common commands are grouped by themes
 +# these groups are output by 'git help' in the order declared here.
 +# map each common command in the command list to one of these groups.
 +### common groups (do not change this line)
 +init start a working area (see also: git help tutorial)
 +worktree work on the current change (see also: git help everyday)
 +info examine the history and state (see also: git help revisions)
 +history  grow, mark and tweak your common history
 +remote   collaborate (see also: git help workflows)
 +
 +# List of known git commands.

This is odd. The above line was removed in 1/5 but then re-appears
here in 2/5. I think the intent is that it should remain removed.

  ### command list (do not change this line)
  # command name  category [deprecated] [common]
  git-add mainporcelain common
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] git-send-email.perl: Add sendmail aliases support

2015-05-21 Thread Allen Hubbe
Support sendmail (and postfix) style email aliases in git-send-email.
This is the format of /etc/mail/aliases, documented by `man 5 aliases`.
The man page may be provided by sendmail or postfix on your system, or
by another email service that uses the same configuration file.

See also: http://www.postfix.org/aliases.5.html.

This format is more simple than the other alias file formats, so it may
be preferred by some users.  The format is as follows.

alias: address|alias[, address|alias...]

Example (no indent in aliases file):
alice: Alice W Land a...@example.com
bob: Robert Bobbyton b...@example.com
chloe: ch...@example.com
abgroup: alice, bob
bcgrp: bob, chloe, Other o...@example.com

This patch DOES NOT support line continuations as are described by the
specification.  Line continuations are indicated by either (or both) a
trailing '\' on the line to be continued, or leading whitespace on the
continuing line.  This patch does not do anything with the trailing '\',
and ignores lines starting with whitespace.

This patch also ignores lines that starts with '#', comment character,
and empty lines.

Signed-off-by: Allen Hubbe alle...@gmail.com
---
 git-send-email.perl | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index e1e9b14..5f2ec0d 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -515,7 +515,12 @@ my %parse_alias = (
   $aliases{$alias} = [ split_addrs($addr) ];
  }
  } },
-
+   sendmail = sub { my $fh = shift; while ($fh) {
+   next if /^$|^#|^\s/;
+   if (/^(\S+)\s*:\s*(.*?)\\?$/) {
+   my ($alias, $addr) = ($1, $2);
+   $aliases{$alias} = [ split_addrs($addr) ];
+   }}},
gnus = sub { my $fh = shift; while ($fh) {
if (/\(define-mail-alias\s+(\S+?)\s+(\S+?)\)/) {
$aliases{$1} = [ $2 ];
-- 
2.3.4

--
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 08/14] remote.c: report specific errors from branch_get_upstream

2015-05-21 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Thu, May 21, 2015 at 11:33:58AM -0700, Junio C Hamano wrote:

  +static const char *error_buf(struct strbuf *err, const char *fmt, ...)
   {
  -  if (!branch || !branch-merge || !branch-merge[0])
  -  return NULL;
  +  if (err) {
  +  va_list ap;
  +  va_start(ap, fmt);
  +  strbuf_vaddf(err, fmt, ap);
  +  va_end(ap);
  +  }
  +  return NULL;
  +}
 
 Many of our functions return -1 to signal an error, and that is why
 it makes sense for our error() helper to return -1 to save code in
 the caller, but only because the callers of this private helper use
 a NULL to signal an error, this also returns NULL.  If we were to
 use the callers can opt into detailed message by passing strbuf
 pattern more widely, we would want a variant of the above that
 returns -1, too.  And such a helper would do the same thing as
 above, with only difference from the above is to return -1.

 Yeah, this is not really a good general-purpose purpose function in that
 sense. I have often wanted an error() that returns NULL, but avoided
 adding just because it seemed like needless proliferation.

 The real reason to include the return value at all in error() is to let
 you turn two-liners into one-liners.

Yeah, our error() returns -1 to save code in the caller.  And the
callers of this private helper all want to return NULL, so this
returns NULL for the same reason.

 We could drop the return value from
 this helper entirely (or make it -1, but of course no callers would use
 it) and write it out long-hand in the callers:

   if (!branch-merge) {
   error_buf(err, ...);
   return NULL;
   }

 That is really not so bad, as there are only a handful of callers.

That may happen when somebody (perhaps Jonathan?) wants to push the
let's optionally pass strbuf to format messages into approach
forward, and most likely be done by adding a similar function to
usage.c next to error_builtin() and friends.  As long as we do not
forget to reimplement this helper in terms of that public function
when it happens, what we have right now after this patch would be
fine.

 Yeah, my goal here was to faithfully keep the same logic, but I had a
 similar head-scratching moment. What would make more sense to me is:

   if (!branch)
   return error(HEAD does not point to a branch);

   if (!branch-merge || !branch-merge[0]) {
   /*
* no merge config; is it because the user didn't define any, or
* because it is not a real branch, and get_branch auto-vivified
* it?
*/
   if (!ref_exists(branch-refname))
   return error(no such branch);
   return error(no upstream configured for branch);
   }

   if (!branch-merge[0]-dst)
   return error(upstream branch not stored as a remote-tracking branch);

   return branch-merge[0]-dst;

 Hopefully the comment there answers your question; it is not that we
 truly care whether the ref exists, but only that we are trying to tell
 the difference between a real branch and a struct that is an artifact
 of our internal code.

Well, answering my question here on the list wouldn't help future
readers of the code very much ;-)

 Note also that the original may dereference branch-merge[0] even if it
 is NULL. I think that can't actually happen in practice (we only
 allocate branch-merge if we have at least one item to put in it, and
 all of the checks of branch-merge[0] are actually being over-careful).
 But the code I just wrote above does not have that problem.

Perhaps update the patch with this explanation in the log message,
as a separate preparatory step?

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


  1   2   >