Re: [PATCH v3 3/3] git-merge-one-file: revise merge error reporting
From: David Aguilar <hidden>
Date: 2016-06-15 22:56:22
On Tue, Mar 12, 2013 at 6:12 PM, Kevin Bracey [off-list ref] wrote:
Commit 718135e improved the merge error reporting for the resolve strategy's merge conflict and permission conflict cases, but led to a malformed "ERROR: in myfile.c" message in the case of a file added differently. This commit reverts that change, and uses an alternative approach without this flaw. Signed-off-by: Kevin Bracey <redacted> ---
I wonder whether before these changes we should update the style in this file to follow Documentation/CodingGuidelines. Not in this patch, but in the file right now there's this part that stands out: if [ "$2" ]; then echo "Removing $4" I think that expression would read more clearly as: if test -n "$2" then echo "Removing $4" Ditto `if [ "$1" = '' ]` is better written as `test -z "$1"`. Can you please send a patch to true these up? It'd be especially nice if the style patch could come first, followed by the fixes/features ;-)
quoted hunk ↗ jump to hunk
git-merge-one-file.sh | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-)diff --git a/git-merge-one-file.sh b/git-merge-one-file.sh index 0f164e5..78b07a8 100755 --- a/git-merge-one-file.sh +++ b/git-merge-one-file.sh@@ -104,11 +104,13 @@ case "${1:-.}${2:-.}${3:-.}" in ;; esac + ret=0 src1=$(git-unpack-file $2) src2=$(git-unpack-file $3) case "$1" in '') - echo "Added $4 in both, but differently." + echo "ERROR: Added $4 in both, but differently." + ret=1 orig=$(git-unpack-file $2) create_virtual_base "$orig" "$src2" ;;@@ -121,10 +123,9 @@ case "${1:-.}${2:-.}${3:-.}" in # Be careful for funny filename such as "-L" in "$4", which # would confuse "merge" greatly. git merge-file "$src1" "$orig" "$src2" - ret=$? - msg= - if [ $ret -ne 0 ]; then - msg='content conflict' + if [ $? -ne 0 ]; then + echo "ERROR: Content conflict in $4" + ret=1
if test $? != 0 then Also.. should this error not go to stderr? I guess the existing script was not doing that, but it seems like anything that says "ERROR" should go there.
quoted hunk ↗ jump to hunk
fi # Create the working tree file, using "our tree" version from the@@ -133,18 +134,11 @@ case "${1:-.}${2:-.}${3:-.}" in rm -f -- "$orig" "$src1" "$src2" if [ "$6" != "$7" ]; then - if [ -n "$msg" ]; then - msg="$msg, " - fi - msg="${msg}permissions conflict: $5->$6,$7" - ret=1 - fi - if [ "$1" = '' ]; then + echo "ERROR: Permissions conflict: $5->$6,$7" ret=1 fi if [ $ret -ne 0 ]; then - echo "ERROR: $msg in $4" exit 1 fi exec git update-index -- "$4"
same notes as above. I think a style patch should come first. -- David