Re: [PATCH] gc: remove gc.pid file at end of execution

2013-09-27 Thread Duy Nguyen
On Sat, Sep 28, 2013 at 7:33 AM, Jonathan Nieder  wrote:
> Matthieu Moy wrote:
>
>> This file isn't really harmful, but isn't useful either, and can create
>> minor annoyance for the user:
>
> Would something like the following make sense, to ensure the gc.pid file is
> always removed on normal exit?
>
> Signed-off-by: Jonathan Nieder 
>
> diff --git c/builtin/gc.c i/builtin/gc.c
> index 6e0d81a..fbbc144 100644
> --- c/builtin/gc.c
> +++ i/builtin/gc.c
> @@ -14,6 +14,7 @@
>  #include "cache.h"
>  #include "parse-options.h"
>  #include "run-command.h"
> +#include "sigchain.h"
>  #include "argv-array.h"
>
>  #define FAILED_RUN "failed to run %s"
> @@ -167,6 +168,21 @@ static int need_to_gc(void)
> return 1;
>  }
>
> +static char *pidfile;
> +
> +static void remove_pidfile(void)
> +{
> +   if (pidfile)
> +   unlink(pidfile);
> +}
> +
> +static void remove_pidfile_on_signal(int signo)
> +{
> +   remove_pidfile();
> +   sigchain_pop(signo);
> +   raise(signo);
> +}
> +
>  /* return NULL on success, else hostname running the gc */
>  static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
>  {
> @@ -179,13 +195,19 @@ static const char *lock_repo_for_gc(int force, pid_t* 
> ret_pid)
> FILE *fp;
> int fd, should_exit;
>
> +   if (pidfile)
> +   /* already locked */
> +   return NULL;
> +
> if (gethostname(my_host, sizeof(my_host)))
> strcpy(my_host, "unknown");
>
> -   fd = hold_lock_file_for_update(&lock, git_path("gc.pid"),
> +   pidfile = git_pathdup("gc.pid");
> +
> +   fd = hold_lock_file_for_update(&lock, pidfile,
>LOCK_DIE_ON_ERROR);
> if (!force) {
> -   fp = fopen(git_path("gc.pid"), "r");
> +   fp = fopen(pidfile, "r");
> memset(locking_host, 0, sizeof(locking_host));
> should_exit =
> fp != NULL &&
> @@ -208,6 +230,7 @@ static const char *lock_repo_for_gc(int force, pid_t* 
> ret_pid)
> if (should_exit) {
> if (fd >= 0)
> rollback_lock_file(&lock);
> +   pidfile = NULL;
> *ret_pid = pid;
> return locking_host;
> }
> @@ -219,6 +242,9 @@ static const char *lock_repo_for_gc(int force, pid_t* 
> ret_pid)
> strbuf_release(&sb);

It may be a bit simpler to delay setting pidfile until we get here.
lock.filename still contains gc.pid until commit_lock_file is called.

> commit_lock_file(&lock);
>
> +   sigchain_push_common(remove_pidfile_on_signal);
> +   atexit(remove_pidfile);
> +
> return NULL;
>  }
-- 
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 v2 3/3] for-each-ref: introduce %(upstream:track[short])

2013-09-27 Thread Ramkumar Ramachandra
Introduce %(upstream:track) to display "[ahead M, behind N]" and
%(upstream:trackshort) to display "=", ">", "<", or "<>"
appropriately (inspired by contrib/completion/git-prompt.sh).

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

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

to display refs with terse tracking information.

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

Signed-off-by: Ramkumar Ramachandra 
---
 Documentation/git-for-each-ref.txt |  6 +-
 builtin/for-each-ref.c | 39 --
 2 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index bb9c4c1..3ef6aa8 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -93,7 +93,11 @@ objectname::
 upstream::
The name of a local ref which can be considered ``upstream''
from the displayed ref. Respects `:short` in the same way as
-   `refname` above.
+   `refname` above.  Additionally respects `:track` to show
+   "[ahead N, behind M]" and `:trackshort` to show the terse
+   version (like the prompt) ">", "<", "<>", or "=".  Has no
+   effect if the ref does not have tracking information
+   associated with it.
 
 HEAD::
Used to indicate the currently checked out branch.  Is '*' if
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index b841545..7d5c174 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -648,6 +648,7 @@ static void populate_value(struct refinfo *ref)
int deref = 0;
const char *refname;
const char *formatp;
+   struct branch *branch;
 
if (*name == '*') {
deref = 1;
@@ -659,7 +660,6 @@ static void populate_value(struct refinfo *ref)
else if (!prefixcmp(name, "symref"))
refname = ref->symref ? ref->symref : "";
else if (!prefixcmp(name, "upstream")) {
-   struct branch *branch;
/* only local branches may have an upstream */
if (prefixcmp(ref->refname, "refs/heads/"))
continue;
@@ -686,6 +686,7 @@ static void populate_value(struct refinfo *ref)
} else if (!strcmp(name, "HEAD")) {
const char *head;
unsigned char sha1[20];
+
head = resolve_ref_unsafe("HEAD", sha1, 1, NULL);
if (!strcmp(ref->refname, head))
v->s = "*";
@@ -698,11 +699,45 @@ static void populate_value(struct refinfo *ref)
formatp = strchr(name, ':');
/* look for "short" refname format */
if (formatp) {
+   int num_ours, num_theirs;
+
formatp++;
if (!strcmp(formatp, "short"))
refname = shorten_unambiguous_ref(refname,
  warn_ambiguous_refs);
-   else
+   else if (!strcmp(formatp, "track") &&
+   !prefixcmp(name, "upstream")) {
+   char buf[40];
+
+   stat_tracking_info(branch, &num_ours, 
&num_theirs);
+   if (!num_ours && !num_theirs)
+   v->s = "";
+   else if (!num_ours) {
+   sprintf(buf, "[behind %d]", num_theirs);
+   v->s = xstrdup(buf);
+   } else if (!num_theirs) {
+   sprintf(buf, "[ahead %d]", num_ours);
+   v->s = xstrdup(buf);
+   } else {
+   sprintf(buf, "[ahead %d, behind %d]",
+   num_ours, num_theirs);
+   v->s = xstrdup(buf);
+   }
+   continue;
+   } else if (!strcmp(formatp, "trackshort") &&
+   !prefixcmp(name, "upstream")) {
+
+   stat_tracking_info(branch, &num_ours, 
&num_theirs);
+   if (!num_ours && !num_theirs)
+   v->s = "=";
+   else if (!num_ours)
+   v->s = "<";
+   else if (!num_theirs)
+   v->s = ">";
+   else
+   v->s = "<>";
+  

[PATCH v2 2/3] for-each-ref: introduce %(HEAD) asterisk marker

2013-09-27 Thread Ramkumar Ramachandra
'git branch' shows which branch you are currently on with an '*', but
'git for-each-ref' misses this feature.  So, extend its format with
%(HEAD) for the same effect.

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

  %C(red)%(HEAD)%C(reset) %(refname:short)

to display a red asterisk next to the current ref.

Signed-off-by: Ramkumar Ramachandra 
---
 Documentation/git-for-each-ref.txt |  4 
 builtin/for-each-ref.c | 13 +++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 6fa4464..bb9c4c1 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -95,6 +95,10 @@ upstream::
from the displayed ref. Respects `:short` in the same way as
`refname` above.
 
+HEAD::
+   Used to indicate the currently checked out branch.  Is '*' if
+   HEAD points to the current ref, and ' ' otherwise.
+
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
 be used to specify the value in the header field.
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 6da2903..b841545 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -76,6 +76,7 @@ static struct {
{ "upstream" },
{ "symref" },
{ "flag" },
+   { "HEAD" },
 };
 
 /*
@@ -682,8 +683,16 @@ static void populate_value(struct refinfo *ref)
v->s = xstrdup(buf + 1);
}
continue;
-   }
-   else
+   } else if (!strcmp(name, "HEAD")) {
+   const char *head;
+   unsigned char sha1[20];
+   head = resolve_ref_unsafe("HEAD", sha1, 1, NULL);
+   if (!strcmp(ref->refname, head))
+   v->s = "*";
+   else
+   v->s = " ";
+   continue;
+   } else
continue;
 
formatp = strchr(name, ':');
-- 
1.8.4.477.g4cae6f5

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


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

2013-09-27 Thread Ramkumar Ramachandra
Enhance 'git for-each-ref' with color formatting options.  You can now
use the following format in for-each-ref:

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

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

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index f2e08d1..6fa4464 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -45,7 +45,9 @@ OPTIONS
It also interpolates `%%` to `%`, and `%xx` where `xx`
are hex digits interpolates to character with hex code
`xx`; for example `%00` interpolates to `\0` (NUL),
-   `%09` to `\t` (TAB) and `%0a` to `\n` (LF).
+   `%09` to `\t` (TAB) and `%0a` to `\n` (LF). Additionally,
+   colors can be specified using `%C(...)`, with names
+   described in color.branch.*.
 
 ...::
If one or more patterns are given, only refs are shown that
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 1d4083c..6da2903 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -9,6 +9,7 @@
 #include "quote.h"
 #include "parse-options.h"
 #include "remote.h"
+#include "color.h"
 
 /* Quoting styles */
 #define QUOTE_NONE 0
@@ -155,10 +156,13 @@ static const char *find_next(const char *cp)
while (*cp) {
if (*cp == '%') {
/*
+* %C( is the start of a color;
 * %( is the start of an atom;
 * %% is a quoted per-cent.
 */
-   if (cp[1] == '(')
+   if (cp[1] == 'C' && cp[2] == '(')
+   return cp;
+   else if (cp[1] == '(')
return cp;
else if (cp[1] == '%')
cp++; /* skip over two % */
@@ -180,8 +184,11 @@ static int verify_format(const char *format)
const char *ep = strchr(sp, ')');
if (!ep)
return error("malformed format string %s", sp);
-   /* sp points at "%(" and ep points at the closing ")" */
-   parse_atom(sp + 2, ep);
+   /* Ignore color specifications: %C(
+* sp points at "%(" and ep points at the closing ")"
+*/
+   if (prefixcmp(sp, "%C("))
+   parse_atom(sp + 2, ep);
cp = ep + 1;
}
return 0;
@@ -933,12 +940,20 @@ static void emit(const char *cp, const char *ep)
 static void show_ref(struct refinfo *info, const char *format, int quote_style)
 {
const char *cp, *sp, *ep;
+   char color[COLOR_MAXLEN] = "";
 
for (cp = format; *cp && (sp = find_next(cp)); cp = ep + 1) {
ep = strchr(sp, ')');
if (cp < sp)
emit(cp, sp);
-   print_value(info, parse_atom(sp + 2, ep), quote_style);
+
+   /* Do we have a color specification? */
+   if (!prefixcmp(sp, "%C("))
+   color_parse_mem(sp + 3, ep - sp - 3, "--format", color);
+   else {
+   printf("%s", color);
+   print_value(info, parse_atom(sp + 2, ep), quote_style);
+   }
}
if (*cp) {
sp = cp + strlen(cp);
-- 
1.8.4.477.g4cae6f5

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


[PATCH v2 0/3] Juggling between hot branches

2013-09-27 Thread Ramkumar Ramachandra
Hi,

This iteration incorporates the suggestions made by Phil Hord.

Thanks.

Ramkumar Ramachandra (3):
  for-each-ref: introduce %C(...) for color
  for-each-ref: introduce %(HEAD) asterisk marker
  for-each-ref: introduce %(upstream:track[short])

 Documentation/git-for-each-ref.txt | 14 ++-
 builtin/for-each-ref.c | 75 ++
 2 files changed, 79 insertions(+), 10 deletions(-)

-- 
1.8.4.477.g4cae6f5

--
To unsubscribe from this list: send the line "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] gc: remove gc.pid file at end of execution

2013-09-27 Thread Jonathan Nieder
Matthieu Moy wrote:

> This file isn't really harmful, but isn't useful either, and can create
> minor annoyance for the user:

Would something like the following make sense, to ensure the gc.pid file is
always removed on normal exit?

Signed-off-by: Jonathan Nieder 

diff --git c/builtin/gc.c i/builtin/gc.c
index 6e0d81a..fbbc144 100644
--- c/builtin/gc.c
+++ i/builtin/gc.c
@@ -14,6 +14,7 @@
 #include "cache.h"
 #include "parse-options.h"
 #include "run-command.h"
+#include "sigchain.h"
 #include "argv-array.h"
 
 #define FAILED_RUN "failed to run %s"
@@ -167,6 +168,21 @@ static int need_to_gc(void)
return 1;
 }
 
+static char *pidfile;
+
+static void remove_pidfile(void)
+{
+   if (pidfile)
+   unlink(pidfile);
+}
+
+static void remove_pidfile_on_signal(int signo)
+{
+   remove_pidfile();
+   sigchain_pop(signo);
+   raise(signo);
+}
+
 /* return NULL on success, else hostname running the gc */
 static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
 {
@@ -179,13 +195,19 @@ static const char *lock_repo_for_gc(int force, pid_t* 
ret_pid)
FILE *fp;
int fd, should_exit;
 
+   if (pidfile)
+   /* already locked */
+   return NULL;
+
if (gethostname(my_host, sizeof(my_host)))
strcpy(my_host, "unknown");
 
-   fd = hold_lock_file_for_update(&lock, git_path("gc.pid"),
+   pidfile = git_pathdup("gc.pid");
+
+   fd = hold_lock_file_for_update(&lock, pidfile,
   LOCK_DIE_ON_ERROR);
if (!force) {
-   fp = fopen(git_path("gc.pid"), "r");
+   fp = fopen(pidfile, "r");
memset(locking_host, 0, sizeof(locking_host));
should_exit =
fp != NULL &&
@@ -208,6 +230,7 @@ static const char *lock_repo_for_gc(int force, pid_t* 
ret_pid)
if (should_exit) {
if (fd >= 0)
rollback_lock_file(&lock);
+   pidfile = NULL;
*ret_pid = pid;
return locking_host;
}
@@ -219,6 +242,9 @@ static const char *lock_repo_for_gc(int force, pid_t* 
ret_pid)
strbuf_release(&sb);
commit_lock_file(&lock);
 
+   sigchain_push_common(remove_pidfile_on_signal);
+   atexit(remove_pidfile);
+
return NULL;
 }
 
--
To unsubscribe from this list: send the line "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] RelNotes/1.8.5: direct script writers to "git status --porcelain"

2013-09-27 Thread Jonathan Nieder
Keshav Kini wrote:
> Jakub Narebski  writes:
>> Matthieu Moy  writes:

>>>   * "git status" now omits the prefix to make its output a comment in a
>>> commit log editor, which is not necessary for human consumption.
>>> +   Scripts that parse the output of "git status" are advised to use
>>> +   "git status --porcelain", which is both easier to parse and stable,
>>> +   instead.
>>
>> Good addition.
[...]
>   rewording it to the following is much better, IMHO:
> '... are advised to use "git status --porcelain" instead, as it is both
> stable and easier to parse.'

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


Re: [PATCH 0/2] Update 'git remote set-head' doc and uasage

2013-09-27 Thread Jonathan Nieder
Philip Oakley wrote:

> In Junio's recent patch series ([PATCH v3 0/7] Removing the guesswork
> of HEAD in "clone" $gmane/234950), his first patch updated t5505: 'fix
> "set-head --auto with ambiguous HEAD" test'.
>
> A quick look at the git remote man page showed that --auto was not
> documented, nor listed in usage stings.

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: git rebase is confused about commits w/o textual changes (e.g. chmod's)

2013-09-27 Thread Paul Sokolovsky
Hello Brian,

On Fri, 27 Sep 2013 22:28:07 +
"brian m. carlson"  wrote:

> On Tue, Sep 24, 2013 at 10:56:48PM +0300, Paul Sokolovsky wrote:
> > Hello,
> > 
> > git rebase is confused about commits like
> > https://github.com/pfalcon/civetweb/commit/ce8493837bf7676c6d824cdcb1d5e3a7ed476fe1
> > - it stops, telling user to just run rebase --continue. I remember
[]

> 
> I'm interested in solving this, but I can't seem to reproduce it with
> the following script.  Can you provide more information about which
> branches specifically you were using (as well as which git version)
> so I can reproduce the problem and look into fixing it?

Thanks for your reply - I wondered if my message went thru (I'm not
subscribed to the list). I'm running:

$ git --version
git version 1.8.4

Specifically from Ubuntu PPA:
http://ppa.launchpad.net/git-core/ppa/ubuntu


Script to reproduce the issue is:
https://gist.github.com/pfalcon/6736632 , based on a real-world case of
merging histories of a fork created from a flat tree snapshot with
the original project it was created from.


Thanks,
 Paul  mailto:pmis...@gmail.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: git rebase is confused about commits w/o textual changes (e.g. chmod's)

2013-09-27 Thread brian m. carlson
On Tue, Sep 24, 2013 at 10:56:48PM +0300, Paul Sokolovsky wrote:
> Hello,
> 
> git rebase is confused about commits like
> https://github.com/pfalcon/civetweb/commit/ce8493837bf7676c6d824cdcb1d5e3a7ed476fe1
> - it stops, telling user to just run rebase --continue. I remember like
> few years ago rebase was confused like that oftentimes, which is in turn
> confused novices trying rebase-based workflow. There's big progress
> over years, and it would be nice to make it just perfect.
> 
> The exact messages are:
> 
> + git rebase --preserve-merges --onto upstream-master 
> e61d4efbe4d34d64e6be50ad5009045e4ff06764 HEAD
> The previous cherry-pick is now empty, possibly due to conflict resolution.
> If you wish to commit it anyway, use:
> 
> git commit --allow-empty
> 
> Otherwise, please use 'git reset'
> # rebase in progress; onto a0b43ae
> # You are currently rebasing.
> #   (all conflicts fixed: run "git rebase --continue")
> #
> nothing to commit, working directory clean
> Could not pick 5831bf1affad12bfa3146c37b8b622ba4e584ca3

I'm interested in solving this, but I can't seem to reproduce it with
the following script.  Can you provide more information about which
branches specifically you were using (as well as which git version) so I
can reproduce the problem and look into fixing it?

  git checkout -b test-base
  printf "line 1\n" >example
  git add example
  git commit -m "add line 1"

  git checkout -b to-rebase
  chmod 755 example
  git add example
  git commit -m "change permissions"

  git checkout test-base
  printf "line 2\n" >>example
  git add example
  git commit -m "add line"

  git checkout to-rebase
  git rebase test-base
  test -x example

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


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

2013-09-27 Thread Jonathan Nieder
Johannes Sixt wrote:
> Am 9/27/2013 14:10, schrieb Ramkumar Ramachandra:

>> +v->s = xstrdup(buf);
>> +}
>
> These strdupped strings are leaked, right?

The convention seems to be that each refinfo owns its atom_value,
which owns its string that is kept on the heap.  Except when it isn't
(e.g., "v->s = typename(obj->type);").  So at least this patch doesn't
make the muddle any worse. ;-)

A nice followup would be to consistently allocate atom_value.s on the
heap, check for a GIT_FREE_AT_EXIT envvar, and free the refinfos
if that envvar is set at exit.  That would make sure that the code is
careful enough with memory to some day free some refinfo earlier when
there are many refs.  Until that's ready, I think continuing to mix
and match like this (constant strings left as is, dynamically
generated strings on the heap) is the best we can do.

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] clone: tighten "local paths with colons" check a bit

2013-09-27 Thread Jonathan Nieder
Nguyễn Thái Ngọc Duy wrote:

> commit 6000334 (clone: allow cloning local paths with colons in them -
> 2013-05-04) is added to make it possible to specify a path that has
> colons in it without file://, e.g. ../foo:bar/somewhere. But the check
> is a bit loose.
[...]
> Make sure we only check so when no protocol is specified and the url
> is not started with '['.

More precisely, this disables the "'/' before ':'" check when the
url has been mangled by '[]' unwrapping (which only happens if the
URL starts with '[' and contains an ']' at some point later).

If I try to clone "[foo]bar/baz:qux", after this change it will act as
though I specified the remote repository "foo:qux" instead of the local
repository "./foo:qux" as before this change.  Both are wrong ---
that's a bug for another day.

Thanks, both.
--
To unsubscribe from this list: send the line "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: [ANNOUNCE] Git v1.8.4.1

2013-09-27 Thread Jonathan Nieder
Marc Branchaud wrote:
> On 13-09-27 02:52 PM, Jonathan Nieder wrote:

>> The following public repositories all have a copy of the v1.8.4.1
>> tag and the maint branch that the tag points at:
>>
>>   url = https://googlers.googlesource.com/jrn/git
>>   url = git://repo.or.cz/git/jrn.git
>>   url = git://gitorious.org/git/jrn.git
>>   url = https://github.com/jrn/git
>
> Are none of Junio's regular repos going to be updated while he's on vacation?

Yes, that's right.

> I don't think it's a big deal if they're not, though some mention of it in
> this annoucement would've been appreciated

True.  Next time. ;-)

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: [ANNOUNCE] Git v1.8.4.1

2013-09-27 Thread Marc Branchaud
On 13-09-27 02:52 PM, Jonathan Nieder wrote:
> The latest maintenance release Git v1.8.4.1 is now available.
> 
> The release tarballs are found at:
> 
> http://alioth.debian.org/~jrnieder-guest/git/
> 
> and their SHA-1 checksums are:
> 
> 49004a8dfcbb7c0848147737d9877fd7313a42ec  git-1.8.4.1.tar.gz
> 1f0e5c5934ec333b5630a8c93a0fb0b1895dfcb8  git-htmldocs-1.8.4.1.tar.gz
> dc0f9de1cacc8912f131b67dc5a19a96768ecc95  git-manpages-1.8.4.1.tar.gz
> 
> The following public repositories all have a copy of the v1.8.4.1
> tag and the maint branch that the tag points at:
> 
>   url = https://googlers.googlesource.com/jrn/git
>   url = git://repo.or.cz/git/jrn.git
>   url = git://gitorious.org/git/jrn.git
>   url = https://github.com/jrn/git

Are none of Junio's regular repos going to be updated while he's on vacation?

I don't think it's a big deal if they're not, though some mention of it in
this annoucement would've been appreciated -- I was about to email a
complaint that there's no v1.8.4.1 tag...

(My apologies if I missed mention of this elsewhere!)

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: [PATCH] clone: tighten "local paths with colons" check a bit

2013-09-27 Thread Jeff King
On Fri, Sep 27, 2013 at 08:48:13PM +0700, Nguyen Thai Ngoc Duy wrote:

> ---
>  I wanted to add a test then realized there were no ssh tests in the
>  test suite. So laziness won :p

There is one in t5602, but it's not very reusable. How about squashing
in the patch below, which does a basic ssh-works test, and confirms your
fix?

---
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 0629149..a3e3d48 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -280,9 +280,53 @@ test_expect_success NOT_MINGW,NOT_CYGWIN 'clone local path 
foo:bar' '
test_cmp fetch.expected fetch.actual
 '
 
+test_expect_success 'setup ssh wrapper' '
+   write_script "$TRASH_DIRECTORY/ssh-wrapper" <<-\EOF &&
+   echo >>"$TRASH_DIRECTORY/ssh-output" "ssh: $*" &&
+   # throw away all but the last argument, which should be the
+   # command
+   while test $# -gt 1; do shift; done
+   eval "$1"
+   EOF
+
+   GIT_SSH="$TRASH_DIRECTORY/ssh-wrapper" &&
+   export GIT_SSH &&
+   export TRASH_DIRECTORY
+'
+
+clear_ssh () {
+   >"$TRASH_DIRECTORY/ssh-output"
+}
+
+expect_ssh () {
+   {
+   case "$1" in
+   none)
+   ;;
+   *)
+   echo "ssh: $1 git-upload-pack '$2'"
+   esac
+   } >"$TRASH_DIRECTORY/ssh-expect" &&
+   (cd "$TRASH_DIRECTORY" && test_cmp ssh-expect ssh-output)
+}
+
+test_expect_success 'cloning myhost:src uses ssh' '
+   clear_ssh &&
+   git clone myhost:src ssh-clone &&
+   expect_ssh myhost src
+'
+
 test_expect_success NOT_MINGW,NOT_CYGWIN 'clone local path foo:bar' '
+   clear_ssh &&
cp -R src "foo:bar" &&
-   git clone "./foo:bar" foobar
+   git clone "./foo:bar" foobar &&
+   expect_ssh none
+'
+
+test_expect_success 'bracketed hostnames are still ssh' '
+   clear_ssh &&
+   git clone "[myhost:123]:src" ssh-bracket-clone &&
+   expect_ssh myhost:123 src
 '
 
 test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[ANNOUNCE] Git v1.8.4.1

2013-09-27 Thread Jonathan Nieder
The latest maintenance release Git v1.8.4.1 is now available.

The release tarballs are found at:

http://alioth.debian.org/~jrnieder-guest/git/

and their SHA-1 checksums are:

49004a8dfcbb7c0848147737d9877fd7313a42ec  git-1.8.4.1.tar.gz
1f0e5c5934ec333b5630a8c93a0fb0b1895dfcb8  git-htmldocs-1.8.4.1.tar.gz
dc0f9de1cacc8912f131b67dc5a19a96768ecc95  git-manpages-1.8.4.1.tar.gz

The following public repositories all have a copy of the v1.8.4.1
tag and the maint branch that the tag points at:

  url = https://googlers.googlesource.com/jrn/git
  url = git://repo.or.cz/git/jrn.git
  url = git://gitorious.org/git/jrn.git
  url = https://github.com/jrn/git

Git v1.8.4.1 Release Notes


Fixes since v1.8.4
--

 * Some old versions of bash do not grok some constructs like
   'printf -v varname' which the prompt and completion code started
   to use recently.  The completion and prompt scripts have been
   adjusted to work better with these old versions of bash.

 * In FreeBSD's and NetBSD's "sh", a return in a dot script in a
   function returns from the function, not only in the dot script,
   breaking "git rebase" on these platforms (regression introduced
   in 1.8.4-rc1).

 * "git rebase -i" and other scripted commands were feeding a
   random, data dependant error message to 'echo' and expecting it
   to come out literally.

 * Setting the "submodule..path" variable to the empty
   "true" caused the configuration parser to segfault.

 * Output from "git log --full-diff -- " looked strange
   because comparison was done with the previous ancestor that
   touched the specified , causing the patches for paths
   outside the pathspec to show more than the single commit has
   changed.

 * The auto-tag-following code in "git fetch" tries to reuse the
   same transport twice when the serving end does not cooperate and
   does not give tags that point to commits that are asked for as
   part of the primary transfer.  Unfortunately, Git-aware transport
   helper interface is not designed to be used more than once, hence
   this did not work over smart-http transfer.  Fixed.

 * Send a large request to read(2)/write(2) as a smaller but still
   reasonably large chunks, which would improve the latency when the
   operation needs to be killed and incidentally works around broken
   64-bit systems that cannot take a 2GB write or read in one go.

 * A ".mailmap" file that ends with an incomplete line, when read
   from a blob, was not handled properly.

 * The recent "short-cut clone connectivity check" topic broke a
   shallow repository when a fetch operation tries to auto-follow
   tags.

 * When send-email comes up with an error message to die with upon
   failure to start an SSL session, it tried to read the error
   string from a wrong place.

 * A call to xread() was used without a loop to cope with short
   read in the codepath to stream large blobs to a pack.

 * On platforms with fgetc() and friends defined as macros, the
   configuration parser did not compile.

 * New versions of MediaWiki introduced a new API for returning
   more than 500 results in response to a query, which would cause
   the MediaWiki remote helper to go into an infinite loop.

 * Subversion's serf access method (the only one available in
   Subversion 1.8) for http and https URLs in skelta mode tells its
   caller to open multiple files at a time, which made "git svn
   fetch" complain that "Temp file with moniker 'svn_delta' already
   in use" instead of fetching.


Also contains a handful of trivial code clean-ups, documentation
updates, updates to the test suite, etc.



Changes since v1.8.4 are as follows:

Andreas Schwab (1):
  Documentation/git-merge.txt: fix formatting of example block

Benoit Person (1):
  git-remote-mediawiki: bugfix for pages w/ >500 revisions

Brandon Casey (3):
  git-completion.bash: use correct Bash/Zsh array length syntax
  t9902-completion.sh: old Bash still does not support array+=('') notation
  contrib/git-prompt.sh: handle missing 'printf -v' more gracefully

Jeff King (2):
  config: do not use C function names as struct members
  mailmap: handle mailmap blobs without trailing newlines

Jharrod LaFon (1):
  avoid segfault on submodule.*.path set to an empty "true"

Johannes Sixt (1):
  stream_to_pack: xread does not guarantee to read all requested bytes

Jonathan Nieder (1):
  Git 1.8.4.1

Junio C Hamano (6):
  t5802: add test for connect helper
  fetch: rename file-scope global "transport" to "gtransport"
  fetch: refactor code that prepares a transport
  fetch: refactor code that fetches leftover tags
  fetch: work around "transport-take-over" hack
  Start preparing for 1.8.4.1

Kyle J. McKay (3):
  Git.pm: add new temp_is_locked function
  git-svn: allow git-svn fetching to work using serf
  Git.pm: revert _temp_cache use

Re: What's cooking in git.git (Sep 2013, #08; Wed, 25)

2013-09-27 Thread Jonathan Nieder
Philip Oakley wrote:

> Jonathan,
> Did you spot my recent
> http://thread.gmane.org/gmane.comp.version-control.git/235127?

Will take a look.  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: What's cooking in git.git (Sep 2013, #08; Wed, 25)

2013-09-27 Thread Philip Oakley

From: "Jonathan Nieder" 

What's cooking in git.git (Sep 2013, #08; Wed, 25)




* po/dot-url (2013-09-13) 2 commits
 (merged to 'next' on 2013-09-20 at 6a12786)
+ config doc: update dot-repository notes
+ doc: command line interface (cli) dot-repository dwimmery

Explain how '.' can be used to refer to the "current repository"
in the documentation.

Will merge to 'master'.


Jonathan,
Did you spot my recent 
http://thread.gmane.org/gmane.comp.version-control.git/235127?


I'd been off ill, and away on business, so hadn't had much chance to 
properly keep up.


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 3/3] for-each-ref: introduce %(upstream:track[short])

2013-09-27 Thread Philip Oakley

From: "Ramkumar Ramachandra" 

Philip Oakley wrote:
"=" and "<>" I can easily understand (binary choice), but ">" and "<" 
will

need to be clear which way they indicate in terms of matching
the "[ahead N]" and  "[behind M]" options.


The ">" corresponds to ahead, while "<" is behind. You'll get used to
it pretty quickly :)



But this documentation section could say ;-)

diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt



regards

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 3/3] for-each-ref: introduce %(upstream:track[short])

2013-09-27 Thread Ramkumar Ramachandra
Philip Oakley wrote:
> "=" and "<>" I can easily understand (binary choice), but ">" and "<" will
> need to be clear which way they indicate in terms of matching
> the "[ahead N]" and  "[behind M]" options.

The ">" corresponds to ahead, while "<" is behind. You'll get used to
it pretty quickly :)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-09-27 Thread Philip Oakley
- Original Message - 
From: "Ramkumar Ramachandra" 

Sent: Friday, September 27, 2013 1:10 PM

Introduce %(upstream:track) to display "[ahead M, behind N]" and
%(upstream:trackshort) to display "=", ">", "<", or "<>"
appropriately (inspired by contrib/completion/git-prompt.sh).

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

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

to display refs with terse tracking information.

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

Signed-off-by: Ramkumar Ramachandra 
---
Documentation/git-for-each-ref.txt |  6 +-
builtin/for-each-ref.c | 44 
--

2 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt

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



"=" and "<>" I can easily understand (binary choice), but ">" and "<" 
will

need to be clear which way they indicate in terms of matching
the "[ahead N]" and  "[behind M]" options.

Otherwise a good idea.

Philip


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

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

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

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

--


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


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

2013-09-27 Thread Morten Stenshorne
Phil Hord  writes:

> On Fri, Sep 27, 2013 at 4:07 AM, Morten Stenshorne  wrote:
>> If I don't go via the ssh tunnel (I finally have some VPN stuff these
>> days, so I don't really need the tunnel thing anymore, but that's going
>> to be a lot of remotes to update, so I'd prefer it just worked like it
>> used to):
>>
>> -url = [localhost:2223]:blink.git
>> +url = git:blink.git
>>
>> ... it works fine.
>
>
> Until you get a proper fix, I wonder if this will help:
>
>   git config --global --add url."git:".insteadOf  "[localhost:2223]:"
>
> See "git help config" for details on the insteadOf config setting.

Yes, that works. Thanks!

-- 
 Morten Stenshorne, developer, Opera Software ASA 
-- http://www.opera.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 3/3] for-each-ref: introduce %(upstream:track[short])

2013-09-27 Thread Ramkumar Ramachandra
Johannes Sixt wrote:
>> + else if (!num_ours) {
>> + sprintf(buf, "[behind %d]", 
>> num_theirs);
>> + v->s = xstrdup(buf);
>> + } else if (!num_theirs) {
>> + sprintf(buf, "[ahead %d]", num_ours);
>> + v->s = xstrdup(buf);
>> + } else {
>> + sprintf(buf, "[ahead %d, behind %d]",
>> + num_ours, num_theirs);
>> + v->s = xstrdup(buf);
>> + }
>
> These strdupped strings are leaked, right?

Yes, there's a minor leakage; there are quite a few instances of this
in the rest of the file. Do you see an easy 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: Bug: [hostname:port]:repo.git notation no longer works (for ssh)

2013-09-27 Thread Phil Hord
On Fri, Sep 27, 2013 at 4:07 AM, Morten Stenshorne  wrote:
> If I don't go via the ssh tunnel (I finally have some VPN stuff these
> days, so I don't really need the tunnel thing anymore, but that's going
> to be a lot of remotes to update, so I'd prefer it just worked like it
> used to):
>
> -url = [localhost:2223]:blink.git
> +url = git:blink.git
>
> ... it works fine.


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

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

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

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


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

2013-09-27 Thread Ramkumar Ramachandra
Phil Hord wrote:
>> --- a/builtin/for-each-ref.c
>> +++ b/builtin/for-each-ref.c
>> @@ -631,6 +631,7 @@ static void populate_value(struct refinfo *ref)
>> int eaten, i;
>> unsigned long size;
>> const unsigned char *tagged;
>> +   int upstream_present = 0;
>
> This flag is out of place.  It should be in the same scope as 'branch'
> since the code which depends on this flag also depends on '!!branch'.

Agreed. Fixed.

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

Yeah, you're right. I was setting upstream_present in this snippet:

  else if (!prefixcmp(name, "upstream")) {
  /* only local branches may have an upstream */
if (prefixcmp(ref->refname, "refs/heads/"))
  continue;

If the refname doesn't begin with "refs/heads" in the first place
(which is what I was guarding against), the code will loop and never
reach the track[short] code anyway.

upstream_present factored out now.

>> @@ -698,11 +701,48 @@ static void populate_value(struct refinfo *ref)
>> formatp = strchr(name, ':');
>> /* look for "short" refname format */
>> if (formatp) {
>> +   int num_ours, num_theirs;
>> +
>> formatp++;
>> if (!strcmp(formatp, "short"))
>> refname = shorten_unambiguous_ref(refname,
>>   warn_ambiguous_refs);
>> -   else
>> +   else if (!strcmp(formatp, "track") &&
>> +   !prefixcmp(name, "upstream")) {
>> +   char buf[40];
>> +
>> +   if (!upstream_present)
>> +   continue;
>> +   stat_tracking_info(branch, &num_ours, 
>> &num_theirs);
>> +   if (!num_ours && !num_theirs)
>> +   v->s = "";
>
> Is this the same as 'continue'?

I'll leave this as it is for readability reasons.

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


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

2013-09-27 Thread Johannes Sixt
Am 9/27/2013 14:10, schrieb Ramkumar Ramachandra:
> + else if (!strcmp(formatp, "track") &&
> + !prefixcmp(name, "upstream")) {
> + char buf[40];
> +
> + if (!upstream_present)
> + continue;
> + stat_tracking_info(branch, &num_ours, 
> &num_theirs);
> + if (!num_ours && !num_theirs)
> + v->s = "";
> + else if (!num_ours) {
> + sprintf(buf, "[behind %d]", num_theirs);
> + v->s = xstrdup(buf);
> + } else if (!num_theirs) {
> + sprintf(buf, "[ahead %d]", num_ours);
> + v->s = xstrdup(buf);
> + } else {
> + sprintf(buf, "[ahead %d, behind %d]",
> + num_ours, num_theirs);
> + v->s = xstrdup(buf);
> + }

These strdupped strings are leaked, right?

> + continue;
> + } else if (!strcmp(formatp, "trackshort") &&
> + !prefixcmp(name, "upstream")) {
> + if (!upstream_present)
> + continue;
> + stat_tracking_info(branch, &num_ours, 
> &num_theirs);
> + if (!num_ours && !num_theirs)
> + v->s = "=";
> + else if (!num_ours)
> + v->s = "<";
> + else if (!num_theirs)
> + v->s = ">";
> + else
> + v->s = "<>";
> + continue;
> + } else
>   die("unknown %.*s format %s",
>   (int)(formatp - name), name, formatp);
>   }
> 

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


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

2013-09-27 Thread Phil Hord
On Fri, Sep 27, 2013 at 8:10 AM, Ramkumar Ramachandra
 wrote:
> Introduce %(upstream:track) to display "[ahead M, behind N]" and
> %(upstream:trackshort) to display "=", ">", "<", or "<>"
> appropriately (inspired by contrib/completion/git-prompt.sh).
>
> Now you can use the following format in for-each-ref:
>
>   %C(green)%(refname:short)%C(reset)%(upstream:trackshort)
>
> to display refs with terse tracking information.

Thanks.  I like this.

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

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

>
> Signed-off-by: Ramkumar Ramachandra 
> ---
>  Documentation/git-for-each-ref.txt |  6 +-
>  builtin/for-each-ref.c | 44 
> --
>  2 files changed, 47 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-for-each-ref.txt 
> b/Documentation/git-for-each-ref.txt
> index f1d4e9e..682eaa8 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -93,7 +93,11 @@ objectname::
>  upstream::
> The name of a local ref which can be considered ``upstream''
> from the displayed ref. Respects `:short` in the same way as
> -   `refname` above.
> +   `refname` above.  Additionally respects `:track` to show
> +   "[ahead N, behind M]" and `:trackshort` to show the terse
> +   version (like the prompt) ">", "<", "<>", or "=".  Has no
> +   effect if the ref does not have tracking information
> +   associated with it.
>
>  HEAD::
> Used to indicate the currently checked out branch.  Is '*' if
> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
> index e54b5d8..10843bb 100644
> --- a/builtin/for-each-ref.c
> +++ b/builtin/for-each-ref.c
> @@ -631,6 +631,7 @@ static void populate_value(struct refinfo *ref)
> int eaten, i;
> unsigned long size;
> const unsigned char *tagged;
> +   int upstream_present = 0;

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

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

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

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

2013-09-27 Thread Duy Nguyen
On Fri, Sep 27, 2013 at 3:55 PM, Stefan Näwe
 wrote:
>> [remote "exp"]
>> url = [localhost:2223]:blink.git
>> fetch = +refs/heads/*:refs/remotes/exp/*
>>
>> However, now I get this message:
>>
>> $ git fetch exp
>> fatal: ':blink.git' does not appear to be a git repository
>> fatal: Could not read from remote repository.
>
> I wonder why that worked (especially the "[...]") at all ?
> I thought specifying a port for a SSH connection was always only
> possible when using
>
>ssh://user@host:port/path/to/repo.git
> - or -
>ssh://user@host:port/~user/path/to/repo.git
>
> At least that's what I always read out of the git-clone man page.

[] is used to wrap ipv6 and because we don't know if it's actually
ipv6 or v4, we accept it in both cases, so [abc] can be used in place
"host" above. No [host:port]:path won't work because "host:port" is
considered host name. But [host or ip]:path may work (that is after I
fix my bug).
-- 
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] clone: tighten "local paths with colons" check a bit

2013-09-27 Thread Nguyễn Thái Ngọc Duy
commit 6000334 (clone: allow cloning local paths with colons in them -
2013-05-04) is added to make it possible to specify a path that has
colons in it without file://, e.g. ../foo:bar/somewhere. But the check
is a bit loose.

Consider the url '[foo]:bar', the '[]' unwrapping code will turn the
string to 'foo\0:bar'. The effect of this new string is the same as
'foo/:bar' to the expression "path < strchrnul(host, '/')", which
mistakes it as a sign of local paths while it's actually not.

Make sure we only check so when no protocol is specified and the url
is not started with '['.

Noticed-by: Morten Stenshorne 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 I wanted to add a test then realized there were no ssh tests in the
 test suite. So laziness won :p

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

diff --git a/connect.c b/connect.c
index a0783d4..303f850 100644
--- a/connect.c
+++ b/connect.c
@@ -551,7 +551,7 @@ struct child_process *git_connect(int fd[2], const char 
*url_orig,
path = strchr(end, c);
if (path && !has_dos_drive_prefix(end)) {
if (c == ':') {
-   if (path < strchrnul(host, '/')) {
+   if (host != url || path < strchrnul(host, '/')) {
protocol = PROTO_SSH;
*path++ = '\0';
} else /* '/' in the host part, assume local path */
-- 
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: [PATCH 1/3] for-each-ref: introduce %C(...) for color

2013-09-27 Thread Phil Hord
On Fri, Sep 27, 2013 at 8:10 AM, Ramkumar Ramachandra
 wrote:
> Enhance 'git for-each-ref' with color formatting options.  You can now
> use the following format in for-each-ref:
>
>   %C(green)%(refname:short)%C(reset)
>
> Signed-off-by: Ramkumar Ramachandra 
> ---
>  Documentation/git-for-each-ref.txt |  4 +++-
>  builtin/for-each-ref.c | 23 +++
>  2 files changed, 22 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/git-for-each-ref.txt 
> b/Documentation/git-for-each-ref.txt
> index f2e08d1..078a116 100644
> --- a/Documentation/git-for-each-ref.txt
> +++ b/Documentation/git-for-each-ref.txt
> @@ -45,7 +45,9 @@ OPTIONS
> It also interpolates `%%` to `%`, and `%xx` where `xx`
> are hex digits interpolates to character with hex code
> `xx`; for example `%00` interpolates to `\0` (NUL),
> -   `%09` to `\t` (TAB) and `%0a` to `\n` (LF).
> +   `%09` to `\t` (TAB) and `%0a` to `\n` (LF). Additionally,
> +   colors can be specified using `%C(colorname)`. Use
> +   `%C(reset)` to reset the color.

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

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

>
>  ...::
> If one or more patterns are given, only refs are shown that
> diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
> index 1d4083c..a1ca186 100644
> --- a/builtin/for-each-ref.c
> +++ b/builtin/for-each-ref.c
> @@ -9,6 +9,7 @@
>  #include "quote.h"
>  #include "parse-options.h"
>  #include "remote.h"
> +#include "color.h"
>
>  /* Quoting styles */
>  #define QUOTE_NONE 0
> @@ -155,10 +156,13 @@ static const char *find_next(const char *cp)
> while (*cp) {
> if (*cp == '%') {
> /*
> +* %C( is the start of a color;
>  * %( is the start of an atom;
>  * %% is a quoted per-cent.
>  */
> -   if (cp[1] == '(')
> +   if (cp[1] == 'C' && cp[2] == '(')
> +   return cp;
> +   else if (cp[1] == '(')
> return cp;
> else if (cp[1] == '%')
> cp++; /* skip over two % */
> @@ -180,8 +184,11 @@ static int verify_format(const char *format)
> const char *ep = strchr(sp, ')');
> if (!ep)
> return error("malformed format string %s", sp);
> -   /* sp points at "%(" and ep points at the closing ")" */
> -   parse_atom(sp + 2, ep);
> +   /* Ignore color specifications: %C(
> +* sp points at "%(" and ep points at the closing ")"
> +*/
> +   if (prefixcmp(sp, "%C("))
> +   parse_atom(sp + 2, ep);
> cp = ep + 1;
> }
> return 0;
> @@ -933,12 +940,20 @@ static void emit(const char *cp, const char *ep)
>  static void show_ref(struct refinfo *info, const char *format, int 
> quote_style)
>  {
> const char *cp, *sp, *ep;
> +   char color[COLOR_MAXLEN];
>
> for (cp = format; *cp && (sp = find_next(cp)); cp = ep + 1) {
> ep = strchr(sp, ')');
> if (cp < sp)
> emit(cp, sp);
> -   print_value(info, parse_atom(sp + 2, ep), quote_style);
> +
> +   /* Do we have a color specification? */
> +   if (!prefixcmp(sp, "%C("))
> +   color_parse_mem(sp + 3, ep - sp - 3, "--format", 
> color);
> +   else {
> +   printf("%s", color);

'color' used uninitialized here?

> +   print_value(info, parse_atom(sp + 2, ep), 
> quote_style);
> +   }
> }
> if (*cp) {
> sp = cp + strlen(cp);
> --
> 1.8.4.478.g55109e3
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-09-27 Thread Ramkumar Ramachandra
Introduce %(upstream:track) to display "[ahead M, behind N]" and
%(upstream:trackshort) to display "=", ">", "<", or "<>"
appropriately (inspired by contrib/completion/git-prompt.sh).

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

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

to display refs with terse tracking information.

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

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

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index f1d4e9e..682eaa8 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -93,7 +93,11 @@ objectname::
 upstream::
The name of a local ref which can be considered ``upstream''
from the displayed ref. Respects `:short` in the same way as
-   `refname` above.
+   `refname` above.  Additionally respects `:track` to show
+   "[ahead N, behind M]" and `:trackshort` to show the terse
+   version (like the prompt) ">", "<", "<>", or "=".  Has no
+   effect if the ref does not have tracking information
+   associated with it.
 
 HEAD::
Used to indicate the currently checked out branch.  Is '*' if
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index e54b5d8..10843bb 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -631,6 +631,7 @@ static void populate_value(struct refinfo *ref)
int eaten, i;
unsigned long size;
const unsigned char *tagged;
+   int upstream_present = 0;
 
ref->value = xcalloc(sizeof(struct atom_value), used_atom_cnt);
 
@@ -648,6 +649,7 @@ static void populate_value(struct refinfo *ref)
int deref = 0;
const char *refname;
const char *formatp;
+   struct branch *branch;
 
if (*name == '*') {
deref = 1;
@@ -659,7 +661,6 @@ static void populate_value(struct refinfo *ref)
else if (!prefixcmp(name, "symref"))
refname = ref->symref ? ref->symref : "";
else if (!prefixcmp(name, "upstream")) {
-   struct branch *branch;
/* only local branches may have an upstream */
if (prefixcmp(ref->refname, "refs/heads/"))
continue;
@@ -669,6 +670,7 @@ static void populate_value(struct refinfo *ref)
!branch->merge[0]->dst)
continue;
refname = branch->merge[0]->dst;
+   upstream_present = 1;
}
else if (!strcmp(name, "flag")) {
char buf[256], *cp = buf;
@@ -686,6 +688,7 @@ static void populate_value(struct refinfo *ref)
} else if (!strcmp(name, "HEAD")) {
const char *head;
unsigned char sha1[20];
+
head = resolve_ref_unsafe("HEAD", sha1, 1, NULL);
if (!strcmp(ref->refname, head))
v->s = "*";
@@ -698,11 +701,48 @@ static void populate_value(struct refinfo *ref)
formatp = strchr(name, ':');
/* look for "short" refname format */
if (formatp) {
+   int num_ours, num_theirs;
+
formatp++;
if (!strcmp(formatp, "short"))
refname = shorten_unambiguous_ref(refname,
  warn_ambiguous_refs);
-   else
+   else if (!strcmp(formatp, "track") &&
+   !prefixcmp(name, "upstream")) {
+   char buf[40];
+
+   if (!upstream_present)
+   continue;
+   stat_tracking_info(branch, &num_ours, 
&num_theirs);
+   if (!num_ours && !num_theirs)
+   v->s = "";
+   else if (!num_ours) {
+   sprintf(buf, "[behind %d]", num_theirs);
+   v->s = xstrdup(buf);
+   } else if (!num_theirs) {
+   sprintf(buf, "[ahead %d]", num_ours);
+   v->s = xstrdup(buf);
+   } else {
+   sprintf(buf, "[ahead %d, behind %d]",
+   num_ours, num_theirs);
+

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

2013-09-27 Thread Ramkumar Ramachandra
Enhance 'git for-each-ref' with color formatting options.  You can now
use the following format in for-each-ref:

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

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

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index f2e08d1..078a116 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -45,7 +45,9 @@ OPTIONS
It also interpolates `%%` to `%`, and `%xx` where `xx`
are hex digits interpolates to character with hex code
`xx`; for example `%00` interpolates to `\0` (NUL),
-   `%09` to `\t` (TAB) and `%0a` to `\n` (LF).
+   `%09` to `\t` (TAB) and `%0a` to `\n` (LF). Additionally,
+   colors can be specified using `%C(colorname)`. Use
+   `%C(reset)` to reset the color.
 
 ...::
If one or more patterns are given, only refs are shown that
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 1d4083c..a1ca186 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -9,6 +9,7 @@
 #include "quote.h"
 #include "parse-options.h"
 #include "remote.h"
+#include "color.h"
 
 /* Quoting styles */
 #define QUOTE_NONE 0
@@ -155,10 +156,13 @@ static const char *find_next(const char *cp)
while (*cp) {
if (*cp == '%') {
/*
+* %C( is the start of a color;
 * %( is the start of an atom;
 * %% is a quoted per-cent.
 */
-   if (cp[1] == '(')
+   if (cp[1] == 'C' && cp[2] == '(')
+   return cp;
+   else if (cp[1] == '(')
return cp;
else if (cp[1] == '%')
cp++; /* skip over two % */
@@ -180,8 +184,11 @@ static int verify_format(const char *format)
const char *ep = strchr(sp, ')');
if (!ep)
return error("malformed format string %s", sp);
-   /* sp points at "%(" and ep points at the closing ")" */
-   parse_atom(sp + 2, ep);
+   /* Ignore color specifications: %C(
+* sp points at "%(" and ep points at the closing ")"
+*/
+   if (prefixcmp(sp, "%C("))
+   parse_atom(sp + 2, ep);
cp = ep + 1;
}
return 0;
@@ -933,12 +940,20 @@ static void emit(const char *cp, const char *ep)
 static void show_ref(struct refinfo *info, const char *format, int quote_style)
 {
const char *cp, *sp, *ep;
+   char color[COLOR_MAXLEN];
 
for (cp = format; *cp && (sp = find_next(cp)); cp = ep + 1) {
ep = strchr(sp, ')');
if (cp < sp)
emit(cp, sp);
-   print_value(info, parse_atom(sp + 2, ep), quote_style);
+
+   /* Do we have a color specification? */
+   if (!prefixcmp(sp, "%C("))
+   color_parse_mem(sp + 3, ep - sp - 3, "--format", color);
+   else {
+   printf("%s", color);
+   print_value(info, parse_atom(sp + 2, ep), quote_style);
+   }
}
if (*cp) {
sp = cp + strlen(cp);
-- 
1.8.4.478.g55109e3

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


[PATCH 0/3] Juggling between hot branches

2013-09-27 Thread Ramkumar Ramachandra
Hi,

I juggle between several hot branches, and an alphabetical listing
from 'git branch' doesn't cut it for me. I've chosen to enhance
for-each-ref so that I get output like (with color):

  $ git hot
um-build>
perf-manifest=
  * master=
sparse=
ia32-asm-cleanup>
menuconfig-jk<>
perf-build=
perf-completion<

where hot is the following alias:

  for-each-ref --format='%C(red)%(HEAD)%C(reset)
  %C(green)%(refname:short)%C(reset)%(upstream:trackshort)' --count 10
  --sort='-committerdate' refs/heads

While the alias might look a bit horrendous, I get the desired output.

The last time I tried to get this feature merged, there was some
confusion about unifying the format of for-each-ref with
pretty-formats, and enhacing git-branch while at it. I tried going
down that road, but got no reviews; everyone was generally more
unhappy due to the added complexity. Months have passed since, and we
still don't have this feature.

Let's keep it simple and stupid. A terse +84,-10 (with documentation)
for this wonderful feature now. Let's get it merged, and defer the
kitchen-sink-unification efforts.

Thanks.

Ramkumar Ramachandra (3):
  for-each-ref: introduce %C(...) for color
  for-each-ref: introduce %(HEAD) asterisk marker
  for-each-ref: introduce %(upstream:track[short])

 Documentation/git-for-each-ref.txt | 14 ++-
 builtin/for-each-ref.c | 80 ++
 2 files changed, 84 insertions(+), 10 deletions(-)

-- 
1.8.4.478.g55109e3

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


[PATCH 2/3] for-each-ref: introduce %(HEAD) asterisk marker

2013-09-27 Thread Ramkumar Ramachandra
'git branch' shows which branch you are currently on with an '*', but
'git for-each-ref' misses this feature.  So, extend its format with
%(HEAD) for the same effect.

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

  %C(red)%(HEAD)%C(reset) %(refname:short)

to display a red asterisk next to the current ref.

Signed-off-by: Ramkumar Ramachandra 
---
 Documentation/git-for-each-ref.txt |  4 
 builtin/for-each-ref.c | 13 +++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-for-each-ref.txt 
b/Documentation/git-for-each-ref.txt
index 078a116..f1d4e9e 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -95,6 +95,10 @@ upstream::
from the displayed ref. Respects `:short` in the same way as
`refname` above.
 
+HEAD::
+   Used to indicate the currently checked out branch.  Is '*' if
+   HEAD points to the current ref, and ' ' otherwise.
+
 In addition to the above, for commit and tag objects, the header
 field names (`tree`, `parent`, `object`, `type`, and `tag`) can
 be used to specify the value in the header field.
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index a1ca186..e54b5d8 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -76,6 +76,7 @@ static struct {
{ "upstream" },
{ "symref" },
{ "flag" },
+   { "HEAD" },
 };
 
 /*
@@ -682,8 +683,16 @@ static void populate_value(struct refinfo *ref)
v->s = xstrdup(buf + 1);
}
continue;
-   }
-   else
+   } else if (!strcmp(name, "HEAD")) {
+   const char *head;
+   unsigned char sha1[20];
+   head = resolve_ref_unsafe("HEAD", sha1, 1, NULL);
+   if (!strcmp(ref->refname, head))
+   v->s = "*";
+   else
+   v->s = " ";
+   continue;
+   } else
continue;
 
formatp = strchr(name, ':');
-- 
1.8.4.478.g55109e3

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


Re: [PATCH v2] git submodule foreach: Skip eval for more than one argument

2013-09-27 Thread Johan Herland
On Fri, Sep 27, 2013 at 12:23 PM, Anders Kaseorg  wrote:
> ‘eval "$@"’ created an extra layer of shell interpretation, which was
> probably not expected by a user who passed multiple arguments to git
> submodule foreach:
>
> $ git grep "'"
> [searches for single quotes]
> $ git submodule foreach git grep "'"
> Entering '[submodule]'
> /usr/lib/git-core/git-submodule: 1: eval: Syntax error: Unterminated quoted 
> string
> Stopping at '[submodule]'; script returned non-zero status.
>
> To fix this, if the user passed more than one argument, just execute
> "$@" directly instead of passing it to eval.
>
> Signed-off-by: Anders Kaseorg 

Acked-by: Johan Herland 

> On Fri, 27 Sep 2013, Johan Herland wrote:
>> 2. If we are unlucky there might be existing users that work around the
>> existing behavior by adding an extra level of quoting (i.e. doing the
>> equivalent of git submodule foreach git grep "\'" in your example
>> above). Will their workaround break as a result of your change? Is that
>> acceptable?
>
> Anyone adding an extra level of quoting ought to realize that they should
> be passing a single argument to submodule foreach, so that the reason for
> the extra quoting is clear:
>   git submodule foreach "git grep \'"
> will not break.  If someone is actually doing
>   git submodule foreach git grep "\'"
> then this will change in behavior.  I think this change is important.
>
> (One could even imagine someone feeding untrusted input to
>   git submodule foreach git grep "$variable"
> which, without my patch, results in a nonobvious shell code injection
> vulnerability.)
>
> I considered an alternative fix where the first argument is always
> shell-evaulated and any others are not (i.e. cmd=$1 && shift && eval
> "$cmd \"\$@\""), which is potentially more useful in case the command
> needs to use $path.  But that may be too confusing, and this way has some
> precedent (e.g. perl’s system()).

Ok. I have nothing to add.

...Johan

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


[PATCH v2] git submodule foreach: Skip eval for more than one argument

2013-09-27 Thread Anders Kaseorg
‘eval "$@"’ created an extra layer of shell interpretation, which was
probably not expected by a user who passed multiple arguments to git
submodule foreach:

$ git grep "'"
[searches for single quotes]
$ git submodule foreach git grep "'"
Entering '[submodule]'
/usr/lib/git-core/git-submodule: 1: eval: Syntax error: Unterminated quoted 
string
Stopping at '[submodule]'; script returned non-zero status.

To fix this, if the user passed more than one argument, just execute
"$@" directly instead of passing it to eval.

Signed-off-by: Anders Kaseorg 
---

On Fri, 27 Sep 2013, Johan Herland wrote:
> 1. Please add the use case you mention above as a new test case, so
> that we can easily catch future regressions.

Test added.

> 2. If we are unlucky there might be existing users that work around the 
> existing behavior by adding an extra level of quoting (i.e. doing the 
> equivalent of git submodule foreach git grep "\'" in your example 
> above). Will their workaround break as a result of your change? Is that 
> acceptable?

Anyone adding an extra level of quoting ought to realize that they should 
be passing a single argument to submodule foreach, so that the reason for 
the extra quoting is clear:
  git submodule foreach "git grep \'"
will not break.  If someone is actually doing
  git submodule foreach git grep "\'"
then this will change in behavior.  I think this change is important.

(One could even imagine someone feeding untrusted input to
  git submodule foreach git grep "$variable"
which, without my patch, results in a nonobvious shell code injection 
vulnerability.)

I considered an alternative fix where the first argument is always 
shell-evaulated and any others are not (i.e. cmd=$1 && shift && eval 
"$cmd \"\$@\""), which is potentially more useful in case the command 
needs to use $path.  But that may be too confusing, and this way has some 
precedent (e.g. perl’s system()).

Anders


 git-submodule.sh | 7 ++-
 t/t7407-submodule-foreach.sh | 9 +
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index c17bef1..3381864 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -545,7 +545,12 @@ cmd_foreach()
sm_path=$(relative_path "$sm_path") &&
# we make $path available to scripts ...
path=$sm_path &&
-   eval "$@" &&
+   if [ $# -eq 1 ]
+   then
+   eval "$1"
+   else
+   "$@"
+   fi &&
if test -n "$recursive"
then
cmd_foreach "--recursive" "$@"
diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
index be93f10..6b2fd39 100755
--- a/t/t7407-submodule-foreach.sh
+++ b/t/t7407-submodule-foreach.sh
@@ -329,4 +329,13 @@ test_expect_success 'command passed to foreach --recursive 
retains notion of std
test_cmp expected actual
 '
 
+test_expect_success 'multi-argument command passed to foreach is not 
shell-evaluated twice' '
+   (
+   cd super &&
+   git submodule foreach "echo \\\"quoted\\\"" > ../expected &&
+   git submodule foreach echo \"quoted\" > ../actual
+   ) &&
+   test_cmp expected actual
+'
+
 test_done
-- 
1.8.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: Question about "git log --cherry"

2013-09-27 Thread Francis Moreau
On Fri, Sep 27, 2013 at 11:14 AM, John Keeping  wrote:
> On Fri, Sep 27, 2013 at 10:28:05AM +0200, Francis Moreau wrote:
>> On Fri, Sep 27, 2013 at 10:11 AM, John Keeping  wrote:
>> > On Fri, Sep 27, 2013 at 07:09:03AM +0200, Francis Moreau wrote:
>> >> Hi,
>> >>
>> >> On Thu, Sep 26, 2013 at 10:21 PM, John Keeping  wrote:
>> >> > On Thu, Sep 26, 2013 at 06:35:57PM +0200, Francis Moreau wrote:
>> >> >> I'm trying to use "git log --cherry ..." in order to display new, kept
>> >> >> and removed commits between two branches A and B.
>> >> >>
>> >> >> So commits which are only in B are considered new and should be marked
>> >> >> with '+'. Commits which are in both branches are marked with '=' but
>> >> >> only commit in branch B are shown. Eventually commits which are in A
>> >> >> but not in B anymore should be marked with '-'.
>> >> >>
>> >> >> So far I found this solution:
>> >> >>
>> >> >>   $ git log --cherry-mark --right-only A...B
>> >> >>   $ git log --cherry-pick  --left-only   A...B
>> >> >>
>> >> >> but I have to call twice git-log. This can be annoying since depending
>> >> >> on A and B, calling git-log can take time.
>> >> >>
>> >> >> Is there another option that I'm missing which would do the job but
>> >> >> with only one call to git-log ?
>> >> >
>> >> > Does this do what you want?
>> >> >
>> >> > git log --cherry-mark --left-right A...B |
>> >> > sed -e '/^commit / {
>> >> > y/<>/-+/
>> >> > }'
>> >>
>> >> Nope because --left-right shows common commits (with '=' mark) that
>> >> belong to A *and* B, and I'd like to have only the ones in B.
>> >
>> > I think the only way you can address this is to post-process the result,
>> > I don't know any way to remove a left side commit only if it is
>> > patch-identical to a right side commit.
>> >
>> > It should be relatively easy to filter out any '=' commits that are in
>> > the output of "git rev-list --left-only A...B".
>>
>> yes that's what I'm doing but I was wondering if that's possible to do
>> that with only one run of git-log/git-rev-list.
>
> I don't think it is.  But you only need to use the --cherry-mark option
> with one of the runs, so the other one should be very quick - the added
> work of calculating patch IDs slows down "git log" a lot.

That's true, rev-list should be way faster. I think I'll do that.

Thanks.
-- 
Francis
--
To unsubscribe from this list: send the line "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: Question about "git log --cherry"

2013-09-27 Thread John Keeping
On Fri, Sep 27, 2013 at 10:28:05AM +0200, Francis Moreau wrote:
> On Fri, Sep 27, 2013 at 10:11 AM, John Keeping  wrote:
> > On Fri, Sep 27, 2013 at 07:09:03AM +0200, Francis Moreau wrote:
> >> Hi,
> >>
> >> On Thu, Sep 26, 2013 at 10:21 PM, John Keeping  wrote:
> >> > On Thu, Sep 26, 2013 at 06:35:57PM +0200, Francis Moreau wrote:
> >> >> I'm trying to use "git log --cherry ..." in order to display new, kept
> >> >> and removed commits between two branches A and B.
> >> >>
> >> >> So commits which are only in B are considered new and should be marked
> >> >> with '+'. Commits which are in both branches are marked with '=' but
> >> >> only commit in branch B are shown. Eventually commits which are in A
> >> >> but not in B anymore should be marked with '-'.
> >> >>
> >> >> So far I found this solution:
> >> >>
> >> >>   $ git log --cherry-mark --right-only A...B
> >> >>   $ git log --cherry-pick  --left-only   A...B
> >> >>
> >> >> but I have to call twice git-log. This can be annoying since depending
> >> >> on A and B, calling git-log can take time.
> >> >>
> >> >> Is there another option that I'm missing which would do the job but
> >> >> with only one call to git-log ?
> >> >
> >> > Does this do what you want?
> >> >
> >> > git log --cherry-mark --left-right A...B |
> >> > sed -e '/^commit / {
> >> > y/<>/-+/
> >> > }'
> >>
> >> Nope because --left-right shows common commits (with '=' mark) that
> >> belong to A *and* B, and I'd like to have only the ones in B.
> >
> > I think the only way you can address this is to post-process the result,
> > I don't know any way to remove a left side commit only if it is
> > patch-identical to a right side commit.
> >
> > It should be relatively easy to filter out any '=' commits that are in
> > the output of "git rev-list --left-only A...B".
> 
> yes that's what I'm doing but I was wondering if that's possible to do
> that with only one run of git-log/git-rev-list.

I don't think it is.  But you only need to use the --cherry-mark option
with one of the runs, so the other one should be very quick - the added
work of calculating patch IDs slows down "git log" a lot.
--
To unsubscribe from this list: send the line "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: [hostname:port]:repo.git notation no longer works (for ssh)

2013-09-27 Thread Stefan Näwe
Am 27.09.2013 10:07, schrieb Morten Stenshorne:
> I've just upgraded to Debian testing (jessie), and with that I got a
> brand new (for me) git version:
> 
> $ git --version
> git version 1.8.4.rc3
> 
> Some of my repos I use an ssh tunnel to reach, so when I want to reach a
> repo forwarded to local port 2223, using the ssh protocol, the following
> used to work (.git/config) in older git versions:
> 
> [remote "exp"]
> url = [localhost:2223]:blink.git
> fetch = +refs/heads/*:refs/remotes/exp/*
> 
> However, now I get this message:
> 
> $ git fetch exp
> fatal: ':blink.git' does not appear to be a git repository
> fatal: Could not read from remote repository.

I wonder why that worked (especially the "[...]") at all ?
I thought specifying a port for a SSH connection was always only
possible when using

   ssh://user@host:port/path/to/repo.git
- or -
   ssh://user@host:port/~user/path/to/repo.git

At least that's what I always read out of the git-clone man page.

Stefan
-- 

/dev/random says: Don't ask me, I have random access memory.
python -c "print 
'73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')"
--
To unsubscribe from this list: send the line "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 submodule foreach: Skip eval for more than one argument

2013-09-27 Thread Johan Herland
On Thu, Sep 26, 2013 at 10:10 PM, Anders Kaseorg  wrote:
> ‘eval "$@"’ created an extra layer of shell interpretation, which was
> probably not expected by a user who passed multiple arguments to git
> submodule foreach:
>
> $ git grep "'"
> [searches for single quotes]
> $ git submodule foreach git grep "'"
> Entering '[submodule]'
> /usr/lib/git-core/git-submodule: 1: eval: Syntax error: Unterminated quoted 
> string
> Stopping at '[submodule]'; script returned non-zero status.
>
> To fix this, if the user passed more than one argument, just execute
> "$@" directly instead of passing it to eval.
>
> Signed-off-by: Anders Kaseorg 

The change looks good, and the existing tests (in t7407) pass. :-)

Two comments, however:

1. Please add the use case you mention above as a new test case, so
that we can easily catch future regressions.

2. If we are unlucky there might be existing users that work around
the existing behavior by adding an extra level of quoting (i.e. doing
the equivalent of git submodule foreach git grep "\'" in your example
above). Will their workaround break as a result of your change? Is
that acceptable?


Have fun! :)

...Johan

> ---
>  git-submodule.sh | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index c17bef1..3381864 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -545,7 +545,12 @@ cmd_foreach()
> sm_path=$(relative_path "$sm_path") &&
> # we make $path available to scripts ...
> path=$sm_path &&
> -   eval "$@" &&
> +   if [ $# -eq 1 ]
> +   then
> +   eval "$1"
> +   else
> +   "$@"
> +   fi &&
> if test -n "$recursive"
> then
> cmd_foreach "--recursive" "$@"
> --
> 1.8.4
>

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


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

2013-09-27 Thread Duy Nguyen
On Fri, Sep 27, 2013 at 3:07 PM, Morten Stenshorne  wrote:
> I've just upgraded to Debian testing (jessie), and with that I got a
> brand new (for me) git version:
>
> $ git --version
> git version 1.8.4.rc3
>
> Some of my repos I use an ssh tunnel to reach, so when I want to reach a
> repo forwarded to local port 2223, using the ssh protocol, the following
> used to work (.git/config) in older git versions:
>
> [remote "exp"]
> url = [localhost:2223]:blink.git
> fetch = +refs/heads/*:refs/remotes/exp/*
>
> However, now I get this message:
>
> $ git fetch exp
> fatal: ':blink.git' does not appear to be a git repository
> fatal: Could not read from remote repository.

Ugh.. bisect pointed to my commit 6000334 (clone: allow cloning local
paths with colons in them - 2013-05-04). Will have a closer look
tonight.
-- 
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: Question about "git log --cherry"

2013-09-27 Thread Francis Moreau
On Fri, Sep 27, 2013 at 10:11 AM, John Keeping  wrote:
> On Fri, Sep 27, 2013 at 07:09:03AM +0200, Francis Moreau wrote:
>> Hi,
>>
>> On Thu, Sep 26, 2013 at 10:21 PM, John Keeping  wrote:
>> > On Thu, Sep 26, 2013 at 06:35:57PM +0200, Francis Moreau wrote:
>> >> I'm trying to use "git log --cherry ..." in order to display new, kept
>> >> and removed commits between two branches A and B.
>> >>
>> >> So commits which are only in B are considered new and should be marked
>> >> with '+'. Commits which are in both branches are marked with '=' but
>> >> only commit in branch B are shown. Eventually commits which are in A
>> >> but not in B anymore should be marked with '-'.
>> >>
>> >> So far I found this solution:
>> >>
>> >>   $ git log --cherry-mark --right-only A...B
>> >>   $ git log --cherry-pick  --left-only   A...B
>> >>
>> >> but I have to call twice git-log. This can be annoying since depending
>> >> on A and B, calling git-log can take time.
>> >>
>> >> Is there another option that I'm missing which would do the job but
>> >> with only one call to git-log ?
>> >
>> > Does this do what you want?
>> >
>> > git log --cherry-mark --left-right A...B |
>> > sed -e '/^commit / {
>> > y/<>/-+/
>> > }'
>>
>> Nope because --left-right shows common commits (with '=' mark) that
>> belong to A *and* B, and I'd like to have only the ones in B.
>
> I think the only way you can address this is to post-process the result,
> I don't know any way to remove a left side commit only if it is
> patch-identical to a right side commit.
>
> It should be relatively easy to filter out any '=' commits that are in
> the output of "git rev-list --left-only A...B".

yes that's what I'm doing but I was wondering if that's possible to do
that with only one run of git-log/git-rev-list.

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


[PATCH v2] RelNotes/1.8.5: direct script writers to "git status --porcelain"

2013-09-27 Thread Matthieu Moy
Signed-off-by: Matthieu Moy 
---
Jakub Narebski  writes:

> Perhaps "to use instead ..." would be easier to understand than
> proposed "to use ..., instead." (with "..." being one line long).

Actually, I had the version below staged, but forgot to "commit
--amend" before sending. Should be clear enough.

 Documentation/RelNotes/1.8.5.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/RelNotes/1.8.5.txt b/Documentation/RelNotes/1.8.5.txt
index ac5c3fa..e295266 100644
--- a/Documentation/RelNotes/1.8.5.txt
+++ b/Documentation/RelNotes/1.8.5.txt
@@ -96,6 +96,9 @@ UI, Workflows & Features
 
  * "git status" now omits the prefix to make its output a comment in a
commit log editor, which is not necessary for human consumption.
+   Scripts that parse the output of "git status" are advised to use
+   "git status --porcelain" instead. Its format is both easier to
+   parse and stable.
 
  * Make "foo^{tag}" to peel a tag to itself, i.e. no-op., and fail if
"foo" is not a tag.  "git rev-parse --verify v1.0^{tag}" would be
-- 
1.8.4.474.g128a96c

--
To unsubscribe from this list: send the line "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: [hostname:port]:repo.git notation no longer works (for ssh)

2013-09-27 Thread Morten Stenshorne
I've just upgraded to Debian testing (jessie), and with that I got a
brand new (for me) git version:

$ git --version
git version 1.8.4.rc3

Some of my repos I use an ssh tunnel to reach, so when I want to reach a
repo forwarded to local port 2223, using the ssh protocol, the following
used to work (.git/config) in older git versions:

[remote "exp"]
url = [localhost:2223]:blink.git
fetch = +refs/heads/*:refs/remotes/exp/*

However, now I get this message:

$ git fetch exp
fatal: ':blink.git' does not appear to be a git repository
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

If I don't go via the ssh tunnel (I finally have some VPN stuff these
days, so I don't really need the tunnel thing anymore, but that's going
to be a lot of remotes to update, so I'd prefer it just worked like it
used to):

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

... it works fine.

-- 
 Morten Stenshorne, developer, Opera Software ASA 
-- http://www.opera.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: Question about "git log --cherry"

2013-09-27 Thread John Keeping
On Fri, Sep 27, 2013 at 07:09:03AM +0200, Francis Moreau wrote:
> Hi,
> 
> On Thu, Sep 26, 2013 at 10:21 PM, John Keeping  wrote:
> > On Thu, Sep 26, 2013 at 06:35:57PM +0200, Francis Moreau wrote:
> >> I'm trying to use "git log --cherry ..." in order to display new, kept
> >> and removed commits between two branches A and B.
> >>
> >> So commits which are only in B are considered new and should be marked
> >> with '+'. Commits which are in both branches are marked with '=' but
> >> only commit in branch B are shown. Eventually commits which are in A
> >> but not in B anymore should be marked with '-'.
> >>
> >> So far I found this solution:
> >>
> >>   $ git log --cherry-mark --right-only A...B
> >>   $ git log --cherry-pick  --left-only   A...B
> >>
> >> but I have to call twice git-log. This can be annoying since depending
> >> on A and B, calling git-log can take time.
> >>
> >> Is there another option that I'm missing which would do the job but
> >> with only one call to git-log ?
> >
> > Does this do what you want?
> >
> > git log --cherry-mark --left-right A...B |
> > sed -e '/^commit / {
> > y/<>/-+/
> > }'
> 
> Nope because --left-right shows common commits (with '=' mark) that
> belong to A *and* B, and I'd like to have only the ones in B.

I think the only way you can address this is to post-process the result,
I don't know any way to remove a left side commit only if it is
patch-identical to a right side commit.

It should be relatively easy to filter out any '=' commits that are in
the output of "git rev-list --left-only A...B".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html