* [PATCH v3 23/27] attr: remove maybe-real, maybe-macro from git_attr
From: Brandon Williams @ 2017-01-28 2:02 UTC (permalink / raw)
To: git; +Cc: Brandon Williams, sbeller, gitster, pclouds
In-Reply-To: <20170128020207.179015-1-bmwill@google.com>
Whether or not a git attribute is real or a macro isn't a property of
the attribute but rather it depends on the attribute stack (which
.gitattribute files were read).
This patch removes the 'maybe_real' and 'maybe_macro' fields in a
git_attr and instead adds the 'macro' field to a attr_check_item. The
'macro' indicates (if non-NULL) that a particular attribute is a macro
for the given attribute stack. It's populated, through a quick scan of
the attribute stack, with the match_attr that corresponds to the macro's
definition. This way the attribute stack only needs to be scanned a
single time prior to attribute collection instead of each time a macro
needs to be expanded.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
attr.c | 75 +++++++++++++++++++++++++++++++++---------------------------------
1 file changed, 37 insertions(+), 38 deletions(-)
diff --git a/attr.c b/attr.c
index 2637804b1..8f4402ef3 100644
--- a/attr.c
+++ b/attr.c
@@ -30,20 +30,9 @@ static const char git_attr__unknown[] = "(builtin)unknown";
struct git_attr {
int attr_nr; /* unique attribute number */
- int maybe_macro;
- int maybe_real;
char name[FLEX_ARRAY]; /* attribute name */
};
-/*
- * NEEDSWORK: maybe-real, maybe-macro are not property of
- * an attribute, as it depends on what .gitattributes are
- * read. Once we introduce per git_attr_check attr_stack
- * and check_all_attr, the optimization based on them will
- * become unnecessary and can go away. So is this variable.
- */
-static int cannot_trust_maybe_real;
-
const char *git_attr_name(const struct git_attr *attr)
{
return attr->name;
@@ -142,6 +131,12 @@ static void attr_hashmap_add(struct attr_hashmap *map,
struct all_attrs_item {
const struct git_attr *attr;
const char *value;
+ /*
+ * If 'macro' is non-NULL, indicates that 'attr' is a macro based on
+ * the current attribute stack and contains a pointer to the match_attr
+ * definition of the macro
+ */
+ const struct match_attr *macro;
};
/*
@@ -187,6 +182,7 @@ static void all_attrs_init(struct attr_hashmap *map, struct attr_check *check)
*/
for (i = 0; i < check->all_attrs_nr; i++) {
check->all_attrs[i].value = ATTR__UNKNOWN;
+ check->all_attrs[i].macro = NULL;
}
}
@@ -238,8 +234,6 @@ static struct git_attr *git_attr_internal(const char *name, int namelen)
if (!a) {
FLEX_ALLOC_MEM(a, name, name, namelen);
a->attr_nr = g_attr_hashmap.map.size;
- a->maybe_real = 0;
- a->maybe_macro = 0;
attr_hashmap_add(&g_attr_hashmap, a->name, namelen, a);
assert(a->attr_nr == (g_attr_hashmap.map.size - 1));
@@ -402,7 +396,6 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
(is_macro ? 0 : namelen + 1));
if (is_macro) {
res->u.attr = git_attr_internal(name, namelen);
- res->u.attr->maybe_macro = 1;
} else {
char *p = (char *)&(res->state[num_attr]);
memcpy(p, name, namelen);
@@ -423,10 +416,6 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,
/* Second pass to fill the attr_states */
for (cp = states, i = 0; *cp; i++) {
cp = parse_attr(src, lineno, cp, &(res->state[i]));
- if (!is_macro)
- res->state[i].attr->maybe_real = 1;
- if (res->state[i].attr->maybe_macro)
- cannot_trust_maybe_real = 1;
}
strbuf_release(&pattern);
@@ -904,7 +893,7 @@ static int path_matches(const char *pathname, int pathlen,
static int macroexpand_one(struct all_attrs_item *all_attrs, int nr, int rem);
static int fill_one(const char *what, struct all_attrs_item *all_attrs,
- struct match_attr *a, int rem)
+ const struct match_attr *a, int rem)
{
int i;
@@ -945,24 +934,34 @@ static int fill(const char *path, int pathlen, int basename_offset,
static int macroexpand_one(struct all_attrs_item *all_attrs, int nr, int rem)
{
- struct attr_stack *stk;
- int i;
+ const struct all_attrs_item *item = &all_attrs[nr];
- if (all_attrs[nr].value != ATTR__TRUE ||
- !all_attrs[nr].attr->maybe_macro)
+ if (item->macro && item->value == ATTR__TRUE)
+ return fill_one("expand", all_attrs, item->macro, rem);
+ else
return rem;
+}
- for (stk = attr_stack; stk; stk = stk->prev) {
- for (i = stk->num_matches - 1; 0 <= i; i--) {
- struct match_attr *ma = stk->attrs[i];
- if (!ma->is_macro)
- continue;
- if (ma->u.attr->attr_nr == nr)
- return fill_one("expand", all_attrs, ma, rem);
+/*
+ * Marks the attributes which are macros based on the attribute stack.
+ * This prevents having to search through the attribute stack each time
+ * a macro needs to be expanded during the fill stage.
+ */
+static void determine_macros(struct all_attrs_item *all_attrs,
+ const struct attr_stack *stack)
+{
+ for (; stack; stack = stack->prev) {
+ int i;
+ for (i = stack->num_matches - 1; i >= 0; i--) {
+ const struct match_attr *ma = stack->attrs[i];
+ if (ma->is_macro) {
+ int n = ma->u.attr->attr_nr;
+ if (!all_attrs[n].macro) {
+ all_attrs[n].macro = ma;
+ }
+ }
}
}
-
- return rem;
}
/*
@@ -992,15 +991,15 @@ static void collect_some_attrs(const char *path, struct attr_check *check)
prepare_attr_stack(path, dirlen);
all_attrs_init(&g_attr_hashmap, check);
+ determine_macros(check->all_attrs, attr_stack);
- if (check->nr && !cannot_trust_maybe_real) {
+ if (check->nr) {
rem = 0;
for (i = 0; i < check->nr; i++) {
- const struct git_attr *a = check->items[i].attr;
- if (!a->maybe_real) {
- struct all_attrs_item *c;
- c = check->all_attrs + a->attr_nr;
- c->value = ATTR__UNSET;
+ int n = check->items[i].attr->attr_nr;
+ struct all_attrs_item *item = &check->all_attrs[n];
+ if (item->macro) {
+ item->value = ATTR__UNSET;
rem++;
}
}
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* Re: difflame
From: Jeff King @ 2017-01-28 3:53 UTC (permalink / raw)
To: Edmundo Carmona Antoranz; +Cc: Git List
In-Reply-To: <CAOc6etaCk=OEyarMNhorM94MBnYRscCkJBM-K08snv1ecmOaPQ@mail.gmail.com>
On Tue, Jan 17, 2017 at 11:24:02PM -0600, Edmundo Carmona Antoranz wrote:
> For a very long time I had wanted to get the output of diff to include
> blame information as well (to see when something was added/removed).
This is something I've wanted, too. The trickiest part, though, is
blaming deletions, because git-blame only tracks the origin of content,
not the origin of a change.
For example, try this case:
git init
for i in $(seq 1 10); do
echo $i >>file
git add file
git commit -m "add $i"
done
sed 4d <file >tmp && mv tmp file
git commit -am "drop 4"
Running "difflame HEAD~5 HEAD" produces this output:
diff --git a/file b/file
index b414108..051c298 100644
--- a/file
+++ b/file
@@ -1,6 +1,9 @@
^2b028ff (Jeff King 2017-01-27 22:44:10 -0500 1) 1
ed056366 (Jeff King 2017-01-27 22:44:10 -0500 2) 2
771030d8 (Jeff King 2017-01-27 22:44:10 -0500 3) 3
-89c09c82 (Jeff King 2017-01-27 22:44:10 -0500 4) 4
b619039c (Jeff King 2017-01-27 22:44:10 -0500 4) 5
6a7aa0e5 (Jeff King 2017-01-27 22:44:10 -0500 5) 6
+39bc9dc4 (Jeff King 2017-01-27 22:44:10 -0500 6) 7
+f253cc8f (Jeff King 2017-01-27 22:44:10 -0500 7) 8
+85c10f46 (Jeff King 2017-01-27 22:44:10 -0500 8) 9
+89c09c82 (Jeff King 2017-01-27 22:44:10 -0500 9) 10
The last 4 lines are right; they correspond to the addition commits. But
the line taking away 4 is wrong. You can see even without looking at its
patch, because it is blamed to the same commit that added "10", which
is wrong.
Sorry I don't have a solution. I think it's an open problem with
git-blame, though you could probably script something around "git blame
--reverse". See the commit message of 85af7929e (git-blame --reverse,
2008-04-02) for some discussion.
-Peff
^ permalink raw reply
* Re: [PATCH v5 1/3] color.c: fix color_parse_mem() with value_len == 0
From: Jeff King @ 2017-01-28 4:07 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano
In-Reply-To: <20170119163841.zhtpz6rxdieaywuy@sigill.intra.peff.net>
On Thu, Jan 19, 2017 at 11:38:41AM -0500, Jeff King wrote:
> On Thu, Jan 19, 2017 at 06:41:21PM +0700, Nguyễn Thái Ngọc Duy wrote:
>
> > In this code we want to match the word "reset". If len is zero,
> > strncasecmp() will return zero and we incorrectly assume it's "reset" as
> > a result.
>
> This is probably a good idea. This _is_ user-visible, so it's possible
> somebody was using empty config as a synonym for "reset". But since it
> was never documented, I feel like relying on that is somewhat crazy.
Hrm. This seems to break the add--interactive script if you do not have
color.diff.plain set:
$ GIT_TRACE=1 git add -p
...
22:58:12.568990 [pid=11401] git.c:387 trace: built-in: git 'config' '--get-color' 'color.diff.plain' ''
fatal: unable to parse default color value
config --get-color color.diff.plain : command returned error: 128
As you can see, the default value the empty string, which is now an
error.
The default in the C code for that value is GIT_COLOR_NORMAL, which
really is the empty string. So I think the old code was buggy to choose
"reset", but the new one is worse because it fails entirely. :)
We probably want something like this instead:
diff --git a/color.c b/color.c
index 190b2da96..dee61557e 100644
--- a/color.c
+++ b/color.c
@@ -212,8 +212,10 @@ int color_parse_mem(const char *value, int value_len, char *dst)
len--;
}
- if (!len)
- return -1;
+ if (!len) {
+ dst[0] = '\0';
+ return 0;
+ }
if (!strncasecmp(ptr, "reset", len)) {
xsnprintf(dst, end - dst, GIT_COLOR_RESET);
The breakage is in 'next' (it looks like it went out a few days ago; I'm
surprised I didn't notice until now).
-Peff
^ permalink raw reply related
* Re: show all merge conflicts
From: G. Sylvie Davies @ 2017-01-28 5:42 UTC (permalink / raw)
To: Jeff King; +Cc: Michael Spiegel, git
In-Reply-To: <20170127175151.srhhczliqgvbzcre@sigill.intra.peff.net>
On Fri, Jan 27, 2017 at 9:51 AM, Jeff King <peff@peff.net> 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
> 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 -- <paths>" 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
^ permalink raw reply
* Re: show all merge conflicts
From: Philip Oakley @ 2017-01-28 13:43 UTC (permalink / raw)
To: G. Sylvie Davies, Jeff King; +Cc: Michael Spiegel, git
In-Reply-To: <CAAj3zPzO4+9t9_L2OXFmkihw-HwFvzybb7GXs4tTeFRyZHOaNQ@mail.gmail.com>
From: "G. Sylvie Davies" <sylvie@bit-booster.com>
> On Fri, Jan 27, 2017 at 9:51 AM, Jeff King <peff@peff.net> 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 -- <paths>" 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
>
^ permalink raw reply
* Re: show all merge conflicts
From: Jeff King @ 2017-01-28 14:28 UTC (permalink / raw)
To: G. Sylvie Davies; +Cc: Michael Spiegel, git
In-Reply-To: <CAAj3zPzO4+9t9_L2OXFmkihw-HwFvzybb7GXs4tTeFRyZHOaNQ@mail.gmail.com>
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/ <file >tmp && mv tmp file
git commit -am master
git checkout -b other HEAD^
sed s/4/other/ <file >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
^ permalink raw reply
* Re: [PATCH 1/3] Documentation/stash: remove mention of git reset --hard
From: Thomas Gummerer @ 2017-01-28 19:30 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, git, Stephan Beyer, Marc Strapetz, Johannes Schindelin
In-Reply-To: <xmqqfuk6alba.fsf@gitster.mtv.corp.google.com>
On 01/25, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > On Sat, Jan 21, 2017 at 08:08:02PM +0000, 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] [<message>]::
> >>
> >> - Save your local modifications to a new 'stash', and run `git reset
> >> - --hard` to revert them. The <message> 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 <message> 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.
^ permalink raw reply
* [PATCH] t0001: don't let a default ACL interfere with the umask test
From: Matt McCutchen @ 2017-01-28 20:25 UTC (permalink / raw)
To: git
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 <matt@mattmccutchen.net>
---
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
^ permalink raw reply related
* [PATCH] merge-recursive: make "CONFLICT (rename/delete)" message show both paths
From: Matt McCutchen @ 2017-01-28 20:37 UTC (permalink / raw)
To: git
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 <matt@mattmccutchen.net>
---
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, path,
+ change_branch, change_branch, 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->branch2, change_past,
- o->branch1, o->branch1, path, renamed);
- ret = update_file(o, 0, a_oid, a_mode, renamed);
+ if (!old_path) {
+ 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, delete_branch, change_past,
+ change_branch, change_branch, path, alt_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 at %s."),
+ change, old_path, delete_branch, change_past, path,
+ change_branch, change_branch, path, alt_path);
+ }
}
/*
- * No need to call update_file() on path when !renamed, since
- * that would needlessly touch path. We could call
- * update_file_flags() with update_cache=0 and update_wd=0,
- * but that's a no-op.
+ * No need to call update_file() on path when change_branch ==
+ * o->branch1 && !alt_path, since that would needlessly touch
+ * path. We could call update_file_flags() with update_cache=0
+ * and update_wd=0, but that's a no-op.
*/
+ if (change_branch != o->branch1 || alt_path)
+ ret = update_file(o, 0, changed_oid, changed_mode, update_path);
}
- free(renamed);
+ free(alt_path);
return ret;
}
@@ -1125,28 +1129,17 @@ static int handle_change_delete(struct merge_options *o,
static int conflict_rename_delete(struct merge_options *o,
struct diff_filepair *pair,
const char *rename_branch,
- const char *other_branch)
+ const char *delete_branch)
{
const struct diff_filespec *orig = pair->one;
const struct diff_filespec *dest = pair->two;
- const struct object_id *a_oid = NULL;
- const struct object_id *b_oid = NULL;
- int a_mode = 0;
- int b_mode = 0;
-
- if (rename_branch == o->branch1) {
- a_oid = &dest->oid;
- a_mode = dest->mode;
- } else {
- b_oid = &dest->oid;
- b_mode = dest->mode;
- }
if (handle_change_delete(o,
o->call_depth ? orig->path : dest->path,
+ o->call_depth ? NULL : orig->path,
&orig->oid, orig->mode,
- a_oid, a_mode,
- b_oid, b_mode,
+ &dest->oid, dest->mode,
+ rename_branch, delete_branch,
_("rename"), _("renamed")))
return -1;
@@ -1665,11 +1658,27 @@ static int handle_modify_delete(struct merge_options *o,
struct object_id *a_oid, int a_mode,
struct object_id *b_oid, int b_mode)
{
+ const char *modify_branch, *delete_branch;
+ struct object_id *changed_oid;
+ int changed_mode;
+
+ if (a_oid) {
+ modify_branch = o->branch1;
+ delete_branch = o->branch2;
+ changed_oid = a_oid;
+ changed_mode = a_mode;
+ } else {
+ modify_branch = o->branch2;
+ delete_branch = o->branch1;
+ changed_oid = b_oid;
+ changed_mode = b_mode;
+ }
+
return handle_change_delete(o,
- path,
+ path, NULL,
o_oid, o_mode,
- a_oid, a_mode,
- b_oid, b_mode,
+ changed_oid, changed_mode,
+ modify_branch, delete_branch,
_("modify"), _("modified"));
}
diff --git a/t/t6045-merge-rename-delete.sh b/t/t6045-merge-rename-delete.sh
new file mode 100755
index 0000000..8f33244
--- /dev/null
+++ b/t/t6045-merge-rename-delete.sh
@@ -0,0 +1,23 @@
+#!/bin/sh
+
+test_description='Merge-recursive rename/delete conflict message'
+. ./test-lib.sh
+
+test_expect_success 'rename/delete' '
+echo foo >A &&
+git add A &&
+git commit -m "initial" &&
+
+git checkout -b rename &&
+git mv A B &&
+git commit -m "rename" &&
+
+git checkout master &&
+git rm A &&
+git commit -m "delete" &&
+
+test_must_fail git merge --strategy=recursive rename >output &&
+test_i18ngrep "CONFLICT (rename/delete): A deleted in HEAD and renamed to B in rename. Version rename of B left in tree." output
+'
+
+test_done
--
2.9.3
^ permalink raw reply related
* [PATCH 0/5] introduce SWAP macro
From: René Scharfe @ 2017-01-28 21:13 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Johannes Schindelin
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
^ permalink raw reply
* [PATCH 1/5] add SWAP macro
From: René Scharfe @ 2017-01-28 21:38 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <8ef4c833-45bd-6831-0683-6d01f30aa518@web.de>
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 <l.s.r@web.de>
---
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 0000000000..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
^ permalink raw reply related
* [PATCH 2/5] apply: use SWAP macro
From: René Scharfe @ 2017-01-28 21:40 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <8ef4c833-45bd-6831-0683-6d01f30aa518@web.de>
Use the exported macro SWAP instead of the file-scoped macro swap and
remove the latter's definition.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
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
^ permalink raw reply related
* [PATCH 3/5] use SWAP macro
From: René Scharfe @ 2017-01-28 21:40 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <8ef4c833-45bd-6831-0683-6d01f30aa518@web.de>
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 <l.s.r@web.de>
---
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..b7ff1ada3c 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1390,14 +1390,11 @@ static int process_renames(struct merge_options *o,
branch1 = o->branch1;
branch2 = o->branch2;
} else {
- struct rename *tmp;
renames1 = b_renames;
renames2Dst = &a_by_dst;
branch1 = o->branch2;
branch2 = o->branch1;
- tmp = ren2;
- ren2 = ren1;
- ren1 = tmp;
+ SWAP(ren2, ren1);
}
if (ren1->processed)
diff --git a/object.c b/object.c
index 67d9a9e221..e680d881a4 100644
--- a/object.c
+++ b/object.c
@@ -104,9 +104,7 @@ struct object *lookup_object(const unsigned char *sha1)
* that we do not need to walk the hash table the next
* time we look for it.
*/
- struct object *tmp = obj_hash[i];
- obj_hash[i] = obj_hash[first];
- obj_hash[first] = tmp;
+ SWAP(obj_hash[i], obj_hash[first]);
}
return obj;
}
diff --git a/pack-revindex.c b/pack-revindex.c
index 6bc7c94033..1b7ebd8d7e 100644
--- a/pack-revindex.c
+++ b/pack-revindex.c
@@ -59,7 +59,6 @@ static void sort_revindex(struct revindex_entry *entries, unsigned n, off_t max)
* be a no-op, as everybody lands in the same zero-th bucket.
*/
for (bits = 0; max >> bits; bits += DIGIT_SIZE) {
- struct revindex_entry *swap;
unsigned i;
memset(pos, 0, BUCKETS * sizeof(*pos));
@@ -97,9 +96,7 @@ static void sort_revindex(struct revindex_entry *entries, unsigned n, off_t max)
* Now "to" contains the most sorted list, so we swap "from" and
* "to" for the next iteration.
*/
- swap = from;
- from = to;
- to = swap;
+ SWAP(from, to);
}
/*
diff --git a/prio-queue.c b/prio-queue.c
index e4365b00d6..17252d231b 100644
--- a/prio-queue.c
+++ b/prio-queue.c
@@ -12,9 +12,7 @@ static inline int compare(struct prio_queue *queue, int i, int j)
static inline void swap(struct prio_queue *queue, int i, int j)
{
- struct prio_queue_entry tmp = queue->array[i];
- queue->array[i] = queue->array[j];
- queue->array[j] = tmp;
+ SWAP(queue->array[i], queue->array[j]);
}
void prio_queue_reverse(struct prio_queue *queue)
diff --git a/strbuf.h b/strbuf.h
index 2262b12683..cf1b5409e7 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -109,9 +109,7 @@ extern void strbuf_attach(struct strbuf *, void *, size_t, size_t);
*/
static inline void strbuf_swap(struct strbuf *a, struct strbuf *b)
{
- struct strbuf tmp = *a;
- *a = *b;
- *b = tmp;
+ SWAP(*a, *b);
}
--
2.11.0
^ permalink raw reply related
* [PATCH 4/5] diff: use SWAP macro
From: René Scharfe @ 2017-01-28 21:41 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <8ef4c833-45bd-6831-0683-6d01f30aa518@web.de>
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 <l.s.r@web.de>
---
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
^ permalink raw reply related
* [PATCH 5/5] graph: use SWAP macro
From: René Scharfe @ 2017-01-28 21:42 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Johannes Schindelin
In-Reply-To: <8ef4c833-45bd-6831-0683-6d01f30aa518@web.de>
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 <l.s.r@web.de>
---
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
^ permalink raw reply related
* [PATCH] use oidcpy() for copying hashes between instances of struct object_id
From: René Scharfe @ 2017-01-28 22:03 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, brian m. carlson
Patch generated by Coccinelle and contrib/coccinelle/object_id.cocci.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
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
^ permalink raw reply related
* [PATCH] checkout: convert post_checkout_hook() to struct object_id
From: René Scharfe @ 2017-01-28 22:14 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, brian m. carlson
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
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
^ permalink raw reply related
* [PATCH] use oid_to_hex_r() for converting struct object_id hashes to hex strings
From: René Scharfe @ 2017-01-28 22:03 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, brian m. carlson
Patch generated by Coccinelle and contrib/coccinelle/object_id.cocci.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
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
^ permalink raw reply related
* Re: [PATCH 1/3] Documentation/stash: remove mention of git reset --hard
From: Jeff King @ 2017-01-28 23:54 UTC (permalink / raw)
To: Thomas Gummerer
Cc: Junio C Hamano, git, Stephan Beyer, Marc Strapetz,
Johannes Schindelin
In-Reply-To: <20170128192958.GB31189@hank>
On Sat, Jan 28, 2017 at 07:30:28PM +0000, 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
^ permalink raw reply
* Re: [PATCH v3 16/27] attr: convert git_all_attrs() to use "struct attr_check"
From: Stefan Beller @ 2017-01-28 23:50 UTC (permalink / raw)
To: Brandon Williams; +Cc: git@vger.kernel.org, Junio C Hamano, Duy Nguyen
In-Reply-To: <20170128020207.179015-17-bmwill@google.com>
On Fri, Jan 27, 2017 at 6:01 PM, Brandon Williams <bmwill@google.com> wrote:
> From: Junio C Hamano <gitster@pobox.com>
>
> 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 <gitster@pobox.com>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
> 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)
> +{
> + 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);
> +}
> +
> static const char *builtin_attr[] = {
> "[attr]binary -diff -merge -text",
> NULL,
> @@ -906,32 +906,22 @@ int git_check_attrs(const char *path, int num, struct attr_check_item *check)
> return 0;
> }
Below is where the actual change for this patch starts?
>
> -int git_all_attrs(const char *path, int *num, struct attr_check_item **check)
> +void git_all_attrs(const char *path, struct attr_check *check)
> {
> - int i, count, j;
> + int i;
>
> - collect_some_attrs(path, 0, NULL);
> + attr_check_reset(check);
> + collect_some_attrs(path, check->nr, check->items);
>
> - /* Count the number of attributes that are set. */
> - count = 0;
> - for (i = 0; i < attr_nr; i++) {
> - const char *value = check_all_attr[i].value;
> - if (value != ATTR__UNSET && value != ATTR__UNKNOWN)
> - ++count;
> - }
> - *num = count;
> - ALLOC_ARRAY(*check, count);
> - j = 0;
> for (i = 0; i < attr_nr; i++) {
> + const char *name = check_all_attr[i].attr->name;
> const char *value = check_all_attr[i].value;
> - if (value != ATTR__UNSET && value != ATTR__UNKNOWN) {
> - (*check)[j].attr = check_all_attr[i].attr;
> - (*check)[j].value = value;
> - ++j;
> - }
> + struct attr_check_item *item;
> + if (value == ATTR__UNSET || value == ATTR__UNKNOWN)
> + continue;
> + item = attr_check_append(check, git_attr(name));
> + item->value = value;
> }
> -
> - return 0;
> }
>
> int git_check_attr(const char *path, struct attr_check *check)
> diff --git a/attr.h b/attr.h
> index e611b139a..9f2729842 100644
> --- a/attr.h
> +++ b/attr.h
> @@ -56,13 +56,10 @@ int git_check_attrs(const char *path, int, struct attr_check_item *);
> extern int git_check_attr(const char *path, struct attr_check *check);
>
> /*
> - * Retrieve all attributes that apply to the specified path. *num
> - * will be set to the number of attributes on the path; **check will
> - * be set to point at a newly-allocated array of git_attr_check
> - * objects describing the attributes and their values. *check must be
> - * free()ed by the caller.
> + * Retrieve all attributes that apply to the specified path.
> + * check holds the attributes and their values.
> */
> -int git_all_attrs(const char *path, int *num, struct attr_check_item **check);
> +extern void git_all_attrs(const char *path, struct attr_check *check);
>
> enum git_attr_direction {
> GIT_ATTR_CHECKIN,
> diff --git a/builtin/check-attr.c b/builtin/check-attr.c
> index 889264a5b..40cdff13e 100644
> --- a/builtin/check-attr.c
> +++ b/builtin/check-attr.c
> @@ -24,12 +24,13 @@ static const struct option check_attr_options[] = {
> OPT_END()
> };
>
> -static void output_attr(int cnt, struct attr_check_item *check,
> - const char *file)
> +static void output_attr(struct attr_check *check, const char *file)
> {
> int j;
> + int cnt = check->nr;
> +
> for (j = 0; j < cnt; j++) {
> - const char *value = check[j].value;
> + const char *value = check->items[j].value;
>
> if (ATTR_TRUE(value))
> value = "set";
> @@ -42,36 +43,38 @@ static void output_attr(int cnt, struct attr_check_item *check,
> printf("%s%c" /* path */
> "%s%c" /* attrname */
> "%s%c" /* attrvalue */,
> - file, 0, git_attr_name(check[j].attr), 0, value, 0);
> + file, 0,
> + git_attr_name(check->items[j].attr), 0, value, 0);
> } else {
> quote_c_style(file, NULL, stdout, 0);
> - printf(": %s: %s\n", git_attr_name(check[j].attr), value);
> + printf(": %s: %s\n",
> + git_attr_name(check->items[j].attr), value);
> }
> -
> }
> }
>
> static void check_attr(const char *prefix,
> - int cnt, struct attr_check_item *check,
> + struct attr_check *check,
> + int collect_all,
> const char *file)
> {
> char *full_path =
> prefix_path(prefix, prefix ? strlen(prefix) : 0, file);
> - if (check != NULL) {
> - if (git_check_attrs(full_path, cnt, check))
> - die("git_check_attrs died");
> - output_attr(cnt, check, file);
> +
> + if (collect_all) {
> + git_all_attrs(full_path, check);
> } else {
> - if (git_all_attrs(full_path, &cnt, &check))
> - die("git_all_attrs died");
> - output_attr(cnt, check, file);
> - free(check);
> + if (git_check_attr(full_path, check))
> + die("git_check_attr died");
> }
> + output_attr(check, file);
> +
> free(full_path);
> }
>
> static void check_attr_stdin_paths(const char *prefix,
> - int cnt, struct attr_check_item *check)
> + struct attr_check *check,
> + int collect_all)
> {
> struct strbuf buf = STRBUF_INIT;
> struct strbuf unquoted = STRBUF_INIT;
> @@ -85,7 +88,7 @@ static void check_attr_stdin_paths(const char *prefix,
> die("line is badly quoted");
> strbuf_swap(&buf, &unquoted);
> }
> - check_attr(prefix, cnt, check, buf.buf);
> + check_attr(prefix, check, collect_all, buf.buf);
> maybe_flush_or_die(stdout, "attribute to stdout");
> }
> strbuf_release(&buf);
> @@ -100,7 +103,7 @@ static NORETURN void error_with_usage(const char *msg)
>
> int cmd_check_attr(int argc, const char **argv, const char *prefix)
> {
> - struct attr_check_item *check;
> + struct attr_check *check;
> int cnt, i, doubledash, filei;
>
> if (!is_bare_repository())
> @@ -160,28 +163,25 @@ int cmd_check_attr(int argc, const char **argv, const char *prefix)
> error_with_usage("No file specified");
> }
>
> - if (all_attrs) {
> - check = NULL;
> - } else {
> - check = xcalloc(cnt, sizeof(*check));
> + check = attr_check_alloc();
> + if (!all_attrs) {
> for (i = 0; i < cnt; i++) {
> - const char *name;
> - struct git_attr *a;
> - name = argv[i];
> - a = git_attr(name);
> + struct git_attr *a = git_attr(argv[i]);
> if (!a)
> return error("%s: not a valid attribute name",
> - name);
> - check[i].attr = a;
> + argv[i]);
> + attr_check_append(check, a);
> }
> }
>
> if (stdin_paths)
> - check_attr_stdin_paths(prefix, cnt, check);
> + check_attr_stdin_paths(prefix, check, all_attrs);
> else {
> for (i = filei; i < argc; i++)
> - check_attr(prefix, cnt, check, argv[i]);
> + check_attr(prefix, check, all_attrs, argv[i]);
> maybe_flush_or_die(stdout, "attribute to stdout");
> }
> +
> + attr_check_free(check);
> return 0;
> }
> --
> 2.11.0.483.g087da7b7c-goog
>
^ permalink raw reply
* git-daemon shallow checkout fail
From: Bob Proulx @ 2017-01-29 0:29 UTC (permalink / raw)
To: git
I am trying to understand a problem with shallow checkouts through the
git-daemon. The server side fails trying to create a shallow_XXXXXX
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_XXXXXX 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_XXXXXX
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_XXXXXX 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
^ permalink raw reply
* Re: [PATCH v3 16/27] attr: convert git_all_attrs() to use "struct attr_check"
From: Brandon Williams @ 2017-01-29 2:44 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, Junio C Hamano, Duy Nguyen
In-Reply-To: <CAGZ79kax8Q6bD1Xm-kGAK=yTG6TYWcR3SCOjN9BeX9rzwkyaig@mail.gmail.com>
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
^ permalink raw reply
* Re: [PATCH 2/3] stash: introduce push verb
From: Thomas Gummerer @ 2017-01-29 12:39 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Stephan Beyer, Marc Strapetz, Jeff King, Johannes Schindelin
In-Reply-To: <xmqqziihskbi.fsf@gitster.mtv.corp.google.com>
On 01/23, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
>
> > + stash_msg="$*"
> > +
> > + if test -z stash_msg
>
> A dollar-sign is missing here, I think.
Yes, thanks.
> > + then
> > + push_stash $push_options
> > + else
> > + push_stash $push_options -m "$stash_msg"
> > + fi
> > +}
--
Thomas
^ permalink raw reply
* [PATCH] receive-pack: call string_list_clear() unconditionally
From: René Scharfe @ 2017-01-29 13:09 UTC (permalink / raw)
To: Git List; +Cc: Stefan Beller, Junio C Hamano
string_list_clear() handles empty lists just fine, so remove the
redundant check.
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
builtin/receive-pack.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 6b97cbdbe..1dbb8a069 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1942,8 +1942,7 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
run_receive_hook(commands, "post-receive", 1,
&push_options);
run_update_post_hook(commands);
- if (push_options.nr)
- string_list_clear(&push_options, 0);
+ string_list_clear(&push_options, 0);
if (auto_gc) {
const char *argv_gc_auto[] = {
"gc", "--auto", "--quiet", NULL,
--
2.11.0
^ permalink raw reply related
* Re: [PATCH 3/3] stash: support filename argument
From: Thomas Gummerer @ 2017-01-29 13:29 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Stephan Beyer, Marc Strapetz, Jeff King, Johannes Schindelin
In-Reply-To: <xmqqinp5sj98.fsf@gitster.mtv.corp.google.com>
On 01/23, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
>
> > diff --git a/git-stash.sh b/git-stash.sh
> > index d6b4ae3290..7dcce629bd 100755
> > --- a/git-stash.sh
> > +++ b/git-stash.sh
> > @@ -41,7 +41,7 @@ no_changes () {
> > untracked_files () {
> > excl_opt=--exclude-standard
> > test "$untracked" = "all" && excl_opt=
> > - git ls-files -o -z $excl_opt
> > + git ls-files -o -z $excl_opt -- $1
>
> Does $1 need to be quoted to prevent it from split at $IFS?
Not sure, I'll check and add a test for the re-roll.
> > @@ -56,6 +56,23 @@ clear_stash () {
> > }
> >
> > create_stash () {
> > + files=
> > + while test $# != 0
> > + do
> > + case "$1" in
> > + --)
> > + shift
> > + break
> > + ;;
> > + --files)
> > + ;;
> > + *)
> > + files="$1 $files"
> > + ;;
>
> Hmph. What is this "no-op" option about? Did you mean to say
> something like this instead?
>
> case "$1" in
> ...
> --file)
> case $# in
> 1)
> die "--file needs a pathspec" ;;
> *)
> shift
> files="$files$1 " ;;
> esac
> ;;
>
Hmm that would require multiple --file arguments to create_stash,
which I wanted to avoid. But probably the correct solution is to
introduce a new format for create_stash, which allows a -m before the
message, and uses -- to disambiguate before the file name arguments.
This would be similar to what Johannes suggested in [1], deprecating
the old syntax in git stash create. While this didn't work in git
stash save, it would work in git stash create, as "--" isn't used to
disambiguate anything there yet.
> Another thing I noticed. We won't support filenames with embedded
> $IFS characters at all?
>
> I somehow had an impression that the script was carefully done
> (e.g. by using -z option where appropriate) to add such a
> limitation.
>
> Perhaps we have broken it over time and it no longer matters
> (i.e. there already may be existing breakages), but this troubles
> me somehow.
Good point, I didn't think about $IFS characters. Fill fix in the
next round.
> By the way, in addition to "push" thing that corrects the argument
> convention by requiring "-m" before the message, we need to correct
> create_stash that is used internally from "stash push" somehow?
Yeah, I think that would make sense, see above.
[1]: http://public-inbox.org/git/alpine.DEB.2.20.1701241148300.3469@virtualbox/
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox