Re: No dialog box appears

2015-08-19 Thread Johannes Schindelin
Hi Aleksey,

On 2015-08-19 09:33, Цапков Алексей wrote:

 When installing the Git is not a dialog box appears with a choice ssh.
 What could be the reason?

I assume that you are referring to the Git for Windows installer (please state 
such details in the future). And I assume you were wondering why you could not 
choose putty as your ssh program. If that is the case, I think you might need 
to add connections to your putty first, otherwise the installer will not even 
present the choice to you.

But maybe you want to move this to the issue tracker at 
https://github.com/git-for-windows/git/issues?

Ciao,
Johannes
--
To unsubscribe from this list: send the line 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] untracked-cache: fix subdirectory handling

2015-08-19 Thread Duy Nguyen
On Sun, Aug 16, 2015 at 7:16 PM, Duy Nguyen pclo...@gmail.com wrote:
 On Sun, Aug 16, 2015 at 12:17 PM, David Turner dtur...@twopensource.com 
 wrote:
 Previously, some calls lookup_untracked would pass a full path.  But
 lookup_untracked assumes that the portion of the path up to and
 including to the untracked_cache_dir has been removed.  So
 lookup_untracked would be looking in the untracked_cache for 'foo' for
 'foo/bar' (instead of just looking for 'bar').  This would cause
 untracked cache corruption.

 Instead, treat_directory learns to track the base length of the parent
 directory, so that only the last path component is passed to
 lookup_untracked.

 Your v2 also fixes untracked_cache_invalidate_path(), which is not
 included here. Maybe it's in another patch?

No I was wrong. Your changes and the original code are effectively the
same (I misread strrchr as strchr). But I think there's a bug
somewhere as I'm writing tests to understand that code..
-- 
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 v4 1/3] t7063: use --force-untracked-cache to speed up a bit

2015-08-19 Thread Nguyễn Thái Ngọc Duy
When in the middle of t7063, we are sure untracked cache is supported,
so we can use --force-untracked-cache to skip the support detection
phase and save a few seconds. It's also good that --force-untracked-cache
is exercised in the test suite.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 t/t7063-status-untracked-cache.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t7063-status-untracked-cache.sh 
b/t/t7063-status-untracked-cache.sh
index ff23f4e..744e922 100755
--- a/t/t7063-status-untracked-cache.sh
+++ b/t/t7063-status-untracked-cache.sh
@@ -402,7 +402,7 @@ test_expect_success 'set up sparse checkout' '
echo done/[a-z]* .git/info/sparse-checkout 
test_config core.sparsecheckout true 
git checkout master 
-   git update-index --untracked-cache 
+   git update-index --force-untracked-cache 
git status --porcelain /dev/null  # prime the cache
test_path_is_missing done/.gitignore 
test_path_is_file done/one
-- 
2.3.0.rc1.137.g477eb31

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


[PATCH v4 0/3] dt/untracked-subdir

2015-08-19 Thread Nguyễn Thái Ngọc Duy
The first patch is a split from David's v3 patch because it should
belong to dt/untracked-sparse. The second is basically David's v3.
The third patch fixes untracked_cache_invalidate_path(). David fixed
it in v2, but there's another bug in this code.

David Turner (1):
  untracked-cache: fix subdirectory handling

Nguyễn Thái Ngọc Duy (2):
  t7063: use --force-untracked-cache to speed up a bit
  untracked cache: fix entry invalidation

 dir.c |  82 +++---
 t/t7063-status-untracked-cache.sh | 102 --
 2 files changed, 163 insertions(+), 21 deletions(-)

-- 
2.3.0.rc1.137.g477eb31

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


[PATCH v4 2/3] untracked-cache: fix subdirectory handling

2015-08-19 Thread Nguyễn Thái Ngọc Duy
From: David Turner dtur...@twopensource.com

Previously, some calls lookup_untracked would pass a full path.  But
lookup_untracked assumes that the portion of the path up to and
including to the untracked_cache_dir has been removed.  So
lookup_untracked would be looking in the untracked_cache for 'foo' for
'foo/bar' (instead of just looking for 'bar').  This would cause
untracked cache corruption.

Instead, treat_directory learns to track the base length of the parent
directory, so that only the last path component is passed to
lookup_untracked.

Helped-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
Signed-off-by: David Turner dtur...@twopensource.com
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 dir.c | 14 
 t/t7063-status-untracked-cache.sh | 72 ++-
 2 files changed, 79 insertions(+), 7 deletions(-)

diff --git a/dir.c b/dir.c
index e7b89fe..cd4ac77 100644
--- a/dir.c
+++ b/dir.c
@@ -1297,7 +1297,7 @@ static enum exist_status directory_exists_in_index(const 
char *dirname, int len)
  */
 static enum path_treatment treat_directory(struct dir_struct *dir,
struct untracked_cache_dir *untracked,
-   const char *dirname, int len, int exclude,
+   const char *dirname, int len, int baselen, int exclude,
const struct path_simplify *simplify)
 {
/* The len-1 is to strip the final '/' */
@@ -1324,7 +1324,8 @@ static enum path_treatment treat_directory(struct 
dir_struct *dir,
if (!(dir-flags  DIR_HIDE_EMPTY_DIRECTORIES))
return exclude ? path_excluded : path_untracked;
 
-   untracked = lookup_untracked(dir-untracked, untracked, dirname, len);
+   untracked = lookup_untracked(dir-untracked, untracked,
+dirname + baselen, len - baselen);
return read_directory_recursive(dir, dirname, len,
untracked, 1, simplify);
 }
@@ -1444,6 +1445,7 @@ static int get_dtype(struct dirent *de, const char *path, 
int len)
 static enum path_treatment treat_one_path(struct dir_struct *dir,
  struct untracked_cache_dir *untracked,
  struct strbuf *path,
+ int baselen,
  const struct path_simplify *simplify,
  int dtype, struct dirent *de)
 {
@@ -1495,8 +1497,8 @@ static enum path_treatment treat_one_path(struct 
dir_struct *dir,
return path_none;
case DT_DIR:
strbuf_addch(path, '/');
-   return treat_directory(dir, untracked, path-buf, path-len, 
exclude,
-   simplify);
+   return treat_directory(dir, untracked, path-buf, path-len,
+  baselen, exclude, simplify);
case DT_REG:
case DT_LNK:
return exclude ? path_excluded : path_untracked;
@@ -1557,7 +1559,7 @@ static enum path_treatment treat_path(struct dir_struct 
*dir,
return path_none;
 
dtype = DTYPE(de);
-   return treat_one_path(dir, untracked, path, simplify, dtype, de);
+   return treat_one_path(dir, untracked, path, baselen, simplify, dtype, 
de);
 }
 
 static void add_untracked(struct untracked_cache_dir *dir, const char *name)
@@ -1827,7 +1829,7 @@ static int treat_leading_path(struct dir_struct *dir,
break;
if (simplify_away(sb.buf, sb.len, simplify))
break;
-   if (treat_one_path(dir, NULL, sb, simplify,
+   if (treat_one_path(dir, NULL, sb, baselen, simplify,
   DT_DIR, NULL) == path_none)
break; /* do not recurse into it */
if (len = baselen) {
diff --git a/t/t7063-status-untracked-cache.sh 
b/t/t7063-status-untracked-cache.sh
index 744e922..22393b9 100755
--- a/t/t7063-status-untracked-cache.sh
+++ b/t/t7063-status-untracked-cache.sh
@@ -408,7 +408,8 @@ test_expect_success 'set up sparse checkout' '
test_path_is_file done/one
 '
 
-test_expect_success 'create files, some of which are gitignored' '
+test_expect_success 'create/modify files, some of which are gitignored' '
+   echo two bis done/two 
echo three done/three  # three is gitignored
echo four done/four  # four is gitignored at a higher level
echo five done/five # five is not gitignored
@@ -420,6 +421,7 @@ test_expect_success 'test sparse status with untracked 
cache' '
GIT_TRACE_UNTRACKED_STATS=$TRASH_DIRECTORY/trace \
git status --porcelain ../status.actual 
cat ../status.expect EOF 
+ M done/two
 ?? .gitignore
 ?? done/five
 ?? dtwo/
@@ -459,6 +461,7 @@ test_expect_success 'test sparse status again with 
untracked cache' '
GIT_TRACE_UNTRACKED_STATS=$TRASH_DIRECTORY/trace \
git 

[PATCH v4 3/3] untracked cache: fix entry invalidation

2015-08-19 Thread Nguyễn Thái Ngọc Duy
First, the current code in untracked_cache_invalidate_path() is wrong
because it can only handle paths a or a/b, not a/b/c because
lookup_untracked() only looks for entries directly under the given
directory. In the last case, it will look for the entry b/c in
directory a instead. This means if you delete or add an entry in a
subdirectory, untracked cache may become out of date because it does not
invalidate properly. This is noticed by David Turner.

The second problem is about invalidation inside a fully untracked/excluded
directory. In this case we may have to invalidate back to root. See the
comment block for detail.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 dir.c | 68 ---
 t/t7063-status-untracked-cache.sh | 28 +++-
 2 files changed, 83 insertions(+), 13 deletions(-)

diff --git a/dir.c b/dir.c
index cd4ac77..c1edabf 100644
--- a/dir.c
+++ b/dir.c
@@ -2616,23 +2616,67 @@ done2:
return uc;
 }
 
+static void invalidate_one_directory(struct untracked_cache *uc,
+struct untracked_cache_dir *ucd)
+{
+   uc-dir_invalidated++;
+   ucd-valid = 0;
+   ucd-untracked_nr = 0;
+}
+
+/*
+ * Normally when an entry is added or removed from a directory,
+ * invalidating that directory is enough. No need to touch its
+ * ancestors. When a directory is shown as foo/bar/ in git-status
+ * however, deleting or adding an entry may have cascading effect.
+ *
+ * Say the foo/bar/file has become untracked, we need to tell the
+ * untracked_cache_dir of foo that bar/ is not an untracked
+ * directory any more (because bar is managed by foo as an untracked
+ * file).
+ *
+ * Similarly, if foo/bar/file moves from untracked to tracked and it
+ * was the last untracked entry in the entire foo, we should show
+ * foo/ instead. Which means we have to invalidate past bar up to
+ * foo.
+ *
+ * This function traverses all directories from root to leaf. If there
+ * is a chance of one of the above cases happening, we invalidate back
+ * to root. Otherwise we just invalidate the leaf. There may be a more
+ * sophisticated way than checking for SHOW_OTHER_DIRECTORIES to
+ * detect these cases and avoid unnecessary invalidation, for example,
+ * checking for the untracked entry named bar/ in foo, but for now
+ * stick to something safe and simple.
+ */
+static int invalidate_one_component(struct untracked_cache *uc,
+   struct untracked_cache_dir *dir,
+   const char *path, int len)
+{
+   const char *rest = strchr(path, '/');
+
+   if (rest) {
+   int component_len = rest - path;
+   struct untracked_cache_dir *d =
+   lookup_untracked(uc, dir, path, component_len);
+   int ret =
+   invalidate_one_component(uc, d, rest + 1,
+len - (component_len + 1));
+   if (ret)
+   invalidate_one_directory(uc, dir);
+   return ret;
+   }
+
+   invalidate_one_directory(uc, dir);
+   return uc-dir_flags  DIR_SHOW_OTHER_DIRECTORIES;
+}
+
 void untracked_cache_invalidate_path(struct index_state *istate,
 const char *path)
 {
-   const char *sep;
-   struct untracked_cache_dir *d;
if (!istate-untracked || !istate-untracked-root)
return;
-   sep = strrchr(path, '/');
-   if (sep)
-   d = lookup_untracked(istate-untracked,
-istate-untracked-root,
-path, sep - path);
-   else
-   d = istate-untracked-root;
-   istate-untracked-dir_invalidated++;
-   d-valid = 0;
-   d-untracked_nr = 0;
+   invalidate_one_component(istate-untracked, istate-untracked-root,
+path, strlen(path));
 }
 
 void untracked_cache_remove_from_index(struct index_state *istate,
diff --git a/t/t7063-status-untracked-cache.sh 
b/t/t7063-status-untracked-cache.sh
index 22393b9..37a24c1 100755
--- a/t/t7063-status-untracked-cache.sh
+++ b/t/t7063-status-untracked-cache.sh
@@ -375,7 +375,7 @@ EOF
 node creation: 0
 gitignore invalidation: 0
 directory invalidation: 0
-opendir: 1
+opendir: 2
 EOF
test_cmp ../trace.expect ../trace
 '
@@ -543,4 +543,30 @@ EOF
test_cmp ../trace.expect ../trace
 '
 
+test_expect_success 'move entry in subdir from untracked to cached' '
+   git add dtwo/two 
+   git status --porcelain ../status.actual 
+   cat ../status.expect EOF 
+ M done/two
+A  dtwo/two
+?? .gitignore
+?? done/five
+?? done/sub/
+EOF
+   test_cmp ../status.expect ../status.actual
+'
+
+test_expect_success 'move entry in subdir from cached to untracked' '
+   git rm --cached dtwo/two 
+   git status --porcelain ../status.actual 
+   cat 

Re: [PATCH v2 3/8] branch: roll show_detached HEAD into regular ref_list

2015-08-19 Thread Matthieu Moy
Karthik Nayak karthik@gmail.com writes:

 + /*
 +  * First we obtain all regular branch refs and if the HEAD is
 +  * detached then we insert that ref to the end of the ref_fist
 +  * so that it can be printed first.
 +  */

Nit: the end of the sentence reads funny (why put at the end so that it
can be printed first.).

Perhaps ... so that it can be printed and removed first..

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 7/9] builtin/send-pack.c: Use option parsing API

2015-08-19 Thread Dave Borowitz
The old option parsing code in this plumbing command predates this
API, so option parsing was done more manually. Using the new API
brings send-pack more in line with push, and accepts new variants
like --no-* for negating options.

Signed-off-by: Dave Borowitz dborow...@google.com
---
 builtin/send-pack.c | 163 +++-
 1 file changed, 59 insertions(+), 104 deletions(-)

diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 23b2962..5f2c744 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -12,10 +12,15 @@
 #include version.h
 #include sha1-array.h
 #include gpg-interface.h
+#include gettext.h
 
-static const char send_pack_usage[] =
-git send-pack [--all | --mirror] [--dry-run] [--force] 
[--receive-pack=git-receive-pack] [--verbose] [--thin] [--atomic] 
[host:]directory [ref...]\n
-  --all and explicit ref specification are mutually exclusive.;
+static const char * const send_pack_usage[] = {
+   N_(git send-pack [--all | --mirror] [--dry-run] [--force] 
+ [--receive-pack=git-receive-pack] [--verbose] [--thin] [--atomic] 
+ [host:]directory [ref...]\n
+   --all and explicit ref specification are mutually exclusive.),
+   NULL,
+};
 
 static struct send_pack_args args;
 
@@ -107,116 +112,66 @@ int cmd_send_pack(int argc, const char **argv, const 
char *prefix)
int ret;
int helper_status = 0;
int send_all = 0;
+   int verbose = 0;
const char *receivepack = git-receive-pack;
+   unsigned dry_run = 0;
+   unsigned send_mirror = 0;
+   unsigned force_update = 0;
+   unsigned quiet = 0;
+   unsigned push_cert = 0;
+   unsigned use_thin_pack = 0;
+   unsigned atomic = 0;
+   unsigned stateless_rpc = 0;
int flags;
unsigned int reject_reasons;
int progress = -1;
int from_stdin = 0;
struct push_cas_option cas = {0};
 
-   git_config(git_gpg_config, NULL);
+   struct option options[] = {
+   OPT__VERBOSITY(verbose),
+   OPT_STRING(0, receive-pack, receivepack, receive-pack, 
N_(receive pack program)),
+   OPT_STRING(0, exec, receivepack, receive-pack, N_(receive 
pack program)),
+   OPT_STRING(0, remote, remote_name, remote, N_(remote 
name)),
+   OPT_BOOL(0, all, send_all, N_(push all refs)),
+   OPT_BOOL('n' , dry-run, dry_run, N_(dry run)),
+   OPT_BOOL(0, mirror, send_mirror, N_(mirror all refs)),
+   OPT_BOOL('f', force, force_update, N_(force updates)),
+   OPT_BOOL(0, signed, push_cert, N_(GPG sign the push)),
+   OPT_BOOL(0, progress, progress, N_(force progress 
reporting)),
+   OPT_BOOL(0, thin, use_thin_pack, N_(use thin pack)),
+   OPT_BOOL(0, atomic, atomic, N_(request atomic transaction 
on remote side)),
+   OPT_BOOL(0, stateless-rpc, stateless_rpc, N_(use stateless 
RPC protocol)),
+   OPT_BOOL(0, stdin, from_stdin, N_(read refs from stdin)),
+   OPT_BOOL(0, helper-status, helper_status, N_(print status 
from remote helper)),
+   { OPTION_CALLBACK,
+ 0, CAS_OPT_NAME, cas, N_(refname:expect),
+ N_(require old value of ref to be at this value),
+ PARSE_OPT_OPTARG, parseopt_push_cas_option },
+   OPT_END()
+   };
 
-   argv++;
-   for (i = 1; i  argc; i++, argv++) {
-   const char *arg = *argv;
-
-   if (*arg == '-') {
-   if (starts_with(arg, --receive-pack=)) {
-   receivepack = arg + 15;
-   continue;
-   }
-   if (starts_with(arg, --exec=)) {
-   receivepack = arg + 7;
-   continue;
-   }
-   if (starts_with(arg, --remote=)) {
-   remote_name = arg + 9;
-   continue;
-   }
-   if (!strcmp(arg, --all)) {
-   send_all = 1;
-   continue;
-   }
-   if (!strcmp(arg, --dry-run)) {
-   args.dry_run = 1;
-   continue;
-   }
-   if (!strcmp(arg, --mirror)) {
-   args.send_mirror = 1;
-   continue;
-   }
-   if (!strcmp(arg, --force)) {
-   args.force_update = 1;
-   continue;
-   }
-   if (!strcmp(arg, --quiet)) {
-   args.quiet = 1;
-   continue;
-   }
-

[PATCH v2 5/9] transport: Remove git_transport_options.push_cert

2015-08-19 Thread Dave Borowitz
This field was set in transport_set_option, but never read in the push
code. The push code basically ignores the smart_options field
entirely, and derives its options from the flags arguments to the
push* callbacks. Note that in git_transport_push there are already
several args set from flags that have no corresponding field in
git_transport_options; after this change, push_cert is just like
those.

Signed-off-by: Dave Borowitz dborow...@google.com
---
 transport.c | 3 ---
 transport.h | 1 -
 2 files changed, 4 deletions(-)

diff --git a/transport.c b/transport.c
index 40692f8..3dd6e30 100644
--- a/transport.c
+++ b/transport.c
@@ -476,9 +476,6 @@ static int set_git_option(struct git_transport_options 
*opts,
die(transport: invalid depth option '%s', 
value);
}
return 0;
-   } else if (!strcmp(name, TRANS_OPT_PUSH_CERT)) {
-   opts-push_cert = !!value;
-   return 0;
}
return 1;
 }
diff --git a/transport.h b/transport.h
index 18d2cf8..79190df 100644
--- a/transport.h
+++ b/transport.h
@@ -12,7 +12,6 @@ struct git_transport_options {
unsigned check_self_contained_and_connected : 1;
unsigned self_contained_and_connected : 1;
unsigned update_shallow : 1;
-   unsigned push_cert : 1;
int depth;
const char *uploadpack;
const char *receivepack;
-- 
2.5.0.276.gf5e568e

--
To unsubscribe from this list: send the line 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 6/9] config.c: Expose git_parse_maybe_bool

2015-08-19 Thread Dave Borowitz
Signed-off-by: Dave Borowitz dborow...@google.com
---
 cache.h  | 1 +
 config.c | 6 +++---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/cache.h b/cache.h
index 6bb7119..95d9594 100644
--- a/cache.h
+++ b/cache.h
@@ -1392,6 +1392,7 @@ extern int git_config_with_options(config_fn_t fn, void *,
   int respect_includes);
 extern int git_config_early(config_fn_t fn, void *, const char *repo_config);
 extern int git_parse_ulong(const char *, unsigned long *);
+extern int git_parse_maybe_bool(const char *);
 extern int git_config_int(const char *, const char *);
 extern int64_t git_config_int64(const char *, const char *);
 extern unsigned long git_config_ulong(const char *, const char *);
diff --git a/config.c b/config.c
index 9fd275f..e5d7959 100644
--- a/config.c
+++ b/config.c
@@ -618,7 +618,7 @@ unsigned long git_config_ulong(const char *name, const char 
*value)
return ret;
 }
 
-static int git_config_maybe_bool_text(const char *name, const char *value)
+int git_parse_maybe_bool(const char *value)
 {
if (!value)
return 1;
@@ -637,7 +637,7 @@ static int git_config_maybe_bool_text(const char *name, 
const char *value)
 
 int git_config_maybe_bool(const char *name, const char *value)
 {
-   int v = git_config_maybe_bool_text(name, value);
+   int v = git_parse_maybe_bool(value);
if (0 = v)
return v;
if (git_parse_int(value, v))
@@ -647,7 +647,7 @@ int git_config_maybe_bool(const char *name, const char 
*value)
 
 int git_config_bool_or_int(const char *name, const char *value, int *is_bool)
 {
-   int v = git_config_maybe_bool_text(name, value);
+   int v = git_parse_maybe_bool(value);
if (0 = v) {
*is_bool = 1;
return v;
-- 
2.5.0.276.gf5e568e

--
To unsubscribe from this list: send the line 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/9] Documentation/git-send-pack.txt: Document --signed

2015-08-19 Thread Dave Borowitz
Signed-off-by: Dave Borowitz dborow...@google.com
---
 Documentation/git-send-pack.txt | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt
index 6a6..0a0a3fb 100644
--- a/Documentation/git-send-pack.txt
+++ b/Documentation/git-send-pack.txt
@@ -10,7 +10,8 @@ SYNOPSIS
 
 [verse]
 'git send-pack' [--all] [--dry-run] [--force] 
[--receive-pack=git-receive-pack]
-   [--verbose] [--thin] [--atomic] [host:]directory [ref...]
+   [--verbose] [--thin] [--atomic] [--signed]
+   [host:]directory [ref...]
 
 DESCRIPTION
 ---
@@ -68,6 +69,14 @@ be in a separate packet, and the list must end with a flush 
packet.
fails to update then the entire push will fail without changing any
refs.
 
+--signed::
+   GPG-sign the push request to update refs on the receiving
+   side, to allow it to be checked by the hooks and/or be
+   logged.  See linkgit:git-receive-pack[1] for the details
+   on the receiving end.  If the attempt to sign with `gpg` fails,
+   or if the server does not support signed pushes, the push will
+   fail.
+
 host::
A remote host to house the repository.  When this
part is specified, 'git-receive-pack' is invoked via
-- 
2.5.0.276.gf5e568e

--
To unsubscribe from this list: send the line 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 9/9] Add a config option push.gpgSign for default signed pushes

2015-08-19 Thread Dave Borowitz
Signed-off-by: Dave Borowitz dborow...@google.com
---
 Documentation/config.txt |  8 
 builtin/push.c   | 50 ++--
 builtin/send-pack.c  | 27 +-
 3 files changed, 70 insertions(+), 15 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 016f6e9..4ba0e4b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2178,6 +2178,14 @@ push.followTags::
may override this configuration at time of push by specifying
'--no-follow-tags'.
 
+push.gpgSign::
+   May be set to a boolean value, or the string 'if-asked'. A true
+   value causes all pushes to be GPG signed, as if '--signed' is
+   passed to linkgit:git-push[1]. The string 'if-asked' causes
+   pushes to be signed if the server supports it, as if
+   '--signed=if-asked' is passed to 'git push'. A false value may
+   override a value from a lower-priority config file. An explicit
+   command-line flag always overrides this config option.
 
 rebase.stat::
Whether to show a diffstat of what changed upstream since the last
diff --git a/builtin/push.c b/builtin/push.c
index 85a82cd..3bda430 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -472,6 +472,24 @@ static int option_parse_recurse_submodules(const struct 
option *opt,
return 0;
 }
 
+static void set_push_cert_flags(int *flags, int v)
+{
+   switch (v) {
+   case SEND_PACK_PUSH_CERT_NEVER:
+   *flags = ~(TRANSPORT_PUSH_CERT_ALWAYS | 
TRANSPORT_PUSH_CERT_IF_ASKED);
+   break;
+   case SEND_PACK_PUSH_CERT_ALWAYS:
+   *flags |= TRANSPORT_PUSH_CERT_ALWAYS;
+   *flags = ~TRANSPORT_PUSH_CERT_IF_ASKED;
+   break;
+   case SEND_PACK_PUSH_CERT_IF_ASKED:
+   *flags |= TRANSPORT_PUSH_CERT_IF_ASKED;
+   *flags = ~TRANSPORT_PUSH_CERT_ALWAYS;
+   break;
+   }
+}
+
+
 static int git_push_config(const char *k, const char *v, void *cb)
 {
int *flags = cb;
@@ -487,6 +505,23 @@ static int git_push_config(const char *k, const char *v, 
void *cb)
else
*flags = ~TRANSPORT_PUSH_FOLLOW_TAGS;
return 0;
+   } else if (!strcmp(k, push.gpgsign)) {
+   const char *value;
+   if (!git_config_get_value(push.gpgsign, value)) {
+   switch (git_config_maybe_bool(push.gpgsign, value)) {
+   case 0:
+   set_push_cert_flags(flags, 
SEND_PACK_PUSH_CERT_NEVER);
+   break;
+   case 1:
+   set_push_cert_flags(flags, 
SEND_PACK_PUSH_CERT_ALWAYS);
+   break;
+   default:
+   if (value  !strcasecmp(value, if-asked))
+   set_push_cert_flags(flags, 
SEND_PACK_PUSH_CERT_IF_ASKED);
+   else
+   return error(Invalid value for '%s', 
k);
+   }
+   }
}
 
return git_default_config(k, v, NULL);
@@ -538,6 +573,7 @@ int cmd_push(int argc, const char **argv, const char 
*prefix)
packet_trace_identity(push);
git_config(git_push_config, flags);
argc = parse_options(argc, argv, prefix, options, push_usage, 0);
+   set_push_cert_flags(flags, push_cert);
 
if (deleterefs  (tags || (flags  (TRANSPORT_PUSH_ALL | 
TRANSPORT_PUSH_MIRROR
die(_(--delete is incompatible with --all, --mirror and 
--tags));
@@ -552,20 +588,6 @@ int cmd_push(int argc, const char **argv, const char 
*prefix)
set_refspecs(argv + 1, argc - 1, repo);
}
 
-   switch (push_cert) {
-   case SEND_PACK_PUSH_CERT_NEVER:
-   flags = ~(TRANSPORT_PUSH_CERT_ALWAYS | 
TRANSPORT_PUSH_CERT_IF_ASKED);
-   break;
-   case SEND_PACK_PUSH_CERT_ALWAYS:
-   flags |= TRANSPORT_PUSH_CERT_ALWAYS;
-   flags = ~TRANSPORT_PUSH_CERT_IF_ASKED;
-   break;
-   case SEND_PACK_PUSH_CERT_IF_ASKED:
-   flags |= TRANSPORT_PUSH_CERT_IF_ASKED;
-   flags = ~TRANSPORT_PUSH_CERT_ALWAYS;
-   break;
-   }
-
rc = do_push(repo, flags);
if (rc == -1)
usage_with_options(push_usage, options);
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 0ce3bc8..f6e5d64 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -97,6 +97,31 @@ static void print_helper_status(struct ref *ref)
strbuf_release(buf);
 }
 
+static int send_pack_config(const char *k, const char *v, void *cb)
+{
+   git_gpg_config(k, v, NULL);
+
+   if (!strcmp(k, push.gpgsign)) {
+   const char *value;
+   if 

Re: [PATCH v12 03/13] ref-filter: introduce the ref_formatting_state stack machinery

2015-08-19 Thread Matthieu Moy
Karthik Nayak karthik@gmail.com writes:

 --- a/ref-filter.c
 +++ b/ref-filter.c
 @@ -155,6 +155,8 @@ static void push_new_state(struct ref_formatting_state 
 **stack)

 strbuf_init(s-output, 0);
 s-prev = *stack;
 +   if (*stack)
 +   s-quote_style = (*stack)-quote_style;
 *stack = s;
  }



 This seems about right, why do you think it's a stupid fix?

If you have a stack of N elemments, why replicate a field N times if all
the N instances always have the same value?

There's nothing to be pushed or poped with quote_style, so having it in
the stack is confusing to the reader (one has to infer the property all
instances have the same value by reading the code instead of having
just one variable), and error-prone for the author: you already got
it wrong once.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH 10/11] branch.c: use 'ref-filter' APIs

2015-08-19 Thread Karthik Nayak
On Wed, Aug 19, 2015 at 9:19 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
 Karthik Nayak karthik@gmail.com writes:

 On Thu, Jul 30, 2015 at 12:59 PM, Matthieu Moy
 matthieu@grenoble-inp.fr wrote:

 IIRC, historicaly Git allowed some weirdly named refs which made some
 commands ambiguous (e.g. a branch named after an option like '-d').
 We're forbidding their creation so people shouldn't have any, but we
 it's important to continue showing them in case some people have old
 bad-named branches lying around.
 [...]
 Agreed. But then again the warning tells about the broken ref, as in it's 
 name
 So I think It's ok?

 for e.g. t1430 :
 [trash directory.t1430-bad-ref-name] ../../git branch
 warning: ignoring ref with broken name refs/heads/broken...ref
 * master

 [ Late answer, I'm still catching up with my holiday's emails ;-) ]

 OK, the warning gives a different interface but it seems as good as the
 old one.


Ah! yes :)

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


Re: [PATCH 6/7] Support signing pushes iff the server supports it

2015-08-19 Thread Dave Borowitz
On Fri, Aug 14, 2015 at 7:22 PM, Junio C Hamano gits...@pobox.com wrote:
 Dave Borowitz dborow...@google.com writes:

 diff --git a/send-pack.c b/send-pack.c
 index 2a64fec..6ae9f45 100644
 --- a/send-pack.c
 +++ b/send-pack.c
 @@ -370,7 +370,7 @@ int send_pack(struct send_pack_args *args,
   args-use_thin_pack = 0;
   if (server_supports(atomic))
   atomic_supported = 1;
 - if (args-push_cert) {
 + if (args-push_cert == SEND_PACK_PUSH_CERT_ALWAYS) {
   int len;

   push_cert_nonce = server_feature_value(push-cert, len);
 @@ -379,6 +379,18 @@ int send_pack(struct send_pack_args *args,
   reject_invalid_nonce(push_cert_nonce, len);
   push_cert_nonce = xmemdupz(push_cert_nonce, len);
   }
 + if (args-push_cert == SEND_PACK_PUSH_CERT_IF_POSSIBLE) {
 + int len;
 +
 + push_cert_nonce = server_feature_value(push-cert, len);
 + if (push_cert_nonce) {
 + reject_invalid_nonce(push_cert_nonce, len);
 + push_cert_nonce = xmemdupz(push_cert_nonce, len);
 + } else
 + warning(_(not sending a push certificate since the
 +receiving end does not support --signed
 +push));
 + }

 I wonder if the bodies of these two if statements can be a bit
 better organized to avoid duplication (I suspect you have tried
 and you may already know that the above is the most readable
 version, but I haven't tried to do so myself, so...).

Found a slightly less repetitious way.
--
To unsubscribe from this list: send the line 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 v12 01/13] ref-filter: move `struct atom_value` to ref-filter.c

2015-08-19 Thread Karthik Nayak
On Wed, Aug 19, 2015 at 8:26 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
 Karthik Nayak karthik@gmail.com writes:

 +static void end_atom_handler(struct atom_value *atomv, struct 
 ref_formatting_state **state)
 +{
 + struct ref_formatting_state *current = *state;
 + if (!current-at_end)
 + die(_(format: `end` atom used without a supporting atom));

 You error out on %(end) without %(align), but not on %(align) without
 %(end).

 You should probably check that the stack is empty at the end and error
 out otherwise.


You're right,

if (state-prev)
die(_(format: `end` atom missing));

should do before printing.

-- 
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line 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 v12 10/13] tag.c: use 'ref-filter' data structures

2015-08-19 Thread Karthik Nayak
On Wed, Aug 19, 2015 at 8:26 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
 Karthik Nayak karthik@gmail.com writes:

 This is a temporary step before porting 'tag.c' to use 'ref-filter'
 completely. As this is a temporary step, most of the code
 introduced here will be removed when 'tag.c' is ported over to use
 'ref-filter' APIs

 If you resend: missing '.' at the end of sentence.


Ok will add.

 - if (lines != -1)
 + if (filter.lines != -1)
   die(_(-n option is only allowed with -l.));
 - if (with_commit)
 + if (filter.with_commit)
   die(_(--contains option is only allowed with -l.));
 - if (points_at.nr)
 + if (filter.points_at.nr)
   die(_(--points-at option is only allowed with -l.));

 It may make sense to factor these checks into a function like

   void check_filter_consistancy(struct ref_filter *filter)

 in ref-filter.c, since for-each-ref, branch and tag will eventually have
 the same set of constraints on the options.


Ah! this is needed only in branch and tag where we need to see if the options
are used with tag.c and both have a different sort of check for this.
Might need more of a cleanup in branch.c and tag.c before we could do something
like this so that we could have a similar check.

for reference branch.c uses:

if (!!delete + !!rename + !!new_upstream +
list + unset_upstream  1)
usage_with_options(builtin_branch_usage, options);

whereas tag.c uses what you stated above.

-- 
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line 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 v12 03/13] ref-filter: introduce the ref_formatting_state stack machinery

2015-08-19 Thread Matthieu Moy
Karthik Nayak karthik@gmail.com writes:

 +static void pop_state(struct ref_formatting_state **stack)
 +{
 + struct ref_formatting_state *current = *stack;
 + struct ref_formatting_state *prev = current-prev;
 +
 + if (prev)
 + strbuf_addbuf(prev-output, current-output);

I find this if (prev) suspicious: if there's a previous element in the
stack, push to it, but otherwise, you're throwing away the content of
the stack top silently.

Given the rest of the patch, this is correct, since you're using
state-output before pop_state(), but I find it weird to have the same
function to actually pop a state, and to destroy the last element.

Just thinking out loudly, I don't have specific alternative to propose
here.

 @@ -1262,23 +1284,24 @@ static void append_literal(const char *cp, const char 
 *ep, struct ref_formatting
  void show_ref_array_item(struct ref_array_item *info, const char *format, 
 int quote_style)
  {
   const char *cp, *sp, *ep;
 - struct ref_formatting_state state;
 + struct strbuf *final_buf;
 + struct ref_formatting_state *state = NULL;
  
 - strbuf_init(state.output, 0);
 - state.quote_style = quote_style;
 + push_new_state(state);
 + state-quote_style = quote_style;

I do not think that the quote_style should belong to the stack. At the
moment, only the bottom of the stack has it set, and as a result you're
getting weird results like:

$ ./git for-each-ref --shell --format '|%(align:80,left)%(author)%(end)|' | 
head -n 3 
|Junio C Hamano gits...@pobox.com 1435173702 -0700  
 ''|
|Junio C Hamano gits...@pobox.com 1435173701 -0700  
 ''|
|Junio C Hamano gits...@pobox.com 1433277352 -0700  
 ''|

See, the '' are inserted where the %(end) was, but not around atoms as
one would expect.

One stupid fix would be to propagate the quote_style accross the stack,
like this:

--- a/ref-filter.c
+++ b/ref-filter.c
@@ -155,6 +155,8 @@ static void push_new_state(struct ref_formatting_state 
**stack)
 
strbuf_init(s-output, 0);
s-prev = *stack;
+   if (*stack)
+   s-quote_style = (*stack)-quote_style;
*stack = s;
 }
 

After applying this, I do get the '' around the author (= correct
behavior I think), but then one wonders even more why this is part of
the stack.

You replaced the quote_style argument with ref_formatting_state, and I
think you should have kept this argument and added ref_formatting_state.
The other option is to add an extra indirection like

struct ref_formatting_state {
int quote_style;
struct ref_formatting_stack *stack;
}

(ref_formatting_stack would be what you currently call
ref_formatting_state). But that's probably overkill.

Also, after applying my toy patch above, I get useless '' around
%(align) and %(end). I can get rid of them with

--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1499,7 +1501,8 @@ void show_ref_array_item(struct ref_array_item *info, 
const char *format,
get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), 
atomv);
if (atomv-handler)
atomv-handler(atomv, state);
-   append_atom(atomv, state);
+   else
+   append_atom(atomv, state);
}
if (*cp) {
sp = cp + strlen(cp);

Unless I missed something, this second patch is sensible anyway and
should be squashed into [PATCH v12 05/13]: you don't need to call
append_atom() when you have a handler, right?

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH 10/11] branch.c: use 'ref-filter' APIs

2015-08-19 Thread Matthieu Moy
Karthik Nayak karthik@gmail.com writes:

 On Thu, Jul 30, 2015 at 12:59 PM, Matthieu Moy
 matthieu@grenoble-inp.fr wrote:

 IIRC, historicaly Git allowed some weirdly named refs which made some
 commands ambiguous (e.g. a branch named after an option like '-d').
 We're forbidding their creation so people shouldn't have any, but we
 it's important to continue showing them in case some people have old
 bad-named branches lying around.
[...]
 Agreed. But then again the warning tells about the broken ref, as in it's name
 So I think It's ok?

 for e.g. t1430 :
 [trash directory.t1430-bad-ref-name] ../../git branch
 warning: ignoring ref with broken name refs/heads/broken...ref
 * master

[ Late answer, I'm still catching up with my holiday's emails ;-) ]

OK, the warning gives a different interface but it seems as good as the
old one.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v12 03/13] ref-filter: introduce the ref_formatting_state stack machinery

2015-08-19 Thread Karthik Nayak
On Wed, Aug 19, 2015 at 8:26 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
 Karthik Nayak karthik@gmail.com writes:

 +static void pop_state(struct ref_formatting_state **stack)
 +{
 + struct ref_formatting_state *current = *stack;
 + struct ref_formatting_state *prev = current-prev;
 +
 + if (prev)
 + strbuf_addbuf(prev-output, current-output);

 I find this if (prev) suspicious: if there's a previous element in the
 stack, push to it, but otherwise, you're throwing away the content of
 the stack top silently.

 Given the rest of the patch, this is correct, since you're using
 state-output before pop_state(), but I find it weird to have the same
 function to actually pop a state, and to destroy the last element.

 Just thinking out loudly, I don't have specific alternative to propose
 here.


Hmm, but destroying the last element is also pop'ing it off the stack in a way.
I can't think of a something else.

 @@ -1262,23 +1284,24 @@ static void append_literal(const char *cp, const 
 char *ep, struct ref_formatting
  void show_ref_array_item(struct ref_array_item *info, const char *format, 
 int quote_style)
  {
   const char *cp, *sp, *ep;
 - struct ref_formatting_state state;
 + struct strbuf *final_buf;
 + struct ref_formatting_state *state = NULL;

 - strbuf_init(state.output, 0);
 - state.quote_style = quote_style;
 + push_new_state(state);
 + state-quote_style = quote_style;

 I do not think that the quote_style should belong to the stack. At the
 moment, only the bottom of the stack has it set, and as a result you're
 getting weird results like:

 $ ./git for-each-ref --shell --format '|%(align:80,left)%(author)%(end)|' | 
 head -n 3
 |Junio C Hamano gits...@pobox.com 1435173702 -0700
''|
 |Junio C Hamano gits...@pobox.com 1435173701 -0700
''|
 |Junio C Hamano gits...@pobox.com 1433277352 -0700
''|

 See, the '' are inserted where the %(end) was, but not around atoms as
 one would expect.

 One stupid fix would be to propagate the quote_style accross the stack,
 like this:

 --- a/ref-filter.c
 +++ b/ref-filter.c
 @@ -155,6 +155,8 @@ static void push_new_state(struct ref_formatting_state 
 **stack)

 strbuf_init(s-output, 0);
 s-prev = *stack;
 +   if (*stack)
 +   s-quote_style = (*stack)-quote_style;
 *stack = s;
  }



This seems about right, why do you think it's a stupid fix?

 After applying this, I do get the '' around the author (= correct
 behavior I think), but then one wonders even more why this is part of
 the stack.

 You replaced the quote_style argument with ref_formatting_state, and I
 think you should have kept this argument and added ref_formatting_state.
 The other option is to add an extra indirection like

 struct ref_formatting_state {
 int quote_style;
 struct ref_formatting_stack *stack;
 }

 (ref_formatting_stack would be what you currently call
 ref_formatting_state). But that's probably overkill.


Yes, seems like an overkill.

 Also, after applying my toy patch above, I get useless '' around
 %(align) and %(end). I can get rid of them with

 --- a/ref-filter.c
 +++ b/ref-filter.c
 @@ -1499,7 +1501,8 @@ void show_ref_array_item(struct ref_array_item *info, 
 const char *format,
 get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), 
 atomv);
 if (atomv-handler)
 atomv-handler(atomv, state);
 -   append_atom(atomv, state);
 +   else
 +   append_atom(atomv, state);
 }
 if (*cp) {
 sp = cp + strlen(cp);

 Unless I missed something, this second patch is sensible anyway and
 should be squashed into [PATCH v12 05/13]: you don't need to call
 append_atom() when you have a handler, right?

Yes, this I'll squash into 05/13.

-- 
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line 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 5/8] branch: drop non-commit error reporting

2015-08-19 Thread Karthik Nayak
On Wed, Aug 19, 2015 at 8:53 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
 Karthik Nayak karthik@gmail.com writes:

 Remove the error reporting variable to make the code easier to port
 over to using ref-filter APIs. This variable is not require as in

 s/is not require/d/


Will change.



-- 
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line 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 5/8] branch: drop non-commit error reporting

2015-08-19 Thread Matthieu Moy
Karthik Nayak karthik@gmail.com writes:

 Remove the error reporting variable to make the code easier to port
 over to using ref-filter APIs. This variable is not require as in

s/is not require/d/

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 3/8] branch: roll show_detached HEAD into regular ref_list

2015-08-19 Thread Karthik Nayak
On Wed, Aug 19, 2015 at 8:48 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
 Karthik Nayak karthik@gmail.com writes:

 + /*
 +  * First we obtain all regular branch refs and if the HEAD is
 +  * detached then we insert that ref to the end of the ref_fist
 +  * so that it can be printed first.
 +  */

 Nit: the end of the sentence reads funny (why put at the end so that it
 can be printed first.).

 Perhaps ... so that it can be printed and removed first..


Will change, thanks :)

-- 
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line 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 v12 01/13] ref-filter: move `struct atom_value` to ref-filter.c

2015-08-19 Thread Matthieu Moy
Karthik Nayak karthik@gmail.com writes:

 +static void end_atom_handler(struct atom_value *atomv, struct 
 ref_formatting_state **state)
 +{
 + struct ref_formatting_state *current = *state;
 + if (!current-at_end)
 + die(_(format: `end` atom used without a supporting atom));

You error out on %(end) without %(align), but not on %(align) without
%(end).

You should probably check that the stack is empty at the end and error
out otherwise.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v12 10/13] tag.c: use 'ref-filter' data structures

2015-08-19 Thread Matthieu Moy
Karthik Nayak karthik@gmail.com writes:

 This is a temporary step before porting 'tag.c' to use 'ref-filter'
 completely. As this is a temporary step, most of the code
 introduced here will be removed when 'tag.c' is ported over to use
 'ref-filter' APIs

If you resend: missing '.' at the end of sentence.

 - if (lines != -1)
 + if (filter.lines != -1)
   die(_(-n option is only allowed with -l.));
 - if (with_commit)
 + if (filter.with_commit)
   die(_(--contains option is only allowed with -l.));
 - if (points_at.nr)
 + if (filter.points_at.nr)
   die(_(--points-at option is only allowed with -l.));

It may make sense to factor these checks into a function like

  void check_filter_consistancy(struct ref_filter *filter)

in ref-filter.c, since for-each-ref, branch and tag will eventually have
the same set of constraints on the options.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 8/9] Support signing pushes iff the server supports it

2015-08-19 Thread Dave Borowitz
Add a new flag --signed-if-possible to push and send-pack that sends a
push certificate if and only if the server advertised a push cert
nonce. If not, at least warn the user that their push may not be as
secure as they thought.

Signed-off-by: Dave Borowitz dborow...@google.com
---
 Documentation/git-push.txt  | 17 +---
 Documentation/git-send-pack.txt | 16 +--
 builtin/push.c  | 20 ++-
 builtin/send-pack.c |  6 --
 remote-curl.c   | 16 ++-
 send-pack.c | 43 ++---
 send-pack.h | 12 +++-
 transport-helper.c  | 34 
 transport.c |  8 +++-
 transport.h |  5 +++--
 10 files changed, 128 insertions(+), 49 deletions(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index da0a98d..1495e34 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -11,7 +11,8 @@ SYNOPSIS
 [verse]
 'git push' [--all | --mirror | --tags] [--follow-tags] [--atomic] [-n | 
--dry-run] [--receive-pack=git-receive-pack]
   [--repo=repository] [-f | --force] [--prune] [-v | --verbose]
-  [-u | --set-upstream] [--signed]
+  [-u | --set-upstream]
+  [--[no-]signed|--sign=(true|false|if-asked)]
   [--force-with-lease[=refname[:expect]]]
   [--no-verify] [repository [refspec...]]
 
@@ -132,14 +133,16 @@ already exists on the remote side.
with configuration variable 'push.followTags'.  For more
information, see 'push.followTags' in linkgit:git-config[1].
 
-
---signed::
+--[no-]signed::
+--sign=(true|false|if-asked)::
GPG-sign the push request to update refs on the receiving
side, to allow it to be checked by the hooks and/or be
-   logged.  See linkgit:git-receive-pack[1] for the details
-   on the receiving end.  If the attempt to sign with `gpg` fails,
-   or if the server does not support signed pushes, the push will
-   fail.
+   logged.  If `false` or `--no-signed`, no signing will be
+   attempted.  If `true` or `--signed`, the push will fail if the
+   server does not support signed pushes.  If set to `if-asked`,
+   sign if and only if the server supports signed pushes.  The push
+   will also fail if the actual call to `gpg --sign` fails.  See
+   linkgit:git-receive-pack[1] for the details on the receiving end.
 
 --[no-]atomic::
Use an atomic transaction on the remote side if available.
diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt
index 0a0a3fb..6aa91e8 100644
--- a/Documentation/git-send-pack.txt
+++ b/Documentation/git-send-pack.txt
@@ -10,7 +10,8 @@ SYNOPSIS
 
 [verse]
 'git send-pack' [--all] [--dry-run] [--force] 
[--receive-pack=git-receive-pack]
-   [--verbose] [--thin] [--atomic] [--signed]
+   [--verbose] [--thin] [--atomic]
+   [--[no-]signed|--sign=(true|false|if-asked)]
[host:]directory [ref...]
 
 DESCRIPTION
@@ -69,13 +70,16 @@ be in a separate packet, and the list must end with a flush 
packet.
fails to update then the entire push will fail without changing any
refs.
 
---signed::
+--[no-]signed::
+--sign=(true|false|if-asked)::
GPG-sign the push request to update refs on the receiving
side, to allow it to be checked by the hooks and/or be
-   logged.  See linkgit:git-receive-pack[1] for the details
-   on the receiving end.  If the attempt to sign with `gpg` fails,
-   or if the server does not support signed pushes, the push will
-   fail.
+   logged.  If `false` or `--no-signed`, no signing will be
+   attempted.  If `true` or `--signed`, the push will fail if the
+   server does not support signed pushes.  If set to `if-asked`,
+   sign if and only if the server supports signed pushes.  The push
+   will also fail if the actual call to `gpg --sign` fails.  See
+   linkgit:git-receive-pack[1] for the details on the receiving end.
 
 host::
A remote host to house the repository.  When this
diff --git a/builtin/push.c b/builtin/push.c
index 57c138b..85a82cd 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -9,6 +9,7 @@
 #include transport.h
 #include parse-options.h
 #include submodule.h
+#include send-pack.h
 
 static const char * const push_usage[] = {
N_(git push [options] [repository [refspec...]]),
@@ -495,6 +496,7 @@ int cmd_push(int argc, const char **argv, const char 
*prefix)
 {
int flags = 0;
int tags = 0;
+   int push_cert = -1;
int rc;
const char *repo = NULL;/* default repository */
struct option options[] = {
@@ -526,7 +528,9 @@ int cmd_push(int argc, const char **argv, const char 
*prefix)
OPT_BIT(0, no-verify, flags, 

[PATCH v2 4/9] gitremote-helpers.txt: Document pushcert option

2015-08-19 Thread Dave Borowitz
Signed-off-by: Dave Borowitz dborow...@google.com
---
 Documentation/gitremote-helpers.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/gitremote-helpers.txt 
b/Documentation/gitremote-helpers.txt
index 82e2d15..78e0b27 100644
--- a/Documentation/gitremote-helpers.txt
+++ b/Documentation/gitremote-helpers.txt
@@ -448,6 +448,9 @@ set by Git if the remote helper has the 'option' capability.
 'option update-shallow {'true'|'false'}::
Allow to extend .git/shallow if the new refs require it.
 
+'option pushcert {'true'|'false'}::
+   GPG sign pushes.
+
 SEE ALSO
 
 linkgit:git-remote[1]
-- 
2.5.0.276.gf5e568e

--
To unsubscribe from this list: send the line 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/9] Documentation/git-push.txt: Document when --signed may fail

2015-08-19 Thread Dave Borowitz
Like --atomic, --signed will fail if the server does not advertise the
necessary capability. In addition, it requires gpg on the client side.

Signed-off-by: Dave Borowitz dborow...@google.com
---
 Documentation/git-push.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 135d810..da0a98d 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -137,7 +137,9 @@ already exists on the remote side.
GPG-sign the push request to update refs on the receiving
side, to allow it to be checked by the hooks and/or be
logged.  See linkgit:git-receive-pack[1] for the details
-   on the receiving end.
+   on the receiving end.  If the attempt to sign with `gpg` fails,
+   or if the server does not support signed pushes, the push will
+   fail.
 
 --[no-]atomic::
Use an atomic transaction on the remote side if available.
-- 
2.5.0.276.gf5e568e

--
To unsubscribe from this list: send the line 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 2/9] Documentation/git-send-pack.txt: Flow long synopsis line

2015-08-19 Thread Dave Borowitz
Signed-off-by: Dave Borowitz dborow...@google.com
---
 Documentation/git-send-pack.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt
index b5d09f7..6a6 100644
--- a/Documentation/git-send-pack.txt
+++ b/Documentation/git-send-pack.txt
@@ -9,7 +9,8 @@ git-send-pack - Push objects over Git protocol to another 
repository
 SYNOPSIS
 
 [verse]
-'git send-pack' [--all] [--dry-run] [--force] 
[--receive-pack=git-receive-pack] [--verbose] [--thin] [--atomic] 
[host:]directory [ref...]
+'git send-pack' [--all] [--dry-run] [--force] 
[--receive-pack=git-receive-pack]
+   [--verbose] [--thin] [--atomic] [host:]directory [ref...]
 
 DESCRIPTION
 ---
-- 
2.5.0.276.gf5e568e

--
To unsubscribe from this list: send the line 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/9] Flags and config to sign pushes by default

2015-08-19 Thread Dave Borowitz
Changes since v1:
 - Rebased on 44e02239
 - Use options --[no-]signed|--signed=(yes|no|if-asked)
 - Support general yes/true/1/etc. option parsing.
 - Convert builtin/send-pack.c to use option parsing API for better code
   reuse.
 - Various cleanups as suggested by Junio.

v1 can be found at:
http://thread.gmane.org/gmane.comp.version-control.git/275881

Dave Borowitz (9):
  Documentation/git-push.txt: Document when --signed may fail
  Documentation/git-send-pack.txt: Flow long synopsis line
  Documentation/git-send-pack.txt: Document --signed
  gitremote-helpers.txt: Document pushcert option
  transport: Remove git_transport_options.push_cert
  config.c: Expose git_parse_maybe_bool
  builtin/send-pack.c: Use option parsing API
  Support signing pushes iff the server supports it
  Add a config option push.gpgSign for default signed pushes

 Documentation/config.txt|   8 ++
 Documentation/git-push.txt  |  15 ++-
 Documentation/git-send-pack.txt |  16 ++-
 Documentation/gitremote-helpers.txt |   3 +
 builtin/push.c  |  42 +++-
 builtin/send-pack.c | 192 
 cache.h |   1 +
 config.c|   6 +-
 remote-curl.c   |  16 ++-
 send-pack.c |  43 ++--
 send-pack.h |  12 ++-
 transport-helper.c  |  34 +++
 transport.c |  11 ++-
 transport.h |   6 +-
 14 files changed, 253 insertions(+), 152 deletions(-)

-- 
2.5.0.276.gf5e568e

--
To unsubscribe from this list: send the line 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 v12 03/13] ref-filter: introduce the ref_formatting_state stack machinery

2015-08-19 Thread Karthik Nayak
On Wed, Aug 19, 2015 at 9:14 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
 Karthik Nayak karthik@gmail.com writes:

 --- a/ref-filter.c
 +++ b/ref-filter.c
 @@ -155,6 +155,8 @@ static void push_new_state(struct ref_formatting_state 
 **stack)

 strbuf_init(s-output, 0);
 s-prev = *stack;
 +   if (*stack)
 +   s-quote_style = (*stack)-quote_style;
 *stack = s;
  }



 This seems about right, why do you think it's a stupid fix?

 If you have a stack of N elemments, why replicate a field N times if all
 the N instances always have the same value?

 There's nothing to be pushed or poped with quote_style, so having it in
 the stack is confusing to the reader (one has to infer the property all
 instances have the same value by reading the code instead of having
 just one variable), and error-prone for the author: you already got
 it wrong once.

Thats also there, I'll guess it makes more sense to remove it from
ref_formatting_state.

-- 
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line 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 v12 03/13] ref-filter: introduce the ref_formatting_state stack machinery

2015-08-19 Thread Karthik Nayak
On Wed, Aug 19, 2015 at 9:24 PM, Karthik Nayak karthik@gmail.com wrote:
 On Wed, Aug 19, 2015 at 9:14 PM, Matthieu Moy
 matthieu@grenoble-inp.fr wrote:
 Karthik Nayak karthik@gmail.com writes:

 --- a/ref-filter.c
 +++ b/ref-filter.c
 @@ -155,6 +155,8 @@ static void push_new_state(struct ref_formatting_state 
 **stack)

 strbuf_init(s-output, 0);
 s-prev = *stack;
 +   if (*stack)
 +   s-quote_style = (*stack)-quote_style;
 *stack = s;
  }



 This seems about right, why do you think it's a stupid fix?

 If you have a stack of N elemments, why replicate a field N times if all
 the N instances always have the same value?

 There's nothing to be pushed or poped with quote_style, so having it in
 the stack is confusing to the reader (one has to infer the property all
 instances have the same value by reading the code instead of having
 just one variable), and error-prone for the author: you already got
 it wrong once.

 Thats also there, I'll guess it makes more sense to remove it from
 ref_formatting_state.


Speaking of quote_value, The quote doesn't work well with color's
for e.g.
git for-each-ref --shell --format=%(color:green)%(refname)
'''refs/heads/allow-unknown-type'''
Seems like an simple fix, probably after GSoC I'll do this :)

-- 
Regards,
Karthik Nayak
--
To unsubscribe from this list: send the line 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 7/9] builtin/send-pack.c: Use option parsing API

2015-08-19 Thread Stefan Beller
On Wed, Aug 19, 2015 at 8:26 AM, Dave Borowitz dborow...@google.com wrote:
 The old option parsing code in this plumbing command predates this
 API, so option parsing was done more manually. Using the new API
 brings send-pack more in line with push, and accepts new variants
 like --no-* for negating options.

 Signed-off-by: Dave Borowitz dborow...@google.com
 ---
  builtin/send-pack.c | 163 
 +++-
  1 file changed, 59 insertions(+), 104 deletions(-)

 diff --git a/builtin/send-pack.c b/builtin/send-pack.c
 index 23b2962..5f2c744 100644
 --- a/builtin/send-pack.c
 +++ b/builtin/send-pack.c
 @@ -12,10 +12,15 @@
  #include version.h
  #include sha1-array.h
  #include gpg-interface.h
 +#include gettext.h

 -static const char send_pack_usage[] =
 -git send-pack [--all | --mirror] [--dry-run] [--force] 
 [--receive-pack=git-receive-pack] [--verbose] [--thin] [--atomic] 
 [host:]directory [ref...]\n
 -  --all and explicit ref specification are mutually exclusive.;
 +static const char * const send_pack_usage[] = {
 +   N_(git send-pack [--all | --mirror] [--dry-run] [--force] 
 + [--receive-pack=git-receive-pack] [--verbose] [--thin] 
 [--atomic] 
 + [host:]directory [ref...]\n
 +   --all and explicit ref specification are mutually exclusive.),
 +   NULL,
 +};

  static struct send_pack_args args;

 @@ -107,116 +112,66 @@ int cmd_send_pack(int argc, const char **argv, const 
 char *prefix)
 int ret;
 int helper_status = 0;
 int send_all = 0;
 +   int verbose = 0;
 const char *receivepack = git-receive-pack;
 +   unsigned dry_run = 0;
 +   unsigned send_mirror = 0;
 +   unsigned force_update = 0;
 +   unsigned quiet = 0;
 +   unsigned push_cert = 0;
 +   unsigned use_thin_pack = 0;
 +   unsigned atomic = 0;
 +   unsigned stateless_rpc = 0;

First I thought:
You could write to the args flags directly from the options. No
need to have (most of)
the variables around here and copy over the values. You'd need to
use OPT_BIT instead
for setting a specific bit though
but then I realized we do not have a direct bit field in args, which
would make it a bit unreadable.

 int flags;
 unsigned int reject_reasons;
 int progress = -1;
 int from_stdin = 0;
 struct push_cas_option cas = {0};

 -   git_config(git_gpg_config, NULL);
 +   struct option options[] = {
 +   OPT__VERBOSITY(verbose),
 +   OPT_STRING(0, receive-pack, receivepack, receive-pack, 
 N_(receive pack program)),
 +   OPT_STRING(0, exec, receivepack, receive-pack, 
 N_(receive pack program)),
 +   OPT_STRING(0, remote, remote_name, remote, N_(remote 
 name)),
 +   OPT_BOOL(0, all, send_all, N_(push all refs)),
 +   OPT_BOOL('n' , dry-run, dry_run, N_(dry run)),
 +   OPT_BOOL(0, mirror, send_mirror, N_(mirror all refs)),
 +   OPT_BOOL('f', force, force_update, N_(force updates)),

-f and -n are new here now?

 +   OPT_BOOL(0, signed, push_cert, N_(GPG sign the push)),
 +   OPT_BOOL(0, progress, progress, N_(force progress 
 reporting)),
 +   OPT_BOOL(0, thin, use_thin_pack, N_(use thin pack)),
 +   OPT_BOOL(0, atomic, atomic, N_(request atomic transaction 
 on remote side)),
 +   OPT_BOOL(0, stateless-rpc, stateless_rpc, N_(use 
 stateless RPC protocol)),
 +   OPT_BOOL(0, stdin, from_stdin, N_(read refs from stdin)),
 +   OPT_BOOL(0, helper-status, helper_status, N_(print status 
 from remote helper)),
 +   { OPTION_CALLBACK,
 + 0, CAS_OPT_NAME, cas, N_(refname:expect),
 + N_(require old value of ref to be at this value),
 + PARSE_OPT_OPTARG, parseopt_push_cas_option },
 +   OPT_END()
 +   };

 -   argv++;
 -   for (i = 1; i  argc; i++, argv++) {
 -   const char *arg = *argv;
 -
 -   if (*arg == '-') {
 -   if (starts_with(arg, --receive-pack=)) {
 -   receivepack = arg + 15;
 -   continue;
 -   }
 -   if (starts_with(arg, --exec=)) {
 -   receivepack = arg + 7;
 -   continue;
 -   }
 -   if (starts_with(arg, --remote=)) {
 -   remote_name = arg + 9;
 -   continue;
 -   }
 -   if (!strcmp(arg, --all)) {
 -   send_all = 1;
 -   continue;
 -   }
 -   if (!strcmp(arg, --dry-run)) {
 -   args.dry_run = 1;
 -   continue;
 -   

Re: [PATCH 5/7] submodule: implement `module_list` as a builtin helper

2015-08-19 Thread Junio C Hamano
Stefan Beller sbel...@google.com writes:

 +static int module_list_compute(int argc, const char **argv,
 + const char *prefix,
 + struct pathspec *pathspec)
 +{
 + int i;
 + char *max_prefix, *ps_matched = NULL;
 + int max_prefix_len;
 + parse_pathspec(pathspec, 0,
 +PATHSPEC_PREFER_FULL |
 +PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
 +prefix, argv);
 +
 + /* Find common prefix for all pathspec's */
 + max_prefix = common_prefix(pathspec);
 + max_prefix_len = max_prefix ? strlen(max_prefix) : 0;
 +
 + if (pathspec-nr)
 + ps_matched = xcalloc(1, pathspec-nr);

Micronit.  Even though multiplication is commutative, the order of
arguments to xcalloc() looks odd.  It lets you say I want an array
with nmemb elements, and each of its is size-bytes long by giving
it nmemb and then size.

No need to resend; will fix it up while queuing.

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


Re: [PATCH v4 0/3] dt/untracked-subdir

2015-08-19 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy  pclo...@gmail.com writes:

 The first patch is a split from David's v3 patch because it should
 belong to dt/untracked-sparse. The second is basically David's v3.
 The third patch fixes untracked_cache_invalidate_path(). David fixed
 it in v2, but there's another bug in this code.

 David Turner (1):
   untracked-cache: fix subdirectory handling

 Nguyễn Thái Ngọc Duy (2):
   t7063: use --force-untracked-cache to speed up a bit
   untracked cache: fix entry invalidation

  dir.c |  82 +++---
  t/t7063-status-untracked-cache.sh | 102 
 --
  2 files changed, 163 insertions(+), 21 deletions(-)

They all look reasonable.  Will replace v3 with these and wait a bit
to give chance to David and others to comment.

Thanks.

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


Re: [PATCH 5/7] submodule: implement `module_list` as a builtin helper

2015-08-19 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 Micronit.  Even though multiplication is commutative, the order of
 arguments to xcalloc() looks odd.  It lets you say I want an array
 with nmemb elements, and each of its is size-bytes long by giving
 it nmemb and then size.

Unrelated tangent, but while I have output from git grep 'calloc.*pathspec'
on my screen... ;-)

 builtin/checkout.c | 2 +-
 builtin/ls-files.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 7ea533e..e3b28e4 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -283,7 +283,7 @@ static int checkout_paths(const struct checkout_opts *opts,
if (opts-source_tree)
read_tree_some(opts-source_tree, opts-pathspec);
 
-   ps_matched = xcalloc(1, opts-pathspec.nr);
+   ps_matched = xcalloc(opts-pathspec.nr, 1);
 
/*
 * Make sure all pathspecs participated in locating the paths
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 6fa2205..b6a7cb0 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -516,7 +516,7 @@ int cmd_ls_files(int argc, const char **argv, const char 
*cmd_prefix)
 
/* Treat unmatching pathspec elements as errors */
if (pathspec.nr  error_unmatch)
-   ps_matched = xcalloc(1, pathspec.nr);
+   ps_matched = xcalloc(pathspec.nr, 1);
 
if ((dir.flags  DIR_SHOW_IGNORED)  !exc_given)
die(ls-files --ignored needs some exclude pattern);
--
To unsubscribe from this list: send the line 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] am --skip/--abort: merge HEAD/ORIG_HEAD tree into index

2015-08-19 Thread Junio C Hamano
Paul Tan pyoka...@gmail.com writes:

 (In addition, I fixed a small mistake with the struct tree_desc array
 size.)

Yeah, one thing I forgot to mention was that I suspect this kind of
oneway merge already in other places in the code, and a future
mini-project might be to inspect them all and try to see if their
commonalities can be taken advantage of.  If we had a small helper
function to do the do the equivalent of 'git reset $TREE', this
miscounting wouldn't have happened.

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


Re: [PATCH 4/7] submodule: Allow errornous values for the fetchrecursesubmodules option

2015-08-19 Thread Junio C Hamano
Stefan Beller sbel...@google.com writes:

 From: Heiko Voigt hvo...@hvoigt.net

 We should not die when reading the submodule config cache since the user
 might not be able to get out of that situation when the configuration is
 part of the history.

 We should handle this condition later when the value is about to be
 used.

 Signed-off-by: Heiko Voigt hvo...@hvoigt.net
 Signed-off-by: Stefan Beller sbel...@google.com
 ---

The retitle is good but we'd need to typofix erroneous there, I
think (will do so while queuing, no need to resend).


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


Re: [PATCH 5/7] submodule: implement `module_list` as a builtin helper

2015-08-19 Thread Stefan Beller
On Wed, Aug 19, 2015 at 11:17 AM, Junio C Hamano gits...@pobox.com wrote:
 Stefan Beller sbel...@google.com writes:

 +static int module_list_compute(int argc, const char **argv,
 + const char *prefix,
 + struct pathspec *pathspec)
 +{
 + int i;
 + char *max_prefix, *ps_matched = NULL;
 + int max_prefix_len;
 + parse_pathspec(pathspec, 0,
 +PATHSPEC_PREFER_FULL |
 +PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
 +prefix, argv);
 +
 + /* Find common prefix for all pathspec's */
 + max_prefix = common_prefix(pathspec);
 + max_prefix_len = max_prefix ? strlen(max_prefix) : 0;
 +
 + if (pathspec-nr)
 + ps_matched = xcalloc(1, pathspec-nr);

 Micronit.  Even though multiplication is commutative, the order of
 arguments to xcalloc() looks odd.  It lets you say I want an array
 with nmemb elements, and each of its is size-bytes long by giving
 it nmemb and then size.

Thanks for pointing that out!
I think this would be an alignment issue in theory, if the items were larger and
the number of items not dividable by the size of items.

So because 1 char has no alignment problems, we can happily allocate
just one block of the member size here.


 No need to resend; will fix it up while queuing.

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


Re: [PATCH 5/7] submodule: implement `module_list` as a builtin helper

2015-08-19 Thread Stefan Beller
On Wed, Aug 19, 2015 at 11:22 AM, Junio C Hamano gits...@pobox.com wrote:
 Junio C Hamano gits...@pobox.com writes:

 Micronit.  Even though multiplication is commutative, the order of
 arguments to xcalloc() looks odd.  It lets you say I want an array
 with nmemb elements, and each of its is size-bytes long by giving
 it nmemb and then size.

 Unrelated tangent, but while I have output from git grep 'calloc.*pathspec'
 on my screen... ;-)

  builtin/checkout.c | 2 +-
  builtin/ls-files.c | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

 diff --git a/builtin/checkout.c b/builtin/checkout.c
 index 7ea533e..e3b28e4 100644
 --- a/builtin/checkout.c
 +++ b/builtin/checkout.c
 @@ -283,7 +283,7 @@ static int checkout_paths(const struct checkout_opts 
 *opts,
 if (opts-source_tree)
 read_tree_some(opts-source_tree, opts-pathspec);

 -   ps_matched = xcalloc(1, opts-pathspec.nr);
 +   ps_matched = xcalloc(opts-pathspec.nr, 1);

 /*
  * Make sure all pathspecs participated in locating the paths
 diff --git a/builtin/ls-files.c b/builtin/ls-files.c
 index 6fa2205..b6a7cb0 100644
 --- a/builtin/ls-files.c
 +++ b/builtin/ls-files.c
 @@ -516,7 +516,7 @@ int cmd_ls_files(int argc, const char **argv, const char 
 *cmd_prefix)

 /* Treat unmatching pathspec elements as errors */
 if (pathspec.nr  error_unmatch)
 -   ps_matched = xcalloc(1, pathspec.nr);
 +   ps_matched = xcalloc(pathspec.nr, 1);

 if ((dir.flags  DIR_SHOW_IGNORED)  !exc_given)
 die(ls-files --ignored needs some exclude pattern);

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


Re: [PATCH 7/7] submodule: implement `module_clone` as a builtin helper

2015-08-19 Thread Stefan Beller
On Wed, Aug 19, 2015 at 12:08 PM, Junio C Hamano gits...@pobox.com wrote:
 Stefan Beller sbel...@google.com writes:

 `module_clone` is part of the update command, which I want to convert
 to C next.

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

 This is the only patch that broke by a last minute fix,
 so this replaces the previous PATCH 7/7.

 ... which still breaks 7400.82, though.

because I messed up the resending. :(

Squashing in this fixes it:
---8---
diff --git a/git-submodule.sh b/git-submodule.sh
index e50451b..e7d221e 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -338,7 +338,7 @@ Use -f if you really want to add it. 2
echo $(eval_gettext Reactivating
local git directory for submodule '\$sm_name'.)
fi
fi
-   git submodule--helper module_clone --prefix
$wt_prefix --path $sm_path --name $sm_name --url $realrepo
--reference $reference_path --depth $depth || exit
+   git submodule--helper module_clone --prefix
$wt_prefix --path $sm_path --name $sm_name --url $realrepo
$reference_path $depth || exit
(
clear_local_git_env
cd $sm_path 
@@ -634,7 +634,7 @@ cmd_update()
shift
;;
--depth=*)
-   depth=$1
+   depth=$1
;;
--)
shift
---
because both depth and reference are either empty string or containing
both key and value
(i.e. $depth will be --depth=5 and we should not pass --depth into
submodule--helper module_clone
twice)

I can resend the patch as a whole if you'd like. I'd send it together
 in the next series later today, which will demonstrate a possible
implementation of  `git submodule foreach-parallel`



 Output from running it with -i -v ends like this:

 expecting success:
 mkdir super 
 pwd=$(pwd) 
 (
 cd super 
 git init 
 git submodule add --depth=1 file://$pwd/example2 submodule 
 
 (
 cd submodule 
 test 1 = $(git log --oneline | wc -l)
 )
 )

 Initialized empty Git repository in /home/gitster/w/git.git/t/trash 
 directory.t7400-submodule-basic/super/.git/
 fatal: Clone of 'file:///home/gitster/w/git.git/t/trash 
 directory.t7400-submodule-basic/example2' into submodule path 'submodule' 
 failed
 not ok 82 - submodule add clone shallow submodule
 #
 #   mkdir super 
 #   pwd=$(pwd) 
 #   (
 #   cd super 
 #   git init 
 #   git submodule add --depth=1 file://$pwd/example2 
 submodule 
 #   (
 #   cd submodule 
 #   test 1 = $(git log --oneline | wc -l)
 #   )
 #   )
 #
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/9] Documentation/git-send-pack.txt: Flow long synopsis line

2015-08-19 Thread Junio C Hamano
Dave Borowitz dborow...@google.com writes:

 Signed-off-by: Dave Borowitz dborow...@google.com
 ---
  Documentation/git-send-pack.txt | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

 diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt
 index b5d09f7..6a6 100644
 --- a/Documentation/git-send-pack.txt
 +++ b/Documentation/git-send-pack.txt
 @@ -9,7 +9,8 @@ git-send-pack - Push objects over Git protocol to another 
 repository
  SYNOPSIS
  
  [verse]
 -'git send-pack' [--all] [--dry-run] [--force]
 [--receive-pack=git-receive-pack] [--verbose] [--thin] [--atomic]
 [host:]directory [ref...]
 +'git send-pack' [--all] [--dry-run] [--force] 
 [--receive-pack=git-receive-pack]
 + [--verbose] [--thin] [--atomic] [host:]directory [ref...]
  
  DESCRIPTION
  ---

As can be expected from the Subject: line, this patch is
line-wrapped and does not apply ;-)

I've done a trivial fix-up and took the liberty of making the result
of this step into three lines, not two.  That would make 3/9 look
more trivial.

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


[PATCH v2] git-p4: fix faulty paths for case insensitive systems

2015-08-19 Thread larsxschneider
From: Lars Schneider larsxschnei...@gmail.com

Hi,

as discussed with Luke in [PATCH] git-p4: fix faulty paths for case insensitive
systems I added a test case for my path fix. I also changed the trigger for the
fix to the command line parameter --fix-paths.

Cheers,
Lars

Lars Schneider (1):
  git-p4: fix faulty paths for case insensitive systems

 git-p4.py | 83 +--
 t/t9821-git-p4-path-variations.sh | 91 +++
 2 files changed, 170 insertions(+), 4 deletions(-)
 create mode 100755 t/t9821-git-p4-path-variations.sh

--
1.9.5 (Apple Git-50.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


Re: [PATCH v2 7/9] builtin/send-pack.c: Use option parsing API

2015-08-19 Thread Dave Borowitz
On Wed, Aug 19, 2015 at 2:00 PM, Stefan Beller sbel...@google.com wrote:
 On Wed, Aug 19, 2015 at 8:26 AM, Dave Borowitz dborow...@google.com wrote:
 The old option parsing code in this plumbing command predates this
 API, so option parsing was done more manually. Using the new API
 brings send-pack more in line with push, and accepts new variants
 like --no-* for negating options.

 Signed-off-by: Dave Borowitz dborow...@google.com
 ---
  builtin/send-pack.c | 163 
 +++-
  1 file changed, 59 insertions(+), 104 deletions(-)

 diff --git a/builtin/send-pack.c b/builtin/send-pack.c
 index 23b2962..5f2c744 100644
 --- a/builtin/send-pack.c
 +++ b/builtin/send-pack.c
 @@ -12,10 +12,15 @@
  #include version.h
  #include sha1-array.h
  #include gpg-interface.h
 +#include gettext.h

 -static const char send_pack_usage[] =
 -git send-pack [--all | --mirror] [--dry-run] [--force] 
 [--receive-pack=git-receive-pack] [--verbose] [--thin] [--atomic] 
 [host:]directory [ref...]\n
 -  --all and explicit ref specification are mutually exclusive.;
 +static const char * const send_pack_usage[] = {
 +   N_(git send-pack [--all | --mirror] [--dry-run] [--force] 
 + [--receive-pack=git-receive-pack] [--verbose] [--thin] 
 [--atomic] 
 + [host:]directory [ref...]\n
 +   --all and explicit ref specification are mutually 
 exclusive.),
 +   NULL,
 +};

  static struct send_pack_args args;

 @@ -107,116 +112,66 @@ int cmd_send_pack(int argc, const char **argv, const 
 char *prefix)
 int ret;
 int helper_status = 0;
 int send_all = 0;
 +   int verbose = 0;
 const char *receivepack = git-receive-pack;
 +   unsigned dry_run = 0;
 +   unsigned send_mirror = 0;
 +   unsigned force_update = 0;
 +   unsigned quiet = 0;
 +   unsigned push_cert = 0;
 +   unsigned use_thin_pack = 0;
 +   unsigned atomic = 0;
 +   unsigned stateless_rpc = 0;

 First I thought:
 You could write to the args flags directly from the options. No
 need to have (most of)
 the variables around here and copy over the values. You'd need to
 use OPT_BIT instead
 for setting a specific bit though
 but then I realized we do not have a direct bit field in args, which
 would make it a bit unreadable.

Right, and args-push_cert etc. is invalid, and I didn't know if it
was ok to expand the args struct to be several words longer. But I'm
not a C programmer so I'm happy to take suggestions how to make this
more idiomatic.

 int flags;
 unsigned int reject_reasons;
 int progress = -1;
 int from_stdin = 0;
 struct push_cas_option cas = {0};

 -   git_config(git_gpg_config, NULL);
 +   struct option options[] = {
 +   OPT__VERBOSITY(verbose),
 +   OPT_STRING(0, receive-pack, receivepack, receive-pack, 
 N_(receive pack program)),
 +   OPT_STRING(0, exec, receivepack, receive-pack, 
 N_(receive pack program)),
 +   OPT_STRING(0, remote, remote_name, remote, N_(remote 
 name)),
 +   OPT_BOOL(0, all, send_all, N_(push all refs)),
 +   OPT_BOOL('n' , dry-run, dry_run, N_(dry run)),
 +   OPT_BOOL(0, mirror, send_mirror, N_(mirror all refs)),
 +   OPT_BOOL('f', force, force_update, N_(force updates)),

 -f and -n are new here now?

Yeah, I was going for consistency with push.c (and also just copy/pasted ;)

 +   OPT_BOOL(0, signed, push_cert, N_(GPG sign the push)),
 +   OPT_BOOL(0, progress, progress, N_(force progress 
 reporting)),
 +   OPT_BOOL(0, thin, use_thin_pack, N_(use thin pack)),
 +   OPT_BOOL(0, atomic, atomic, N_(request atomic 
 transaction on remote side)),
 +   OPT_BOOL(0, stateless-rpc, stateless_rpc, N_(use 
 stateless RPC protocol)),
 +   OPT_BOOL(0, stdin, from_stdin, N_(read refs from 
 stdin)),
 +   OPT_BOOL(0, helper-status, helper_status, N_(print 
 status from remote helper)),
 +   { OPTION_CALLBACK,
 + 0, CAS_OPT_NAME, cas, N_(refname:expect),
 + N_(require old value of ref to be at this value),
 + PARSE_OPT_OPTARG, parseopt_push_cas_option },
 +   OPT_END()
 +   };

 -   argv++;
 -   for (i = 1; i  argc; i++, argv++) {
 -   const char *arg = *argv;
 -
 -   if (*arg == '-') {
 -   if (starts_with(arg, --receive-pack=)) {
 -   receivepack = arg + 15;
 -   continue;
 -   }
 -   if (starts_with(arg, --exec=)) {
 -   receivepack = arg + 7;
 -   continue;
 -   }
 -   if (starts_with(arg, --remote=)) {
 -   remote_name = arg + 9;
 -  

[PATCH v2] git-p4: fix faulty paths for case insensitive systems

2015-08-19 Thread larsxschneider
From: Lars Schneider larsxschnei...@gmail.com

PROBLEM:
We run P4 servers on Linux and P4 clients on Windows. For an unknown
reason the file path for a number of files in P4 does not match the
directory path with respect to case sensitivity.

E.g. `p4 files` might return
//depot/path/to/file1
//depot/PATH/to/file2

If you use P4/P4V then these files end up in the same directory, e.g.
//depot/path/to/file1
//depot/path/to/file2

If you use git-p4 then all files not matching the correct file path
(e.g. `file2`) will be ignored.

SOLUTION:
Identify files that are different with respect to case sensitivity.
If there are any then run `p4 dirs` to build up a dictionary
containing the correct cases for each path. It looks like P4
interprets correct here as the existing path of the first file in a
directory. The path dictionary is used later on to fix all paths.

This is only applied if the parameter --fix-paths is passed to the
git-p4 clone command.

Signed-off-by: Lars Schneider larsxschnei...@gmail.com
---
 git-p4.py | 83 +--
 t/t9821-git-p4-path-variations.sh | 91 +++
 2 files changed, 170 insertions(+), 4 deletions(-)
 create mode 100755 t/t9821-git-p4-path-variations.sh

diff --git a/git-p4.py b/git-p4.py
index 073f87b..a21809d 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -1931,7 +1931,7 @@ class View(object):
 (self.client_prefix, clientFile))
 return clientFile[len(self.client_prefix):]

-def update_client_spec_path_cache(self, files):
+def update_client_spec_path_cache(self, files, fixPathCase = None):
  Caching file paths by p4 where batch query 

 # List depot file paths exclude that already cached
@@ -1950,6 +1950,8 @@ class View(object):
 if unmap in res:
 # it will list all of them, but only one not unmap-ped
 continue
+if fixPathCase:
+res['depotFile'] = fixPathCase(res['depotFile'])
 self.client_spec_path_cache[res['depotFile']] = 
self.convert_client_path(res[clientFile])

 # not found files or unmap files set to 
@@ -1987,6 +1989,7 @@ class P4Sync(Command, P4UserMap):
  help=Maximum number of changes to 
import),
 optparse.make_option(--changes-block-size, 
dest=changes_block_size, type=int,
  help=Internal block size to use when 
iteratively calling p4 changes),
+optparse.make_option(--fix-paths, dest=fixPaths, 
action=store_true),
 optparse.make_option(--keep-path, dest=keepRepoPath, 
action='store_true',
  help=Keep entire BRANCH/DIR/SUBDIR 
prefix during import),
 optparse.make_option(--use-client-spec, 
dest=useClientSpec, action='store_true',
@@ -2017,6 +2020,7 @@ class P4Sync(Command, P4UserMap):
 self.maxChanges = 
 self.changes_block_size = None
 self.keepRepoPath = False
+self.fixPaths = False
 self.depotPaths = None
 self.p4BranchesInGit = []
 self.cloneExclude = []
@@ -2049,7 +2053,8 @@ class P4Sync(Command, P4UserMap):
 files = []
 fnum = 0
 while commit.has_key(depotFile%s % fnum):
-path =  commit[depotFile%s % fnum]
+path = commit[depotFile%s % fnum]
+path = self.fixPathCase(path)

 if [p for p in self.cloneExclude
 if p4PathStartsWith(path, p)]:
@@ -2113,7 +2118,9 @@ class P4Sync(Command, P4UserMap):
 branches = {}
 fnum = 0
 while commit.has_key(depotFile%s % fnum):
-path =  commit[depotFile%s % fnum]
+path = commit[depotFile%s % fnum]
+path = self.fixPathCase(path)
+
 found = [p for p in self.depotPaths
  if p4PathStartsWith(path, p)]
 if not found:
@@ -2240,6 +2247,10 @@ class P4Sync(Command, P4UserMap):
 if marshalled[code] == error:
 if data in marshalled:
 err = marshalled[data].rstrip()
+
+if depotFile in marshalled:
+marshalled['depotFile'] = self.fixPathCase(marshalled['depotFile'])
+
 if err:
 f = None
 if self.stream_have_file_info:
@@ -2314,6 +2325,7 @@ class P4Sync(Command, P4UserMap):

 # do the last chunk
 if self.stream_file.has_key('depotFile'):
+self.stream_file['depotFile'] = 
self.fixPathCase(self.stream_file['depotFile'])
 self.streamOneP4File(self.stream_file, self.stream_contents)

 def make_email(self, userid):
@@ -2371,7 +2383,8 @@ class P4Sync(Command, P4UserMap):
 sys.stderr.write(Ignoring file outside of prefix: %s\n % 
f['path'])

 if self.clientSpecDirs:
-self.clientSpecDirs.update_client_spec_path_cache(files)
+  

Re: [PATCH v2 8/9] Support signing pushes iff the server supports it

2015-08-19 Thread Junio C Hamano
Dave Borowitz dborow...@google.com writes:

 Add a new flag --signed-if-possible to push and send-pack that sends a
 push certificate if and only if the server advertised a push cert
 nonce. If not, at least warn the user that their push may not be as
 secure as they thought.

 Signed-off-by: Dave Borowitz dborow...@google.com
 ---

Obviously, the above description needs updating.  Here is what I've
queued tentatively.

Thanks.

commit 32d273dfabb0a70b2839971f5afff7fa86a8f4c2
Author: Dave Borowitz dborow...@google.com
Date:   Wed Aug 19 11:26:46 2015 -0400

push: support signing pushes iff the server supports it

Add a new flag --sign=true (or --sign=false), which means the same
thing as the original --signed (or --no-signed).  Give it a third
value --sign=if-asked to tell push and send-pack to send a push
certificate if and only if the server advertised a push cert nonce.

If not, warn the user that their push may not be as secure as they
thought.

Signed-off-by: Dave Borowitz dborow...@google.com
Signed-off-by: Junio C Hamano gits...@pobox.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


[PATCH] generate-cmdlist: re-implement as shell script

2015-08-19 Thread Eric Sunshine
527ec39 (generate-cmdlist: parse common group commands, 2015-05-21)
replaced generate-cmdlist.sh with a more functional Perl version,
generate-cmdlist.perl. The Perl version gleans named tags from a new
common groups section in command-list.txt and recognizes those tags in
command list section entries in place of the old 'common' tag. This
allows git-help to, not only recognize, but also group common commands.

Although the tests require Perl, 527ec39 creates an unconditional
dependence upon Perl in the build system itself, which can not be
overridden with NO_PERL. Such a dependency may be undesirable; for
instance, the 'git-lite' package in the FreeBSD ports tree is intended
as a minimal Git installation (which may, for example, be useful on
servers needing only local clone and update capability), which,
historically, has not depended upon Perl[1].

Therefore, revive generate-cmdlist.sh and extend it to recognize common
groups and its named tags. Retire generate-cmdlist.perl.

[1]: http://thread.gmane.org/gmane.comp.version-control.git/275905/focus=276132

Signed-off-by: Eric Sunshine sunsh...@sunshineco.com
---

In addition to reviving 527ec39^:generate-cmdlist.sh and extending it, I
also re-indented it with tabs instead of spaces, so it's helpful to
ignore whitespace changes when comparing the old and new versions of the
shell script.

Makefile  |  4 ++--
 generate-cmdlist.perl | 50 --
 generate-cmdlist.sh   | 50 ++
 3 files changed, 52 insertions(+), 52 deletions(-)
 delete mode 100755 generate-cmdlist.perl
 create mode 100755 generate-cmdlist.sh

diff --git a/Makefile b/Makefile
index e39ca6c..ddfe7a1 100644
--- a/Makefile
+++ b/Makefile
@@ -1697,10 +1697,10 @@ $(BUILT_INS): git$X
ln -s $ $@ 2/dev/null || \
cp $ $@
 
-common-cmds.h: generate-cmdlist.perl command-list.txt
+common-cmds.h: generate-cmdlist.sh command-list.txt
 
 common-cmds.h: $(wildcard Documentation/git-*.txt)
-   $(QUIET_GEN)$(PERL_PATH) generate-cmdlist.perl command-list.txt  $@+ 
 mv $@+ $@
+   $(QUIET_GEN)./generate-cmdlist.sh command-list.txt $@+  mv $@+ $@
 
 SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\
$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
diff --git a/generate-cmdlist.perl b/generate-cmdlist.perl
deleted file mode 100755
index 31516e3..000
--- a/generate-cmdlist.perl
+++ /dev/null
@@ -1,50 +0,0 @@
-#!/usr/bin/perl
-use strict;
-use warnings;
-
-print EOT;
-/* Automatically generated by $0 */
-
-struct cmdname_help {
-   char name[16];
-   char help[80];
-   unsigned char group;
-};
-
-static char *common_cmd_groups[] = {
-EOT
-
-my $n = 0;
-my %grp;
-while () {
-   last if /^### command list/;
-   next if (1../^### common groups/) || /^#/ || /^\s*$/;
-   chop;
-   my ($k, $v) = split ' ', $_, 2;
-   $grp{$k} = $n++;
-   print \tN_(\$v\),\n;
-}
-
-print };\n\nstatic struct cmdname_help common_cmds[] = {\n;
-
-while () {
-   next if /^#/ || /^\s*$/;
-   my @tags = split;
-   my $cmd = shift @tags;
-   for my $t (@tags) {
-   if (exists $grp{$t}) {
-   my $s;
-   open my $f, '', Documentation/$cmd.txt or die;
-   while ($f) {
-   ($s) = /^$cmd - (.+)$/;
-   last if $s;
-   }
-   close $f;
-   $cmd =~ s/^git-//;
-   print \t{\$cmd\, N_(\$s\), $grp{$t}},\n;
-   last;
-   }
-   }
-}
-
-print };\n;
diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
new file mode 100755
index 000..1ac329d
--- /dev/null
+++ b/generate-cmdlist.sh
@@ -0,0 +1,50 @@
+#!/bin/sh
+
+echo /* Automatically generated by $0 */
+struct cmdname_help {
+   char name[16];
+   char help[80];
+   unsigned char group;
+};
+
+static const char *common_cmd_groups[] = {
+
+tmp=cmdgrps$$.tmp
+trap rm -fr $tmp 0 1 2 3 15
+
+sed -n '
+   1,/^### common groups/b
+   /^### command list/q
+   /^#/d; /^[  ]*$/b
+   h;s/^[^ ][^ ]*[ ][  ]*\(.*\)/   N_(\1),/p
+   g;s/^\([^   ][^ ]*\)[   ].*/\1/w '$tmp'
+   '
+printf '};\n\n'
+
+n=0
+matchgrp=
+substnum=
+while read grp
+do
+   matchgrp=$matchgrp${matchgrp:+
+}^git-..*[ ]$grp
+   substnum=$substnum${substnum:+;}s/[]$grp/$n/
+   n=$(($n+1))
+done $tmp
+
+printf 'static struct cmdname_help common_cmds[] = {\n'
+grep $matchgrp |
+sed 's/^git-//' |
+sort |
+while read cmd tags
+do
+   tag=$(echo $tags | sed $substnum; s/[^0-9]//g)
+   sed -n '
+   /^NAME/,/git-'$cmd'/H
+   ${
+   x
+   s/.*git-'$cmd' - \(.*\)/  {'$cmd', N_(\1), 
'$tag'},/
+   p
+   }' 

Re: [PATCH v2 2/9] Documentation/git-send-pack.txt: Flow long synopsis line

2015-08-19 Thread Dave Borowitz
On Wed, Aug 19, 2015 at 3:56 PM, Junio C Hamano gits...@pobox.com wrote:
 Dave Borowitz dborow...@google.com writes:

 Signed-off-by: Dave Borowitz dborow...@google.com
 ---
  Documentation/git-send-pack.txt | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

 diff --git a/Documentation/git-send-pack.txt 
 b/Documentation/git-send-pack.txt
 index b5d09f7..6a6 100644
 --- a/Documentation/git-send-pack.txt
 +++ b/Documentation/git-send-pack.txt
 @@ -9,7 +9,8 @@ git-send-pack - Push objects over Git protocol to another 
 repository
  SYNOPSIS
  
  [verse]
 -'git send-pack' [--all] [--dry-run] [--force]
 [--receive-pack=git-receive-pack] [--verbose] [--thin] [--atomic]
 [host:]directory [ref...]
 +'git send-pack' [--all] [--dry-run] [--force] 
 [--receive-pack=git-receive-pack]
 + [--verbose] [--thin] [--atomic] [host:]directory [ref...]

  DESCRIPTION
  ---

 As can be expected from the Subject: line, this patch is
 line-wrapped and does not apply ;-)

I produced the patch with git format-patch --subject-prefix='PATCH
v2' --cover-letter @{u}.. and mailed with git send-email
--to=git@vger.kernel.org,gits...@pobox.com 0*.patch; is there a way
that would have preserved whitespace better?

 I've done a trivial fix-up and took the liberty of making the result
 of this step into three lines, not two.  That would make 3/9 look
 more trivial.

Ok by me.

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


Re: [PATCH v2 8/9] Support signing pushes iff the server supports it

2015-08-19 Thread Dave Borowitz
On Wed, Aug 19, 2015 at 3:58 PM, Junio C Hamano gits...@pobox.com wrote:
 Dave Borowitz dborow...@google.com writes:

 Add a new flag --signed-if-possible to push and send-pack that sends a
 push certificate if and only if the server advertised a push cert
 nonce. If not, at least warn the user that their push may not be as
 secure as they thought.

 Signed-off-by: Dave Borowitz dborow...@google.com
 ---

 Obviously, the above description needs updating.  Here is what I've
 queued tentatively.

Sound good.

 Thanks.

 commit 32d273dfabb0a70b2839971f5afff7fa86a8f4c2
 Author: Dave Borowitz dborow...@google.com
 Date:   Wed Aug 19 11:26:46 2015 -0400

 push: support signing pushes iff the server supports it

 Add a new flag --sign=true (or --sign=false), which means the same
 thing as the original --signed (or --no-signed).  Give it a third
 value --sign=if-asked to tell push and send-pack to send a push
 certificate if and only if the server advertised a push cert nonce.

 If not, warn the user that their push may not be as secure as they
 thought.

 Signed-off-by: Dave Borowitz dborow...@google.com
 Signed-off-by: Junio C Hamano gits...@pobox.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


BUG REPORT: gitk displays diffs with angle brackets incorrectly

2015-08-19 Thread Yojem Mejoy
Summary: Certain diffs containing angle brackets ('' or '' characters) are 
displayed incorrectly in gitk's markup words and color words modes. The bug 
is caused by a feature that isn't aware of the formatting difference for the 
word diff modes.

How to reproduce the bug:
 
Create a file with the following, and commit:
x  1
y  2
 
Modify it to this, and commit:
z  1
w  2
 
Then view the results with:
gitk --color-words
 
You will see:
 * Diff shows bogus removal on 1st line and bogus addition on 2nd line.
 * Old version cuts off at right angle bracket.
 * New Version cuts off at left angle bracket.

I personally use a 2-line patch to fix this by disabling/breaking the feature 
that does string comparisons for    and   , but I know that's not the 
correct fix, so I'm not including it in this post.
--
To unsubscribe from this list: send the line 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 v12 03/13] ref-filter: introduce the ref_formatting_state stack machinery

2015-08-19 Thread Junio C Hamano
Matthieu Moy matthieu@grenoble-inp.fr writes:

 You replaced the quote_style argument with ref_formatting_state, and I
 think you should have kept this argument and added ref_formatting_state.
 The other option is to add an extra indirection like

 struct ref_formatting_state {
   int quote_style;
   struct ref_formatting_stack *stack;
 }

 (ref_formatting_stack would be what you currently call
 ref_formatting_state). But that's probably overkill.

I think this is the right way to go.  As you explained in your later
messages, this is conceptually a global setting that applies to
anybody working in the callchain and not something individual
recursion levels would want to muck with.

Thanks.

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


Re: [PATCH 7/7] submodule: implement `module_clone` as a builtin helper

2015-08-19 Thread Junio C Hamano
Stefan Beller sbel...@google.com writes:

 `module_clone` is part of the update command, which I want to convert
 to C next.

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

 This is the only patch that broke by a last minute fix,
 so this replaces the previous PATCH 7/7.

... which still breaks 7400.82, though.

Output from running it with -i -v ends like this:

expecting success: 
mkdir super 
pwd=$(pwd) 
(
cd super 
git init 
git submodule add --depth=1 file://$pwd/example2 submodule 
(
cd submodule 
test 1 = $(git log --oneline | wc -l)
)
)

Initialized empty Git repository in /home/gitster/w/git.git/t/trash 
directory.t7400-submodule-basic/super/.git/
fatal: Clone of 'file:///home/gitster/w/git.git/t/trash 
directory.t7400-submodule-basic/example2' into submodule path 'submodule' failed
not ok 82 - submodule add clone shallow submodule
#
#   mkdir super 
#   pwd=$(pwd) 
#   (
#   cd super 
#   git init 
#   git submodule add --depth=1 file://$pwd/example2 
submodule 
#   (
#   cd submodule 
#   test 1 = $(git log --oneline | wc -l)
#   )
#   )
#
--
To unsubscribe from this list: send the line 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/PATCH] t9350-fast-export: Add failing test for symlink-to-directory

2015-08-19 Thread Anders Kaseorg
git fast-export | git fast-import fails to preserve a commit that replaces 
a symlink with a directory.  Add a failing test case demonstrating this 
bug.

The fast-export output for the commit in question looks like

  commit refs/heads/master
  mark :4
  author …
  committer …
  data 4
  two
  M 100644 :1 foo/world
  D foo

fast-import deletes the symlink foo and ignores foo/world.  Swapping the M 
line with the D line would give the correct result.

Signed-off-by: Anders Kaseorg ande...@mit.edu
---
 t/t9350-fast-export.sh | 24 
 1 file changed, 24 insertions(+)

diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 66c8b0a..5fb8a04 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -419,6 +419,30 @@ test_expect_success 'directory becomes symlink''
(cd result  git show master:foo)
 '
 
+test_expect_failure 'symlink becomes directory''
+   git init symlinktodir 
+   git init symlinktodirresult 
+   (
+   cd symlinktodir 
+   mkdir bar 
+   echo hello  bar/world 
+   test_ln_s_add bar foo 
+   git add foo bar/world 
+   git commit -q -mone 
+   git rm foo 
+   mkdir foo 
+   echo hello  foo/world 
+   git add foo/world 
+   git commit -q -mtwo
+   ) 
+   (
+   cd symlinktodir 
+   git fast-export master -- foo |
+   (cd ../symlinktodirresult  git fast-import --quiet)
+   ) 
+   (cd symlinktodirresult  git show master:foo)
+'
+
 test_expect_success 'fast-export quotes pathnames' '
git init crazy-paths 
(cd crazy-paths 
-- 
2.5.0

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


Re: [PATCH v2 2/9] Documentation/git-send-pack.txt: Flow long synopsis line

2015-08-19 Thread Junio C Hamano
Dave Borowitz dborow...@google.com writes:

 I produced the patch with git format-patch --subject-prefix='PATCH
 v2' --cover-letter @{u}.. and mailed with git send-email
 --to=git@vger.kernel.org,gits...@pobox.com 0*.patch; is there a way
 that would have preserved whitespace better?

No need to worry, I suspect that this is a local Emacs/GNUS glitch
on the receiving end.  Sorry for a noise.
--
To unsubscribe from this list: send the line 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] generate-cmdlist: re-implement as shell script

2015-08-19 Thread Junio C Hamano
Eric Sunshine sunsh...@sunshineco.com writes:

 In addition to reviving 527ec39^:generate-cmdlist.sh and extending it, I
 also re-indented it with tabs instead of spaces, so it's helpful to
 ignore whitespace changes when comparing the old and new versions of the
 shell script.

Hmph.  Perhaps we can view it as part of reverting 527ec39 and then
redoing it in shell.  The way the shell script accumulates matchgrp
variable (i.e. the literal LF in ${var:+string}) makes me feel some
possible portability scare, which might be solved in a more stupid
(i.e. not giving various reimplementations of Bourne shells a chance
to screw it up) way by using another temporary file, but other than
that the resurrected script looks OK to me.

Thanks.

 diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
 new file mode 100755
 index 000..1ac329d
 --- /dev/null
 +++ b/generate-cmdlist.sh
 @@ -0,0 +1,50 @@
 +#!/bin/sh
 +
 +echo /* Automatically generated by $0 */
 +struct cmdname_help {
 + char name[16];
 + char help[80];
 + unsigned char group;
 +};
 +
 +static const char *common_cmd_groups[] = {
 +
 +tmp=cmdgrps$$.tmp
 +trap rm -fr $tmp 0 1 2 3 15
 +
 +sed -n '
 + 1,/^### common groups/b
 + /^### command list/q
 + /^#/d; /^[  ]*$/b
 + h;s/^[^ ][^ ]*[ ][  ]*\(.*\)/   N_(\1),/p
 + g;s/^\([^   ][^ ]*\)[   ].*/\1/w '$tmp'
 + '
 +printf '};\n\n'
 +
 +n=0
 +matchgrp=
 +substnum=
 +while read grp
 +do
 + matchgrp=$matchgrp${matchgrp:+
 +}^git-..*[   ]$grp
 + substnum=$substnum${substnum:+;}s/[]$grp/$n/
 + n=$(($n+1))
 +done $tmp
 +
 +printf 'static struct cmdname_help common_cmds[] = {\n'
 +grep $matchgrp |
 +sed 's/^git-//' |
 +sort |
 +while read cmd tags
 +do
 + tag=$(echo $tags | sed $substnum; s/[^0-9]//g)
 + sed -n '
 + /^NAME/,/git-'$cmd'/H
 + ${
 + x
 + s/.*git-'$cmd' - \(.*\)/  {'$cmd', N_(\1), 
 '$tag'},/
 + p
 + }' Documentation/git-$cmd.txt
 +done
 +echo };
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Minor bug with help.autocorrect.

2015-08-19 Thread Bjørnar Snoksrud
If you mis-type a git command starting with a non-letter, git
internals will spit out some errors at you.

$ git 5fetch
error: invalid key: pager.5fetch
error: invalid key: alias.5fetch
git: '5fetch' is not a git command. See 'git --help'.

Did you mean this?
fetch

$ git \#fetch
error: invalid key: pager.#fetch
error: invalid key: alias.#fetch
git: '#fetch' is not a git command. See 'git --help'.

Did you mean this?
fetch

-- 
bjor...@snoksrud.no
--
To unsubscribe from this list: send the line 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: Moved code detection with `git apply` a la `git blame -C -C`

2015-08-19 Thread Jacob Keller
Maybe something along the lines of a git-subtree merge. I am not sure
how to do that exactly, and I am not sure that it would be worth the
trouble to setup for a small case...

On Tue, Aug 18, 2015 at 1:11 PM, Anish Tondwalkar
tondwal...@virginia.edu wrote:
 I stashed some changes, then refactored my code to move the function
 they're in into a new file. Now, I want to apply it to the new file. I
 know git can figure out this relationship between the files, because
 `git blame -C -C` can find it, but is there a more idiomatic way to
 apply this diff than to have git spit out a diff, and edit it
 manually?

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


Re: [PATCH] git-am: add am.threeWay config variable

2015-08-19 Thread Matthieu Moy
Paul Tan pyoka...@gmail.com writes:

 On Tue, Aug 18, 2015 at 5:36 PM, Matthieu Moy
 matthieu@grenoble-inp.fr wrote:
 I don't remember the details of the regression we had with the shell
 version, but that would probably deserve an additional test to enforce
 the Hopefully there will be no regressions part of your message.

 Actually, technically, I think this patch by its own would reintroduce
 the regression ;)

 The reason is that the bug was caused by the overall structure of the
 git-am.sh code, and not the patch itself[1].

 This is fixed in another patch series[2] on top of this patch which
 also implements a test for git am --3way.

OK, perfect. I had missed that. Thanks again.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


No dialog box appears

2015-08-19 Thread Цапков Алексей
Hello.
When installing the Git is not a dialog box appears with a choice ssh. What 
could be the reason?

Здравствуйте.
При установке гита у меня не появляется диалоговое окно с выбором ssh. 
Пожалуйста, подскажите в чем может быть причина?

-- 
Shincho itte kudasai. Anime miteru hito!
--
To unsubscribe from this list: send the line 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] am --skip/--abort: merge HEAD/ORIG_HEAD tree into index

2015-08-19 Thread Paul Tan
On Mon, Aug 17, 2015 at 12:33:40PM -0700, Junio C Hamano wrote:
 Have you checked how this change affects that codepath?  To put it
 differently, does am skip have the same issue without this fix?

Hmm, I adopted Dscho's test to run git am --skip and it did not fail.
I think it's because am_skip() calls am_run(), which calls
refresh_cache(), so the resulting index will have the updated stat info.
However, there should still be a performance penalty because
refresh_cache() would have to scan all files for changes.

 If so, I wonder if we can have a test for that, too?

So yeah, we should have a test for that too.

(In addition, I fixed a small mistake with the struct tree_desc array
size.)

Thanks,
Paul

-- 8 --
Subject: [PATCH v2] am --skip/--abort: merge HEAD/ORIG_HEAD tree into index

After running git am --abort, and then running git reset --hard,
files that were not modified would still be re-checked out.

This is because clean_index() in builtin/am.c mistakenly called the
read_tree() function, which overwrites all entries in the index,
including the stat info.

git am --skip did not seem to have this issue because am_skip() called
am_run(), which called refresh_cache() to update the stat info. However,
there's still a performance penalty as the lack of stat info meant that
refresh_cache() would have to scan all files for changes.

Fix this by using unpack_trees() instead to merge the tree into the
index, so that the stat info from the index is kept.

Reported-by: Linus Torvalds torva...@linux-foundation.org
Helped-by: Junio C Hamano gits...@pobox.com
Signed-off-by: Johannes Schindelin johannes.schinde...@gmx.de
Signed-off-by: Paul Tan pyoka...@gmail.com
---
 builtin/am.c| 49 -
 t/t4151-am-abort.sh | 24 
 2 files changed, 60 insertions(+), 13 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 1399c8d..3e7e66f 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1940,15 +1940,48 @@ static int fast_forward_to(struct tree *head, struct 
tree *remote, int reset)
 }
 
 /**
+ * Merges a tree into the index. The index's stat info will take precedence
+ * over the merged tree's. Returns 0 on success, -1 on failure.
+ */
+static int merge_tree(struct tree *tree)
+{
+   struct lock_file *lock_file;
+   struct unpack_trees_options opts;
+   struct tree_desc t[1];
+
+   if (parse_tree(tree))
+   return -1;
+
+   lock_file = xcalloc(1, sizeof(struct lock_file));
+   hold_locked_index(lock_file, 1);
+
+   memset(opts, 0, sizeof(opts));
+   opts.head_idx = 1;
+   opts.src_index = the_index;
+   opts.dst_index = the_index;
+   opts.merge = 1;
+   opts.fn = oneway_merge;
+   init_tree_desc(t[0], tree-buffer, tree-size);
+
+   if (unpack_trees(1, t, opts)) {
+   rollback_lock_file(lock_file);
+   return -1;
+   }
+
+   if (write_locked_index(the_index, lock_file, COMMIT_LOCK))
+   die(_(unable to write new index file));
+
+   return 0;
+}
+
+/**
  * Clean the index without touching entries that are not modified between
  * `head` and `remote`.
  */
 static int clean_index(const unsigned char *head, const unsigned char *remote)
 {
-   struct lock_file *lock_file;
struct tree *head_tree, *remote_tree, *index_tree;
unsigned char index[GIT_SHA1_RAWSZ];
-   struct pathspec pathspec;
 
head_tree = parse_tree_indirect(head);
if (!head_tree)
@@ -1973,18 +2006,8 @@ static int clean_index(const unsigned char *head, const 
unsigned char *remote)
if (fast_forward_to(index_tree, remote_tree, 0))
return -1;
 
-   memset(pathspec, 0, sizeof(pathspec));
-
-   lock_file = xcalloc(1, sizeof(struct lock_file));
-   hold_locked_index(lock_file, 1);
-
-   if (read_tree(remote_tree, 0, pathspec)) {
-   rollback_lock_file(lock_file);
+   if (merge_tree(remote_tree))
return -1;
-   }
-
-   if (write_locked_index(the_index, lock_file, COMMIT_LOCK))
-   die(_(unable to write new index file));
 
remove_branch_state();
 
diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh
index 05bdc3e..ea5ace9 100755
--- a/t/t4151-am-abort.sh
+++ b/t/t4151-am-abort.sh
@@ -168,4 +168,28 @@ test_expect_success 'am --abort on unborn branch will keep 
local commits intact'
test_cmp expect actual
 '
 
+test_expect_success 'am --skip leaves index stat info alone' '
+   git checkout -f --orphan skip-stat-info 
+   git reset 
+   test_commit skip-should-be-untouched 
+   test-chmtime =0 skip-should-be-untouched.t 
+   git update-index --refresh 
+   git diff-files --exit-code --quiet 
+   test_must_fail git am 0001-*.patch 
+   git am --skip 
+   git diff-files --exit-code --quiet
+'
+
+test_expect_success 'am --abort leaves index stat info alone' '
+   git checkout -f --orphan 

Re: [PATCH] git-am: add am.threeWay config variable

2015-08-19 Thread Paul Tan
On Tue, Aug 18, 2015 at 5:36 PM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
 I don't remember the details of the regression we had with the shell
 version, but that would probably deserve an additional test to enforce
 the Hopefully there will be no regressions part of your message.

Actually, technically, I think this patch by its own would reintroduce
the regression ;)

The reason is that the bug was caused by the overall structure of the
git-am.sh code, and not the patch itself[1].

This is fixed in another patch series[2] on top of this patch which
also implements a test for git am --3way.

[1] http://thread.gmane.org/gmane.comp.version-control.git/274577
[2] http://thread.gmane.org/gmane.comp.version-control.git/275322

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