* [PATCH 2/9] Add flag to make unpack_trees() not print errors.
@ 2008-01-25 23:24 Daniel Barkalow
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Barkalow @ 2008-01-25 23:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
This will allow builtin-checkout to suppress merge errors if it's
going to try more merging methods.
Additionally, if unpack_trees() returns with an error, but without
printing anything, it will roll back any changes to the index (by
rereading the index, currently). This obviously could be done by the
caller, but chances are that the caller would forget and debugging
this is difficult.
Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
unpack-trees.c | 43 +++++++++++++++++++++++++++++--------------
unpack-trees.h | 1 +
2 files changed, 30 insertions(+), 14 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index a56e1c9..fc0d2c7 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -357,12 +357,23 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
posns[i] = create_tree_entry_list(t+i);
if (unpack_trees_rec(posns, len, o->prefix ? o->prefix : "",
- o, &df_conflict_list))
+ o, &df_conflict_list)) {
+ if (o->gently) {
+ discard_cache();
+ read_cache();
+ }
return -1;
+ }
}
- if (o->trivial_merges_only && o->nontrivial_merge)
- return error("Merge requires file-level merging");
+ if (o->trivial_merges_only && o->nontrivial_merge) {
+ if (o->gently) {
+ discard_cache();
+ read_cache();
+ }
+ return o->gently ? -1 :
+ error("Merge requires file-level merging");
+ }
check_updates(active_cache, active_nr, o);
return 0;
@@ -416,7 +427,8 @@ static int verify_uptodate(struct cache_entry *ce,
}
if (errno == ENOENT)
return 0;
- return error("Entry '%s' not uptodate. Cannot merge.", ce->name);
+ return o->gently ? -1 :
+ error("Entry '%s' not uptodate. Cannot merge.", ce->name);
}
static void invalidate_ce_path(struct cache_entry *ce)
@@ -502,8 +514,9 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action,
d.exclude_per_dir = o->dir->exclude_per_dir;
i = read_directory(&d, ce->name, pathbuf, namelen+1, NULL);
if (i)
- return error("Updating '%s' would lose untracked files in it",
- ce->name);
+ return o->gently ? -1 :
+ error("Updating '%s' would lose untracked files in it",
+ ce->name);
free(pathbuf);
return cnt;
}
@@ -575,8 +588,9 @@ static int verify_absent(struct cache_entry *ce, const char *action,
return 0;
}
- return error("Untracked working tree file '%s' "
- "would be %s by merge.", ce->name, action);
+ return o->gently ? -1 :
+ error("Untracked working tree file '%s' "
+ "would be %s by merge.", ce->name, action);
}
return 0;
}
@@ -708,7 +722,7 @@ int threeway_merge(struct cache_entry **stages,
/* #14, #14ALT, #2ALT */
if (remote && !df_conflict_head && head_match && !remote_match) {
if (index && !same(index, remote) && !same(index, head))
- return reject_merge(index);
+ return o->gently ? -1 : reject_merge(index);
return merged_entry(remote, index, o);
}
/*
@@ -716,7 +730,7 @@ int threeway_merge(struct cache_entry **stages,
* make sure that it matches head.
*/
if (index && !same(index, head)) {
- return reject_merge(index);
+ return o->gently ? -1 : reject_merge(index);
}
if (head) {
@@ -867,11 +881,11 @@ int twoway_merge(struct cache_entry **src,
/* all other failures */
remove_entry(remove);
if (oldtree)
- return reject_merge(oldtree);
+ return o->gently ? -1 : reject_merge(oldtree);
if (current)
- return reject_merge(current);
+ return o->gently ? -1 : reject_merge(current);
if (newtree)
- return reject_merge(newtree);
+ return o->gently ? -1 : reject_merge(newtree);
return -1;
}
}
@@ -898,7 +912,8 @@ int bind_merge(struct cache_entry **src,
return error("Cannot do a bind merge of %d trees\n",
o->merge_size);
if (a && old)
- return error("Entry '%s' overlaps. Cannot bind.", a->name);
+ return o->gently ? -1 :
+ error("Entry '%s' overlaps. Cannot bind.", a->name);
if (!a)
return keep_entry(old, o);
else
diff --git a/unpack-trees.h b/unpack-trees.h
index 5517faa..619950d 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -16,6 +16,7 @@ struct unpack_trees_options {
int trivial_merges_only;
int verbose_update;
int aggressive;
+ int gently;
const char *prefix;
int pos;
struct dir_struct *dir;
--
1.5.4.rc3.4.g16335
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/9] Add flag to make unpack_trees() not print errors.
@ 2008-02-04 18:35 Daniel Barkalow
2008-02-05 1:16 ` Johannes Schindelin
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Barkalow @ 2008-02-04 18:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
This will allow builtin-checkout to suppress merge errors if it's
going to try more merging methods.
Additionally, if unpack_trees() returns with an error, but without
printing anything, it will roll back any changes to the index (by
rereading the index, currently). This obviously could be done by the
caller, but chances are that the caller would forget and debugging
this is difficult.
Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---
unpack-trees.c | 43 +++++++++++++++++++++++++++++--------------
unpack-trees.h | 1 +
2 files changed, 30 insertions(+), 14 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index 9e6587f..45f40c2 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -356,12 +356,23 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
posns[i] = create_tree_entry_list(t+i);
if (unpack_trees_rec(posns, len, o->prefix ? o->prefix : "",
- o, &df_conflict_list))
+ o, &df_conflict_list)) {
+ if (o->gently) {
+ discard_cache();
+ read_cache();
+ }
return -1;
+ }
}
- if (o->trivial_merges_only && o->nontrivial_merge)
- return error("Merge requires file-level merging");
+ if (o->trivial_merges_only && o->nontrivial_merge) {
+ if (o->gently) {
+ discard_cache();
+ read_cache();
+ }
+ return o->gently ? -1 :
+ error("Merge requires file-level merging");
+ }
check_updates(active_cache, active_nr, o);
return 0;
@@ -415,7 +426,8 @@ static int verify_uptodate(struct cache_entry *ce,
}
if (errno == ENOENT)
return 0;
- return error("Entry '%s' not uptodate. Cannot merge.", ce->name);
+ return o->gently ? -1 :
+ error("Entry '%s' not uptodate. Cannot merge.", ce->name);
}
static void invalidate_ce_path(struct cache_entry *ce)
@@ -501,8 +513,9 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action,
d.exclude_per_dir = o->dir->exclude_per_dir;
i = read_directory(&d, ce->name, pathbuf, namelen+1, NULL);
if (i)
- return error("Updating '%s' would lose untracked files in it",
- ce->name);
+ return o->gently ? -1 :
+ error("Updating '%s' would lose untracked files in it",
+ ce->name);
free(pathbuf);
return cnt;
}
@@ -574,8 +587,9 @@ static int verify_absent(struct cache_entry *ce, const char *action,
return 0;
}
- return error("Untracked working tree file '%s' "
- "would be %s by merge.", ce->name, action);
+ return o->gently ? -1 :
+ error("Untracked working tree file '%s' "
+ "would be %s by merge.", ce->name, action);
}
return 0;
}
@@ -707,7 +721,7 @@ int threeway_merge(struct cache_entry **stages,
/* #14, #14ALT, #2ALT */
if (remote && !df_conflict_head && head_match && !remote_match) {
if (index && !same(index, remote) && !same(index, head))
- return reject_merge(index);
+ return o->gently ? -1 : reject_merge(index);
return merged_entry(remote, index, o);
}
/*
@@ -715,7 +729,7 @@ int threeway_merge(struct cache_entry **stages,
* make sure that it matches head.
*/
if (index && !same(index, head)) {
- return reject_merge(index);
+ return o->gently ? -1 : reject_merge(index);
}
if (head) {
@@ -866,11 +880,11 @@ int twoway_merge(struct cache_entry **src,
/* all other failures */
remove_entry(remove);
if (oldtree)
- return reject_merge(oldtree);
+ return o->gently ? -1 : reject_merge(oldtree);
if (current)
- return reject_merge(current);
+ return o->gently ? -1 : reject_merge(current);
if (newtree)
- return reject_merge(newtree);
+ return o->gently ? -1 : reject_merge(newtree);
return -1;
}
}
@@ -897,7 +911,8 @@ int bind_merge(struct cache_entry **src,
return error("Cannot do a bind merge of %d trees\n",
o->merge_size);
if (a && old)
- return error("Entry '%s' overlaps. Cannot bind.", a->name);
+ return o->gently ? -1 :
+ error("Entry '%s' overlaps. Cannot bind.", a->name);
if (!a)
return keep_entry(old, o);
else
diff --git a/unpack-trees.h b/unpack-trees.h
index 197a004..83d1229 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -16,6 +16,7 @@ struct unpack_trees_options {
int trivial_merges_only;
int verbose_update;
int aggressive;
+ int gently;
const char *prefix;
int pos;
struct dir_struct *dir;
--
1.5.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/9] Add flag to make unpack_trees() not print errors.
2008-02-04 18:35 [PATCH 2/9] Add flag to make unpack_trees() not print errors Daniel Barkalow
@ 2008-02-05 1:16 ` Johannes Schindelin
2008-02-05 20:38 ` Daniel Barkalow
0 siblings, 1 reply; 6+ messages in thread
From: Johannes Schindelin @ 2008-02-05 1:16 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Junio C Hamano, git
Hi,
On Mon, 4 Feb 2008, Daniel Barkalow wrote:
> This will allow builtin-checkout to suppress merge errors if it's going
> to try more merging methods.
I saw one error() in twoway_merge(), one in bind_merge(), and one in
onway_merge() that were not guarded by o->gently.
Also, I'd call it "silent", not "gently".
> Additionally, if unpack_trees() returns with an error, but without
> printing anything, it will roll back any changes to the index (by
> rereading the index, currently). This obviously could be done by the
> caller, but chances are that the caller would forget and debugging this
> is difficult.
Granted, it is easy to forget. But maybe the caller does not need the
index? Or maybe it wants a different one? I'd prefer the caller to clean
up, if necessary.
Ciao,
Dscho
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/9] Add flag to make unpack_trees() not print errors.
2008-02-05 1:16 ` Johannes Schindelin
@ 2008-02-05 20:38 ` Daniel Barkalow
2008-02-05 23:43 ` Junio C Hamano
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Barkalow @ 2008-02-05 20:38 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
On Tue, 5 Feb 2008, Johannes Schindelin wrote:
> Hi,
>
> On Mon, 4 Feb 2008, Daniel Barkalow wrote:
>
> > This will allow builtin-checkout to suppress merge errors if it's going
> > to try more merging methods.
>
> I saw one error() in twoway_merge(), one in bind_merge(), and one in
> onway_merge() that were not guarded by o->gently.
>
> Also, I'd call it "silent", not "gently".
>
> > Additionally, if unpack_trees() returns with an error, but without
> > printing anything, it will roll back any changes to the index (by
> > rereading the index, currently). This obviously could be done by the
> > caller, but chances are that the caller would forget and debugging this
> > is difficult.
>
> Granted, it is easy to forget. But maybe the caller does not need the
> index? Or maybe it wants a different one? I'd prefer the caller to clean
> up, if necessary.
That's what makes it "gently" instead of just "silent"; it has no effect
if it doesn't succeed. Longer term, I'd like to have unpack_trees() unpack
into a separate index, which should actually be faster (since it doesn't
have to keep shifting the entries in the index it's working on) and make
this moot. In any case, it only rolls it back with the option that's only
used by a caller that wants the index unchanged on failure, currently. If
a caller turns out to just want a return code and not care about the index
or the error message, and the code hasn't been reworked, we can add a
separate flag then.
I'd done some analysis at the time that suggested that, if you didn't want
to give a message on failure, you must want to do something else to the
index to replace what hadn't worked, so you must want the index reset, but
I've forgotten why I was so sure at the time, aside from that nobody's
wanted it before now.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/9] Add flag to make unpack_trees() not print errors.
2008-02-05 20:38 ` Daniel Barkalow
@ 2008-02-05 23:43 ` Junio C Hamano
2008-02-06 1:11 ` Daniel Barkalow
0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2008-02-05 23:43 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Johannes Schindelin, git
Daniel Barkalow <barkalow@iabervon.org> writes:
> On Tue, 5 Feb 2008, Johannes Schindelin wrote:
> ...
>> > Additionally, if unpack_trees() returns with an error, but without
>> > printing anything, it will roll back any changes to the index (by
>> > rereading the index, currently). This obviously could be done by the
>> > caller, but chances are that the caller would forget and debugging this
>> > is difficult.
>>
>> Granted, it is easy to forget. But maybe the caller does not need the
>> index? Or maybe it wants a different one? I'd prefer the caller to clean
>> up, if necessary.
>
> That's what makes it "gently" instead of just "silent"; it has no effect
> if it doesn't succeed. Longer term, I'd like to have unpack_trees() unpack
> into a separate index, which should actually be faster (since it doesn't
> have to keep shifting the entries in the index it's working on) and make
> this moot.
Absolutely. That is the original motivation I did the_index
thing for.
But "re-reading" may not be quite nice. It would defeat the
optimization introduced by the change to use CE_UPTODATE flag to
avoid unnecessary lstat(2) calls.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/9] Add flag to make unpack_trees() not print errors.
2008-02-05 23:43 ` Junio C Hamano
@ 2008-02-06 1:11 ` Daniel Barkalow
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Barkalow @ 2008-02-06 1:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
On Tue, 5 Feb 2008, Junio C Hamano wrote:
> Daniel Barkalow <barkalow@iabervon.org> writes:
>
> > On Tue, 5 Feb 2008, Johannes Schindelin wrote:
> > ...
> >> > Additionally, if unpack_trees() returns with an error, but without
> >> > printing anything, it will roll back any changes to the index (by
> >> > rereading the index, currently). This obviously could be done by the
> >> > caller, but chances are that the caller would forget and debugging this
> >> > is difficult.
> >>
> >> Granted, it is easy to forget. But maybe the caller does not need the
> >> index? Or maybe it wants a different one? I'd prefer the caller to clean
> >> up, if necessary.
> >
> > That's what makes it "gently" instead of just "silent"; it has no effect
> > if it doesn't succeed. Longer term, I'd like to have unpack_trees() unpack
> > into a separate index, which should actually be faster (since it doesn't
> > have to keep shifting the entries in the index it's working on) and make
> > this moot.
>
> Absolutely. That is the original motivation I did the_index
> thing for.
>
> But "re-reading" may not be quite nice. It would defeat the
> optimization introduced by the change to use CE_UPTODATE flag to
> avoid unnecessary lstat(2) calls.
For now, I want to re-read the index, because I need the changes undone
and I don't have any way of reverting them. That's another reason to have
unpack_trees return with the index unchanged: it may have a better way of
getting there than the caller does, such as having never changed that
memory in the first place. I'll add doing it efficiently to my list of
things to do, but I want to keep it from being necessary for correctness
of this series.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-02-06 1:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-02-04 18:35 [PATCH 2/9] Add flag to make unpack_trees() not print errors Daniel Barkalow
2008-02-05 1:16 ` Johannes Schindelin
2008-02-05 20:38 ` Daniel Barkalow
2008-02-05 23:43 ` Junio C Hamano
2008-02-06 1:11 ` Daniel Barkalow
-- strict thread matches above, loose matches on Subject: below --
2008-01-25 23:24 Daniel Barkalow
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).