[PATCH v7 2/7] graph: add support for --line-prefix on all graph-aware output

[Jacob Keller]

From: Jacob Keller 

Add an extension to git-diff and git-log (and any other graph-aware
displayable output) such that "--line-prefix=" will print the
additional line-prefix on every line of output.

To make this work, we have to fix a few bugs in the graph API that force
graph_show_commit_msg to be used only when you have a valid graph.
Additionally, we extend the default_diff_output_prefix handler to work
even when no graph is enabled.

This is somewhat of a hack on top of the graph API, but I think it
should be acceptable here.

This will be used by a future extension of submodule display which
displays the submodule diff as the actual diff between the pre and post
commit in the submodule project.

Add some tests for both git-log and git-diff to ensure that the prefix
is honored correctly.

Signed-off-by: Jacob Keller 
---
 Documentation/diff-options.txt |   3 +
 builtin/rev-list.c |  70 ++---
 diff.c |   7 +
 diff.h |   2 +
 graph.c| 104 ---
 graph.h|  22 +-
 log-tree.c |   5 +-
 t/t4013-diff-various.sh|   6 +
 ...diff.diff_--line-prefix=abc_master_master^_side |  29 ++
 t/t4013/diff.diff_--line-prefix_--cached_--_file0  |  15 +
 t/t4202-log.sh | 323 +
 11 files changed, 506 insertions(+), 80 deletions(-)
 create mode 100644 t/t4013/diff.diff_--line-prefix=abc_master_master^_side
 create mode 100644 t/t4013/diff.diff_--line-prefix_--cached_--_file0

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 705a87394200..cc4342e2034d 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.
 
+--line-prefix=::
+   Prepend an additional prefix to every line of output.
+
 For more detailed explanation on these common options, see also
 linkgit:gitdiffcore[7].
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 0ba82b1635b6..1a75a83538f4 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -122,48 +122,40 @@ static void show_commit(struct commit *commit, void *data)
ctx.fmt = revs->commit_format;
ctx.output_encoding = get_log_output_encoding();
pretty_print_commit(, commit, );
-   if (revs->graph) {
-   if (buf.len) {
-   if (revs->commit_format != CMIT_FMT_ONELINE)
-   graph_show_oneline(revs->graph);
+   if (buf.len) {
+   if (revs->commit_format != CMIT_FMT_ONELINE)
+   graph_show_oneline(revs->graph);
 
-   graph_show_commit_msg(revs->graph, );
+   graph_show_commit_msg(revs->graph, stdout, );
 
-   /*
-* Add a newline after the commit message.
-*
-* Usually, this newline produces a blank
-* padding line between entries, in which case
-* we need to add graph padding on this line.
-*
-* However, the commit message may not end in a
-* newline.  In this case the newline simply
-* ends the last line of the commit message,
-* and we don't need any graph output.  (This
-* always happens with CMIT_FMT_ONELINE, and it
-* happens with CMIT_FMT_USERFORMAT when the
-* format doesn't explicitly end in a newline.)
-*/
-   if (buf.len && buf.buf[buf.len - 1] == '\n')
-   graph_show_padding(revs->graph);
-   putchar('\n');
-   } else {
-   /*
-* If the message buffer is empty, just show
-* the rest of the graph output for this
-* commit.
-*/
-   if (graph_show_remainder(revs->graph))
-   putchar('\n');
-   if (revs->commit_format == CMIT_FMT_ONELINE)
-   putchar('\n');
-   }
+   /*
+   

[PATCH v7 4/7] submodule: allow do_submodule_path to work if given gitdir directly

[Jacob Keller]

From: Jacob Keller 

Currently, do_submodule_path relies on read_gitfile, which will die() if
it can't read from the specified gitfile. Unfortunately, this means that
do_submodule_path will not work when given the path to a submodule which
is checked out directly, such as a newly added submodule which you
cloned and then "git submodule add". Instead, replace the call with
resolve_gitdir. This first checks to see if we've been given a gitdir
already.

Because resolve_gitdir may return the same buffer it was passed, we have
to check for this case as well, since strbuf_reset() will not work as
expected here, and indeed is not necessary.

Signed-off-by: Jacob Keller 
---
 path.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/path.c b/path.c
index 17551c483476..d1af029152a2 100644
--- a/path.c
+++ b/path.c
@@ -477,8 +477,8 @@ static void do_submodule_path(struct strbuf *buf, const 
char *path,
strbuf_complete(buf, '/');
strbuf_addstr(buf, ".git");
 
-   git_dir = read_gitfile(buf->buf);
-   if (git_dir) {
+   git_dir = resolve_gitdir(buf->buf);
+   if (git_dir && git_dir != buf->buf) {
strbuf_reset(buf);
strbuf_addstr(buf, git_dir);
}
-- 
2.10.0.rc0.217.g609f9e8.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 5/7] submodule: correct output of submodule log format

[Jacob Keller]

From: Jacob Keller 

The submodule log diff output incorrectly states that the submodule is
"not checked out" in cases where it wants to say the submodule is "not
initialized". Change the wording to reflect the actual check being
performed.

Signed-off-by: Jacob Keller 
---
 submodule.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/submodule.c b/submodule.c
index 1b5cdfb7e784..e1a51b7506ff 100644
--- a/submodule.c
+++ b/submodule.c
@@ -348,7 +348,7 @@ void show_submodule_summary(FILE *f, const char *path,
if (is_null_sha1(two))
message = "(submodule deleted)";
else if (add_submodule_odb(path))
-   message = "(not checked out)";
+   message = "(not initialized)";
else if (is_null_sha1(one))
message = "(new submodule)";
else if (!(left = lookup_commit_reference(one)) ||
-- 
2.10.0.rc0.217.g609f9e8.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 3/7] diff: prepare for additional submodule formats

[Jacob Keller]

From: Jacob Keller 

A future patch will add a new format for displaying the difference of
a submodule. Make it easier by changing how we store the current
selected format. Replace the DIFF_OPT flag with an enumeration, as each
format will be mutually exclusive.

Signed-off-by: Jacob Keller 
---
 diff.c | 12 ++--
 diff.h |  7 ++-
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/diff.c b/diff.c
index e57cf39ad109..d6b321da3d1d 100644
--- a/diff.c
+++ b/diff.c
@@ -132,9 +132,9 @@ 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, "short"))
-   DIFF_OPT_CLR(options, SUBMODULE_LOG);
+   options->submodule_format = DIFF_SUBMODULE_SHORT;
else
return -1;
return 0;
@@ -2300,9 +2300,9 @@ 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))) {
+   if (o->submodule_format == DIFF_SUBMODULE_LOG &&
+   (!one->mode || S_ISGITLINK(one->mode)) &&
+   (!two->mode || S_ISGITLINK(two->mode))) {
const char *del = diff_get_color_opt(o, DIFF_FILE_OLD);
const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
show_submodule_summary(o->file, one->path ? one->path : 
two->path,
@@ -3916,7 +3916,7 @@ int diff_opt_parse(struct diff_options *options,
DIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);
handle_ignore_submodules_arg(options, arg);
} else if (!strcmp(arg, "--submodule"))
-   DIFF_OPT_SET(options, SUBMODULE_LOG);
+   options->submodule_format = DIFF_SUBMODULE_LOG;
else if (skip_prefix(arg, "--submodule=", ))
return parse_submodule_opt(options, arg);
else if (skip_prefix(arg, "--ws-error-highlight=", ))
diff --git a/diff.h b/diff.h
index 1f57aad25c71..ea5aba668eaa 100644
--- a/diff.h
+++ b/diff.h
@@ -83,7 +83,6 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct 
diff_options *opt, void *data)
 #define DIFF_OPT_DIRSTAT_BY_FILE (1 << 20)
 #define DIFF_OPT_ALLOW_TEXTCONV  (1 << 21)
 #define DIFF_OPT_DIFF_FROM_CONTENTS  (1 << 22)
-#define DIFF_OPT_SUBMODULE_LOG   (1 << 23)
 #define DIFF_OPT_DIRTY_SUBMODULES(1 << 24)
 #define DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES (1 << 25)
 #define DIFF_OPT_IGNORE_DIRTY_SUBMODULES (1 << 26)
@@ -110,6 +109,11 @@ enum diff_words_type {
DIFF_WORDS_COLOR
 };
 
+enum diff_submodule_format {
+   DIFF_SUBMODULE_SHORT = 0,
+   DIFF_SUBMODULE_LOG,
+};
+
 struct diff_options {
const char *orderfile;
const char *pickaxe;
@@ -157,6 +161,7 @@ struct diff_options {
int stat_count;
const char *word_regex;
enum diff_words_type word_diff;
+   enum diff_submodule_format submodule_format;
 
/* this is set by diffcore for DIFF_FORMAT_PATCH */
int found_changes;
-- 
2.10.0.rc0.217.g609f9e8.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/7] implement inline submodule diff format

[Jacob Keller]

From: Jacob Keller 

As suggested by Junio, I broke this patch into several pieces, and
made a common helper function for the submodule header. Note that there
are a couple of complicated modifications to the submodule header
portion which (should) still result in the same header but allow the
diff format a bit better control. It's a bit awkward but it does pass
all the current tests, and the format between the two should be similar.

I tried to break apart the patches properly, so I hope I didn't
accidentally munge them too badly.

Jacob Keller (6):
  graph: add support for --line-prefix on all graph-aware output
  diff: prepare for additional submodule formats
  submodule: allow do_submodule_path to work if given gitdir directly
  submodule: correct output of submodule log format
  submodule: refactor show_submodule_summary with helper function
  diff: teach diff to display submodule difference with an inline diff

Junio C Hamano (1):
  diff.c: remove output_prefix_length field

 Documentation/diff-config.txt  |   9 +-
 Documentation/diff-options.txt |  20 +-
 builtin/rev-list.c |  70 +-
 diff.c |  52 +-
 diff.h |  11 +-
 graph.c| 106 +--
 graph.h|  22 +-
 log-tree.c |   5 +-
 path.c |   4 +-
 submodule.c| 162 -
 submodule.h|   6 +
 t/t4013-diff-various.sh|   6 +
 ...diff.diff_--line-prefix=abc_master_master^_side |  29 +
 t/t4013/diff.diff_--line-prefix_--cached_--_file0  |  15 +
 t/t4059-diff-submodule-option-diff-format.sh   | 733 +
 t/t4202-log.sh | 323 +
 16 files changed, 1432 insertions(+), 141 deletions(-)
 create mode 100644 t/t4013/diff.diff_--line-prefix=abc_master_master^_side
 create mode 100644 t/t4013/diff.diff_--line-prefix_--cached_--_file0
 create mode 100755 t/t4059-diff-submodule-option-diff-format.sh

-- 
2.10.0.rc0.217.g609f9e8.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/7] diff.c: remove output_prefix_length field

[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 to subtract the display columns used for the
history graph part from the total "terminal width"; this matters
when the "git log --graph -p" option is in use.

The field must be set to the number of display columns needed to
show the output from the output_prefix() callback, 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 534c12e28ea8..50bef1f07658 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 7883729edf10..747a204d75a4 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();
graph_padding_line(graph, );
return 
@@ -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.10.0.rc0.217.g609f9e8.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 7/7] diff: teach diff to display submodule difference with an inline diff

[Jacob Keller]

From: Jacob Keller 

Teach git-diff and friends a new format for displaying the difference
of a submodule. The new format is an inline diff of the contents of the
submodule between the commit range of the update. This allows the user
to see the actual code change caused by a submodule update.

Add tests for the new format and option.

Signed-off-by: Jacob Keller 
---
 Documentation/diff-config.txt|   9 +-
 Documentation/diff-options.txt   |  17 +-
 diff.c   |  31 +-
 diff.h   |   1 +
 submodule.c  |  61 +++
 submodule.h  |   6 +
 t/t4059-diff-submodule-option-diff-format.sh | 733 +++
 7 files changed, 838 insertions(+), 20 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..0eded24034b5 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
@@ -122,10 +122,11 @@ 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
-   format just shows the names of the commits at the beginning
-   and end of the range.  Defaults to short.
+   shown.  The "short" format just shows the names of the commits
+   at the beginning and end of the range. The "log" format lists
+   the commits in the range like linkgit:git-submodule[1] `summary`
+   does. The "diff" format shows an inline diff of the changed
+   contents of the submodule. Defaults to "short".
 
 diff.wordRegex::
A POSIX Extended Regular Expression used to determine what is a "word"
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index cc4342e2034d..7805a0ccadf2 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -210,13 +210,16 @@ any of those replacements occurred.
of the `--diff-filter` option on what the status letters mean.
 
 --submodule[=]::
-   Specify how differences in submodules are shown.  When `--submodule`
-   or `--submodule=log` is given, the 'log' format is used.  This format 
lists
-   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.
+   Specify how differences in submodules are shown.  When specifying
+   `--submodule=short` the 'short' format is used.  This format just
+   shows the names of the commits at the beginning and end of the range.
+   When `--submodule` or `--submodule=log` is specified, the 'log'
+   format is used.  This format lists the commits in the range like
+   linkgit:git-submodule[1] `summary` does.  When `--submodule=diff`
+   is specified, the 'diff' format is used.  This format shows an
+   inline diff of the changes in the submodule contents between the
+   commit range.  Defaults to `diff.submodule` or the 'short' format
+   if the config option is unset.
 
 --color[=]::
Show colored diff.
diff --git a/diff.c b/diff.c
index d6b321da3d1d..e119758aba82 100644
--- a/diff.c
+++ b/diff.c
@@ -135,6 +135,8 @@ static int parse_submodule_params(struct diff_options 
*options, const char *valu
options->submodule_format = DIFF_SUBMODULE_LOG;
else if (!strcmp(value, "short"))
options->submodule_format = DIFF_SUBMODULE_SHORT;
+   else if (!strcmp(value, "diff"))
+   options->submodule_format = DIFF_SUBMODULE_INLINE_DIFF;
else
return -1;
return 0;
@@ -2300,6 +2302,15 @@ static void builtin_diff(const char *name_a,
struct strbuf header = STRBUF_INIT;
const char *line_prefix = diff_line_prefix(o);
 
+   diff_set_mnemonic_prefix(o, "a/", "b/");
+   if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
+   a_prefix = o->b_prefix;
+   b_prefix = o->a_prefix;
+   } else {
+   a_prefix = o->a_prefix;
+   b_prefix = o->b_prefix;
+   }
+
if (o->submodule_format == DIFF_SUBMODULE_LOG &&
(!one->mode || S_ISGITLINK(one->mode)) &&
(!two->mode || S_ISGITLINK(two->mode))) {
@@ -2311,6 +2322,17 @@ static void builtin_diff(const char *name_a,
two->dirty_submodule,
meta, del, add, reset);
return;
+   } else if (o->submodule_format == DIFF_SUBMODULE_INLINE_DIFF &&
+   

[PATCH v7 6/7] submodule: refactor show_submodule_summary with helper function

[Jacob Keller]

From: Jacob Keller 

A future patch is going to add a new submodule diff format which
displays an inline diff of the submodule changes. To make this easier,
and to ensure that both submodule diff formats use the same initial
header, factor out show_submodule_header() function which will print the
current submodule header line, and then leave the show_submodule_summary
function to lookup and print the submodule log format.

This does create one format change in that "(revision walker failed)"
will now be displayed on its own line rather than as part of the message
because we no longer perform this step directly in the header display
flow. However, this is a rare case and shouldn't impact much. In
addition, since we no longer need to run prepare_submodule_summary to
get the fast_backward and fast_forward variables, these have been
removed from that call, and the show_submodule_header() function does
its own mergebase lookup.

Signed-off-by: Jacob Keller 
---
 submodule.c | 103 +++-
 1 file changed, 74 insertions(+), 29 deletions(-)

diff --git a/submodule.c b/submodule.c
index e1a51b7506ff..e341ca7ffefd 100644
--- a/submodule.c
+++ b/submodule.c
@@ -277,8 +277,7 @@ void handle_ignore_submodules_arg(struct diff_options 
*diffopt,
 }
 
 static int prepare_submodule_summary(struct rev_info *rev, const char *path,
-   struct commit *left, struct commit *right,
-   int *fast_forward, int *fast_backward)
+   struct commit *left, struct commit *right)
 {
struct commit_list *merge_bases, *list;
 
@@ -290,12 +289,6 @@ static int prepare_submodule_summary(struct rev_info *rev, 
const char *path,
add_pending_object(rev, >object, path);
add_pending_object(rev, >object, path);
merge_bases = get_merge_bases(left, right);
-   if (merge_bases) {
-   if (merge_bases->item == left)
-   *fast_forward = 1;
-   else if (merge_bases->item == right)
-   *fast_backward = 1;
-   }
for (list = merge_bases; list; list = list->next) {
list->item->object.flags |= UNINTERESTING;
add_pending_object(rev, >item->object,
@@ -333,31 +326,23 @@ static void print_submodule_summary(struct rev_info *rev, 
FILE *f,
strbuf_release();
 }
 
-void show_submodule_summary(FILE *f, const char *path,
+/* Helper function to display the submodule header line prior to the full
+ * summary output. If it can locate the submodule objects directory it will
+ * attempt to lookup both the left and right commits and put them into the
+ * left and right pointers.
+ */
+static void show_submodule_header(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 *del, const char *add, const char *reset)
+   const char *reset,
+   struct commit **left, struct commit **right)
 {
-   struct rev_info rev;
-   struct commit *left = NULL, *right = NULL;
+   struct commit_list *merge_bases;
const char *message = NULL;
struct strbuf sb = STRBUF_INIT;
int fast_forward = 0, fast_backward = 0;
 
-   if (is_null_sha1(two))
-   message = "(submodule deleted)";
-   else if (add_submodule_odb(path))
-   message = "(not initialized)";
-   else if (is_null_sha1(one))
-   message = "(new submodule)";
-   else if (!(left = lookup_commit_reference(one)) ||
-!(right = lookup_commit_reference(two)))
-   message = "(commits not present)";
-   else if (prepare_submodule_summary(, path, left, right,
-  _forward, _backward))
-   message = "(revision walker failed)";
-
if (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
fprintf(f, "%sSubmodule %s contains untracked content\n",
line_prefix, path);
@@ -365,11 +350,46 @@ void show_submodule_summary(FILE *f, const char *path,
fprintf(f, "%sSubmodule %s contains modified content\n",
line_prefix, path);
 
+   if (is_null_sha1(one))
+   message = "(new submodule)";
+   else if (is_null_sha1(two))
+   message = "(submodule deleted)";
+
+   if (add_submodule_odb(path)) {
+   if (!message)
+   message = "(submodule not initialized)";
+   goto output_header;
+   }
+
+   /*
+* Attempt to lookup the commit references, and determine if this is
+* a fast forward or fast backwards update
+*/
+   *left = lookup_commit_reference(one);
+   *right = lookup_commit_reference(two);
+
+   /*
+* Warn about missing commits 

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

[Junio C Hamano]

Stefan Beller  writes:

>  and now with error handling of invalid options. 

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

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

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

 and now with error handling of invalid options. 

 Documentation/config.txt   |  12 +
 builtin/clone.c|  19 
 builtin/submodule--helper.c| 102 +
 t/t7408-submodule-reference.sh |  43 +
 4 files changed, 176 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index bc1c433..e2571ea 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2837,6 +2837,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`. Default is `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 0182665..404c5e8 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -947,6 +947,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(_config,
+   "submodule.alternateLocation=superproject");
+   string_list_append(_config,
+   "submodule.alternateErrorStrategy=die");
+   } else if (option_optional_reference.nr) {
+   string_list_append(_config,
+   "submodule.alternateLocation=superproject");
+   string_list_append(_config,
+   "submodule.alternateErrorStrategy=info");
+   }
+   }
+
init_db(option_template, INIT_DB_QUIET);
write_config(_config);
 
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 7096848..a1da3ea 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -472,6 +472,105 @@ static int clone_submodule(const char *path, const char 
*gitdir, const char *url
return run_command();
 }
 
+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 }
+
+static 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(, alt->base, namelen);
+
+   /*
+* If the alternate object store is another repository, try the
+* standard layout with .git/modules//objects
+*/
+   if 

Re: Working from different machines.

[Junio C Hamano]

Tobiah  writes:

> In other words, I want to work from different machines, and always
> sit down to the environment exactly as I last left it.

You have several options, but it depends on untold expectations you
have beyond "exactly as I last left it."  So the following is only
to show directions and possibilities without going into details.

For example, do you save all changes you made in your editor buffers
before you leave the 'desktop'?  If the answer is "no", then that is
not a problem Git-the-tool is interested in solving, and the only
solution I could think of is to use mechanism to share the session
between 'desktop' and 'home', using things like remote-desktop or
screen session.

If the answer is "yes", the next question is if you commit all the
changes you made before you leave the 'desktop'.  If the answer is
"no", again, that is not a problem Git-the-tool is interested in
solving, and the only solution I could think of (in addition to the
"share the session" above) is to use networked filesystem shared
between 'desktop' and 'home'.

If the answer is "yes", then you are in the problem space that
Git-the-tool is interested in solving.  Assuming that you have
network connection into 'desktop' from 'home', the solution would
involve making it the first thing to do when get home to run "git
fetch" on 'home' to get the latest state from the 'desktop', and run
"git push" on 'home' to push out the latest state to the 'desktop'
before you leave 'home'.  If your 'server' is for your sole use, and
if 'home' has network connection into 'server', then you could
instead rendezvous at 'server' by running "git push server" on
'desktop' (or 'home') to 'server' as the last thing before you leave
'desktop' (or 'home'), and running "git fetch server" on 'home' (or
'desktop') as the first thing before you start working on 'home' (or
'desktop').




--
To unsubscribe from this list: send the line "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: [PATCHv7 8/8] clone: recursive and reference option triggers submodule alternates

[Stefan Beller]

On Wed, Aug 17, 2016 at 3:12 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>>  Added a default for alternateErrorStrategy and hence fixed the null pointer
>>  for error_strategy in prepare_possible_alternates(),
>
> Looks much better.
>
>> +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.
>
> I am not seeing a code that handles 'no' and any other string that
> is not 'superproject' differently, though.
>
> I can see that "clone" has codepath that writes 'superproject' to
> the variable, but the only thing that seems to care what value the
> variable is set to checks "no".  When somebody sets the variable to
> "yes", shouldn't we at least say "Sorry, I do not understand" and
> preferrably stop before spreading potential damage?  We'd surely end
> up doing something that the user who set the value to "yes" did not
> expect.
>
> There is something still missing?
>

>> +static void prepare_possible_alternates(const char *sm_name,
>> + struct string_list *reference)
>> +{
...
>> + sas.submodule_name = sm_name;
>> + sas.reference = reference;
>> + if (!strcmp(error_strategy, "die"))
>> + sas.error_mode = SUBMODULE_ALTERNATE_ERROR_DIE;
>> + if (!strcmp(error_strategy, "info"))
>> + sas.error_mode = SUBMODULE_ALTERNATE_ERROR_INFO;

(see below first)
Here goes the same for alternateErrorStrategy

>> + if (!strcmp(sm_alternate, "superproject"))
>> + foreach_alt_odb(add_possible_reference_from_superproject, 
>> );

here is where we would add

else if (!strcmp(sm_alternate, "no")
; /* do nothing */
else
die(_("What's wrong with you?"));

Initially I did not add that as I considered it not relevant. But I
guess it helps as a typo checker as well and it is more comforting
if a wrong value yields an error. Also it is consistent with the rest of
options.

Thanks again,
Stefan
--
To unsubscribe from this list: send the line "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: [PATCHv7 8/8] clone: recursive and reference option triggers submodule alternates

[Junio C Hamano]

Stefan Beller  writes:

>  Added a default for alternateErrorStrategy and hence fixed the null pointer
>  for error_strategy in prepare_possible_alternates(),

Looks much better.

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

I am not seeing a code that handles 'no' and any other string that
is not 'superproject' differently, though.

I can see that "clone" has codepath that writes 'superproject' to
the variable, but the only thing that seems to care what value the
variable is set to checks "no".  When somebody sets the variable to
"yes", shouldn't we at least say "Sorry, I do not understand" and
preferrably stop before spreading potential damage?  We'd surely end
up doing something that the user who set the value to "yes" did not
expect.

There is something still missing?

> +submodule.alternateErrorStrategy
> + Specifies how to treat errors with the alternates for a submodule
> + as computed via `submodule.alternateLocation`. Possible values are
> + `ignore`, `info`, `die`. Default is `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 0182665..404c5e8 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -947,6 +947,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(_config,
> + "submodule.alternateLocation=superproject");
> + string_list_append(_config,
> + "submodule.alternateErrorStrategy=die");
> + } else if (option_optional_reference.nr) {
> + string_list_append(_config,
> + "submodule.alternateLocation=superproject");
> + string_list_append(_config,
> + "submodule.alternateErrorStrategy=info");
> + }
> + }
> +
>   init_db(option_template, INIT_DB_QUIET);
>   write_config(_config);
>  
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 7096848..f8f35c1 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -472,6 +472,96 @@ static int clone_submodule(const char *path, const char 
> *gitdir, const char *url
>   return run_command();
>  }
>  
> +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 }
> +
> +static 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(, 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_alternate;
> + struct strbuf sb = STRBUF_INIT;
> + struct strbuf err = STRBUF_INIT;
> + strbuf_add(, name.buf, name.len - strlen("objects"));
> + /*
> +  * We need to end the new path with '/' to mark it as a dir,
> +  * otherwise a submodule name containing '/' will be broken
> +  * as the last part of a missing submodule reference would
> +  * be taken as a file name.
> +  */
> + strbuf_addf(, "modules/%s/", sas->submodule_name);
> +
> + sm_alternate = compute_alternate_path(sb.buf, );
> + if (sm_alternate) {
> + 

Working from different machines.

[Tobiah]

Right now I have a server at work where I keep a bare repository as
a source and backup for projects.  So I clone a project from there
to my desktop, and work, making a few branches as I try out ideas
for new features.

Then I go home, and I want to work as though I was sitting at my
desktop.  If I clone the committed work, I don't get all my branches.
How can I work so that I now easily have all my branches, then after
I work at home, when I go back to my desktop, the branches now reflect
whatever state I last left them in?

In other words, I want to work from different machines, and always
sit down to the environment exactly as I last left it.

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

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

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

 Added a default for alternateErrorStrategy and hence fixed the null pointer
 for error_strategy in prepare_possible_alternates(),
 
 Thanks,
 Stefan

 Documentation/config.txt   | 12 ++
 builtin/clone.c| 19 +
 builtin/submodule--helper.c| 93 ++
 t/t7408-submodule-reference.sh | 43 +++
 4 files changed, 167 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index bc1c433..e2571ea 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2837,6 +2837,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`. Default is `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 0182665..404c5e8 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -947,6 +947,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(_config,
+   "submodule.alternateLocation=superproject");
+   string_list_append(_config,
+   "submodule.alternateErrorStrategy=die");
+   } else if (option_optional_reference.nr) {
+   string_list_append(_config,
+   "submodule.alternateLocation=superproject");
+   string_list_append(_config,
+   "submodule.alternateErrorStrategy=info");
+   }
+   }
+
init_db(option_template, INIT_DB_QUIET);
write_config(_config);
 
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 7096848..f8f35c1 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -472,6 +472,96 @@ static int clone_submodule(const char *path, const char 
*gitdir, const char *url
return run_command();
 }
 
+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 }
+
+static 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(, alt->base, namelen);
+
+   /*
+* If the alternate object store is another repository, 

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

[Stefan Beller]

On Wed, Aug 17, 2016 at 2:31 PM, Junio C Hamano  wrote:
> Junio C Hamano  writes:
>
>> Stefan Beller  writes:
>>
>>> +static void prepare_possible_alternates(const char *sm_name,
>>> +struct string_list *reference)
>>> +{
>>> +char *sm_alternate = NULL, *error_strategy = NULL;
>>> +struct submodule_alternate_setup sas = SUBMODULE_ALTERNATE_SETUP_INIT;
>>> +
>>> +git_config_get_string("submodule.alternateLocation", _alternate);
>>> +if (!sm_alternate)
>>> +return;
>>> +
>>> +git_config_get_string("submodule.alternateErrorStrategy", 
>>> _strategy);
>>
>> I have to admit that I need to follow the codepath in config.c every
>> time to check, but I _think_ git_config_get_string() gives you your
>> own copy of the value.  As this function does not give ownership of
>> sm_alternate or error_strategy to something else, they are leaked
>> every time this function is called, I think.
>>
>>> +sas.submodule_name = sm_name;
>>> +sas.reference = reference;
>>> +if (!strcmp(error_strategy, "die"))
>>> +sas.error_mode = SUBMODULE_ALTERNATE_ERROR_DIE;
>
> Another thing I noticed but forgot to mention.  Can error_strategy
> be NULL here?  We are assuming sm_alternate can be, so I presume
> that it is sensible to protect against dereferencing a NULL here,
> too?

Oh!

Of course we need to fix that too. That slipped in because I assumed that those
two variables go in tandem (if sm_alternate is set, then
error_strategy is set as well),
but that is not the case of course.

Which leads to another thing: we need to have a default for error_strategy.
I think "die" is reasonable here. That also needs a documentation update.

>
>>> +if (!strcmp(error_strategy, "info"))
>>> +sas.error_mode = SUBMODULE_ALTERNATE_ERROR_INFO;
>>> +if (!strcmp(sm_alternate, "superproject"))
>>> +foreach_alt_odb(add_possible_reference_from_superproject, 
>>> );
>>> +}
--
To unsubscribe from this list: send the line "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: [PATCHv4 8/8] clone: recursive and reference option triggers submodule alternates

[Junio C Hamano]

Junio C Hamano  writes:

> Stefan Beller  writes:
>
>> +static void prepare_possible_alternates(const char *sm_name,
>> +struct string_list *reference)
>> +{
>> +char *sm_alternate = NULL, *error_strategy = NULL;
>> +struct submodule_alternate_setup sas = SUBMODULE_ALTERNATE_SETUP_INIT;
>> +
>> +git_config_get_string("submodule.alternateLocation", _alternate);
>> +if (!sm_alternate)
>> +return;
>> +
>> +git_config_get_string("submodule.alternateErrorStrategy", 
>> _strategy);
>
> I have to admit that I need to follow the codepath in config.c every
> time to check, but I _think_ git_config_get_string() gives you your
> own copy of the value.  As this function does not give ownership of
> sm_alternate or error_strategy to something else, they are leaked
> every time this function is called, I think.
>
>> +sas.submodule_name = sm_name;
>> +sas.reference = reference;
>> +if (!strcmp(error_strategy, "die"))
>> +sas.error_mode = SUBMODULE_ALTERNATE_ERROR_DIE;

Another thing I noticed but forgot to mention.  Can error_strategy
be NULL here?  We are assuming sm_alternate can be, so I presume
that it is sensible to protect against dereferencing a NULL here,
too?

>> +if (!strcmp(error_strategy, "info"))
>> +sas.error_mode = SUBMODULE_ALTERNATE_ERROR_INFO;
>> +if (!strcmp(sm_alternate, "superproject"))
>> +foreach_alt_odb(add_possible_reference_from_superproject, );
>> +}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

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

  This replaces the tip of remotes/origin/sb/submodule-clone-rr.
  The difference to the queued version are 2 free calls at the end of
  prepare_possible_alternates()
  
  Thanks,
  Stefan

 Documentation/config.txt   | 12 ++
 builtin/clone.c| 19 +
 builtin/submodule--helper.c| 90 ++
 t/t7408-submodule-reference.sh | 43 
 4 files changed, 164 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index bc1c433..602f43a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2837,6 +2837,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 0182665..404c5e8 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -947,6 +947,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(_config,
+   "submodule.alternateLocation=superproject");
+   string_list_append(_config,
+   "submodule.alternateErrorStrategy=die");
+   } else if (option_optional_reference.nr) {
+   string_list_append(_config,
+   "submodule.alternateLocation=superproject");
+   string_list_append(_config,
+   "submodule.alternateErrorStrategy=info");
+   }
+   }
+
init_db(option_template, INIT_DB_QUIET);
write_config(_config);
 
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 7096848..fd571bf 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -472,6 +472,93 @@ static int clone_submodule(const char *path, const char 
*gitdir, const char *url
return run_command();
 }
 
+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 }
+
+static 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(, alt->base, namelen);
+
+   /*
+* If the alternate object store 

Re: git-mergetool reverse file ordering

[David Aguilar]

Hi Luis and Hannes,

On Wed, Aug 17, 2016 at 09:35:56AM +0200, Johannes Sixt wrote:
> Am 17.08.2016 um 08:46 schrieb David Aguilar:
> > The only thing that using diff-files doesn't address is the
> > rerere support in mergetool where it processes the files in
> > the order specified by "git rerere remaining".  This is why I
> > initially thought we needed a generic sort-like command.
> 
> I see. This is actually an important code path. How about this code
> structure:
> 
> if test $# -eq 0
> then
>   cd_to_toplevel
> 
>   if test -e "$GIT_DIR/MERGE_RR"
>   then
>   set -- $(git rerere remaining)
>   fi
> fi
> files=$(git diff-files --name-only --diff-filter=U -- "$@")
> 

Beautiful.

> This does not require an enhancement of rerere-remaining and still captures
> all three cases that currently go through separate branches. (Throw in some
> version of --ignore-submodules= if necessary, but I guess it is not.)
> 
> We do have a problem if there are file names with spaces, but it is not a
> new problem.

Thanks for the heads-up about file names with spaces.  We set,

IFS='
'

in git-mergetool--lib.sh so file names with spaces should be ok.
Naturally, we won't be able to support paths with embedded
newlines, but that's not a new problem ;-)

We should probably also set core.quotePath=false when calling
diff-files so that git doesn't try to quote "unusual" paths,
e.g.

git -c core.quotePath=false diff-files ...

Lastly, for anyone that's curious, I was wondering why we were
passing "-u" to "sort", and why we won't need to use "uniq" in
the new version.

The reason is that "ls-files -u" lists the different index
stages separately, and thus it reports duplicate paths.

"diff-files" with "--diff-filter=U" does not do that, so that's
another benefit to be gained from this change.

I think we've touched all the bases now.

Luis, I hope that makes sense.  Let us know if any of this is
unclear.


ciao,
-- 
David
--
To unsubscribe from this list: send the line "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] push: change submodule default to check

[Stefan Beller]

On Wed, Aug 17, 2016 at 2:05 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> If working with submodules, there are more reports of missing submodules
>> rather than the desire to push the superproject without the submodules
>> to be pushed out.
>
> I do not know how you are counting the "more reports" part of that
> assertion, but it is very likely that it is biased by the current
> default.  If you flip the default, you would see more reports that
> say "I know I wasn't ready to push the submodule part out; don't bug
> me".
>
> IOW, those who want to have something different always sound louder,
> because people who are satisified tend to stay silent.

Yeah I thought about this mistake and how to get real numbers, but
I was just misled by some people on #git IRC waving hands. ;)

>
>> Flipping the default to check for submodules is safer
>> than the current default of ignoring submodules while pushing.
>
> That part of the assertion, on the other hand, is justifiable.

ok.

>
>> Signed-off-by: Stefan Beller 
>> ---
>>
>>  Probably too late for the 2.10 release as I'd propose to keep it in master 
>> for
>>  quite a while before actually doing a release with this.
>
> I think you meant 'next' not 'master'.  We have a few "let's keep it
> in 'next' to see if people scream" topics there already--the more
> the merrier? ;-)

Well we put it into next, but that are not as many people as those
running master I would think, so I would want to maximize both times
in next as well as in master, e.g. if you put it into next today (unreasonable,
but let's assume), then it could make it into master next week, and then be
released as part of 2.10 IIUC the release schedule.

I would say that is too fast. rather I'd see this patch transition from next to
master just after a release, such that it lives in master for a full
release cycle
before being actually in a release.

So far for my line of thinking.

Thanks,
Stefan
--
To unsubscribe from this list: send the line "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] push: change submodule default to check

[Junio C Hamano]

Stefan Beller  writes:

> If working with submodules, there are more reports of missing submodules
> rather than the desire to push the superproject without the submodules
> to be pushed out.

I do not know how you are counting the "more reports" part of that
assertion, but it is very likely that it is biased by the current
default.  If you flip the default, you would see more reports that
say "I know I wasn't ready to push the submodule part out; don't bug
me".

IOW, those who want to have something different always sound louder,
because people who are satisified tend to stay silent.

> Flipping the default to check for submodules is safer
> than the current default of ignoring submodules while pushing.

That part of the assertion, on the other hand, is justifiable.

> Signed-off-by: Stefan Beller 
> ---
>  
>  Probably too late for the 2.10 release as I'd propose to keep it in master 
> for
>  quite a while before actually doing a release with this.

I think you meant 'next' not 'master'.  We have a few "let's keep it
in 'next' to see if people scream" topics there already--the more
the merrier? ;-)
--
To unsubscribe from this list: send the line "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: [PATCHv4 8/8] clone: recursive and reference option triggers submodule alternates

[Stefan Beller]

On Wed, Aug 17, 2016 at 1:02 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> +static void prepare_possible_alternates(const char *sm_name,
>> + struct string_list *reference)
>> +{
>> + char *sm_alternate = NULL, *error_strategy = NULL;
>> + struct submodule_alternate_setup sas = SUBMODULE_ALTERNATE_SETUP_INIT;
>> +
>> + git_config_get_string("submodule.alternateLocation", _alternate);
>> + if (!sm_alternate)
>> + return;
>> +
>> + git_config_get_string("submodule.alternateErrorStrategy", 
>> _strategy);
>
> I have to admit that I need to follow the codepath in config.c every
> time to check, but I _think_ git_config_get_string() gives you your
> own copy of the value.  As this function does not give ownership of
> sm_alternate or error_strategy to something else, they are leaked
> every time this function is called, I think.

There are quite a few more occurrences of git_config_get_string
in the submodule helper, I'll also look at those.

Thanks,
Stefan
--
To unsubscribe from this list: send the line "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] push: change submodule default to check

[Stefan Beller]

When working with submodules, it is easy to forget to push the submodules.
Change the default to 'check' if any existing submodule is present on at
least one remote of the submodule remotes.

This doesn't affect you if you do not work with submodules.
If working with submodules, there are more reports of missing submodules
rather than the desire to push the superproject without the submodules
to be pushed out. Flipping the default to check for submodules is safer
than the current default of ignoring submodules while pushing.

Signed-off-by: Stefan Beller 
---
 
 Probably too late for the 2.10 release as I'd propose to keep it in master for
 quite a while before actually doing a release with this.
 
 Thanks,
 Stefan

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

diff --git a/builtin/push.c b/builtin/push.c
index 3bb9d6b..479150a 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -22,7 +22,7 @@ static int deleterefs;
 static const char *receivepack;
 static int verbosity;
 static int progress = -1;
-static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
+static int recurse_submodules = RECURSE_SUBMODULES_CHECK;
 static enum transport_family family;
 
 static struct push_cas_option cas;
-- 
2.9.2.730.g525ad04.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: [PATCHv4 8/8] clone: recursive and reference option triggers submodule alternates

[Junio C Hamano]

Stefan Beller  writes:

> +static void prepare_possible_alternates(const char *sm_name,
> + struct string_list *reference)
> +{
> + char *sm_alternate = NULL, *error_strategy = NULL;
> + struct submodule_alternate_setup sas = SUBMODULE_ALTERNATE_SETUP_INIT;
> +
> + git_config_get_string("submodule.alternateLocation", _alternate);
> + if (!sm_alternate)
> + return;
> +
> + git_config_get_string("submodule.alternateErrorStrategy", 
> _strategy);

I have to admit that I need to follow the codepath in config.c every
time to check, but I _think_ git_config_get_string() gives you your
own copy of the value.  As this function does not give ownership of
sm_alternate or error_strategy to something else, they are leaked
every time this function is called, I think.

> + sas.submodule_name = sm_name;
> + sas.reference = reference;
> + if (!strcmp(error_strategy, "die"))
> + sas.error_mode = SUBMODULE_ALTERNATE_ERROR_DIE;
> + if (!strcmp(error_strategy, "info"))
> + sas.error_mode = SUBMODULE_ALTERNATE_ERROR_INFO;
> + if (!strcmp(sm_alternate, "superproject"))
> + foreach_alt_odb(add_possible_reference_from_superproject, );
> +}
--
To unsubscribe from this list: send the line "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] imap-send: Tell cURL to use imap:// or imaps://

[Anders Kaseorg]

Right now the imap:// or imaps:// part of imap.host is not being
passed on to cURL.  Perhaps it was able to guess correctly under some
circumstances, but I was not able to find one; it was just trying to
make HTTP requests for me.  It’s better to be explicit in any case.

Signed-off-by: Anders Kaseorg 
---
 imap-send.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/imap-send.c b/imap-send.c
index 938c691..7dd5acf 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1410,6 +1410,7 @@ static CURL *setup_curl(struct imap_server_conf *srvc)
curl_easy_setopt(curl, CURLOPT_USERNAME, server.user);
curl_easy_setopt(curl, CURLOPT_PASSWORD, server.pass);
 
+   strbuf_addstr(, server.use_ssl ? "imaps://" : "imap://");
strbuf_addstr(, server.host);
if (!path.len || path.buf[path.len - 1] != '/')
strbuf_addch(, '/');
-- 
2.10.0.rc0

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

Re: [PATCH v2 2/3] diff-highlight: add failing test for handling --graph output.

[Junio C Hamano]

Brian Henderson  writes:

> Signed-off-by: Brian Henderson 
> ---
>  contrib/diff-highlight/t/t9400-diff-highlight.sh | 13 +++
>  contrib/diff-highlight/t/test-diff-highlight.sh  | 43 
> 
>  2 files changed, 56 insertions(+)
>
> diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh 
> b/contrib/diff-highlight/t/t9400-diff-highlight.sh
> index 8eff178..39707c6 100755
> --- a/contrib/diff-highlight/t/t9400-diff-highlight.sh
> +++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
> @@ -59,4 +59,17 @@ test_expect_success 'diff-highlight does not highlight 
> mismatched hunk size' '
>  
>  # TODO add multi-byte test
>  
> +test_expect_success 'diff-highlight highlights the beginning of a line' '
> + dh_graph_test \
> + "aaa\nbbb\nccc\n" \
> + "aaa\n0bb\nccc\n" \
> + "aaa\nb0b\nccc\n" \
> +"
> + aaa
> +-${CW}b${CR}bb
> ++${CW}0${CR}bb
> + ccc
> +"
> +'

Is this expected to pass after applying 1/3 and 2/3?  The title says
"add faililng test", so I am assuming this is expected to fail, in
which case the test should start out as "test_expect_failure".  A
later patch that makes it pass should turn "test_expect_failure"
into "test_expect_success".

>  test_done
> diff --git a/contrib/diff-highlight/t/test-diff-highlight.sh 
> b/contrib/diff-highlight/t/test-diff-highlight.sh
> index 38323e8..67f742c 100644
> --- a/contrib/diff-highlight/t/test-diff-highlight.sh
> +++ b/contrib/diff-highlight/t/test-diff-highlight.sh
> @@ -64,6 +64,49 @@ dh_commit_test() {
>   test_cmp commit.exp commit.act
>  }
>  
> +dh_graph_test() {
> + a="$1" b="$2" c="$3"
> +
> + {
> + printf "$a" >file
> + ...
> + git merge master
> + git checkout master
> + git merge branch --no-ff
> + } >/dev/null 2>&1
> +
> + git log -p --graph --no-merges >graph.raw

Hmph, when does it make sense to have "--no-merges" together with
"--graph".  Doesn't it result in a disconnected mess?  Is that an
interesting and most often used case?

> +
> + # git log --graph orders the commits different than git log so we hack 
> it by
> + # using sed to remove the graph part.

Would it help if you knew "log --topo-order" to remove the "hack"?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/3] diff-highlight: add some tests.

[Junio C Hamano]

Brian Henderson  writes:

> Signed-off-by: Brian Henderson 
> ---
>  contrib/diff-highlight/Makefile  |  5 ++
>  contrib/diff-highlight/t/Makefile| 22 
>  contrib/diff-highlight/t/t9400-diff-highlight.sh | 62 +
>  contrib/diff-highlight/t/test-diff-highlight.sh  | 69 
> 
>  4 files changed, 158 insertions(+)
>  create mode 100644 contrib/diff-highlight/Makefile
>  create mode 100644 contrib/diff-highlight/t/Makefile
>  create mode 100755 contrib/diff-highlight/t/t9400-diff-highlight.sh
>  create mode 100644 contrib/diff-highlight/t/test-diff-highlight.sh

I am not sure test-diff-highlight.sh should be there; the function
definitions would still be useful but move them to the beginning of
t9400-diff-highlight.sh perhaps?

> diff --git a/contrib/diff-highlight/Makefile b/contrib/diff-highlight/Makefile
> new file mode 100644
> index 000..b866259
> --- /dev/null
> +++ b/contrib/diff-highlight/Makefile
> @@ -0,0 +1,5 @@
> +# nothing to build
> +all:;

Drop ';'.

> diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh 
> b/contrib/diff-highlight/t/t9400-diff-highlight.sh
> new file mode 100755
> index 000..8eff178
> --- /dev/null
> +++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
> @@ -0,0 +1,62 @@
> +#!/bin/sh
> +
> +test_description='Test diff-highlight'
> +
> +. ./test-diff-highlight.sh
> +. "$TEST_DIRECTORY"/test-lib.sh
> +
> +# PERL is required, but assumed to be present, although not necessarily 
> modern
> +# some tests require 5.8
> +test_expect_success PERL 'name' 'true'

If the platform lacks PERL prerequisite, this will simply be
skipped, and if the platform has it, it will always succeed.

I am not sure what you are trying to achieve by having this line
here.

> +test_expect_success 'diff-highlight does not highlight whole line' '
> + dh_test \
> + "aaa\nbbb\nccc\n" \
> + "aaa\n000\nccc\n"
> +'

Hmm, does this express the desired outcome, or just document the
current (possibly broken--I dunno) behaviour?  The same question for
the next one.

> +test_expect_success 'diff-highlight does not highlight mismatched hunk size' 
> '
> + dh_test \
> + "aaa\nbbb\n" \
> + "aaa\nb0b\nccc\n"
> +'

> +dh_test() {

Style: "dh_test () {"

The other functions in this file share the same.

> + dh_diff_test "$@" &&
> + dh_commit_test "$@"
> +}
> +
> +dh_diff_test() {
> + a="$1" b="$2"
> +
> + printf "$a" >file
> + git add file
> +
> + printf "$b" >file
> + git diff file >diff.raw
> +
> + if test $# -eq 3
> + then
> + # remove last newline
> + head -n5 diff.raw | test_chomp_eof >diff.exp

A reader can see "remove last newline" by seeing test_chomp_eof and
what it does without a comment, but it is totally unclear why you
need to remove.  The comment that says what it does without saying
why it does it is useless.

> + printf "$3" >>diff.exp
> + else
> + cat diff.raw >diff.exp
> + fi
> +
> + diff.act &&

Even though this is technically kosher, I do not see a merit of
deviating from the common practice of starting a line with the
command, i.e.

"$DIFF_HIGHLIGHT" diff.actual

would be much easier to read.

> + test -s diff.act &&

Why?  If you always have the expected output that you are going to
compare with, wouldn't that sufficient to do that test without this?
Besides, having "test -s" means that you can never make sure that a
certain pair of input does not show any changes.  Perhaps drop it?

> + diff diff.exp diff.act

Use test_cmp unless there is a strong reason why you shouldn't?

> +}
> +
> +dh_commit_test() {
> + a="$1" b="$2"
> +
> + printf "$a" >file
> + git add file
> + git commit -m"Add a file" >/dev/null

Avoid sticking a short-option to its argument, i.e.

git commit -m "Add a file"

> +
> + printf "$b" >file
> + git commit -am"Update a file" >/dev/null

Likewise.

git commit -a -m "Update a file"

The remainder of the file invites the same set of questions and
comments you see for dh_diff_test() above, so I won't repeat them.

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: diff --diff-filter on modified but locally deleted files

[Robert Dailey]

On Wed, Aug 17, 2016 at 1:34 PM, Junio C Hamano  wrote:
> Robert Dailey  writes:
>
>> My use case is that I do a merge from branch A to branch B. Branch A
>> modified a file which is already deleted on B some time before the
>> merge.
>>
>> When I do a `git status -sb`, these locally deleted but remotely
>> modified files show up as "DU".
>>
>> I want to invoke git status or diff (or something else) to get a list
>> of these specific conflicts (locally deleted, remotely modified).
>
> As far as "git diff [--cached]" (which is "compare HEAD with the
> working tree through index") is concerned, these paths are in the
> "U"nmerged category, so you'd give "U" to diff-filter to view them.
> Of course, that would give you other kinds of unmerged entries.
>
> If you know they show up as DU, why not "grep DU" in that output?

I'm happy to do that and in fact considered it, but since
--diff-filter exists, I figured it might solve the problem. I also
need a better understanding of how diff-filter works since it
technically has 2 sides during a merge conflict. I guess it functions
only on the right (local) side.

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

Re: [PATCH] git-multimail: update to release 1.4.0

[Junio C Hamano]

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

Re: diff --diff-filter on modified but locally deleted files

[Junio C Hamano]

Robert Dailey  writes:

> My use case is that I do a merge from branch A to branch B. Branch A
> modified a file which is already deleted on B some time before the
> merge.
>
> When I do a `git status -sb`, these locally deleted but remotely
> modified files show up as "DU".
>
> I want to invoke git status or diff (or something else) to get a list
> of these specific conflicts (locally deleted, remotely modified).

As far as "git diff [--cached]" (which is "compare HEAD with the
working tree through index") is concerned, these paths are in the
"U"nmerged category, so you'd give "U" to diff-filter to view them.
Of course, that would give you other kinds of unmerged entries.

If you know they show up as DU, why not "grep DU" in that output?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [ANNOUNCE] Git v2.10.0-rc0

[Torsten Bögershausen]

On 17.08.16 19:38, Junio C Hamano wrote:
> Junio C Hamano  writes:
> 
>> Torsten Bögershausen  writes:
>>
>>> Is this `text=auto` correct ?
>>
>> Thanks for spotting a typo.  Definitely.
>>
>>> I think it should be
>>>
>>>used to have the same effect as
>>>$ echo "* text eol=crlf" >.gitattributes
>>
>> Thanks.
>>
>>> # In other words, the `auto` was ignored, as explained here:
>>> +   $ git config core.eol crlf
>>> +   i.e. declaring all files are text; the combination now is
>>> +   equivalent to doing
>>> +   $ git config core.autocrlf true
>>> +
> 
> Just to make sure I got you right, how does this look?
> 
>  Documentation/RelNotes/2.10.0.txt | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/RelNotes/2.10.0.txt 
> b/Documentation/RelNotes/2.10.0.txt
> index 179e575..ccf5f64 100644
> --- a/Documentation/RelNotes/2.10.0.txt
> +++ b/Documentation/RelNotes/2.10.0.txt
> @@ -235,13 +235,13 @@ Performance, Internal Implementation, Development 
> Support etc.
>   * The API to iterate over all the refs (i.e. for_each_ref(), etc.)
> has been revamped.
>  
> - * The handling of the "text = auto" attribute has been updated.
> + * The handling of the "text=auto" attribute has been corrected.
> $ echo "* text=auto eol=crlf" >.gitattributes
> used to have the same effect as
> -   $ echo "* text=auto eol=crlf" >.gitattributes
> +   $ echo "* text eol=crlf" >.gitattributes
# Now we describe/define  the eol at checkouts twice.
# I think we can drop this line:
> $ git config core.eol crlf
# The rest looks good:
> -   i.e. declaring all files are text; the combination now is
> -   equivalent to doing
> +   i.e. declaring all files are text (ignoring "auto").  The
> +   combination has been fixed to be equivalent to doing
> $ git config core.autocrlf true
>  
>   * A few tests that specifically target "git rebase -i" have been
> 

# Just to be sure: the whole paragraph may look like this:

* The handling of the "text=auto" attribute has been corrected.
  $ echo "* text=auto eol=crlf" >.gitattributes
  used to have the same effect as
  $ echo "* text eol=crlf" >.gitattributes
  i.e. declaring all files are text (ignoring "auto").  The
  combination has been fixed to be equivalent to doing
  $ git config core.autocrlf true
















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

diff --diff-filter on modified but locally deleted files

[Robert Dailey]

My use case is that I do a merge from branch A to branch B. Branch A
modified a file which is already deleted on B some time before the
merge.

When I do a `git status -sb`, these locally deleted but remotely
modified files show up as "DU".

I want to invoke git status or diff (or something else) to get a list
of these specific conflicts (locally deleted, remotely modified). I
tried this:

$ git diff --diff-filter=D --name-status

This gave me no results. I also tried adding --cached, didn't make a
difference. If I just do this:

$ git diff --name-status

This lists those files as just U instead of DU. So I'm not sure where
the "D" is coming from in this case.

Is there a way I can get a list of these specific conflicts? I'd like
to rely on working/index status for this if possible, since as I
resolve these conflicts I want the list to shorten and only show
unresolved conflicts of this type. Which rules out a diff range
between B...A that I could do.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] mingw: ensure temporary file handles are not inherited by child processes

[Junio C Hamano]

Johannes Schindelin  writes:

> From: Ben Wijen 
>
> When the index is locked and child processes inherit the handle to
> said lock and the parent process wants to remove the lock before the
> child process exits, on Windows there is a problem: it won't work
> because files cannot be deleted if a process holds a handle on them.
> The symptom:
>
> Rename from 'xxx/.git/index.lock' to 'xxx/.git/index' failed.
> Should I try again? (y/n)
>
> Spawning child processes with bInheritHandles==FALSE would not work
> because no file handles would be inherited, not even the hStdXxx
> handles in STARTUPINFO (stdin/stdout/stderr).
>
> Opening every file with O_NOINHERIT does not work, either, as e.g.
> git-upload-pack expects inherited file handles.
>
> This leaves us with the only way out: creating temp files with the
> O_NOINHERIT flag. This flag is Windows-specific, however. For our
> purposes, it is equivalent our purposes) to O_CLOEXEC (which does not
> exist on Windows), so let's just open temporary files with the
> O_CLOEXEC flag and map that flag to O_NOINHERIT on Windows.

The callchain that leads to create_tempfile() eventually goes up to
hold_lock_file_for_update() and its _timeout() variant in the
current codebase.  There is no other create_tempfile() caller.

I think all the callers of these public entry points use them to
open a lockfile for its own use, and never delegate the writing to
it to their children, so this change is safe right now, and I do not
think it is too bad that a new caller that wants to do that have to
explicitly drop O_CLOEXEC (perhaps by dup(2)ing).

But it deserves mention in the lockfile and the tempfile API docs in
 and  that the file descriptor returned from
these public entry points does not survive across fork(2).

Something along the line shown by the attached patch, perhaps.

Other than that, this is very nicely analyzed and discusses possible
alternative approaches sufficiently to easily convince readers that
this is the best solution.

Very nicely done.  Thanks.

 lockfile.h | 4 
 tempfile.h | 4 
 2 files changed, 8 insertions(+)

diff --git a/lockfile.h b/lockfile.h
index 3d30193..e976c7a 100644
--- a/lockfile.h
+++ b/lockfile.h
@@ -55,6 +55,10 @@
  *   * calling `fdopen_lock_file()` to get a `FILE` pointer for the
  * open file and writing to the file using stdio.
  *
+ *   Note that the file descriptor returned by hold_lock_file_for_update()
+ *   is marked O_CLOEXEC, so the new contents must be written by
+ *   you, and not by your subprocess.
+ *
  * When finished writing, the caller can:
  *
  * * Close the file descriptor and rename the lockfile to its final
diff --git a/tempfile.h b/tempfile.h
index 4219fe4..d357177 100644
--- a/tempfile.h
+++ b/tempfile.h
@@ -33,6 +33,10 @@
  *   * calling `fdopen_tempfile()` to get a `FILE` pointer for the
  * open file and writing to the file using stdio.
  *
+ *   Note that the file descriptor returned by create_tempfile()
+ *   is marked O_CLOEXEC, so the new contents must be written by
+ *   you, and not by your subprocess.
+ *
  * When finished writing, the caller can:
  *
  * * Close the file descriptor and remove the temporary file by
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] t6026-merge-attr: child processes must not inherit index.lock handles

[Junio C Hamano]

Johannes Schindelin  writes:

> From: Ben Wijen 
>
> On Windows, files cannot be removed unless all file handles to it have

s/files/a file/, as the file handles later in the sentence are open
on that single file.

Alternatively s/file handles to it/file handles to them/.

> been released. Hence it is particularly important to close handles when
> spawning children (which would probably not even know that they hold on
> to those handles).
>
> The example chosen for this test is a custom merge driver that indeed
> has no idea that it blocks the deletion of index.lock. The full use case
> is a daemon that lives on after the merge, with subsequent invocations
> handing off to the daemon, thereby avoiding hefty start-up costs. We
> simulate this behavior by simply sleeping one second.
>
> Note that the test only fails on Windows, due to the file locking issue.
> Since we have no way to say "expect failure with MINGW, success
> otherwise", we simply skip this test on Windows for now.
>
> Signed-off-by: Ben Wijen 
> Signed-off-by: Johannes Schindelin 
> ---

Otherwise, nicely done.  Thanks.

>  t/t6026-merge-attr.sh | 13 +
>  1 file changed, 13 insertions(+)
>
> diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh
> index ef0cbce..3d28c78 100755
> --- a/t/t6026-merge-attr.sh
> +++ b/t/t6026-merge-attr.sh
> @@ -181,4 +181,17 @@ test_expect_success 'up-to-date merge without common 
> ancestor' '
>   )
>  '
>  
> +test_expect_success !MINGW 'custom merge does not lock index' '
> + git reset --hard anchor &&
> + write_script sleep-one-second.sh <<-\EOF &&
> + sleep 1 &
> + EOF
> +
> + test_write_lines >.gitattributes \
> + "* merge=ours" "text merge=sleep-one-second" &&
> + test_config merge.ours.driver true &&
> + test_config merge.sleep-one-second.driver ./sleep-one-second.sh &&
> + git merge master
> +'
> +
>  test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] document how to reference previous commits

[Junio C Hamano]

Heiko Voigt  writes:

> On Thu, Jul 28, 2016 at 02:55:14PM +0200, Heiko Voigt wrote:
>> To reference previous commits people used to put just the abbreviated
>> SHA-1 into commit messages. This is what has evolved as a more
>> stable format for referencing commits. So lets document it for everyone
>> to lookup when needed.
>
> A quick ping about this patch. Maybe you missed to include it Junio? I
> can not find any reference to it in the cooking mails and in your
> repository.

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

Re: What's cooking in git.git (Aug 2016, #06; Sun, 14)

[Junio C Hamano]

Duy Nguyen  writes:

> On Mon, Aug 15, 2016 at 5:46 AM, Junio C Hamano  wrote:
>> * dt/index-helper (2016-07-06) 21 commits
>>
>>  A new "index-helper" daemon has been introduced to give newly
>>  spawned Git process a quicker access to the data in the index, and
>>  optionally interface with the watchman daemon to further reduce the
>>  refresh cost.
>>
>>  Not quite ready yet, it seems.
>>  cf. 
>>  cf. 

Re: [ANNOUNCE] Git v2.10.0-rc0

[Junio C Hamano]

Junio C Hamano  writes:

> Torsten Bögershausen  writes:
>
>> Is this `text=auto` correct ?
>
> Thanks for spotting a typo.  Definitely.
>
>> I think it should be
>>
>>used to have the same effect as
>>$ echo "* text eol=crlf" >.gitattributes
>
> Thanks.
>
>> # In other words, the `auto` was ignored, as explained here:
>> +   $ git config core.eol crlf
>> +   i.e. declaring all files are text; the combination now is
>> +   equivalent to doing
>> +   $ git config core.autocrlf true
>> +

Just to make sure I got you right, how does this look?

 Documentation/RelNotes/2.10.0.txt | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/RelNotes/2.10.0.txt 
b/Documentation/RelNotes/2.10.0.txt
index 179e575..ccf5f64 100644
--- a/Documentation/RelNotes/2.10.0.txt
+++ b/Documentation/RelNotes/2.10.0.txt
@@ -235,13 +235,13 @@ Performance, Internal Implementation, Development Support 
etc.
  * The API to iterate over all the refs (i.e. for_each_ref(), etc.)
has been revamped.
 
- * The handling of the "text = auto" attribute has been updated.
+ * The handling of the "text=auto" attribute has been corrected.
$ echo "* text=auto eol=crlf" >.gitattributes
used to have the same effect as
-   $ echo "* text=auto eol=crlf" >.gitattributes
+   $ echo "* text eol=crlf" >.gitattributes
$ git config core.eol crlf
-   i.e. declaring all files are text; the combination now is
-   equivalent to doing
+   i.e. declaring all files are text (ignoring "auto").  The
+   combination has been fixed to be equivalent to doing
$ git config core.autocrlf true
 
  * A few tests that specifically target "git rebase -i" have been
-- 
2.10.0-rc0-144-gf47a5f1

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

Re: [PATCH v2] rev-parse: respect core.hooksPath in --git-path

[Junio C Hamano]

Remi Galan Alfonso 
writes:

> I tried to see if the `git config` in other tests were in the
> same case or not but the sheer amount made me reconsider. However
> taking a look at a couple of random ones, there are some scripts
> that would benefit from the conversion.

Yes, that is why I said "it is a good idea to do this where
necessary, but the test Dscho touched here does not need it".
It is a given that there are tons of "git config" users as
we have a lot more tests that were written before test_config
was invented.

It is good to occasionally modernise ancient tests; we usually do so
when we need to make some other changes to them (e.g. I added a
feature, and wanted to add a couple of new tests, but I found it
unreadable and unmaintainable because it kept the ancient style, so
I am updating the existing tests to more modern style first before
doing my changes--and then do my changes on top of the cleaned up
codebase).

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

Re: [ANNOUNCE] Git v2.10.0-rc0

[Junio C Hamano]

Torsten Bögershausen  writes:

> Is this `text=auto` correct ?

Thanks for spotting a typo.  Definitely.

> I think it should be
>
>used to have the same effect as
>$ echo "* text eol=crlf" >.gitattributes

Thanks.

> # In other words, the `auto` was ignored, as explained here:
> +   $ git config core.eol crlf
> +   i.e. declaring all files are text; the combination now is
> +   equivalent to doing
> +   $ git config core.autocrlf true
> +
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: git cherry-pick conflict error message is deceptive when cherry-picking multiple commits

[Junio C Hamano]

Remi Galan Alfonso 
writes:

> Either way (3 or 4 lines) I find it strange to have both advices
> start in the same way except that one is split and not the other.

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

[PATCH v2 1/3] diff-highlight: add some tests.

[Brian Henderson]

Signed-off-by: Brian Henderson 
---
 contrib/diff-highlight/Makefile  |  5 ++
 contrib/diff-highlight/t/Makefile| 22 
 contrib/diff-highlight/t/t9400-diff-highlight.sh | 62 +
 contrib/diff-highlight/t/test-diff-highlight.sh  | 69 
 4 files changed, 158 insertions(+)
 create mode 100644 contrib/diff-highlight/Makefile
 create mode 100644 contrib/diff-highlight/t/Makefile
 create mode 100755 contrib/diff-highlight/t/t9400-diff-highlight.sh
 create mode 100644 contrib/diff-highlight/t/test-diff-highlight.sh

diff --git a/contrib/diff-highlight/Makefile b/contrib/diff-highlight/Makefile
new file mode 100644
index 000..b866259
--- /dev/null
+++ b/contrib/diff-highlight/Makefile
@@ -0,0 +1,5 @@
+# nothing to build
+all:;
+
+test:
+   $(MAKE) -C t
diff --git a/contrib/diff-highlight/t/Makefile 
b/contrib/diff-highlight/t/Makefile
new file mode 100644
index 000..5ff5275
--- /dev/null
+++ b/contrib/diff-highlight/t/Makefile
@@ -0,0 +1,22 @@
+-include ../../../config.mak.autogen
+-include ../../../config.mak
+
+# copied from ../../t/Makefile
+SHELL_PATH ?= $(SHELL)
+SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH))
+T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)
+
+all: test
+test: $(T)
+
+.PHONY: help clean all test $(T)
+
+help:
+   @echo 'Run "$(MAKE) test" to launch test scripts'
+   @echo 'Run "$(MAKE) clean" to remove trash folders'
+
+$(T):
+   @echo "*** $@ ***"; '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)
+
+clean:
+   $(RM) -r 'trash directory'.*
diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh 
b/contrib/diff-highlight/t/t9400-diff-highlight.sh
new file mode 100755
index 000..8eff178
--- /dev/null
+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
@@ -0,0 +1,62 @@
+#!/bin/sh
+
+test_description='Test diff-highlight'
+
+. ./test-diff-highlight.sh
+. "$TEST_DIRECTORY"/test-lib.sh
+
+# PERL is required, but assumed to be present, although not necessarily modern
+# some tests require 5.8
+test_expect_success PERL 'name' 'true'
+
+test_expect_success 'diff-highlight highlights the beginning of a line' '
+   dh_test \
+   "aaa\nbbb\nccc\n" \
+   "aaa\n0bb\nccc\n" \
+"
+ aaa
+-${CW}b${CR}bb
++${CW}0${CR}bb
+ ccc
+"
+'
+
+test_expect_success 'diff-highlight highlights the end of a line' '
+   dh_test \
+   "aaa\nbbb\nccc\n" \
+   "aaa\nbb0\nccc\n" \
+"
+ aaa
+-bb${CW}b${CR}
++bb${CW}0${CR}
+ ccc
+"
+'
+
+test_expect_success 'diff-highlight highlights the middle of a line' '
+   dh_test \
+   "aaa\nbbb\nccc\n" \
+   "aaa\nb0b\nccc\n" \
+"
+ aaa
+-b${CW}b${CR}b
++b${CW}0${CR}b
+ ccc
+"
+'
+
+test_expect_success 'diff-highlight does not highlight whole line' '
+   dh_test \
+   "aaa\nbbb\nccc\n" \
+   "aaa\n000\nccc\n"
+'
+
+test_expect_success 'diff-highlight does not highlight mismatched hunk size' '
+   dh_test \
+   "aaa\nbbb\n" \
+   "aaa\nb0b\nccc\n"
+'
+
+# TODO add multi-byte test
+
+test_done
diff --git a/contrib/diff-highlight/t/test-diff-highlight.sh 
b/contrib/diff-highlight/t/test-diff-highlight.sh
new file mode 100644
index 000..38323e8
--- /dev/null
+++ b/contrib/diff-highlight/t/test-diff-highlight.sh
@@ -0,0 +1,69 @@
+#!/bin/sh
+
+CURR_DIR=$(pwd)
+TEST_OUTPUT_DIRECTORY=$(pwd)
+TEST_DIRECTORY="$CURR_DIR"/../../../t
+DIFF_HIGHLIGHT="$CURR_DIR"/../diff-highlight
+
+CW="\033[7m"
+CR="\033[27m"
+
+export TEST_OUTPUT_DIRECTORY TEST_DIRECTORY CW CR
+
+dh_test() {
+   dh_diff_test "$@" &&
+   dh_commit_test "$@"
+}
+
+dh_diff_test() {
+   a="$1" b="$2"
+
+   printf "$a" >file
+   git add file
+
+   printf "$b" >file
+   git diff file >diff.raw
+
+   if test $# -eq 3
+   then
+   # remove last newline
+   head -n5 diff.raw | test_chomp_eof >diff.exp
+   printf "$3" >>diff.exp
+   else
+   cat diff.raw >diff.exp
+   fi
+
+   diff.act &&
+   test -s diff.act &&
+   diff diff.exp diff.act
+}
+
+dh_commit_test() {
+   a="$1" b="$2"
+
+   printf "$a" >file
+   git add file
+   git commit -m"Add a file" >/dev/null
+
+   printf "$b" >file
+   git commit -am"Update a file" >/dev/null
+
+   git show >commit.raw
+
+   if test "$#" = 3
+   then
+   # remove last newline
+   head -n11 commit.raw | test_chomp_eof >commit.exp
+   printf "$3" >>commit.exp
+   else
+   cat commit.raw >commit.exp
+   fi
+
+   commit.act &&
+   test -s commit.act &&
+   test_cmp commit.exp commit.act
+}
+
+test_chomp_eof() {
+   perl -pe 'chomp if eof'
+}
-- 
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 v2 2/3] diff-highlight: add failing test for handling --graph output.

[Brian Henderson]

Signed-off-by: Brian Henderson 
---
 contrib/diff-highlight/t/t9400-diff-highlight.sh | 13 +++
 contrib/diff-highlight/t/test-diff-highlight.sh  | 43 
 2 files changed, 56 insertions(+)

diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh 
b/contrib/diff-highlight/t/t9400-diff-highlight.sh
index 8eff178..39707c6 100755
--- a/contrib/diff-highlight/t/t9400-diff-highlight.sh
+++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
@@ -59,4 +59,17 @@ test_expect_success 'diff-highlight does not highlight 
mismatched hunk size' '
 
 # TODO add multi-byte test
 
+test_expect_success 'diff-highlight highlights the beginning of a line' '
+   dh_graph_test \
+   "aaa\nbbb\nccc\n" \
+   "aaa\n0bb\nccc\n" \
+   "aaa\nb0b\nccc\n" \
+"
+ aaa
+-${CW}b${CR}bb
++${CW}0${CR}bb
+ ccc
+"
+'
+
 test_done
diff --git a/contrib/diff-highlight/t/test-diff-highlight.sh 
b/contrib/diff-highlight/t/test-diff-highlight.sh
index 38323e8..67f742c 100644
--- a/contrib/diff-highlight/t/test-diff-highlight.sh
+++ b/contrib/diff-highlight/t/test-diff-highlight.sh
@@ -64,6 +64,49 @@ dh_commit_test() {
test_cmp commit.exp commit.act
 }
 
+dh_graph_test() {
+   a="$1" b="$2" c="$3"
+
+   {
+   printf "$a" >file
+   git add file
+   git commit -m"Add a file"
+
+   printf "$b" >file
+   git commit -am"Update a file"
+
+   git checkout -b branch
+   printf "$c" >file
+   git commit -am"Update a file on branch"
+
+   git checkout master
+   printf "$a" >file
+   git commit -am"Update a file again"
+
+   git checkout branch
+   printf "$b" >file
+   git commit -am"Update a file similar to master"
+
+   git merge master
+   git checkout master
+   git merge branch --no-ff
+   } >/dev/null 2>&1
+
+   git log -p --graph --no-merges >graph.raw
+
+   # git log --graph orders the commits different than git log so we hack 
it by
+   # using sed to remove the graph part. We know from other tests, that 
DIFF_HIGHLIGHT
+   # works without the graph, so there should be no diff when running it 
with
+   # and without.
+   graph.exp
+   graph.act &&
+
+   test -s graph.act &&
+   # ignore whitespace since we're using a hacky sed command to remove the 
graph
+   # parts.
+   git diff -b --no-index graph.exp graph.act
+}
+
 test_chomp_eof() {
perl -pe 'chomp if eof'
 }
-- 
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 v2 3/3] diff-highlight: add support for --graph output.

[Brian Henderson]

Signed-off-by: Brian Henderson 
---
 contrib/diff-highlight/diff-highlight | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/contrib/diff-highlight/diff-highlight 
b/contrib/diff-highlight/diff-highlight
index ffefc31..9364423 100755
--- a/contrib/diff-highlight/diff-highlight
+++ b/contrib/diff-highlight/diff-highlight
@@ -21,6 +21,10 @@ my $RESET = "\x1b[m";
 my $COLOR = qr/\x1b\[[0-9;]*m/;
 my $BORING = qr/$COLOR|\s/;
 
+# The patch portion of git log -p --graph should only ever have preceding | and
+# not / or \ as merge history only shows up on the commit line.
+my $GRAPH = qr/$COLOR?\|$COLOR?\s+/;
+
 my @removed;
 my @added;
 my $in_hunk;
@@ -32,12 +36,12 @@ $SIG{PIPE} = 'DEFAULT';
 while (<>) {
if (!$in_hunk) {
print;
-   $in_hunk = /^$COLOR*\@/;
+   $in_hunk = /^$GRAPH*$COLOR*\@/;
}
-   elsif (/^$COLOR*-/) {
+   elsif (/^$GRAPH*$COLOR*-/) {
push @removed, $_;
}
-   elsif (/^$COLOR*\+/) {
+   elsif (/^$GRAPH*$COLOR*\+/) {
push @added, $_;
}
else {
@@ -46,7 +50,7 @@ while (<>) {
@added = ();
 
print;
-   $in_hunk = /^$COLOR*[\@ ]/;
+   $in_hunk = /^$GRAPH*$COLOR*[\@ ]/;
}
 
# Most of the time there is enough output to keep things streaming,
@@ -211,8 +215,8 @@ sub is_pair_interesting {
my $suffix_a = join('', @$a[($sa+1)..$#$a]);
my $suffix_b = join('', @$b[($sb+1)..$#$b]);
 
-   return $prefix_a !~ /^$COLOR*-$BORING*$/ ||
-  $prefix_b !~ /^$COLOR*\+$BORING*$/ ||
+   return $prefix_a !~ /^$GRAPH*$COLOR*-$BORING*$/ ||
+  $prefix_b !~ /^$GRAPH*$COLOR*\+$BORING*$/ ||
   $suffix_a !~ /^$BORING*$/ ||
   $suffix_b !~ /^$BORING*$/;
 }
-- 
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 v2 0/3] diff-highlight: add support for git log --graph output.

[Brian Henderson]

Changes made per Eric.

On Wed, Aug 10, 2016 at 08:56:35AM +, Eric Wong wrote:
> Brian Henderson  wrote:
> 
> Hi Brian,
> 
> A few minor portability/style nits below, but contrib/ probably
> (still?) has laxer rules than the rest of git...
> 
> I think we still require Signed-off-by lines in contrib,
> though...

added

> 
> > +++ b/contrib/diff-highlight/t/Makefile
> > @@ -0,0 +1,19 @@
> > +-include ../../../config.mak.autogen
> > +-include ../../../config.mak
> > +
> > +T = $(wildcard t[0-9][0-9][0-9][0-9]-*.sh)
> > +
> > +all: test
> > +test: $(T)
> > +
> > +.PHONY: help clean all test $(T)
> > +
> > +help:
> > +   @echo 'Run "$(MAKE) test" to launch test scripts'
> > +   @echo 'Run "$(MAKE) clean" to remove trash folders'
> > +
> > +$(T):
> > +   @echo "*** $@ ***"; sh $@ $(GIT_TEST_OPTS)
> 
> Probably:
> 
>   @echo "*** $@ ***"; '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)
> 
> like we do in t/Makefile in case we need to override 'sh'.
> 

done, although I basically had to copy the SHELL_PATH_SQ logic from t/Makefile

> > +
> > +clean:
> > +   $(RM) -r 'trash directory'.*
> > diff --git a/contrib/diff-highlight/t/t9400-diff-highlight.sh 
> > b/contrib/diff-highlight/t/t9400-diff-highlight.sh
> > new file mode 100644
> > index 000..ca7605f
> > --- /dev/null
> > +++ b/contrib/diff-highlight/t/t9400-diff-highlight.sh
> > @@ -0,0 +1,63 @@
> > +#!/bin/sh
> > +#
> > +# Copyright (C) 2016
> 
> IANAL, but I think your name (or who you represent) needs to be
> in the copyright.

diff-highlight doesn't have a copyright, so I just removed it. ok?

> 
> > +
> > +test_description='Test diff-highlight'
> > +
> > +. ./test-diff-highlight.sh
> > +. $TEST_DIRECTORY/test-lib.sh
> 
> TEST_DIRECTORY ought to be quoted since it could contain
> shell-unfriendly chars (we intentionally name 'trash directory'
> this way to trigger errors).

done

> 
> > +
> > +# PERL is required, but assumed to be present, although not necessarily 
> > modern
> > +# some tests require 5.8
> > +
> > +test_expect_success 'diff-highlight highlightes the beginning of a line' '
> 
> You can declare a prereq for PERL::
> 
>   test_expect_success PERL 'name' 'true'

done

> 
> And spelling: "highlights" (there's other instances of this, too)

oops, thanks

> 
> > +  dh_test \
> > +"aaa\nbbb\nccc\n" \
> > +"aaa\n0bb\nccc\n" \
> > +"
> 
> We use tabs for shell indentation.

done



> > +
> > +dh_diff_test() {
> > +  local a="$1" b="$2"
> 
> "local" is not a portable construct.  It's common for
> Debian/Ubuntu systems to use dash as /bin/sh instead of bash;
> (dash is faster, and mostly sticks to POSIX)
> 
> The "devscripts" package in Debian/Ubuntu-based systems has a
> handy "checkbashisms" tool for checking portability of shell
> scripts.

checkbashisms didn't output anything, and I found other instances of local in 
some tests. but I removed anyway.

> 
> > +
> > +  printf "$a" > file
> > +  git add file
> > +
> > +  printf "$b" > file
> > +
> > +  git diff file > diff.raw
> 
> Commands should be '&&'-chained here since any of them could fail
> ("git add"/printf/etc could run out of space or fail on disk errors)

I wasn't totally sure what to do here. I added some checks for empty files and
&& chained them with the last command to verify I wasn't diffing 2 empty files.
Hope that is sufficient.

> 
> Also, our redirect style is:  command >file
> without a space between '>' and destination.

done

> 
> See Documentation/CodingGuidelines for more details.
> Unfortunately, the reasoning is not explained for '>NOSPACE'
> and I'm not sure exactly why, either...
> 
> > +  if test "$#" = 3

done

> 
> Better to use -eq for numeric comparisons: test $# -eq 3
> Quoting $# doesn't seem necessary unless your shell is
> hopelessly buggy :)
> 
> > +  then
> > +# remove last newline
> > +head -n5 diff.raw | head -c -1 > diff.act
> 
> "head -c" isn't portable, fortunately Jeff hoisted it out for
> use as test_copy_bytes in commit 48860819e8026
> https://public-inbox.org/git/20160630090753.ga17...@sigill.intra.peff.net/

It didn't look like his version supported a negative number, so I rolled my own.

> 
> > +printf "$3" >> diff.act
> > +  else
> > +cat diff.raw > diff.act
> > +  fi
> > +
> > +  < diff.raw $CMD > diff.exp
> 
> $CMD probably needs to be quoted.  However, by the time I got
> here I had to ask myself:  What is $CMD again?
> A: Oh, look up at the top!
> 
> *shrug*  My attention span is tiny and my fonts are gigantic.
> 
> Perhaps using:
> 
>   DIFF_HIGHLIGHT="$CURR_DIR"/../diff-highlight
> 
> Would be more-readable?

good, done.

> 
> > +
> > +  diff diff.exp diff.act
> 
> Maybe use test_cmp to avoid depending external diff.
> (or "git diff -b --no-index" in your later test)
> 
> Same comments for the rest of the series, I think.
> 
> Typically, we expect a reroll in a few days, and I guess there's
> no rush (so you can squash your comment patch in addressing
> Junio's concern into 3/3).
> 

Re: [ANNOUNCE] Git for Windows 2.9.3

[Junio C Hamano]

Johannes Schindelin  writes:

>> And then your "git cat-file" patch can be upstreamed with the option
>> renamed to (or with an additional synonym) "--filters", which would make
>> things consistent.
>
> Right. I would like to ask for a `--smudge` synonym nevertheless, just
> because I already use this. On the other hand, it is early enough to tell
> everybody who knows about this feature to change their invocation (anybody
> who would know about `--smudge` would be in that 1% of users that have
> read the release notes, so most likely would read the next release notes,
> too).

It is OK if it were your private edition, but you end up hurting
your users if you need to redo the feature differently.

That's the price of your using open source and taking a short-cut.
Adding a "--smudge" synonym is spreading the same hurt to outside
your fork.  Let's see if we can avoid doing that.  Perhaps mark that
"--smudge" as experimental-and-subject-to-change and re-announce?

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

Re: git cherry-pick conflict error message is deceptive when cherry-picking multiple commits

[Remi Galan Alfonso]

Stephen Morton  writes:
> [snip]
> On 2016-08-16 4:44 AM, Remi Galan Alfonso wrote:
>> Hi Stephen,
>>
>> Stephen Morton  writes:
>>> +if  (multiple_commits)
>>> +   advise(_("after resolving the conflicts,
>>> mark the corrected paths with 'git add ' or 'git rm '\n"
>>> +"then continue with 'git %s
>>> --continue'\n"
>>> +"or cancel with 'git %s
>>> --abort'" ), action_name(opts), action_name(opts));
>>> +else
>>> +advise(_("after resolving the
>>> conflicts, mark the corrected paths\n"
>>> +"with 'git add ' or 'git
>>> rm '\n"
>>> +"and commit the result with
>>> 'git commit'"));
>> In both cases (multiple_commits or not), the beginning of the advise
>> is nearly the same, with only a '\n' in the middle being the
>> difference:
>>
>> multiple_commits:
>>   "after resolving the conflicts, mark the corrected paths with 'git
>>   add ' or 'git rm '\n"
>>
>> !multiple_commits:
>>   "after resolving the conflicts, mark the corrected paths\n with 'git
>>   add ' or 'git rm '\n"
>>~~~^
>>
>> In 'multiple_commits' case the advise is more than 80 characters long,
>> did you forget the '\n' in that case?
> A previous comment had indicated that having 4 lines was too many. And I
> tend to agree. So I tried to squash it into 3. Back in xterm days, 80
> characters was sacrosanct, but is it really a big deal to exceed it now?

Either way (3 or 4 lines) I find it strange to have both advices
start in the same way except that one is split and not the other.

I cannot tell if it's a big deal or not to exceed 80 characters
but FWIW most of my stuff (terminal and emacs) is 80 columns
long, and I haven't known the "xterm days".

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

Re: [ANNOUNCE] Git for Windows 2.9.3

[Johannes Schindelin]

Hi Junio,

On Sat, 13 Aug 2016, Junio C Hamano wrote:

> Johannes Schindelin  writes:
> 
> > New Features
> >
> >   • Comes with Git 2.9.3.
> 
> For future reference, what time (in UTC) of the day is convenient
> for you to see an upstream tarball?

Heh... I don't do tarballs anymore, I now use this newfangled tool to
manage source code... "gyt" or something like that, it is called.

:-)

Given that between you and me there is currently a time zone difference of
9h (except for four weeks, two in spring, when it is only 8h, and two in
fall, when it is 10h), I believe we cannot find a time that is convenient
for both of us.

But I also think it is fine, when I discover a new upstream Git version in
the morning, I can spend all day on fixing any problems and on packaging
the result ;-)

> >   • Sports a new --smudge option for git cat-file that lets it pass
> >   blob contents through smudge filters configured for the specified
> >   path.
> 
> Perhaps we want to upstream this, together with a new "--clean" option
> for git hash-object?

No question about that. I just needed this in a hurry and short-circuited
it into Git for Windows before submitting it upstream.

> And after writing all of the above, I noticed that hash-object by
> default uses the clean machinery and that can be turned off by giving
> the "--no-filters" option.  The reason why the option is not called
> "--no-clean" is because it is not just about the clean filter but is
> about using the entirety of convert_to_git() filter chain.

Right, as is the --smudge option (it is about the entirety of
convert_to_worktree()).

> We probably should teach "hash-objects" to take "--filters" for
> consistency.

I actually thought about that, too. Which was one of the reasons I did not
submit the patch to the Git mailing list first, as I expect several
iterations to be necessary to get everything into `master`.

> And then your "git cat-file" patch can be upstreamed with the option
> renamed to (or with an additional synonym) "--filters", which would make
> things consistent.

Right. I would like to ask for a `--smudge` synonym nevertheless, just
because I already use this. On the other hand, it is early enough to tell
everybody who knows about this feature to change their invocation (anybody
who would know about `--smudge` would be in that 1% of users that have
read the release notes, so most likely would read the next release notes,
too).

Ciao,
Dscho

Re: git cherry-pick conflict error message is deceptive when cherry-picking multiple commits

[Stephen Morton]

Responding to a few comments...

On 2016-08-14 7:44 AM, Christian Couder wrote:

multiple_commits)
... but here multiple_commits is the last argument.
It would be better if it was more consistent.

(Johannes made the same comment.)
Yes. Will do.




multiple_commits = (todo_list->next) != NULL;
Why not "last_commit" instead of "multiple_commits"?



Because it *isn't*. You can see that in pick_commits(), I set 
multiple_commits outside of the `for todo_list` loop. It is not 
re-evaluated at every iteration of the loop. As per my comment when 
emailing the patch "I intentionally print the '--continue' hint even in 
the case where it's last of n commits that fails. " I think it makes 
much more sense that "this is the message you always get when 
cherry-picking multiple commits as opposed to "this is the message you 
sometimes get, except when it's the last one". (Yes, the careful 
observer will realize that if when cherry-picking multiple commits, 
there are conflicts in the second-last and last then the --continue  
from the second-last will result in multiple_commits being set to 0. I 
can live with that.)



On 2016-08-16 4:44 AM, Remi Galan Alfonso wrote:

Hi Stephen,

Stephen Morton  writes:

+if  (multiple_commits)
+   advise(_("after resolving the conflicts,
mark the corrected paths with 'git add ' or 'git rm '\n"
+"then continue with 'git %s
--continue'\n"
+"or cancel with 'git %s
--abort'" ), action_name(opts), action_name(opts));
+else
+advise(_("after resolving the
conflicts, mark the corrected paths\n"
+"with 'git add ' or 'git
rm '\n"
+"and commit the result with
'git commit'"));

In both cases (multiple_commits or not), the beginning of the advise
is nearly the same, with only a '\n' in the middle being the
difference:

multiple_commits:
  "after resolving the conflicts, mark the corrected paths with 'git
  add ' or 'git rm '\n"

!multiple_commits:
  "after resolving the conflicts, mark the corrected paths\n with 'git
  add ' or 'git rm '\n"
   ~~~^

In 'multiple_commits' case the advise is more than 80 characters long,
did you forget the '\n' in that case?
A previous comment had indicated that having 4 lines was too many. And I 
tend to agree. So I tried to squash it into 3. Back in xterm days, 80 
characters was sacrosanct, but is it really a big deal to exceed it now?



On 2016-08-14 7:44 AM, Christian Couder wrote:

...but please try to send a real patch.

There is https://github.com/git/git/blob/master/Documentation/SubmittingPatches
and also SubmitGit that can help you do that.
Agreed. I just want to send a patch that stands a reasonable chance of 
getting accepted.


Stephen


--
Stephen Morton, 7750 SR Product Group, SW Development Tools/DevOps
w: +1-613-784-6026 (int: 2-825-6026) m: +1-613-302-2589 | EST Time Zone

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

Re: [PATCH 2/2] mingw: ensure temporary file handles are not inherited by child processes

[Lars Schneider]

> On 17 Aug 2016, at 14:41, Johannes Schindelin  
> wrote:
> 
> From: Ben Wijen 
> 
> When the index is locked and child processes inherit the handle to
> said lock and the parent process wants to remove the lock before the
> child process exits, on Windows there is a problem: it won't work
> because files cannot be deleted if a process holds a handle on them.
> The symptom:
> 
>Rename from 'xxx/.git/index.lock' to 'xxx/.git/index' failed.
>Should I try again? (y/n)
> 
> Spawning child processes with bInheritHandles==FALSE would not work
> because no file handles would be inherited, not even the hStdXxx
> handles in STARTUPINFO (stdin/stdout/stderr).
> 
> Opening every file with O_NOINHERIT does not work, either, as e.g.
> git-upload-pack expects inherited file handles.
> 
> This leaves us with the only way out: creating temp files with the
> O_NOINHERIT flag. This flag is Windows-specific, however. For our
> purposes, it is equivalent our purposes) to O_CLOEXEC (which does not
> exist on Windows), so let's just open temporary files with the
> O_CLOEXEC flag and map that flag to O_NOINHERIT on Windows.
> 
> This fixes the test that we just introduced to demonstrate the problem.
> 
> Signed-off-by: Ben Wijen 
> Signed-off-by: Johannes Schindelin 
> ---
> compat/mingw.h| 4 
> t/t6026-merge-attr.sh | 2 +-
> tempfile.c| 2 +-
> 3 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/compat/mingw.h b/compat/mingw.h
> index 95e128f..753e641 100644
> --- a/compat/mingw.h
> +++ b/compat/mingw.h
> @@ -67,6 +67,10 @@ typedef int pid_t;
> #define F_SETFD 2
> #define FD_CLOEXEC 0x1
> 
> +#if !defined O_CLOEXEC && defined O_NOINHERIT
> +#define O_CLOEXECO_NOINHERIT
> +#endif
> +
> #ifndef EAFNOSUPPORT
> #define EAFNOSUPPORT WSAEAFNOSUPPORT
> #endif
> diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh
> index 3d28c78..dd8f88d 100755
> --- a/t/t6026-merge-attr.sh
> +++ b/t/t6026-merge-attr.sh
> @@ -181,7 +181,7 @@ test_expect_success 'up-to-date merge without common 
> ancestor' '
>   )
> '
> 
> -test_expect_success !MINGW 'custom merge does not lock index' '
> +test_expect_success 'custom merge does not lock index' '
>   git reset --hard anchor &&
>   write_script sleep-one-second.sh <<-\EOF &&
>   sleep 1 &
> diff --git a/tempfile.c b/tempfile.c
> index 0af7ebf..db3981d 100644
> --- a/tempfile.c
> +++ b/tempfile.c
> @@ -120,7 +120,7 @@ int create_tempfile(struct tempfile *tempfile, const char 
> *path)
>   prepare_tempfile_object(tempfile);
> 
>   strbuf_add_absolute_path(>filename, path);
> - tempfile->fd = open(tempfile->filename.buf, O_RDWR | O_CREAT | O_EXCL, 
> 0666);
> + tempfile->fd = open(tempfile->filename.buf, O_RDWR | O_CREAT | O_EXCL | 
> O_CLOEXEC, 0666);

This solution works great for me. I struggled with a similar problem 
in my Git filter protocol series:
http://public-inbox.org/git/20160810130411.12419-16-larsxschnei...@gmail.com/

I also tried selectively disabling inheritance for file handles but it looks 
like
as this is not easily possible right now: 
https://github.com/git-for-windows/git/issues/770#issuecomment-238816331

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

Re: What's cooking in git.git (Aug 2016, #06; Sun, 14)

[David Turner]

On Wed, 2016-08-17 at 17:49 +0700, Duy Nguyen wrote:
> On Mon, Aug 15, 2016 at 5:46 AM, Junio C Hamano  wrote:
> > * dt/index-helper (2016-07-06) 21 commits
> >
> >  A new "index-helper" daemon has been introduced to give newly
> >  spawned Git process a quicker access to the data in the index, and
> >  optionally interface with the watchman daemon to further reduce the
> >  refresh cost.
> >
> >  Not quite ready yet, it seems.
> >  cf. 
> >  cf. 

Re: [PATCH 1/3] t3404: become resilient to GETTEXT_POISON

[Johannes Schindelin]

Hi Vasco,

On Fri, 12 Aug 2016, Vasco Almeida wrote:

> The concerned test greps the output of exit_with_patch() in
> git-rebase--interactive.sh script.
> 
> Signed-off-by: Vasco Almeida 

Thank you for keeping an eye out for these issues. I have to admit that I
am a bit confused when to use i18ngrep and when not, so it is good to know
that my mistakes won't survive for long!

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

Re: [PATCH 2/2] mingw: ensure temporary file handles are not inherited by child processes

[Eric Sunshine]

On Wed, Aug 17, 2016 at 8:41 AM, Johannes Schindelin
 wrote:
> When the index is locked and child processes inherit the handle to
> said lock and the parent process wants to remove the lock before the
> child process exits, on Windows there is a problem: it won't work
> because files cannot be deleted if a process holds a handle on them.
> The symptom:
>
> Rename from 'xxx/.git/index.lock' to 'xxx/.git/index' failed.
> Should I try again? (y/n)
>
> Spawning child processes with bInheritHandles==FALSE would not work
> because no file handles would be inherited, not even the hStdXxx
> handles in STARTUPINFO (stdin/stdout/stderr).
>
> Opening every file with O_NOINHERIT does not work, either, as e.g.
> git-upload-pack expects inherited file handles.
>
> This leaves us with the only way out: creating temp files with the
> O_NOINHERIT flag. This flag is Windows-specific, however. For our
> purposes, it is equivalent our purposes) to O_CLOEXEC (which does not

s/our purposes)//

> exist on Windows), so let's just open temporary files with the
> O_CLOEXEC flag and map that flag to O_NOINHERIT on Windows.
>
> This fixes the test that we just introduced to demonstrate the problem.
>
> Signed-off-by: Ben Wijen 
> Signed-off-by: Johannes Schindelin 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/1] convert: Correct NNO tests and missing `LF will be replaced by CRLF`

[Johannes Schindelin]

Hi Torsten,

On Sat, 13 Aug 2016, tbo...@web.de wrote:

> From: Torsten Bögershausen 
> 
> Change since v1:
> - The changes done in 1/2 in t0027 needed to be reverted in 2/2.
>   Put both changes for convert.c and t0027 into a single commit
> Thanks to Johannes Sixt.
> Torsten Bögershausen (1):
>   convert: Correct NNO tests and missing `LF will be replaced by CRLF`

Thank you so much for the fix!

Ciao,
Dscho

[PATCH 2/2] mingw: ensure temporary file handles are not inherited by child processes

[Johannes Schindelin]

From: Ben Wijen 

When the index is locked and child processes inherit the handle to
said lock and the parent process wants to remove the lock before the
child process exits, on Windows there is a problem: it won't work
because files cannot be deleted if a process holds a handle on them.
The symptom:

Rename from 'xxx/.git/index.lock' to 'xxx/.git/index' failed.
Should I try again? (y/n)

Spawning child processes with bInheritHandles==FALSE would not work
because no file handles would be inherited, not even the hStdXxx
handles in STARTUPINFO (stdin/stdout/stderr).

Opening every file with O_NOINHERIT does not work, either, as e.g.
git-upload-pack expects inherited file handles.

This leaves us with the only way out: creating temp files with the
O_NOINHERIT flag. This flag is Windows-specific, however. For our
purposes, it is equivalent our purposes) to O_CLOEXEC (which does not
exist on Windows), so let's just open temporary files with the
O_CLOEXEC flag and map that flag to O_NOINHERIT on Windows.

This fixes the test that we just introduced to demonstrate the problem.

Signed-off-by: Ben Wijen 
Signed-off-by: Johannes Schindelin 
---
 compat/mingw.h| 4 
 t/t6026-merge-attr.sh | 2 +-
 tempfile.c| 2 +-
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/compat/mingw.h b/compat/mingw.h
index 95e128f..753e641 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -67,6 +67,10 @@ typedef int pid_t;
 #define F_SETFD 2
 #define FD_CLOEXEC 0x1
 
+#if !defined O_CLOEXEC && defined O_NOINHERIT
+#define O_CLOEXEC  O_NOINHERIT
+#endif
+
 #ifndef EAFNOSUPPORT
 #define EAFNOSUPPORT WSAEAFNOSUPPORT
 #endif
diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh
index 3d28c78..dd8f88d 100755
--- a/t/t6026-merge-attr.sh
+++ b/t/t6026-merge-attr.sh
@@ -181,7 +181,7 @@ test_expect_success 'up-to-date merge without common 
ancestor' '
)
 '
 
-test_expect_success !MINGW 'custom merge does not lock index' '
+test_expect_success 'custom merge does not lock index' '
git reset --hard anchor &&
write_script sleep-one-second.sh <<-\EOF &&
sleep 1 &
diff --git a/tempfile.c b/tempfile.c
index 0af7ebf..db3981d 100644
--- a/tempfile.c
+++ b/tempfile.c
@@ -120,7 +120,7 @@ int create_tempfile(struct tempfile *tempfile, const char 
*path)
prepare_tempfile_object(tempfile);
 
strbuf_add_absolute_path(>filename, path);
-   tempfile->fd = open(tempfile->filename.buf, O_RDWR | O_CREAT | O_EXCL, 
0666);
+   tempfile->fd = open(tempfile->filename.buf, O_RDWR | O_CREAT | O_EXCL | 
O_CLOEXEC, 0666);
if (tempfile->fd < 0) {
strbuf_reset(>filename);
return -1;
-- 
2.9.2.691.g78954f3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] t6026-merge-attr: child processes must not inherit index.lock handles

[Johannes Schindelin]

From: Ben Wijen 

On Windows, files cannot be removed unless all file handles to it have
been released. Hence it is particularly important to close handles when
spawning children (which would probably not even know that they hold on
to those handles).

The example chosen for this test is a custom merge driver that indeed
has no idea that it blocks the deletion of index.lock. The full use case
is a daemon that lives on after the merge, with subsequent invocations
handing off to the daemon, thereby avoiding hefty start-up costs. We
simulate this behavior by simply sleeping one second.

Note that the test only fails on Windows, due to the file locking issue.
Since we have no way to say "expect failure with MINGW, success
otherwise", we simply skip this test on Windows for now.

Signed-off-by: Ben Wijen 
Signed-off-by: Johannes Schindelin 
---
 t/t6026-merge-attr.sh | 13 +
 1 file changed, 13 insertions(+)

diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh
index ef0cbce..3d28c78 100755
--- a/t/t6026-merge-attr.sh
+++ b/t/t6026-merge-attr.sh
@@ -181,4 +181,17 @@ test_expect_success 'up-to-date merge without common 
ancestor' '
)
 '
 
+test_expect_success !MINGW 'custom merge does not lock index' '
+   git reset --hard anchor &&
+   write_script sleep-one-second.sh <<-\EOF &&
+   sleep 1 &
+   EOF
+
+   test_write_lines >.gitattributes \
+   "* merge=ours" "text merge=sleep-one-second" &&
+   test_config merge.ours.driver true &&
+   test_config merge.sleep-one-second.driver ./sleep-one-second.sh &&
+   git merge master
+'
+
 test_done
-- 
2.9.2.691.g78954f3


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

[PATCH 0/2] Do not lock temporary files via child processes on Windows

[Johannes Schindelin]

This issue was originally reported and fixed in
https://github.com/git-for-windows/git/pull/755

The problem is that file handles to temporary files (such as
index.lock) were inherited by spawned processes. If those spawned
processes do not exit before the parent process wants to delete or
rename them, we are in big trouble.

The original use case triggering the bug is a merge driver that does
not quit, but listen to subsequent merge requests.

However, the same issue turned up in Lars Schneider's work on making
clean/smudge filters batchable (i.e. more efficient by avoiding
possibly thousands of child processes, one per file).


Ben Wijen (2):
  t6026-merge-attr: child processes must not inherit index.lock handles
  mingw: ensure temporary file handles are not inherited by child
processes

 compat/mingw.h|  4 
 t/t6026-merge-attr.sh | 13 +
 tempfile.c|  2 +-
 3 files changed, 18 insertions(+), 1 deletion(-)

Published-As: https://github.com/dscho/git/releases/tag/mingw-index-lock-v1
Fetch-It-Via: git fetch https://github.com/dscho/git mingw-index-lock-v1

-- 
2.9.2.691.g78954f3

base-commit: 07c92928f2b782330df6e78dd9d019e984d820a7
--
To unsubscribe from this list: send the line "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] document how to reference previous commits

[Heiko Voigt]

Hi,

On Thu, Jul 28, 2016 at 02:55:14PM +0200, Heiko Voigt wrote:
> To reference previous commits people used to put just the abbreviated
> SHA-1 into commit messages. This is what has evolved as a more
> stable format for referencing commits. So lets document it for everyone
> to lookup when needed.

A quick ping about this patch. Maybe you missed to include it Junio? I
can not find any reference to it in the cooking mails and in your
repository.

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

Re: What's cooking in git.git (Aug 2016, #06; Sun, 14)

[Duy Nguyen]

On Mon, Aug 15, 2016 at 5:46 AM, Junio C Hamano  wrote:
> * dt/index-helper (2016-07-06) 21 commits
>
>  A new "index-helper" daemon has been introduced to give newly
>  spawned Git process a quicker access to the data in the index, and
>  optionally interface with the watchman daemon to further reduce the
>  refresh cost.
>
>  Not quite ready yet, it seems.
>  cf. 
>  cf. 

[ANNOUNCE] Git Rev News edition 18

[Christian Couder]

Hi everyone,

I'm happy announce that the 18th edition of Git Rev News is now published:

https://git.github.io/rev_news/2016/08/17/edition-18/

Thanks a lot to all the contributors and helpers, especially Lars,
Dscho, Jakub, Roberto and Josh!

Enjoy,
Christian and Thomas.
--
To unsubscribe from this list: send the line "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: Working with zip files

[David Lang]

On Tue, 16 Aug 2016, Nikolaus Rath wrote:


On Aug 16 2016, David Lang  wrote:

On Tue, 16 Aug 2016, Nikolaus Rath wrote:


I would like to store Simulink models in a Git
repository. Unfortunately, the file format is binary. But luckily, the
binary format happens to be a zipfile containing nicely formatted XML
files.

Is there a way to teach Git to take advantage of this when storing,
diff-ing and merging these files?


you should be able to use clean/smudge to have git store the files
uncompressed, which will help a lot.


Having looked at that, I'm not sure if this really helps:

As I understand, the smudge command is run on checkout to convert the
blob in the repository to the format that is desired in the working
tree. But this is the opposite of what I need: on checkout, I need to
convert the text data in the repository to a blob in the working tree.

Furthermore, I need to convert multiple text files into one blob, will
smudge/clean seem to do just 1:1 conversions.

Am I missing something? Are there any other options?


so the smudge command would zip the file and the clean command would unzip the 
file (assuming it's a single file, if the zip is multiple files, you will have 
to add something to combine them)


you want the working tree to have a zip file and the repository to have text.

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

Re: git cherry-pick conflict error message is deceptive when cherry-picking multiple commits

[Johannes Schindelin]

Hi,

On Tue, 16 Aug 2016, Remi Galan Alfonso wrote:

> Stephen Morton  writes:
> > +if  (multiple_commits)
> > +   advise(_("after resolving the conflicts,
> > mark the corrected paths with 'git add ' or 'git rm '\n"
> > +"then continue with 'git %s
> > --continue'\n"
> > +"or cancel with 'git %s
> > --abort'" ), action_name(opts), action_name(opts));
> > +else
> > +advise(_("after resolving the
> > conflicts, mark the corrected paths\n"
> > +"with 'git add ' or 'git
> > rm '\n"
> > +"and commit the result with
> > 'git commit'"));
> 
> In both cases (multiple_commits or not), the beginning of the advise
> is nearly the same, with only a '\n' in the middle being the
> difference:
> 
> multiple_commits:
>  "after resolving the conflicts, mark the corrected paths with 'git
>  add ' or 'git rm '\n"
> 
> !multiple_commits:
>  "after resolving the conflicts, mark the corrected paths\n with 'git
>  add ' or 'git rm '\n"
>   ~~~^
> 
> In 'multiple_commits' case the advise is more than 80 characters long,
> did you forget the '\n' in that case?
> 
> If you end up using the same beginning of advice, maybe it's possible
> to give it before the 'if(multiple_commits)' and avoid duplication of
> the lines.

I concur with this, and also with Christian's advice to append the
parameter consistently as last one, and also with renaming it to
"last_commit".

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

Re: [PATCH v2] rev-parse: respect core.hooksPath in --git-path

[Remi Galan Alfonso]

Junio C Hamano  writes:
> Remi Galan Alfonso 
> writes:
>
>> Johannes Schindelin  writes:
>>> Hi Rémi,
>>>
>>> On Tue, 16 Aug 2016, Remi Galan Alfonso wrote:
>>>
>>> > Johannes Schindelin  writes:
>>> > > diff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh
>>> > > index 5e3fb3a..f1f9aee 100755
>>> > > --- a/t/t1350-config-hooks-path.sh
>>> > > +++ b/t/t1350-config-hooks-path.sh
>>> > > @@ -34,4 +34,10 @@ test_expect_success 'Check that various forms of 
>>> > > specifying core.hooksPath work'
>>> > >  test_cmp expect actual
>>> > >  '
>>> > >  
>>> > > +test_expect_success 'git rev-parse --git-path hooks' '
>>> > > +git config core.hooksPath .git/custom-hooks &&
>>> >
>>> > Any reason to not use 'test_config' here?
>>>
>>> Yes: consistency. The rest of the script uses `git config`, not
>>> `test_config`.
>>
>> Fine by me, then. Sorry for the noise.
>
> No, thanks for reviewing.  I'll take Dscho's patch as-is but once it
> hits 'next', it probably is a good idea to do a separate clean-up
> patch on top to use test_config where necessary.
>
> Having said that, this entire script is about constantly changing
> the value of that single configuration variable and see how the code
> performs, so any new test added after existing ones is expected to
> ignore left-over values in the variable and set it to a value of its
> own liking.  So I suspect there is no existing "git config" call in
> this script, with or without Dscho's patch, that would benefit from
> getting converted to test_config.

Thanks for checking the ones in this file, considering the lack
of benefits it might not be worth it to change it for now.

I tried to see if the `git config` in other tests were in the
same case or not but the sheer amount made me reconsider. However
taking a look at a couple of random ones, there are some scripts
that would benefit from the conversion.
For example in t3200-branch there is:

test_expect_success 'git branch with column.*' '
git config column.ui column &&
git config column.branch "dense" &&
COLUMNS=80 git branch >actual &&
git config --unset column.branch &&
git config --unset column.ui &&
cat >expected <<\EOF &&
  a/b/c   bam   foo   l   * mastern o/p   r
  abc bar   j/k   m/m   master2   o/o   q
EOF
test_cmp expected actual
'

The conversion would drop 2 lines in this particular case and
would avoid bleeding the config should `git branch` fail (not
sure how possible that is...).

(I can make a patch for t3200 but that won't be before
days/weeks, so if someone else wants to take care of it I won't
mind)

If I have some time to kill, I'll try looking at a few others but
I won't expect that any time soon.

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

Re: git-mergetool reverse file ordering

[Johannes Sixt]

Am 17.08.2016 um 08:46 schrieb David Aguilar:

The only thing that using diff-files doesn't address is the
rerere support in mergetool where it processes the files in
the order specified by "git rerere remaining".  This is why I
initially thought we needed a generic sort-like command.


I see. This is actually an important code path. How about this code 
structure:


if test $# -eq 0
then
cd_to_toplevel

if test -e "$GIT_DIR/MERGE_RR"
then
set -- $(git rerere remaining)
fi
fi
files=$(git diff-files --name-only --diff-filter=U -- "$@")

This does not require an enhancement of rerere-remaining and still 
captures all three cases that currently go through separate branches. 
(Throw in some version of --ignore-submodules= if necessary, but I guess 
it is not.)


We do have a problem if there are file names with spaces, but it is not 
a new problem.



The patches could then be:

1. switch to diff-files, add tests, and document how
   diff.orderFile affects mergetool.

2. Teach mergetool about the "-O" flag so that it can
   override the configuration, and add tests.  It could be
   argued that this should be squashed into (1).

3. (optional) teach "rerere remaining" to honor the
   -O flag and teach mergetool to supply the option.

Sound good?


Sure, except that 3. won't be necessary.

-- Hannes

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

Re: git-mergetool reverse file ordering

[David Aguilar]

On Wed, Aug 17, 2016 at 08:10:46AM +0200, Johannes Sixt wrote:
> Am 17.08.2016 um 08:05 schrieb Johannes Sixt:
> > Am 17.08.2016 um 03:25 schrieb David Aguilar:
> > > Hmm, I do like the idea of reusing the diff orderFile, but a
> > > mechanism for sorting arbitrary inputs based on the orderFile
> > > isn't currently exposed in a way that mergetool could use it.
> > 
> > Instead of using 'git ls-files -u | sed ... | sort -u' you could use
> > 
> >   git diff-files --name-status -O... | sed -ne '/^U/s/^..//p'
> 
> Or even better:
> 
> git diff-files --name-only --diff-filter=U -O...

Nice!


> > > But, that sort is honestly kinda crude.  It can't implement the
> > > interesting case where we want bar.h to sort before bar.c but
> > > not foo.h.
> > > 

Note: I had a subtle typo above -- I meant to describe this desired
order:
bar.h
bar.c
foo.h
foo.c

which is not possible with an orderFile that has only:

*.h
*.c

But...

> > > If we did the sort option, we could have both.
> > 
> > I don't think that we need a new filter when the diff machinery is
> > capable of re-ordering files. We should enhance the diff machinery instead.
> > 

Reading the code more thoroughly, I fully agree.

Switching to a diff-files invocation lets us reuse the
diff.orderFile machinery and does not require exposing any
additional helpers.

While it would be *nice* if we had a way to sort foo.h before
foo.c and after bar.c, that's just a pie-in-the-sky idea.
Consistency is king.


The only thing that using diff-files doesn't address is the
rerere support in mergetool where it processes the files in
the order specified by "git rerere remaining".  This is why I
initially thought we needed a generic sort-like command.

That code path does not currently do any sorting (which may
explain why we didn't notice it if we were just looking for
"sort" invocations) but maybe that's an edge case that we don't
need to address initially (if at all).

Would it be okay for the rerere code path to not honor the
orderFile?  IMO if we documented the behavior then it would be
acceptable, and perhaps can be improved as a follow-up.

For the basic support the implementation becomes really
simple: switch to using diff-files instead of ls-files.

If we did want consistency in the "git rerere remaining" code
path, would it be acceptable to teach "rerere" about
"-O" so that we could drive it from mergetool?

I only suggest an option, and not config support, because I
would be hesitant to make "rerere remaining" unconditionally
honor the orderFile since that feature is diff-centric, while
"rerere remaining" is a different beast altogether.  Adding a
flag is a middle-ground that would allow mergetool to drive it
while not suddently changing rerere's behavior.


The patches could then be:

1. switch to diff-files, add tests, and document how
   diff.orderFile affects mergetool.

2. Teach mergetool about the "-O" flag so that it can
   override the configuration, and add tests.  It could be
   argued that this should be squashed into (1).

3. (optional) teach "rerere remaining" to honor the
   -O flag and teach mergetool to supply the option.

Sound good?


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

Re: [ANNOUNCE] Git v2.10.0-rc0

[Torsten Bögershausen]

On Mon, Aug 15, 2016 at 08:42:48AM -0700, Junio C Hamano wrote:
> Torsten Bögershausen  writes:
> 
> > On 15.08.16 00:47, Junio C Hamano wrote:
> >> Torsten Bögershausen (1):
> >>   convert: unify the "auto" handling of CRLF
> >
> > Should we mention this change in the release notes?
> >
> > The handling of "*.text = auto" was changed, and now
> >
> > $ echo "* text=auto eol=crlf" >.gitattributes
> > has the same effect as
> > $ git config core.autocrlf true
> 
> Oh, definitely.  Thanks.
Hm, one question remains:
 git show 07c92928f2b782330df6e78
[]
+ * The handling of the "text = auto" attribute has been updated.
+   $ echo "* text=auto eol=crlf" >.gitattributes
+   used to have the same effect as
+   $ echo "* text=auto eol=crlf" >.gitattributes
Is this `text=auto` correct ?
I think it should be

   used to have the same effect as
   $ echo "* text eol=crlf" >.gitattributes

# In other words, the `auto` was ignored, as explained here:
+   $ git config core.eol crlf
+   i.e. declaring all files are text; the combination now is
+   equivalent to doing
+   $ git config core.autocrlf true
+
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] git-multimail: update to release 1.4.0

[Matthieu Moy]

Changes are described in CHANGES.

Contributions-by: Matthieu Moy 
Contributions-by: Irfan Adilovic 
Signed-off-by: Matthieu Moy 
---
 contrib/hooks/multimail/CHANGES |  59 ++
 contrib/hooks/multimail/CONTRIBUTING.rst|   9 +-
 contrib/hooks/multimail/README  |  85 ++-
 contrib/hooks/multimail/README.Git  |   4 +-
 contrib/hooks/multimail/doc/troubleshooting.rst |  34 +
 contrib/hooks/multimail/git_multimail.py| 810 
 6 files changed, 725 insertions(+), 276 deletions(-)

diff --git a/contrib/hooks/multimail/CHANGES b/contrib/hooks/multimail/CHANGES
index 100cc7a..2076cf9 100644
--- a/contrib/hooks/multimail/CHANGES
+++ b/contrib/hooks/multimail/CHANGES
@@ -1,3 +1,62 @@
+Release 1.4.0
+=
+
+New features to troubleshoot a git-multimail installation
+-
+
+* One can now perform a basic check of git-multimail's setup by
+  running the hook with the environment variable
+  GIT_MULTIMAIL_CHECK_SETUP set to a non-empty string. See
+  doc/troubleshooting.rst for details.
+
+* A new log files system was added. See the multimailhook.logFile,
+  multimailhook.errorLogFile and multimailhook.debugLogFile variables.
+
+* git_multimail.py can now be made more verbose using
+  multimailhook.verbose.
+
+* A new option --check-ref-filter is now available to help debugging
+  the refFilter* options.
+
+Formatting emails
+-
+
+* Formatting of emails was made slightly more compact, to reduce the
+  odds of having long subject lines truncated or wrapped in short list
+  of commits.
+
+* multimailhook.emailPrefix may now use the '%(repo_shortname)s'
+  placeholder for the repository's short name.
+
+* A new option multimailhook.subjectMaxLength is available to truncate
+  overly long subject lines.
+
+Bug fixes and minor changes
+---
+
+* Options refFilterDoSendRegex and refFilterDontSendRegex were
+  essentially broken. They should work now.
+
+* The behavior when both refFilter{Do,Dont}SendRegex and
+  refFilter{Exclusion,Inclusion}Regex are set have been slightly
+  changed. Exclusion/Inclusion is now strictly stronger than
+  DoSend/DontSend.
+
+* The management of precedence when a setting can be computed in
+  multiple ways has been considerably refactored and modified.
+  multimailhook.from and multimailhook.reponame now have precedence
+  over the environment-specific settings ($GL_REPO/$GL_USER for
+  gitolite, --stash-user/repo for Stash, --submitter/--project for
+  Gerrit).
+
+* The coverage of the testsuite has been considerably improved. All
+  configuration variables now appear at least once in the testsuite.
+
+This version was tested with Python 2.6 to 3.5. It also mostly works
+with Python 2.4, but there is one known breakage in the testsuite
+related to non-ascii characters. It was tested with Git
+1.7.10.406.gdc801, 1.8.5.6, 2.1.4, and 2.10.0.rc0.1.g07c9292.
+
 Release 1.3.1 (bugfix-only release)
 ===
 
diff --git a/contrib/hooks/multimail/CONTRIBUTING.rst 
b/contrib/hooks/multimail/CONTRIBUTING.rst
index 530ecbf..da65570 100644
--- a/contrib/hooks/multimail/CONTRIBUTING.rst
+++ b/contrib/hooks/multimail/CONTRIBUTING.rst
@@ -4,8 +4,9 @@ Contributing
 git-multimail is an open-source project, built by volunteers. We would
 welcome your help!
 
-The current maintainers are Michael Haggerty 
-and Matthieu Moy .
+The current maintainers are Matthieu Moy
+ and Michael Haggerty
+.
 
 Please note that although a copy of git-multimail is distributed in
 the "contrib" section of the main Git project, development takes place
@@ -22,6 +23,10 @@ to the maintainers). Please sign off your patches as per the 
`Git
 project practice
 
`__.
 
+Please vote for issues you would like to be addressed in priority
+(click "add your reaction" and then the "+1" thumbs-up button on the
+GitHub issue).
+
 General discussion of git-multimail can take place on the main `Git
 mailing list`_.
 
diff --git a/contrib/hooks/multimail/README b/contrib/hooks/multimail/README
index 22a23cd..5105373 100644
--- a/contrib/hooks/multimail/README
+++ b/contrib/hooks/multimail/README
@@ -1,11 +1,11 @@
-git-multimail 1.3.1
-===
+git-multimail version 1.4.0
+===
 
 .. image:: https://travis-ci.org/git-multimail/git-multimail.svg?branch=master
 :target: https://travis-ci.org/git-multimail/git-multimail
 
 git-multimail is a tool for sending notification emails on pushes to a
-Git repository.  It includes a Python module called git_multimail.py,
+Git repository.  It includes a Python module called ``git_multimail.py``,
 which can either be used as a 

Re: git-mergetool reverse file ordering

[Johannes Sixt]

Am 17.08.2016 um 08:05 schrieb Johannes Sixt:

Am 17.08.2016 um 03:25 schrieb David Aguilar:

Hmm, I do like the idea of reusing the diff orderFile, but a
mechanism for sorting arbitrary inputs based on the orderFile
isn't currently exposed in a way that mergetool could use it.


Instead of using 'git ls-files -u | sed ... | sort -u' you could use

  git diff-files --name-status -O... | sed -ne '/^U/s/^..//p'


Or even better:

git diff-files --name-only --diff-filter=U -O...




But, that sort is honestly kinda crude.  It can't implement the
interesting case where we want bar.h to sort before bar.c but
not foo.h.

If we did the sort option, we could have both.


I don't think that we need a new filter when the diff machinery is
capable of re-ordering files. We should enhance the diff machinery instead.

-- Hannes



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

Re: git-mergetool reverse file ordering

[Johannes Sixt]

Am 17.08.2016 um 03:25 schrieb David Aguilar:

Hmm, I do like the idea of reusing the diff orderFile, but a
mechanism for sorting arbitrary inputs based on the orderFile
isn't currently exposed in a way that mergetool could use it.


Instead of using 'git ls-files -u | sed ... | sort -u' you could use

  git diff-files --name-status -O... | sed -ne '/^U/s/^..//p'


But, that sort is honestly kinda crude.  It can't implement the
interesting case where we want bar.h to sort before bar.c but
not foo.h.

If we did the sort option, we could have both.


I don't think that we need a new filter when the diff machinery is 
capable of re-ordering files. We should enhance the diff machinery instead.


-- Hannes

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