Re: [PATCH v2] Provide some linguistic guidance for the documentation.

2013-08-02 Thread Jonathan Nieder
Junio C Hamano wrote:

 Is that accurate?  My impression has been:

 The documentation liberally mixes US and UK English (en_US/UK)
 norms for spelling and grammar, which is somewhat unfortunate.
 In an ideal world, it would have been better if it consistently
 used only one and not the other, and we would have picked en_US.

I'm not convinced that would be better, even in an ideal world.

It's certainly useful to have a consistent spelling of each term to
make searching with grep easier.  But searches with grep do not
work well with line breaks anyway, and search engines for larger
collections of documents seem to know about the usual spelling
variants (along with knowing about stemming, etc).  Unless we are
planning to provide a separate en_GB translation, it seems unfortunate
to consistently have everything spelled in the natural way for one
group of people and in an unnatural way for another, just in the name
of having a convention.

I am not sure it makes sense for the documentation to say A huge
disruptive patch of such-and-such specific kind of no immediate
benefit is unwelcome.  Isn't there some more general principle that
implies that?  Or the CodingGuidelines could simply say

The documentation uses a mixture of U.S. and British English.

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


[regression] Re: git-cat-file --batch reversion; cannot query filenames with spaces

2013-08-02 Thread Jonathan Nieder
Hi Joey,

Joey Hess wrote[1]:

 Commit c334b87b30c1464a1ab563fe1fb8de5eaf0e5bac caused a reversion in
 git-cat-file --batch. 

 With an older version:

 joey@gnu:~/tmp/rrrgit cat-file --batch
 :file name
 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 blob 0

 With the new version:

 joey@wren:~/tmp/rgit cat-file --batch
 :file name
 :file missing

 This has broken git-annex's support for operating on files/directories
 containing whitespace. I cannot see a way to query such a filename using
 the new interface.

Oh dear.  Luckily you caught this before the final 1.8.4 release.  I
wonder if we should just revert c334b87b (cat-file: split --batch
input lines on whitespace, 2013-07-11) for now.

Thanks,
Jonathan

[1] http://bugs.debian.org/718517
--
To unsubscribe from this list: send the line 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 03/13] contrib/subtree: Better Error Handling for add

2013-08-02 Thread Andreas Schwab
David A. Greene gree...@obbligato.org writes:

   elif [ $# -eq 2 ]; then
 - cmd_add_repository $@
 + git rev-parse -q --verify $2^{commit} /dev/null ||
 + die '$2' does not refer to a commit

That doesn't make any sense.  $2 is a commit in a remote repository
which hasn't been fetched yet, so it can never be verified against the
local repository.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
And now for something completely different.
--
To unsubscribe from this list: send the line 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: [regression] Re: git-cat-file --batch reversion; cannot query filenames with spaces

2013-08-02 Thread Jeff King
On Thu, Aug 01, 2013 at 11:40:03PM -0700, Jonathan Nieder wrote:

  Commit c334b87b30c1464a1ab563fe1fb8de5eaf0e5bac caused a reversion in
  git-cat-file --batch.
 
  With an older version:
 
  joey@gnu:~/tmp/rrrgit cat-file --batch
  :file name
  e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 blob 0
 
  With the new version:
 
  joey@wren:~/tmp/rgit cat-file --batch
  :file name
  :file missing
 [...]
 Oh dear.  Luckily you caught this before the final 1.8.4 release.  I
 wonder if we should just revert c334b87b (cat-file: split --batch
 input lines on whitespace, 2013-07-11) for now.

Ugh. Yeah, the incorrect assumption from the commit message of c334b87b
is Object names cannot contain spaces Refs cannot, but filename
specifiers after a colon can.

We need to revert that commit before the release. It can either be
replaced with:

  1. A --split (or similar) option to use the behavior only when
 desired.

  2. Enabling splitting only when %(rest) is used in the output format.

And I suppose it is too late in the cycle for either of those to go into
v1.8.4. That's a shame, but I think losing that particular patch does
not affect the rest of the series, so we are OK to ship without it.

Thanks Joey for a timely bug report.

-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: [regression] Re: git-cat-file --batch reversion; cannot query filenames with spaces

2013-08-02 Thread Jeff King
On Fri, Aug 02, 2013 at 03:54:02AM -0700, Jeff King wrote:

 We need to revert that commit before the release. It can either be
 replaced with:
 
   1. A --split (or similar) option to use the behavior only when
  desired.
 
   2. Enabling splitting only when %(rest) is used in the output format.

Of the two, I think the latter is more sensible; the former is
unnecessarily placing the burden on the user to match --split with
their use of %(rest). The second is pointless without the first.

A patch to implement (2) is below.

By the way, Joey, I am not sure how safe git cat-file --batch-check is
for arbitrary filenames. In particular, I don't know how it would react
to a filename with an embedded newline (and I do not think it will undo
quoting). Certainly that does not excuse this regression; even if what
you are doing is not 100% reliable, it is good enough in sane situations
and we should not be breaking it. But you may want to double-check the
behavior of your scripts in such a case, and we may need to add a -z
to support it reliably.

The rev-list --objects output may contain such paths, of course, but
they will be quoted, and %(rest) does not care (it is not trying to
interpret the paths, but will reliably relay the quoted bits to the
output).

-- 8 --
Subject: [PATCH] cat-file: only split on whitespace when %(rest) is used

Commit c334b87 recently taught `cat-file --batch-check` to
split input lines on whitespace, and stash everything after
the first token into the %(rest) output format element. That
commit claims:

   Object names cannot contain spaces, so any input with
   spaces would have resulted in a missing line.

But that is not correct. Refs, object sha1s, and various
peeling suffixes cannot contain spaces, but some object
names can. In particular:

  1. Tree paths like [tree]:path with whitespace

  2. Reflog specifications like @{2 days ago}

  3. Commit searches like rev^{/grep me} or :/grep me

To remain backwards compatible, we cannot split on
whitespace by default. This patch teaches cat-file to only
do the splitting when %(rest) is used by the output
format. Since that element did not exist at all until
c334b87, old scripts cannot be affected.

The existence of object names with spaces does mean that you
cannot reliably do:

  echo :path with space and other data |
  git cat-file --batch-check=%(objectname) %(rest)

as it would split the path and feed only :path to
get_sha1. But that command is nonsensical. If you wanted to
see and other data in %(rest), git cannot possibly know
where the filename ends and the rest begins.

It might be more robust to have something like -z to
separate the input elements. But this patch is still a
reasonable step before having that.  It makes the easy cases
easy; people who do not care about %(rest) do not have to
consider it, and the %(rest) code handles the spaces and
newlines of rev-list --objects correctly.

Hard cases remain hard but possible (if you might get
whitespace in your input, you do not get to use %(rest) and
must split and join the output yourself using more flexible
tools). And most importantly, it does not preclude us from
having different splitting rules later if a -z (or
similar) option is added.  So we can make the hard
cases easier later, if we choose to.

Signed-off-by: Jeff King p...@peff.net
---
 Documentation/git-cat-file.txt | 16 
 builtin/cat-file.c | 31 +--
 t/t1006-cat-file.sh|  8 
 3 files changed, 37 insertions(+), 18 deletions(-)

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 3ddec0b..21cffe2 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -86,12 +86,9 @@ If `--batch` or `--batch-check` is given, `cat-file` will 
read objects
 
 
 If `--batch` or `--batch-check` is given, `cat-file` will read objects
-from stdin, one per line, and print information about them.
-
-Each line is split at the first whitespace boundary. All characters
-before that whitespace are considered as a whole object name, and are
-parsed as if given to linkgit:git-rev-parse[1]. Characters after that
-whitespace can be accessed using the `%(rest)` atom (see below).
+from stdin, one per line, and print information about them. By default,
+the whole line is considered as an object, as if it were fed to
+linkgit:git-rev-parse[1].
 
 You can specify the information shown for each object by using a custom
 `format`. The `format` is copied literally to stdout for each
@@ -113,8 +110,11 @@ newline. The available atoms are:
note about on-disk sizes in the `CAVEATS` section below.
 
 `rest`::
-   The text (if any) found after the first run of whitespace on the
-   input line (i.e., the rest of the line).
+   If this atom is used in the output string, input lines are split
+   at the first whitespace boundary. All characters before that
+   whitespace are considered to 

Rewriting git-repack.sh in C

2013-08-02 Thread Stefan Beller
Hello,

I'd like to rewrite the repack shell script in C.
So I tried the naive approach reading the man page and 
the script itself and write C program by matching each block/line 
of the script with a function in C

Now I stumble upon other git commands (git pack-objects).
What's the best way to approach such a plumbing command?

I don't think just calling cmd_pack_objects(argc, **argv) would 
be the right thing to do, as we're not using all the command 
line parameters, so some of the logic in cmd_pack_object could 
be skipped.
Another approach would be to use some of the functions as used 
by cmd_pack_objects, but these mostly reside in builtin/pack_objects.c
They'd need to be moved up to pack.h/pack.c.

So my question is, how you'd generally approach rewriting a 
shell script in C.

Stefan







signature.asc
Description: OpenPGP digital signature


Re: Rewriting git-repack.sh in C

2013-08-02 Thread Duy Nguyen
On Fri, Aug 2, 2013 at 8:48 PM, Stefan Beller
stefanbel...@googlemail.com wrote:
 Hello,

 I'd like to rewrite the repack shell script in C.
 So I tried the naive approach reading the man page and
 the script itself and write C program by matching each block/line
 of the script with a function in C

 Now I stumble upon other git commands (git pack-objects).
 What's the best way to approach such a plumbing command?

 I don't think just calling cmd_pack_objects(argc, **argv) would
 be the right thing to do, as we're not using all the command
 line parameters, so some of the logic in cmd_pack_object could
 be skipped.
 Another approach would be to use some of the functions as used
 by cmd_pack_objects, but these mostly reside in builtin/pack_objects.c
 They'd need to be moved up to pack.h/pack.c.

 So my question is, how you'd generally approach rewriting a
 shell script in C.

Start a new process via start_command/run_command interface. It's
safer to retain the process boundary at this stage. You can try to
integrate further later.
-- 
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 v2] Provide some linguistic guidance for the documentation.

2013-08-02 Thread Marc Branchaud

On 13-08-02 02:25 AM, Jonathan Nieder wrote:

Junio C Hamano wrote:


Is that accurate?  My impression has been:

 The documentation liberally mixes US and UK English (en_US/UK)
 norms for spelling and grammar, which is somewhat unfortunate.
 In an ideal world, it would have been better if it consistently
 used only one and not the other, and we would have picked en_US.


I'm not convinced that would be better, even in an ideal world.

It's certainly useful to have a consistent spelling of each term to
make searching with grep easier.  But searches with grep do not
work well with line breaks anyway, and search engines for larger
collections of documents seem to know about the usual spelling
variants (along with knowing about stemming, etc).  Unless we are
planning to provide a separate en_GB translation, it seems unfortunate
to consistently have everything spelled in the natural way for one
group of people and in an unnatural way for another, just in the name
of having a convention.


Personally I find it distracting when the norms are mixed.  I don't 
think the current mishmash pleases anyone (as evidenced by the steady 
stream of patches that change spellings).



I am not sure it makes sense for the documentation to say A huge
disruptive patch of such-and-such specific kind of no immediate
benefit is unwelcome.  Isn't there some more general principle that
implies that?  Or the CodingGuidelines could simply say

The documentation uses a mixture of U.S. and British English.


I'm hoping this patch will help the list avoid seeing patches that 
merely flip between alternate spellings.  (Perhaps the commit message 
should state this?)  I think it's important to be clear about the kind 
of work the git community wants to see.  So I don't think that single 
sentence by itself would be helpful.


In the case of alternate spellings in the documentation, I think there's 
a temptation to create a blanket patch that fixes lots of perceived 
misspellings since such changes are mere typographic tweaks.  It's a bit 
easy to overlook the disruptive nature of such a patch, so IMO it bears 
repeating here.


Are you suggesting maybe that there should be no official dialect? 
That the guidance should simply say that we don't want to see patches 
that merely flip between alternate spellings (because we don't really care)?


M.

--
To unsubscribe from this list: send the line 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: [regression] Re: git-cat-file --batch reversion; cannot query filenames with spaces

2013-08-02 Thread Joey Hess
Jeff King wrote:
 By the way, Joey, I am not sure how safe git cat-file --batch-check is
 for arbitrary filenames. In particular, I don't know how it would react
 to a filename with an embedded newline (and I do not think it will undo
 quoting). Certainly that does not excuse this regression; even if what
 you are doing is not 100% reliable, it is good enough in sane situations
 and we should not be breaking it. But you may want to double-check the
 behavior of your scripts in such a case, and we may need to add a -z
 to support it reliably.

Yes, I would prefer to have a -z mode. I think my code otherwise handles
newlines.

Thanks for the quick fix. I agree that only enabling the behavior with
%{rest} makes sense.

-- 
see shy jo


signature.asc
Description: Digital signature


Re: [regression] Re: git-cat-file --batch reversion; cannot query filenames with spaces

2013-08-02 Thread Brandon Casey
On Fri, Aug 2, 2013 at 8:27 AM, Joey Hess j...@kitenet.net wrote:
 Jeff King wrote:
 By the way, Joey, I am not sure how safe git cat-file --batch-check is
 for arbitrary filenames. In particular, I don't know how it would react
 to a filename with an embedded newline (and I do not think it will undo
 quoting). Certainly that does not excuse this regression; even if what
 you are doing is not 100% reliable, it is good enough in sane situations
 and we should not be breaking it. But you may want to double-check the
 behavior of your scripts in such a case, and we may need to add a -z
 to support it reliably.

 Yes, I would prefer to have a -z mode. I think my code otherwise handles
 newlines.

 Thanks for the quick fix. I agree that only enabling the behavior with
 %{rest} makes sense.

 --
 see shy jo

/methinks we've identified a gap in our test coverage.  Care to add a
test that covers the functionality that git-annex depends on?

-Brandon
--
To unsubscribe from this list: send the line 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] sha1_file: introduce close_one_pack() to close packs on fd pressure

2013-08-02 Thread Junio C Hamano
Brandon Casey draf...@gmail.com writes:

 +/*
 + * The LRU pack is the one with the oldest MRU window, preferring packs
 + * with no used windows, or the oldest mtime if it has no windows allocated.
 + */
 +static void find_lru_pack(struct packed_git *p, struct packed_git **lru_p, 
 struct pack_window **mru_w, int *accept_windows_inuse)
 +{
 + struct pack_window *w, *this_mru_w;
 + int has_windows_inuse = 0;
 +
 + /*
 +  * Reject this pack if it has windows and the previously selected
 +  * one does not.  If this pack does not have windows, reject
 +  * it if the pack file is newer than the previously selected one.
 +  */
 + if (*lru_p  !*mru_w  (p-windows || p-mtime  (*lru_p)-mtime))
 + return;
 +
 + for (w = this_mru_w = p-windows; w; w = w-next) {
 + /*
 +  * Reject this pack if any of its windows are in use,
 +  * but the previously selected pack did not have any
 +  * inuse windows.  Otherwise, record that this pack
 +  * has windows in use.
 +  */
 + if (w-inuse_cnt) {
 + if (*accept_windows_inuse)
 + has_windows_inuse = 1;
 + else
 + return;
 + }
 +
 + if (w-last_used  this_mru_w-last_used)
 + this_mru_w = w;
 +
 + /*
 +  * Reject this pack if it has windows that have been
 +  * used more recently than the previously selected pack.
 +  * If the previously selected pack had windows inuse and
 +  * we have not encountered a window in this pack that is
 +  * inuse, skip this check since we prefer a pack with no
 +  * inuse windows to one that has inuse windows.
 +  */
 + if (*mru_w  *accept_windows_inuse == has_windows_inuse 
 + this_mru_w-last_used  (*mru_w)-last_used)
 + return;

The *accept_windows_inuse == has_windows_inuse part is hard to
grok, together with the fact that this statement is evaluated for
each and every w, even though it is about this_mru_w and that
variable is not updated in every iteration of the loop.  Can you
clarify/simplify this part of the code a bit more?

For example, would the above be equivalent to this?

if (w-last_used  this_mru_w-last_used)
continue;

this_mru_w = w;
if (has_windows_inuse  *mru_w 
w-last_used  (*mru_w)-last_used)
return;

That is, if we already know a more recently used window in this
pack, we do not have to do anything to maintain mru_w.  Otherwise,
remember that this window is the most recently used one in this
pack, and if it is newer than the newest one from the pack we are
going to pick, we refrain from picking this pack.

But we do not reject ourselves if we haven't seen a window that is
in use (yet).

 + }
 +
 + /*
 +  * Select this pack.
 +  */
 + *mru_w = this_mru_w;
 + *lru_p = p;
 + *accept_windows_inuse = has_windows_inuse;
 +}
 +
 +static int close_one_pack(void)
 +{
 + struct packed_git *p, *lru_p = NULL;
 + struct pack_window *mru_w = NULL;
 + int accept_windows_inuse = 1;
 +
 + for (p = packed_git; p; p = p-next) {
 + if (p-pack_fd == -1)
 + continue;
 + find_lru_pack(p, lru_p, mru_w, accept_windows_inuse);
 + }
 +
 + if (lru_p) {
 + close(lru_p-pack_fd);
 + pack_open_fds--;
 + lru_p-pack_fd = -1;
 + return 1;
 + }
 +
 + return 0;
 +}
 +
  void unuse_pack(struct pack_window **w_cursor)
  {
   struct pack_window *w = *w_cursor;
 @@ -768,7 +845,7 @@ static int open_packed_git_1(struct packed_git *p)
   pack_max_fds = 1;
   }
  
 - while (pack_max_fds = pack_open_fds  unuse_one_window(NULL, -1))
 + while (pack_max_fds = pack_open_fds  close_one_pack())
   ; /* nothing */
  
   p-pack_fd = git_open_noatime(p-pack_name);
--
To unsubscribe from this list: send the line 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: [regression] Re: git-cat-file --batch reversion; cannot query filenames with spaces

2013-08-02 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 We need to revert that commit before the release. It can either be
 replaced with:

   1. A --split (or similar) option to use the behavior only when
  desired.

   2. Enabling splitting only when %(rest) is used in the output format.

 And I suppose it is too late in the cycle for either of those to go into
 v1.8.4. That's a shame, but I think losing that particular patch does
 not affect the rest of the series, so we are OK to ship without it.

 Thanks Joey for a timely bug report.

Thanks.  Will do this to jk/cat-file-batch-optim topic and merge it
to 'master' for now.

-- 8 --
Subject: [PATCH] Revert cat-file: split --batch input lines on whitespace

This reverts commit c334b87b30c1464a1ab563fe1fb8de5eaf0e5bac; the
update assumed that people only used the command to read from
rev-list --objects output, whose lines begin with a 40-hex object
name followed by a whitespace, but it turns out that scripts feed
random extended SHA-1 expressions (e.g. HEAD:$pathname) in which
a whitespace has to be kept.
---
 Documentation/git-cat-file.txt | 10 ++
 builtin/cat-file.c | 20 +---
 t/t1006-cat-file.sh|  7 ---
 3 files changed, 3 insertions(+), 34 deletions(-)

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 3ddec0b..10fbc6a 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -88,10 +88,8 @@ BATCH OUTPUT
 If `--batch` or `--batch-check` is given, `cat-file` will read objects
 from stdin, one per line, and print information about them.
 
-Each line is split at the first whitespace boundary. All characters
-before that whitespace are considered as a whole object name, and are
-parsed as if given to linkgit:git-rev-parse[1]. Characters after that
-whitespace can be accessed using the `%(rest)` atom (see below).
+Each line is considered as a whole object name, and is parsed as if
+given to linkgit:git-rev-parse[1].
 
 You can specify the information shown for each object by using a custom
 `format`. The `format` is copied literally to stdout for each
@@ -112,10 +110,6 @@ newline. The available atoms are:
The size, in bytes, that the object takes up on disk. See the
note about on-disk sizes in the `CAVEATS` section below.
 
-`rest`::
-   The text (if any) found after the first run of whitespace on the
-   input line (i.e., the rest of the line).
-
 If no format is specified, the default format is `%(objectname)
 %(objecttype) %(objectsize)`.
 
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 163ce6c..4253460 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -119,7 +119,6 @@ struct expand_data {
enum object_type type;
unsigned long size;
unsigned long disk_size;
-   const char *rest;
 
/*
 * If mark_query is true, we do not expand anything, but rather
@@ -164,9 +163,6 @@ static void expand_atom(struct strbuf *sb, const char 
*atom, int len,
data-info.disk_sizep = data-disk_size;
else
strbuf_addf(sb, %lu, data-disk_size);
-   } else if (is_atom(rest, atom, len)) {
-   if (!data-mark_query  data-rest)
-   strbuf_addstr(sb, data-rest);
} else
die(unknown format element: %.*s, len, atom);
 }
@@ -277,21 +273,7 @@ static int batch_objects(struct batch_options *opt)
warn_on_object_refname_ambiguity = 0;
 
while (strbuf_getline(buf, stdin, '\n') != EOF) {
-   char *p;
-   int error;
-
-   /*
-* Split at first whitespace, tying off the beginning of the
-* string and saving the remainder (or NULL) in data.rest.
-*/
-   p = strpbrk(buf.buf,  \t);
-   if (p) {
-   while (*p  strchr( \t, *p))
-   *p++ = '\0';
-   }
-   data.rest = p;
-
-   error = batch_one_object(buf.buf, opt, data);
+   int error = batch_one_object(buf.buf, opt, data);
if (error)
return error;
}
diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index d499d02..4e911fb 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -78,13 +78,6 @@ $content
echo $sha1 | git cat-file --batch-check=%(objecttype) %(objectname) 
actual 
test_cmp expect actual
 '
-
-test_expect_success '--batch-check with %(rest)' '
-   echo $type this is some extra content expect 
-   echo $sha1this is some extra content |
-   git cat-file --batch-check=%(objecttype) %(rest) actual 
-   test_cmp expect actual
-'
 }
 
 hello_content=Hello World
-- 
1.8.4-rc1-125-g7a0ec02

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

Re: Rewriting git-repack.sh in C

2013-08-02 Thread Duy Nguyen
On Fri, Aug 02, 2013 at 09:10:59PM +0700, Duy Nguyen wrote:
 On Fri, Aug 2, 2013 at 8:48 PM, Stefan Beller
 stefanbel...@googlemail.com wrote:
  Hello,
 
  I'd like to rewrite the repack shell script in C.
  So I tried the naive approach reading the man page and
  the script itself and write C program by matching each block/line
  of the script with a function in C
 
  ...
 
  So my question is, how you'd generally approach rewriting a
  shell script in C.
 
 Start a new process via start_command/run_command interface. It's
 safer to retain the process boundary at this stage. You can try to
 integrate further later.

I was in the middle of something and somehow read this as rewriting
git-rebase.sh :-X

For git-repack, because it ends with a single pack-objects call, we
might not need a new process after all (very much like tail call
optimization). This is what I got for some time, but still not polish
it for submission (and it may be a bit broken after the last
rebase). Maybe you can work off this, or from scratch if you think
it's too messy. It basically teaches pack-objects extra features that
repack needs, then make repack a wrapper of pack-objects.

-- 8 --
commit 25569a3958c3272b3eb5fa50dea680948f7a2768 (build-in-repack)
Author: Nguyễn Thái Ngọc Duy pclo...@gmail.com
Date:   Wed Nov 9 19:21:39 2011 +0700

Build in git-repack

pack-objects learns a few more options to take over what's been done
by git-repack.sh. cmd_repack() becomes a wrapper around
cmd_pack_objects().

diff --git a/Makefile b/Makefile
index 0f931a2..b4010a6 100644
--- a/Makefile
+++ b/Makefile
@@ -460,7 +460,6 @@ SCRIPT_SH += git-mergetool.sh
 SCRIPT_SH += git-pull.sh
 SCRIPT_SH += git-quiltimport.sh
 SCRIPT_SH += git-rebase.sh
-SCRIPT_SH += git-repack.sh
 SCRIPT_SH += git-request-pull.sh
 SCRIPT_SH += git-stash.sh
 SCRIPT_SH += git-submodule.sh
@@ -584,6 +583,7 @@ BUILT_INS += git-init$X
 BUILT_INS += git-merge-subtree$X
 BUILT_INS += git-peek-remote$X
 BUILT_INS += git-repo-config$X
+BUILT_INS += git-repack$X
 BUILT_INS += git-show$X
 BUILT_INS += git-stage$X
 BUILT_INS += git-status$X
diff --git a/builtin.h b/builtin.h
index 64bab6b..feb958f 100644
--- a/builtin.h
+++ b/builtin.h
@@ -117,6 +117,7 @@ extern int cmd_reflog(int argc, const char **argv, const 
char *prefix);
 extern int cmd_remote(int argc, const char **argv, const char *prefix);
 extern int cmd_remote_ext(int argc, const char **argv, const char *prefix);
 extern int cmd_remote_fd(int argc, const char **argv, const char *prefix);
+extern int cmd_repack(int argc, const char **argv, const char *prefix);
 extern int cmd_repo_config(int argc, const char **argv, const char *prefix);
 extern int cmd_rerere(int argc, const char **argv, const char *prefix);
 extern int cmd_reset(int argc, const char **argv, const char *prefix);
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index f069462..1742ea1 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -18,10 +18,17 @@
 #include refs.h
 #include streaming.h
 #include thread-utils.h
+#include sigchain.h
 
 static const char *pack_usage[] = {
N_(git pack-objects --stdout [options...] [ ref-list |  
object-list]),
N_(git pack-objects [options...] base-name [ ref-list |  
object-list]),
+   N_(git pack-objects --repack [options...]),
+   NULL
+};
+
+static char const * const repack_usage[] = {
+   N_(git repack [options]),
NULL
 };
 
@@ -103,6 +110,15 @@ static struct object_entry *locate_object_entry(const 
unsigned char *sha1);
 static uint32_t written, written_delta;
 static uint32_t reused, reused_delta;
 
+#define REPACK_IN_PROGRESS (1  0)
+#define REPACK_UPDATE_INFO (1  1)
+#define REPACK_ALL_INTO_ONE(1  2)
+#define REPACK_REMOVE_REDUNDANT(1  3)
+
+static int repack_flags, nr_written_packs;
+static int repack_usedeltabaseoffset;
+static struct string_list written_packs;
+static struct string_list backup_files;
 
 static void *get_delta(struct object_entry *entry)
 {
@@ -792,9 +808,19 @@ static void write_pack_file(void)
snprintf(tmpname, sizeof(tmpname), %s-, base_name);
finish_tmp_packfile(tmpname, pack_tmp_name,
written_list, nr_written,
-   pack_idx_opts, sha1);
+   pack_idx_opts, sha1,
+   repack_flags  REPACK_IN_PROGRESS ?
+   backup_files : NULL);
free(pack_tmp_name);
-   puts(sha1_to_hex(sha1));
+   if (repack_flags  REPACK_IN_PROGRESS) {
+   int len = strlen(tmpname);
+   char *s = xmalloc(len + 2);
+   memcpy(s, tmpname, len - 4);
+   memcpy(s + len - 4, .pack, 6);

Re: [regression] Re: git-cat-file --batch reversion; cannot query filenames with spaces

2013-08-02 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Fri, Aug 02, 2013 at 03:54:02AM -0700, Jeff King wrote:

 We need to revert that commit before the release. It can either be
 replaced with:
 
   1. A --split (or similar) option to use the behavior only when
  desired.
 
   2. Enabling splitting only when %(rest) is used in the output format.

 Of the two, I think the latter is more sensible; the former is
 unnecessarily placing the burden on the user to match --split with
 their use of %(rest). The second is pointless without the first.

 A patch to implement (2) is below.

As I'd queue this on top of the revert, I had to wrangle it a bit to
make it relative, i.e. this resurrects what the other reverted
patch did but in a weaker/safer form.

This will be kept outside this cycle.  Thanks for a quick fix.


--
To unsubscribe from this list: send the line 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] sha1_file: introduce close_one_pack() to close packs on fd pressure

2013-08-02 Thread Brandon Casey
On Fri, Aug 2, 2013 at 9:26 AM, Junio C Hamano gits...@pobox.com wrote:
 Brandon Casey draf...@gmail.com writes:

 +/*
 + * The LRU pack is the one with the oldest MRU window, preferring packs
 + * with no used windows, or the oldest mtime if it has no windows allocated.
 + */
 +static void find_lru_pack(struct packed_git *p, struct packed_git **lru_p, 
 struct pack_window **mru_w, int *accept_windows_inuse)
 +{
 + struct pack_window *w, *this_mru_w;
 + int has_windows_inuse = 0;
 +
 + /*
 +  * Reject this pack if it has windows and the previously selected
 +  * one does not.  If this pack does not have windows, reject
 +  * it if the pack file is newer than the previously selected one.
 +  */
 + if (*lru_p  !*mru_w  (p-windows || p-mtime  (*lru_p)-mtime))
 + return;
 +
 + for (w = this_mru_w = p-windows; w; w = w-next) {
 + /*
 +  * Reject this pack if any of its windows are in use,
 +  * but the previously selected pack did not have any
 +  * inuse windows.  Otherwise, record that this pack
 +  * has windows in use.
 +  */
 + if (w-inuse_cnt) {
 + if (*accept_windows_inuse)
 + has_windows_inuse = 1;
 + else
 + return;
 + }
 +
 + if (w-last_used  this_mru_w-last_used)
 + this_mru_w = w;
 +
 + /*
 +  * Reject this pack if it has windows that have been
 +  * used more recently than the previously selected pack.
 +  * If the previously selected pack had windows inuse and
 +  * we have not encountered a window in this pack that is
 +  * inuse, skip this check since we prefer a pack with no
 +  * inuse windows to one that has inuse windows.
 +  */
 + if (*mru_w  *accept_windows_inuse == has_windows_inuse 
 + this_mru_w-last_used  (*mru_w)-last_used)
 + return;

 The *accept_windows_inuse == has_windows_inuse part is hard to
 grok, together with the fact that this statement is evaluated for
 each and every w, even though it is about this_mru_w and that
 variable is not updated in every iteration of the loop.  Can you
 clarify/simplify this part of the code a bit more?

 For example, would the above be equivalent to this?

 if (w-last_used  this_mru_w-last_used)
 continue;

 this_mru_w = w;
 if (has_windows_inuse  *mru_w 
 w-last_used  (*mru_w)-last_used)
 return;

 That is, if we already know a more recently used window in this
 pack, we do not have to do anything to maintain mru_w.  Otherwise,
 remember that this window is the most recently used one in this
 pack, and if it is newer than the newest one from the pack we are
 going to pick, we refrain from picking this pack.

 But we do not reject ourselves if we haven't seen a window that is
 in use (yet).

No that wouldn't be the same.  The function of *accept_windows_inuse
== has_windows_inuse and the testing of this_mru_w in every loop
rather than w, is too subtle.  I tried to draw attention to it in the
comment, but I agree it's not enough.

The case that your example would not catch is when the new pack's mru
window has already been found, but has_windows_inuse is not set until
later.  When has_windows_inuse is later set, we need to test
this_mru_w regardless of whether we have just assigned it.

For example, if mru_w points to a pack with last_used == 11 and
*accept_windows_inuse = 1, and p-windows looks like this:

   last_used  in_use
   12 0
   10 1

Then the first time through the loop, this_mru_w would be set to the
first window with last_used equal to 12.  The if statement that tests
this_mru_w-last_used  (*mru_w)-last_used would be skipped since
has_windows_inuse would still be 0.  The second time through the loop,
this_mru_w would _not_ be reset, but has_windows_inuse _would_ be set.
 This time, we would want to enter the last for loop so that we can
reject the pack.

I'll try to rework this loop or add comments to clarify.

-Brandon


 + }
 +
 + /*
 +  * Select this pack.
 +  */
 + *mru_w = this_mru_w;
 + *lru_p = p;
 + *accept_windows_inuse = has_windows_inuse;
 +}
 +
 +static int close_one_pack(void)
 +{
 + struct packed_git *p, *lru_p = NULL;
 + struct pack_window *mru_w = NULL;
 + int accept_windows_inuse = 1;
 +
 + for (p = packed_git; p; p = p-next) {
 + if (p-pack_fd == -1)
 + continue;
 + find_lru_pack(p, lru_p, mru_w, accept_windows_inuse);
 + }
 +
 + if (lru_p) {
 + close(lru_p-pack_fd);
 + pack_open_fds--;
 + lru_p-pack_fd = -1;
 + return 1;
 + }
 +
 + 

Re: [regression] Re: git-cat-file --batch reversion; cannot query filenames with spaces

2013-08-02 Thread Jeff King
On Fri, Aug 02, 2013 at 09:41:52AM -0700, Junio C Hamano wrote:

  Of the two, I think the latter is more sensible; the former is
  unnecessarily placing the burden on the user to match --split with
  their use of %(rest). The second is pointless without the first.
 
  A patch to implement (2) is below.
 
 As I'd queue this on top of the revert, I had to wrangle it a bit to
 make it relative, i.e. this resurrects what the other reverted
 patch did but in a weaker/safer form.

Yeah, sorry. After doing the patch I had the thought that maybe the
least invasive thing would be the fix rather than the straight revert
(we are counting on my assertion that just reverting out part of the
series will be OK; I'm pretty sure that is the case, but it is not
risk-free, either).

I didn't see the result of your wrangling in pu, but I will keep an eye
out to double-check it (unless you did not finish, in which case I am
happy to do the wrangling myself).

-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: [regression] Re: git-cat-file --batch reversion; cannot query filenames with spaces

2013-08-02 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Fri, Aug 02, 2013 at 09:41:52AM -0700, Junio C Hamano wrote:

  Of the two, I think the latter is more sensible; the former is
  unnecessarily placing the burden on the user to match --split with
  their use of %(rest). The second is pointless without the first.
 
  A patch to implement (2) is below.
 
 As I'd queue this on top of the revert, I had to wrangle it a bit to
 make it relative, i.e. this resurrects what the other reverted
 patch did but in a weaker/safer form.

 Yeah, sorry. After doing the patch I had the thought that maybe the
 least invasive thing would be the fix rather than the straight revert
 (we are counting on my assertion that just reverting out part of the
 series will be OK; I'm pretty sure that is the case, but it is not
 risk-free, either).

 I didn't see the result of your wrangling in pu, but I will keep an eye
 out to double-check it (unless you did not finish, in which case I am
 happy to do the wrangling myself).

Here is what is on top of the revert that has been pushed out on
'pu'.

Thanks.

-- 8 --
From: Jeff King p...@peff.net
Date: Fri, 2 Aug 2013 04:59:07 -0700
Subject: [PATCH] cat-file: only split on whitespace when %(rest) is used

Commit c334b87b (cat-file: split --batch input lines on whitespace,
2013-07-11) taught `cat-file --batch-check` to split input lines on
the first whitespace, and stash everything after the first token
into the %(rest) output format element.  It claimed:

   Object names cannot contain spaces, so any input with
   spaces would have resulted in a missing line.

But that is not correct.  Refs, object sha1s, and various peeling
suffixes cannot contain spaces, but some object names can. In
particular:

  1. Tree paths like [tree]:path with whitespace

  2. Reflog specifications like @{2 days ago}

  3. Commit searches like rev^{/grep me} or :/grep me

To remain backwards compatible, we cannot split on whitespace by
default, hence we will ship 1.8.4 with the commit reverted.

Resurrect its attempt but in a weaker form; only do the splitting
when %(rest) is used in the output format. Since that element did
not exist at all before c334b87, old scripts cannot be affected.

The existence of object names with spaces does mean that you
cannot reliably do:

  echo :path with space and other data |
  git cat-file --batch-check=%(objectname) %(rest)

as it would split the path and feed only :path to get_sha1. But
that command is nonsensical. If you wanted to see and other data
in %(rest), git cannot possibly know where the filename ends and
the rest begins.

It might be more robust to have something like -z to separate the
input elements. But this patch is still a reasonable step before
having that.  It makes the easy cases easy; people who do not care
about %(rest) do not have to consider it, and the %(rest) code
handles the spaces and newlines of rev-list --objects correctly.

Hard cases remain hard but possible (if you might get whitespace in
your input, you do not get to use %(rest) and must split and join
the output yourself using more flexible tools). And most
importantly, it does not preclude us from having different splitting
rules later if a -z (or similar) option is added.  So we can make
the hard cases easier later, if we choose to.

Signed-off-by: Jeff King p...@peff.net
Signed-off-by: Junio C Hamano gits...@pobox.com
---
 Documentation/git-cat-file.txt | 14 ++
 builtin/cat-file.c | 31 ++-
 t/t1006-cat-file.sh| 15 +++
 3 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-cat-file.txt b/Documentation/git-cat-file.txt
index 10fbc6a..21cffe2 100644
--- a/Documentation/git-cat-file.txt
+++ b/Documentation/git-cat-file.txt
@@ -86,10 +86,9 @@ BATCH OUTPUT
 
 
 If `--batch` or `--batch-check` is given, `cat-file` will read objects
-from stdin, one per line, and print information about them.
-
-Each line is considered as a whole object name, and is parsed as if
-given to linkgit:git-rev-parse[1].
+from stdin, one per line, and print information about them. By default,
+the whole line is considered as an object, as if it were fed to
+linkgit:git-rev-parse[1].
 
 You can specify the information shown for each object by using a custom
 `format`. The `format` is copied literally to stdout for each
@@ -110,6 +109,13 @@ newline. The available atoms are:
The size, in bytes, that the object takes up on disk. See the
note about on-disk sizes in the `CAVEATS` section below.
 
+`rest`::
+   If this atom is used in the output string, input lines are split
+   at the first whitespace boundary. All characters before that
+   whitespace are considered to be the object name; characters
+   after that first run of whitespace (i.e., the rest of the
+   line) are output in place of the `%(rest)` atom.
+
 If no format is specified, the default format is `%(objectname)
 %(objecttype) 

[PATCH] Documentation/rev-list-options: add missing word in --*-parents

2013-08-02 Thread Torstein Hegge
A commit has parent commits or parents, not commits.

Signed-off-by: Torstein Hegge he...@resisty.net
---
 Documentation/rev-list-options.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 27f8de3..5bdfb42 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -119,7 +119,7 @@ if it is part of the log message.
 --no-min-parents::
 --no-max-parents::
 
-   Show only commits which have at least (or at most) that many
+   Show only commits which have at least (or at most) that many parent
commits. In particular, `--max-parents=1` is the same as `--no-merges`,
`--min-parents=2` is the same as `--merges`.  `--max-parents=0`
gives all root commits and `--min-parents=3` all octopus merges.
-- 
1.8.4.rc1.372.g12d6635

--
To unsubscribe from this list: send the line 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: How to hierarchically merge from the root to the leaf of a branch tree? (Patch stack management)

2013-08-02 Thread Jens Müller
Am 02.08.2013 06:33, schrieb Jens Müller:
 Am 01.08.2013 09:28, schrieb Jakub Narebski:
  There is also TopGit, which is feature-branch management tools (which
  seems like what you want, from what you written below).
 Indeed, thank you very much for pointing me to it. I have not read the
 whole documentation, but it sounds promising enough that I will try it
 out with some real patches I have flying around and need to combine and
 do further development on.

Seems nice until now, but lacks some essential functionality ... For
example, you can add a dependency to a parent branch, but not remove 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: [regression] Re: git-cat-file --batch reversion; cannot query filenames with spaces

2013-08-02 Thread Jonathan Nieder
Junio C Hamano wrote:

 Here is what is on top of the revert that has been pushed out on
 'pu'.

For what it's worth,
Reviewed-by: Jonathan Nieder jrnie...@gmail.com

[...]
 To remain backwards compatible, we cannot split on whitespace by
 default, hence we will ship 1.8.4 with the commit reverted.
[...]
 It might be more robust to have something like -z to separate the
 input elements. But this patch is still a reasonable step before
 having that.  It makes the easy cases easy; people who do not care
 about %(rest) do not have to consider it, and the %(rest) code
 handles the spaces and newlines of rev-list --objects correctly.

Another idea for the future might be to start rejecting refnames
starting with a double-quote '', which would make it safe to treat a
leading quote-mark as the start of a C-style quoted string.  But
currently that would technically be a breaking change, making -z
more useful in the meantime.

I think several commands already don't deal well with filenames with
newlines.  I hope POSIX forbids them (with some suitable migration
plan) soonish and even wouldn't mind if git were taught to refuse to
track them.

Thanks,
Jonathan
--
To unsubscribe from this list: send the line 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-p4: use p4 fstat to interpret View setting

2013-08-02 Thread Pete Wyckoff
ksaitoh...@gmail.com wrote on Fri, 02 Aug 2013 17:02 +0900:
 I trying clone Perforce project and I found git-p4. It's a great tool!
 
 And I don't know how to exclude special extension file in a directory?
 (Practically, I want to exclude Excel files at git p4 clone/sync.)
 
 In Perforce, View setting of p4 client can describe
   -//depot/project/files/*.xls //client/project/files/*.xls
 to exclude Excel files.
 But git p4 --use-client-spec cannot support '*'.
 
 In git-p4.py, map_in_client method analyzes View setting and return
 client file path.
 So I modify the method to just use p4 command, p4 fstat -T clientFile.
 
 I'd like to know your opinions about that and what you think about the
 suggestion.

This is either incredibly brilliant or fundamentally broken.  I'm
hoping for the former.  :)

Your theory is:  there is a client spec, and p4 knows how to
interpret these things, so instead of figuring out and
implementing the algorithms for %% and * and ... in git-p4, just
ask p4 directly.

Let me play with this for a bit.  I wonder about the performance
aspects of doing a p4 fstat for every file.  Would it be
possible to do one or a few batch queries higher up somewhere?

A few nit-picky questions below, just on the test bits, while I
play with your code.

 diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
 index 2098b9b..fbda1ad 100644
 --- a/t/lib-git-p4.sh
 +++ b/t/lib-git-p4.sh
 @@ -48,6 +48,9 @@ P4DPORT=$((10669 + ($testid - $git_p4_test_start)))
  P4PORT=localhost:$P4DPORT
  P4CLIENT=client
  P4EDITOR=:
 +P4USER=
 +P4PASS=
 +P4CHARSET=

Why do you need these?

 diff --git a/t/t9801-git-p4-branch.sh b/t/t9801-git-p4-branch.sh
 index 2bf142d..b97bdda 100755
 --- a/t/t9801-git-p4-branch.sh
 +++ b/t/t9801-git-p4-branch.sh
 @@ -480,6 +480,7 @@ test_expect_success 'use-client-spec detect-branches
 skips branches setup' '
  test_expect_success 'use-client-spec detect-branches skips branches' '
   client_view //depot/usecs/... //client/... \
  -//depot/usecs/b3/... //client/b3/... 
 +( p4 sync ) 
   test_when_finished cleanup_git 

How does the p4 sync help here?

 +test_expect_success 'view wildcard *' '
 + client_view //depot/... //client/... \
 + -//depot/dir1/*.junk //client/dir1/*.junk \
 + -//depot/dir2/*.junk //client/dir2/*.junk 
 + (p4 sync ) 
 + files=dir1/file11 dir1/file12 dir2/file21 dir2/file22 
 + client_verify $files 
 + git p4 clone --use-client-spec --dest=$git //depot 
 + git_verify $files
 +'

It works!  Cool.

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


Re: Making a patch: git format-patch does not produce the documented format

2013-08-02 Thread Dale R. Worley
 From: John Keeping j...@keeping.me.uk
 
 git-format-patch(1) says:
 
 By default, the subject of a single patch is [PATCH]  followed
 by the concatenation of lines from the commit message up to the
 first blank line (see the DISCUSSION section of git-commit(1)).
 
 I think that accurately describes what you're seeing.  The referenced
 DISCUSSION section describes how to write a commit message that is
 formatted in a suitable way, with a short first subject line and then a
 blank line before the body of the message.

Thanks for the confirmation.  I've figured out what is going wrong:
Documentation/SubmittingPatches says:

The first line of the commit message should be a short description (50
characters is the soft limit, see DISCUSSION in git-commit(1)), and
should skip the full stop.

What it *doesn't* say is that the second line of the commit message
should be empty -- precisely so that git format-patch turns the first
line into the Subject: but does not merge the remainder of the commit
message (the body) into the Subject: line.  Now that I know to look
for this, I can see that the commit messages in the Git repository
show this pattern.

I'm preparing some clarifications of SubmittingPatches to explain
things that a new person (e.g., me) would not know.  To fix this
issue, I am inserting:

This first line should be a separate paragraph, that is, it should be
followed by an empty line, which is then followed by the body of the
commit message.

Dale
--
To unsubscribe from this list: send the line 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] log doc: the argument to --encoding is not optional

2013-08-02 Thread Jonathan Nieder
 $ git log --encoding
 fatal: Option '--encoding' requires a value
 $ git rev-list --encoding
 fatal: Option '--encoding' requires a value

The argument to --encoding has always been mandatory.  Unfortunately
manpages like git-rev-list(1), git-log(1), and git-show(1) have
described the option's syntax as --encoding[=encoding] since it
was first documented.  Clarify by removing the extra brackets.

Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
 Documentation/git-rev-list.txt   | 2 +-
 Documentation/pretty-options.txt | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index 65ac27e0..045b37b8 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -40,7 +40,7 @@ SYNOPSIS
 [ \--right-only ]
 [ \--cherry-mark ]
 [ \--cherry-pick ]
-[ \--encoding[=encoding] ]
+[ \--encoding=encoding ]
 [ \--(author|committer|grep)=pattern ]
 [ \--regexp-ignore-case | -i ]
 [ \--extended-regexp | -E ]
diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
index 5e499421..eea0e306 100644
--- a/Documentation/pretty-options.txt
+++ b/Documentation/pretty-options.txt
@@ -28,7 +28,7 @@ people using 80-column terminals.
This is a shorthand for --pretty=oneline --abbrev-commit
used together.
 
---encoding[=encoding]::
+--encoding=encoding::
The commit objects record the encoding used for the log message
in their encoding header; this option can be used to tell the
command to re-code the commit log message in the encoding
-- 
1.8.4.rc1

--
To unsubscribe from this list: send the line 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/RFC] log doc: explain --encoding=none and default output encoding

2013-08-02 Thread Jonathan Nieder
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
I'm not thrilled with the wording.  This can probably be explained
more simply.  Ideas?

 Documentation/pretty-options.txt | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/Documentation/pretty-options.txt b/Documentation/pretty-options.txt
index 5e499421..e31fd494 100644
--- a/Documentation/pretty-options.txt
+++ b/Documentation/pretty-options.txt
@@ -32,8 +32,14 @@ people using 80-column terminals.
The commit objects record the encoding used for the log message
in their encoding header; this option can be used to tell the
command to re-code the commit log message in the encoding
-   preferred by the user.  For non plumbing commands this
-   defaults to UTF-8.
+   preferred by the user.  --encoding=none means to use the
+   raw log message without paying attention to its encoding header.
++
+For non plumbing commands, the output encoding defaults to the commit
+encoding (as set using the `i18n.commitEncoding` variable, or UTF-8
+by default).  This default can be overridden using the
+`i18n.logOutputEncoding` configuration item. See linkgit:git-config[1]
+for details.
 
 --notes[=ref]::
Show the notes (see linkgit:git-notes[1]) that annotate the
-- 
1.8.4.rc1

--
To unsubscribe from this list: send the line 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 9/6] push: teach --force-with-lease to smart-http transport

2013-08-02 Thread Junio C Hamano
We have been passing enough information to enable the
compare-and-swap logic down to the transport layer, but the
transport helper was not passing it to smart-http transport.

Signed-off-by: Junio C Hamano gits...@pobox.com
---

 * I didn't bother with the dumb commit walker push for obvious
   reasons, but if somebody is inclined to add one, it shouldn't be
   too hard to add.

 remote-curl.c| 16 +++-
 t/lib-httpd.sh   |  3 ++-
 t/t5541-http-push.sh |  2 +-
 transport-helper.c   | 24 ++--
 4 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index 60eda63..53c8a3d 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -6,6 +6,7 @@
 #include exec_cmd.h
 #include run-command.h
 #include pkt-line.h
+#include string-list.h
 #include sideband.h
 
 static struct remote *remote;
@@ -20,6 +21,7 @@ struct options {
thin : 1;
 };
 static struct options options;
+static struct string_list cas_options = STRING_LIST_INIT_DUP;
 
 static int set_option(const char *name, const char *value)
 {
@@ -66,6 +68,13 @@ static int set_option(const char *name, const char *value)
return -1;
return 0;
}
+   else if (!strcmp(name, cas)) {
+   struct strbuf val = STRBUF_INIT;
+   strbuf_addf(val, -- CAS_OPT_NAME =%s, value);
+   string_list_append(cas_options, val.buf);
+   strbuf_release(val);
+   return 0;
+   }
else {
return 1 /* unsupported */;
}
@@ -789,8 +798,9 @@ static int push_git(struct discovery *heads, int nr_spec, 
char **specs)
struct rpc_state rpc;
const char **argv;
int argc = 0, i, err;
+   struct string_list_item *cas_option;
 
-   argv = xmalloc((10 + nr_spec) * sizeof(char*));
+   argv = xmalloc((10 + nr_spec + cas_options.nr) * sizeof(char *));
argv[argc++] = send-pack;
argv[argc++] = --stateless-rpc;
argv[argc++] = --helper-status;
@@ -803,6 +813,10 @@ static int push_git(struct discovery *heads, int nr_spec, 
char **specs)
else if (options.verbosity  1)
argv[argc++] = --verbose;
argv[argc++] = options.progress ? --progress : --no-progress;
+
+   for_each_string_list_item(cas_option, cas_options)
+   argv[argc++] = cas_option-string;
+
argv[argc++] = url;
for (i = 0; i  nr_spec; i++)
argv[argc++] = specs[i];
diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index e2eca1f..dab405d 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -141,10 +141,11 @@ stop_httpd() {
-f $TEST_PATH/apache.conf $HTTPD_PARA -k stop
 }
 
-test_http_push_nonff() {
+test_http_push_nonff () {
REMOTE_REPO=$1
LOCAL_REPO=$2
BRANCH=$3
+   EXPECT_CAS_RESULT=${4-failure}
 
test_expect_success 'non-fast-forward push fails' '
cd $REMOTE_REPO 
diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh
index beb00be..470ac54 100755
--- a/t/t5541-http-push.sh
+++ b/t/t5541-http-push.sh
@@ -153,7 +153,7 @@ test_expect_success 'used receive-pack service' '
 '
 
 test_http_push_nonff $HTTPD_DOCUMENT_ROOT_PATH/test_repo.git \
-   $ROOT_PATH/test_repo_clone master
+   $ROOT_PATH/test_repo_clone master success
 
 test_expect_success 'push fails for non-fast-forward refs unmatched by remote 
helper' '
# create a dissimilarly-named remote ref so that git is unable to match 
the
diff --git a/transport-helper.c b/transport-helper.c
index 95d22f8..e3a60d7 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -742,13 +742,15 @@ static void push_update_refs_status(struct helper_data 
*data,
 }
 
 static int push_refs_with_push(struct transport *transport,
-   struct ref *remote_refs, int flags)
+  struct ref *remote_refs, int flags)
 {
int force_all = flags  TRANSPORT_PUSH_FORCE;
int mirror = flags  TRANSPORT_PUSH_MIRROR;
struct helper_data *data = transport-data;
struct strbuf buf = STRBUF_INIT;
struct ref *ref;
+   struct string_list cas_options = STRING_LIST_INIT_DUP;
+   struct string_list_item *cas_option;
 
get_helper(transport);
if (!data-push)
@@ -784,11 +786,29 @@ static int push_refs_with_push(struct transport 
*transport,
strbuf_addch(buf, ':');
strbuf_addstr(buf, ref-name);
strbuf_addch(buf, '\n');
+
+   /*
+* The --force-with-lease options without explicit
+* values to expect have already been expanded into
+* the ref-old_sha1_expect[] field; we can ignore
+* transport-smart_options-cas altogether and instead
+* can enumerate them from the refs.
+*/
+   if (ref-expect_old_sha1) {
+   struct 

[PATCH/RFC 0/3] post-receive-email: explicitly set Content-Type header

2013-08-02 Thread Jonathan Nieder
Hi all,

This is a revival of [1], which declares encoding in emails to make it
more likely that they can be read.  I like to think it avoids the
mistakes of previous attempts, but I'll let you judge. :)

Sorry for the long delay.  Thoughts of all kinds welcome, as always.

Thanks,
Jonathan

Gerrit Pape (1):
  hooks/post-receive-email: set declared encoding to utf-8

Jonathan Nieder (2):
  hooks/post-receive-email: use plumbing instead of 'git log' and 'git show'
  hooks/post-receive-email: force log messages in UTF-8

 contrib/hooks/post-receive-email | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

[1] http://thread.gmane.org/gmane.comp.version-control.git/181737/focus=183070
--
To unsubscribe from this list: send the line 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/3] hooks/post-receive-email: use plumbing instead of git log/show

2013-08-02 Thread Jonathan Nieder
This way the hook doesn't have to keep being tweaked as porcelain
learns new features like color and pagination.

While at it, replace the git rev-list | git shortlog idiom with
plain git shortlog for simplicity.

Except for depending less on the value of settings like '[log]
abbrevCommit', no change in output intended.

Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
 contrib/hooks/post-receive-email | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email
index 15311502..72084511 100755
--- a/contrib/hooks/post-receive-email
+++ b/contrib/hooks/post-receive-email
@@ -471,7 +471,7 @@ generate_delete_branch_email()
echowas  $oldrev
echo 
echo $LOGBEGIN
-   git show -s --pretty=oneline $oldrev
+   git diff-tree -s --always --pretty=oneline $oldrev
echo $LOGEND
 }
 
@@ -547,11 +547,11 @@ generate_atag_email()
# performed on them
if [ -n $prevtag ]; then
# Show changes since the previous release
-   git rev-list --pretty=short $prevtag..$newrev | git 
shortlog
+   git shortlog $prevtag..$newrev
else
# No previous tag, show all the changes since time
# began
-   git rev-list --pretty=short $newrev | git shortlog
+   git shortlog $newrev
fi
;;
*)
@@ -571,7 +571,7 @@ generate_delete_atag_email()
echowas  $oldrev
echo 
echo $LOGBEGIN
-   git show -s --pretty=oneline $oldrev
+   git diff-tree -s --always --pretty=oneline $oldrev
echo $LOGEND
 }
 
@@ -617,7 +617,7 @@ generate_general_email()
echo 
if [ $newrev_type = commit ]; then
echo $LOGBEGIN
-   git show --no-color --root -s --pretty=medium $newrev
+   git diff-tree -s --always --pretty=medium $newrev
echo $LOGEND
else
# What can we do here?  The tag marks an object that is not
@@ -636,7 +636,7 @@ generate_delete_general_email()
echowas  $oldrev
echo 
echo $LOGBEGIN
-   git show -s --pretty=oneline $oldrev
+   git diff-tree -s --always --pretty=oneline $oldrev
echo $LOGEND
 }
 
-- 
1.8.4.rc1

--
To unsubscribe from this list: send the line 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 3/3] hooks/post-receive-email: set declared encoding to utf-8

2013-08-02 Thread Jonathan Nieder
From: Gerrit Pape p...@smarden.org
Date: Thu, 11 Dec 2008 20:27:21 +

Some email clients (e.g., claws-mail) display the message body
incorrectly when the charset is not defined explicitly in a
Content-Type header.  git log generates logs in UTF-8 encoding by
default, so add a Content-Type header declaring that encoding to
the emails the post-receive-email example hook sends.

[jn: also setting the Content-Transfer-Encoding so MTAs know what
 kind of mangling might be needed when sending to a non 8-bit clean
 SMTP host]

Requested-by: Alexander Gerasiov g...@debian.org
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
That's the end of the series.  Thanks for reading.

 contrib/hooks/post-receive-email | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email
index ba93a0d8..8ee410f8 100755
--- a/contrib/hooks/post-receive-email
+++ b/contrib/hooks/post-receive-email
@@ -242,6 +242,9 @@ generate_email_header()
cat -EOF
To: $recipients
Subject: ${emailprefix}$projectdesc $refname_type $short_refname 
${change_type}d. $describe
+   MIME-Version: 1.0
+   Content-Type: text/plain; charset=utf-8
+   Content-Transfer-Encoding: 8bit
X-Git-Refname: $refname
X-Git-Reftype: $refname_type
X-Git-Oldrev: $oldrev
-- 
1.8.4.rc1

--
To unsubscribe from this list: send the line 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/3] hooks/post-receive-email: force log messages in UTF-8

2013-08-02 Thread Jonathan Nieder
Git commands write commit messages in UTF-8 by default, but that
default can be overridden by the [i18n] commitEncoding and
logOutputEncoding settings.  With such a setting, the emails written
by the post-receive-email hook use a mixture of encodings:

 1. Log messages use the configured log output encoding, which is
meant to be whatever encoding works best with local terminals
(and does not have much to do with what encoding should be used
for email)

 2. Filenames are left as is: on Linux, usually UTF-8, and in the Mingw
port (which uses Unicode filesystem APIs), always UTF-8

 3. The This is an automated email preface uses a project description
from .git/description, which is typically in UTF-8 to support
gitweb.

So (1) is configurable, and (2) and (3) are unconfigurable and
typically UTF-8.  Override the log output encoding to always use UTF-8
when writing the email to get the best chance of a comprehensible
single-encoding email.

Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
 contrib/hooks/post-receive-email | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/contrib/hooks/post-receive-email b/contrib/hooks/post-receive-email
index 72084511..ba93a0d8 100755
--- a/contrib/hooks/post-receive-email
+++ b/contrib/hooks/post-receive-email
@@ -471,7 +471,7 @@ generate_delete_branch_email()
echowas  $oldrev
echo 
echo $LOGBEGIN
-   git diff-tree -s --always --pretty=oneline $oldrev
+   git diff-tree -s --always --encoding=UTF-8 --pretty=oneline $oldrev
echo $LOGEND
 }
 
@@ -571,7 +571,7 @@ generate_delete_atag_email()
echowas  $oldrev
echo 
echo $LOGBEGIN
-   git diff-tree -s --always --pretty=oneline $oldrev
+   git diff-tree -s --always --encoding=UTF-8 --pretty=oneline $oldrev
echo $LOGEND
 }
 
@@ -617,7 +617,7 @@ generate_general_email()
echo 
if [ $newrev_type = commit ]; then
echo $LOGBEGIN
-   git diff-tree -s --always --pretty=medium $newrev
+   git diff-tree -s --always --encoding=UTF-8 --pretty=medium 
$newrev
echo $LOGEND
else
# What can we do here?  The tag marks an object that is not
@@ -636,7 +636,7 @@ generate_delete_general_email()
echowas  $oldrev
echo 
echo $LOGBEGIN
-   git diff-tree -s --always --pretty=oneline $oldrev
+   git diff-tree -s --always --encoding=UTF-8 --pretty=oneline $oldrev
echo $LOGEND
 }
 
-- 
1.8.4.rc1

--
To unsubscribe from this list: send the line 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: Making a patch: git format-patch does not produce the documented format

2013-08-02 Thread Junio C Hamano
wor...@alum.mit.edu (Dale R. Worley) writes:

 I'm preparing some clarifications of SubmittingPatches to explain
 things that a new person (e.g., me) would not know.

I am not sure if SubmittingPatches is a good place, though.
The document is a guidance for people who contribute to _this_
project.

But the specialness of the first paragraph applies to any project
that uses Git, so people other than those who contribute to this
project should be aware of it.

Originally we literally used first line, but that made many things
like shortlog output and patch Subject: useless when people write a
block of text starting from the first line without a title.  Also
after resurrecting such a text from e-mail, am couldn't tell if
the first line on the Subject: is meant to be the first line of
the same first paragraph (which is not what we encourage), or it is
properly a single line title, and need a blank line before the first
line of the body.  So quite a while ago, we changed the rule to take
the first paragraph and use that in these places where we want to
give a title of a patch.


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


[TIG][PATCH 0/3] Refactoring of the log view

2013-08-02 Thread Kumar Appaiah
Hi.

These set of patches refactor the log view to provide a behaviour that
is quite similar to, say, e-mail with Mutt. The key improvements are:

- The current commit is inferred based on the context. For example, if
  you focus on the commit message of a particular commit, the correct
  commit is inferred automagically.

- Scrolling the log view when the diff is open shows the correct
  commit on the screen, rather than have to scroll up and cross the
  commit line to display the screen.

I have decided to revert 888611dd5d407775245d574a3dc5c01b5963a5ba,
since the behaviour with the updated scrolling pattern is much more
consistent.

As always, I will gladly alter the patch based on comments on coding
style and all other aspects.

Thanks!

Kumar

Kumar Appaiah (3):
  Add log_select function to find commit from context in log view
  Display correct diff the context in split log view
  Revert Scroll diff with arrow keys in log view

 NEWS  |  1 +
 tig.c | 67 ++-
 2 files changed, 63 insertions(+), 5 deletions(-)

-- 
1.8.3.2

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


[[TIG][PATCH] 3/3] Revert Scroll diff with arrow keys in log view

2013-08-02 Thread Kumar Appaiah
This reverts commit 888611dd5d407775245d574a3dc5c01b5963a5ba. This is
because, in the re-engineered log view, scrolling the log with the
arrows now updates the diff in the diff view when the screen is
split. This resembles the earlier behaviour, and is also what users of
software like Mutt (which uses the pager view concept) would expect.

Signed-Off-By: Kumar Appaiah a.ku...@alumni.iitm.ac.in

Conflicts:
tig.c
---
 tig.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tig.c b/tig.c
index 53947b7..65c91a0 100644
--- a/tig.c
+++ b/tig.c
@@ -1901,7 +1901,6 @@ enum view_flag {
VIEW_STDIN  = 1  8,
VIEW_SEND_CHILD_ENTER   = 1  9,
VIEW_FILE_FILTER= 1  10,
-   VIEW_NO_PARENT_NAV  = 1  11,
 };
 
 #define view_has_flags(view, flag) ((view)-ops-flags  (flag))
@@ -3774,7 +3773,7 @@ view_driver(struct view *view, enum request request)
 
case REQ_NEXT:
case REQ_PREVIOUS:
-   if (view-parent  !view_has_flags(view-parent, 
VIEW_NO_PARENT_NAV)) {
+   if (view-parent) {
int line;
 
view = view-parent;
@@ -4498,7 +4497,7 @@ log_request(struct view *view, enum request request, 
struct line *line)
 static struct view_ops log_ops = {
line,
{ log },
-   VIEW_ADD_PAGER_REFS | VIEW_OPEN_DIFF | VIEW_SEND_CHILD_ENTER | 
VIEW_NO_PARENT_NAV,
+   VIEW_ADD_PAGER_REFS | VIEW_OPEN_DIFF | VIEW_SEND_CHILD_ENTER,
sizeof(struct log_state),
log_open,
pager_read,
-- 
1.8.3.2

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


[[TIG][PATCH] 2/3] Display correct diff the context in split log view

2013-08-02 Thread Kumar Appaiah
In the log view, when scrolling across a commit, the diff view should
automatically switch to the commit whose context the cursor is on in
the log view. This commit changes things to catch the REQ_ENTER in the
log view and handle recalculation of the commit and diff display from
log_request, rather than delegating it to pager_request. In addition,
it also gets rid of unexpected upward scrolling of the log view.

Fixes GH #155

Signed-Off-By: Kumar Appaiah a.ku...@alumni.iitm.ac.in
---
 NEWS  |  1 +
 tig.c | 12 
 2 files changed, 13 insertions(+)

diff --git a/NEWS b/NEWS
index 0394407..f59e517 100644
--- a/NEWS
+++ b/NEWS
@@ -46,6 +46,7 @@ Bug fixes:
  - Fix rendering glitch for branch names.
  - Do not apply diff styling to untracked files in the stage view. (GH #153)
  - Fix tree indentation for entries containing combining characters. (GH #170)
+ - Introduce a more natural context-sensitive log display. (GH #155)
 
 tig-1.1
 ---
diff --git a/tig.c b/tig.c
index dd4b0f4..53947b7 100644
--- a/tig.c
+++ b/tig.c
@@ -4478,6 +4478,18 @@ log_request(struct view *view, enum request request, 
struct line *line)
state-update_commit_ref = TRUE;
return pager_request(view, request, line);
 
+   case REQ_ENTER:
+   /* Recalculate the correct commit for the context. */
+   state-update_commit_ref = TRUE;
+
+   open_view(view, REQ_VIEW_DIFF, OPEN_SPLIT);
+   update_view_title(view);
+
+   /* We don't want to delegate this to pager_request,
+  since we don't want the extra scrolling of the log
+  view. */
+   return REQ_NONE;
+
default:
return pager_request(view, request, line);
}
-- 
1.8.3.2

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


[[TIG][PATCH] 1/3] Add log_select function to find commit from context in log view

2013-08-02 Thread Kumar Appaiah
This commit introduces and uses the log_select function to find the
correct commit in the unsplit log view. In the log view, if one
scrolls down across a commit line, the current commit (as displayed in
the status bar) gets updated, but not so when scrolling upward across
a commit. The log_select function handles this scenario to to the
``right thing''. In addition, it introduces the log_state structure as
the private entry of the log view to hold a flag that decides whether
to re-evaluate the current commit based on scrolling.

Signed-off-by: Kumar Appaiah a.ku...@alumni.iitm.ac.in
---
 tig.c | 50 --
 1 file changed, 48 insertions(+), 2 deletions(-)

diff --git a/tig.c b/tig.c
index 72f132a..dd4b0f4 100644
--- a/tig.c
+++ b/tig.c
@@ -4384,6 +4384,33 @@ pager_select(struct view *view, struct line *line)
}
 }
 
+struct log_state {
+   bool update_commit_ref;
+};
+
+static void
+log_select(struct view *view, struct line *line)
+{
+   struct log_state *state = (struct log_state *) view-private;
+
+   if (state-update_commit_ref  line-lineno  1) {
+   /* We need to recalculate the previous commit,
+  since the user has likely scrolled up. */
+   const struct line *commit_line = find_prev_line_by_type(view, 
line, LINE_COMMIT);
+
+   if (commit_line)
+   string_copy_rev(view-ref, (char *) (commit_line-data 
+ STRING_SIZE(commit )));
+   }
+   if (line-type == LINE_COMMIT) {
+   char *text = (char *)line-data + STRING_SIZE(commit );
+
+   if (!view_has_flags(view, VIEW_NO_REF))
+   string_copy_rev(view-ref, text);
+   }
+   string_copy_rev(ref_commit, view-ref);
+   state-update_commit_ref = FALSE;
+}
+
 static bool
 pager_open(struct view *view, enum open_flags flags)
 {
@@ -4427,11 +4454,30 @@ log_open(struct view *view, enum open_flags flags)
 static enum request
 log_request(struct view *view, enum request request, struct line *line)
 {
+   struct log_state *state = (struct log_state *) view-private;
+
switch (request) {
case REQ_REFRESH:
load_refs();
refresh_view(view);
return REQ_NONE;
+
+   case REQ_MOVE_UP:
+   case REQ_PREVIOUS:
+   if (line-type == LINE_COMMIT  line-lineno  1) {
+   /* We are at a commit, and heading upward. We
+  force log_select to find the previous
+  commit above, from the context. */
+   state-update_commit_ref = TRUE;
+   }
+   return pager_request(view, request, line);
+
+   case REQ_MOVE_PAGE_UP:
+   case REQ_MOVE_PAGE_DOWN:
+   /* We need to figure out the right commit again. */
+   state-update_commit_ref = TRUE;
+   return pager_request(view, request, line);
+
default:
return pager_request(view, request, line);
}
@@ -4441,13 +4487,13 @@ static struct view_ops log_ops = {
line,
{ log },
VIEW_ADD_PAGER_REFS | VIEW_OPEN_DIFF | VIEW_SEND_CHILD_ENTER | 
VIEW_NO_PARENT_NAV,
-   0,
+   sizeof(struct log_state),
log_open,
pager_read,
pager_draw,
log_request,
pager_grep,
-   pager_select,
+   log_select,
 };
 
 struct diff_state {
-- 
1.8.3.2

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


[PATCH] git_mkstemps: improve test suite test

2013-08-02 Thread Dale R. Worley
Commit 52749 fixes a bug regarding testing the return of an open()
call for success/failure.  Improve the testsuite test for that fix by
removing the helper program 'test-close-fd-0' and replacing it with
the shell redirection '-'.  (The redirection is Posix, so it should
be portable.)

Signed-off-by: Dale Worley wor...@ariadne.com
---

 From: Junio C Hamano gits...@pobox.com
 Date: Fri, 19 Jul 2013 07:29:47 -0700
 
 The change itself looks good; care to write it up as a proper patch
 with a proposed log message?

My apologies for the delay; I've had to do some yak-shaving to learn
how to construct patches properly.  (I've written some clarifications
for Document/SubmittingPatches, which I will submit separately.)

Someone has gone ahead and made the code change, so all that remains
is to update the testsuite test by replacing the helper program
'test-close-fd-0' with the Posix shell redirection '-'.

Dale


 Makefile  |1 -
 test-close-fd-0.c |   14 --
 2 files changed, 0 insertions(+), 15 deletions(-)
 delete mode 100644 test-close-fd-0.c

diff --git a/Makefile b/Makefile
index 8ad40d4..3588ca1 100644
--- a/Makefile
+++ b/Makefile
@@ -557,7 +557,6 @@ X =
 PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
 
 TEST_PROGRAMS_NEED_X += test-chmtime
-TEST_PROGRAMS_NEED_X += test-close-fd-0
 TEST_PROGRAMS_NEED_X += test-ctype
 TEST_PROGRAMS_NEED_X += test-date
 TEST_PROGRAMS_NEED_X += test-delta
diff --git a/test-close-fd-0.c b/test-close-fd-0.c
deleted file mode 100644
index 3745c34..000
--- a/test-close-fd-0.c
+++ /dev/null
@@ -1,14 +0,0 @@
-#include unistd.h
-
-/* Close file descriptor 0 (which is standard-input), then execute the
- * remainder of the command line as a command. */
-
-int main(int argc, char **argv)
-{
-   /* Close fd 0. */
-   close(0);
-   /* Execute the requested command. */
-   execvp(argv[1], argv[1]);
-   /* If execve() failed, return an error. */
-   return 1;
-}
-- 
1.7.7.6

--
To unsubscribe from this list: send the line 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_mkstemps: improve test suite test

2013-08-02 Thread Jonathan Nieder
Hi,

Dale R. Worley wrote:

 Commit 52749 fixes a bug regarding testing the return of an open()

 $ git show 52749
 fatal: ambiguous argument '52749': unknown revision or path not in the working 
tree.

Could you mention its subject line or date so it's easier to find?

 call for success/failure.  Improve the testsuite test for that fix by
 removing the helper program 'test-close-fd-0' and replacing it with
 the shell redirection '-'.  (The redirection is Posix, so it should
 be portable.)
 
 Signed-off-by: Dale Worley wor...@ariadne.com
[...]
 Someone has gone ahead and made the code change, so all that remains
 is to update the testsuite test by replacing the helper program
 'test-close-fd-0' with the Posix shell redirection '-'.

The above paragraph should be part of the commit message, since
otherwise the patch is hard to understand.

The patch text looks good.

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


[BUG?] gc and impatience

2013-08-02 Thread Ramkumar Ramachandra
Hi,

I was pulling in some changes in the morning to find:

 Auto packing the repository for optimum performance. You may also
 run git gc manually. See git help gc for more information.

Being my usual impatient self, I opened another prompt and started
merging changes. After the checkout, it started running another gc
(why!?), which I attempted to kill using ^C.

  Counting objects: 449291   x$

It didn't just fail to stop, but it kept writing output making my
prompt completely unusable. I finally just killed the pane. Now, it's
struggling to update-index and update my cache (read: more waiting).

Why is gc not designed for impatient people, and what needs to be done
to change this?

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


change remote to track new branch

2013-08-02 Thread Daniel Convissor
Hi:

Long ago I added a remote to my repo.  It is set to track what was then
WordPress' main release branch (3.4-branch) and created a local branch
to use it.  Well, time marches on.  I want to update my remote and
branch to track the new main release branch (3.6-branch).

Here's how I set things up at the time:

git remote add -t 3.4-branch -f wp https://github.com/WordPress/WordPress
git checkout -b wp wp/3.4-branch

I've tried various fetch, remote and branch commands to effectuate this
change, to no avail.  I get messages like Not a valid ref.

I know a major part of this is that my repo only knows of one remote
branch (of the many that exist).  What I don't know are the commands
needed to fetch the right remote branch, set my remote and local branch
to track it.  Your help will be appreciated, please.

Here's some information on the current state of things:



git branch -av
  dev a18677e [ahead 153] backup of 3.4 db
  master  0d6f9b5 Merge branch 'dev'

* wp  42abc67 [ahead 5] Merge branch
'3.4-branch' of https://github.com/WordPress/WordPress into wp

  remotes/git_push_deployer/wordpress c8a5d69 Make my branch names generic.
  remotes/prod/HEAD   - prod/master
  remotes/prod/master 0d6f9b5 Merge branch 'dev'
  remotes/wp/3.4-branch   b535358 POT, generated from r24100
  remotes/wpconfig/master 3e45a81 LSS 0.27.0.



git remote show wp
* remote wp
  Fetch URL: https://github.com/WordPress/WordPress
  Push  URL: https://github.com/WordPress/WordPress
  HEAD branch: master
  Remote branch:
3.4-branch tracked
  Local branch configured for 'git pull':
wp merges with remote 3.4-branch
  Local ref configured for 'git push':
master pushes to master (local out of date)


Thanks,

--Dan

-- 
 T H E   A N A L Y S I S   A N D   S O L U T I O N S   C O M P A N Y
data intensive web and database programming
http://www.AnalysisAndSolutions.com/
4015 7th Ave #4, Brooklyn NY 11232  v: 718-854-0335
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [BUG?] gc and impatience

2013-08-02 Thread Duy Nguyen
On Sat, Aug 3, 2013 at 8:48 AM, Ramkumar Ramachandra artag...@gmail.com wrote:
  Auto packing the repository for optimum performance. You may also
  run git gc manually. See git help gc for more information.

 Being my usual impatient self, I opened another prompt and started
 merging changes. After the checkout, it started running another gc
 (why!?),

Good point. I think that is because gc does not check if gc is already
running. Adding such a check should not be too hard. I think gc could
save its pid in $GIT_DIR/auto-gc.pid. The next auto-gc checks this, if
the pid is valid, skip auto-gc.

 Why is gc not designed for impatient people, and what needs to be done
 to change this?

Some improvements could be made in gc, for example warn users about
upcoming gc so they can run it in background (of course the above bug
should be fixed)

http://thread.gmane.org/gmane.comp.version-control.git/197716/focus=197877

or speed up repack by implementing pack-objects --merge-pack:

http://thread.gmane.org/gmane.comp.version-control.git/57672/focus=57943

Or you could just make a cron job to gc all repos every week and the
problem goes away ;-)
-- 
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


[PATCH] gc: reject if another gc is running, unless --force is given

2013-08-02 Thread Nguyễn Thái Ngọc Duy
This may happen when `git gc --auto` is run automatically, then the
user, to avoid wait time, switches to a new terminal, keeps working
and `git gc --auto` is started again because the first gc instance has
not clean up the repository.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 I don't know if the kill(pid, 0) trick works on Windows..

 Documentation/git-gc.txt |  6 +-
 builtin/gc.c | 18 ++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt
index 2402ed6..e158a3b 100644
--- a/Documentation/git-gc.txt
+++ b/Documentation/git-gc.txt
@@ -9,7 +9,7 @@ git-gc - Cleanup unnecessary files and optimize the local 
repository
 SYNOPSIS
 
 [verse]
-'git gc' [--aggressive] [--auto] [--quiet] [--prune=date | --no-prune]
+'git gc' [--aggressive] [--auto] [--quiet] [--prune=date | --no-prune] 
[--force]
 
 DESCRIPTION
 ---
@@ -72,6 +72,10 @@ automatic consolidation of packs.
 --quiet::
Suppress all progress reports.
 
+--force::
+   Force `git gc` to run even if there may be another `git gc`
+   instance running on this repository.
+
 Configuration
 -
 
diff --git a/builtin/gc.c b/builtin/gc.c
index 6be6c8d..07c566c 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -169,9 +169,11 @@ static int need_to_gc(void)
 
 int cmd_gc(int argc, const char **argv, const char *prefix)
 {
+   FILE *fp;
int aggressive = 0;
int auto_gc = 0;
int quiet = 0;
+   int force = 0;
 
struct option builtin_gc_options[] = {
OPT__QUIET(quiet, N_(suppress progress reporting)),
@@ -180,6 +182,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
PARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire },
OPT_BOOLEAN(0, aggressive, aggressive, N_(be more thorough 
(increased runtime))),
OPT_BOOLEAN(0, auto, auto_gc, N_(enable auto-gc mode)),
+   OPT_BOOL(0, force, force, N_(force running gc even if there 
may be another gc running)),
OPT_END()
};
 
@@ -225,6 +228,21 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
} else
add_repack_all_option();
 
+   if (!force  (fp = fopen(git_path(gc.pid), r)) != NULL) {
+   uintmax_t pid;
+   if (fscanf(fp, %PRIuMAX, pid) == 1  !kill(pid, 0)) {
+   if (auto_gc)
+   return 0; /* be quiet on --auto */
+   die(_(gc is already running));
+   }
+   fclose(fp);
+   }
+
+   if ((fp = fopen(git_path(gc.pid), w)) != NULL) {
+   fprintf(fp, %PRIuMAX\n, (uintmax_t) getpid());
+   fclose(fp);
+   }
+
if (pack_refs  run_command_v_opt(pack_refs_cmd.argv, RUN_GIT_CMD))
return error(FAILED_RUN, pack_refs_cmd.argv[0]);
 
-- 
1.8.2.83.gc99314b

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


Re: [BUG?] gc and impatience

2013-08-02 Thread Junio C Hamano
On Fri, Aug 2, 2013 at 8:53 PM, Duy Nguyen pclo...@gmail.com wrote:
 Good point. I think that is because gc does not check if gc is already
 running. Adding such a check should not be too hard. I think gc could
 save its pid in $GIT_DIR/auto-gc.pid. The next auto-gc checks this, if
 the pid is valid, skip auto-gc.

Defining valid is a tricky business, though, as pid can and will
wrap around, and the directory could be shared on multiple machines. A
pid written by a process on one machine has no relation to any pid on
another machine.
--
To unsubscribe from this list: send the line 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] cherry-pick: allow - as abbreviation of '@{-1}'

2013-08-02 Thread Hiroshige Umino
As git cherry-pick - or git merge - is convenient to
switch back to or merge the previous branch,
git cherry-pick - is abbreviation of git cherry-pick @{-1}
to pick up a commit from the previous branch conveniently.

Signed-off-by: Hiroshige Umino hiroshig...@gmail.com
---
 builtin/revert.c|  2 ++
 t/t3512-cherry-pick-last.sh | 28 
 2 files changed, 30 insertions(+)
 create mode 100755 t/t3512-cherry-pick-last.sh

diff --git a/builtin/revert.c b/builtin/revert.c
index 1d2648b..cb403f8 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -234,6 +234,8 @@ int cmd_cherry_pick(int argc, const char **argv, const char 
*prefix)
memset(opts, 0, sizeof(opts));
opts.action = REPLAY_PICK;
git_config(git_default_config, NULL);
+   if (!strcmp(argv[1], -))
+   argv[1] = @{-1};
parse_args(argc, argv, opts);
res = sequencer_pick_revisions(opts);
if (res  0)
diff --git a/t/t3512-cherry-pick-last.sh b/t/t3512-cherry-pick-last.sh
new file mode 100755
index 000..8b05f81
--- /dev/null
+++ b/t/t3512-cherry-pick-last.sh
@@ -0,0 +1,28 @@
+#!/bin/sh
+
+test_description='Test cherry-pick -'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ echo hello world 
+ git add world 
+ git commit -m initial 
+ git branch other 
+ echo hello again world 
+ git add world 
+ git commit -m second
+'
+
+test_expect_success 'cherry-pick - does not work initially' '
+ test_must_fail git cherry-pick -
+'
+
+test_expect_success 'cherry-pick the commit in the previous branch' '
+ prev=$(git rev-parse HEAD) 
+ git checkout other 
+ git cherry-pick - 
+ test z$(git rev-parse HEAD) = z$prev
+'
+
+test_done
-- 
1.8.3.3

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