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

[Andreas Gruenbacher]

On Thursday 09 June 2005 13:23, Jean Delvare wrote:
> 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)" ) )  <--

Yes, makes sense.

> 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
> }

... even more so. I'll apply that.

-- Andreas.