Re: [Quilt-dev] [PATCH] Add a rename command

[Jean Delvare]

Hi Andreas,

[Jean Delvare]
> +if ( is_applied $patch && \
> +     ( ! rename_in_db "$patch" "$new_patch" || \
> +       ! mv "$QUILT_PC/$patch" "$QUILT_PC/$new_patch" ) ) || \
> +   ! rename_in_series "$patch" "$new_patch" || \
> +   ( [ -e "$(patch_file_name $patch)" ] && \
> +     ! mv "$(patch_file_name $patch)" \
> +       "$(patch_file_name $new_patch)" )

[Andreas Gruenbacher]
> This will fail when new_patch contains new path components, like:
>
>   quilt rename -p old.diff newdir/new.diff

Ah, correct, I am usually not working with subdirectories myself so I
didn't notice, my bad. More on this below...

[Andreas Gruenbacher]
> I would rather like to keep details like the .pc directory out of the
> test suite as far as possible: someone might use a different backend one
> day, and then those tests would fail. I think we are fine relying on the
> applied, series, etc. commands, and trying to pop/push the patch here
> should make sure enough that everything is fine here, no?

Yes, your approach should work equally well. I thought the test suite was
meant for both the interface and the backend, but it indeed isn't.

> I assume you are fine with these changes?

Changes to the test case are fine with me, but changes to have the new
subdirectories properly created aren't. I applied them this morning
quickly tried and it failed.

[Andreas Gruenbacher]
> --- quilt/rename.in   8 Jun 2005 20:40:54 -0000       1.1
> +++ quilt/rename.in   8 Jun 2005 20:48:55 -0000
> @@ -89,9 +89,11 @@
> 
>  if ( is_applied $patch && \
>       ( ! rename_in_db "$patch" "$new_patch" || \
> +       ! mkdir -p "$(dirname "$new_patch")" || \
>         ! mv "$QUILT_PC/$patch" "$QUILT_PC/$new_patch" ) ) || \
>     ! rename_in_series "$patch" "$new_patch" || \
>     ( [ -e "$(patch_file_name $patch)" ] && \
> +     ! mkdir -p "$(dirname "$(patch_file_name $new_patch)")" || \
>       ! mv "$(patch_file_name $patch)" \
>         "$(patch_file_name $new_patch)" )
>  then

On second reading, I think the problem is a missing level of parentheses
near the end of the expression. Shouldn't it be:

   ( [ -e "$(patch_file_name $patch)" ] && \
-->  ( ! mkdir -p "$(dirname "$(patch_file_name $new_patch)")" || \
       ! mv "$(patch_file_name $patch)" \
            "$(patch_file_name $new_patch)" ) )  <--

? Can't test right now...

Or maybe we can make the whole thing a little bit more readable by having
a rename_file() function:

rename_file()
{
        local old=$1
        local new=$2
        local newdir=$(dirname "$new")

        [ -d "$dirname" ] || mkdir -p "$dirname" || return 1
        mv "$old" "$new" || return 1
        rmdir -p $(dirname "$old") 2> /dev/null

        return 0
}

This would have the additional benefit of calling mkdir only when needed,
and removing empty directories as needed. But I have no strong opinion
on this, either way will be fine with me.

Thanks,
--
Jean Delvare