`git stash --help` tries to pull up nonexistent file gitstack.html

2016-08-11 Thread Joseph Musser
Looks like a simple typo.


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


[PATCHv2] checkout: do not mention detach advice for explicit --detach option

2016-08-11 Thread Stefan Beller
When a user asked for a detached HEAD specifically with `--detach`,
we do not need to give advice on what a detached HEAD state entails as
we can assume they know what they're getting into as they asked for it.

Signed-off-by: Stefan Beller 
---
 
 jrnieder wrote:
 > Examples?
 
 What do you mean by example?
  
 builtin/checkout.c |  2 +-
 t/t2020-checkout-detach.sh | 13 +
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 8d852d4..85408b1 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -658,7 +658,7 @@ static void update_refs_for_switch(const struct 
checkout_opts *opts,
update_ref(msg.buf, "HEAD", new->commit->object.oid.hash, NULL,
   REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);
if (!opts->quiet) {
-   if (old->path && advice_detached_head)
+   if (old->path && advice_detached_head && 
!opts->force_detach)
detach_advice(new->name);
describe_detached_head(_("HEAD is now at"), 
new->commit);
}
diff --git a/t/t2020-checkout-detach.sh b/t/t2020-checkout-detach.sh
index 5d68729..941ea49 100755
--- a/t/t2020-checkout-detach.sh
+++ b/t/t2020-checkout-detach.sh
@@ -163,4 +163,17 @@ test_expect_success 'tracking count is accurate after 
orphan check' '
test_i18ncmp expect stdout
 '
 
+test_expect_success 'no advice given for explicit detached head state' '
+   git config advice.detachedHead false &&
+   git checkout child &&
+   git checkout --detach HEAD >expect &&
+   git config advice.detachedHead true &&
+   git checkout child &&
+   git checkout --detach HEAD >actual &&
+   test_cmp expect actual &&
+   git checkout child &&
+   git checkout HEAD >actual &&
+   ! test_cmp expect actual
+'
+
 test_done
-- 
2.9.2.730.g46b112d

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


[PATCHv4 6/8] clone: clarify option_reference as required

2016-08-11 Thread Stefan Beller
In the next patch we introduce optional references; To better distinguish
between optional and required references we rename the variable.

Signed-off-by: Stefan Beller 
---
 builtin/clone.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 24b17539..7ccbdef 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -50,7 +50,7 @@ static int option_verbosity;
 static int option_progress = -1;
 static enum transport_family family;
 static struct string_list option_config = STRING_LIST_INIT_NODUP;
-static struct string_list option_reference = STRING_LIST_INIT_NODUP;
+static struct string_list option_required_reference = STRING_LIST_INIT_NODUP;
 static int option_dissociate;
 static int max_jobs = -1;
 
@@ -79,7 +79,7 @@ static struct option builtin_clone_options[] = {
N_("number of submodules cloned in parallel")),
OPT_STRING(0, "template", &option_template, N_("template-directory"),
   N_("directory from which templates will be used")),
-   OPT_STRING_LIST(0, "reference", &option_reference, N_("repo"),
+   OPT_STRING_LIST(0, "reference", &option_required_reference, N_("repo"),
N_("reference repository")),
OPT_BOOL(0, "dissociate", &option_dissociate,
 N_("use --reference only while cloning")),
@@ -297,7 +297,7 @@ static int add_one_reference(struct string_list_item *item, 
void *cb_data)
 
 static void setup_reference(void)
 {
-   for_each_string_list(&option_reference, add_one_reference, NULL);
+   for_each_string_list(&option_required_reference, add_one_reference, 
NULL);
 }
 
 static void copy_alternates(struct strbuf *src, struct strbuf *dst,
@@ -949,7 +949,7 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
git_config_set(key.buf, repo);
strbuf_reset(&key);
 
-   if (option_reference.nr)
+   if (option_required_reference.nr)
setup_reference();
 
fetch_pattern = value.buf;
-- 
2.9.2.737.g4a14654

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


[PATCHv4 7/8] clone: implement optional references

2016-08-11 Thread Stefan Beller
In a later patch we want to try to create alternates for submodules,
but they might not exist in the referenced superproject. So add a way
to skip the non existing references and report them.

Signed-off-by: Stefan Beller 
---
 Documentation/git-clone.txt |  5 -
 builtin/clone.c | 29 ++---
 2 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
index ec41d3d..e316c4b 100644
--- a/Documentation/git-clone.txt
+++ b/Documentation/git-clone.txt
@@ -90,13 +90,16 @@ If you want to break the dependency of a repository cloned 
with `-s` on
 its source repository, you can simply run `git repack -a` to copy all
 objects from the source repository into a pack in the cloned repository.
 
---reference ::
+--reference[-if-able] ::
If the reference repository is on the local machine,
automatically setup `.git/objects/info/alternates` to
obtain objects from the reference repository.  Using
an already existing repository as an alternate will
require fewer objects to be copied from the repository
being cloned, reducing network and local storage costs.
+   When using the `--reference-if-able`, a non existing
+   directory is skipped with a warning instead of aborting
+   the clone.
 +
 *NOTE*: see the NOTE for the `--shared` option, and also the
 `--dissociate` option.
diff --git a/builtin/clone.c b/builtin/clone.c
index 7ccbdef..fdb2ca9 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -51,6 +51,7 @@ static int option_progress = -1;
 static enum transport_family family;
 static struct string_list option_config = STRING_LIST_INIT_NODUP;
 static struct string_list option_required_reference = STRING_LIST_INIT_NODUP;
+static struct string_list option_optional_reference = STRING_LIST_INIT_NODUP;
 static int option_dissociate;
 static int max_jobs = -1;
 
@@ -81,6 +82,8 @@ static struct option builtin_clone_options[] = {
   N_("directory from which templates will be used")),
OPT_STRING_LIST(0, "reference", &option_required_reference, N_("repo"),
N_("reference repository")),
+   OPT_STRING_LIST(0, "reference-if-able", &option_optional_reference,
+   N_("repo"), N_("reference repository")),
OPT_BOOL(0, "dissociate", &option_dissociate,
 N_("use --reference only while cloning")),
OPT_STRING('o', "origin", &option_origin, N_("name"),
@@ -283,13 +286,20 @@ static void strip_trailing_slashes(char *dir)
 static int add_one_reference(struct string_list_item *item, void *cb_data)
 {
struct strbuf sb = STRBUF_INIT;
+   int *required = cb_data;
char *ref_git = compute_alternate_path(item->string, &sb);
 
-   if (!ref_git)
-   die("%s", sb.buf);
-
-   strbuf_addf(&sb, "%s/objects", ref_git);
-   add_to_alternates_file(sb.buf);
+   if (!ref_git) {
+   if (*required)
+   die("%s", sb.buf);
+   else
+   fprintf(stderr,
+   _("info: Could not add alternate for '%s': 
%s\n"),
+   item->string, sb.buf);
+   } else {
+   strbuf_addf(&sb, "%s/objects", ref_git);
+   add_to_alternates_file(sb.buf);
+   }
 
strbuf_release(&sb);
return 0;
@@ -297,7 +307,12 @@ static int add_one_reference(struct string_list_item 
*item, void *cb_data)
 
 static void setup_reference(void)
 {
-   for_each_string_list(&option_required_reference, add_one_reference, 
NULL);
+   int required = 1;
+   for_each_string_list(&option_required_reference,
+add_one_reference, &required);
+   required = 0;
+   for_each_string_list(&option_optional_reference,
+add_one_reference, &required);
 }
 
 static void copy_alternates(struct strbuf *src, struct strbuf *dst,
@@ -949,7 +964,7 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
git_config_set(key.buf, repo);
strbuf_reset(&key);
 
-   if (option_required_reference.nr)
+   if (option_required_reference.nr || option_optional_reference.nr)
setup_reference();
 
fetch_pattern = value.buf;
-- 
2.9.2.737.g4a14654

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


[PATCHv4 5/8] clone: factor out checking for an alternate path

2016-08-11 Thread Stefan Beller
In a later patch we want to determine if a path is suitable as an
alternate from other commands than builtin/clone. Move the checking
functionality of `add_one_reference` to `compute_alternate_path` that is
defined in cache.h.

Signed-off-by: Stefan Beller 
---
 builtin/clone.c | 42 ++--
 cache.h |  1 +
 sha1_file.c | 74 +
 3 files changed, 82 insertions(+), 35 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index f044a8c..24b17539 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -282,44 +282,16 @@ static void strip_trailing_slashes(char *dir)
 
 static int add_one_reference(struct string_list_item *item, void *cb_data)
 {
-   char *ref_git;
-   const char *repo;
-   struct strbuf alternate = STRBUF_INIT;
-
-   /* Beware: read_gitfile(), real_path() and mkpath() return static 
buffer */
-   ref_git = xstrdup(real_path(item->string));
-
-   repo = read_gitfile(ref_git);
-   if (!repo)
-   repo = read_gitfile(mkpath("%s/.git", ref_git));
-   if (repo) {
-   free(ref_git);
-   ref_git = xstrdup(repo);
-   }
-
-   if (!repo && is_directory(mkpath("%s/.git/objects", ref_git))) {
-   char *ref_git_git = mkpathdup("%s/.git", ref_git);
-   free(ref_git);
-   ref_git = ref_git_git;
-   } else if (!is_directory(mkpath("%s/objects", ref_git))) {
-   struct strbuf sb = STRBUF_INIT;
-   if (get_common_dir(&sb, ref_git))
-   die(_("reference repository '%s' as a linked checkout 
is not supported yet."),
-   item->string);
-   die(_("reference repository '%s' is not a local repository."),
-   item->string);
-   }
+   struct strbuf sb = STRBUF_INIT;
+   char *ref_git = compute_alternate_path(item->string, &sb);
 
-   if (!access(mkpath("%s/shallow", ref_git), F_OK))
-   die(_("reference repository '%s' is shallow"), item->string);
+   if (!ref_git)
+   die("%s", sb.buf);
 
-   if (!access(mkpath("%s/info/grafts", ref_git), F_OK))
-   die(_("reference repository '%s' is grafted"), item->string);
+   strbuf_addf(&sb, "%s/objects", ref_git);
+   add_to_alternates_file(sb.buf);
 
-   strbuf_addf(&alternate, "%s/objects", ref_git);
-   add_to_alternates_file(alternate.buf);
-   strbuf_release(&alternate);
-   free(ref_git);
+   strbuf_release(&sb);
return 0;
 }
 
diff --git a/cache.h b/cache.h
index 95a0bd3..35f41f7 100644
--- a/cache.h
+++ b/cache.h
@@ -1344,6 +1344,7 @@ extern struct alternate_object_database {
 } *alt_odb_list;
 extern void prepare_alt_odb(void);
 extern void read_info_alternates(const char * relative_base, int depth);
+extern char *compute_alternate_path(const char *path, struct strbuf *err);
 extern void add_to_alternates_file(const char *reference);
 typedef int alt_odb_fn(struct alternate_object_database *, void *);
 extern int foreach_alt_odb(alt_odb_fn, void*);
diff --git a/sha1_file.c b/sha1_file.c
index 02940f1..7351d8c 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -418,6 +418,80 @@ void add_to_alternates_file(const char *reference)
free(alts);
 }
 
+/*
+ * Compute the exact path an alternate is at and returns it. In case of
+ * error NULL is returned and the human readable error is added to `err`
+ * `path` may be relative and should point to $GITDIR.
+ * `err` must not be null.
+ */
+char *compute_alternate_path(const char *path, struct strbuf *err)
+{
+   char *ref_git = NULL;
+   const char *repo, *ref_git_s;
+   struct strbuf err_buf = STRBUF_INIT;
+
+   ref_git_s = real_path_if_valid(path);
+   if (!ref_git_s) {
+   strbuf_addf(&err_buf, _("path '%s' does not exist"), path);
+   goto out;
+   } else
+   /*
+* Beware: read_gitfile(), real_path() and mkpath()
+* return static buffer
+*/
+   ref_git = xstrdup(ref_git_s);
+
+   repo = read_gitfile(ref_git);
+   if (!repo)
+   repo = read_gitfile(mkpath("%s/.git", ref_git));
+   if (repo) {
+   free(ref_git);
+   ref_git = xstrdup(repo);
+   }
+
+   if (!repo && is_directory(mkpath("%s/.git/objects", ref_git))) {
+   char *ref_git_git = mkpathdup("%s/.git", ref_git);
+   free(ref_git);
+   ref_git = ref_git_git;
+   } else if (!is_directory(mkpath("%s/objects", ref_git))) {
+   struct strbuf sb = STRBUF_INIT;
+   if (get_common_dir(&sb, ref_git)) {
+   strbuf_addf(&err_buf,
+   _("reference repository '%s' as a linked "
+ "checkout is not supported yet."),
+   path);
+ 

[PATCHv4 8/8] clone: recursive and reference option triggers submodule alternates

2016-08-11 Thread Stefan Beller
When `--recursive` and `--reference` is given, it is reasonable to
expect that the submodules are created with references to the submodules
of the given alternate for the superproject.

  An initial attempt to do this was presented to the mailing list, which
  used flags that are passed around ("--super-reference") that instructed
  the submodule clone to look for a reference in the submodules of the
  referenced superproject. This is not well thought out, as any further
  `submodule update` should also respect the initial setup.

  When a new  submodule is added to the superproject and the alternate
  of the superproject does not know about that submodule yet, we rather
  error out informing the user instead of being unclear if we did or did
  not use a submodules alternate.

To solve this problem introduce new options that store the configuration
for what the user wanted originally.

Signed-off-by: Stefan Beller 
---
 Documentation/config.txt   | 12 ++
 builtin/clone.c| 19 +
 builtin/submodule--helper.c| 87 ++
 t/t7408-submodule-reference.sh | 43 +
 4 files changed, 161 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 0bcb679..e09d32c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2847,6 +2847,18 @@ submodule.fetchJobs::
in parallel. A value of 0 will give some reasonable default.
If unset, it defaults to 1.
 
+submodule.alternateLocation::
+   Specifies how the submodules obtain alternates when submodules are
+   cloned. Possible values are `no`, `superproject`.
+   By default `no` is assumed, which doesn't add references. When the
+   value is set to `superproject` the submodule to be cloned computes
+   its alternates location relative to the superprojects alternate.
+
+submodule.alternateErrorStrategy
+   Specifies how to treat errors with the alternates for a submodule
+   as computed via `submodule.alternateLocation`. Possible values are
+   `ignore`, `info`, `die`.
+
 tag.forceSignAnnotated::
A boolean to specify whether annotated tags created should be GPG 
signed.
If `--annotate` is specified on the command line, it takes
diff --git a/builtin/clone.c b/builtin/clone.c
index fdb2ca9..0593aee 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -944,6 +944,25 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
else
fprintf(stderr, _("Cloning into '%s'...\n"), dir);
}
+
+   if (option_recursive) {
+   if (option_required_reference.nr &&
+   option_optional_reference.nr)
+   die(_("clone --recursive is not compatible with "
+ "both --reference and --reference-if-able"));
+   else if (option_required_reference.nr) {
+   string_list_append(&option_config,
+   "submodule.alternateLocation=superproject");
+   string_list_append(&option_config,
+   "submodule.alternateErrorStrategy=die");
+   } else if (option_optional_reference.nr) {
+   string_list_append(&option_config,
+   "submodule.alternateLocation=superproject");
+   string_list_append(&option_config,
+   "submodule.alternateErrorStrategy=info");
+   }
+   }
+
init_db(option_template, INIT_DB_QUIET);
write_config(&option_config);
 
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index a10a777..a71c903 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -472,6 +472,90 @@ static int clone_submodule(const char *path, const char 
*gitdir, const char *url
return run_command(&cp);
 }
 
+struct submodule_alternate_setup {
+   const char *submodule_name;
+   enum SUBMODULE_ALTERNATE_ERROR_MODE {
+   SUBMODULE_ALTERNATE_ERROR_DIE,
+   SUBMODULE_ALTERNATE_ERROR_INFO,
+   SUBMODULE_ALTERNATE_ERROR_IGNORE
+   } error_mode;
+   struct string_list *reference;
+};
+#define SUBMODULE_ALTERNATE_SETUP_INIT { NULL, \
+   SUBMODULE_ALTERNATE_ERROR_IGNORE, NULL }
+
+int add_possible_reference_from_superproject(
+   struct alternate_object_database *alt, void *sas_cb)
+{
+   struct submodule_alternate_setup *sas = sas_cb;
+
+   /* directory name, minus trailing slash */
+   size_t namelen = alt->name - alt->base - 1;
+   struct strbuf name = STRBUF_INIT;
+   strbuf_add(&name, alt->base, namelen);
+
+   /*
+* If the alternate object store is another repository, try the
+* standard layout with .git/modules//objects
+*/
+   if (ends_with(name.buf, ".git/objects")) {
+   char *sm_alt

[PATCHv4 2/8] t7408: merge short tests, factor out testing method

2016-08-11 Thread Stefan Beller
Tests consisting of one line each can be consolidated to have fewer tests
to run as well as fewer lines of code.

When having just a few git commands, do not create a new shell but
use the -C flag in Git to execute in the correct directory.

Signed-off-by: Stefan Beller 
---
 t/t7408-submodule-reference.sh | 48 ++
 1 file changed, 25 insertions(+), 23 deletions(-)

diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh
index b84c6748..dff47af 100755
--- a/t/t7408-submodule-reference.sh
+++ b/t/t7408-submodule-reference.sh
@@ -8,6 +8,15 @@ test_description='test clone --reference'
 
 base_dir=$(pwd)
 
+test_alternate_is_used () {
+   alternates_file="$1" &&
+   working_dir="$2" &&
+   test_line_count = 1 "$alternates_file" &&
+   echo "0 objects, 0 kilobytes" >expect &&
+   git -C "$working_dir" count-objects >actual &&
+   test_cmp expect actual
+}
+
 test_expect_success 'preparing first repository' '
test_create_repo A &&
(
@@ -40,16 +49,14 @@ test_expect_success 'preparing superproject' '
)
 '
 
-test_expect_success 'submodule add --reference' '
+test_expect_success 'submodule add --reference uses alternates' '
(
cd super &&
git submodule add --reference ../B "file://$base_dir/A" sub &&
-   git commit -m B-super-added
-   )
-'
-
-test_expect_success 'after add: existence of info/alternates' '
-   test_line_count = 1 super/.git/modules/sub/objects/info/alternates
+   git commit -m B-super-added &&
+   git repack -ad
+   ) &&
+   test_alternate_is_used super/.git/modules/sub/objects/info/alternates 
super/sub
 '
 
 test_expect_success 'that reference gets used with add' '
@@ -61,23 +68,18 @@ test_expect_success 'that reference gets used with add' '
)
 '
 
-test_expect_success 'cloning superproject' '
-   git clone super super-clone
-'
-
-test_expect_success 'update with reference' '
-   cd super-clone && git submodule update --init --reference ../B
-'
-
-test_expect_success 'after update: existence of info/alternates' '
-   test_line_count = 1 super-clone/.git/modules/sub/objects/info/alternates
-'
+# The tests up to this point, and repositories created by them
+# (A, B, super and super/sub), are about setting up the stage
+# for subsequent tests and meant to be kept throughout the
+# remainder of the test.
+# Tests from here on, if they create their own test repository,
+# are expected to clean after themselves.
 
-test_expect_success 'that reference gets used with update' '
-   cd super-clone/sub &&
-   echo "0 objects, 0 kilobytes" >expected &&
-   git count-objects >current &&
-   diff expected current
+test_expect_success 'updating superproject keeps alternates' '
+   test_when_finished "rm -rf super-clone" &&
+   git clone super super-clone &&
+   git -C super-clone submodule update --init --reference ../B &&
+   test_alternate_is_used 
super-clone/.git/modules/sub/objects/info/alternates super-clone/sub
 '
 
 test_done
-- 
2.9.2.737.g4a14654

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


[PATCHv4 3/8] submodule--helper module-clone: allow multiple references

2016-08-11 Thread Stefan Beller
Allow users to pass in multiple references, just as clone accepts multiple
references as well.

Signed-off-by: Stefan Beller 
---
 builtin/submodule--helper.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index b09632e..db9270e 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -442,7 +442,7 @@ static int module_name(int argc, const char **argv, const 
char *prefix)
 }
 
 static int clone_submodule(const char *path, const char *gitdir, const char 
*url,
-  const char *depth, const char *reference, int quiet)
+  const char *depth, struct string_list *reference, 
int quiet)
 {
struct child_process cp;
child_process_init(&cp);
@@ -453,8 +453,12 @@ static int clone_submodule(const char *path, const char 
*gitdir, const char *url
argv_array_push(&cp.args, "--quiet");
if (depth && *depth)
argv_array_pushl(&cp.args, "--depth", depth, NULL);
-   if (reference && *reference)
-   argv_array_pushl(&cp.args, "--reference", reference, NULL);
+   if (reference->nr) {
+   struct string_list_item *item;
+   for_each_string_list_item(item, reference)
+   argv_array_pushl(&cp.args, "--reference",
+item->string, NULL);
+   }
if (gitdir && *gitdir)
argv_array_pushl(&cp.args, "--separate-git-dir", gitdir, NULL);
 
@@ -470,13 +474,13 @@ static int clone_submodule(const char *path, const char 
*gitdir, const char *url
 
 static int module_clone(int argc, const char **argv, const char *prefix)
 {
-   const char *name = NULL, *url = NULL;
-   const char *reference = NULL, *depth = NULL;
+   const char *name = NULL, *url = NULL, *depth = NULL;
int quiet = 0;
FILE *submodule_dot_git;
char *p, *path = NULL, *sm_gitdir;
struct strbuf rel_path = STRBUF_INIT;
struct strbuf sb = STRBUF_INIT;
+   struct string_list reference = STRING_LIST_INIT_NODUP;
 
struct option module_clone_options[] = {
OPT_STRING(0, "prefix", &prefix,
@@ -491,8 +495,8 @@ static int module_clone(int argc, const char **argv, const 
char *prefix)
OPT_STRING(0, "url", &url,
   N_("string"),
   N_("url where to clone the submodule from")),
-   OPT_STRING(0, "reference", &reference,
-  N_("string"),
+   OPT_STRING_LIST(0, "reference", &reference,
+  N_("repo"),
   N_("reference repository")),
OPT_STRING(0, "depth", &depth,
   N_("string"),
@@ -528,7 +532,7 @@ static int module_clone(int argc, const char **argv, const 
char *prefix)
if (!file_exists(sm_gitdir)) {
if (safe_create_leading_directories_const(sm_gitdir) < 0)
die(_("could not create directory '%s'"), sm_gitdir);
-   if (clone_submodule(path, sm_gitdir, url, depth, reference, 
quiet))
+   if (clone_submodule(path, sm_gitdir, url, depth, &reference, 
quiet))
die(_("clone of '%s' into submodule path '%s' failed"),
url, path);
} else {
-- 
2.9.2.737.g4a14654

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


[PATCHv4 4/8] submodule--helper update-clone: allow multiple references

2016-08-11 Thread Stefan Beller
Allow the user to pass in multiple references to update_clone.
Currently this is only internal API, but once the shell script is
replaced by a C version, this is needed.

This fixes an API bug between the shell script and the helper.
Currently the helper accepts "--reference" "--reference=foo"
as a OPT_STRING whose value happens to be "--reference=foo", and
then uses

if (suc->reference)
argv_array_push(&child->args, suc->reference)

where suc->reference _is_ "--reference=foo" when invoking the
underlying "git clone", it cancels out.

With this change we omit one of the "--reference" arguments when
passing references from the shell script to the helper.

Signed-off-by: Stefan Beller 
---
 builtin/submodule--helper.c | 14 +-
 git-submodule.sh|  2 +-
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index db9270e..a10a777 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -584,7 +584,7 @@ struct submodule_update_clone {
/* configuration parameters which are passed on to the children */
int quiet;
int recommend_shallow;
-   const char *reference;
+   struct string_list references;
const char *depth;
const char *recursive_prefix;
const char *prefix;
@@ -600,7 +600,8 @@ struct submodule_update_clone {
int failed_clones_nr, failed_clones_alloc;
 };
 #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \
-   SUBMODULE_UPDATE_STRATEGY_INIT, 0, -1, NULL, NULL, NULL, NULL, \
+   SUBMODULE_UPDATE_STRATEGY_INIT, 0, -1, STRING_LIST_INIT_DUP, \
+   NULL, NULL, NULL, \
STRING_LIST_INIT_DUP, 0, NULL, 0, 0}
 
 
@@ -710,8 +711,11 @@ static int prepare_to_clone_next_submodule(const struct 
cache_entry *ce,
argv_array_pushl(&child->args, "--path", sub->path, NULL);
argv_array_pushl(&child->args, "--name", sub->name, NULL);
argv_array_pushl(&child->args, "--url", url, NULL);
-   if (suc->reference)
-   argv_array_push(&child->args, suc->reference);
+   if (suc->references.nr) {
+   struct string_list_item *item;
+   for_each_string_list_item(item, &suc->references)
+   argv_array_pushl(&child->args, "--reference", 
item->string, NULL);
+   }
if (suc->depth)
argv_array_push(&child->args, suc->depth);
 
@@ -830,7 +834,7 @@ static int update_clone(int argc, const char **argv, const 
char *prefix)
OPT_STRING(0, "update", &update,
   N_("string"),
   N_("rebase, merge, checkout or none")),
-   OPT_STRING(0, "reference", &suc.reference, N_("repo"),
+   OPT_STRING_LIST(0, "reference", &suc.references, N_("repo"),
   N_("reference repository")),
OPT_STRING(0, "depth", &suc.depth, "",
   N_("Create a shallow clone truncated to the "
diff --git a/git-submodule.sh b/git-submodule.sh
index b57f87d..a1cc71b 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -576,7 +576,7 @@ cmd_update()
${wt_prefix:+--prefix "$wt_prefix"} \
${prefix:+--recursive-prefix "$prefix"} \
${update:+--update "$update"} \
-   ${reference:+--reference "$reference"} \
+   ${reference:+"$reference"} \
${depth:+--depth "$depth"} \
${recommend_shallow:+"$recommend_shallow"} \
${jobs:+$jobs} \
-- 
2.9.2.737.g4a14654

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


[PATCHv4 0/6] git clone: Marry --recursive and --reference

2016-08-11 Thread Stefan Beller
v4:
Thanks to Junios critial questions regarding the design, I took a step back
to look at the bigger picture, again.

new patches:
  clone: factor out checking for an alternate path
  clone: recursive and reference option triggers submodule alternates
  
The last patch redesigns completely how we approach the problem.
Now there are no new command line options (that relate to the problem
of marrying --recursive and --reference), but instead we communicate
everything via configuration options to have a lasting effect (i.e.
submodule update remembers the decision of the initial setup)

Thanks,
Stefan

v3:

Thanks to Junios critial questions regarding the design, I took a step back
to look at the bigger picture. 

--super-reference sounds confusing. (what is the super referring to?)
So drop that approach.

Instead we'll compute where the reference might be in the superproject scope
and ask the submodule clone operation to consider an optional reference.
If the referenced alternate is not there, we'll just warn about it and
carry on.


* fixed the style in patch 2.

* fixed another bug in the last patch, that is unrelated, but would have helped
  me a lot.

Thanks,
Stefan

 Documentation/git-clone.txt|   9 -
 builtin/clone.c|  36 
 builtin/submodule--helper.c| 105 
-
 git-submodule.sh   |   2 +-
 t/t7408-submodule-reference.sh | 162 
--
 5 files changed, 217 insertions(+), 97 deletions(-)

v2:
 * fixed the p1,2 cleanups
 * added documentation to patches 5,6
 * improved commit message in v4
 
 Thanks,
 Stefan
 
v1:
 
 Currently when cloning a superproject with --recursive and --reference
 only the superproject learns about its alternates. The submodules are
 cloned independently, which may incur lots of network costs.
 
 Assume that the reference repository has the submodules at the same
 paths as the to-be-cloned submodule and try to setup alternates from
 there.
 
 Some submodules in the referenced superproject may not be there, 
 (they are just not initialized/cloned/checked out), which yields
 an error for now. In future work we may want to soften the alternate
 check and not die in the clone when one of the given alternates doesn't
 exist.
 
 patch 1,2 are modernizing style of t7408, 
 patches 3,4 are not strictly necessary, but I think it is a good thing
 to not leave the submodule related C code in a crippled state (i.e.
 allowing only one reference). The shell code would also need this update,
 but it looked ugly to me, so I postpone it until more of the submodule code
 is written in C. 
 
 Thanks,
 Stefan 

Stefan Beller (6):
  t7408: modernize style
  t7408: merge short tests, factor out testing method
  submodule--helper module-clone: allow multiple references
  submodule--helper update-clone: allow multiple references
  submodule update: add super-reference flag
  clone: reference flag is used for submodules as well

 builtin/clone.c|  22 --
 builtin/submodule--helper.c|  45 
 git-submodule.sh   |  12 +++-
 t/t7408-submodule-reference.sh | 153 +++--
 4 files changed, 147 insertions(+), 85 deletions(-)

-- 
2.9.2.572.g9d9644e.dirty

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


[PATCHv4 1/8] t7408: modernize style

2016-08-11 Thread Stefan Beller
No functional change intended. This commit only changes formatting
to the style we recently use, e.g. starting the body of a test with a
single quote on the same line as the header, and then having the test
indented in the following lines.

Whenever we change directories, we do that in subshells.

Signed-off-by: Stefan Beller 
---
 t/t7408-submodule-reference.sh | 140 +
 1 file changed, 71 insertions(+), 69 deletions(-)

diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh
index eaea19b..b84c6748 100755
--- a/t/t7408-submodule-reference.sh
+++ b/t/t7408-submodule-reference.sh
@@ -8,74 +8,76 @@ test_description='test clone --reference'
 
 base_dir=$(pwd)
 
-U=$base_dir/UPLOAD_LOG
-
-test_expect_success 'preparing first repository' \
-'test_create_repo A && cd A &&
-echo first > file1 &&
-git add file1 &&
-git commit -m A-initial'
-
-cd "$base_dir"
-
-test_expect_success 'preparing second repository' \
-'git clone A B && cd B &&
-echo second > file2 &&
-git add file2 &&
-git commit -m B-addition &&
-git repack -a -d &&
-git prune'
-
-cd "$base_dir"
-
-test_expect_success 'preparing superproject' \
-'test_create_repo super && cd super &&
-echo file > file &&
-git add file &&
-git commit -m B-super-initial'
-
-cd "$base_dir"
-
-test_expect_success 'submodule add --reference' \
-'cd super && git submodule add --reference ../B "file://$base_dir/A" sub &&
-git commit -m B-super-added'
-
-cd "$base_dir"
-
-test_expect_success 'after add: existence of info/alternates' \
-'test_line_count = 1 super/.git/modules/sub/objects/info/alternates'
-
-cd "$base_dir"
-
-test_expect_success 'that reference gets used with add' \
-'cd super/sub &&
-echo "0 objects, 0 kilobytes" > expected &&
-git count-objects > current &&
-diff expected current'
-
-cd "$base_dir"
-
-test_expect_success 'cloning superproject' \
-'git clone super super-clone'
-
-cd "$base_dir"
-
-test_expect_success 'update with reference' \
-'cd super-clone && git submodule update --init --reference ../B'
-
-cd "$base_dir"
-
-test_expect_success 'after update: existence of info/alternates' \
-'test_line_count = 1 super-clone/.git/modules/sub/objects/info/alternates'
-
-cd "$base_dir"
-
-test_expect_success 'that reference gets used with update' \
-'cd super-clone/sub &&
-echo "0 objects, 0 kilobytes" > expected &&
-git count-objects > current &&
-diff expected current'
-
-cd "$base_dir"
+test_expect_success 'preparing first repository' '
+   test_create_repo A &&
+   (
+   cd A &&
+   echo first >file1 &&
+   git add file1 &&
+   git commit -m A-initial
+   )
+'
+
+test_expect_success 'preparing second repository' '
+   git clone A B &&
+   (
+   cd B &&
+   echo second >file2 &&
+   git add file2 &&
+   git commit -m B-addition &&
+   git repack -a -d &&
+   git prune
+   )
+'
+
+test_expect_success 'preparing superproject' '
+   test_create_repo super &&
+   (
+   cd super &&
+   echo file >file &&
+   git add file &&
+   git commit -m B-super-initial
+   )
+'
+
+test_expect_success 'submodule add --reference' '
+   (
+   cd super &&
+   git submodule add --reference ../B "file://$base_dir/A" sub &&
+   git commit -m B-super-added
+   )
+'
+
+test_expect_success 'after add: existence of info/alternates' '
+   test_line_count = 1 super/.git/modules/sub/objects/info/alternates
+'
+
+test_expect_success 'that reference gets used with add' '
+   (
+   cd super/sub &&
+   echo "0 objects, 0 kilobytes" >expected &&
+   git count-objects >current &&
+   diff expected current
+   )
+'
+
+test_expect_success 'cloning superproject' '
+   git clone super super-clone
+'
+
+test_expect_success 'update with reference' '
+   cd super-clone && git submodule update --init --reference ../B
+'
+
+test_expect_success 'after update: existence of info/alternates' '
+   test_line_count = 1 super-clone/.git/modules/sub/objects/info/alternates
+'
+
+test_expect_success 'that reference gets used with update' '
+   cd super-clone/sub &&
+   echo "0 objects, 0 kilobytes" >expected &&
+   git count-objects >current &&
+   diff expected current
+'
 
 test_done
-- 
2.9.2.737.g4a14654

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


[PATCH v5 00/12] Update git revisions

2016-08-11 Thread Philip Oakley
This has grown like topsy from a little two patch series that tried to
name the 2-dots notation [1] into this extended set of tweaks.

As documentation can be rather personal, I've split out each small change
so each can be individually justified.

Since V4, I've confirmed that the format breaking issue is that we cannot
quote code in headings in the man page layout - ultimately it's a docbook
decision, and follows the line of analysis Peff identified (see commentary
in the patch).

In addition the multi-parent notations have been clarified and extended.

Thus the old patch 4 has been split into three. The first three patches are
unchanged. The following 4 patches are also unchanged.

Finally, at the end an extra 2 patches are added to build up the examples by
including details of the notation expansions.

This updates po/range-doc (2016-07-20) 8 commits.

Hopefully my updated workflow will get the right patches to the right people.

[1] 
https://public-inbox.org/git/0648000B273C412AB7140AE959EBC99A%40PhilipOakley/

Philip Oakley (12):
  doc: use 'symmetric difference' consistently
  doc: revisions - name the left and right sides
  doc: show the actual left, right, and boundary marks
  doc: revisions: give headings for the two and three dot notations
  doc: revisions: extra clarification of ^! notation effects
  doc: revisions: single vs multi-parent notation comparison
  doc: gitrevisions - use 'reachable' in page description
  doc: gitrevisions - clarify 'latter case' is revision walk
  doc: revisions  - define `reachable`
  doc: revisions - clarify reachability examples
  doc: revisions: show revision expansion in examples
  doc: revisions: sort examples and fix alignment of the unchanged

 Documentation/gitk.txt |   2 +-
 Documentation/gitrevisions.txt |   6 +-
 Documentation/pretty-formats.txt   |   2 +-
 Documentation/rev-list-options.txt |   4 +-
 Documentation/revisions.txt| 121 +++--
 5 files changed, 84 insertions(+), 51 deletions(-)

-- 
2.9.0.windows.1

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


[PATCH v5 01/12] doc: use 'symmetric difference' consistently

2016-08-11 Thread Philip Oakley
Signed-off-by: Philip Oakley 
---
unchanged
---
 Documentation/gitk.txt | 2 +-
 Documentation/rev-list-options.txt | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt
index 6ade002..6c3eb15 100644
--- a/Documentation/gitk.txt
+++ b/Documentation/gitk.txt
@@ -70,7 +70,7 @@ linkgit:git-rev-list[1] for a complete list.
 
 --left-right::
 
-   Mark which side of a symmetric diff a commit is reachable
+   Mark which side of a symmetric difference a commit is reachable
from.  Commits from the left side are prefixed with a `<`
symbol and those from the right with a `>` symbol.
 
diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 4f009d4..6dc0bb0 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -225,7 +225,7 @@ excluded from the output.
 
 --left-only::
 --right-only::
-   List only commits on the respective side of a symmetric range,
+   List only commits on the respective side of a symmetric difference,
i.e. only those which would be marked `<` resp. `>` by
`--left-right`.
 +
@@ -766,7 +766,7 @@ ifdef::git-rev-list[]
 endif::git-rev-list[]
 
 --left-right::
-   Mark which side of a symmetric diff a commit is reachable from.
+   Mark which side of a symmetric difference a commit is reachable from.
Commits from the left side are prefixed with `<` and those from
the right with `>`.  If combined with `--boundary`, those
commits are prefixed with `-`.
-- 
2.9.0.windows.1

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


Re: [PATCH v5 00/12] Update git revisions

2016-08-11 Thread Philip Oakley
I'm having trouble with my ISP is not playing ball. only cover and patch 1 
were sent.


Will retry with alternate service.

Philip
- Original Message - 
From: "Philip Oakley" 

Sent: Thursday, August 11, 2016 10:50 PM
Subject: [PATCH v5 00/12] Update git revisions



This has grown like topsy from a little two patch series that tried to
name the 2-dots notation [1] into this extended set of tweaks.

As documentation can be rather personal, I've split out each small change
so each can be individually justified.

Since V4, I've confirmed that the format breaking issue is that we cannot
quote code in headings in the man page layout - ultimately it's a docbook
decision, and follows the line of analysis Peff identified (see commentary
in the patch).

In addition the multi-parent notations have been clarified and extended.

Thus the old patch 4 has been split into three. The first three patches 
are

unchanged. The following 4 patches are also unchanged.

Finally, at the end an extra 2 patches are added to build up the examples 
by

including details of the notation expansions.

This updates po/range-doc (2016-07-20) 8 commits.

Hopefully my updated workflow will get the right patches to the right 
people.


[1] 
https://public-inbox.org/git/0648000B273C412AB7140AE959EBC99A%40PhilipOakley/


Philip Oakley (12):
 doc: use 'symmetric difference' consistently
 doc: revisions - name the left and right sides
 doc: show the actual left, right, and boundary marks
 doc: revisions: give headings for the two and three dot notations
 doc: revisions: extra clarification of ^! notation effects
 doc: revisions: single vs multi-parent notation comparison
 doc: gitrevisions - use 'reachable' in page description
 doc: gitrevisions - clarify 'latter case' is revision walk
 doc: revisions  - define `reachable`
 doc: revisions - clarify reachability examples
 doc: revisions: show revision expansion in examples
 doc: revisions: sort examples and fix alignment of the unchanged

Documentation/gitk.txt |   2 +-
Documentation/gitrevisions.txt |   6 +-
Documentation/pretty-formats.txt   |   2 +-
Documentation/rev-list-options.txt |   4 +-
Documentation/revisions.txt| 121 
+++--

5 files changed, 84 insertions(+), 51 deletions(-)

--
2.9.0.windows.1





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


[PATCH v5 2/3] diff: add --diff-line-prefix option for passing in a prefix

2016-08-11 Thread Jacob Keller
From: Jacob Keller 

Add an option to pass additional prefix to be displayed before diff
output. This feature will be used in a following patch to output correct
--graph prefix when using a child_process/run_command interface for
submodules.

The prefix shall come first prior to any other prefix associated with
the --graph option or other source.

Add tests for the same.

Signed-off-by: Jacob Keller 
---
- v5
* Changed name to --diff-line-prefix since --line-prefix might indicate
  for other commands such as log, when it only modifies diff output

 Documentation/diff-options.txt |  3 +++
 diff.c | 16 ++--
 diff.h |  1 +
 t/t4013-diff-various.sh|  6 +
 ...diff_--diff-line-prefix=-->_master_master^_side | 29 ++
 .../diff.diff_--diff-line-prefix_--cached_--_file0 | 15 +++
 6 files changed, 68 insertions(+), 2 deletions(-)
 create mode 100644 t/t4013/diff.diff_--diff-line-prefix=-->_master_master^_side
 create mode 100644 t/t4013/diff.diff_--diff-line-prefix_--cached_--_file0

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 705a87394200..f924f57d4f62 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -569,5 +569,8 @@ endif::git-format-patch[]
 --no-prefix::
Do not show any source or destination prefix.
 
+--diff-line-prefix=::
+   Prepend an additional prefix to every line of diff output.
+
 For more detailed explanation on these common options, see also
 linkgit:gitdiffcore[7].
diff --git a/diff.c b/diff.c
index ae069c303077..73dda58c440c 100644
--- a/diff.c
+++ b/diff.c
@@ -1167,10 +1167,18 @@ const char *diff_get_color(int diff_use_color, enum 
color_diff ix)
 const char *diff_line_prefix(struct diff_options *opt)
 {
struct strbuf *msgbuf;
-   if (!opt->output_prefix)
-   return "";
+
+   if (!opt->output_prefix) {
+   if (opt->line_prefix)
+   return opt->line_prefix;
+   else
+   return "";
+   }
 
msgbuf = opt->output_prefix(opt, opt->output_prefix_data);
+   /* line prefix must be printed before the output_prefix() */
+   if (opt->line_prefix)
+   strbuf_insert(msgbuf, 0, opt->line_prefix, 
strlen(opt->line_prefix));
return msgbuf->buf;
 }
 
@@ -3966,6 +3974,10 @@ int diff_opt_parse(struct diff_options *options,
options->a_prefix = optarg;
return argcount;
}
+   else if ((argcount = parse_long_opt("diff-line-prefix", av, &optarg))) {
+   options->line_prefix = optarg;
+   return argcount;
+   }
else if ((argcount = parse_long_opt("dst-prefix", av, &optarg))) {
options->b_prefix = optarg;
return argcount;
diff --git a/diff.h b/diff.h
index 49e4aaafb2da..83d0b1ae8580 100644
--- a/diff.h
+++ b/diff.h
@@ -115,6 +115,7 @@ struct diff_options {
const char *pickaxe;
const char *single_follow;
const char *a_prefix, *b_prefix;
+   const char *line_prefix;
unsigned flags;
unsigned touched_flags;
 
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 94ef5000e787..5204645eb92b 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -306,6 +306,8 @@ diff --no-index --name-status dir2 dir
 diff --no-index --name-status -- dir2 dir
 diff --no-index dir dir3
 diff master master^ side
+# Can't use spaces...
+diff --diff-line-prefix=--> master master^ side
 diff --dirstat master~1 master~2
 diff --dirstat initial rearrange
 diff --dirstat-by-file initial rearrange
@@ -325,6 +327,10 @@ test_expect_success 'diff --cached -- file on unborn 
branch' '
git diff --cached -- file0 >result &&
test_cmp "$TEST_DIRECTORY/t4013/diff.diff_--cached_--_file0" result
 '
+test_expect_success 'diff --diff-line-prefix with spaces' '
+   git diff --diff-line-prefix="| | | " --cached -- file0 >result &&
+   test_cmp 
"$TEST_DIRECTORY/t4013/diff.diff_--diff-line-prefix_--cached_--_file0" result
+'
 
 test_expect_success 'diff-tree --stdin with log formatting' '
cat >expect <<-\EOF &&
diff --git a/t/t4013/diff.diff_--diff-line-prefix=-->_master_master^_side 
b/t/t4013/diff.diff_--diff-line-prefix=-->_master_master^_side
new file mode 100644
index ..5cc90f27c2d9
--- /dev/null
+++ b/t/t4013/diff.diff_--diff-line-prefix=-->_master_master^_side
@@ -0,0 +1,29 @@
+$ git diff --diff-line-prefix=--> master master^ side
+-->diff --cc dir/sub
+-->index cead32e,7289e35..992913c
+-->--- a/dir/sub
+-->+++ b/dir/sub
+-->@@@ -1,6 -1,4 +1,8 @@@
+-->  A
+-->  B
+--> +C
+--> +D
+--> +E
+--> +F
+-->+ 1
+-->+ 2
+-->diff --cc file0
+-->index b414108,f4615da..10a8a9f
+-->--- a/file0
+-->+++ b/file0
+-->@@@ -1,6 -1,6 +1,9 @@@
+-->  1
+-->  2
+-->  3
+--> +4
+

[PATCH v5 3/3] diff: add SUBMODULE_DIFF format to display submodule diff

2016-08-11 Thread Jacob Keller
From: Jacob Keller 

Teach git-diff and friends a new format for displaying the difference of
a submodule using git-diff inside the submodule project. This allows
users to easily see exactly what source changed in a given commit that
updates the submodule pointer. To do this, remove DIFF_SUBMODULE_LOG bit
from the diff options, and instead add a new enum type for these
formats.

Add some tests for the new format and option.

Signed-off-by: Jacob Keller 
---
Major changes in v5 after adding tests, I will try to give an overview.
Unfortunately an interdiff is not easy as I do not remember which reflog
entry was v4 and it's been many edits, so I haven't bothered to dig
enough yet.

- v5
* reword "can be tweaked" text in the diff options
* remove unused variables
* print the submodule contains untracked or modified header piece
* print (new submodule) and (deleted submodule)
* handle case where the submodule is not initialized at all, and thus
  has no .git/modules/xyz, making a diff impossible
* handle the git directory better, plus comments explaining
* handle modified files better. Untracked unfortunately is more
  difficult. Suggestions for this welcome.
* handle null sha1 correctly (replace with empty tree)

I also added tests, mostly for diff output. These should be viewed
carefully since it's possible I didn't check the diff output very
carefully. Also, I haven't yet added a test for --graph -p mode yet, as
I haven't found a suitable place for it to be located.

 Documentation/diff-config.txt|   3 +-
 Documentation/diff-options.txt   |   7 +-
 diff.c   |  41 +-
 diff.h   |   9 +-
 submodule.c  | 130 +
 submodule.h  |   6 +
 t/t4059-diff-submodule-option-diff-format.sh | 738 +++
 7 files changed, 915 insertions(+), 19 deletions(-)
 create mode 100755 t/t4059-diff-submodule-option-diff-format.sh

diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index d5a5b17d5088..f5d693afad6c 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -123,7 +123,8 @@ diff.suppressBlankEmpty::
 diff.submodule::
Specify the format in which differences in submodules are
shown.  The "log" format lists the commits in the range like
-   linkgit:git-submodule[1] `summary` does.  The "short" format
+   linkgit:git-submodule[1] `summary` does.  The "diff" format shows the
+   diff between the beginning and end of the range. The "short" format
format just shows the names of the commits at the beginning
and end of the range.  Defaults to short.
 
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index f924f57d4f62..9a8e86ba1477 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -215,8 +215,11 @@ any of those replacements occurred.
the commits in the range like linkgit:git-submodule[1] `summary` does.
Omitting the `--submodule` option or specifying `--submodule=short`,
uses the 'short' format. This format just shows the names of the commits
-   at the beginning and end of the range.  Can be tweaked via the
-   `diff.submodule` configuration variable.
+   at the beginning and end of the range. When `--submodule=diff` is
+   given, the 'diff' format is used. This format shows the diff between
+   the old and new submodule commmit from the perspective of the
+   submodule.  Defaults to `diff.submodule` or 'short' if the config
+   option is unset.
 
 --color[=]::
Show colored diff.
diff --git a/diff.c b/diff.c
index 73dda58c440c..660fdb4cfe46 100644
--- a/diff.c
+++ b/diff.c
@@ -131,9 +131,11 @@ static int parse_dirstat_params(struct diff_options 
*options, const char *params
 static int parse_submodule_params(struct diff_options *options, const char 
*value)
 {
if (!strcmp(value, "log"))
-   DIFF_OPT_SET(options, SUBMODULE_LOG);
+   options->submodule_format = DIFF_SUBMODULE_LOG;
+   else if (!strcmp(value, "diff"))
+   options->submodule_format = DIFF_SUBMODULE_DIFF;
else if (!strcmp(value, "short"))
-   DIFF_OPT_CLR(options, SUBMODULE_LOG);
+   options->submodule_format = DIFF_SUBMODULE_SHORT;
else
return -1;
return 0;
@@ -2307,9 +2309,18 @@ static void builtin_diff(const char *name_a,
struct strbuf header = STRBUF_INIT;
const char *line_prefix = diff_line_prefix(o);
 
-   if (DIFF_OPT_TST(o, SUBMODULE_LOG) &&
-   (!one->mode || S_ISGITLINK(one->mode)) &&
-   (!two->mode || S_ISGITLINK(two->mode))) {
+   diff_set_mnemonic_prefix(o, "a/", "b/");
+   if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
+   a_prefix = o->b_prefix;
+   b_prefix = o->a_pre

[PATCH v5 1/3] diff.c: remove output_prefix_length field

2016-08-11 Thread Jacob Keller
From: Junio C Hamano 

"diff/log --stat" has a logic that determines the display columns
available for the diffstat part of the output and apportions it for
pathnames and diffstat graph automatically.

5e71a84a (Add output_prefix_length to diff_options, 2012-04-16)
added the output_prefix_length field to diff_options structure to
allow this logic subtract the display columns used for display the
history graph part from the total "terminal width"; this matters
when the "git log --graph -p" option is in use.

The field be set to the number of display columns needed to show the
output from the output_prefix() callback.  Any new output_prefix()
callback must also update the field accordingly, which is error
prone.  As there is only one user of the field, and the user has the
actual value of the prefix string, let's get rid of the field and
have the user count the display width itself.

Signed-off-by: Junio C Hamano 
---
This is the same as what Junio submitted already, just thought I'd add
it for clarity.

 diff.c  | 2 +-
 diff.h  | 1 -
 graph.c | 2 --
 3 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/diff.c b/diff.c
index b43d3dd2ecb7..ae069c303077 100644
--- a/diff.c
+++ b/diff.c
@@ -1625,7 +1625,7 @@ static void show_stats(struct diffstat_t *data, struct 
diff_options *options)
 */
 
if (options->stat_width == -1)
-   width = term_columns() - options->output_prefix_length;
+   width = term_columns() - strlen(line_prefix);
else
width = options->stat_width ? options->stat_width : 80;
number_width = decimal_width(max_change) > number_width ?
diff --git a/diff.h b/diff.h
index 125447be09eb..49e4aaafb2da 100644
--- a/diff.h
+++ b/diff.h
@@ -174,7 +174,6 @@ struct diff_options {
diff_format_fn_t format_callback;
void *format_callback_data;
diff_prefix_fn_t output_prefix;
-   int output_prefix_length;
void *output_prefix_data;
 
int diff_path_counter;
diff --git a/graph.c b/graph.c
index dd1720148dc5..a46803840511 100644
--- a/graph.c
+++ b/graph.c
@@ -197,7 +197,6 @@ static struct strbuf *diff_output_prefix_callback(struct 
diff_options *opt, void
assert(opt);
assert(graph);
 
-   opt->output_prefix_length = graph->width;
strbuf_reset(&msgbuf);
graph_padding_line(graph, &msgbuf);
return &msgbuf;
@@ -245,7 +244,6 @@ struct git_graph *graph_init(struct rev_info *opt)
 */
opt->diffopt.output_prefix = diff_output_prefix_callback;
opt->diffopt.output_prefix_data = graph;
-   opt->diffopt.output_prefix_length = 0;
 
return graph;
 }
-- 
2.9.2.872.g367ebef.dirty

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


Re: Issue with global config defaults "user.useConfigOnly = true" + "pull.rebase = preserve" - "user.email"

2016-08-11 Thread Junio C Hamano
Earlier, Peff sent this patch (slightly buried in a discussion) on
"rebase -i" in <20160729223134.ga22...@sigill.intra.peff.net>.

> Subject: rebase-interactive: drop early check for valid ident
>
> Since the very inception of interactive-rebase in 1b1dce4
> (Teach rebase an interactive mode, 2007-06-25), there has
> been a preemptive check, before looking at any commits, to
> see whether the user has a valid name/email combination.
>
> This is convenient, because it means that we abort the
> operation before even beginning (rather than just
> complaining that we are unable to pick a particular commit).
>
> However, it does the wrong thing when the rebase does not
> actually need to generate any new commits (e.g., a
> fast-forward with no commits to pick, or one where the base
> stays the same, and we just pick the same commits without
> rewriting anything). In this case it may complain about the
> lack of ident, even though one would not be needed to
> complete the operation.
>
> This may seem like mere nit-picking, but because interactive
> rebase underlies the "preserve-merges" rebase, somebody who
> has set "pull.rebase" to "preserve" cannot make even a
> fast-forward pull without a valid ident, as we bail before
> even realizing the fast-forward nature.
>
> This commit drops the extra ident check entirely. This means
> we rely on individual commands that generate commit objects
> to complain. So we will continue to notice and prevent cases
> that actually do create commits, but with one important
> difference: we fail while actually executing the "pick"
> operations, and leave the rebase in a conflicted, half-done
> state.
>
> In some ways this is less convenient, but in some ways it is
> more so; the user can then manually commit or even "git
> rebase --continue" after setting up their ident (or
> providing it as a one-off on the command line).
>
> Reported-by: Dakota Hawkins 
> Signed-off-by: Jeff King 
> ---

To which, I responded (referring to the last paragraph):

Yup, that is the controvercial bit, and I suspect Dscho's original
was siding for the "set up ident first, as you will need it anyway
eventually", so I'll let others with viewpoints different from us to
chime in first before picking it up.

Do you have a preference either way to help us decide if we want to
take this change or not?

Thanks.

>  git-rebase--interactive.sh |  3 ---
>  t/t7517-per-repo-email.sh  | 47 
> ++
>  2 files changed, 47 insertions(+), 3 deletions(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index ded4595..f0f4777 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -1180,9 +1180,6 @@ To continue rebase after editing, run:
>   ;;
>  esac
>  
> -git var GIT_COMMITTER_IDENT >/dev/null ||
> - die "$(gettext "You need to set your committer info first")"
> -
>  comment_for_reflog start
>  
>  if test ! -z "$switch_to"
> diff --git a/t/t7517-per-repo-email.sh b/t/t7517-per-repo-email.sh
> index 337e6e3..2a22fa7 100755
> --- a/t/t7517-per-repo-email.sh
> +++ b/t/t7517-per-repo-email.sh
> @@ -36,4 +36,51 @@ test_expect_success 'succeeds cloning if global email is 
> not set' '
>   git clone . clone
>  '
>  
> +test_expect_success 'set up rebase scenarios' '
> + # temporarily enable an actual ident for this setup
> + test_config user.email f...@example.com &&
> + test_commit new &&
> + git branch side-without-commit HEAD^ &&
> + git checkout -b side-with-commit HEAD^ &&
> + test_commit side
> +'
> +
> +test_expect_success 'fast-forward rebase does not care about ident' '
> + git checkout -B tmp side-without-commit &&
> + git rebase master
> +'
> +
> +test_expect_success 'non-fast-forward rebase refuses to write commits' '
> + test_when_finished "git rebase --abort || true" &&
> + git checkout -B tmp side-with-commit &&
> + test_must_fail git rebase master
> +'
> +
> +test_expect_success 'fast-forward rebase does not care about ident 
> (interactive)' '
> + git checkout -B tmp side-without-commit &&
> + git rebase -i master
> +'
> +
> +test_expect_success 'non-fast-forward rebase refuses to write commits 
> (interactive)' '
> + test_when_finished "git rebase --abort || true" &&
> + git checkout -B tmp side-with-commit &&
> + test_must_fail git rebase -i master
> +'
> +
> +test_expect_success 'noop interactive rebase does not care about ident' '
> + git checkout -B tmp side-with-commit &&
> + git rebase -i HEAD^
> +'
> +
> +test_expect_success 'fast-forward rebase does not care about ident 
> (preserve)' '
> + git checkout -B tmp side-without-commit &&
> + git rebase -p master
> +'
> +
> +test_expect_success 'non-fast-forward rebase refuses to write commits 
> (preserve)' '
> + test_when_finished "git rebase --abort || true" &&
> + git checkout -B tmp side-with-commit &&
> + test_must_fail git rebase -p 

Re: public-inbox.org/git updates

2016-08-11 Thread Eric Wong
Junio C Hamano  wrote:
> Eric Wong  writes:
> 
> > There'll be over 5000K injected messages from 2006 I missed from
> > the initial import :x
> >
> > I noticed this while adding gmane: mapping support to the
> > search engine:
> >   https://public-inbox.org/git/20160811002819.GA8311@starla/T/#u
> >
> > There will still be some missing messages because some are spam.
> > news.gmane.org remains up if you want to check my work
> > (please do, because I am careless)
> 
> Thanks for doing this.

No problem!

> I wanted to try out your NTTP service, but it took me a while to dig
> in my inbox to find your announcement of news.public-inbox.org that
> hosts inbox.comp.version-control.git "newsgroup".  Is it possible to
> make this a bit more discoverable, or there is not enough NNTP
> audience these days to warrant such an addition?  I first went to
> http://public-inbox.org/git as I expected there may be some pointers
> to other instances of the service, where you list the "git clone"
> URL of the archive.

Oops, I keep forgetting to do that :x  Should be easier to do
now since I cleaned up the footer generation a few weeks back.

> By the way, it felt quite strange to see messages from 8 years ago
> mixed with more recent messages when I gold Gnus to fetch the most
> recent 333 messages (and of course that fetches 333 messages with
> largest message numbers, not sorted by "Date:").  I am assuming that
> this is an artifact of "over 5k injected messages" bundle that was
> added out of order.

Yes, definitely an artifact of fixing that screwup.
For future inbox imports (maybe LKML), I may leave gaps in the
sequence along the way, but I'm not sure how big the gaps
should or could be.

I suppose one could assign NNTP article numbers like line
numbers in BASIC...
--
To unsubscribe from this list: send the line "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: Mapping old gmane numbers to existing amil servers?

2016-08-11 Thread Eric Wong
Philip Oakley  wrote:
> The raw download works. My home ISP is currently blocking your email for
> some reason, though I do see it at my work - my iee.org alias is via my
> professional body which duplicates the email when it relays it.

Hmm, do other emails from other users get blocked?
I wonder if it's lack of DKIM (which gets invalidated by the
list sig vger appends) or something else...

> On thing I did note on the web display of the threads is that it would be
> good to have a leader of " . . . . . `" so that one can count the level of
> indent, and see the alignments via the columns of dots.
> 
> When looking at
> https://public-inbox.org/git/0648000B273C412AB7140AE959EBC99A%40PhilipOakley/
> I had difficulty working out which email the last 4 were replying to.

https://public-inbox.org/git/0648000B273C412AB7140AE959EBC99A%40PhilipOakley/
So, right now, it's:

2016-07-20 21:10   ` [PATCH v4 8/8] doc: revisions - clarify 
reachability examples Philip Oakley
2016-07-20 22:22   ` Junio C Hamano
2016-08-11 21:50   ` [PATCH v5 00/12] Update git revisions Philip 
Oakley
2016-08-11 21:50 ` [PATCH v5 01/12] doc: use 'symmetric 
difference' consistently Philip Oakley
2016-06-25 19:49   ` Junio C Hamano
2016-06-27 13:37 ` Philip Oakley
2016-06-27 15:08   ` Junio C Hamano
2016-06-27 15:39 ` Philip Oakley

And you would rather see something like:

2016-07-20 21:10   .   ` [PATCH v4 8/8] doc: revisions - clarify 
reachability examples Philip Oakley
2016-07-20 22:22   .   ` Junio C Hamano
2016-08-11 21:50   .   ` [PATCH v5 00/12] Update git revisions Philip 
Oakley
2016-08-11 21:50   . ` [PATCH v5 01/12] doc: use 'symmetric 
difference' consistently Philip Oakley
2016-06-25 19:49   ` Junio C Hamano
2016-06-27 13:37 ` Philip Oakley
2016-06-27 15:08   ` Junio C Hamano
2016-06-27 15:39 ` Philip Oakley

?

Actually, my initial inclination was to use the '|' character
which is what mutt does

2016-07-20 21:10   |   } [PATCH v4 8/8] doc: revisions - clarify 
reachability examples Philip Oakley
2016-07-20 22:22   |   } Junio C Hamano
2016-08-11 21:50   |   ` [PATCH v5 00/12] Update git revisions Philip 
Oakley
2016-08-11 21:50   | ` [PATCH v5 01/12] doc: use 'symmetric 
difference' consistently Philip Oakley
2016-06-25 19:49   ` Junio C Hamano
2016-06-27 13:37 ` Philip Oakley
2016-06-27 15:08   ` Junio C Hamano
2016-06-27 15:39 ` Philip Oakley

It should be doable, and the '`' immediately following the last
'|' probably ought to be a link to the parent, too.

I would also like to use '}' (as above) or '+' where mutt would
use "├─>" or just '├', but I don't think I can introduce multibyte
chars without causing problems for some users.

Anyways, it's definitely on my ever-growing todo list.


(Wow, it is refreshing to be a "web designer" who can live
 entirely in a terminal without relying on screenshots :D)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


What's cooking in git.git (Aug 2016, #04; Thu, 11)

2016-08-11 Thread Junio C Hamano
Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.  The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.

The 'maint' branch has been accumulating enough material to make it
the next maintenance release v2.9.3, which hopefully will happen very
soon.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[Graduated to "master"]

* cc/mailmap-tuxfamily (2016-08-08) 1 commit
  (merged to 'next' on 2016-08-10 at 5905fbf)
 + .mailmap: use Christian Couder's Tuxfamily address


* jk/completion-diff-submodule (2016-08-09) 1 commit
  (merged to 'next' on 2016-08-10 at 146ca11)
 + completion: add completion for --submodule=* diff option


* jk/push-force-with-lease-creation (2016-08-04) 4 commits
  (merged to 'next' on 2016-08-04 at e42ce85)
 + t5533: make it pass on case-sensitive filesystems
  (merged to 'next' on 2016-08-03 at 475c080)
 + push: allow pushing new branches with --force-with-lease
 + push: add shorthand for --force-with-lease branch creation
 + Documentation/git-push: fix placeholder formatting

 "git push --force-with-lease" already had enough logic to allow
 ensuring that such a push results in creation of a ref (i.e. the
 receiving end did not have another push from sideways that would be
 discarded by our force-pushing), but didn't expose this possibility
 to the users.  It does so now.


* jk/reset-ident-time-per-commit (2016-08-01) 1 commit
  (merged to 'next' on 2016-08-03 at 76d569c)
 + am: reset cached ident date for each patch

 Not-so-recent rewrite of "git am" that started making internal
 calls into the commit machinery had an unintended regression, in
 that no matter how many seconds it took to apply many patches, the
 resulting committer timestamp for the resulting commits were all
 the same.


* js/am-3-merge-recursive-direct (2016-08-01) 16 commits
  (merged to 'next' on 2016-08-05 at dc1c9bb)
 + merge-recursive: flush output buffer even when erroring out
 + merge_trees(): ensure that the callers release output buffer
 + merge-recursive: offer an option to retain the output in 'obuf'
 + merge-recursive: write the commit title in one go
 + merge-recursive: flush output buffer before printing error messages
 + am -3: use merge_recursive() directly again
 + merge-recursive: switch to returning errors instead of dying
 + merge-recursive: handle return values indicating errors
 + merge-recursive: allow write_tree_from_memory() to error out
 + merge-recursive: avoid returning a wholesale struct
 + merge_recursive: abort properly upon errors
 + prepare the builtins for a libified merge_recursive()
 + merge-recursive: clarify code in was_tracked()
 + die(_("BUG")): avoid translating bug messages
 + die("bug"): report bugs consistently
 + t5520: verify that `pull --rebase` shows the helpful advice when failing

 "git am -3" calls "git merge-recursive" when it needs to fall back
 to a three-way merge; this call has been turned into an internal
 subroutine call instead of spawning a separate subprocess.


* js/commit-slab-decl-fix (2016-08-09) 2 commits
  (merged to 'next' on 2016-08-10 at 6675402)
 + commit-slab.h: avoid duplicated global static variables
 + config.c: avoid duplicated global static variables


* jt/format-patch-from-config (2016-08-01) 1 commit
  (merged to 'next' on 2016-08-05 at 897e986)
 + format-patch: format.from gives the default for --from

 "git format-patch" learned format.from configuration variable to
 specify the default settings for its "--from" option.


* sb/submodule-update-dot-branch (2016-08-10) 8 commits
  (merged to 'next' on 2016-08-10 at 40ba945)
 + t7406: fix breakage on OSX
  (merged to 'next' on 2016-08-04 at 47bff41)
 + submodule update: allow '.' for branch value
 + submodule--helper: add remote-branch helper
 + submodule-config: keep configured branch around
 + submodule--helper: fix usage string for relative-path
 + submodule update: narrow scope of local variable
 + submodule update: respect depth in subsequent fetches
 + t7406: future proof tests with hard coded depth

 A few updates to "git submodule update".

--
[New Topics]

* sb/submodule-clone-retry (2016-08-09) 1 commit
  (merged to 'next' on 2016-08-11 at 4600b20)
 + submodule--helper: use parallel processor correctly

 Fix-up to an error codepath in a topic already in 'master'.

 Will merge to 'master'.


* vs/completion-branch-fully-spelled-d-m-r (2016-08-09) 1 commit
  (merged to 'next' on 2016-08-11 at a6f189c)
 + completion: complete --delete, --move, and --remotes for git branch

 Will merge to 'master'.


* vs/typofix (2016-08-11) 1 commit
  (merged to 'next' on 2016-08-11 at 435c418)
 + Spelling fixes

 Will merge to 

[PATCH v5 01/12] doc: use 'symmetric difference' consistently

2016-08-11 Thread Philip Oakley
Signed-off-by: Philip Oakley 
---
unchanged
---
 Documentation/gitk.txt | 2 +-
 Documentation/rev-list-options.txt | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt
index 6ade002..6c3eb15 100644
--- a/Documentation/gitk.txt
+++ b/Documentation/gitk.txt
@@ -70,7 +70,7 @@ linkgit:git-rev-list[1] for a complete list.
 
 --left-right::
 
-   Mark which side of a symmetric diff a commit is reachable
+   Mark which side of a symmetric difference a commit is reachable
from.  Commits from the left side are prefixed with a `<`
symbol and those from the right with a `>` symbol.
 
diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 4f009d4..6dc0bb0 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -225,7 +225,7 @@ excluded from the output.
 
 --left-only::
 --right-only::
-   List only commits on the respective side of a symmetric range,
+   List only commits on the respective side of a symmetric difference,
i.e. only those which would be marked `<` resp. `>` by
`--left-right`.
 +
@@ -766,7 +766,7 @@ ifdef::git-rev-list[]
 endif::git-rev-list[]
 
 --left-right::
-   Mark which side of a symmetric diff a commit is reachable from.
+   Mark which side of a symmetric difference a commit is reachable from.
Commits from the left side are prefixed with `<` and those from
the right with `>`.  If combined with `--boundary`, those
commits are prefixed with `-`.
-- 
2.9.0.windows.1

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


[PATCH v5 00/12] Update git revisions

2016-08-11 Thread Philip Oakley
This has grown like topsy from a little two patch series that tried to
name the 2-dots notation [1] into this extended set of tweaks.

As documentation can be rather personal, I've split out each small change
so each can be individually justified.

Since V4, I've confirmed that the format breaking issue is that we cannot
quote code in headings in the man page layout - ultimately it's a docbook
decision, and follows the line of analysis Peff identified (see commentary
in the patch).

In addition the multi-parent notations have been clarified and extended.

Thus the old patch 4 has been split into three. The first three patches are
unchanged. The following 4 patches are also unchanged.

Finally, at the end an extra 2 patches are added to build up the examples by
including details of the notation expansions.

This updates po/range-doc (2016-07-20) 8 commits.

Hopefully my updated workflow will get the right patches to the right people.

[1] 
https://public-inbox.org/git/0648000B273C412AB7140AE959EBC99A%40PhilipOakley/

Philip Oakley (12):
  doc: use 'symmetric difference' consistently
  doc: revisions - name the left and right sides
  doc: show the actual left, right, and boundary marks
  doc: revisions: give headings for the two and three dot notations
  doc: revisions: extra clarification of ^! notation effects
  doc: revisions: single vs multi-parent notation comparison
  doc: gitrevisions - use 'reachable' in page description
  doc: gitrevisions - clarify 'latter case' is revision walk
  doc: revisions  - define `reachable`
  doc: revisions - clarify reachability examples
  doc: revisions: show revision expansion in examples
  doc: revisions: sort examples and fix alignment of the unchanged

 Documentation/gitk.txt |   2 +-
 Documentation/gitrevisions.txt |   6 +-
 Documentation/pretty-formats.txt   |   2 +-
 Documentation/rev-list-options.txt |   4 +-
 Documentation/revisions.txt| 121 +++--
 5 files changed, 84 insertions(+), 51 deletions(-)

-- 
2.9.0.windows.1

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


Re: Mapping old gmane numbers to existing amil servers?

2016-08-11 Thread Philip Oakley

From: "Eric Wong" 
Sent: Thursday, August 11, 2016 8:56 AM

Philip Oakley  wrote:
Is there an accessible mapping from the old gmane message numbers to one 
of

the remaining email list servers for the git list?


Yes, I just posted about this after posting an initial mapping
a few weeks ago:
https://public-inbox.org/git/20160811002819.GA8311@starla/T/#u

Using "gmane:$NUM" in the search bar should work, now.
(there's a few missing messages which might've been due to
off-by-one errors, will fill them in soon)

I've seen discussions about the public-inbox, but no mention of any 
mapping

for old message references.


I had a gzipped text file published originally if you look
upthread there.



Thanks.

The raw download works. My home ISP is currently blocking your email for 
some reason, though I do see it at my work - my iee.org alias is via my 
professional body which duplicates the email when it relays it.


On thing I did note on the web display of the threads is that it would be 
good to have a leader of " . . . . . `" so that one can count the level of 
indent, and see the alignments via the columns of dots.


When looking at 
https://public-inbox.org/git/0648000B273C412AB7140AE959EBC99A%40PhilipOakley/ I 
had difficulty working out which email the last 4 were replying to.


--
To unsubscribe from this list: send the line "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 v7 0/9] status: V2 porcelain status

2016-08-11 Thread Junio C Hamano
Jeff Hostetler  writes:

> From: Jeff Hostetler 
>
> This patch series adds porcelain V2 format to status.
> This provides detailed information about file changes
> and about the current branch.
>
> The new output is accessed via:
> git status --porcelain=v2 [--branch]
>
> This v7 patch series address the most recent feedback
> on the unit tests.
>
> This series has been rebased onto a more current master
> branch (to get the EMPTY_BLOB changes in the test suite).

Thanks for a note.  I've been queuing the series on top of f8f7adc,
so I am assuming I won't see any changes in 1-8.

Unfortunately I cannot be taking new rerolls or new topics for the
rest of the day as I need to start today's integration cycle and
I'll have to delay taking a look at this round, but from a quick
glance you still seem to have a lot of dirt in the new test and I
cannot quite tell if they are intended.

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 v7 6/9] status: print branch info with --porcelain=v2 --branch

2016-08-11 Thread Jeff Hostetler
From: Jeff Hostetler 

Expand porcelain v2 output to include branch and tracking
branch information. This includes the commit id, the branch,
the upstream branch, and the ahead and behind counts.

Signed-off-by: Jeff Hostetler 
---
 builtin/commit.c |  5 
 wt-status.c  | 90 
 wt-status.h  |  1 +
 3 files changed, 96 insertions(+)

diff --git a/builtin/commit.c b/builtin/commit.c
index c50faf9..bb9f79b 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -510,6 +510,8 @@ static int run_status(FILE *fp, const char *index_file, 
const char *prefix, int
s->fp = fp;
s->nowarn = nowarn;
s->is_initial = get_sha1(s->reference, sha1) ? 1 : 0;
+   if (!s->is_initial)
+   hashcpy(s->sha1_commit, sha1);
s->status_format = status_format;
s->ignore_submodule_arg = ignore_submodule_arg;
 
@@ -1378,6 +1380,9 @@ int cmd_status(int argc, const char **argv, const char 
*prefix)
fd = hold_locked_index(&index_lock, 0);
 
s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;
+   if (!s.is_initial)
+   hashcpy(s.sha1_commit, sha1);
+
s.ignore_submodule_arg = ignore_submodule_arg;
s.status_format = status_format;
s.verbose = verbose;
diff --git a/wt-status.c b/wt-status.c
index 163b453..539aac1 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1816,6 +1816,92 @@ static void wt_porcelain_print(struct wt_status *s)
 }
 
 /*
+ * Print branch information for porcelain v2 output.  These lines
+ * are printed when the '--branch' parameter is given.
+ *
+ *# branch.oid 
+ *# branch.head 
+ *   [# branch.upstream 
+ *   [# branch.ab + -]]
+ *
+ *   ::= the current commit hash or the the literal
+ *   "(initial)" to indicate an initialized repo
+ *   with no commits.
+ *
+ * ::=  the current branch name or
+ *   "(detached)" literal when detached head or
+ *   "(unknown)" when something is wrong.
+ *
+ * ::= the upstream branch name, when set.
+ *
+ *::= integer ahead value, when upstream set
+ *   and the commit is present (not gone).
+ *
+ *   ::= integer behind value, when upstream set
+ *   and commit is present.
+ *
+ *
+ * The end-of-line is defined by the -z flag.
+ *
+ *  ::= NUL when -z,
+ *   LF when NOT -z.
+ *
+ */
+static void wt_porcelain_v2_print_tracking(struct wt_status *s)
+{
+   struct branch *branch;
+   const char *base;
+   const char *branch_name;
+   struct wt_status_state state;
+   int ab_info, nr_ahead, nr_behind;
+   char eol = s->null_termination ? '\0' : '\n';
+
+   memset(&state, 0, sizeof(state));
+   wt_status_get_state(&state, s->branch && !strcmp(s->branch, "HEAD"));
+
+   fprintf(s->fp, "# branch.oid %s%c",
+   (s->is_initial ? "(initial)" : 
sha1_to_hex(s->sha1_commit)),
+   eol);
+
+   if (!s->branch)
+   fprintf(s->fp, "# branch.head %s%c", "(unknown)", eol);
+   else {
+   if (!strcmp(s->branch, "HEAD")) {
+   fprintf(s->fp, "# branch.head %s%c", "(detached)", eol);
+
+   if (state.rebase_in_progress || 
state.rebase_interactive_in_progress)
+   branch_name = state.onto;
+   else if (state.detached_from)
+   branch_name = state.detached_from;
+   else
+   branch_name = "";
+   } else {
+   branch_name = NULL;
+   skip_prefix(s->branch, "refs/heads/", &branch_name);
+
+   fprintf(s->fp, "# branch.head %s%c", branch_name, eol);
+   }
+
+   /* Lookup stats on the upstream tracking branch, if set. */
+   branch = branch_get(branch_name);
+   base = NULL;
+   ab_info = (stat_tracking_info(branch, &nr_ahead, &nr_behind, 
&base) == 0);
+   if (base) {
+   base = shorten_unambiguous_ref(base, 0);
+   fprintf(s->fp, "# branch.upstream %s%c", base, eol);
+   free((char *)base);
+
+   if (ab_info)
+   fprintf(s->fp, "# branch.ab +%d -%d%c", 
nr_ahead, nr_behind, eol);
+   }
+   }
+
+   free(state.branch);
+   free(state.onto);
+   free(state.detached_from);
+}
+
+/*
  * Convert various submodule status values into a
  * fixed-length string of characters in the buffer provided.
  */
@@ -2061,6 +2147,7 @@ static void wt_porcelain_v2_print_other(
 /*
  * Print porcelain V2 status.
  *
+ * []
  * []*
  * []*
  * []*
@@ -2073,6 +2160,9 @@ static void wt_porcelain_v2_print(struct wt_status *s)
struct string_list_item *it;
 

[PATCH v7 5/9] status: print per-file porcelain v2 status data

2016-08-11 Thread Jeff Hostetler
From: Jeff Hostetler 

Print per-file information in porcelain v2 format.

Signed-off-by: Jeff Hostetler 
---
 wt-status.c | 285 +++-
 1 file changed, 284 insertions(+), 1 deletion(-)

diff --git a/wt-status.c b/wt-status.c
index aa804b5..163b453 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1815,6 +1815,289 @@ static void wt_porcelain_print(struct wt_status *s)
wt_shortstatus_print(s);
 }
 
+/*
+ * Convert various submodule status values into a
+ * fixed-length string of characters in the buffer provided.
+ */
+static void wt_porcelain_v2_submodule_state(
+   struct wt_status_change_data *d,
+   char sub[5])
+{
+   if (S_ISGITLINK(d->mode_head) ||
+   S_ISGITLINK(d->mode_index) ||
+   S_ISGITLINK(d->mode_worktree)) {
+   sub[0] = 'S';
+   sub[1] = d->new_submodule_commits ? 'C' : '.';
+   sub[2] = (d->dirty_submodule & DIRTY_SUBMODULE_MODIFIED) ? 'M' 
: '.';
+   sub[3] = (d->dirty_submodule & DIRTY_SUBMODULE_UNTRACKED) ? 'U' 
: '.';
+   } else {
+   sub[0] = 'N';
+   sub[1] = '.';
+   sub[2] = '.';
+   sub[3] = '.';
+   }
+   sub[4] = 0;
+}
+
+/*
+ * Fix-up changed entries before we print them.
+ */
+static void wt_porcelain_v2_fix_up_changed(
+   struct string_list_item *it,
+   struct wt_status *s)
+{
+   struct wt_status_change_data *d = it->util;
+
+   if (!d->index_status) {
+   /*
+* This entry is unchanged in the index (relative to the head).
+* Therefore, the collect_updated_cb was never called for this
+* entry (during the head-vs-index scan) and so the head column
+* fields were never set.
+*
+* We must have data for the index column (from the
+* index-vs-worktree scan (otherwise, this entry should not be
+* in the list of changes)).
+*
+* Copy index column fields to the head column, so that our
+* output looks complete.
+*/
+   assert(d->mode_head == 0);
+   d->mode_head = d->mode_index;
+   oidcpy(&d->oid_head, &d->oid_index);
+   }
+
+   if (!d->worktree_status) {
+   /*
+* This entry is unchanged in the worktree (relative to the 
index).
+* Therefore, the collect_changed_cb was never called for this 
entry
+* (during the index-vs-worktree scan) and so the worktree 
column
+* fields were never set.
+*
+* We must have data for the index column (from the 
head-vs-index
+* scan).
+*
+* Copy the index column fields to the worktree column so that
+* our output looks complete.
+*
+* Note that we only have a mode field in the worktree column
+* because the scan code tries really hard to not have to 
compute it.
+*/
+   assert(d->mode_worktree == 0);
+   d->mode_worktree = d->mode_index;
+   }
+}
+
+/*
+ * Print porcelain v2 info for tracked entries with changes.
+ */
+static void wt_porcelain_v2_print_changed_entry(
+   struct string_list_item *it,
+   struct wt_status *s)
+{
+   struct wt_status_change_data *d = it->util;
+   struct strbuf buf_index = STRBUF_INIT;
+   struct strbuf buf_head = STRBUF_INIT;
+   const char *path_index = NULL;
+   const char *path_head = NULL;
+   char key[3];
+   char submodule_token[5];
+   char sep_char, eol_char;
+
+   wt_porcelain_v2_fix_up_changed(it, s);
+   wt_porcelain_v2_submodule_state(d, submodule_token);
+
+   key[0] = d->index_status ? d->index_status : '.';
+   key[1] = d->worktree_status ? d->worktree_status : '.';
+   key[2] = 0;
+
+   if (s->null_termination) {
+   /*
+* In -z mode, we DO NOT C-quote pathnames.  Current path is 
ALWAYS first.
+* A single NUL character separates them.
+*/
+   sep_char = '\0';
+   eol_char = '\0';
+   path_index = it->string;
+   path_head = d->head_path;
+   } else {
+   /*
+* Path(s) are C-quoted if necessary. Current path is ALWAYS 
first.
+* The source path is only present when necessary.
+* A single TAB separates them (because paths can contain spaces
+* which are not escaped and C-quoting does escape TAB 
characters).
+*/
+   sep_char = '\t';
+   eol_char = '\n';
+   path_index = quote_path(it->string, s->prefix, &buf_index);
+   if (d->head_path)
+   path_head 

Re: [PATCH v4 1/2] diff: add --line-prefix option for passing in a prefix

2016-08-11 Thread Junio C Hamano
Subject: SQUASH??? clarify the if/{if/else} nesting

Otherwise GCC helpfully warns you.

Signed-off-by: Junio C Hamano 
---
 diff.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/diff.c b/diff.c
index bfc0a6b..d128b9d 100644
--- a/diff.c
+++ b/diff.c
@@ -1170,11 +1170,12 @@ const char *diff_line_prefix(struct diff_options *opt)
 {
struct strbuf *msgbuf;
 
-   if (!opt->output_prefix)
+   if (!opt->output_prefix) {
if (opt->line_prefix)
return opt->line_prefix;
else
return "";
+   }
 
msgbuf = opt->output_prefix(opt, opt->output_prefix_data);
/* line prefix must be printed before the output_prefix() */
-- 
2.9.2-863-g71ed253

--
To unsubscribe from this list: send the line "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 v7 3/9] status: support --porcelain[=]

2016-08-11 Thread Jeff Hostetler
From: Jeff Hostetler 

Update --porcelain argument to take optional version parameter
to allow multiple porcelain formats to be supported in the future.

The token "v1" is the default value and indicates the traditional
porcelain format.  (The token "1" is an alias for that.)

Signed-off-by: Jeff Hostetler 
---
 Documentation/git-status.txt |  7 +--
 builtin/commit.c | 21 ++---
 t/t7060-wtstatus.sh  | 21 +
 3 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index e1e8f57..6b1454b 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -32,11 +32,14 @@ OPTIONS
 --branch::
Show the branch and tracking info even in short-format.
 
---porcelain::
+--porcelain[=]::
Give the output in an easy-to-parse format for scripts.
This is similar to the short output, but will remain stable
across Git versions and regardless of user configuration. See
below for details.
++
+The version parameter is used to specify the format version.
+This is optional and defaults to the original version 'v1' format.
 
 --long::
Give the output in the long-format. This is the default.
@@ -96,7 +99,7 @@ configuration variable documented in linkgit:git-config[1].
 
 -z::
Terminate entries with NUL, instead of LF.  This implies
-   the `--porcelain` output format if no other format is given.
+   the `--porcelain=v1` output format if no other format is given.
 
 --column[=]::
 --no-column::
diff --git a/builtin/commit.c b/builtin/commit.c
index c5e0173..c9d24d5 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -144,6 +144,21 @@ static struct strbuf message = STRBUF_INIT;
 
 static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
 
+static int opt_parse_porcelain(const struct option *opt, const char *arg, int 
unset)
+{
+   enum wt_status_format *value = (enum wt_status_format *)opt->value;
+   if (unset)
+   *value = STATUS_FORMAT_NONE;
+   else if (!arg)
+   *value = STATUS_FORMAT_PORCELAIN;
+   else if (!strcmp(arg, "v1") || !strcmp(arg, "1"))
+   *value = STATUS_FORMAT_PORCELAIN;
+   else
+   die("unsupported porcelain version '%s'", arg);
+
+   return 0;
+}
+
 static int opt_parse_m(const struct option *opt, const char *arg, int unset)
 {
struct strbuf *buf = opt->value;
@@ -1316,9 +1331,9 @@ int cmd_status(int argc, const char **argv, const char 
*prefix)
N_("show status concisely"), STATUS_FORMAT_SHORT),
OPT_BOOL('b', "branch", &s.show_branch,
 N_("show branch information")),
-   OPT_SET_INT(0, "porcelain", &status_format,
-   N_("machine-readable output"),
-   STATUS_FORMAT_PORCELAIN),
+   { OPTION_CALLBACK, 0, "porcelain", &status_format,
+ N_("version"), N_("machine-readable output"),
+ PARSE_OPT_OPTARG, opt_parse_porcelain },
OPT_SET_INT(0, "long", &status_format,
N_("show status in long format (default)"),
STATUS_FORMAT_LONG),
diff --git a/t/t7060-wtstatus.sh b/t/t7060-wtstatus.sh
index 4d17363..53cf42f 100755
--- a/t/t7060-wtstatus.sh
+++ b/t/t7060-wtstatus.sh
@@ -232,4 +232,25 @@ test_expect_success 'status --branch with detached HEAD' '
test_i18ncmp expected actual
 '
 
+## Duplicate the above test and verify --porcelain=v1 arg parsing.
+test_expect_success 'status --porcelain=v1 --branch with detached HEAD' '
+   git reset --hard &&
+   git checkout master^0 &&
+   git status --branch --porcelain=v1 >actual &&
+   cat >expected <<-EOF &&
+   ## HEAD (no branch)
+   ?? .gitconfig
+   ?? actual
+   ?? expect
+   ?? expected
+   ?? mdconflict/
+   EOF
+   test_i18ncmp expected actual
+'
+
+## Verify parser error on invalid --porcelain argument.
+test_expect_success 'status --porcelain=bogus' '
+   test_must_fail git status --porcelain=bogus
+'
+
 test_done
-- 
2.8.0.rc4.17.gac42084.dirty

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


[PATCH v7 1/9] status: rename long-format print routines

2016-08-11 Thread Jeff Hostetler
From: Jeff Hostetler 

Rename the various wt_status_print*() routines to be
wt_longstatus_print*() to make it clear that these
routines are only concerned with the normal/long
status output and reduce developer confusion as other
status formats are added in the future.

Signed-off-by: Jeff Hostetler 
---
 builtin/commit.c |   4 +-
 wt-status.c  | 110 +++
 wt-status.h  |   2 +-
 3 files changed, 58 insertions(+), 58 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 77e3dc8..ecfc965 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -515,7 +515,7 @@ static int run_status(FILE *fp, const char *index_file, 
const char *prefix, int
break;
case STATUS_FORMAT_NONE:
case STATUS_FORMAT_LONG:
-   wt_status_print(s);
+   wt_longstatus_print(s);
break;
}
 
@@ -1403,7 +1403,7 @@ int cmd_status(int argc, const char **argv, const char 
*prefix)
case STATUS_FORMAT_LONG:
s.verbose = verbose;
s.ignore_submodule_arg = ignore_submodule_arg;
-   wt_status_print(&s);
+   wt_longstatus_print(&s);
break;
}
return 0;
diff --git a/wt-status.c b/wt-status.c
index 6225a2d..bae9507 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -139,7 +139,7 @@ void wt_status_prepare(struct wt_status *s)
s->display_comment_prefix = 0;
 }
 
-static void wt_status_print_unmerged_header(struct wt_status *s)
+static void wt_longstatus_print_unmerged_header(struct wt_status *s)
 {
int i;
int del_mod_conflict = 0;
@@ -191,7 +191,7 @@ static void wt_status_print_unmerged_header(struct 
wt_status *s)
status_printf_ln(s, c, "%s", "");
 }
 
-static void wt_status_print_cached_header(struct wt_status *s)
+static void wt_longstatus_print_cached_header(struct wt_status *s)
 {
const char *c = color(WT_STATUS_HEADER, s);
 
@@ -207,9 +207,9 @@ static void wt_status_print_cached_header(struct wt_status 
*s)
status_printf_ln(s, c, "%s", "");
 }
 
-static void wt_status_print_dirty_header(struct wt_status *s,
-int has_deleted,
-int has_dirty_submodules)
+static void wt_longstatus_print_dirty_header(struct wt_status *s,
+int has_deleted,
+int has_dirty_submodules)
 {
const char *c = color(WT_STATUS_HEADER, s);
 
@@ -226,9 +226,9 @@ static void wt_status_print_dirty_header(struct wt_status 
*s,
status_printf_ln(s, c, "%s", "");
 }
 
-static void wt_status_print_other_header(struct wt_status *s,
-const char *what,
-const char *how)
+static void wt_longstatus_print_other_header(struct wt_status *s,
+const char *what,
+const char *how)
 {
const char *c = color(WT_STATUS_HEADER, s);
status_printf_ln(s, c, "%s:", what);
@@ -238,7 +238,7 @@ static void wt_status_print_other_header(struct wt_status 
*s,
status_printf_ln(s, c, "%s", "");
 }
 
-static void wt_status_print_trailer(struct wt_status *s)
+static void wt_longstatus_print_trailer(struct wt_status *s)
 {
status_printf_ln(s, color(WT_STATUS_HEADER, s), "%s", "");
 }
@@ -304,8 +304,8 @@ static int maxwidth(const char *(*label)(int), int minval, 
int maxval)
return result;
 }
 
-static void wt_status_print_unmerged_data(struct wt_status *s,
- struct string_list_item *it)
+static void wt_longstatus_print_unmerged_data(struct wt_status *s,
+ struct string_list_item *it)
 {
const char *c = color(WT_STATUS_UNMERGED, s);
struct wt_status_change_data *d = it->util;
@@ -331,9 +331,9 @@ static void wt_status_print_unmerged_data(struct wt_status 
*s,
strbuf_release(&onebuf);
 }
 
-static void wt_status_print_change_data(struct wt_status *s,
-   int change_type,
-   struct string_list_item *it)
+static void wt_longstatus_print_change_data(struct wt_status *s,
+   int change_type,
+   struct string_list_item *it)
 {
struct wt_status_change_data *d = it->util;
const char *c = color(change_type, s);
@@ -378,7 +378,7 @@ static void wt_status_print_change_data(struct wt_status *s,
status = d->worktree_status;
break;
default:
-   die("BUG: unhandled change_type %d in 
wt_status_print_change_data",
+   die("BUG: unhandled change_type %d in 
wt_longstatus_print_change_data",
change_type);
}
 
@@

Re: [PATCH v6 9/9] status: unit tests for --porcelain=v2

2016-08-11 Thread Jeff Hostetler



On 08/11/2016 02:36 PM, Junio C Hamano wrote:

Jeff Hostetler  writes:


From: Jeff Hostetler 
+. ./test-lib.sh
+
+OID_EMPTY=e69de29bb2d1d6434b8b29ae775ad8c2e48c5391


It seems that test-lib.sh these days has EMPTY_BLOB defined for your
use.  You can remove this and replace its use (just two lines) with
$EMPTY_BLOB down in the "add -N" test.


That's a recent addition to test-lib.sh. I'll rebase my series
onto a more recent master to get that.



+test_expect_success setup '
+   test_tick &&
+   git config --local core.autocrlf false &&


I'd suggest removing "--local".

Existing use of "git config" in the test suite, unless their use is
about testing "git config" itself to validate the operation of the
--local/--global/--system options, do not seem to explicitly say
"--local", which is the default anyway.


ok. just being cautious, but if that's the convention, that's fine.



+test_expect_success 'after first commit, make dirt, confirm unstaged changes' '


Did you mean s/dirt/dirty/?  "make and confirm unstaged changes"
would be sufficient.  Because "confirming" is implicit (as these
are all tests), "after the first commit, modify working tree files"
might even be better, perhaps?


+   echo x >>file_x &&
+   OID_X1=$(git hash-object -t blob -- file_x) &&
+   rm file_z &&
+   H0=$(git rev-parse HEAD) &&
+ ...


got it.



+test_expect_success 'after first commit, stage dirt, confirm staged changes' '


What you "git add" is meant to be good changes, so they are no
longer dirt ;-)  More importantly, because I never heard of "dirt"
used in Git context, it is unclear if it is an untracked file, a
modification that is not meant to be committed immediately, or what.

"after the first commit, fully add changes to the index"?


reworded and simplified.



+   git add file_x &&
+   git rm file_z &&
+   H0=$(git rev-parse HEAD) &&
+
+   cat >expect <<-EOF &&
+   # branch.oid $H0
+   # branch.head master
+   1 M. N... 100644 100644 100644 $OID_X $OID_X1 file_x
+   1 D. N... 100644 00 00 $OID_Z $_z40 file_z
+   ? actual
+   ? expect
+   EOF



+test_expect_success 'after first commit, also stage rename, confirm 2 path 
line format' '
+   git mv file_y renamed_y &&
+   H0=$(git rev-parse HEAD) &&
+
+   q_to_tab >expect <<-EOF &&
+   # branch.oid $H0
+   # branch.head master
+   1 M. N... 100644 100644 100644 $OID_X $OID_X1 file_x
+   1 D. N... 100644 00 00 $OID_Z $_z40 file_z
+   2 R. N... 100644 100644 100644 $OID_Y $OID_Y R100 renamed_yQfile_y
+   ? actual
+   ? expect
+   EOF
+
+   git status --porcelain=v2 --branch --untracked-files=all >actual &&
+   test_cmp expect actual
+'


Do we want to test -z format on this, too?


Sure.


I'll send these up in a v7 series in a few minutes.

Thanks
Jeff

--
To unsubscribe from this list: send the line "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 v7 2/9] status: cleanup API to wt_status_print

2016-08-11 Thread Jeff Hostetler
From: Jeff Hostetler 

Refactor the API between builtin/commit.c and wt-status.[ch].

Hide the details of the various wt_*status_print() routines inside
wt-status.c behind a single (new) wt_status_print() routine.
Eliminate the switch statements from builtin/commit.c.
Allow details of new status formats to be isolated within wt-status.c

Signed-off-by: Jeff Hostetler 
---
 builtin/commit.c | 51 +--
 wt-status.c  | 25 ++---
 wt-status.h  | 16 
 3 files changed, 43 insertions(+), 49 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index ecfc965..c5e0173 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -142,14 +142,7 @@ static int show_ignored_in_status, have_option_m;
 static const char *only_include_assumed;
 static struct strbuf message = STRBUF_INIT;
 
-static enum status_format {
-   STATUS_FORMAT_NONE = 0,
-   STATUS_FORMAT_LONG,
-   STATUS_FORMAT_SHORT,
-   STATUS_FORMAT_PORCELAIN,
-
-   STATUS_FORMAT_UNSPECIFIED
-} status_format = STATUS_FORMAT_UNSPECIFIED;
+static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
 
 static int opt_parse_m(const struct option *opt, const char *arg, int unset)
 {
@@ -500,24 +493,11 @@ static int run_status(FILE *fp, const char *index_file, 
const char *prefix, int
s->fp = fp;
s->nowarn = nowarn;
s->is_initial = get_sha1(s->reference, sha1) ? 1 : 0;
+   s->status_format = status_format;
+   s->ignore_submodule_arg = ignore_submodule_arg;
 
wt_status_collect(s);
-
-   switch (status_format) {
-   case STATUS_FORMAT_SHORT:
-   wt_shortstatus_print(s);
-   break;
-   case STATUS_FORMAT_PORCELAIN:
-   wt_porcelain_print(s);
-   break;
-   case STATUS_FORMAT_UNSPECIFIED:
-   die("BUG: finalize_deferred_config() should have been called");
-   break;
-   case STATUS_FORMAT_NONE:
-   case STATUS_FORMAT_LONG:
-   wt_longstatus_print(s);
-   break;
-   }
+   wt_status_print(s);
 
return s->commitable;
 }
@@ -1099,7 +1079,7 @@ static const char *read_commit_message(const char *name)
  * is not in effect here.
  */
 static struct status_deferred_config {
-   enum status_format status_format;
+   enum wt_status_format status_format;
int show_branch;
 } status_deferred_config = {
STATUS_FORMAT_UNSPECIFIED,
@@ -1381,6 +1361,9 @@ int cmd_status(int argc, const char **argv, const char 
*prefix)
 
s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;
s.ignore_submodule_arg = ignore_submodule_arg;
+   s.status_format = status_format;
+   s.verbose = verbose;
+
wt_status_collect(&s);
 
if (0 <= fd)
@@ -1389,23 +1372,7 @@ int cmd_status(int argc, const char **argv, const char 
*prefix)
if (s.relative_paths)
s.prefix = prefix;
 
-   switch (status_format) {
-   case STATUS_FORMAT_SHORT:
-   wt_shortstatus_print(&s);
-   break;
-   case STATUS_FORMAT_PORCELAIN:
-   wt_porcelain_print(&s);
-   break;
-   case STATUS_FORMAT_UNSPECIFIED:
-   die("BUG: finalize_deferred_config() should have been called");
-   break;
-   case STATUS_FORMAT_NONE:
-   case STATUS_FORMAT_LONG:
-   s.verbose = verbose;
-   s.ignore_submodule_arg = ignore_submodule_arg;
-   wt_longstatus_print(&s);
-   break;
-   }
+   wt_status_print(&s);
return 0;
 }
 
diff --git a/wt-status.c b/wt-status.c
index bae9507..59bfb0b 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1450,7 +1450,7 @@ static void wt_longstatus_print_state(struct wt_status *s,
show_bisect_in_progress(s, state, state_color);
 }
 
-void wt_longstatus_print(struct wt_status *s)
+static void wt_longstatus_print(struct wt_status *s)
 {
const char *branch_color = color(WT_STATUS_ONBRANCH, s);
const char *branch_status_color = color(WT_STATUS_HEADER, s);
@@ -1717,7 +1717,7 @@ static void wt_shortstatus_print_tracking(struct 
wt_status *s)
fputc(s->null_termination ? '\0' : '\n', s->fp);
 }
 
-void wt_shortstatus_print(struct wt_status *s)
+static void wt_shortstatus_print(struct wt_status *s)
 {
int i;
 
@@ -1749,7 +1749,7 @@ void wt_shortstatus_print(struct wt_status *s)
}
 }
 
-void wt_porcelain_print(struct wt_status *s)
+static void wt_porcelain_print(struct wt_status *s)
 {
s->use_color = 0;
s->relative_paths = 0;
@@ -1757,3 +1757,22 @@ void wt_porcelain_print(struct wt_status *s)
s->no_gettext = 1;
wt_shortstatus_print(s);
 }
+
+void wt_status_print(struct wt_status *s)
+{
+   switch (s->status_format) {
+   case STATUS_FORMAT_SHORT:
+   wt_shortstatus_print(s);
+   break;
+   ca

[PATCH v7 9/9] status: unit tests for --porcelain=v2

2016-08-11 Thread Jeff Hostetler
From: Jeff Hostetler 

Test porcelain v2 status format.

Signed-off-by: Jeff Hostetler 
---
 t/t7064-wtstatus-pv2.sh | 592 
 1 file changed, 592 insertions(+)
 create mode 100755 t/t7064-wtstatus-pv2.sh

diff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh
new file mode 100755
index 000..34d25b5
--- /dev/null
+++ b/t/t7064-wtstatus-pv2.sh
@@ -0,0 +1,592 @@
+#!/bin/sh
+
+test_description='git status --porcelain=v2
+
+This test exercises porcelain V2 output for git status.'
+
+
+. ./test-lib.sh
+
+
+test_expect_success setup '
+   test_tick &&
+   git config core.autocrlf false &&
+   echo x >file_x &&
+   echo y >file_y &&
+   echo z >file_z &&
+   mkdir dir1 &&
+   echo a >dir1/file_a &&
+   echo b >dir1/file_b
+'
+
+test_expect_success 'before initial commit, nothing added, only untracked' '
+   cat >expect <<-EOF &&
+   # branch.oid (initial)
+   # branch.head master
+   ? actual
+   ? dir1/
+   ? expect
+   ? file_x
+   ? file_y
+   ? file_z
+   EOF
+
+   git status --porcelain=v2 --branch --untracked-files=normal >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'before initial commit, things added' '
+   git add file_x file_y file_z dir1 &&
+   OID_A=$(git hash-object -t blob -- dir1/file_a) &&
+   OID_B=$(git hash-object -t blob -- dir1/file_b) &&
+   OID_X=$(git hash-object -t blob -- file_x) &&
+   OID_Y=$(git hash-object -t blob -- file_y) &&
+   OID_Z=$(git hash-object -t blob -- file_z) &&
+
+   cat >expect <<-EOF &&
+   # branch.oid (initial)
+   # branch.head master
+   1 A. N... 00 100644 100644 $_z40 $OID_A dir1/file_a
+   1 A. N... 00 100644 100644 $_z40 $OID_B dir1/file_b
+   1 A. N... 00 100644 100644 $_z40 $OID_X file_x
+   1 A. N... 00 100644 100644 $_z40 $OID_Y file_y
+   1 A. N... 00 100644 100644 $_z40 $OID_Z file_z
+   ? actual
+   ? expect
+   EOF
+
+   git status --porcelain=v2 --branch --untracked-files=all >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'before initial commit, things added (-z)' '
+   lf_to_nul >expect <<-EOF &&
+   # branch.oid (initial)
+   # branch.head master
+   1 A. N... 00 100644 100644 $_z40 $OID_A dir1/file_a
+   1 A. N... 00 100644 100644 $_z40 $OID_B dir1/file_b
+   1 A. N... 00 100644 100644 $_z40 $OID_X file_x
+   1 A. N... 00 100644 100644 $_z40 $OID_Y file_y
+   1 A. N... 00 100644 100644 $_z40 $OID_Z file_z
+   ? actual
+   ? expect
+   EOF
+
+   git status -z --porcelain=v2 --branch --untracked-files=all >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'make first commit, comfirm HEAD oid and branch' '
+   git commit -m initial &&
+   H0=$(git rev-parse HEAD) &&
+   cat >expect <<-EOF &&
+   # branch.oid $H0
+   # branch.head master
+   ? actual
+   ? expect
+   EOF
+
+   git status --porcelain=v2 --branch --untracked-files=all >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'after first commit, create unstaged changes' '
+   echo x >>file_x &&
+   OID_X1=$(git hash-object -t blob -- file_x) &&
+   rm file_z &&
+   H0=$(git rev-parse HEAD) &&
+
+   cat >expect <<-EOF &&
+   # branch.oid $H0
+   # branch.head master
+   1 .M N... 100644 100644 100644 $OID_X $OID_X file_x
+   1 .D N... 100644 100644 00 $OID_Z $OID_Z file_z
+   ? actual
+   ? expect
+   EOF
+
+   git status --porcelain=v2 --branch --untracked-files=all >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'hide untracked files' '
+   cat >expect <<-EOF &&
+   1 .M N... 100644 100644 100644 $OID_X $OID_X file_x
+   1 .D N... 100644 100644 00 $OID_Z $OID_Z file_z
+   EOF
+
+   git status --porcelain=v2 --untracked-files=no >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'after first commit, stage changes' '
+   git add file_x &&
+   git rm file_z &&
+   H0=$(git rev-parse HEAD) &&
+
+   cat >expect <<-EOF &&
+   # branch.oid $H0
+   # branch.head master
+   1 M. N... 100644 100644 100644 $OID_X $OID_X1 file_x
+   1 D. N... 100644 00 00 $OID_Z $_z40 file_z
+   ? actual
+   ? expect
+   EOF
+
+   git status --porcelain=v2 --branch --untracked-files=all >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'rename causes 2 path lines' '
+   git mv file_y renamed_y &&
+   H0=$(git rev-parse HEAD) &&
+
+   q_to_tab >expect <<-EOF &&
+   # branch.oid $H0
+   # branch.head master
+   1 M. N... 100644 100644 100644 $OID_X $OID_X1 file_x
+   1 D. N... 100644 00 00 $OID_Z $_z40 file_z
+   2 R. N... 100644 100644 100644 $OID_Y $OID_Y R100 renamed_yQfile_y
+   ? actual

[PATCH v7 4/9] status: collect per-file data for --porcelain=v2

2016-08-11 Thread Jeff Hostetler
From: Jeff Hostetler 

Collect extra per-file data for porcelain V2 format.

The output of `git status --porcelain` leaves out many
details about the current status that clients might like
to have.  This can force them to be less efficient as they
may need to launch secondary commands (and try to match
the logic within git) to accumulate this extra information.
For example, a GUI IDE might want the file mode to display
the correct icon for a changed item (without having to stat
it afterwards).

Signed-off-by: Jeff Hostetler 
---
 builtin/commit.c |  3 +++
 wt-status.c  | 64 ++--
 wt-status.h  |  4 
 3 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index c9d24d5..c50faf9 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -153,6 +153,8 @@ static int opt_parse_porcelain(const struct option *opt, 
const char *arg, int un
*value = STATUS_FORMAT_PORCELAIN;
else if (!strcmp(arg, "v1") || !strcmp(arg, "1"))
*value = STATUS_FORMAT_PORCELAIN;
+   else if (!strcmp(arg, "v2") || !strcmp(arg, "2"))
+   *value = STATUS_FORMAT_PORCELAIN_V2;
else
die("unsupported porcelain version '%s'", arg);
 
@@ -1104,6 +1106,7 @@ static struct status_deferred_config {
 static void finalize_deferred_config(struct wt_status *s)
 {
int use_deferred_config = (status_format != STATUS_FORMAT_PORCELAIN &&
+  status_format != STATUS_FORMAT_PORCELAIN_V2 
&&
   !s->null_termination);
 
if (s->null_termination) {
diff --git a/wt-status.c b/wt-status.c
index 59bfb0b..aa804b5 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -434,6 +434,31 @@ static void wt_status_collect_changed_cb(struct 
diff_queue_struct *q,
if (S_ISGITLINK(p->two->mode))
d->new_submodule_commits = !!oidcmp(&p->one->oid,
&p->two->oid);
+
+   switch (p->status) {
+   case DIFF_STATUS_ADDED:
+   die("BUG: worktree status add???");
+   break;
+
+   case DIFF_STATUS_DELETED:
+   d->mode_index = p->one->mode;
+   oidcpy(&d->oid_index, &p->one->oid);
+   /* mode_worktree is zero for a delete. */
+   break;
+
+   case DIFF_STATUS_MODIFIED:
+   case DIFF_STATUS_TYPE_CHANGED:
+   case DIFF_STATUS_UNMERGED:
+   d->mode_index = p->one->mode;
+   d->mode_worktree = p->two->mode;
+   oidcpy(&d->oid_index, &p->one->oid);
+   break;
+
+   case DIFF_STATUS_UNKNOWN:
+   die("BUG: worktree status unknown???");
+   break;
+   }
+
}
 }
 
@@ -479,12 +504,36 @@ static void wt_status_collect_updated_cb(struct 
diff_queue_struct *q,
if (!d->index_status)
d->index_status = p->status;
switch (p->status) {
+   case DIFF_STATUS_ADDED:
+   /* Leave {mode,oid}_head zero for an add. */
+   d->mode_index = p->two->mode;
+   oidcpy(&d->oid_index, &p->two->oid);
+   break;
+   case DIFF_STATUS_DELETED:
+   d->mode_head = p->one->mode;
+   oidcpy(&d->oid_head, &p->one->oid);
+   /* Leave {mode,oid}_index zero for a delete. */
+   break;
+
case DIFF_STATUS_COPIED:
case DIFF_STATUS_RENAMED:
d->head_path = xstrdup(p->one->path);
+   d->score = p->score * 100 / MAX_SCORE;
+   /* fallthru */
+   case DIFF_STATUS_MODIFIED:
+   case DIFF_STATUS_TYPE_CHANGED:
+   d->mode_head = p->one->mode;
+   d->mode_index = p->two->mode;
+   oidcpy(&d->oid_head, &p->one->oid);
+   oidcpy(&d->oid_index, &p->two->oid);
break;
case DIFF_STATUS_UNMERGED:
d->stagemask = unmerged_mask(p->two->path);
+   /*
+* Don't bother setting {mode,oid}_{head,index} since 
the print
+* code will output the stage values directly and not 
use the
+* values in these fields.
+*/
break;
}
}
@@ -565,9 +614,17 @@ static void wt_status_collect_changes_initial(struct 
wt_status *s)
if (ce_stage(ce)) {
d->index_status = DIFF_STATUS_UNMERGED;
  

[PATCH v7 7/9] git-status.txt: describe --porcelain=v2 format

2016-08-11 Thread Jeff Hostetler
From: Jeff Hostetler 

Update status manpage to include information about
porcelain v2 format.

Signed-off-by: Jeff Hostetler 
---
 Documentation/git-status.txt | 126 +--
 1 file changed, 122 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 6b1454b..a58973b 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -183,12 +183,12 @@ in which case `XY` are `!!`.
 
 If -b is used the short-format status is preceded by a line
 
-## branchname tracking info
+## branchname tracking info
 
-Porcelain Format
-
+Porcelain Format Version 1
+~~
 
-The porcelain format is similar to the short format, but is guaranteed
+Version 1 porcelain format is similar to the short format, but is guaranteed
 not to change in a backwards-incompatible way between Git versions or
 based on user configuration. This makes it ideal for parsing by scripts.
 The description of the short format above also describes the porcelain
@@ -210,6 +210,124 @@ field from the first filename).  Third, filenames 
containing special
 characters are not specially formatted; no quoting or
 backslash-escaping is performed.
 
+Porcelain Format Version 2
+~~
+
+Version 2 format adds more detailed information about the state of
+the worktree and changed items.  Version 2 also defines an extensible
+set of easy to parse optional headers.
+
+Header lines start with "#" and are added in response to specific
+command line arguments.  Parsers should ignore headers they
+don't recognize.
+
+### Branch Headers
+
+If `--branch` is given, a series of header lines are printed with
+information about the current branch.
+
+Line Notes
+
+# branch.oid  | (initial)Current commit.
+# branch.head  | (detached)  Current branch.
+# branch.upstream   If upstream is set.
+# branch.ab + -   If upstream is set and
+ the commit is present.
+
+
+### Changed Tracked Entries
+
+Following the headers, a series of lines are printed for tracked
+entries.  One of three different line formats may be used to describe
+an entry depending on the type of change.  Tracked entries are printed
+in an undefined order; parsers should allow for a mixture of the 3
+line types in any order.
+
+Ordinary changed entries have the following format:
+
+1
+
+Renamed or copied entries have the following format:
+
+2 
+
+Field   Meaning
+
+A 2 character field containing the staged and
+unstaged XY values described in the short format,
+with unchanged indicated by a "." rather than
+a space.
+   A 4 character field describing the submodule state.
+"N..." when the entry is not a submodule.
+"S" when the entry is a submodule.
+ is "C" if the commit changed; otherwise ".".
+ is "M" if it has tracked changes; otherwise ".".
+ is "U" if there are untracked changes; otherwise ".".
+The octal file mode in HEAD.
+The octal file mode in the index.
+The octal file mode in the worktree.
+The object name in HEAD.
+The object name in the index.
+  The rename or copy score (denoting the percentage
+of similarity between the source and target of the
+move or copy). For example "R100" or "C75".
+  The pathname.  In a renamed/copied entry, this
+is the path in the index and in the working tree.
+   When the `-z` option is used, the 2 pathnames are separated
+with a NUL (ASCII 0x00) byte; otherwise, a tab (ASCII 0x09)
+byte separates them.
+  The pathname in the commit at HEAD.  This is only
+present in a renamed/copied entry, and tells
+where the renamed/copied contents came from.
+
+
+Unmerged entries have the following format; the first character is
+a "u" to distinguish from ordinary changed entries.
+
+u  
+
+Field   Meaning
+
+A 2 character field describing the conflict type
+as described in the short format.
+   A 4 character field describing the submodule state
+as described above.
+The octal file mode in stage 1.
+The octal file mode in stage 2.
+The octal file mode in stage 3.
+The octal file mode in the worktree.
+

[PATCH v7 8/9] test-lib-functions.sh: Add lf_to_nul

2016-08-11 Thread Jeff Hostetler
From: Jeff Hostetler 

Add lf_to_nul() function to test-lib-functions.

Signed-off-by: Jeff Hostetler 
---
 t/test-lib-functions.sh | 4 
 1 file changed, 4 insertions(+)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 4f7eadb..fdaeb3a 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -81,6 +81,10 @@ test_decode_color () {
'
 }
 
+lf_to_nul () {
+   perl -pe 'y/\012/\000/'
+}
+
 nul_to_q () {
perl -pe 'y/\000/Q/'
 }
-- 
2.8.0.rc4.17.gac42084.dirty

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


[PATCH v7 0/9] status: V2 porcelain status

2016-08-11 Thread Jeff Hostetler
From: Jeff Hostetler 

This patch series adds porcelain V2 format to status.
This provides detailed information about file changes
and about the current branch.

The new output is accessed via:
git status --porcelain=v2 [--branch]

This v7 patch series address the most recent feedback
on the unit tests.

This series has been rebased onto a more current master
branch (to get the EMPTY_BLOB changes in the test suite).

Jeff Hostetler (9):
  status: rename long-format print routines
  status: cleanup API to wt_status_print
  status: support --porcelain[=]
  status: collect per-file data for --porcelain=v2
  status: print per-file porcelain v2 status data
  status: print branch info with --porcelain=v2 --branch
  git-status.txt: describe --porcelain=v2 format
  test-lib-functions.sh: Add lf_to_nul
  status: unit tests for --porcelain=v2

 Documentation/git-status.txt | 133 +-
 builtin/commit.c |  78 +++---
 t/t7060-wtstatus.sh  |  21 ++
 t/t7064-wtstatus-pv2.sh  | 592 +++
 t/test-lib-functions.sh  |   4 +
 wt-status.c  | 570 -
 wt-status.h  |  19 +-
 7 files changed, 1305 insertions(+), 112 deletions(-)
 create mode 100755 t/t7064-wtstatus-pv2.sh

-- 
2.8.0.rc4.17.gac42084.dirty

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


Warning: You tried to send an email with blocked content

2016-08-11 Thread Service Desk
The UCL E-Mail Virus Protection System has been triggered by a message you sent.

One or more of the original e-mail attachments have been removed and replaced 
with this message.

* The attachment may have contained a virus or malware
* The attachment may have an extension of a type unacceptable for UCL

Consider renaming the file extension (eg rename file.ext to file.ex_)
to avoid this constraint.  The recipient will be required to rename the
file back to its original extension.

--
UCL E-mail Virus Protection System
serviced...@ucl.ac.uk
Phone: +44 (0)20 767925000 
Internal phone: 25000

--- Additional Information ---:

Subject: 
Sender: git@vger.kernel.org

Time received: 8/11/2016 8:20:16 PM
Message ID:<000101d1f440$0e128336$c0a80...@cyberturbo.com.br>
Detections found: 
059015008745095s.guillas.zip JS/Nemucod.OB
--
To unsubscribe from this list: send the line "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 2/2] diff: add SUBMODULE_DIFF format to display submodule diff

2016-08-11 Thread Junio C Hamano
Jacob Keller  writes:

> diff --git a/submodule.c b/submodule.c
> index 1b5cdfb7e784..36f8fd00c3ce 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -333,6 +333,73 @@ static void print_submodule_summary(struct rev_info 
> *rev, FILE *f,
>   strbuf_release(&sb);
>  }
>  
> +void show_submodule_diff(FILE *f, const char *path,
> + const char *line_prefix,
> + unsigned char one[20], unsigned char two[20],
> + unsigned dirty_submodule, const char *meta,
> + const char *a_prefix, const char *b_prefix,
> + const char *reset)
> +{
> + struct child_process cp = CHILD_PROCESS_INIT;
> + struct strbuf sb = STRBUF_INIT;
> + struct strbuf submodule_git_dir = STRBUF_INIT;
> + const char *git_dir, *message = NULL;
> + int create_dirty_diff = 0;
> + FILE *diff;
> + char c;

The variables message, diff and c are not used, are they?
--
To unsubscribe from this list: send the line "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: public-inbox.org/git updates

2016-08-11 Thread Junio C Hamano
Eric Wong  writes:

> There'll be over 5000K injected messages from 2006 I missed from
> the initial import :x
>
> I noticed this while adding gmane: mapping support to the
> search engine:
>   https://public-inbox.org/git/20160811002819.GA8311@starla/T/#u
>
> There will still be some missing messages because some are spam.
> news.gmane.org remains up if you want to check my work
> (please do, because I am careless)

Thanks for doing this.

I wanted to try out your NTTP service, but it took me a while to dig
in my inbox to find your announcement of news.public-inbox.org that
hosts inbox.comp.version-control.git "newsgroup".  Is it possible to
make this a bit more discoverable, or there is not enough NNTP
audience these days to warrant such an addition?  I first went to
http://public-inbox.org/git as I expected there may be some pointers
to other instances of the service, where you list the "git clone"
URL of the archive.

By the way, it felt quite strange to see messages from 8 years ago
mixed with more recent messages when I gold Gnus to fetch the most
recent 333 messages (and of course that fetches 333 messages with
largest message numbers, not sorted by "Date:").  I am assuming that
this is an artifact of "over 5k injected messages" bundle that was
added out of order.

--
To unsubscribe from this list: send the line "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 12/13] apply: learn to use a different index file

2016-08-11 Thread Junio C Hamano
Christian Couder  writes:

> Sometimes we want to apply in a different index file.
>
> Before the apply functionality was libified it was possible to
> use the GIT_INDEX_FILE environment variable, for this purpose.
>
> But now, as the apply functionality has been libified, it should
> be possible to do that in a libified way.
>
> Signed-off-by: Christian Couder 
> ---
>  apply.c | 10 --
>  apply.h |  1 +
>  2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/apply.c b/apply.c
> index 2ec2a8a..7e561a4 100644
> --- a/apply.c
> +++ b/apply.c
> @@ -4674,8 +4674,14 @@ static int apply_patch(struct apply_state *state,
>   state->apply = 0;
>  
>   state->update_index = state->check_index && state->apply;
> - if (state->update_index && state->newfd < 0)
> - state->newfd = hold_locked_index(state->lock_file, 1);
> + if (state->update_index && state->newfd < 0) {
> + if (state->index_file)
> + state->newfd = 
> hold_lock_file_for_update(state->lock_file,
> +  
> state->index_file,
> +  
> LOCK_DIE_ON_ERROR);
> + else
> + state->newfd = hold_locked_index(state->lock_file, 1);
> + }
>  
>   if (state->check_index && read_cache() < 0) {
>   error(_("unable to read index file"));

Here is a call to read_cache() that reads the default index file on
the filesystem into the default in-core index "the_index".

Shouldn't it be reading from state->index_file instead?

If we limit the review only to the context of your series, I think

fall_back_threeway()
 -> build_fake_ancestor() -- uses index_path to use custom index
 -> discard_cache()
 -> read_cache_from(index_path) -- reads back the fake ancestor
 -> write_index_as_tree() -- writes the fake_ancestor
 -> run_apply(index_path)
-> apply_all_patches()
   -> apply_patch()

is the only codepath that uses a custom index file, so when the
control reaches this function with a custom index file, the in-core
index is already populated, making read_cache() a no-op, and that is
the only thing that makes the resulting code avoid triggering this
bug, but as part of a general "libified" codepath, I think it should
be made to read from state->index_file using read_cache_from().

I only noticed this call to read_cache(), but there may be others
lurking nearby.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] submodule: mark a file-local symbol as static

2016-08-11 Thread Stefan Beller
On Thu, Aug 11, 2016 at 12:57 PM, Ramsay Jones
 wrote:
>
> Signed-off-by: Ramsay Jones 
> ---
>
> Hi Stefan,
>
> If you need to re-roll your 'sb/submodule-clone-rr' branch, could
> you please squash this into the relevant patch (commit 336c21d,
> "submodule: try alternates when superproject has an alternate",
> 08-08-2016).

Not just reroll but rethink and rewrite. ;)

Thanks for catching!
Stefan

>
> Thanks!
>
> ATB,
> Ramsay Jones
>
>  builtin/submodule--helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 4c765e1..4c7d03c 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -641,7 +641,7 @@ struct submodule_alternate_setup {
> struct strbuf *out;
>  };
>
> -int add_possible_reference(struct alternate_object_database *alt, void 
> *sas_cb)
> +static int add_possible_reference(struct alternate_object_database *alt, 
> void *sas_cb)
>  {
> struct submodule_alternate_setup *sas = sas_cb;
>
> --
> 2.9.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


[PATCH] submodule: mark a file-local symbol as static

2016-08-11 Thread Ramsay Jones

Signed-off-by: Ramsay Jones 
---

Hi Stefan,

If you need to re-roll your 'sb/submodule-clone-rr' branch, could
you please squash this into the relevant patch (commit 336c21d,
"submodule: try alternates when superproject has an alternate",
08-08-2016).

Thanks!

ATB,
Ramsay Jones

 builtin/submodule--helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 4c765e1..4c7d03c 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -641,7 +641,7 @@ struct submodule_alternate_setup {
struct strbuf *out;
 };
 
-int add_possible_reference(struct alternate_object_database *alt, void *sas_cb)
+static int add_possible_reference(struct alternate_object_database *alt, void 
*sas_cb)
 {
struct submodule_alternate_setup *sas = sas_cb;
 
-- 
2.9.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


public-inbox.org/git updates

2016-08-11 Thread Eric Wong
This mainly affects the folks following the top-level
Atom feed at https://public-inbox.org/git/new.atom
or over NNTP.

There'll be over 5000K injected messages from 2006 I missed from
the initial import :x

I noticed this while adding gmane: mapping support to the
search engine:
  https://public-inbox.org/git/20160811002819.GA8311@starla/T/#u

There will still be some missing messages because some are spam.
news.gmane.org remains up if you want to check my work
(please do, because I am careless)


Also, the following two .onions are geographically separate
so it should remain up if the main server at
http://ou63pmih66umazou.onion/git/ goes down.  These two are
on better hardware than the main one:

http://czquwvybam4bgbro.onion/git/
http://hjrcffqmbrq6wope.onion/git/

Users without tor installed may use one of the MITM proxies at
run by www.tor2web.org, too.

And yes, I break stuff all the time and often run barely-tested
development code on my server(s) :>


Finally, HTTPS termination for public-inbox.org is provided by
yahns, a Ruby/Rack server: git clone git://yhbt.net/yahns
(more barely-tested code running off ruby/trunk)
--
To unsubscribe from this list: send the line "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: t0027 racy?

2016-08-11 Thread Junio C Hamano
Torsten Bögershausen  writes:

> Good ideas, I will work on a series that fixes bugs first, and then we
> can see if there is room for optimization.
>
> What do you think about this as a starting point, more things will
> follow.
> I like to here comments about the commit msg first ;-)

Throughout t0027 there is no mention on what NNO stands for.  Are
they about operations that result in un-normalized index entries?

> commit 3754404d3d1ea4a0cbbed4986cc4ac1b5fe6b66e
> Author: Torsten Bögershausen 
> Date:   Thu Aug 11 18:47:29 2016 +0200
>
> t0027: Correct raciness in NNO test
> 
> When a non-reversible CRLF conversion is done in "git add",
> a warning is printed on stderr.
> 
> The commit_chk_wrnNNO() function  in t0027 was written to test this,
> but did the wrong thing: Instead of looking at the warning
> from "git add", it looked at the warning from "git commit".
> 
> Correct this and replace the commit for each and every file with a commit
> of all files in one go.
> 
> The function commit_chk_wrnNNO() will to be renamed in a separate commit.
> Thanks to Jeff King  for analizing t0027.
> Reporyed-By: Johannes Schindelin 

Reporyed?  

An obligatory "comments-about-the-commit-msg";-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v10 33/40] environment: add set_index_file()

2016-08-11 Thread Junio C Hamano
Christian Couder  writes:

> Yeah, it is feasible and perhaps even simpler using
> hold_lock_file_for_update() than with set_index_file(), so I
> dropped the set_index_file() patch and added a new one that uses
> hold_lock_file_for_update().

I wasn't paying too close an attention while reading the changes,
but anyway that is a great news.

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: Can cc's be included in patches sent by send-email

2016-08-11 Thread Philip Oakley

From: "Junio C Hamano" 

Jacob Keller  writes:

On Thu, Aug 11, 2016 at 12:58 AM, Jacob Keller  
wrote:
On Thu, Aug 11, 2016 at 12:32 AM, Philip Oakley  
wrote:
While 'git send-email' can have multiple --cc="addressee" options on 
the
command line, is it possible for the "cc:addressee" to actually 
be
included in the patches that are to be sent, so that different patches 
can

have different addressee?

The fortmat-patch can include appropriate from lines, so it feels as if 
the
sender should be able to add cc: lines at the same place. Maybe its 
just

part of th docs I've missed.



Yes, just put them in the body as tags below the signed-off-by. It
should be on by default unless you change supresscc or signedoffbycc
configuration.

Thanks,
Jake



See --suppress-cc or --signed-off-by-cc help in git help send-email.


Also, those who do not want to see Cc: in headers (like me) can
instead edit the header part of the format-patch output to add Cc:
lines and they should be picked up.
--
When done via git gui, it looks like the Cc: is also included in the commit 
message (first initial tries)


I'd probably place them after a --- break so that they don't get into the 
applied commit message, but will carry around during my rebasing. I've still 
to test if it works though.


Philip 


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


Re: [PATCH v10 33/40] environment: add set_index_file()

2016-08-11 Thread Christian Couder
On Wed, Aug 10, 2016 at 7:34 PM, Junio C Hamano  wrote:
> Christian Couder  writes:
>
>>> Isn't the mention on NO_THE_INDEX_COMPATIBILITY_MACROS in the added
>>> comments (there are two) pure red-herring?
>>
>> Yeah, true.
>>
>> So do you want me to refactor the code to use
>> hold_lock_file_for_update() instead of hold_locked_index() and to
>> avoid the set_index_file() hack?
>
> I somehow thought that we already agreed to accept the technical
> debt, taking your "it is too much work" assessment at the face
> value.  If you now think it is feasible within the scope of the
> series, of course we'd prefer it done "right" ;-)

Yeah, it is feasible and perhaps even simpler using
hold_lock_file_for_update() than with set_index_file(), so I dropped
the set_index_file() patch and added a new one that uses
hold_lock_file_for_update().
Sorry to have understood only recently what you said in some previous
emails and thanks for the explanations.

> Is the problematic hold_locked_index() something you do yourself, or
> buried deep inside the callchain of a helper function you use?

It is something done by the apply code, so we can easily modify that.

>  I am
> sensing that it is the latter (otherwise you wouldn't be doing the
> hack or at least will trivially fixing it up in a later "now we did
> a faithful conversion from the previous code using GIT_INDEX_FILE
> enviornment variable in an earlier step. Let's clean it up by being
> in full control of what file we read from and write to" step in the
> series).

It was more a misunderstanding of how the index related functions are working.

> Do you also have an issue on the reading side (i.e. you write it out
> to temporary file and then later re-read the in-core cache from that
> temporary file, for example)?  Or is a single "write to a temporary
> index" the only thing you need to work around?

It looks like it is the latter.

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


Re: [PATCH v6 0/9] status: V2 porcelain status

2016-08-11 Thread Junio C Hamano
Jeff Hostetler  writes:

> From: Jeff Hostetler 
>
> This patch series adds porcelain V2 format to status.
> This provides detailed information about file changes
> and about the current branch.

Everything up to 8/9 I didn't have anything to comment on; I didn't
see anything that smelled fishy, or anything I'd want to improve.

I did have a few minor nits in 9/9, though.

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: t0027 racy?

2016-08-11 Thread Torsten Bögershausen
[]
> FWIW I would strongly prefer to use the warning of `git add` and not even
> bother with `git commit`. What we are interested in is the warning
> message, generated by convert_to_git(). 
The commit is needed, because we check the content of the commit later.
> Not using the first one and
[]
> 
> On that matter, I wonder whether there would be a chance to revamp t0027
> in a major way, with the following goals:
> 
> - to make it very obvious to the casual reader what is being tested
> 
> - to combine Git invocations when possible, e.g. running one big `git add`
>   on a couple of files and then verify the relevant parts of the output
> 
> - dramatically decreasing the time required to run the test, without
>   sacrificing correctness (I would wager a bet that not only a few of
>   those 1388 test cases essentially exercise identical code paths)
Good ideas, I will work on a series that fixes bugs first, and then we
can see if there is room for optimization.

What do you think about this as a starting point,
more things will follow.
I like to here comments about the commit msg first ;-)

commit 3754404d3d1ea4a0cbbed4986cc4ac1b5fe6b66e
Author: Torsten Bögershausen 
Date:   Thu Aug 11 18:47:29 2016 +0200

t0027: Correct raciness in NNO test

When a non-reversible CRLF conversion is done in "git add",
a warning is printed on stderr.

The commit_chk_wrnNNO() function  in t0027 was written to test this,
but did the wrong thing: Instead of looking at the warning
from "git add", it looked at the warning from "git commit".

Correct this and replace the commit for each and every file with a commit
of all files in one go.

The function commit_chk_wrnNNO() will to be renamed in a separate commit.
Thanks to Jeff King  for analizing t0027.
Reporyed-By: Johannes Schindelin 

diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index 2860d2d..6e44382 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -119,8 +119,7 @@ commit_chk_wrnNNO () {
fname=${pfx}_$f.txt &&
cp $f $fname &&
printf Z >>"$fname" &&
-   git -c core.autocrlf=$crlf add $fname 2>/dev/null &&
-   git -c core.autocrlf=$crlf commit -m "commit_$fname" $fname 
>"${pfx}_$f.err" 2>&1
+   git -c core.autocrlf=$crlf add $fname 2>"${pfx}_$f.err"
done
 
test_expect_success "commit NNO files crlf=$crlf attr=$attr LF" '
@@ -394,11 +393,10 @@ test_expect_success 'commit files attr=crlf' '
 
 # attrLFCRLF  CRLFmixLF   
LF_mix_CR   CRLFNUL
 commit_chk_wrnNNO ""  ""  false   """"""  ""   
   ""
-commit_chk_wrnNNO ""  ""  trueLF_CRLF   """"  ""   
   ""
+commit_chk_wrnNNO ""  ""  true""""""  ""   
   ""
 commit_chk_wrnNNO ""  ""  input   """"""  ""   
   ""
-
-commit_chk_wrnNNO "auto"  ""  false   "$WILC"   """"  ""   
   ""
-commit_chk_wrnNNO "auto"  ""  trueLF_CRLF   """"  ""   
   ""
+commit_chk_wrnNNO "auto"  ""  false   """"""  ""   
   ""
+commit_chk_wrnNNO "auto"  ""  true""""""  ""   
   ""
 commit_chk_wrnNNO "auto"  ""  input   """"""  ""   
   ""
 for crlf in true false input
 do
@@ -408,7 +406,7 @@ do
commit_chk_wrnNNO ""lf  $crlf   ""   CRLF_LFCRLF_LF 
 "" CRLF_LF
commit_chk_wrnNNO ""crlf$crlf   LF_CRLF   ""LF_CRLF 
LF_CRLF ""
commit_chk_wrnNNO auto  lf  $crlf   """"""  
""  ""
-   commit_chk_wrnNNO auto  crlf$crlf   LF_CRLF   """"  
""  ""
+   commit_chk_wrnNNO auto  crlf$crlf   """"""  
""  ""
commit_chk_wrnNNO text  lf  $crlf   ""   CRLF_LFCRLF_LF 
""  CRLF_LF
commit_chk_wrnNNO text  crlf$crlf   LF_CRLF   ""LF_CRLF 
LF_CRLF ""
 done
@@ -417,7 +415,8 @@ commit_chk_wrnNNO "text"  ""  false   "$WILC"   "$WICL" 
  "$WAMIX""$WILC
 commit_chk_wrnNNO "text"  ""  trueLF_CRLF   ""LF_CRLF 
LF_CRLF ""
 commit_chk_wrnNNO "text"  ""  input   ""CRLF_LF   CRLF_LF ""   
   CRLF_LF
 
-test_expect_success 'create files cleanup' '
+test_expect_success 'commit NNO and cleanup' '
+   git commit -m "commit files on top of NNO" &&
rm -f *.txt &&
git -c core.autocrlf=false reset --hard
 '
--
To unsubscribe from this list: send the line "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: Can cc's be included in patches sent by send-email

2016-08-11 Thread Philip Oakley

From: "Jacob Keller" 
On Thu, Aug 11, 2016 at 12:58 AM, Jacob Keller  
wrote:
On Thu, Aug 11, 2016 at 12:32 AM, Philip Oakley  
wrote:

While 'git send-email' can have multiple --cc="addressee" options on the
command line, is it possible for the "cc:addressee" to actually 
be
included in the patches that are to be sent, so that different patches 
can

have different addressee?

The fortmat-patch can include appropriate from lines, so it feels as if 
the

sender should be able to add cc: lines at the same place. Maybe its just
part of th docs I've missed.



Yes, just put them in the body as tags below the signed-off-by. It
should be on by default unless you change supresscc or signedoffbycc
configuration.

Thanks,
Jake



See --suppress-cc or --signed-off-by-cc help in git help send-email.

Thanks,
Jake


Thanks,
I'll look at that. It wasn't immediately obvious what to do.

Philip 


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


Re: [PATCH] gc: default aggressive depth to 50

2016-08-11 Thread Junio C Hamano
Jeff King  writes:

> On Thu, Aug 11, 2016 at 12:13:09PM -0400, Jeff King wrote:
>
>> Here are the numbers for linux.git:
>> 
>>depth |  size |  %| rev-list |  % | log -Sfoo |   %
>>   ---+---+---+--++---+---
>> 250  | 967MB |  n/a  | 48.159s  |   n/a  | 378.088   |   n/a
>> 100  | 971MB | +0.4% | 41.471s  | -13.9% | 342.060   |  -9.5%
>>  50  | 979MB | +1.2% | 37.778s  | -21.6% | 311.040s  | -17.7%
>>  10  | 1.1GB | +6.6% | 32.518s  | -32.5% | 279.890s  | -25.9%
>> [...]
>> 
>> You can see that that the CPU savings for regular operations improves as we
>> decrease the depth. The savings are less for "rev-list" on a smaller 
>> repository
>> than they are for blob-accessing operations, or even rev-list on a larger
>> repository. This may mean that a larger delta cache would help (though 
>> setting
>> core.deltaBaseCacheLimit by itself doesn't).
>
> The problem with deltaBaseCacheLimit is that it only changes the memory
> parameter, but there are a fixed number of slots in the data structure.
> Bumping it like this:
>
> diff --git a/sha1_file.c b/sha1_file.c
> index 02940f1..ca79703 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -2073,7 +2073,7 @@ static void *unpack_compressed_entry(struct packed_git 
> *p,
>   return buffer;
>  }
>  
> -#define MAX_DELTA_CACHE (256)
> +#define MAX_DELTA_CACHE (1024)
>  
>  static size_t delta_base_cached;
>
> along with the cache size does help (this was discussed a year or two
> ago, but nobody ever followed up with numbers or patches).

Yeah, and I also think Linus's "--depth=250 is just a sample; it
will not perform well" already cited the number of delta-cache
entries being the limiting factor.

> I don't think bumping MAX_DELTA_CACHE naively is a good idea, though. I
> seem to recall that it has scaling problems as it grows, so we may want
> a better data structure (but I haven't looked at it recently enough to
> say anything intelligent).

Me neither.  In any case, I do think reducing the aggressive depth
down to 50 is a very sensible move.  I also suspect that window size
may want to be a bit increased (or even made dynamic; the first time
we need the window size determined is after to_pack.objects[] array
is fully populated, so we could use the number of commits as one of
the hint, for example), but that can be treated as a separate topic.


--
To unsubscribe from this list: send the line "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 v12 13/13] builtin/am: use apply API in run_apply()

2016-08-11 Thread Christian Couder
This replaces run_apply() implementation with a new one that
uses the apply API that has been previously prepared in
apply.c and apply.h.

This shoud improve performance a lot in certain cases.

As the previous implementation was creating a new `git apply`
process to apply each patch, it could be slow on systems like
Windows where it is costly to create new processes.

Also the new `git apply` process had to read the index from
disk, and when the process was done the calling process
discarded its own index and read back from disk the new
index that had been created by the `git apply` process.

This could be very inefficient with big repositories that
have big index files, especially when the system decided
that it was a good idea to run the `git apply` processes on
a different processor core.

Also eliminating index reads enables further performance
improvements by using:

`git update-index --split-index`

For example here is a benchmark of a multi hundred commit
rebase on the Linux kernel on a Debian laptop with SSD:

command: git rebase --onto 1993b17 52bef0c 29dde7c

Vanilla "next" without split index:1m54.953s
Vanilla "next" with split index:   1m22.476s
This series on top of "next" without split index:  1m12.034s
This series on top of "next" with split index: 0m15.678s

(using branch "next" from mid April 2016.)

Benchmarked-by: Ævar Arnfjörð Bjarmason 
Signed-off-by: Christian Couder 
---
 builtin/am.c | 65 
 1 file changed, 43 insertions(+), 22 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 739b34d..0e5d384 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -28,6 +28,7 @@
 #include "rerere.h"
 #include "prompt.h"
 #include "mailinfo.h"
+#include "apply.h"
 
 /**
  * Returns 1 if the file is empty or does not exist, 0 otherwise.
@@ -1522,39 +1523,59 @@ static int parse_mail_rebase(struct am_state *state, 
const char *mail)
  */
 static int run_apply(const struct am_state *state, const char *index_file)
 {
-   struct child_process cp = CHILD_PROCESS_INIT;
-
-   cp.git_cmd = 1;
-
-   if (index_file)
-   argv_array_pushf(&cp.env_array, "GIT_INDEX_FILE=%s", 
index_file);
+   struct argv_array apply_paths = ARGV_ARRAY_INIT;
+   struct argv_array apply_opts = ARGV_ARRAY_INIT;
+   struct apply_state apply_state;
+   int res, opts_left;
+   static struct lock_file lock_file;
+   int force_apply = 0;
+   int options = 0;
+
+   if (init_apply_state(&apply_state, NULL, &lock_file))
+   die("BUG: init_apply_state() failed");
+
+   argv_array_push(&apply_opts, "apply");
+   argv_array_pushv(&apply_opts, state->git_apply_opts.argv);
+
+   opts_left = apply_parse_options(apply_opts.argc, apply_opts.argv,
+   &apply_state, &force_apply, &options,
+   NULL);
+
+   if (opts_left != 0)
+   die("unknown option passed thru to git apply");
+
+   if (index_file) {
+   apply_state.index_file = index_file;
+   apply_state.cached = 1;
+   } else
+   apply_state.check_index = 1;
 
/*
 * If we are allowed to fall back on 3-way merge, don't give false
 * errors during the initial attempt.
 */
-   if (state->threeway && !index_file) {
-   cp.no_stdout = 1;
-   cp.no_stderr = 1;
-   }
+   if (state->threeway && !index_file)
+   apply_state.apply_verbosity = verbosity_silent;
 
-   argv_array_push(&cp.args, "apply");
+   if (check_apply_state(&apply_state, force_apply))
+   die("BUG: check_apply_state() failed");
 
-   argv_array_pushv(&cp.args, state->git_apply_opts.argv);
+   argv_array_push(&apply_paths, am_path(state, "patch"));
 
-   if (index_file)
-   argv_array_push(&cp.args, "--cached");
-   else
-   argv_array_push(&cp.args, "--index");
+   res = apply_all_patches(&apply_state, apply_paths.argc, 
apply_paths.argv, options);
 
-   argv_array_push(&cp.args, am_path(state, "patch"));
+   argv_array_clear(&apply_paths);
+   argv_array_clear(&apply_opts);
+   clear_apply_state(&apply_state);
 
-   if (run_command(&cp))
-   return -1;
+   if (res)
+   return res;
 
-   /* Reload index as git-apply will have modified it. */
-   discard_cache();
-   read_cache_from(index_file ? index_file : get_index_file());
+   if (index_file) {
+   /* Reload index as apply_all_patches() will have modified it. */
+   discard_cache();
+   read_cache_from(index_file);
+   }
 
return 0;
 }
-- 
2.9.2.769.gc0f0333

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

[PATCH v12 10/13] apply: change error_routine when silent

2016-08-11 Thread Christian Couder
To avoid printing anything when applying with
`state->apply_verbosity == verbosity_silent`, let's save the
existing warn and error routines before applying, and let's
replace them with a routine that does nothing.

Then after applying, let's restore the saved routines.

Helped-by: Stefan Beller 
Signed-off-by: Christian Couder 
---
 apply.c | 21 -
 apply.h |  8 
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/apply.c b/apply.c
index ddbb0a2..bf81b70 100644
--- a/apply.c
+++ b/apply.c
@@ -112,6 +112,11 @@ void clear_apply_state(struct apply_state *state)
/* &state->fn_table is cleared at the end of apply_patch() */
 }
 
+static void mute_routine(const char *bla, va_list params)
+{
+   /* do nothing */
+}
+
 int check_apply_state(struct apply_state *state, int force_apply)
 {
int is_not_gitdir = !startup_info->have_repository;
@@ -144,6 +149,13 @@ int check_apply_state(struct apply_state *state, int 
force_apply)
if (!state->lock_file)
return error("BUG: state->lock_file should not be NULL");
 
+   if (state->apply_verbosity <= verbosity_silent) {
+   state->saved_error_routine = get_error_routine();
+   state->saved_warn_routine = get_warn_routine();
+   set_error_routine(mute_routine);
+   set_warn_routine(mute_routine);
+   }
+
return 0;
 }
 
@@ -4864,7 +4876,7 @@ int apply_all_patches(struct apply_state *state,
state->newfd = -1;
}
 
-   return !!errs;
+   res = !!errs;
 
 end:
if (state->newfd >= 0) {
@@ -4872,5 +4884,12 @@ int apply_all_patches(struct apply_state *state,
state->newfd = -1;
}
 
+   if (state->apply_verbosity <= verbosity_silent) {
+   set_error_routine(state->saved_error_routine);
+   set_warn_routine(state->saved_warn_routine);
+   }
+
+   if (res > -1)
+   return res;
return (res == -1 ? 1 : 128);
 }
diff --git a/apply.h b/apply.h
index bd4eb6d..5cde641 100644
--- a/apply.h
+++ b/apply.h
@@ -94,6 +94,14 @@ struct apply_state {
 */
struct string_list fn_table;
 
+   /*
+* This is to save reporting routines before using
+* set_error_routine() or set_warn_routine() to install muting
+* routines when in verbosity_silent mode.
+*/
+   void (*saved_error_routine)(const char *err, va_list params);
+   void (*saved_warn_routine)(const char *warn, va_list params);
+
/* These control whitespace errors */
enum apply_ws_error_action ws_error_action;
enum apply_ws_ignore ws_ignore_action;
-- 
2.9.2.769.gc0f0333

--
To unsubscribe from this list: send the line "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 v12 08/13] usage: add set_warn_routine()

2016-08-11 Thread Christian Couder
There are already set_die_routine() and set_error_routine(),
so let's add set_warn_routine() as this will be needed in a
following commit.

Signed-off-by: Christian Couder 
---
 git-compat-util.h | 1 +
 usage.c   | 5 +
 2 files changed, 6 insertions(+)

diff --git a/git-compat-util.h b/git-compat-util.h
index 590bfdd..c7a51f8 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -440,6 +440,7 @@ static inline int const_error(void)
 
 extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, 
va_list params));
 extern void set_error_routine(void (*routine)(const char *err, va_list 
params));
+extern void set_warn_routine(void (*routine)(const char *warn, va_list 
params));
 extern void set_die_is_recursing_routine(int (*routine)(void));
 extern void set_error_handle(FILE *);
 
diff --git a/usage.c b/usage.c
index 1dad03f..67e5526 100644
--- a/usage.c
+++ b/usage.c
@@ -70,6 +70,11 @@ void set_error_routine(void (*routine)(const char *err, 
va_list params))
error_routine = routine;
 }
 
+void set_warn_routine(void (*routine)(const char *warn, va_list params))
+{
+   warn_routine = routine;
+}
+
 void set_die_is_recursing_routine(int (*routine)(void))
 {
die_is_recursing = routine;
-- 
2.9.2.769.gc0f0333

--
To unsubscribe from this list: send the line "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 v12 09/13] usage: add get_error_routine() and get_warn_routine()

2016-08-11 Thread Christian Couder
Let's make it possible to get the current error_routine and warn_routine,
so that we can store them before using set_error_routine() or
set_warn_routine() to use new ones.

This way we will be able put back the original routines, when we are done
with using new ones.

Signed-off-by: Christian Couder 
---
 git-compat-util.h |  2 ++
 usage.c   | 10 ++
 2 files changed, 12 insertions(+)

diff --git a/git-compat-util.h b/git-compat-util.h
index c7a51f8..de04df1 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -440,7 +440,9 @@ static inline int const_error(void)
 
 extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, 
va_list params));
 extern void set_error_routine(void (*routine)(const char *err, va_list 
params));
+extern void (*get_error_routine(void))(const char *err, va_list params);
 extern void set_warn_routine(void (*routine)(const char *warn, va_list 
params));
+extern void (*get_warn_routine(void))(const char *warn, va_list params);
 extern void set_die_is_recursing_routine(int (*routine)(void));
 extern void set_error_handle(FILE *);
 
diff --git a/usage.c b/usage.c
index 67e5526..2fd3045 100644
--- a/usage.c
+++ b/usage.c
@@ -70,11 +70,21 @@ void set_error_routine(void (*routine)(const char *err, 
va_list params))
error_routine = routine;
 }
 
+void (*get_error_routine(void))(const char *err, va_list params)
+{
+   return error_routine;
+}
+
 void set_warn_routine(void (*routine)(const char *warn, va_list params))
 {
warn_routine = routine;
 }
 
+void (*get_warn_routine(void))(const char *warn, va_list params)
+{
+   return warn_routine;
+}
+
 void set_die_is_recursing_routine(int (*routine)(void))
 {
die_is_recursing = routine;
-- 
2.9.2.769.gc0f0333

--
To unsubscribe from this list: send the line "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 v12 11/13] apply: refactor `git apply` option parsing

2016-08-11 Thread Christian Couder
Parsing `git apply` options can be useful to other commands that
want to call the libified apply functionality, because this way
they can easily pass some options from their own command line to
the libified apply functionality.

This will be used by `git am` in a following patch.

To make this possible, let's refactor the `git apply` option
parsing code into a new libified apply_parse_options() function.

Doing that makes it possible to remove some functions definitions
from "apply.h" and make them static in "apply.c".

Helped-by: Ramsay Jones 
Signed-off-by: Christian Couder 
---
 apply.c | 103 +---
 apply.h |  18 +++---
 builtin/apply.c |  74 ++--
 3 files changed, 97 insertions(+), 98 deletions(-)

diff --git a/apply.c b/apply.c
index bf81b70..2ec2a8a 100644
--- a/apply.c
+++ b/apply.c
@@ -4730,16 +4730,16 @@ static int apply_patch(struct apply_state *state,
return res;
 }
 
-int apply_option_parse_exclude(const struct option *opt,
-  const char *arg, int unset)
+static int apply_option_parse_exclude(const struct option *opt,
+ const char *arg, int unset)
 {
struct apply_state *state = opt->value;
add_name_limit(state, arg, 1);
return 0;
 }
 
-int apply_option_parse_include(const struct option *opt,
-  const char *arg, int unset)
+static int apply_option_parse_include(const struct option *opt,
+ const char *arg, int unset)
 {
struct apply_state *state = opt->value;
add_name_limit(state, arg, 0);
@@ -4747,9 +4747,9 @@ int apply_option_parse_include(const struct option *opt,
return 0;
 }
 
-int apply_option_parse_p(const struct option *opt,
-const char *arg,
-int unset)
+static int apply_option_parse_p(const struct option *opt,
+   const char *arg,
+   int unset)
 {
struct apply_state *state = opt->value;
state->p_value = atoi(arg);
@@ -4757,8 +4757,8 @@ int apply_option_parse_p(const struct option *opt,
return 0;
 }
 
-int apply_option_parse_space_change(const struct option *opt,
-   const char *arg, int unset)
+static int apply_option_parse_space_change(const struct option *opt,
+  const char *arg, int unset)
 {
struct apply_state *state = opt->value;
if (unset)
@@ -4768,8 +4768,8 @@ int apply_option_parse_space_change(const struct option 
*opt,
return 0;
 }
 
-int apply_option_parse_whitespace(const struct option *opt,
- const char *arg, int unset)
+static int apply_option_parse_whitespace(const struct option *opt,
+const char *arg, int unset)
 {
struct apply_state *state = opt->value;
state->whitespace_option = arg;
@@ -4778,8 +4778,8 @@ int apply_option_parse_whitespace(const struct option 
*opt,
return 0;
 }
 
-int apply_option_parse_directory(const struct option *opt,
-const char *arg, int unset)
+static int apply_option_parse_directory(const struct option *opt,
+   const char *arg, int unset)
 {
struct apply_state *state = opt->value;
strbuf_reset(&state->root);
@@ -4893,3 +4893,80 @@ int apply_all_patches(struct apply_state *state,
return res;
return (res == -1 ? 1 : 128);
 }
+
+int apply_parse_options(int argc, const char **argv,
+   struct apply_state *state,
+   int *force_apply, int *options,
+   const char * const *apply_usage)
+{
+   struct option builtin_apply_options[] = {
+   { OPTION_CALLBACK, 0, "exclude", state, N_("path"),
+   N_("don't apply changes matching the given path"),
+   0, apply_option_parse_exclude },
+   { OPTION_CALLBACK, 0, "include", state, N_("path"),
+   N_("apply changes matching the given path"),
+   0, apply_option_parse_include },
+   { OPTION_CALLBACK, 'p', NULL, state, N_("num"),
+   N_("remove  leading slashes from traditional diff 
paths"),
+   0, apply_option_parse_p },
+   OPT_BOOL(0, "no-add", &state->no_add,
+   N_("ignore additions made by the patch")),
+   OPT_BOOL(0, "stat", &state->diffstat,
+   N_("instead of applying the patch, output diffstat for 
the input")),
+   OPT_NOOP_NOARG(0, "allow-binary-replacement"),
+   OPT_NOOP_NOARG(0, "binary"),
+   OPT_BOOL(0, "numstat", &state->numstat,
+   N_("show number o

[PATCH v12 12/13] apply: learn to use a different index file

2016-08-11 Thread Christian Couder
Sometimes we want to apply in a different index file.

Before the apply functionality was libified it was possible to
use the GIT_INDEX_FILE environment variable, for this purpose.

But now, as the apply functionality has been libified, it should
be possible to do that in a libified way.

Signed-off-by: Christian Couder 
---
 apply.c | 10 --
 apply.h |  1 +
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/apply.c b/apply.c
index 2ec2a8a..7e561a4 100644
--- a/apply.c
+++ b/apply.c
@@ -4674,8 +4674,14 @@ static int apply_patch(struct apply_state *state,
state->apply = 0;
 
state->update_index = state->check_index && state->apply;
-   if (state->update_index && state->newfd < 0)
-   state->newfd = hold_locked_index(state->lock_file, 1);
+   if (state->update_index && state->newfd < 0) {
+   if (state->index_file)
+   state->newfd = 
hold_lock_file_for_update(state->lock_file,
+
state->index_file,
+
LOCK_DIE_ON_ERROR);
+   else
+   state->newfd = hold_locked_index(state->lock_file, 1);
+   }
 
if (state->check_index && read_cache() < 0) {
error(_("unable to read index file"));
diff --git a/apply.h b/apply.h
index e2b89e8..1ba4f8d 100644
--- a/apply.h
+++ b/apply.h
@@ -63,6 +63,7 @@ struct apply_state {
int unsafe_paths;
 
/* Other non boolean parameters */
+   const char *index_file;
enum apply_verbosity apply_verbosity;
const char *fake_ancestor;
const char *patch_input_file;
-- 
2.9.2.769.gc0f0333

--
To unsubscribe from this list: send the line "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 v12 06/13] apply: make it possible to silently apply

2016-08-11 Thread Christian Couder
This changes 'int apply_verbosely' into 'enum apply_verbosity', and
changes the possible values of the variable from a bool to
a tristate.

The previous 'false' state is changed into 'verbosity_normal'.
The previous 'true' state is changed into 'verbosity_verbose'.

The new added state is 'verbosity_silent'. It should prevent
anything to be printed on both stderr and stdout.

This is needed because `git am` wants to first call apply
functionality silently, if it can then fall back on 3-way merge
in case of error.

Printing on stdout, and calls to warning() or error() are not
taken care of in this patch, as that will be done in following
patches.

Signed-off-by: Christian Couder 
---
 apply.c | 62 +
 apply.h |  8 +++-
 builtin/apply.c |  2 +-
 3 files changed, 48 insertions(+), 24 deletions(-)

diff --git a/apply.c b/apply.c
index 41a33d3..df85cbc 100644
--- a/apply.c
+++ b/apply.c
@@ -125,8 +125,11 @@ int check_apply_state(struct apply_state *state, int 
force_apply)
return error(_("--3way outside a repository"));
state->check_index = 1;
}
-   if (state->apply_with_reject)
-   state->apply = state->apply_verbosely = 1;
+   if (state->apply_with_reject) {
+   state->apply = 1;
+   if (state->apply_verbosity == verbosity_normal)
+   state->apply_verbosity = verbosity_verbose;
+   }
if (!force_apply && (state->diffstat || state->numstat || 
state->summary || state->check || state->fake_ancestor))
state->apply = 0;
if (state->check_index && is_not_gitdir)
@@ -1620,8 +1623,9 @@ static void record_ws_error(struct apply_state *state,
return;
 
err = whitespace_error_string(result);
-   fprintf(stderr, "%s:%d: %s.\n%.*s\n",
-   state->patch_input_file, linenr, err, len, line);
+   if (state->apply_verbosity > verbosity_silent)
+   fprintf(stderr, "%s:%d: %s.\n%.*s\n",
+   state->patch_input_file, linenr, err, len, line);
free(err);
 }
 
@@ -1816,7 +1820,7 @@ static int parse_single_patch(struct apply_state *state,
return error(_("new file %s depends on old contents"), 
patch->new_name);
if (0 < patch->is_delete && newlines)
return error(_("deleted file %s still has contents"), 
patch->old_name);
-   if (!patch->is_delete && !newlines && context)
+   if (!patch->is_delete && !newlines && context && state->apply_verbosity 
> verbosity_silent)
fprintf_ln(stderr,
   _("** warning: "
 "file %s becomes empty but is not deleted"),
@@ -2911,7 +2915,7 @@ static int apply_one_fragment(struct apply_state *state,
/* Ignore it, we already handled it */
break;
default:
-   if (state->apply_verbosely)
+   if (state->apply_verbosity > verbosity_normal)
error(_("invalid start of line: '%c'"), first);
applied_pos = -1;
goto out;
@@ -3026,7 +3030,7 @@ static int apply_one_fragment(struct apply_state *state,
state->apply = 0;
}
 
-   if (state->apply_verbosely && applied_pos != pos) {
+   if (state->apply_verbosity > verbosity_normal && applied_pos != 
pos) {
int offset = applied_pos - pos;
if (state->apply_in_reverse)
offset = 0 - offset;
@@ -3041,14 +3045,14 @@ static int apply_one_fragment(struct apply_state *state,
 * Warn if it was necessary to reduce the number
 * of context lines.
 */
-   if ((leading != frag->leading) ||
-   (trailing != frag->trailing))
+   if ((leading != frag->leading ||
+trailing != frag->trailing) && state->apply_verbosity > 
verbosity_silent)
fprintf_ln(stderr, _("Context reduced to (%ld/%ld)"
 " to apply fragment at %d"),
   leading, trailing, applied_pos+1);
update_image(state, img, applied_pos, &preimage, &postimage);
} else {
-   if (state->apply_verbosely)
+   if (state->apply_verbosity > verbosity_normal)
error(_("while searching for:\n%.*s"),
  (int)(old - oldlines), oldlines);
}
@@ -3539,7 +3543,8 @@ static int try_threeway(struct apply_state *state,
 read_blob_object(&buf, pre_sha1, patch->old_mode))
return error("repository lacks the necessary blob to fall back 
on 3-way merge.");
 
-   fprintf(stderr, 

[PATCH v12 01/13] builtin/apply: rename option parsing functions

2016-08-11 Thread Christian Couder
As these functions are going to be part of the libified
apply API, let's give them a name that is more specific
to the apply API.

Signed-off-by: Christian Couder 
---
 builtin/apply.c | 40 
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index ad403f8..429fe44 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4588,16 +4588,16 @@ static int apply_patch(struct apply_state *state,
return res;
 }
 
-static int option_parse_exclude(const struct option *opt,
-   const char *arg, int unset)
+static int apply_option_parse_exclude(const struct option *opt,
+ const char *arg, int unset)
 {
struct apply_state *state = opt->value;
add_name_limit(state, arg, 1);
return 0;
 }
 
-static int option_parse_include(const struct option *opt,
-   const char *arg, int unset)
+static int apply_option_parse_include(const struct option *opt,
+ const char *arg, int unset)
 {
struct apply_state *state = opt->value;
add_name_limit(state, arg, 0);
@@ -4605,9 +4605,9 @@ static int option_parse_include(const struct option *opt,
return 0;
 }
 
-static int option_parse_p(const struct option *opt,
- const char *arg,
- int unset)
+static int apply_option_parse_p(const struct option *opt,
+   const char *arg,
+   int unset)
 {
struct apply_state *state = opt->value;
state->p_value = atoi(arg);
@@ -4615,8 +4615,8 @@ static int option_parse_p(const struct option *opt,
return 0;
 }
 
-static int option_parse_space_change(const struct option *opt,
-const char *arg, int unset)
+static int apply_option_parse_space_change(const struct option *opt,
+  const char *arg, int unset)
 {
struct apply_state *state = opt->value;
if (unset)
@@ -4626,8 +4626,8 @@ static int option_parse_space_change(const struct option 
*opt,
return 0;
 }
 
-static int option_parse_whitespace(const struct option *opt,
-  const char *arg, int unset)
+static int apply_option_parse_whitespace(const struct option *opt,
+const char *arg, int unset)
 {
struct apply_state *state = opt->value;
state->whitespace_option = arg;
@@ -4636,8 +4636,8 @@ static int option_parse_whitespace(const struct option 
*opt,
return 0;
 }
 
-static int option_parse_directory(const struct option *opt,
- const char *arg, int unset)
+static int apply_option_parse_directory(const struct option *opt,
+   const char *arg, int unset)
 {
struct apply_state *state = opt->value;
strbuf_reset(&state->root);
@@ -4755,13 +4755,13 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix)
struct option builtin_apply_options[] = {
{ OPTION_CALLBACK, 0, "exclude", &state, N_("path"),
N_("don't apply changes matching the given path"),
-   0, option_parse_exclude },
+   0, apply_option_parse_exclude },
{ OPTION_CALLBACK, 0, "include", &state, N_("path"),
N_("apply changes matching the given path"),
-   0, option_parse_include },
+   0, apply_option_parse_include },
{ OPTION_CALLBACK, 'p', NULL, &state, N_("num"),
N_("remove  leading slashes from traditional diff 
paths"),
-   0, option_parse_p },
+   0, apply_option_parse_p },
OPT_BOOL(0, "no-add", &state.no_add,
N_("ignore additions made by the patch")),
OPT_BOOL(0, "stat", &state.diffstat,
@@ -4793,13 +4793,13 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix)
N_("ensure at least  lines of context 
match")),
{ OPTION_CALLBACK, 0, "whitespace", &state, N_("action"),
N_("detect new or modified lines that have whitespace 
errors"),
-   0, option_parse_whitespace },
+   0, apply_option_parse_whitespace },
{ OPTION_CALLBACK, 0, "ignore-space-change", &state, NULL,
N_("ignore changes in whitespace when finding context"),
-   PARSE_OPT_NOARG, option_parse_space_change },
+   PARSE_OPT_NOARG, apply_option_parse_space_change },
{ OPTION_CALLBACK, 0, "ignore-whitespace", &state, NULL,
N_("ignore changes in whitespace when finding context"),
-   

[PATCH v12 02/13] apply: rename and move opt constants to apply.h

2016-08-11 Thread Christian Couder
The constants for the "inaccurate-eof" and the "recount" options will
be used in both "apply.c" and "builtin/apply.c", so they need to go
into "apply.h", and therefore they need a name that is more specific
to the API they belong to.

Signed-off-by: Christian Couder 
---
 apply.h |  3 +++
 builtin/apply.c | 11 ---
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/apply.h b/apply.h
index 53f09b5..ca1dcee 100644
--- a/apply.h
+++ b/apply.h
@@ -108,4 +108,7 @@ extern int init_apply_state(struct apply_state *state,
 extern void clear_apply_state(struct apply_state *state);
 extern int check_apply_state(struct apply_state *state, int force_apply);
 
+#define APPLY_OPT_INACCURATE_EOF   (1<<0)
+#define APPLY_OPT_RECOUNT  (1<<1)
+
 #endif
diff --git a/builtin/apply.c b/builtin/apply.c
index 429fe44..9c396bb 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4463,9 +4463,6 @@ static int write_out_results(struct apply_state *state, 
struct patch *list)
 
 static struct lock_file lock_file;
 
-#define INACCURATE_EOF (1<<0)
-#define RECOUNT(1<<1)
-
 /*
  * Try to apply a patch.
  *
@@ -4495,8 +4492,8 @@ static int apply_patch(struct apply_state *state,
int nr;
 
patch = xcalloc(1, sizeof(*patch));
-   patch->inaccurate_eof = !!(options & INACCURATE_EOF);
-   patch->recount =  !!(options & RECOUNT);
+   patch->inaccurate_eof = !!(options & APPLY_OPT_INACCURATE_EOF);
+   patch->recount =  !!(options & APPLY_OPT_RECOUNT);
nr = parse_chunk(state, buf.buf + offset, buf.len - offset, 
patch);
if (nr < 0) {
free_patch(patch);
@@ -4811,10 +4808,10 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix)
OPT__VERBOSE(&state.apply_verbosely, N_("be verbose")),
OPT_BIT(0, "inaccurate-eof", &options,
N_("tolerate incorrectly detected missing new-line at 
the end of file"),
-   INACCURATE_EOF),
+   APPLY_OPT_INACCURATE_EOF),
OPT_BIT(0, "recount", &options,
N_("do not trust the line counts in the hunk headers"),
-   RECOUNT),
+   APPLY_OPT_RECOUNT),
{ OPTION_CALLBACK, 0, "directory", &state, N_("root"),
N_("prepend  to all filenames"),
0, apply_option_parse_directory },
-- 
2.9.2.769.gc0f0333

--
To unsubscribe from this list: send the line "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 v12 00/13] libify apply and use lib in am, part 3

2016-08-11 Thread Christian Couder
Goal


This is a patch series about libifying `git apply` functionality, and
using this libified functionality in `git am`, so that no 'git apply'
process is spawn anymore. This makes `git am` significantly faster, so
`git rebase`, when it uses the am backend, is also significantly
faster.

Previous discussions and patches series
~~~

This has initially been discussed in the following thread:

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

Then the following patch series were sent:

RFC: http://thread.gmane.org/gmane.comp.version-control.git/288489/
v1: http://thread.gmane.org/gmane.comp.version-control.git/292324/
v2: http://thread.gmane.org/gmane.comp.version-control.git/294248/
v3: http://thread.gmane.org/gmane.comp.version-control.git/295429/
v4: http://thread.gmane.org/gmane.comp.version-control.git/296350/
v5: http://thread.gmane.org/gmane.comp.version-control.git/296490/
v6: http://thread.gmane.org/gmane.comp.version-control.git/297024/
v7: http://thread.gmane.org/gmane.comp.version-control.git/297193/
v8: 
https://public-inbox.org/git/20160627182429.31550-1-chriscool%40tuxfamily.org/
v9: 
https://public-inbox.org/git/20160730172509.22939-1-chriscool%40tuxfamily.org/
v10: 
https://public-inbox.org/git/20160808210337.5038-1-chriscool%40tuxfamily.org/
v11: 
https://public-inbox.org/git/20160811085229.19017-1-chriscool%40tuxfamily.org/

Highlevel view of the patches in the series
~~~

This is "part 3" of the full patch series. I am resending only the
last 13 patches of the full series as "part 3", because I don't want
to resend the first 27 patches of v10 nearly as is. So "part 2" is
made of patch 01/40 from v11 and patches from 02/40 to 27/40 from v10.
The "part 1" is in "master" for some time now.

  - Patch 01/13 was v1, v2, v6, v7, v8, v9, v10 and v11.

Since v10 and v11 only the commit message has been changed. As
suggested by Stefan Naewe, 'api' as been replaced with 'API'.

  - Patches 02/13 to 04/13 were in v1, v2, v6, v7, v8, v9 and v10.

They haven't changed since v10.

Along with 01/13 they finish libifying the apply functionality that
was in builtin/apply.c and move it into apply.{c,h}, but the libified
functionality is not yet used in `git am`.

  - Patch 05/13 was in v6, v7, v8, v9 and v10, and hasn't changed.

It replaces some calls to error() with calls to error_errno().

  - Patches 06/13 to 10/13 were in v2, v6, v7, v8, v9 and v10.

They haven't changed since v10.

They implement a way to make the libified apply code silent by
changing the bool `apply_verbosely` into a tristate enum called
`apply_verbosity`, that can be one of `verbosity_verbose`,
`verbosity_normal` or `verbosity_silent`.

This is because "git am", since it was a shell script, has been
silencing the apply functionality by redirecting file descriptors to
/dev/null, but this is not acceptable in C.

  - Patch 11/13 was in v9 and v10, and hasn't changed.

It refactors `git apply` option parsing to make it possible for `git
am` to easily pass some command line options to the libified applied
code as suggested by Junio.

  - Patch 12/13 is new.

It replaces patch 33/40 in v10 (environment: add set_index_file())
that was a hack to make it possible for `git am` to use the libified
apply functionality on a different index file.

Now for this purpose, in this new patch, we add
"const char *index_file" into "struct apply_state", and when
"index_file" is set, we use hold_lock_file_for_update(), passing it
"state->index_file", instead of using hold_locked_index(), as it is
not possible to pass an index filename in hold_locked_index().

  - Patch 13/13 was in v1, v2, v6, v7, v8, v9 and v10.

This patch makes `git am` use the libified functionality. It now uses
"index_file" in "struct apply_state" added by 12/13 to pass the file
we want the index written into to the libified apply functionality.


General comments


Now this patch series is shorter than previously. Hopefully also the
early part of this series until 05/13 or 11/13 will be ready soon to
be moved to next and master, and I may only need to resend the last 2
patches if anything.

I will send a diff between this version and v10 as a reply to this
email. (Note that in v11 only commit messages changed, so there is no
difference between v10 and v11.)

The benefits are not just related to not creating new processes. When
`git am` launched a `git apply` process, this new process had to read
the index from disk. Then after the `git apply`process had terminated,
`git am` dropped its index and read the index from disk to get the
index that had been modified by the `git apply`process. This was
inefficient and also prevented the split-index mechanism to provide
many performance benefits.

By the way, current work is ongoing to make it possible to use
split-index more easily by adding a config variable, see:

https://public-inbox.org/git/20160711172254.13439-1-chriscoo

[PATCH v12 04/13] apply: make some parsing functions static again

2016-08-11 Thread Christian Couder
Some parsing functions that were used in both "apply.c" and
"builtin/apply.c" are now only used in the former, so they
can be made static to "apply.c".

Signed-off-by: Christian Couder 
---
 apply.c | 6 +++---
 apply.h | 5 -
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/apply.c b/apply.c
index 7b96130..c0cb3f5 100644
--- a/apply.c
+++ b/apply.c
@@ -27,7 +27,7 @@ static void git_apply_config(void)
git_config(git_default_config, NULL);
 }
 
-int parse_whitespace_option(struct apply_state *state, const char *option)
+static int parse_whitespace_option(struct apply_state *state, const char 
*option)
 {
if (!option) {
state->ws_error_action = warn_on_ws_error;
@@ -57,8 +57,8 @@ int parse_whitespace_option(struct apply_state *state, const 
char *option)
return error(_("unrecognized whitespace option '%s'"), option);
 }
 
-int parse_ignorewhitespace_option(struct apply_state *state,
- const char *option)
+static int parse_ignorewhitespace_option(struct apply_state *state,
+const char *option)
 {
if (!option || !strcmp(option, "no") ||
!strcmp(option, "false") || !strcmp(option, "never") ||
diff --git a/apply.h b/apply.h
index 5ec022c..df44b51 100644
--- a/apply.h
+++ b/apply.h
@@ -97,11 +97,6 @@ struct apply_state {
int applied_after_fixing_ws;
 };
 
-extern int parse_whitespace_option(struct apply_state *state,
-  const char *option);
-extern int parse_ignorewhitespace_option(struct apply_state *state,
-const char *option);
-
 extern int apply_option_parse_exclude(const struct option *opt,
  const char *arg, int unset);
 extern int apply_option_parse_include(const struct option *opt,
-- 
2.9.2.769.gc0f0333

--
To unsubscribe from this list: send the line "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 v12 05/13] apply: use error_errno() where possible

2016-08-11 Thread Christian Couder
To avoid possible mistakes and to uniformly show the errno
related messages, let's use error_errno() where possible.

Signed-off-by: Christian Couder 
---
 apply.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/apply.c b/apply.c
index c0cb3f5..41a33d3 100644
--- a/apply.c
+++ b/apply.c
@@ -3497,7 +3497,7 @@ static int load_current(struct apply_state *state,
ce = active_cache[pos];
if (lstat(name, &st)) {
if (errno != ENOENT)
-   return error(_("%s: %s"), name, strerror(errno));
+   return error_errno("%s", name);
if (checkout_target(&the_index, ce, &st))
return -1;
}
@@ -3647,7 +3647,7 @@ static int check_preimage(struct apply_state *state,
} else if (!state->cached) {
stat_ret = lstat(old_name, st);
if (stat_ret && errno != ENOENT)
-   return error(_("%s: %s"), old_name, strerror(errno));
+   return error_errno("%s", old_name);
}
 
if (state->check_index && !previous) {
@@ -3669,7 +3669,7 @@ static int check_preimage(struct apply_state *state,
} else if (stat_ret < 0) {
if (patch->is_new < 0)
goto is_new;
-   return error(_("%s: %s"), old_name, strerror(errno));
+   return error_errno("%s", old_name);
}
 
if (!state->cached && !previous)
@@ -3728,7 +3728,7 @@ static int check_to_create(struct apply_state *state,
 
return EXISTS_IN_WORKTREE;
} else if ((errno != ENOENT) && (errno != ENOTDIR)) {
-   return error("%s: %s", new_name, strerror(errno));
+   return error_errno("%s", new_name);
}
return 0;
 }
@@ -4247,9 +4247,9 @@ static int add_index_file(struct apply_state *state,
if (!state->cached) {
if (lstat(path, &st) < 0) {
free(ce);
-   return error(_("unable to stat newly "
-  "created file '%s': %s"),
-path, strerror(errno));
+   return error_errno(_("unable to stat newly "
+"created file '%s'"),
+  path);
}
fill_stat_cache_info(ce, &st);
}
@@ -4306,7 +4306,7 @@ static int try_create_file(const char *path, unsigned int 
mode, const char *buf,
strbuf_release(&nbuf);
 
if (close(fd) < 0 && !res)
-   return error(_("closing file '%s': %s"), path, strerror(errno));
+   return error_errno(_("closing file '%s'"), path);
 
return res ? -1 : 0;
 }
@@ -4503,7 +4503,7 @@ static int write_out_one_reject(struct apply_state 
*state, struct patch *patch)
 
rej = fopen(namebuf, "w");
if (!rej)
-   return error(_("cannot open %s: %s"), namebuf, strerror(errno));
+   return error_errno(_("cannot open %s"), namebuf);
 
/* Normal git tools never deal with .rej, so do not pretend
 * this is a git patch by saying --git or giving extended
-- 
2.9.2.769.gc0f0333

--
To unsubscribe from this list: send the line "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 v12 07/13] apply: don't print on stdout in verbosity_silent mode

2016-08-11 Thread Christian Couder
When apply_verbosity is set to verbosity_silent nothing should be
printed on both stderr and stdout.

To avoid printing on stdout, we can just skip calling the following
functions:

- stat_patch_list(),
- numstat_patch_list(),
- summary_patch_list().

It is safe to do that because the above functions have no side
effects other than printing:

- stat_patch_list() only computes some local values and then call
show_stats() and print_stat_summary(), those two functions only
compute local values and call printing functions,
- numstat_patch_list() also only computes local values and calls
printing functions,
- summary_patch_list() calls show_file_mode_name(), printf(),
show_rename_copy(), show_mode_change() that are only printing.

Signed-off-by: Christian Couder 
---
 apply.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/apply.c b/apply.c
index df85cbc..ddbb0a2 100644
--- a/apply.c
+++ b/apply.c
@@ -4702,13 +4702,13 @@ static int apply_patch(struct apply_state *state,
goto end;
}
 
-   if (state->diffstat)
+   if (state->diffstat && state->apply_verbosity > verbosity_silent)
stat_patch_list(state, list);
 
-   if (state->numstat)
+   if (state->numstat && state->apply_verbosity > verbosity_silent)
numstat_patch_list(state, list);
 
-   if (state->summary)
+   if (state->summary && state->apply_verbosity > verbosity_silent)
summary_patch_list(list);
 
 end:
-- 
2.9.2.769.gc0f0333

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


Re: [PATCH v6 9/9] status: unit tests for --porcelain=v2

2016-08-11 Thread Junio C Hamano
Jeff Hostetler  writes:

> From: Jeff Hostetler 
>
> Test porcelain v2 status format.
>
> Signed-off-by: Jeff Hostetler 
> ---
>  t/t7064-wtstatus-pv2.sh | 576 
> 
>  1 file changed, 576 insertions(+)
>  create mode 100755 t/t7064-wtstatus-pv2.sh
>
> diff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh
> new file mode 100755
> index 000..44a8671
> --- /dev/null
> +++ b/t/t7064-wtstatus-pv2.sh
> @@ -0,0 +1,576 @@
> +#!/bin/sh
> +
> +test_description='git status --porcelain=v2
> +
> +This test exercises porcelain V2 output for git status.'

A general comment on the titles; with retitling of individual tests,
the result has become a lot easier to understand.  I know coming up
with a short and to-the-point description for them is hard, but that
is effort and time well spent and it shows in the result.  Thanks.

> +. ./test-lib.sh
> +
> +OID_EMPTY=e69de29bb2d1d6434b8b29ae775ad8c2e48c5391

It seems that test-lib.sh these days has EMPTY_BLOB defined for your
use.  You can remove this and replace its use (just two lines) with
$EMPTY_BLOB down in the "add -N" test.

> +test_expect_success setup '
> + test_tick &&
> + git config --local core.autocrlf false &&

I'd suggest removing "--local".

Existing use of "git config" in the test suite, unless their use is
about testing "git config" itself to validate the operation of the
--local/--global/--system options, do not seem to explicitly say
"--local", which is the default anyway.

> +test_expect_success 'after first commit, make dirt, confirm unstaged 
> changes' '

Did you mean s/dirt/dirty/?  "make and confirm unstaged changes"
would be sufficient.  Because "confirming" is implicit (as these
are all tests), "after the first commit, modify working tree files"
might even be better, perhaps?

> + echo x >>file_x &&
> + OID_X1=$(git hash-object -t blob -- file_x) &&
> + rm file_z &&
> + H0=$(git rev-parse HEAD) &&
> + ...

> +test_expect_success 'after first commit, stage dirt, confirm staged changes' 
> '

What you "git add" is meant to be good changes, so they are no
longer dirt ;-)  More importantly, because I never heard of "dirt"
used in Git context, it is unclear if it is an untracked file, a
modification that is not meant to be committed immediately, or what.

"after the first commit, fully add changes to the index"?

> + git add file_x &&
> + git rm file_z &&
> + H0=$(git rev-parse HEAD) &&
> +
> + cat >expect <<-EOF &&
> + # branch.oid $H0
> + # branch.head master
> + 1 M. N... 100644 100644 100644 $OID_X $OID_X1 file_x
> + 1 D. N... 100644 00 00 $OID_Z $_z40 file_z
> + ? actual
> + ? expect
> + EOF

> +test_expect_success 'after first commit, also stage rename, confirm 2 path 
> line format' '
> + git mv file_y renamed_y &&
> + H0=$(git rev-parse HEAD) &&
> +
> + q_to_tab >expect <<-EOF &&
> + # branch.oid $H0
> + # branch.head master
> + 1 M. N... 100644 100644 100644 $OID_X $OID_X1 file_x
> + 1 D. N... 100644 00 00 $OID_Z $_z40 file_z
> + 2 R. N... 100644 100644 100644 $OID_Y $OID_Y R100 renamed_yQfile_y
> + ? actual
> + ? expect
> + EOF
> +
> + git status --porcelain=v2 --branch --untracked-files=all >actual &&
> + test_cmp expect actual
> +'

Do we want to test -z format on this, too?

> ...
> +test_expect_success 'create ignored files, confirm they are not printed' '
> +test_expect_success 'create ignored files, confirm --ignored prints them' '
> ...

These are all good and readably titled. 

> +test_expect_success 'verify upstream fields in branch header' '
> + git checkout master &&
> + test_when_finished rm -rf sub_repo &&

Forgot to quote?
--
To unsubscribe from this list: send the line "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 2/2] diff: add SUBMODULE_DIFF format to display submodule diff

2016-08-11 Thread Jacob Keller
On Thu, Aug 11, 2016 at 10:53 AM, Junio C Hamano  wrote:
> Jacob Keller  writes:
>
>> From: Jacob Keller 
>>
>> Teach git-diff and friends a new format for displaying the difference of
>> a submodule using git-diff inside the submodule project. This allows
>> users to easily see exactly what source changed in a given commit that
>> updates the submodule pointer. To do this, remove DIFF_SUBMODULE_LOG bit
>> from the diff options, and instead add a new enum type for these
>> formats.
>>
>> Signed-off-by: Jacob Keller 
>> ---
>>  Documentation/diff-config.txt  |  3 +-
>>  Documentation/diff-options.txt |  6 ++--
>>  diff.c | 41 --
>>  diff.h |  9 +-
>>  submodule.c| 67 
>> ++
>>  submodule.h|  6 
>>  6 files changed, 113 insertions(+), 19 deletions(-)
>
> This looks good.
>
> You'd want some tests to make sure that the "--submodule" and the
> "--submodule=" command line options and the diff.submodule
> configuration variables are parsed correctly (and when combined, the
> command line option overrides the configured default), and of course
> the machinery does the right thing, with and without "--graph" when
> used in "git log".

Yes, I am adding tests now, but I ran into some interesting corner
cases for this, that still need some work.

There's a bunch of issues with cases involving adding a submodule that
isn't stored in .git/modules/etc, which the current tests for
--submodule=log do.

There's also the case of empty trees, which I believe I have resolved
now. Hopefully I can sort these cases correctly.

>
>> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
>> index e7b729f3644f..988068225463 100644
>> --- a/Documentation/diff-options.txt
>> +++ b/Documentation/diff-options.txt
>> @@ -215,8 +215,10 @@ any of those replacements occurred.
>>   the commits in the range like linkgit:git-submodule[1] `summary` does.
>>   Omitting the `--submodule` option or specifying `--submodule=short`,
>>   uses the 'short' format. This format just shows the names of the 
>> commits
>> - at the beginning and end of the range.  Can be tweaked via the
>> - `diff.submodule` configuration variable.
>> + at the beginning and end of the range. When `--submodule=diff` is
>> + given, the 'diff' format is used. This format shows the diff between
>> + the old and new submodule commmit from the perspective of the
>> + submodule.  Can be tweaked via the `diff.submodule` configuration 
>> variable.
>
> This is carried over from the existing text, but "Can be tweaked
> via" sounds a bit wasteful (and strange); "Defaults to" (or "The
> default is taken from") is the phrase we seem to use more often.

Probably worth fixing here. WIll do.

Thanks,
Jake

>
> 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 1/2] diff: add --line-prefix option for passing in a prefix

2016-08-11 Thread Jacob Keller
On Thu, Aug 11, 2016 at 10:22 AM, Junio C Hamano  wrote:
> -- >8 --
> Subject: diff.c: remove output_prefix_length field
>
> "diff/log --stat" has a logic that determines the display columns
> available for the diffstat part of the output and apportions it for
> pathnames and diffstat graph automatically.
>
> 5e71a84a (Add output_prefix_length to diff_options, 2012-04-16)
> added the output_prefix_length field to diff_options structure to
> allow this logic subtract the display columns used for display the
> history graph part from the total "terminal width"; this matters
> when the "git log --graph -p" option is in use.
>
> The field be set to the number of display columns needed to show the
> output from the output_prefix() callback.  Any new output_prefix()
> callback must also update the field accordingly, which is error
> prone.  As there is only one user of the field, and the user has the
> actual value of the prefix string, let's get rid of the field and
> have the user count the display width itself.
>

Seems correct to me.

Thanks,
Jake
--
To unsubscribe from this list: send the line "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] checkout: do not mention detach advice for explicit --detach option

2016-08-11 Thread Junio C Hamano
Stefan Beller  writes:

> On Wed, Aug 10, 2016 at 10:32 AM, Jonathan Nieder  wrote:
>> Stefan Beller wrote:
>>
>>> When a user asked for a detached HEAD specifically with `--detach`,
>>> we do not need to give advice on what a detached HEAD state entails as
>>> we can assume they know what they're getting into as they asked for it.
>>
>> Example? Tests?
>
> There are no tests for the advice things IIUC.

There seem to already be tests that explicitly sets advice.* to true
like in t7201 and t7512, but even if there weren't any existing
ones, it would be appropriate to make sure that an explicit --detach
does not trigger the advice, even when advice.detachedHEAD left
unconfigured (or set to true), given that doing so is the whole
purpose of this patch, I would think.

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


Re: Can cc's be included in patches sent by send-email

2016-08-11 Thread Junio C Hamano
On Thu, Aug 11, 2016 at 10:57 AM, Junio C Hamano  wrote:
>
> Also, those who do not want to see Cc: in headers (like me) can

Stupid typo that ruins the whole message.  I meant "in FOOTERS".

> instead edit the header part of the format-patch output to add Cc:
> lines and they should be picked up.
--
To unsubscribe from this list: send the line "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: Can cc's be included in patches sent by send-email

2016-08-11 Thread Junio C Hamano
Jacob Keller  writes:

> On Thu, Aug 11, 2016 at 12:58 AM, Jacob Keller  wrote:
>> On Thu, Aug 11, 2016 at 12:32 AM, Philip Oakley  wrote:
>>> While 'git send-email' can have multiple --cc="addressee" options on the
>>> command line, is it possible for the "cc:addressee" to actually be
>>> included in the patches that are to be sent, so that different patches can
>>> have different addressee?
>>>
>>> The fortmat-patch can include appropriate from lines, so it feels as if the
>>> sender should be able to add cc: lines at the same place. Maybe its just
>>> part of th docs I've missed.
>>>
>>
>> Yes, just put them in the body as tags below the signed-off-by. It
>> should be on by default unless you change supresscc or signedoffbycc
>> configuration.
>>
>> Thanks,
>> Jake
>>
>
> See --suppress-cc or --signed-off-by-cc help in git help send-email.

Also, those who do not want to see Cc: in headers (like me) can
instead edit the header part of the format-patch output to add Cc:
lines and they should be picked up.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [ANNOUNCE] tig-2.2

2016-08-11 Thread Junio C Hamano
Jonas Fonseca  writes:

> I've just released the 35th release of Tig. It brings several search
> improvements such as highlighting and wrap around, and machinery for future
> support of typeahead search. This release also gives more choice over how the
> user configuration file is loaded either at built-time or at runtime through
> support of the XDG basedir spec. Among fixes several segfaults and invalid
> reads have been addressed and the tests are now run with Valgrind and
> AddressSanitizer by Travis on each push. There are several breaking changes so
> ensure you read the section on incompatibilities in the release notes before
> upgrading.

Nice.  Thanks for your continued effort on this little Gem.
--
To unsubscribe from this list: send the line "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 2/2] diff: add SUBMODULE_DIFF format to display submodule diff

2016-08-11 Thread Junio C Hamano
Jacob Keller  writes:

> From: Jacob Keller 
>
> Teach git-diff and friends a new format for displaying the difference of
> a submodule using git-diff inside the submodule project. This allows
> users to easily see exactly what source changed in a given commit that
> updates the submodule pointer. To do this, remove DIFF_SUBMODULE_LOG bit
> from the diff options, and instead add a new enum type for these
> formats.
>
> Signed-off-by: Jacob Keller 
> ---
>  Documentation/diff-config.txt  |  3 +-
>  Documentation/diff-options.txt |  6 ++--
>  diff.c | 41 --
>  diff.h |  9 +-
>  submodule.c| 67 
> ++
>  submodule.h|  6 
>  6 files changed, 113 insertions(+), 19 deletions(-)

This looks good.

You'd want some tests to make sure that the "--submodule" and the
"--submodule=" command line options and the diff.submodule
configuration variables are parsed correctly (and when combined, the
command line option overrides the configured default), and of course
the machinery does the right thing, with and without "--graph" when
used in "git log".

> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index e7b729f3644f..988068225463 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -215,8 +215,10 @@ any of those replacements occurred.
>   the commits in the range like linkgit:git-submodule[1] `summary` does.
>   Omitting the `--submodule` option or specifying `--submodule=short`,
>   uses the 'short' format. This format just shows the names of the commits
> - at the beginning and end of the range.  Can be tweaked via the
> - `diff.submodule` configuration variable.
> + at the beginning and end of the range. When `--submodule=diff` is
> + given, the 'diff' format is used. This format shows the diff between
> + the old and new submodule commmit from the perspective of the
> + submodule.  Can be tweaked via the `diff.submodule` configuration 
> variable.

This is carried over from the existing text, but "Can be tweaked
via" sounds a bit wasteful (and strange); "Defaults to" (or "The
default is taken from") is the phrase we seem to use more often.

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 v11 28/40] builtin/apply: rename option parsing functions

2016-08-11 Thread Christian Couder
On Thu, Aug 11, 2016 at 10:58 AM,   wrote:
> Am 11.08.2016 um 10:52 schrieb Christian Couder:
>> As these functions are going to be part of the libified
>> apply api, let's give them a name that is more specific
>
> s/api/API/
>
> ;-)

Ooops.
Anyway as this is patch 28/40 and the other problem you found is in
40/40, I will just resend patches from 28/40 to 40/40 in v12 (so only
the last 13 patches, that I will call part 3 of the whole series).

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 1/2] diff: add --line-prefix option for passing in a prefix

2016-08-11 Thread Junio C Hamano
Jacob Keller  writes:

>  const char *diff_line_prefix(struct diff_options *opt)
>  {
>   struct strbuf *msgbuf;
> +
>   if (!opt->output_prefix)
> - return "";
> + if (opt->line_prefix)
> + return opt->line_prefix;
> + else
> + return "";
>  
>   msgbuf = opt->output_prefix(opt, opt->output_prefix_data);
> + /* line prefix must be printed before the output_prefix() */
> + if (opt->line_prefix)
> + strbuf_insert(msgbuf, 0, opt->line_prefix, 
> strlen(opt->line_prefix));
>   return msgbuf->buf;
>  }

The result of applying this change leaves the semantics of the
line_prefix field, the output_prefix() callback, and the
output_prefix_length field in the diff_options structure a bit
tricky to reason about.

The code pretends as if the remainder of the system does not even
care about the presence of line_prefix (i.e. output_prefix_length is
not updated), but its only user of output_prefix_length cares, I
think.  It is in diff.c::show_stats() where it auto-computes the
allowed display width for the stat portion:

if (options->stat_width == -1)
width = term_columns() - options->output_prefix_length;

The output_prefix_length is initialized to be 0, but when --graph is
in effect, it is set to the width of the graph portion of the output
in the output_prefix callback, diff_output_prefix_callback().

So the above change is clearly wrong in that it needs to add the
number of display columns needed to show opt->line_prefix to the
output_prefix_length, but I wonder if a better "fix" to this is to
get rid of output_prefix_length field from diff_options struct as a
preparatory step.  That would make the bug in this patch disappear.

Perhaps like this.  I do not know if Lucian is still interested in,
or remembers what did for, Git in 2012, but this updates his code
and replaces it with what I hope is an equivalent, so I added him
to the Cc: line.

-- >8 --
Subject: diff.c: remove output_prefix_length field

"diff/log --stat" has a logic that determines the display columns
available for the diffstat part of the output and apportions it for
pathnames and diffstat graph automatically.

5e71a84a (Add output_prefix_length to diff_options, 2012-04-16)
added the output_prefix_length field to diff_options structure to
allow this logic subtract the display columns used for display the
history graph part from the total "terminal width"; this matters
when the "git log --graph -p" option is in use.

The field be set to the number of display columns needed to show the
output from the output_prefix() callback.  Any new output_prefix()
callback must also update the field accordingly, which is error
prone.  As there is only one user of the field, and the user has the
actual value of the prefix string, let's get rid of the field and
have the user count the display width itself.

Signed-off-by: Junio C Hamano 
---

 diff.c  | 2 +-
 diff.h  | 1 -
 graph.c | 2 --
 3 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/diff.c b/diff.c
index b43d3dd..ae069c3 100644
--- a/diff.c
+++ b/diff.c
@@ -1625,7 +1625,7 @@ static void show_stats(struct diffstat_t *data, struct 
diff_options *options)
 */
 
if (options->stat_width == -1)
-   width = term_columns() - options->output_prefix_length;
+   width = term_columns() - strlen(line_prefix);
else
width = options->stat_width ? options->stat_width : 80;
number_width = decimal_width(max_change) > number_width ?
diff --git a/diff.h b/diff.h
index 125447b..49e4aaa 100644
--- a/diff.h
+++ b/diff.h
@@ -174,7 +174,6 @@ struct diff_options {
diff_format_fn_t format_callback;
void *format_callback_data;
diff_prefix_fn_t output_prefix;
-   int output_prefix_length;
void *output_prefix_data;
 
int diff_path_counter;
diff --git a/graph.c b/graph.c
index dd17201..a468038 100644
--- a/graph.c
+++ b/graph.c
@@ -197,7 +197,6 @@ static struct strbuf *diff_output_prefix_callback(struct 
diff_options *opt, void
assert(opt);
assert(graph);
 
-   opt->output_prefix_length = graph->width;
strbuf_reset(&msgbuf);
graph_padding_line(graph, &msgbuf);
return &msgbuf;
@@ -245,7 +244,6 @@ struct git_graph *graph_init(struct rev_info *opt)
 */
opt->diffopt.output_prefix = diff_output_prefix_callback;
opt->diffopt.output_prefix_data = graph;
-   opt->diffopt.output_prefix_length = 0;
 
return graph;
 }








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


Re: [PATCH] gc: default aggressive depth to 50

2016-08-11 Thread Jeff King
On Thu, Aug 11, 2016 at 12:13:09PM -0400, Jeff King wrote:

> Here are the numbers for linux.git:
> 
>depth |  size |  %| rev-list |  % | log -Sfoo |   %
>   ---+---+---+--++---+---
> 250  | 967MB |  n/a  | 48.159s  |   n/a  | 378.088   |   n/a
> 100  | 971MB | +0.4% | 41.471s  | -13.9% | 342.060   |  -9.5%
>  50  | 979MB | +1.2% | 37.778s  | -21.6% | 311.040s  | -17.7%
>  10  | 1.1GB | +6.6% | 32.518s  | -32.5% | 279.890s  | -25.9%
> [...]
> 
> You can see that that the CPU savings for regular operations improves as we
> decrease the depth. The savings are less for "rev-list" on a smaller 
> repository
> than they are for blob-accessing operations, or even rev-list on a larger
> repository. This may mean that a larger delta cache would help (though setting
> core.deltaBaseCacheLimit by itself doesn't).

The problem with deltaBaseCacheLimit is that it only changes the memory
parameter, but there are a fixed number of slots in the data structure.
Bumping it like this:

diff --git a/sha1_file.c b/sha1_file.c
index 02940f1..ca79703 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2073,7 +2073,7 @@ static void *unpack_compressed_entry(struct packed_git *p,
return buffer;
 }
 
-#define MAX_DELTA_CACHE (256)
+#define MAX_DELTA_CACHE (1024)
 
 static size_t delta_base_cached;
 

along with the cache size does help (this was discussed a year or two
ago, but nobody ever followed up with numbers or patches).

Here are best-of-3 numbers for rev-list on linux.git at each depth with
that patch and "-c core.deltabasecachelimit=256m":

  depth |   time
  
   250  | 36.524s
   100  | 33.200s
50  | 31.065s
10  | 28.266s

So there's clearly a lot of room for improvement on the reading side in
general. And doing so clearly lessens the impact of the delta chains.
But you still get a 15% improvement moving to depth=50, versus only a
1.2% storage increase. So I don't think it fundamentally changes the
conclusion of the "depth=50" patch I'm responding to.

I don't think bumping MAX_DELTA_CACHE naively is a good idea, though. I
seem to recall that it has scaling problems as it grows, so we may want
a better data structure (but I haven't looked at it recently enough to
say anything intelligent). I might work on it in the near future, but if
anybody is interested, please be my guest.

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


Re: [PATCH v5] pack-objects mru

2016-08-11 Thread Jeff King
On Thu, Aug 11, 2016 at 08:11:33AM -0700, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > So considering "--depth" as a space-saving measure for --aggressive does
> > not seem that effective. But it feels weird to quietly drop actions
> > people might have done with previous aggressive runs.
> 
> That argument cuts both ways, doesn't it?
> 
> If the user explicitly asks to use lower "--depth" from the command
> line when the second repack runs, the intention is clear: the
> existing pack may use delta chains that are too long and is
> detrimental to the run-time performance, and the user wants to
> correct it by repacking with shorter delta chain.
> 
> Should the act of letting "gc --auto" use lower "--depth", by not
> configuring to always use deeper chain, be interpreted the same way?
> I am not sure.  The old packing with large --depth is something the
> user did long time ago, and the decision the user made not to use
> large depth always is also something the user did long time ago.  I
> do not think it is so cut-and-dried which one of the two conflicting
> wishes we should honor when running the second repack, especially
> when it is run unattended like "gc --auto" does.

Good points. Explicitly saying "repack --depth=..." carries a lot more
weight to me than "git gc --auto" randomly kicking in, as far as knowing
that what the user actually wants. My patch doesn't differentiate, of
course, but I think it could.

The other problem with my patch is the fact that we don't do a good job
of finding new, in-limit deltas for the ones we discard. If you want to
do that, you really need to "git repack -f" (at least with the current
code). At which point we do not reuse the on-disk deltas at all, and the
problem is moot (you could also interpret the fact that the user did
_not_ pass "-f" as "you want to reuse deltas, which means you want to
reuse even long chains", but as you've argued above, you can make a lot
of guesses about the user's intention from what they did or did not
say).

So if we were to go this route, I don't think my patch is quite
sufficient; we'd want something else on top to do a better job of
finding replacement deltas.

Regarding my "does not seem that effective" above, I think we should
drop the aggressive depth to 50, and I just posted a patch with
reasoning and numbers:

  
http://public-inbox.org/git/20160811161309.ecmebaafcz6rk...@sigill.intra.peff.net/

That's maybe orthogonal, but it does remove the weird "gc --aggressive
followed by gc --auto produces a bad pack" issue, because unless you are
doing something clever, the depth will always be 50 (modulo people who
did an aggressive pack with an older version of git :-/ ).

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


[PATCH] gc: default aggressive depth to 50

2016-08-11 Thread Jeff King
This commit message is long and has lots of background and
numbers. The summary is: the current default of 250 doesn't
save much space, and costs CPU. It's not a good tradeoff.
Read on for details.

The "--aggressive" flag to git-gc does three things:

  1. use "-f" to throw out existing deltas and recompute from
 scratch

  2. use "--window=250" to look harder for deltas

  3. use "--depth=250" to make longer delta chains

Items (1) and (2) are good matches for an "aggressive"
repack. They ask the repack to do more computation work in
the hopes of getting a better pack. You pay the costs during
the repack, and other operations see only the benefit.

Item (3) is not so clear. Allowing longer chains means fewer
restrictions on the deltas, which means potentially finding
better ones and saving some space. But it also means that
operations which access the deltas have to follow longer
chains, which affects their performance. So it's a tradeoff,
and it's not clear that the tradeoff is even a good one.

The existing "250" numbers for "--aggressive" come
originally from this thread:

  
http://public-inbox.org/git/alpine.lfd.0..0712060803430.13...@woody.linux-foundation.org/

where Linus says:

  So when I said "--depth=250 --window=250", I chose those
  numbers more as an example of extremely aggressive
  packing, and I'm not at all sure that the end result is
  necessarily wonderfully usable. It's going to save disk
  space (and network bandwidth - the delta's will be re-used
  for the network protocol too!), but there are definitely
  downsides too, and using long delta chains may
  simply not be worth it in practice.

There are some numbers in that thread, but they're mostly
focused on the improved window size, and measure the
improvement from --depth=250 and --window=250 together.
E.g.:

  
http://public-inbox.org/git/9e4733910712062006l651571f3w7f76ce64c6650...@mail.gmail.com/

talks about the improved run-time of "git-blame", which
comes from the reduced pack size. But most of that reduction
is coming from --window=250, whereas most of the extra costs
come from --depth=250. There's a link in that thread showing
that increasing the depth beyond 50 doesn't seem to help
much with the size:

  https://vcscompare.blogspot.com/2008/06/git-repack-parameters.html

but again, no discussion of the timing impact.

In an earlier thread from Ted Ts'o which discussed setting
the non-aggressive default (from 10 to 50):

  http://public-inbox.org/git/20070509134958.GA21489%40thunk.org/

we have more numbers, with the conclusion that going past 50
does not help size much, and hurts the speed of normal
operations.

So from that, we might guess that 50 is actually a sweet
spot, even for aggressive, if we interpret aggressive to
"spend time now to make a better pack". It is not clear that
"--depth=250" is actually a better pack. It may be slightly
_smaller_, but it carries a run-time penalty.

Here are some more recent timings I did to verify that. They
show three things:

  - the size of the resulting pack (so disk saved to store,
bandwidth saved on clones/fetches)

  - the cost of "rev-list --objects --all", which shows the
effect of the delta chains on trees (commits typically
don't delta, and the command doesn't touch the blobs at
all)

  - the cost of "log -Sfoo", which will additionally access
each blob

All cases were repacked with "git repack -adf --depth=$d
--window=250" (so basically, what would happen if we tweaked
the "gc --aggressive" default depth).

The timings are all wall-clock best-of-3. The machine itself
has plenty of RAM compared to the repositories (which is
probably typical of most workstations these days), so we're
really measuring CPU usage, as the whole thing will be in
disk cache after the first run.

The core.deltaBaseCacheLimit is at its default of 96MiB.
It's possible that tweaking it would have some impact on the
tests, as some of them (especially "log -S" on a large repo)
are likely to overflow that. But bumping that carries a
run-time memory cost, so for these tests, I focused on what
we could do just with the on-disk pack tradeoffs.

Each test is done for four depths: 250 (the current value),
50 (the current default that tested well previously), 100
(to show something on the larger side, which previous tests
showed was not a good tradeoff), and 10 (the very old
default, which previous tests showed was worse than 50).

Here are the numbers for linux.git:

   depth |  size |  %| rev-list |  % | log -Sfoo |   %
  ---+---+---+--++---+---
250  | 967MB |  n/a  | 48.159s  |   n/a  | 378.088   |   n/a
100  | 971MB | +0.4% | 41.471s  | -13.9% | 342.060   |  -9.5%
 50  | 979MB | +1.2% | 37.778s  | -21.6% | 311.040s  | -17.7%
 10  | 1.1GB | +6.6% | 32.518s  | -32.5% | 279.890s  | -25.9%

and for git.git:

   depth |  size |  %| rev-list |  % | log -Sfoo |   %
  ---+---+---+--+

Re: [PATCH v5] pack-objects mru

2016-08-11 Thread Junio C Hamano
Jeff King  writes:

> So considering "--depth" as a space-saving measure for --aggressive does
> not seem that effective. But it feels weird to quietly drop actions
> people might have done with previous aggressive runs.

That argument cuts both ways, doesn't it?

If the user explicitly asks to use lower "--depth" from the command
line when the second repack runs, the intention is clear: the
existing pack may use delta chains that are too long and is
detrimental to the run-time performance, and the user wants to
correct it by repacking with shorter delta chain.

Should the act of letting "gc --auto" use lower "--depth", by not
configuring to always use deeper chain, be interpreted the same way?
I am not sure.  The old packing with large --depth is something the
user did long time ago, and the decision the user made not to use
large depth always is also something the user did long time ago.  I
do not think it is so cut-and-dried which one of the two conflicting
wishes we should honor when running the second repack, especially
when it is run unattended like "gc --auto" does.



--
To unsubscribe from this list: send the line "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 v6 8/9] test-lib-functions.sh: Add lf_to_nul

2016-08-11 Thread Jeff Hostetler
From: Jeff Hostetler 

Add lf_to_nul() function to test-lib-functions.

Signed-off-by: Jeff Hostetler 
---
 t/test-lib-functions.sh | 4 
 1 file changed, 4 insertions(+)

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 4f7eadb..fdaeb3a 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -81,6 +81,10 @@ test_decode_color () {
'
 }
 
+lf_to_nul () {
+   perl -pe 'y/\012/\000/'
+}
+
 nul_to_q () {
perl -pe 'y/\000/Q/'
 }
-- 
2.8.0.rc4.17.gac42084.dirty

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


[PATCH v6 7/9] git-status.txt: describe --porcelain=v2 format

2016-08-11 Thread Jeff Hostetler
From: Jeff Hostetler 

Update status manpage to include information about
porcelain v2 format.

Signed-off-by: Jeff Hostetler 
---
 Documentation/git-status.txt | 126 +--
 1 file changed, 122 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index 6b1454b..a58973b 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -183,12 +183,12 @@ in which case `XY` are `!!`.
 
 If -b is used the short-format status is preceded by a line
 
-## branchname tracking info
+## branchname tracking info
 
-Porcelain Format
-
+Porcelain Format Version 1
+~~
 
-The porcelain format is similar to the short format, but is guaranteed
+Version 1 porcelain format is similar to the short format, but is guaranteed
 not to change in a backwards-incompatible way between Git versions or
 based on user configuration. This makes it ideal for parsing by scripts.
 The description of the short format above also describes the porcelain
@@ -210,6 +210,124 @@ field from the first filename).  Third, filenames 
containing special
 characters are not specially formatted; no quoting or
 backslash-escaping is performed.
 
+Porcelain Format Version 2
+~~
+
+Version 2 format adds more detailed information about the state of
+the worktree and changed items.  Version 2 also defines an extensible
+set of easy to parse optional headers.
+
+Header lines start with "#" and are added in response to specific
+command line arguments.  Parsers should ignore headers they
+don't recognize.
+
+### Branch Headers
+
+If `--branch` is given, a series of header lines are printed with
+information about the current branch.
+
+Line Notes
+
+# branch.oid  | (initial)Current commit.
+# branch.head  | (detached)  Current branch.
+# branch.upstream   If upstream is set.
+# branch.ab + -   If upstream is set and
+ the commit is present.
+
+
+### Changed Tracked Entries
+
+Following the headers, a series of lines are printed for tracked
+entries.  One of three different line formats may be used to describe
+an entry depending on the type of change.  Tracked entries are printed
+in an undefined order; parsers should allow for a mixture of the 3
+line types in any order.
+
+Ordinary changed entries have the following format:
+
+1
+
+Renamed or copied entries have the following format:
+
+2 
+
+Field   Meaning
+
+A 2 character field containing the staged and
+unstaged XY values described in the short format,
+with unchanged indicated by a "." rather than
+a space.
+   A 4 character field describing the submodule state.
+"N..." when the entry is not a submodule.
+"S" when the entry is a submodule.
+ is "C" if the commit changed; otherwise ".".
+ is "M" if it has tracked changes; otherwise ".".
+ is "U" if there are untracked changes; otherwise ".".
+The octal file mode in HEAD.
+The octal file mode in the index.
+The octal file mode in the worktree.
+The object name in HEAD.
+The object name in the index.
+  The rename or copy score (denoting the percentage
+of similarity between the source and target of the
+move or copy). For example "R100" or "C75".
+  The pathname.  In a renamed/copied entry, this
+is the path in the index and in the working tree.
+   When the `-z` option is used, the 2 pathnames are separated
+with a NUL (ASCII 0x00) byte; otherwise, a tab (ASCII 0x09)
+byte separates them.
+  The pathname in the commit at HEAD.  This is only
+present in a renamed/copied entry, and tells
+where the renamed/copied contents came from.
+
+
+Unmerged entries have the following format; the first character is
+a "u" to distinguish from ordinary changed entries.
+
+u  
+
+Field   Meaning
+
+A 2 character field describing the conflict type
+as described in the short format.
+   A 4 character field describing the submodule state
+as described above.
+The octal file mode in stage 1.
+The octal file mode in stage 2.
+The octal file mode in stage 3.
+The octal file mode in the worktree.
+

[PATCH v6 5/9] status: print per-file porcelain v2 status data

2016-08-11 Thread Jeff Hostetler
From: Jeff Hostetler 

Print per-file information in porcelain v2 format.

Signed-off-by: Jeff Hostetler 
---
 wt-status.c | 285 +++-
 1 file changed, 284 insertions(+), 1 deletion(-)

diff --git a/wt-status.c b/wt-status.c
index 904b5c2..03c8e23 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1812,6 +1812,289 @@ static void wt_porcelain_print(struct wt_status *s)
wt_shortstatus_print(s);
 }
 
+/*
+ * Convert various submodule status values into a
+ * fixed-length string of characters in the buffer provided.
+ */
+static void wt_porcelain_v2_submodule_state(
+   struct wt_status_change_data *d,
+   char sub[5])
+{
+   if (S_ISGITLINK(d->mode_head) ||
+   S_ISGITLINK(d->mode_index) ||
+   S_ISGITLINK(d->mode_worktree)) {
+   sub[0] = 'S';
+   sub[1] = d->new_submodule_commits ? 'C' : '.';
+   sub[2] = (d->dirty_submodule & DIRTY_SUBMODULE_MODIFIED) ? 'M' 
: '.';
+   sub[3] = (d->dirty_submodule & DIRTY_SUBMODULE_UNTRACKED) ? 'U' 
: '.';
+   } else {
+   sub[0] = 'N';
+   sub[1] = '.';
+   sub[2] = '.';
+   sub[3] = '.';
+   }
+   sub[4] = 0;
+}
+
+/*
+ * Fix-up changed entries before we print them.
+ */
+static void wt_porcelain_v2_fix_up_changed(
+   struct string_list_item *it,
+   struct wt_status *s)
+{
+   struct wt_status_change_data *d = it->util;
+
+   if (!d->index_status) {
+   /*
+* This entry is unchanged in the index (relative to the head).
+* Therefore, the collect_updated_cb was never called for this
+* entry (during the head-vs-index scan) and so the head column
+* fields were never set.
+*
+* We must have data for the index column (from the
+* index-vs-worktree scan (otherwise, this entry should not be
+* in the list of changes)).
+*
+* Copy index column fields to the head column, so that our
+* output looks complete.
+*/
+   assert(d->mode_head == 0);
+   d->mode_head = d->mode_index;
+   oidcpy(&d->oid_head, &d->oid_index);
+   }
+
+   if (!d->worktree_status) {
+   /*
+* This entry is unchanged in the worktree (relative to the 
index).
+* Therefore, the collect_changed_cb was never called for this 
entry
+* (during the index-vs-worktree scan) and so the worktree 
column
+* fields were never set.
+*
+* We must have data for the index column (from the 
head-vs-index
+* scan).
+*
+* Copy the index column fields to the worktree column so that
+* our output looks complete.
+*
+* Note that we only have a mode field in the worktree column
+* because the scan code tries really hard to not have to 
compute it.
+*/
+   assert(d->mode_worktree == 0);
+   d->mode_worktree = d->mode_index;
+   }
+}
+
+/*
+ * Print porcelain v2 info for tracked entries with changes.
+ */
+static void wt_porcelain_v2_print_changed_entry(
+   struct string_list_item *it,
+   struct wt_status *s)
+{
+   struct wt_status_change_data *d = it->util;
+   struct strbuf buf_index = STRBUF_INIT;
+   struct strbuf buf_head = STRBUF_INIT;
+   const char *path_index = NULL;
+   const char *path_head = NULL;
+   char key[3];
+   char submodule_token[5];
+   char sep_char, eol_char;
+
+   wt_porcelain_v2_fix_up_changed(it, s);
+   wt_porcelain_v2_submodule_state(d, submodule_token);
+
+   key[0] = d->index_status ? d->index_status : '.';
+   key[1] = d->worktree_status ? d->worktree_status : '.';
+   key[2] = 0;
+
+   if (s->null_termination) {
+   /*
+* In -z mode, we DO NOT C-quote pathnames.  Current path is 
ALWAYS first.
+* A single NUL character separates them.
+*/
+   sep_char = '\0';
+   eol_char = '\0';
+   path_index = it->string;
+   path_head = d->head_path;
+   } else {
+   /*
+* Path(s) are C-quoted if necessary. Current path is ALWAYS 
first.
+* The source path is only present when necessary.
+* A single TAB separates them (because paths can contain spaces
+* which are not escaped and C-quoting does escape TAB 
characters).
+*/
+   sep_char = '\t';
+   eol_char = '\n';
+   path_index = quote_path(it->string, s->prefix, &buf_index);
+   if (d->head_path)
+   path_head 

[PATCH v6 9/9] status: unit tests for --porcelain=v2

2016-08-11 Thread Jeff Hostetler
From: Jeff Hostetler 

Test porcelain v2 status format.

Signed-off-by: Jeff Hostetler 
---
 t/t7064-wtstatus-pv2.sh | 576 
 1 file changed, 576 insertions(+)
 create mode 100755 t/t7064-wtstatus-pv2.sh

diff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh
new file mode 100755
index 000..44a8671
--- /dev/null
+++ b/t/t7064-wtstatus-pv2.sh
@@ -0,0 +1,576 @@
+#!/bin/sh
+
+test_description='git status --porcelain=v2
+
+This test exercises porcelain V2 output for git status.'
+
+
+. ./test-lib.sh
+
+OID_EMPTY=e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
+
+test_expect_success setup '
+   test_tick &&
+   git config --local core.autocrlf false &&
+   echo x >file_x &&
+   echo y >file_y &&
+   echo z >file_z &&
+   mkdir dir1 &&
+   echo a >dir1/file_a &&
+   echo b >dir1/file_b
+'
+
+test_expect_success 'before initial commit, nothing added, only untracked' '
+   cat >expect <<-EOF &&
+   # branch.oid (initial)
+   # branch.head master
+   ? actual
+   ? dir1/
+   ? expect
+   ? file_x
+   ? file_y
+   ? file_z
+   EOF
+
+   git status --porcelain=v2 --branch --untracked-files=normal >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'before initial commit, things added' '
+   git add file_x file_y file_z dir1 &&
+   OID_A=$(git hash-object -t blob -- dir1/file_a) &&
+   OID_B=$(git hash-object -t blob -- dir1/file_b) &&
+   OID_X=$(git hash-object -t blob -- file_x) &&
+   OID_Y=$(git hash-object -t blob -- file_y) &&
+   OID_Z=$(git hash-object -t blob -- file_z) &&
+
+   cat >expect <<-EOF &&
+   # branch.oid (initial)
+   # branch.head master
+   1 A. N... 00 100644 100644 $_z40 $OID_A dir1/file_a
+   1 A. N... 00 100644 100644 $_z40 $OID_B dir1/file_b
+   1 A. N... 00 100644 100644 $_z40 $OID_X file_x
+   1 A. N... 00 100644 100644 $_z40 $OID_Y file_y
+   1 A. N... 00 100644 100644 $_z40 $OID_Z file_z
+   ? actual
+   ? expect
+   EOF
+
+   git status --porcelain=v2 --branch --untracked-files=all >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'before initial commit, things added (-z)' '
+   lf_to_nul >expect <<-EOF &&
+   # branch.oid (initial)
+   # branch.head master
+   1 A. N... 00 100644 100644 $_z40 $OID_A dir1/file_a
+   1 A. N... 00 100644 100644 $_z40 $OID_B dir1/file_b
+   1 A. N... 00 100644 100644 $_z40 $OID_X file_x
+   1 A. N... 00 100644 100644 $_z40 $OID_Y file_y
+   1 A. N... 00 100644 100644 $_z40 $OID_Z file_z
+   ? actual
+   ? expect
+   EOF
+
+   git status -z --porcelain=v2 --branch --untracked-files=all >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'make first commit, comfirm HEAD oid and branch' '
+   git commit -m initial &&
+   H0=$(git rev-parse HEAD) &&
+   cat >expect <<-EOF &&
+   # branch.oid $H0
+   # branch.head master
+   ? actual
+   ? expect
+   EOF
+
+   git status --porcelain=v2 --branch --untracked-files=all >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'after first commit, make dirt, confirm unstaged changes' '
+   echo x >>file_x &&
+   OID_X1=$(git hash-object -t blob -- file_x) &&
+   rm file_z &&
+   H0=$(git rev-parse HEAD) &&
+
+   cat >expect <<-EOF &&
+   # branch.oid $H0
+   # branch.head master
+   1 .M N... 100644 100644 100644 $OID_X $OID_X file_x
+   1 .D N... 100644 100644 00 $OID_Z $OID_Z file_z
+   ? actual
+   ? expect
+   EOF
+
+   git status --porcelain=v2 --branch --untracked-files=all >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'confirm again unstaged changes, hiding untracked files' '
+   cat >expect <<-EOF &&
+   1 .M N... 100644 100644 100644 $OID_X $OID_X file_x
+   1 .D N... 100644 100644 00 $OID_Z $OID_Z file_z
+   EOF
+
+   git status --porcelain=v2 --untracked-files=no >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'after first commit, stage dirt, confirm staged changes' '
+   git add file_x &&
+   git rm file_z &&
+   H0=$(git rev-parse HEAD) &&
+
+   cat >expect <<-EOF &&
+   # branch.oid $H0
+   # branch.head master
+   1 M. N... 100644 100644 100644 $OID_X $OID_X1 file_x
+   1 D. N... 100644 00 00 $OID_Z $_z40 file_z
+   ? actual
+   ? expect
+   EOF
+
+   git status --porcelain=v2 --branch --untracked-files=all >actual &&
+   test_cmp expect actual
+'
+
+test_expect_success 'after first commit, also stage rename, confirm 2 path 
line format' '
+   git mv file_y renamed_y &&
+   H0=$(git rev-parse HEAD) &&
+
+   q_to_tab >expect <<-EOF &&
+   # branch.oid $H0
+   # branch.head master
+   1 M. N... 100644 100644 100644 $OID_

[PATCH v6 4/9] status: collect per-file data for --porcelain=v2

2016-08-11 Thread Jeff Hostetler
From: Jeff Hostetler 

Collect extra per-file data for porcelain V2 format.

The output of `git status --porcelain` leaves out many
details about the current status that clients might like
to have.  This can force them to be less efficient as they
may need to launch secondary commands (and try to match
the logic within git) to accumulate this extra information.
For example, a GUI IDE might want the file mode to display
the correct icon for a changed item (without having to stat
it afterwards).

Signed-off-by: Jeff Hostetler 
---
 builtin/commit.c |  3 +++
 wt-status.c  | 64 ++--
 wt-status.h  |  4 
 3 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 185ac35..3d222d3 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -153,6 +153,8 @@ static int opt_parse_porcelain(const struct option *opt, 
const char *arg, int un
*value = STATUS_FORMAT_PORCELAIN;
else if (!strcmp(arg, "v1") || !strcmp(arg, "1"))
*value = STATUS_FORMAT_PORCELAIN;
+   else if (!strcmp(arg, "v2") || !strcmp(arg, "2"))
+   *value = STATUS_FORMAT_PORCELAIN_V2;
else
die("unsupported porcelain version '%s'", arg);
 
@@ -1104,6 +1106,7 @@ static struct status_deferred_config {
 static void finalize_deferred_config(struct wt_status *s)
 {
int use_deferred_config = (status_format != STATUS_FORMAT_PORCELAIN &&
+  status_format != STATUS_FORMAT_PORCELAIN_V2 
&&
   !s->null_termination);
 
if (s->null_termination) {
diff --git a/wt-status.c b/wt-status.c
index a9031e4..904b5c2 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -434,6 +434,31 @@ static void wt_status_collect_changed_cb(struct 
diff_queue_struct *q,
if (S_ISGITLINK(p->two->mode))
d->new_submodule_commits = !!oidcmp(&p->one->oid,
&p->two->oid);
+
+   switch (p->status) {
+   case DIFF_STATUS_ADDED:
+   die("BUG: worktree status add???");
+   break;
+
+   case DIFF_STATUS_DELETED:
+   d->mode_index = p->one->mode;
+   oidcpy(&d->oid_index, &p->one->oid);
+   /* mode_worktree is zero for a delete. */
+   break;
+
+   case DIFF_STATUS_MODIFIED:
+   case DIFF_STATUS_TYPE_CHANGED:
+   case DIFF_STATUS_UNMERGED:
+   d->mode_index = p->one->mode;
+   d->mode_worktree = p->two->mode;
+   oidcpy(&d->oid_index, &p->one->oid);
+   break;
+
+   case DIFF_STATUS_UNKNOWN:
+   die("BUG: worktree status unknown???");
+   break;
+   }
+
}
 }
 
@@ -479,12 +504,36 @@ static void wt_status_collect_updated_cb(struct 
diff_queue_struct *q,
if (!d->index_status)
d->index_status = p->status;
switch (p->status) {
+   case DIFF_STATUS_ADDED:
+   /* Leave {mode,oid}_head zero for an add. */
+   d->mode_index = p->two->mode;
+   oidcpy(&d->oid_index, &p->two->oid);
+   break;
+   case DIFF_STATUS_DELETED:
+   d->mode_head = p->one->mode;
+   oidcpy(&d->oid_head, &p->one->oid);
+   /* Leave {mode,oid}_index zero for a delete. */
+   break;
+
case DIFF_STATUS_COPIED:
case DIFF_STATUS_RENAMED:
d->head_path = xstrdup(p->one->path);
+   d->score = p->score * 100 / MAX_SCORE;
+   /* fallthru */
+   case DIFF_STATUS_MODIFIED:
+   case DIFF_STATUS_TYPE_CHANGED:
+   d->mode_head = p->one->mode;
+   d->mode_index = p->two->mode;
+   oidcpy(&d->oid_head, &p->one->oid);
+   oidcpy(&d->oid_index, &p->two->oid);
break;
case DIFF_STATUS_UNMERGED:
d->stagemask = unmerged_mask(p->two->path);
+   /*
+* Don't bother setting {mode,oid}_{head,index} since 
the print
+* code will output the stage values directly and not 
use the
+* values in these fields.
+*/
break;
}
}
@@ -565,9 +614,17 @@ static void wt_status_collect_changes_initial(struct 
wt_status *s)
if (ce_stage(ce)) {
d->index_status = DIFF_STATUS_UNMERGED;
  

[PATCH v6 1/9] status: rename long-format print routines

2016-08-11 Thread Jeff Hostetler
From: Jeff Hostetler 

Rename the various wt_status_print*() routines to be
wt_longstatus_print*() to make it clear that these
routines are only concerned with the normal/long
status output and reduce developer confusion as other
status formats are added in the future.

Signed-off-by: Jeff Hostetler 
---
 builtin/commit.c |   4 +-
 wt-status.c  | 110 +++
 wt-status.h  |   2 +-
 3 files changed, 58 insertions(+), 58 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 1f6dbcd..b80273b 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -515,7 +515,7 @@ static int run_status(FILE *fp, const char *index_file, 
const char *prefix, int
break;
case STATUS_FORMAT_NONE:
case STATUS_FORMAT_LONG:
-   wt_status_print(s);
+   wt_longstatus_print(s);
break;
}
 
@@ -1403,7 +1403,7 @@ int cmd_status(int argc, const char **argv, const char 
*prefix)
case STATUS_FORMAT_LONG:
s.verbose = verbose;
s.ignore_submodule_arg = ignore_submodule_arg;
-   wt_status_print(&s);
+   wt_longstatus_print(&s);
break;
}
return 0;
diff --git a/wt-status.c b/wt-status.c
index de62ab2..b9a58fd 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -139,7 +139,7 @@ void wt_status_prepare(struct wt_status *s)
s->display_comment_prefix = 0;
 }
 
-static void wt_status_print_unmerged_header(struct wt_status *s)
+static void wt_longstatus_print_unmerged_header(struct wt_status *s)
 {
int i;
int del_mod_conflict = 0;
@@ -191,7 +191,7 @@ static void wt_status_print_unmerged_header(struct 
wt_status *s)
status_printf_ln(s, c, "%s", "");
 }
 
-static void wt_status_print_cached_header(struct wt_status *s)
+static void wt_longstatus_print_cached_header(struct wt_status *s)
 {
const char *c = color(WT_STATUS_HEADER, s);
 
@@ -207,9 +207,9 @@ static void wt_status_print_cached_header(struct wt_status 
*s)
status_printf_ln(s, c, "%s", "");
 }
 
-static void wt_status_print_dirty_header(struct wt_status *s,
-int has_deleted,
-int has_dirty_submodules)
+static void wt_longstatus_print_dirty_header(struct wt_status *s,
+int has_deleted,
+int has_dirty_submodules)
 {
const char *c = color(WT_STATUS_HEADER, s);
 
@@ -226,9 +226,9 @@ static void wt_status_print_dirty_header(struct wt_status 
*s,
status_printf_ln(s, c, "%s", "");
 }
 
-static void wt_status_print_other_header(struct wt_status *s,
-const char *what,
-const char *how)
+static void wt_longstatus_print_other_header(struct wt_status *s,
+const char *what,
+const char *how)
 {
const char *c = color(WT_STATUS_HEADER, s);
status_printf_ln(s, c, "%s:", what);
@@ -238,7 +238,7 @@ static void wt_status_print_other_header(struct wt_status 
*s,
status_printf_ln(s, c, "%s", "");
 }
 
-static void wt_status_print_trailer(struct wt_status *s)
+static void wt_longstatus_print_trailer(struct wt_status *s)
 {
status_printf_ln(s, color(WT_STATUS_HEADER, s), "%s", "");
 }
@@ -304,8 +304,8 @@ static int maxwidth(const char *(*label)(int), int minval, 
int maxval)
return result;
 }
 
-static void wt_status_print_unmerged_data(struct wt_status *s,
- struct string_list_item *it)
+static void wt_longstatus_print_unmerged_data(struct wt_status *s,
+ struct string_list_item *it)
 {
const char *c = color(WT_STATUS_UNMERGED, s);
struct wt_status_change_data *d = it->util;
@@ -331,9 +331,9 @@ static void wt_status_print_unmerged_data(struct wt_status 
*s,
strbuf_release(&onebuf);
 }
 
-static void wt_status_print_change_data(struct wt_status *s,
-   int change_type,
-   struct string_list_item *it)
+static void wt_longstatus_print_change_data(struct wt_status *s,
+   int change_type,
+   struct string_list_item *it)
 {
struct wt_status_change_data *d = it->util;
const char *c = color(change_type, s);
@@ -378,7 +378,7 @@ static void wt_status_print_change_data(struct wt_status *s,
status = d->worktree_status;
break;
default:
-   die("BUG: unhandled change_type %d in 
wt_status_print_change_data",
+   die("BUG: unhandled change_type %d in 
wt_longstatus_print_change_data",
change_type);
}
 
@@

[PATCH v6 6/9] status: print branch info with --porcelain=v2 --branch

2016-08-11 Thread Jeff Hostetler
From: Jeff Hostetler 

Expand porcelain v2 output to include branch and tracking
branch information. This includes the commit id, the branch,
the upstream branch, and the ahead and behind counts.

Signed-off-by: Jeff Hostetler 
---
 builtin/commit.c |  5 
 wt-status.c  | 90 
 wt-status.h  |  1 +
 3 files changed, 96 insertions(+)

diff --git a/builtin/commit.c b/builtin/commit.c
index 3d222d3..5504afe 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -510,6 +510,8 @@ static int run_status(FILE *fp, const char *index_file, 
const char *prefix, int
s->fp = fp;
s->nowarn = nowarn;
s->is_initial = get_sha1(s->reference, sha1) ? 1 : 0;
+   if (!s->is_initial)
+   hashcpy(s->sha1_commit, sha1);
s->status_format = status_format;
s->ignore_submodule_arg = ignore_submodule_arg;
 
@@ -1378,6 +1380,9 @@ int cmd_status(int argc, const char **argv, const char 
*prefix)
fd = hold_locked_index(&index_lock, 0);
 
s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;
+   if (!s.is_initial)
+   hashcpy(s.sha1_commit, sha1);
+
s.ignore_submodule_arg = ignore_submodule_arg;
s.status_format = status_format;
s.verbose = verbose;
diff --git a/wt-status.c b/wt-status.c
index 03c8e23..3609531 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1813,6 +1813,92 @@ static void wt_porcelain_print(struct wt_status *s)
 }
 
 /*
+ * Print branch information for porcelain v2 output.  These lines
+ * are printed when the '--branch' parameter is given.
+ *
+ *# branch.oid 
+ *# branch.head 
+ *   [# branch.upstream 
+ *   [# branch.ab + -]]
+ *
+ *   ::= the current commit hash or the the literal
+ *   "(initial)" to indicate an initialized repo
+ *   with no commits.
+ *
+ * ::=  the current branch name or
+ *   "(detached)" literal when detached head or
+ *   "(unknown)" when something is wrong.
+ *
+ * ::= the upstream branch name, when set.
+ *
+ *::= integer ahead value, when upstream set
+ *   and the commit is present (not gone).
+ *
+ *   ::= integer behind value, when upstream set
+ *   and commit is present.
+ *
+ *
+ * The end-of-line is defined by the -z flag.
+ *
+ *  ::= NUL when -z,
+ *   LF when NOT -z.
+ *
+ */
+static void wt_porcelain_v2_print_tracking(struct wt_status *s)
+{
+   struct branch *branch;
+   const char *base;
+   const char *branch_name;
+   struct wt_status_state state;
+   int ab_info, nr_ahead, nr_behind;
+   char eol = s->null_termination ? '\0' : '\n';
+
+   memset(&state, 0, sizeof(state));
+   wt_status_get_state(&state, s->branch && !strcmp(s->branch, "HEAD"));
+
+   fprintf(s->fp, "# branch.oid %s%c",
+   (s->is_initial ? "(initial)" : 
sha1_to_hex(s->sha1_commit)),
+   eol);
+
+   if (!s->branch)
+   fprintf(s->fp, "# branch.head %s%c", "(unknown)", eol);
+   else {
+   if (!strcmp(s->branch, "HEAD")) {
+   fprintf(s->fp, "# branch.head %s%c", "(detached)", eol);
+
+   if (state.rebase_in_progress || 
state.rebase_interactive_in_progress)
+   branch_name = state.onto;
+   else if (state.detached_from)
+   branch_name = state.detached_from;
+   else
+   branch_name = "";
+   } else {
+   branch_name = NULL;
+   skip_prefix(s->branch, "refs/heads/", &branch_name);
+
+   fprintf(s->fp, "# branch.head %s%c", branch_name, eol);
+   }
+
+   /* Lookup stats on the upstream tracking branch, if set. */
+   branch = branch_get(branch_name);
+   base = NULL;
+   ab_info = (stat_tracking_info(branch, &nr_ahead, &nr_behind, 
&base) == 0);
+   if (base) {
+   base = shorten_unambiguous_ref(base, 0);
+   fprintf(s->fp, "# branch.upstream %s%c", base, eol);
+   free((char *)base);
+
+   if (ab_info)
+   fprintf(s->fp, "# branch.ab +%d -%d%c", 
nr_ahead, nr_behind, eol);
+   }
+   }
+
+   free(state.branch);
+   free(state.onto);
+   free(state.detached_from);
+}
+
+/*
  * Convert various submodule status values into a
  * fixed-length string of characters in the buffer provided.
  */
@@ -2058,6 +2144,7 @@ static void wt_porcelain_v2_print_other(
 /*
  * Print porcelain V2 status.
  *
+ * []
  * []*
  * []*
  * []*
@@ -2070,6 +2157,9 @@ static void wt_porcelain_v2_print(struct wt_status *s)
struct string_list_item *it;
 

[PATCH v6 2/9] status: cleanup API to wt_status_print

2016-08-11 Thread Jeff Hostetler
From: Jeff Hostetler 

Refactor the API between builtin/commit.c and wt-status.[ch].

Hide the details of the various wt_*status_print() routines inside
wt-status.c behind a single (new) wt_status_print() routine.
Eliminate the switch statements from builtin/commit.c.
Allow details of new status formats to be isolated within wt-status.c

Signed-off-by: Jeff Hostetler 
---
 builtin/commit.c | 51 +--
 wt-status.c  | 25 ++---
 wt-status.h  | 16 
 3 files changed, 43 insertions(+), 49 deletions(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index b80273b..a792deb 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -142,14 +142,7 @@ static int show_ignored_in_status, have_option_m;
 static const char *only_include_assumed;
 static struct strbuf message = STRBUF_INIT;
 
-static enum status_format {
-   STATUS_FORMAT_NONE = 0,
-   STATUS_FORMAT_LONG,
-   STATUS_FORMAT_SHORT,
-   STATUS_FORMAT_PORCELAIN,
-
-   STATUS_FORMAT_UNSPECIFIED
-} status_format = STATUS_FORMAT_UNSPECIFIED;
+static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
 
 static int opt_parse_m(const struct option *opt, const char *arg, int unset)
 {
@@ -500,24 +493,11 @@ static int run_status(FILE *fp, const char *index_file, 
const char *prefix, int
s->fp = fp;
s->nowarn = nowarn;
s->is_initial = get_sha1(s->reference, sha1) ? 1 : 0;
+   s->status_format = status_format;
+   s->ignore_submodule_arg = ignore_submodule_arg;
 
wt_status_collect(s);
-
-   switch (status_format) {
-   case STATUS_FORMAT_SHORT:
-   wt_shortstatus_print(s);
-   break;
-   case STATUS_FORMAT_PORCELAIN:
-   wt_porcelain_print(s);
-   break;
-   case STATUS_FORMAT_UNSPECIFIED:
-   die("BUG: finalize_deferred_config() should have been called");
-   break;
-   case STATUS_FORMAT_NONE:
-   case STATUS_FORMAT_LONG:
-   wt_longstatus_print(s);
-   break;
-   }
+   wt_status_print(s);
 
return s->commitable;
 }
@@ -1099,7 +1079,7 @@ static const char *read_commit_message(const char *name)
  * is not in effect here.
  */
 static struct status_deferred_config {
-   enum status_format status_format;
+   enum wt_status_format status_format;
int show_branch;
 } status_deferred_config = {
STATUS_FORMAT_UNSPECIFIED,
@@ -1381,6 +1361,9 @@ int cmd_status(int argc, const char **argv, const char 
*prefix)
 
s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;
s.ignore_submodule_arg = ignore_submodule_arg;
+   s.status_format = status_format;
+   s.verbose = verbose;
+
wt_status_collect(&s);
 
if (0 <= fd)
@@ -1389,23 +1372,7 @@ int cmd_status(int argc, const char **argv, const char 
*prefix)
if (s.relative_paths)
s.prefix = prefix;
 
-   switch (status_format) {
-   case STATUS_FORMAT_SHORT:
-   wt_shortstatus_print(&s);
-   break;
-   case STATUS_FORMAT_PORCELAIN:
-   wt_porcelain_print(&s);
-   break;
-   case STATUS_FORMAT_UNSPECIFIED:
-   die("BUG: finalize_deferred_config() should have been called");
-   break;
-   case STATUS_FORMAT_NONE:
-   case STATUS_FORMAT_LONG:
-   s.verbose = verbose;
-   s.ignore_submodule_arg = ignore_submodule_arg;
-   wt_longstatus_print(&s);
-   break;
-   }
+   wt_status_print(&s);
return 0;
 }
 
diff --git a/wt-status.c b/wt-status.c
index b9a58fd..a9031e4 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1447,7 +1447,7 @@ static void wt_longstatus_print_state(struct wt_status *s,
show_bisect_in_progress(s, state, state_color);
 }
 
-void wt_longstatus_print(struct wt_status *s)
+static void wt_longstatus_print(struct wt_status *s)
 {
const char *branch_color = color(WT_STATUS_ONBRANCH, s);
const char *branch_status_color = color(WT_STATUS_HEADER, s);
@@ -1714,7 +1714,7 @@ static void wt_shortstatus_print_tracking(struct 
wt_status *s)
fputc(s->null_termination ? '\0' : '\n', s->fp);
 }
 
-void wt_shortstatus_print(struct wt_status *s)
+static void wt_shortstatus_print(struct wt_status *s)
 {
int i;
 
@@ -1746,7 +1746,7 @@ void wt_shortstatus_print(struct wt_status *s)
}
 }
 
-void wt_porcelain_print(struct wt_status *s)
+static void wt_porcelain_print(struct wt_status *s)
 {
s->use_color = 0;
s->relative_paths = 0;
@@ -1754,3 +1754,22 @@ void wt_porcelain_print(struct wt_status *s)
s->no_gettext = 1;
wt_shortstatus_print(s);
 }
+
+void wt_status_print(struct wt_status *s)
+{
+   switch (s->status_format) {
+   case STATUS_FORMAT_SHORT:
+   wt_shortstatus_print(s);
+   break;
+   ca

[PATCH v6 0/9] status: V2 porcelain status

2016-08-11 Thread Jeff Hostetler
From: Jeff Hostetler 

This patch series adds porcelain V2 format to status.
This provides detailed information about file changes
and about the current branch.

The new output is accessed via:
git status --porcelain=v2 [--branch]

This v6 patch series addresses the unit test discussion
from the previous series.

I also moved the "else {" onto the previous line as
was discussed in a previous comment.


Jeff Hostetler (9):
  status: rename long-format print routines
  status: cleanup API to wt_status_print
  status: support --porcelain[=]
  status: collect per-file data for --porcelain=v2
  status: print per-file porcelain v2 status data
  status: print branch info with --porcelain=v2 --branch
  git-status.txt: describe --porcelain=v2 format
  test-lib-functions.sh: Add lf_to_nul
  status: unit tests for --porcelain=v2

 Documentation/git-status.txt | 133 +-
 builtin/commit.c |  78 +++---
 t/t7060-wtstatus.sh  |  21 ++
 t/t7064-wtstatus-pv2.sh  | 576 +++
 t/test-lib-functions.sh  |   4 +
 wt-status.c  | 570 +-
 wt-status.h  |  19 +-
 7 files changed, 1289 insertions(+), 112 deletions(-)
 create mode 100755 t/t7064-wtstatus-pv2.sh

-- 
2.8.0.rc4.17.gac42084.dirty

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


[PATCH v6 3/9] status: support --porcelain[=]

2016-08-11 Thread Jeff Hostetler
From: Jeff Hostetler 

Update --porcelain argument to take optional version parameter
to allow multiple porcelain formats to be supported in the future.

The token "v1" is the default value and indicates the traditional
porcelain format.  (The token "1" is an alias for that.)

Signed-off-by: Jeff Hostetler 
---
 Documentation/git-status.txt |  7 +--
 builtin/commit.c | 21 ++---
 t/t7060-wtstatus.sh  | 21 +
 3 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index e1e8f57..6b1454b 100644
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -32,11 +32,14 @@ OPTIONS
 --branch::
Show the branch and tracking info even in short-format.
 
---porcelain::
+--porcelain[=]::
Give the output in an easy-to-parse format for scripts.
This is similar to the short output, but will remain stable
across Git versions and regardless of user configuration. See
below for details.
++
+The version parameter is used to specify the format version.
+This is optional and defaults to the original version 'v1' format.
 
 --long::
Give the output in the long-format. This is the default.
@@ -96,7 +99,7 @@ configuration variable documented in linkgit:git-config[1].
 
 -z::
Terminate entries with NUL, instead of LF.  This implies
-   the `--porcelain` output format if no other format is given.
+   the `--porcelain=v1` output format if no other format is given.
 
 --column[=]::
 --no-column::
diff --git a/builtin/commit.c b/builtin/commit.c
index a792deb..185ac35 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -144,6 +144,21 @@ static struct strbuf message = STRBUF_INIT;
 
 static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;
 
+static int opt_parse_porcelain(const struct option *opt, const char *arg, int 
unset)
+{
+   enum wt_status_format *value = (enum wt_status_format *)opt->value;
+   if (unset)
+   *value = STATUS_FORMAT_NONE;
+   else if (!arg)
+   *value = STATUS_FORMAT_PORCELAIN;
+   else if (!strcmp(arg, "v1") || !strcmp(arg, "1"))
+   *value = STATUS_FORMAT_PORCELAIN;
+   else
+   die("unsupported porcelain version '%s'", arg);
+
+   return 0;
+}
+
 static int opt_parse_m(const struct option *opt, const char *arg, int unset)
 {
struct strbuf *buf = opt->value;
@@ -1316,9 +1331,9 @@ int cmd_status(int argc, const char **argv, const char 
*prefix)
N_("show status concisely"), STATUS_FORMAT_SHORT),
OPT_BOOL('b', "branch", &s.show_branch,
 N_("show branch information")),
-   OPT_SET_INT(0, "porcelain", &status_format,
-   N_("machine-readable output"),
-   STATUS_FORMAT_PORCELAIN),
+   { OPTION_CALLBACK, 0, "porcelain", &status_format,
+ N_("version"), N_("machine-readable output"),
+ PARSE_OPT_OPTARG, opt_parse_porcelain },
OPT_SET_INT(0, "long", &status_format,
N_("show status in long format (default)"),
STATUS_FORMAT_LONG),
diff --git a/t/t7060-wtstatus.sh b/t/t7060-wtstatus.sh
index 44bf1d8..00e0ceb 100755
--- a/t/t7060-wtstatus.sh
+++ b/t/t7060-wtstatus.sh
@@ -228,4 +228,25 @@ test_expect_success 'status --branch with detached HEAD' '
test_i18ncmp expected actual
 '
 
+## Duplicate the above test and verify --porcelain=v1 arg parsing.
+test_expect_success 'status --porcelain=v1 --branch with detached HEAD' '
+   git reset --hard &&
+   git checkout master^0 &&
+   git status --branch --porcelain=v1 >actual &&
+   cat >expected <<-EOF &&
+   ## HEAD (no branch)
+   ?? .gitconfig
+   ?? actual
+   ?? expect
+   ?? expected
+   ?? mdconflict/
+   EOF
+   test_i18ncmp expected actual
+'
+
+## Verify parser error on invalid --porcelain argument.
+test_expect_success 'status --porcelain=bogus' '
+   test_must_fail git status --porcelain=bogus
+'
+
 test_done
-- 
2.8.0.rc4.17.gac42084.dirty

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


Re: [PATCH] guilt: update reflog with annotations of guilt-command being run

2016-08-11 Thread Josef 'Jeff' Sipek
On Sat, Jul 09, 2016 at 18:16:05 -0400, Theodore Ts'o wrote:
> Many of the updates made by guilt use git update-ref, which means that
> the output of "git reflog" is extremely unedifying, e.g:

This has been an annoyance for me as well.  Thanks for fixing it.  I'll give
it a test run, and likely push it out later today.

Jeff.

> ff0031d HEAD@{177}: reset: moving to ff0031d848a0cd7002606f9feef958de8d5edf19
> 90f4305 HEAD@{178}:
> a638d43 HEAD@{179}:
> ff0031d HEAD@{180}:
> 079788d HEAD@{181}:
> 87a6280 HEAD@{182}:
> 5b9554d HEAD@{183}:
> de9e918 HEAD@{184}: reset: moving to de9e9181bc066d63d78b768e95b5d949e2a8673a
> 5b9554d HEAD@{185}:
> 
> So teach guilt to use the "set_reflog_action" helper, and since
> git-update-ref doesn't respect the GIT_REFLOG_ACTION environment
> variable, use its -m option so that "git reflog" can look like this
> instead:
> 
> 1eaa566 HEAD@{11}: guilt-push: track-more-dependencies-on-transaction-commit
> ab714af HEAD@{12}: guilt-push: move-lockdep-tracking-to-journal_s
> 7a4b188 HEAD@{13}: guilt-push: move-lockdep-instrumentation-for-jbd2-handles
> 78d9625 HEAD@{14}: guilt-push: 
> respect-nobarrier-mount-option-in-nojournal-mode
> d08854f HEAD@{15}: guilt-pop: updating HEAD
> d08854f HEAD@{16}: guilt-pop: updating HEAD
> d08854f HEAD@{17}: guilt-push: 
> optimize-ext4_should_retry_alloc-to-improve-ENOSPC-performance
> 
> Signed-off-by: Theodore Ts'o 
> Cc: Josef 'Jeff' Sipek 
> ---
>  guilt | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/guilt b/guilt
> index 35177b9..38d426b 100755
> --- a/guilt
> +++ b/guilt
> @@ -114,6 +114,7 @@ if [ $# -ne 0 ]; then
>   disp "" >&2
>   exit 1
>   fi
> + set_reflog_action "guilt-$CMDNAME"
>  
>   shift
>  else
> @@ -640,7 +641,7 @@ commit()
>   commitish=`git commit-tree $treeish -p $2 < "$TMP_MSG"`
>   if $old_style_prefix || git rev-parse --verify --quiet 
> refs/heads/$GUILT_PREFIX$branch >/dev/null
>   then
> - git update-ref HEAD $commitish
> + git update-ref -m "$GIT_REFLOG_ACTION" HEAD $commitish
>   else
>   git branch $GUILT_PREFIX$branch $commitish
>   git symbolic-ref HEAD refs/heads/$GUILT_PREFIX$branch
> @@ -687,7 +688,8 @@ push_patch()
>   fi
>   fi
>  
> - commit "$pname" HEAD
> + GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: $pname" \
> + commit "$pname" HEAD
>  
>   echo "$pname" >> "$applied"
>  
> -- 
> 2.5.0
> 

-- 
Real Programmers consider "what you see is what you get" to be just as bad a
concept in Text Editors as it is in women. No, the Real Programmer wants a
"you asked for it, you got it" text editor -- complicated, cryptic,
powerful, unforgiving, dangerous.
--
To unsubscribe from this list: send the line "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: Deposit|Charitable Gesture

2016-08-11 Thread Susanne Klatten
Hello Sir/Madam,
Your 3.7 million USD deposit from Susanne Klatten 50M Philanthropic Donation is 
ready.
Contact: susannecharityfoundat...@aol.de
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5] pack-objects mru

2016-08-11 Thread Jeff King
On Thu, Aug 11, 2016 at 05:20:30AM -0400, Jeff King wrote:

> Here it is. It ended up needing a few preparatory patches.
> 
>   [1/4]: provide an initializer for "struct object_info"
>   [2/4]: sha1_file: make packed_object_info public
>   [3/4]: pack-objects: break delta cycles before delta-search phase
>   [4/4]: pack-objects: use mru list when iterating over packs
> 
> I had originally intended to include an extra patch to handle the
> --depth limits better. But after writing it, I'm not sure it's actually
> a good idea. I'll post it as an addendum with more discussion.

And here's the depth patch. It does work, as you can see by running the
snippet at the bottom of the commit message.

But I began to wonder if it's actually a desirable thing. For instance,
if you do:

  git gc --aggressive
  ... time passes ...
  git gc

the first gc may generate chains up to 250 objects. When the second gc
runs (and you may not even run it yourself; it might be "gc --auto"!),
it will generally reuse most of your existing deltas, even though the
default depth is only 50.

But with the patch below, it will drop deltas from the middle of those
long chains, undoing the prior --aggressive results. Worse, we don't
find new deltas for those objects, because it falls afoul of the "they
are in the same pack, so don't bother looking for a delta" rule.

An --aggressive repack of my git.git is 52MB. Repacking that with the
patch below and "--depth=50" bumps it to 55MB. Dumping the "do not
bother" condition in try_delta() drops that to 54MB.

So it _is_ worse for the space to drop those high-depth deltas. Even if
we fixed the "do not bother" (e.g., by recording a bit that says "even
though these are in the same pack, try anyway, because we had to break
the delta for other reasons"), it's still a loss.

OTOH, I am not altogether convinced that the tradeoff of a giant --depth
is worth. I'm looking only at the space here, but deeper delta chains
cost more CPU to access (especially if they start to exceed the delta
cache size). And the space savings aren't that amazing. Doing a "git
repack -adf --depth=50 --window=250" (i.e., if aggressive had just
tweaked the window size and not the depth in the first place), the
result is only 53MB.

So considering "--depth" as a space-saving measure for --aggressive does
not seem that effective. But it feels weird to quietly drop actions
people might have done with previous aggressive runs.

-- >8 --
Subject: [PATCH] pack-objects: enforce --depth limit in reused deltas

Since 898b14c (pack-objects: rework check_delta_limit usage,
2007-04-16), we check the delta depth limit only when
figuring out whether we should make a new delta. We don't
consider it at all when reusing deltas, which means that
packing once with --depth=50, and then against with
--depth=10, the second pack my still contain chains larger
than 10.

This is probably not a huge deal, as it is limited to
whatever chains you happened to create in a previous run.
But as we start allowing cross-pack delta reuse in a future
commit, this maximum will rise to the number of packs times
the per-pack depth (in the worst case; on average, it will
likely be much smaller).

We can easily detect this as part of the existing search for
cycles, since we visit every node in a depth-first way. That
lets us compute the depth of any node based on the depth of
its base, because we know the base is DFS_DONE by the time
we look at it (modulo any cycles in the graph, but we know
there cannot be any because we break them as we see them).

There is some subtlety worth mentioning, though. We record
the depth of each object as we compute it. It might seem
like we could save the per-object storage space by just
keeping track of the depth of our traversal (i.e., have
break_delta_chains() report how deep it went). But we may
visit an object through multiple delta paths, and on
subsequent paths we want to know its depth immediately,
without having to walk back down to its final base (doing so
would make our graph walk quadratic rather than linear).

Likewise, one could try to record the depth not from the
base, but from our starting point (i.e., start
recursion_depth at 0, and pass "recursion_depth + 1" to each
invocation of break_delta_chains()). And then when
recursion_depth gets too big, we know that we must cut the
delta chain.  But that technique is wrong if we do not visit
the nodes in topological order. In a chain A->B->C, it
if we visit "C", then "B", then "A", we will never recurse
deeper than 1 link (because we see at each node that we have
already visited it).

Unfortunately there is no automated test, because it's hard
to convince pack-objects to reliably produce delta chains.
Naively, it would seem that a sequence of ever-increasing
blobs would work. E.g., something like:

  for i in 1 2 3 4 5; do
  test-genrandom $i 4096 >>file
  git add file
  git commit -m $i
  done

where a reasonable set of deltas would use "1:file" as the
b

[PATCH v5 4/4] pack-objects: use mru list when iterating over packs

2016-08-11 Thread Jeff King
In the original implementation of want_object_in_pack(), we
always looked for the object in every pack, so the order did
not matter for performance.

As of the last few patches, however, we can now often break
out of the loop early after finding the first instance, and
avoid looking in the other packs at all. In this case, pack
order can make a big difference, because we'd like to find
the objects by looking at as few packs as possible.

This patch switches us to the same packed_git_mru list that
is now used by normal object lookups.

Here are timings for p5303 on linux.git:

Test  HEAD^HEAD

5303.3: rev-list (1)  31.31(31.07+0.23)31.28(31.00+0.27) -0.1%
5303.4: repack (1)40.35(38.84+2.60)40.53(39.31+2.32) +0.4%
5303.6: rev-list (50) 31.37(31.15+0.21)31.41(31.16+0.24) +0.1%
5303.7: repack (50)   58.25(68.54+2.03)47.28(57.66+1.89) -18.8%
5303.9: rev-list (1000)   31.91(31.57+0.33)31.93(31.64+0.28) +0.1%
5303.10: repack (1000)304.80(376.00+3.92)  87.21(159.54+2.84) -71.4%

The rev-list numbers are unchanged, which makes sense (they
are not exercising this code at all). The 50- and 1000-pack
repack cases show considerable improvement.

The single-pack repack case doesn't, of course; there's
nothing to improve. In fact, it gives us a baseline for how
fast we could possibly go. You can see that though rev-list
can approach the single-pack case even with 1000 packs,
repack doesn't. The reason is simple: the loop we are
optimizing is only part of what the repack is doing. After
the "counting" phase, we do delta compression, which is much
more expensive when there are multiple packs, because we
have fewer deltas we can reuse (you can also see that these
numbers come from a multicore machine; the CPU times are
much higher than the wall-clock times due to the delta
phase).

So the good news is that in cases with many packs, we used
to be dominated by the "counting" phase, and now we are
dominated by the delta compression (which is faster, and
which we have already parallelized).

Here are similar numbers for git.git:

Test  HEAD^   HEAD
-
5303.3: rev-list (1)  1.55(1.51+0.02) 1.54(1.53+0.00) -0.6%
5303.4: repack (1)1.82(1.80+0.08) 1.82(1.78+0.09) +0.0%
5303.6: rev-list (50) 1.58(1.57+0.00) 1.58(1.56+0.01) +0.0%
5303.7: repack (50)   2.50(3.12+0.07) 2.31(2.95+0.06) -7.6%
5303.9: rev-list (1000)   2.22(2.20+0.02) 2.23(2.19+0.03) +0.5%
5303.10: repack (1000)10.47(16.78+0.22)   7.50(13.76+0.22) -28.4%

Not as impressive in terms of percentage, but still
measurable wins.  If you look at the wall-clock time
improvements in the 1000-pack case, you can see that linux
improved by roughly 10x as many seconds as git. That's
because it has roughly 10x as many objects, and we'd expect
this improvement to scale linearly with the number of
objects (since the number of packs is kept constant). It's
just that the "counting" phase is a smaller percentage of
the total time spent for a git.git repack, and hence the
percentage win is smaller.

The implementation itself is a straightforward use of the
MRU code. We only bother marking a pack as used when we know
that we are able to break early out of the loop, for two
reasons:

  1. If we can't break out early, it does no good; we have
 to visit each pack anyway, so we might as well avoid
 even the minor overhead of managing the cache order.

  2. The mru_mark() function reorders the list, which would
 screw up our traversal. So it is only safe to mark when
 we are about to break out of the loop. We could record
 the found pack and mark it after the loop finishes, of
 course, but that's more complicated and it doesn't buy
 us anything due to (1).

Note that this reordering does have a potential impact on
the final pack, as we store only a single "found" pack for
each object, even if it is present in multiple packs. In
principle, any copy is acceptable, as they all refer to the
same content. But in practice, they may differ in whether
they are stored as deltas, against which base, etc. This may
have an impact on delta reuse, and even the delta search
(since we skip pairs that were already in the same pack).

It's not clear whether this change of order would hurt or
even help average cases, though. The most likely reason to
have duplicate objects is from the completion of thin packs
(e.g., you have some objects in a "base" pack, then receive
several pushes; the packs you receive may be thin on the
wire, with deltas that refer to bases outside the pack, but
we complete them with duplicate base objects when indexing
them).

In such a case the current code would always find the thin
duplicates (because we currently walk the packs in reverse
chronological order). Whereas with 

[PATCH v5 3/4] pack-objects: break delta cycles before delta-search phase

2016-08-11 Thread Jeff King
We do not allow cycles in the delta graph of a pack (i.e., A
is a delta of B which is a delta of A) for the obvious
reason that you cannot actually access any of the objects in
such a case.

There's a last-ditch attempt to notice cycles during the
write phase, during which we issue a warning to the user and
write one of the objects out in full. However, this is
"last-ditch" for two reasons:

  1. By this time, it's too late to find another delta for
 the object, so the resulting pack is larger than it
 otherwise could be.

  2. The warning is there because this is something that
 _shouldn't_ ever happen. If it does, then either:

   a. a pack we are reusing deltas from had its own
  cycle

   b. we are reusing deltas from multiple packs, and
  we found a cycle among them (i.e., A is a delta of
  B in one pack, but B is a delta of A in another,
  and we choose to use both deltas).

   c. there is a bug in the delta-search code

 So this code serves as a final check that none of these
 things has happened, warns the user, and prevents us
 from writing a bogus pack.

Right now, (2b) should never happen because of the static
ordering of packs in want_object_in_pack(). If two objects
have a delta relationship, then they must be in the same
pack, and therefore we will find them from that same pack.

However, a future patch would like to change that static
ordering, which will make (2b) a common occurrence. In
preparation, we should be able to handle those kinds of
cycles better. This patch does by introducing a
cycle-breaking step during the get_object_details() phase,
when we are deciding which deltas can be reused. That gives
us the chance to feed the objects into the delta search as
if the cycle did not exist.

We'll leave the detection and warning in the write_object()
phase in place, as it still serves as a check for case (2c).

This does mean we will stop warning for (2a). That case is
caused by bogus input packs, and we ideally would warn the
user about it.  However, since those cycles show up after
picking reusable deltas, they look the same as (2b) to us;
our new code will break the cycles early and the last-ditch
check will never see them.

We could do analysis on any cycles that we find to
distinguish the two cases (i.e., it is a bogus pack if and
only if every delta in the cycle is in the same pack), but
we don't need to. If there is a cycle inside a pack, we'll
run into problems not only reusing the delta, but accessing
the object data at all. So when we try to dig up the actual
size of the object, we'll hit that same cycle and kick in
our usual complain-and-try-another-source code.

Signed-off-by: Jeff King 
---
This has the DONE and printf fixes from v4, along with using
packed_object_info() to fill in the correct type when drop a delta.

 builtin/pack-objects.c  |  84 +
 pack-objects.h  |   9 
 t/t5314-pack-cycle-detection.sh | 113 
 3 files changed, 206 insertions(+)
 create mode 100755 t/t5314-pack-cycle-detection.sh

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 4a63398..32c1dba 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1495,6 +1495,83 @@ static int pack_offset_sort(const void *_a, const void 
*_b)
(a->in_pack_offset > b->in_pack_offset);
 }
 
+/*
+ * Drop an on-disk delta we were planning to reuse. Naively, this would
+ * just involve blanking out the "delta" field, but we have to deal
+ * with some extra book-keeping:
+ *
+ *   1. Removing ourselves from the delta_sibling linked list.
+ *
+ *   2. Updating our size/type to the non-delta representation. These were
+ *  either not recorded initially (size) or overwritten with the delta type
+ *  (type) when check_object() decided to reuse the delta.
+ */
+static void drop_reused_delta(struct object_entry *entry)
+{
+   struct object_entry **p = &entry->delta->delta_child;
+   struct object_info oi = OBJECT_INFO_INIT;
+
+   while (*p) {
+   if (*p == entry)
+   *p = (*p)->delta_sibling;
+   else
+   p = &(*p)->delta_sibling;
+   }
+   entry->delta = NULL;
+
+   oi.sizep = &entry->size;
+   oi.typep = &entry->type;
+   if (packed_object_info(entry->in_pack, entry->in_pack_offset, &oi) < 0) 
{
+   /*
+* We failed to get the info from this pack for some reason;
+* fall back to sha1_object_info, which may find another copy.
+* And if that fails, the error will be recorded in entry->type
+* and dealt with in prepare_pack().
+*/
+   entry->type = sha1_object_info(entry->idx.sha1, &entry->size);
+   }
+}
+
+/*
+ * Follow the chain of deltas from this entry onward, throwing away any links
+ * that cause us to hit

[PATCH v5 2/4] sha1_file: make packed_object_info public

2016-08-11 Thread Jeff King
Some code may have a pack/offset pair for an object, but
would like to look up more information. Using
sha1_object_info() is too heavy-weight; it starts from the
sha1 and has to find the pack again (so not only does it waste
time, it might not even find the same instance).

In some cases, this problem is solved by helpers like
get_size_from_delta(), which is used by pack-objects to take
a shortcut for objects whose packed representation has
already been found. But there's no similar function for
getting the object type, for instance. Rather than introduce
one, let's just make the whole packed_object_info() available.
It is smart enough to spend effort only on the items the
caller wants.

Signed-off-by: Jeff King 
---
 cache.h | 1 +
 sha1_file.c | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/cache.h b/cache.h
index b2e4969..f23968e 100644
--- a/cache.h
+++ b/cache.h
@@ -1558,6 +1558,7 @@ struct object_info {
 #define OBJECT_INFO_INIT {NULL}
 
 extern int sha1_object_info_extended(const unsigned char *, struct object_info 
*, unsigned flags);
+extern int packed_object_info(struct packed_git *pack, off_t offset, struct 
object_info *);
 
 /* Dumb servers support */
 extern int update_server_info(int);
diff --git a/sha1_file.c b/sha1_file.c
index b8cc76b..5ca0ffa 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1970,8 +1970,8 @@ static enum object_type packed_to_object_type(struct 
packed_git *p,
goto out;
 }
 
-static int packed_object_info(struct packed_git *p, off_t obj_offset,
- struct object_info *oi)
+int packed_object_info(struct packed_git *p, off_t obj_offset,
+  struct object_info *oi)
 {
struct pack_window *w_curs = NULL;
unsigned long size;
-- 
2.9.2.790.gaa5bc72

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


[PATCH v5 1/4] provide an initializer for "struct object_info"

2016-08-11 Thread Jeff King
An all-zero initializer is fine for this struct, but because
the first element is a pointer, call sites need to know to
use "NULL" instead of "0". Otherwise some static checkers
like "sparse" will complain; see d099b71 (Fix some sparse
warnings, 2013-07-18) for example.  So let's provide an
initializer to make this easier to get right.

But let's also comment that memset() to zero is explicitly
OK[1]. One of the callers embeds object_info in another
struct which is initialized via memset (expand_data in
builtin/cat-file.c). Since our subset of C doesn't allow
assignment from a compound literal, handling this in any
other way is awkward, so we'd like to keep the ability to
initialize by memset(). By documenting this property, it
should make anybody who wants to change the initializer
think twice before doing so.

There's one other caller of interest. In parse_sha1_header(),
we did not initialize the struct fully in the first place.
This turned out not to be a bug because the sub-function it
calls does not look at any other fields except the ones we
did initialize. But that assumption might not hold in the
future, so it's a dangerous construct. This patch switches
it to initializing the whole struct, which protects us
against unexpected reads of the other fields.

[1] Obviously using memset() to initialize a pointer
violates the C standard, but we long ago decided that it
was an acceptable tradeoff in the real world.

Signed-off-by: Jeff King 
---
 builtin/cat-file.c | 5 ++---
 cache.h| 7 +++
 sha1_file.c| 6 ++
 streaming.c| 2 +-
 4 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 2dfe626..2cbc31e 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -28,7 +28,7 @@ static int cat_one_file(int opt, const char *exp_type, const 
char *obj_name,
char *buf;
unsigned long size;
struct object_context obj_context;
-   struct object_info oi = {NULL};
+   struct object_info oi = OBJECT_INFO_INIT;
struct strbuf sb = STRBUF_INIT;
unsigned flags = LOOKUP_REPLACE_OBJECT;
 
@@ -378,8 +378,7 @@ static int batch_objects(struct batch_options *opt)
data.mark_query = 0;
 
if (opt->all_objects) {
-   struct object_info empty;
-   memset(&empty, 0, sizeof(empty));
+   struct object_info empty = OBJECT_INFO_INIT;
if (!memcmp(&data.info, &empty, sizeof(empty)))
data.skip_object_info = 1;
}
diff --git a/cache.h b/cache.h
index 95a0bd3..b2e4969 100644
--- a/cache.h
+++ b/cache.h
@@ -1550,6 +1550,13 @@ struct object_info {
} packed;
} u;
 };
+
+/*
+ * Initializer for a "struct object_info" that wants no items. You may
+ * also memset() the memory to all-zeroes.
+ */
+#define OBJECT_INFO_INIT {NULL}
+
 extern int sha1_object_info_extended(const unsigned char *, struct object_info 
*, unsigned flags);
 
 /* Dumb servers support */
diff --git a/sha1_file.c b/sha1_file.c
index 02940f1..b8cc76b 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1730,11 +1730,9 @@ static int parse_sha1_header_extended(const char *hdr, 
struct object_info *oi,
 
 int parse_sha1_header(const char *hdr, unsigned long *sizep)
 {
-   struct object_info oi;
+   struct object_info oi = OBJECT_INFO_INIT;
 
oi.sizep = sizep;
-   oi.typename = NULL;
-   oi.typep = NULL;
return parse_sha1_header_extended(hdr, &oi, LOOKUP_REPLACE_OBJECT);
 }
 
@@ -2738,7 +2736,7 @@ int sha1_object_info_extended(const unsigned char *sha1, 
struct object_info *oi,
 int sha1_object_info(const unsigned char *sha1, unsigned long *sizep)
 {
enum object_type type;
-   struct object_info oi = {NULL};
+   struct object_info oi = OBJECT_INFO_INIT;
 
oi.typep = &type;
oi.sizep = sizep;
diff --git a/streaming.c b/streaming.c
index 811fcc2..431254b 100644
--- a/streaming.c
+++ b/streaming.c
@@ -135,7 +135,7 @@ struct git_istream *open_istream(const unsigned char *sha1,
 struct stream_filter *filter)
 {
struct git_istream *st;
-   struct object_info oi = {NULL};
+   struct object_info oi = OBJECT_INFO_INIT;
const unsigned char *real = lookup_replace_object(sha1);
enum input_source src = istream_source(real, type, &oi);
 
-- 
2.9.2.790.gaa5bc72

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


[PATCH v5] pack-objects mru

2016-08-11 Thread Jeff King
On Thu, Aug 11, 2016 at 02:57:51AM -0400, Jeff King wrote:

> > One thing to be careful of is that there are more things this
> > drop_reused_delta() should be doing. But I looked through the rest of
> > check_object() and could not find anything else.
> 
> Argh, I spoke too soon.
> 
> It is true that the size lookup is the only part of check_object()
> we skip if we are reusing the delta. But what I didn't notice is that
> when we choose to reuse a delta, we overwrite entry->type (the real
> type!) with the in_pack_type (OFS_DELTA, etc). We need to undo that so
> that later stages see the real type.
> 
> I'm not sure how my existing tests worked (I confirmed that they do
> indeed break the delta). It may be that only some code paths actually
> care about the real type. But when playing with the depth-limit (which
> uses the same drop_reused_delta() helper), I managed to make some pretty
> broken packs.
> 
> So please disregard the v4 patch I just sent; I haven't triggered it,
> but I imagine it has the same problem, and I just didn't manage to
> trigger it.
> 
> I'll clean that up and send out a new series.

Here it is. It ended up needing a few preparatory patches.

  [1/4]: provide an initializer for "struct object_info"
  [2/4]: sha1_file: make packed_object_info public
  [3/4]: pack-objects: break delta cycles before delta-search phase
  [4/4]: pack-objects: use mru list when iterating over packs

I had originally intended to include an extra patch to handle the
--depth limits better. But after writing it, I'm not sure it's actually
a good idea. I'll post it as an addendum with more discussion.

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


Re: [PATCH v11 28/40] builtin/apply: rename option parsing functions

2016-08-11 Thread stefan.naewe
Am 11.08.2016 um 10:52 schrieb Christian Couder:
> As these functions are going to be part of the libified
> apply api, let's give them a name that is more specific

s/api/API/

;-)

> to the apply API.
> 
> Signed-off-by: Christian Couder 
> ---
>  builtin/apply.c | 40 
>  1 file changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/builtin/apply.c b/builtin/apply.c
> index ad403f8..429fe44 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -4588,16 +4588,16 @@ static int apply_patch(struct apply_state *state,
>   return res;
>  }
>  
> -static int option_parse_exclude(const struct option *opt,
> - const char *arg, int unset)
> +static int apply_option_parse_exclude(const struct option *opt,
> +   const char *arg, int unset)
>  {
>   struct apply_state *state = opt->value;
>   add_name_limit(state, arg, 1);
>   return 0;
>  }
>  
> -static int option_parse_include(const struct option *opt,
> - const char *arg, int unset)
> +static int apply_option_parse_include(const struct option *opt,
> +   const char *arg, int unset)
>  {
>   struct apply_state *state = opt->value;
>   add_name_limit(state, arg, 0);
> @@ -4605,9 +4605,9 @@ static int option_parse_include(const struct option 
> *opt,
>   return 0;
>  }
>  
> -static int option_parse_p(const struct option *opt,
> -   const char *arg,
> -   int unset)
> +static int apply_option_parse_p(const struct option *opt,
> + const char *arg,
> + int unset)
>  {
>   struct apply_state *state = opt->value;
>   state->p_value = atoi(arg);
> @@ -4615,8 +4615,8 @@ static int option_parse_p(const struct option *opt,
>   return 0;
>  }
>  
> -static int option_parse_space_change(const struct option *opt,
> -  const char *arg, int unset)
> +static int apply_option_parse_space_change(const struct option *opt,
> +const char *arg, int unset)
>  {
>   struct apply_state *state = opt->value;
>   if (unset)
> @@ -4626,8 +4626,8 @@ static int option_parse_space_change(const struct 
> option *opt,
>   return 0;
>  }
>  
> -static int option_parse_whitespace(const struct option *opt,
> -const char *arg, int unset)
> +static int apply_option_parse_whitespace(const struct option *opt,
> +  const char *arg, int unset)
>  {
>   struct apply_state *state = opt->value;
>   state->whitespace_option = arg;
> @@ -4636,8 +4636,8 @@ static int option_parse_whitespace(const struct option 
> *opt,
>   return 0;
>  }
>  
> -static int option_parse_directory(const struct option *opt,
> -   const char *arg, int unset)
> +static int apply_option_parse_directory(const struct option *opt,
> + const char *arg, int unset)
>  {
>   struct apply_state *state = opt->value;
>   strbuf_reset(&state->root);
> @@ -4755,13 +4755,13 @@ int cmd_apply(int argc, const char **argv, const char 
> *prefix)
>   struct option builtin_apply_options[] = {
>   { OPTION_CALLBACK, 0, "exclude", &state, N_("path"),
>   N_("don't apply changes matching the given path"),
> - 0, option_parse_exclude },
> + 0, apply_option_parse_exclude },
>   { OPTION_CALLBACK, 0, "include", &state, N_("path"),
>   N_("apply changes matching the given path"),
> - 0, option_parse_include },
> + 0, apply_option_parse_include },
>   { OPTION_CALLBACK, 'p', NULL, &state, N_("num"),
>   N_("remove  leading slashes from traditional diff 
> paths"),
> - 0, option_parse_p },
> + 0, apply_option_parse_p },
>   OPT_BOOL(0, "no-add", &state.no_add,
>   N_("ignore additions made by the patch")),
>   OPT_BOOL(0, "stat", &state.diffstat,
> @@ -4793,13 +4793,13 @@ int cmd_apply(int argc, const char **argv, const char 
> *prefix)
>   N_("ensure at least  lines of context 
> match")),
>   { OPTION_CALLBACK, 0, "whitespace", &state, N_("action"),
>   N_("detect new or modified lines that have whitespace 
> errors"),
> - 0, option_parse_whitespace },
> + 0, apply_option_parse_whitespace },
>   { OPTION_CALLBACK, 0, "ignore-space-change", &state, NULL,
>   N_("ignore changes in whitespace when finding context"),
> - PARSE_OPT_NOARG, option_parse_space_change },
> + PARSE_OPT_NOARG, apply_option_pars

Re: [PATCH v10 01/40] apply: make some names more specific

2016-08-11 Thread stefan.naewe
Am 11.08.2016 um 10:40 schrieb Christian Couder:
> On Tue, Aug 9, 2016 at 4:51 PM,   wrote:
>> Am 08.08.2016 um 23:02 schrieb Christian Couder:
>>> To prepare for some structs and constants being moved from
>>> builtin/apply.c to apply.h, we should give them some more
>>> specific names to avoid possible name collisions in th global
>>
>> s/th/the/
>>
>>> namespace.
> 
> Thanks. I will send an update with only a new version of this patch
> and of 28/40.
> 

There's another lowercase 'api' in patch 40

Stefan
-- 

/dev/random says: Tis better to have loved and lost than just to have lost.
python -c "print 
'73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')"
 
GPG Key fingerprint = 2DF5 E01B 09C3 7501 BCA9  9666 829B 49C5 9221 27AF

Re: [PATCH v10 01/40] apply: make some names more specific

2016-08-11 Thread Christian Couder
On Tue, Aug 9, 2016 at 4:51 PM,   wrote:
> Am 08.08.2016 um 23:02 schrieb Christian Couder:
>> To prepare for some structs and constants being moved from
>> builtin/apply.c to apply.h, we should give them some more
>> specific names to avoid possible name collisions in th global
>
> s/th/the/
>
>> namespace.

Thanks. I will send an update with only a new version of this patch
and of 28/40.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


  1   2   >