Hi Benjamin, Le vendredi 30 novembre 2012 à 16:43 -0500, Benjamin Poirier a écrit : > Because of the extra condition that was present, in the case where patching > the original file(s) fails, the situation was incorrectly reported as "Patch > [...] does not remove cleanly" whereas it should be "Failed to patch temporary > files". > --- > quilt/pop.in | 9 +++------ > 1 files changed, 3 insertions(+), 6 deletions(-) > > diff --git a/quilt/pop.in b/quilt/pop.in > index efacf09..8b69f64 100644 > --- a/quilt/pop.in > +++ b/quilt/pop.in > @@ -111,12 +111,9 @@ check_for_pending_changes() > --no-backup-if-mismatch -E \ > >/dev/null 2>/dev/null > then > - if ! [ -e $QUILT_PC/$patch ] > - then > - printf $"Failed to patch temporary files\n" >&2 > - rm -rf $workdir > - return 1 > - fi > + printf $"Failed to patch temporary files\n" >&2 > + rm -rf $workdir > + return 1 > fi > fi >
This change breaks the test suite: [failpop.test] [1] $ mkdir patches -- ok [3] $ cat > test.txt -- ok [6] $ quilt new test.diff -- ok [9] $ quilt add test.txt -- ok [12] $ cat >> test.txt -- ok [15] $ quilt refresh -- ok [18] $ sleep 2 -- ok [19] $ cat patches/test.diff > /tmp/test.diff -- ok [21] $ sed -e "s/ /_/g" patches/test.diff > patches/test.new -- ok [22] $ cat patches/test.new > /tmp/test.new -- ok [23] $ mv patches/test.new patches/test.diff -- ok [24] $ quilt pop -- failed Failed to patch temporary files != Patch patches/test.diff does not remove cleanly (refresh it or enforce with -f) 12 commands (11 passed, 1 failed) I am not claiming that this test makes a lot of sense... Command [21] trashes the patch file completely, leaving patch only garbage to process. I am not sure what was the intent, as this test is not very representative of real-world situations. I am also skeptical about the tests on "$QUILT_PC/$patch" in function check_for_pending_changes(). I just can't see how "$QUILT_PC/$patch" could not exist. It is created unconditionally by "quilt new" and "quilt push". Except by sabotaging "$QUILT_PC" manually, I don't think the error message "Failed to patch temporary files" can ever be printed. If someone can think of a scenario leading to this message being printed, please let me know. So I am fine dropping the test ! [ -e "$QUILT_PC/$patch" ] as you suggested. But I'm now sure what to do when patching fails. The unexpected failure due to the files being read-only is not the only case that would trigger the error. For example, a patch with just a header and no contents would do as well. Unfortunately "patch" returns with status 2 in both cases, so there's no easy way to differentiate. While the read-only error is something that should get fixed and thus no longer happen, patches with just headers is something we want to keep supporting, at least with push -f and pop -f. So I won't take your patch as is, it needs more work. -- Jean Delvare Suse L3 _______________________________________________ Quilt-dev mailing list [email protected] https://lists.nongnu.org/mailman/listinfo/quilt-dev
