Re: [PATCH v3 8/8] merge-recursive: improve rename/rename(1to2)/add[/add] handling
On 11/2/2018 1:27 PM, Elijah Newren wrote:
On Thu, Nov 1, 2018 at 12:01 AM Elijah Newren wrote:
On Wed, Oct 31, 2018 at 8:08 AM Derrick Stolee wrote:
On 10/19/2018 3:31 PM, Elijah Newren wrote:
[snip]
+ char *new_path = NULL;
+ if (dir_in_way(b->path, !o->call_depth, 0)) {
+ new_path = unique_path(o, b->path, ci->branch2);
+ output(o, 1, _("%s is a directory in %s adding "
+"as %s instead"),
+b->path, ci->branch1, new_path);
I tried really hard, but failed to get a test to cover the block below.
I was able to
find that the "check handling of differently renamed file with D/F
conflicts" test
in t6022-merge-rename.sh covers the block above. Trying to tweak the
example using
untracked files seems to hit an error message from unpack-trees.c instead.
+ } else if (would_lose_untracked(b->path)) {
+ new_path = unique_path(o, b->path, ci->branch2);
+ output(o, 1, _("Refusing to lose untracked file"
+" at %s; adding as %s instead"),
+b->path, new_path);
So now I'm confused. This block was not listed in your coverage
report[1]. And, in fact, I think this block IS covered by testcase
10c of t6043. However, there is a very similar looking block about 30
lines up that is uncovered (and which was mentioned in your report):
} else if (would_lose_untracked(a->path)) {
new_path = unique_path(o, a->path, ci->branch1);
output(o, 1, _("Refusing to lose untracked file"
" at %s; adding as %s instead"),
a->path, new_path);
covering it, I think, is just a matter of repeating the 10c test with
the merge repeated in the other direction (checkout B and merge A
instead of checking out A and merging B) -- and touching up the checks
accordingly.
However, now I'm wondering if I'm crazy. Was it really the block you
had highlighted that you were seeing uncovered?
Trust the report (generated by computer) over me (generated by squinting
at an email, trying to match line numbers).
Thanks,
-Stolee
Re: [PATCH v3 8/8] merge-recursive: improve rename/rename(1to2)/add[/add] handling
On Thu, Nov 1, 2018 at 12:01 AM Elijah Newren wrote:
>
> On Wed, Oct 31, 2018 at 8:08 AM Derrick Stolee wrote:
> >
> > On 10/19/2018 3:31 PM, Elijah Newren wrote:
> > > [snip]
> > >
> > > + char *new_path = NULL;
> > > + if (dir_in_way(b->path, !o->call_depth, 0)) {
> > > + new_path = unique_path(o, b->path,
> > > ci->branch2);
> > > + output(o, 1, _("%s is a directory in %s
> > > adding "
> > > +"as %s instead"),
> > > +b->path, ci->branch1, new_path);
> >
> > I tried really hard, but failed to get a test to cover the block below.
> > I was able to
> > find that the "check handling of differently renamed file with D/F
> > conflicts" test
> > in t6022-merge-rename.sh covers the block above. Trying to tweak the
> > example using
> > untracked files seems to hit an error message from unpack-trees.c instead.
> >
> > > + } else if (would_lose_untracked(b->path)) {
> > > + new_path = unique_path(o, b->path,
> > > ci->branch2);
> > > + output(o, 1, _("Refusing to lose untracked
> > > file"
> > > +" at %s; adding as %s
> > > instead"),
> > > +b->path, new_path);
> >
So now I'm confused. This block was not listed in your coverage
report[1]. And, in fact, I think this block IS covered by testcase
10c of t6043. However, there is a very similar looking block about 30
lines up that is uncovered (and which was mentioned in your report):
} else if (would_lose_untracked(a->path)) {
new_path = unique_path(o, a->path, ci->branch1);
output(o, 1, _("Refusing to lose untracked file"
" at %s; adding as %s instead"),
a->path, new_path);
covering it, I think, is just a matter of repeating the 10c test with
the merge repeated in the other direction (checkout B and merge A
instead of checking out A and merging B) -- and touching up the checks
accordingly.
However, now I'm wondering if I'm crazy. Was it really the block you
had highlighted that you were seeing uncovered?
Thanks,
Elijah
[1] https://public-inbox.org/git/62f0bcf6-aa73-c192-d804-e6d69cac1...@gmail.com/
Re: [PATCH v3 8/8] merge-recursive: improve rename/rename(1to2)/add[/add] handling
On Wed, Oct 31, 2018 at 8:08 AM Derrick Stolee wrote:
>
> On 10/19/2018 3:31 PM, Elijah Newren wrote:
> > [snip]
> >
> > + char *new_path = NULL;
> > + if (dir_in_way(b->path, !o->call_depth, 0)) {
> > + new_path = unique_path(o, b->path,
> > ci->branch2);
> > + output(o, 1, _("%s is a directory in %s
> > adding "
> > +"as %s instead"),
> > +b->path, ci->branch1, new_path);
>
> I tried really hard, but failed to get a test to cover the block below.
> I was able to
> find that the "check handling of differently renamed file with D/F
> conflicts" test
> in t6022-merge-rename.sh covers the block above. Trying to tweak the
> example using
> untracked files seems to hit an error message from unpack-trees.c instead.
>
> > + } else if (would_lose_untracked(b->path)) {
> > + new_path = unique_path(o, b->path,
> > ci->branch2);
> > + output(o, 1, _("Refusing to lose untracked
> > file"
> > +" at %s; adding as %s
> > instead"),
> > +b->path, new_path);
>
> It could also be that I failed because I'm less familiar with this part
> of the
> codebase. Elijah, do you think it is possible to hit this block?
Yeah, this one's going to be a little harder; the upper block would be
done with a D/F, but I think for this block you'd need a directory
rename so that unpack-trees.c can't tell that the untracked file in
the way is actually in the way of anything. But since this is in the
rename/rename(1to2) area, I think I had rules around avoiding doing
directory renames if the other side renamed the file to avoid getting
into rename/rename(1to3) situations and other weirdness. So, it might
require a transitive rename (i.e. file renamed on both sides, and on
one side it's renamed into a directory that the other side renamed
away).
I'll try to take a look at it tomorrow, with everything else. We'll
see how much I can get done.
Thanks for digging in to all these and bringing them up.
Re: [PATCH v3 8/8] merge-recursive: improve rename/rename(1to2)/add[/add] handling
On 10/19/2018 3:31 PM, Elijah Newren wrote:
[snip]
+ char *new_path = NULL;
+ if (dir_in_way(b->path, !o->call_depth, 0)) {
+ new_path = unique_path(o, b->path, ci->branch2);
+ output(o, 1, _("%s is a directory in %s adding "
+ "as %s instead"),
+ b->path, ci->branch1, new_path);
I tried really hard, but failed to get a test to cover the block below.
I was able to
find that the "check handling of differently renamed file with D/F
conflicts" test
in t6022-merge-rename.sh covers the block above. Trying to tweak the
example using
untracked files seems to hit an error message from unpack-trees.c instead.
+ } else if (would_lose_untracked(b->path)) {
+ new_path = unique_path(o, b->path, ci->branch2);
+ output(o, 1, _("Refusing to lose untracked file"
+ " at %s; adding as %s instead"),
+ b->path, new_path);
It could also be that I failed because I'm less familiar with this part
of the
codebase. Elijah, do you think it is possible to hit this block?
Thanks,
-Stolee
