Re: [PATCH v3 16/27] attr: convert git_all_attrs() to use "struct attr_check"
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
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"
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 = &check->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 *check, > + const struct git_attr *attr) > +{ > +
Re: [PATCH 1/3] Documentation/stash: remove mention of git reset --hard
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
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, &suspect->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, &ci, 1); - sha1_to_hex_r(hex, suspect->commit->object.oid.hash); + oid_to_hex_r(hex, &suspect->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], &ce->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, &revs->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, &one->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
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 ? &old->object.oid : &null_oid), + oid_to_hex(new ? &new->object.oid : &null_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
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, &entry->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(&d->oid_index, &ce->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(&stages[stage - 1].oid, &ce->oid); sum |= (1 << (stage - 1)); } if (sum != d->stagemask) -- 2.11.0
[PATCH 5/5] graph: use SWAP macro
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
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
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 d327209443..b7ff1a
[PATCH 2/5] apply: use SWAP macro
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, &a, size);\ - memcpy(&a, &b, size); \ - memcpy(&b, 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
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, &len); } +#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
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
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, delete_branch, change_past,
[PATCH] t0001: don't let a default ACL interfere with the umask test
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
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
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
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
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
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.