Re: [PATCH v3 16/27] attr: convert git_all_attrs() to use "struct attr_check"

[Brandon Williams]

On 01/28, Stefan Beller wrote:
> 
> This being moved down to below (being review churn) sounds like a
> rebase mistake. ;)
> 

Yep, thanks for catching that.  I'll need to fix that up.

-- 
Brandon Williams

git-daemon shallow checkout fail

[Bob Proulx]

I am trying to understand a problem with shallow checkouts through the
git-daemon.  The server side fails trying to create a shallow_XX
file in the repository.  But of course it can't due to no permissions
from the git-daemon user.

However the problem driving me crazy is that this only fails this way
on one machine.  Unfortunately failing on the machine I need to use.
If I try this same setup on any other machine I try then there is no
failure and it works okay.  Therefore I conclude that in the failing
case it is trying to write a shallow_XX file in the repository but
in all of the passing cases it does not.  I browsed through the
git-daemon source but couldn't deduce the flow yet.

Does anyone know why one system would try to create shallow_XX
files in the repository while another one would not?

Trying to be unambiguous here is my test case:

  mkdir /run/git
  chmod 755 /run/git
  chown nobody:root /run/git
  cd /run/git && env -i HOME=/run/git PATH=/usr/local/bin:/usr/bin:/bin su -s 
/bin/sh -c "git-daemon --export-all --base-path=/srv/git --verbose" nobody
  [18340] Ready to rumble

That sets up the test case.  Have any bare git repository in /srv/git
for use for cloning.

  git clone --depth 1 git://localhost/test-project.git
  Cloning into 'test-project'...
  fatal: The remote end hung up unexpectedly
  fatal: early EOF
  fatal: index-pack failed

And the server side says:

  [26071] Request upload-pack for '/test-project.git'
  [26071] fatal: Unable to create temporary file 
'/srv/git/test-project.git/shallow_xKwnvZ': Permission denied
  [26055] [26071] Disconnected (with error)

Of course git-daemon running as nobody can't create a temporary file
shallow_XX in the /srv/git/test-project.git because it has no
permissions by design.  But why does this work on other systems and
not work on my target system?

  git --version  # from today's git clone build
  git version 2.11.0.485.g4e59582

Thanks!
Bob

Re: [PATCH v3 16/27] attr: convert git_all_attrs() to use "struct attr_check"

[Stefan Beller]

On Fri, Jan 27, 2017 at 6:01 PM, Brandon Williams  wrote:
> From: Junio C Hamano 
>
> This updates the other two ways the attribute check is done via an
> array of "struct attr_check_item" elements.  These two niches
> appear only in "git check-attr".
>
>  * The caller does not know offhand what attributes it wants to ask
>about and cannot use attr_check_initl() to prepare the
>attr_check structure.
>
>  * The caller may not know what attributes it wants to ask at all,
>and instead wants to learn everything that the given path has.
>
> Such a caller can call attr_check_alloc() to allocate an empty
> attr_check, and then call attr_check_append() to add attribute names
> one by one.
>
> Signed-off-by: Junio C Hamano 
> Signed-off-by: Stefan Beller 
> Signed-off-by: Brandon Williams 
> ---
>  attr.c   | 168 
> ---
>  attr.h   |   9 +--
>  builtin/check-attr.c |  60 +-
>  3 files changed, 112 insertions(+), 125 deletions(-)
>
> diff --git a/attr.c b/attr.c
> index de8bf35a3..40818246f 100644
> --- a/attr.c
> +++ b/attr.c
> @@ -132,75 +132,6 @@ struct git_attr *git_attr(const char *name)
> return git_attr_internal(name, strlen(name));
>  }
>
> -struct attr_check *attr_check_alloc(void)
> -{
> -   return xcalloc(1, sizeof(struct attr_check));
> -}
> -
> -struct attr_check *attr_check_initl(const char *one, ...)
> -{
> -   struct attr_check *check;
> -   int cnt;
> -   va_list params;
> -   const char *param;
> -
> -   va_start(params, one);
> -   for (cnt = 1; (param = va_arg(params, const char *)) != NULL; cnt++)
> -   ;
> -   va_end(params);
> -
> -   check = attr_check_alloc();
> -   check->nr = cnt;
> -   check->alloc = cnt;
> -   check->items = xcalloc(cnt, sizeof(struct attr_check_item));
> -
> -   check->items[0].attr = git_attr(one);
> -   va_start(params, one);
> -   for (cnt = 1; cnt < check->nr; cnt++) {
> -   const struct git_attr *attr;
> -   param = va_arg(params, const char *);
> -   if (!param)
> -   die("BUG: counted %d != ended at %d",
> -   check->nr, cnt);
> -   attr = git_attr(param);
> -   if (!attr)
> -   die("BUG: %s: not a valid attribute name", param);
> -   check->items[cnt].attr = attr;
> -   }
> -   va_end(params);
> -   return check;
> -}

This being moved down to below (being review churn) sounds like a
rebase mistake. ;)

> -
> -struct attr_check_item *attr_check_append(struct attr_check *check,
> - const struct git_attr *attr)
> -{
> -   struct attr_check_item *item;
> -
> -   ALLOC_GROW(check->items, check->nr + 1, check->alloc);
> -   item = >items[check->nr++];
> -   item->attr = attr;
> -   return item;
> -}
> -
> -void attr_check_reset(struct attr_check *check)
> -{
> -   check->nr = 0;
> -}
> -
> -void attr_check_clear(struct attr_check *check)
> -{
> -   free(check->items);
> -   check->items = NULL;
> -   check->alloc = 0;
> -   check->nr = 0;
> -}
> -
> -void attr_check_free(struct attr_check *check)
> -{
> -   attr_check_clear(check);
> -   free(check);
> -}
> -
>  /* What does a matched pattern decide? */
>  struct attr_state {
> struct git_attr *attr;
> @@ -439,6 +370,75 @@ static void free_attr_elem(struct attr_stack *e)
> free(e);
>  }
>
> +struct attr_check *attr_check_alloc(void)
> +{
> +   return xcalloc(1, sizeof(struct attr_check));
> +}
> +
> +struct attr_check *attr_check_initl(const char *one, ...)
> +{
> +   struct attr_check *check;
> +   int cnt;
> +   va_list params;
> +   const char *param;
> +
> +   va_start(params, one);
> +   for (cnt = 1; (param = va_arg(params, const char *)) != NULL; cnt++)
> +   ;
> +   va_end(params);
> +
> +   check = attr_check_alloc();
> +   check->nr = cnt;
> +   check->alloc = cnt;
> +   check->items = xcalloc(cnt, sizeof(struct attr_check_item));
> +
> +   check->items[0].attr = git_attr(one);
> +   va_start(params, one);
> +   for (cnt = 1; cnt < check->nr; cnt++) {
> +   const struct git_attr *attr;
> +   param = va_arg(params, const char *);
> +   if (!param)
> +   die("BUG: counted %d != ended at %d",
> +   check->nr, cnt);
> +   attr = git_attr(param);
> +   if (!attr)
> +   die("BUG: %s: not a valid attribute name", param);
> +   check->items[cnt].attr = attr;
> +   }
> +   va_end(params);
> +   return check;
> +}
> +
> +struct attr_check_item *attr_check_append(struct attr_check 

Re: [PATCH 1/3] Documentation/stash: remove mention of git reset --hard

[Jeff King]

On Sat, Jan 28, 2017 at 07:30:28PM +, Thomas Gummerer wrote:

> Thanks all who chimed in here.  My new description is definitely not
> right.  The reason I wanted to change it is part because it's an
> implementation detail, and part because it's going to be not quite
> right when the filename argument is introduced.
> 
> How about the following:
> 
>   Save your local modifications to a new 'stash' and roll them back
>   both in the working tree and in the index.
> 
> As an added bonus this also matches what git stash save -p does.

IMHO that is both informative and accurate. You could add:
 
  (unless --keep-index was used)

at the end of the sentence, though I am not sure it is necessary.

-Peff

[PATCH] use oid_to_hex_r() for converting struct object_id hashes to hex strings

[René Scharfe]

Patch generated by Coccinelle and contrib/coccinelle/object_id.cocci.

Signed-off-by: Rene Scharfe 
---
 builtin/blame.c   | 4 ++--
 builtin/merge-index.c | 2 +-
 builtin/rev-list.c| 2 +-
 diff.c| 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 126b8c9e5b..cffc626540 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1901,7 +1901,7 @@ static void emit_porcelain(struct scoreboard *sb, struct 
blame_entry *ent,
struct origin *suspect = ent->suspect;
char hex[GIT_SHA1_HEXSZ + 1];
 
-   sha1_to_hex_r(hex, suspect->commit->object.oid.hash);
+   oid_to_hex_r(hex, >commit->object.oid);
printf("%s %d %d %d\n",
   hex,
   ent->s_lno + 1,
@@ -1941,7 +1941,7 @@ static void emit_other(struct scoreboard *sb, struct 
blame_entry *ent, int opt)
int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
 
get_commit_info(suspect->commit, , 1);
-   sha1_to_hex_r(hex, suspect->commit->object.oid.hash);
+   oid_to_hex_r(hex, >commit->object.oid);
 
cp = nth_line(sb, ent->lno);
for (cnt = 0; cnt < ent->num_lines; cnt++) {
diff --git a/builtin/merge-index.c b/builtin/merge-index.c
index ce356b1da1..2d1b6db6bd 100644
--- a/builtin/merge-index.c
+++ b/builtin/merge-index.c
@@ -22,7 +22,7 @@ static int merge_entry(int pos, const char *path)
if (strcmp(ce->name, path))
break;
found++;
-   sha1_to_hex_r(hexbuf[stage], ce->oid.hash);
+   oid_to_hex_r(hexbuf[stage], >oid);
xsnprintf(ownbuf[stage], sizeof(ownbuf[stage]), "%o", 
ce->ce_mode);
arguments[stage] = hexbuf[stage];
arguments[stage + 4] = ownbuf[stage];
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index c43decda70..0aa93d5891 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -237,7 +237,7 @@ static int show_bisect_vars(struct rev_list_info *info, int 
reaches, int all)
cnt = reaches;
 
if (revs->commits)
-   sha1_to_hex_r(hex, revs->commits->item->object.oid.hash);
+   oid_to_hex_r(hex, >commits->item->object.oid);
 
if (flags & BISECT_SHOW_ALL) {
traverse_commit_list(revs, show_commit, show_object, info);
diff --git a/diff.c b/diff.c
index f08cd8e033..d91a344908 100644
--- a/diff.c
+++ b/diff.c
@@ -3015,7 +3015,7 @@ static struct diff_tempfile *prepare_temp_file(const char 
*name,
if (!one->oid_valid)
sha1_to_hex_r(temp->hex, null_sha1);
else
-   sha1_to_hex_r(temp->hex, one->oid.hash);
+   oid_to_hex_r(temp->hex, >oid);
/* Even though we may sometimes borrow the
 * contents from the work tree, we always want
 * one->mode.  mode is trustworthy even when
-- 
2.11.0

[PATCH] checkout: convert post_checkout_hook() to struct object_id

[René Scharfe]

Signed-off-by: Rene Scharfe 
---
 builtin/checkout.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index bfe685c198..80d5e38981 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -56,8 +56,8 @@ static int post_checkout_hook(struct commit *old, struct 
commit *new,
  int changed)
 {
return run_hook_le(NULL, "post-checkout",
-  sha1_to_hex(old ? old->object.oid.hash : null_sha1),
-  sha1_to_hex(new ? new->object.oid.hash : null_sha1),
+  oid_to_hex(old ? >object.oid : _oid),
+  oid_to_hex(new ? >object.oid : _oid),
   changed ? "1" : "0", NULL);
/* "new" can be NULL when checking out from the index before
   a commit exists. */
-- 
2.11.0

[PATCH] use oidcpy() for copying hashes between instances of struct object_id

[René Scharfe]

Patch generated by Coccinelle and contrib/coccinelle/object_id.cocci.

Signed-off-by: Rene Scharfe 
---
 refs/files-backend.c | 2 +-
 wt-status.c  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index f9023939d5..8ee2aba39f 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -697,7 +697,7 @@ static int cache_ref_iterator_peel(struct ref_iterator 
*ref_iterator,
 
if (peel_entry(entry, 0))
return -1;
-   hashcpy(peeled->hash, entry->u.value.peeled.hash);
+   oidcpy(peeled, >u.value.peeled);
return 0;
 }
 
diff --git a/wt-status.c b/wt-status.c
index a715e71906..a05328dc48 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -628,7 +628,7 @@ static void wt_status_collect_changes_initial(struct 
wt_status *s)
d->index_status = DIFF_STATUS_ADDED;
/* Leave {mode,oid}_head zero for adds. */
d->mode_index = ce->ce_mode;
-   hashcpy(d->oid_index.hash, ce->oid.hash);
+   oidcpy(>oid_index, >oid);
}
}
 }
@@ -2096,7 +2096,7 @@ static void wt_porcelain_v2_print_unmerged_entry(
if (strcmp(ce->name, it->string) || !stage)
break;
stages[stage - 1].mode = ce->ce_mode;
-   hashcpy(stages[stage - 1].oid.hash, ce->oid.hash);
+   oidcpy([stage - 1].oid, >oid);
sum |= (1 << (stage - 1));
}
if (sum != d->stagemask)
-- 
2.11.0

[PATCH 5/5] graph: use SWAP macro

[René Scharfe]

Exchange the values of graph->columns and graph->new_columns using the
macro SWAP instead of hand-rolled code.  The result is shorter and
easier to read.

This transformation was not done by the semantic patch swap.cocci
because there's an unrelated statement between the second and the last
step of the exchange, so it didn't match the expected pattern.

Signed-off-by: Rene Scharfe 
---
 graph.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/graph.c b/graph.c
index 4c722303d2..29b0f51dc5 100644
--- a/graph.c
+++ b/graph.c
@@ -463,7 +463,6 @@ static void graph_update_width(struct git_graph *graph,
 static void graph_update_columns(struct git_graph *graph)
 {
struct commit_list *parent;
-   struct column *tmp_columns;
int max_new_columns;
int mapping_idx;
int i, seen_this, is_commit_in_columns;
@@ -476,11 +475,8 @@ static void graph_update_columns(struct git_graph *graph)
 * We'll re-use the old columns array as storage to compute the new
 * columns list for the commit after this one.
 */
-   tmp_columns = graph->columns;
-   graph->columns = graph->new_columns;
+   SWAP(graph->columns, graph->new_columns);
graph->num_columns = graph->num_new_columns;
-
-   graph->new_columns = tmp_columns;
graph->num_new_columns = 0;
 
/*
-- 
2.11.0

[PATCH 4/5] diff: use SWAP macro

[René Scharfe]

Use the macro SWAP to exchange the value of pairs of variables instead
of swapping them manually with the help of a temporary variable.  The
resulting code is shorter and easier to read.

The two cases were not transformed by the semantic patch swap.cocci
because it's extra careful and handles only cases where the types of all
variables are the same -- and here we swap two ints and use an unsigned
temporary variable for that.  Nevertheless the conversion is safe, as
the value range is preserved with and without the patch.

Signed-off-by: Rene Scharfe 
---
 diff-no-index.c | 3 +--
 diff.c  | 4 +---
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/diff-no-index.c b/diff-no-index.c
index 1ae09894d7..df762fd0f7 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -185,8 +185,7 @@ static int queue_diff(struct diff_options *o,
struct diff_filespec *d1, *d2;
 
if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
-   unsigned tmp;
-   tmp = mode1; mode1 = mode2; mode2 = tmp;
+   SWAP(mode1, mode2);
SWAP(name1, name2);
}
 
diff --git a/diff.c b/diff.c
index 9de1ba264f..6c4f3f6b72 100644
--- a/diff.c
+++ b/diff.c
@@ -5117,11 +5117,9 @@ void diff_change(struct diff_options *options,
return;
 
if (DIFF_OPT_TST(options, REVERSE_DIFF)) {
-   unsigned tmp;
SWAP(old_mode, new_mode);
SWAP(old_sha1, new_sha1);
-   tmp = old_sha1_valid; old_sha1_valid = new_sha1_valid;
-   new_sha1_valid = tmp;
+   SWAP(old_sha1_valid, new_sha1_valid);
SWAP(old_dirty_submodule, new_dirty_submodule);
}
 
-- 
2.11.0

[PATCH 3/5] use SWAP macro

[René Scharfe]

Apply the semantic patch swap.cocci to convert hand-rolled swaps to use
the macro SWAP.  The resulting code is shorter and easier to read, the
object code is effectively unchanged.

The patch for object.c had to be hand-edited in order to preserve the
comment before the change; Coccinelle tried to eat it for some reason.

Signed-off-by: Rene Scharfe 
---
 builtin/diff-tree.c | 4 +---
 builtin/diff.c  | 9 +++--
 diff-no-index.c | 3 +--
 diff.c  | 8 +++-
 graph.c | 5 +
 line-range.c| 3 +--
 merge-recursive.c   | 5 +
 object.c| 4 +---
 pack-revindex.c | 5 +
 prio-queue.c| 4 +---
 strbuf.h| 4 +---
 11 files changed, 15 insertions(+), 39 deletions(-)

diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index 806dd7a885..8ce00480cd 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -147,9 +147,7 @@ int cmd_diff_tree(int argc, const char **argv, const char 
*prefix)
tree1 = opt->pending.objects[0].item;
tree2 = opt->pending.objects[1].item;
if (tree2->flags & UNINTERESTING) {
-   struct object *tmp = tree2;
-   tree2 = tree1;
-   tree1 = tmp;
+   SWAP(tree2, tree1);
}
diff_tree_sha1(tree1->oid.hash,
   tree2->oid.hash,
diff --git a/builtin/diff.c b/builtin/diff.c
index 7f91f6d226..3d64b85337 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -45,12 +45,9 @@ static void stuff_change(struct diff_options *opt,
return;
 
if (DIFF_OPT_TST(opt, REVERSE_DIFF)) {
-   unsigned tmp;
-   const unsigned char *tmp_u;
-   const char *tmp_c;
-   tmp = old_mode; old_mode = new_mode; new_mode = tmp;
-   tmp_u = old_sha1; old_sha1 = new_sha1; new_sha1 = tmp_u;
-   tmp_c = old_name; old_name = new_name; new_name = tmp_c;
+   SWAP(old_mode, new_mode);
+   SWAP(old_sha1, new_sha1);
+   SWAP(old_name, new_name);
}
 
if (opt->prefix &&
diff --git a/diff-no-index.c b/diff-no-index.c
index f420786039..1ae09894d7 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -186,9 +186,8 @@ static int queue_diff(struct diff_options *o,
 
if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
unsigned tmp;
-   const char *tmp_c;
tmp = mode1; mode1 = mode2; mode2 = tmp;
-   tmp_c = name1; name1 = name2; name2 = tmp_c;
+   SWAP(name1, name2);
}
 
d1 = noindex_filespec(name1, mode1);
diff --git a/diff.c b/diff.c
index f08cd8e033..9de1ba264f 100644
--- a/diff.c
+++ b/diff.c
@@ -5118,13 +5118,11 @@ void diff_change(struct diff_options *options,
 
if (DIFF_OPT_TST(options, REVERSE_DIFF)) {
unsigned tmp;
-   const unsigned char *tmp_c;
-   tmp = old_mode; old_mode = new_mode; new_mode = tmp;
-   tmp_c = old_sha1; old_sha1 = new_sha1; new_sha1 = tmp_c;
+   SWAP(old_mode, new_mode);
+   SWAP(old_sha1, new_sha1);
tmp = old_sha1_valid; old_sha1_valid = new_sha1_valid;
new_sha1_valid = tmp;
-   tmp = old_dirty_submodule; old_dirty_submodule = 
new_dirty_submodule;
-   new_dirty_submodule = tmp;
+   SWAP(old_dirty_submodule, new_dirty_submodule);
}
 
if (options->prefix &&
diff --git a/graph.c b/graph.c
index d4e8519c90..4c722303d2 100644
--- a/graph.c
+++ b/graph.c
@@ -997,7 +997,6 @@ static void graph_output_post_merge_line(struct git_graph 
*graph, struct strbuf
 static void graph_output_collapsing_line(struct git_graph *graph, struct 
strbuf *sb)
 {
int i;
-   int *tmp_mapping;
short used_horizontal = 0;
int horizontal_edge = -1;
int horizontal_edge_target = -1;
@@ -1132,9 +1131,7 @@ static void graph_output_collapsing_line(struct git_graph 
*graph, struct strbuf
/*
 * Swap mapping and new_mapping
 */
-   tmp_mapping = graph->mapping;
-   graph->mapping = graph->new_mapping;
-   graph->new_mapping = tmp_mapping;
+   SWAP(graph->mapping, graph->new_mapping);
 
/*
 * If graph->mapping indicates that all of the branch lines
diff --git a/line-range.c b/line-range.c
index de4e32f942..323399d16c 100644
--- a/line-range.c
+++ b/line-range.c
@@ -269,8 +269,7 @@ int parse_range_arg(const char *arg, nth_line_fn_t 
nth_line_cb,
return -1;
 
if (*begin && *end && *end < *begin) {
-   long tmp;
-   tmp = *end; *end = *begin; *begin = tmp;
+   SWAP(*end, *begin);
}
 
return 0;
diff --git a/merge-recursive.c b/merge-recursive.c
index 

[PATCH 2/5] apply: use SWAP macro

[René Scharfe]

Use the exported macro SWAP instead of the file-scoped macro swap and
remove the latter's definition.

Signed-off-by: Rene Scharfe 
---
 apply.c | 23 +++
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/apply.c b/apply.c
index 2ed808d429..0e2caeab9c 100644
--- a/apply.c
+++ b/apply.c
@@ -2187,29 +2187,20 @@ static int parse_chunk(struct apply_state *state, char 
*buffer, unsigned long si
return offset + hdrsize + patchsize;
 }
 
-#define swap(a,b) myswap((a),(b),sizeof(a))
-
-#define myswap(a, b, size) do {\
-   unsigned char mytmp[size];  \
-   memcpy(mytmp, , size);\
-   memcpy(, , size);   \
-   memcpy(, mytmp, size);\
-} while (0)
-
 static void reverse_patches(struct patch *p)
 {
for (; p; p = p->next) {
struct fragment *frag = p->fragments;
 
-   swap(p->new_name, p->old_name);
-   swap(p->new_mode, p->old_mode);
-   swap(p->is_new, p->is_delete);
-   swap(p->lines_added, p->lines_deleted);
-   swap(p->old_sha1_prefix, p->new_sha1_prefix);
+   SWAP(p->new_name, p->old_name);
+   SWAP(p->new_mode, p->old_mode);
+   SWAP(p->is_new, p->is_delete);
+   SWAP(p->lines_added, p->lines_deleted);
+   SWAP(p->old_sha1_prefix, p->new_sha1_prefix);
 
for (; frag; frag = frag->next) {
-   swap(frag->newpos, frag->oldpos);
-   swap(frag->newlines, frag->oldlines);
+   SWAP(frag->newpos, frag->oldpos);
+   SWAP(frag->newlines, frag->oldlines);
}
}
 }
-- 
2.11.0

[PATCH 1/5] add SWAP macro

[René Scharfe]

Add a macro for exchanging the values of variables.  It allows users
to avoid repetition and takes care of the temporary variable for them.
It also makes sure that the storage sizes of its two parameters are the
same.  Its memcpy(1) calls are optimized away by current compilers.

Also add a conservative semantic patch for transforming only swaps of
variables of the same type.

Signed-off-by: Rene Scharfe 
---
 contrib/coccinelle/swap.cocci | 28 
 git-compat-util.h | 10 ++
 2 files changed, 38 insertions(+)
 create mode 100644 contrib/coccinelle/swap.cocci

diff --git a/contrib/coccinelle/swap.cocci b/contrib/coccinelle/swap.cocci
new file mode 100644
index 00..a0934d1fda
--- /dev/null
+++ b/contrib/coccinelle/swap.cocci
@@ -0,0 +1,28 @@
+@ swap_with_declaration @
+type T;
+identifier tmp;
+T a, b;
+@@
+- T tmp = a;
++ T tmp;
++ tmp = a;
+  a = b;
+  b = tmp;
+
+@ swap @
+type T;
+T tmp, a, b;
+@@
+- tmp = a;
+- a = b;
+- b = tmp;
++ SWAP(a, b);
+
+@ extends swap @
+identifier unused;
+@@
+  {
+  ...
+- T unused;
+  ... when != unused
+  }
diff --git a/git-compat-util.h b/git-compat-util.h
index 87237b092b..66cd466eea 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -527,6 +527,16 @@ static inline int ends_with(const char *str, const char 
*suffix)
return strip_suffix(str, suffix, );
 }
 
+#define SWAP(a, b) do {\
+   void *_swap_a_ptr = &(a);   \
+   void *_swap_b_ptr = &(b);   \
+   unsigned char _swap_buffer[sizeof(a)];  \
+   memcpy(_swap_buffer, _swap_a_ptr, sizeof(a));   \
+   memcpy(_swap_a_ptr, _swap_b_ptr, sizeof(a) +\
+  BUILD_ASSERT_OR_ZERO(sizeof(a) == sizeof(b)));   \
+   memcpy(_swap_b_ptr, _swap_buffer, sizeof(a));   \
+} while (0)
+
 #if defined(NO_MMAP) || defined(USE_WIN32_MMAP)
 
 #ifndef PROT_READ
-- 
2.11.0

[PATCH 0/5] introduce SWAP macro

[René Scharfe]

Exchanging the value of two variables requires declaring a temporary
variable and repeating their names.  The swap macro in apply.c
simplifies this for its callers without changing the compiled binary.
Polish this macro and export it, then use it throughout the code to
reduce repetition and hide the declaration of temporary variables.

  add SWAP macro
  apply: use SWAP macro
  use SWAP macro
  diff: use SWAP macro
  graph: use SWAP macro

 apply.c   | 23 +++
 builtin/diff-tree.c   |  4 +---
 builtin/diff.c|  9 +++--
 contrib/coccinelle/swap.cocci | 28 
 diff-no-index.c   |  6 ++
 diff.c| 12 
 git-compat-util.h | 10 ++
 graph.c   | 11 ++-
 line-range.c  |  3 +--
 merge-recursive.c |  5 +
 object.c  |  4 +---
 pack-revindex.c   |  5 +
 prio-queue.c  |  4 +---
 strbuf.h  |  4 +---
 14 files changed, 63 insertions(+), 65 deletions(-)
 create mode 100644 contrib/coccinelle/swap.cocci

-- 
2.11.0

[PATCH] merge-recursive: make "CONFLICT (rename/delete)" message show both paths

[Matt McCutchen]

The current message printed by "git merge-recursive" for a rename/delete
conflict is like this:

CONFLICT (rename/delete): new-path deleted in HEAD and renamed in
other-branch. Version other-branch of new-path left in tree.

To be more helpful, the message should show both paths of the rename and
state that the deletion occurred at the old path, not the new path.  So
change the message to the following format:

CONFLICT (rename/delete): old-path deleted in HEAD and renamed to
new-path in other-branch. Version other-branch of new-path left in tree.

Since this doubles the number of cases in handle_change_delete (modify vs.
rename), refactor the code to halve the number of cases again by merging the
cases where o->branch1 has the change and o->branch2 has the delete with the
cases that are the other way around.

Also add a simple test of the new conflict message.

Signed-off-by: Matt McCutchen 
---

This came up at:

https://github.com/cristibalan/braid/issues/41#issuecomment-275826716

Please check that my refactoring is indeed correct!  All the existing tests pass
for me, but the existing test coverage of these conflict messages looks poor.

 merge-recursive.c  | 117 ++---
 t/t6045-merge-rename-delete.sh |  23 
 2 files changed, 86 insertions(+), 54 deletions(-)
 create mode 100755 t/t6045-merge-rename-delete.sh

diff --git a/merge-recursive.c b/merge-recursive.c
index d327209..e8fce10 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1061,16 +1061,20 @@ static int merge_file_one(struct merge_options *o,
 }
 
 static int handle_change_delete(struct merge_options *o,
-const char *path,
+const char *path, const char *old_path,
 const struct object_id *o_oid, int o_mode,
-const struct object_id *a_oid, int a_mode,
-const struct object_id *b_oid, int b_mode,
+const struct object_id *changed_oid,
+int changed_mode,
+const char *change_branch,
+const char *delete_branch,
 const char *change, const char *change_past)
 {
-   char *renamed = NULL;
+   char *alt_path = NULL;
+   const char *update_path = path;
int ret = 0;
+
if (dir_in_way(path, !o->call_depth, 0)) {
-   renamed = unique_path(o, path, a_oid ? o->branch1 : o->branch2);
+   update_path = alt_path = unique_path(o, path, change_branch);
}
 
if (o->call_depth) {
@@ -1081,43 +1085,43 @@ static int handle_change_delete(struct merge_options *o,
 */
ret = remove_file_from_cache(path);
if (!ret)
-   ret = update_file(o, 0, o_oid, o_mode,
- renamed ? renamed : path);
-   } else if (!a_oid) {
-   if (!renamed) {
-   output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
-  "and %s in %s. Version %s of %s left in tree."),
-  change, path, o->branch1, change_past,
-  o->branch2, o->branch2, path);
-   ret = update_file(o, 0, b_oid, b_mode, path);
-   } else {
-   output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
-  "and %s in %s. Version %s of %s left in tree at 
%s."),
-  change, path, o->branch1, change_past,
-  o->branch2, o->branch2, path, renamed);
-   ret = update_file(o, 0, b_oid, b_mode, renamed);
-   }
+   ret = update_file(o, 0, o_oid, o_mode, update_path);
} else {
-   if (!renamed) {
-   output(o, 1, _("CONFLICT (%s/delete): %s deleted in %s "
-  "and %s in %s. Version %s of %s left in tree."),
-  change, path, o->branch2, change_past,
-  o->branch1, o->branch1, path);
+   if (!alt_path) {
+   if (!old_path) {
+   output(o, 1, _("CONFLICT (%s/delete): %s 
deleted in %s "
+  "and %s in %s. Version %s of %s left in 
tree."),
+  change, path, delete_branch, change_past,
+  change_branch, change_branch, path);
+   } else {
+   output(o, 1, _("CONFLICT (%s/delete): %s 
deleted in %s "
+  "and %s to %s in %s. Version %s of %s 
left in tree."),
+  change, old_path, 

[PATCH] t0001: don't let a default ACL interfere with the umask test

[Matt McCutchen]

The "init creates a new deep directory (umask vs. shared)" test expects
the permissions of newly created files to be based on the umask, which
fails if a default ACL is inherited from the working tree for git.  So
attempt to remove a default ACL if there is one.  Same idea as
8ed0a740dd42bd0724aebed6e3b07c4ea2a2d5e8.  (I guess I'm the only one who
ever runs the test suite with a default ACL set.)

Signed-off-by: Matt McCutchen 
---
 t/t0001-init.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index b8fc588..e424de5 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -258,6 +258,9 @@ test_expect_success POSIXPERM 'init creates a new deep 
directory (umask vs. shar
(
# Leading directories should honor umask while
# the repository itself should follow "shared"
+   mkdir newdir &&
+   # Remove a default ACL if possible.
+   (setfacl -k newdir 2>/dev/null || true) &&
umask 002 &&
git init --bare --shared=0660 newdir/a/b/c &&
test_path_is_dir newdir/a/b/c/refs &&
-- 
2.9.3

Re: [PATCH 1/3] Documentation/stash: remove mention of git reset --hard

[Thomas Gummerer]

On 01/25, Junio C Hamano wrote:
> Jeff King  writes:
> 
> > On Sat, Jan 21, 2017 at 08:08:02PM +, Thomas Gummerer wrote:
> >
> >> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
> >> index 2e9cef06e6..0ad5335a3e 100644
> >> --- a/Documentation/git-stash.txt
> >> +++ b/Documentation/git-stash.txt
> >> @@ -47,8 +47,9 @@ OPTIONS
> >>  
> >>  save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] 
> >> [-a|--all] [-q|--quiet] []::
> >>  
> >> -  Save your local modifications to a new 'stash', and run `git reset
> >> -  --hard` to revert them.  The  part is optional and gives
> >> +  Save your local modifications to a new 'stash', and revert the
> >> +  the changes in the working tree to match the index.
> >> +  The  part is optional and gives
> >
> > Hrm. "git reset --hard" doesn't just make the working tree match the
> > index. It also resets the index to HEAD.  So either the original or your
> > new description is wrong.
> >
> > I think it's the latter. We really do reset the index unless
> > --keep-index is specified.
> 
> Correct.  So the patch is a net loss.  Perhaps not requiring readers
> to know "reset --hard" might be an improvement (I personally do not
> think so), but this loses crucial information from the description.
> 
>   Save your local modifications (both in the working tree and
>   to the index) to a new 'stash', and resets the index to HEAD
>   and makes the working tree match the index (i.e. runs "git
>   reset --hard").
> 
> That's one-and-a-half lines longer than the original, though.

Thanks all who chimed in here.  My new description is definitely not
right.  The reason I wanted to change it is part because it's an
implementation detail, and part because it's going to be not quite
right when the filename argument is introduced.

How about the following:

Save your local modifications to a new 'stash' and roll them back
both in the working tree and in the index.

As an added bonus this also matches what git stash save -p does.

Re: show all merge conflicts

[Jeff King]

On Fri, Jan 27, 2017 at 09:42:41PM -0800, G. Sylvie Davies wrote:

> Aside from the usual "git log -cc", I think this should work (replace
> HEAD with whichever commit you are analyzing):
> 
> git diff --name-only HEAD^2...HEAD^1 > m1
> git diff --name-only HEAD^1...HEAD^2 > b1
> git diff --name-only HEAD^1..HEAD> m2
> git diff --name-only HEAD^2..HEAD> b2
> 
> If files listed between m1 and b2 differ, then the merge is dirty.
> Similarly for m2 and b1.
> 
> More information here:
> 
> http://stackoverflow.com/questions/27683077/how-do-you-detect-an-evil-merge-in-git/41356308#41356308

I don't think that can reliably find evil merges, since it looks at the
file level. If you had one hunk resolved for "theirs" and one hunk for
"ours" in a given file, then the file will be listed in each diff,
whether it has evil hunks or not.

I don't think this is just about evil merges, though. For instance,
try:

  seq 1 10 >file
  git add file
  git commit -m base

  sed s/4/master/ tmp && mv tmp file
  git commit -am master

  git checkout -b other HEAD^
  sed s/4/other/ tmp && mv tmp file
  git commit -am other

  git merge master
  git checkout --ours file
  git commit -am merged

  merge=$(git rev-parse HEAD)

The question is: were there conflicts in $merge, and how were they
resolved?

That isn't an evil merge, but there's still something interesting to
show that "git log --cc" won't display.

Replaying the merge like:

  git checkout $merge^1
  git merge $merge^2
  git diff -R $merge

shows you the patch to go from the conflict state to the final one.

-Peff

INFO

[THANDI ROBERT]

Hello my name is Ms. Thandi Robert, from Ivory Coast. My parents were brutally 
mulled by the former president Laurent Gbagbo because of political crisis as 
the only survival of my family. I got your email while searching for a reliable 
personality in my private study on the internet. I am in need of your help and 
stand as my guardian in the management of my family inherited sum of $22.5M 
USD.  Please get back to me with your private telephone number with sincerity. 

Sincerely Yours, 
Ms. Thandi Robert 

Re: show all merge conflicts

[Philip Oakley]

From: "G. Sylvie Davies" 

On Fri, Jan 27, 2017 at 9:51 AM, Jeff King  wrote:

On Fri, Jan 27, 2017 at 11:56:08AM -0500, Michael Spiegel wrote:


I'm trying to determine whether a merge required a conflict to resolve
after the merge has occurred. The git book has some advice
(https://git-scm.com/book/en/v2/Git-Tools-Advanced-Merging) to use
`git show` on the merge commit or use `git log --cc -p -1`. These
strategies work when the merge conflict was resolved with a change
that is different from either parent. When the conflict is resolved
with a change that is the same as one of the parents, then these
commands are indistinguishable from a merge that did not conflict. Is
it possible to distinguish between a conflict-free merge and a merge
conflict that is resolved by with the changes from one the parents?


No. You'd have to replay the merge to know if it would have had
conflicts.




Aside from the usual "git log -cc", I think this should work (replace
HEAD with whichever commit you are analyzing):

git diff --name-only HEAD^2...HEAD^1 > m1
git diff --name-only HEAD^1...HEAD^2 > b1
git diff --name-only HEAD^1..HEAD> m2
git diff --name-only HEAD^2..HEAD> b2

If files listed between m1 and b2 differ, then the merge is dirty.
Similarly for m2 and b1.

More information here:

http://stackoverflow.com/questions/27683077/how-do-you-detect-an-evil-merge-in-git/41356308#41356308


- Sylvie


This feels as though there ought to be some sort of --left-right option to 
get an indication of which side various changes came from





There was a patch series a few years ago that added a new diff-mode to
do exactly that, and show the diff against what was resolved. It had a
few issues (I don't remember exactly what) and never got merged.

Certainly one complication is that you don't know exactly _how_ the
merge was done in the first place (e.g., which merge strategy, which
custom merge drivers were in effect, etc). But in general, replaying
with a standard merge-recursive would get you most of what you want to
know.

I've done this manually sometimes when digging into erroneous merges
(e.g., somebody accidentally runs "git reset -- " in the middle
of a merge and throws away some changes.

You should be able to do:

  git checkout $merge^1
  git merge $merge^2
  git diff -R $merge

to see what the original resolution did.

-Peff

Hopefully

[Rita Micheal]

Dear friend,

My name is Mr Rita Micheal, I am the Bill and Exchange (assistant)
Manager of Bank of Africa Ouagadougou, Burkina Faso. In my department
I discovered an abandoned sum of teen million five hundred thousand United
State of American dollars (10.5MILLION USA DOLLARS) in an account that
belongs to one of our foreign customer who died in airline that
crashed on 4th October 2001.

Since I got information about his death I have been expecting his next
of kin to come over and claim his money because we can not release
it unless somebody applies for it as the next of kin or relation to the
deceased as indicated in our banking guidelines, but unfortunately
we learnt that all his supposed next of kin or relation died alongside
with him in the plane crash leaving nobody behind for the claim. It is
therefore upon this discovery that I decided to make this business
proposal to you and release the money to you as next of kin or relation
to the deceased for safety and subsequent disbursement since nobody
is coming for it and I don't want the money to go into the bank treasury
as unclaimed bill.

You will be entitled with 40% of the total sum while 60% will be for
me after which I will visit your Country to invest my own share when
the fund is successfully transferred into your account, Please I would
like you to keep this transaction confidential and as a top secret as
you may wish to know that I am a bank official.

Yours sincerely,
Mr Rita Micheal.