On Tue, Mar 12, 2013 at 6:12 PM, Kevin Bracey <[email protected]> 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 <[email protected]>
> ---
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 ;-)
> 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.
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
