Better patch for fixing svnadmin hotcopy handling of symlinks

[Nico Kadel-Garcia <nk...@comcast.net>]

I wrote about this before: the "svnadmin hotcopy" tool does not successfully 
replicate symlinks in the repositories.

Someone sent me a nice patch for the "repos.c" file, which added the 
capability of duplicating symlinks, but I also had to add a patch to io.c in 
order to actually have the function called as the walk through the 
directories lists files and directories to move.

I'm attaching a copy of the patch against the current trunk for subversion, 
I'll also include it in the issue tracker page I created.

            Nico Kadel-Garcia
            nkadel@comcast.net

Re: Better patch for fixing svnadmin hotcopy handling of symlinks

[Garrett Rooney <ro...@electricjellyfish.net>]

On 8/9/06, Nico Kadel-Garcia <nk...@comcast.net> wrote:
> Garrett Rooney wrote:
> > On 8/7/06, Nico Kadel-Garcia <nk...@comcast.net> wrote:
> >> Garrett Rooney wrote:
> >>> On 8/5/06, Nico Kadel-Garcia <nk...@comcast.net> wrote:
> >>>> I wrote about this before: the "svnadmin hotcopy" tool does not
> >>>> successfully replicate symlinks in the repositories.
> >>>>
> >>>> Someone sent me a nice patch for the "repos.c" file, which added
> >>>> the capability of duplicating symlinks, but I also had to add a
> >>>> patch to io.c in order to actually have the function called as the
> >>>> walk through the directories lists files and directories to move.
> >>>>
> >>>> I'm attaching a copy of the patch against the current trunk for
> >>>> subversion, I'll also include it in the issue tracker page I
> >>>> created.
> >>>
> >>> The patch seems mostly reasonable, but the bit where you fprintf to
> >>> stderr in the case of something you don't understand is just wrong.
> >>> We can't just spew stuff to stderr in a library function like that,
> >>> there's no way to tell if anyone's even looking at stderr, this
> >>> could be embedded in a GUI app or something.  If we NEED a way to
> >>> warn without erroring, we have to do it via some sort of
> >>> callback/baton pair, otherwise treat it as an error and return an
> >>> svn_error_t just like any other problem.  Or just look at the other
> >>> potential cases and figure out if they need to be handled, if not
> >>> just ignore them.
> >>
> >> A reported error is *fine* with me! I wasn't sure what best to
> >> inject, and I'm always ware of functions relying on being called
> >> only with correct arguments. There should have been a reported error
> >> in the first place, rather than a silently failed function. Do you
> >> know the best syntax for that return?
> >
> > Just return an svn_error_t, like every other error return in that
> > file...
>
> My preliminary attempt to add such an error report is not working well: I'm
> afraid that I would have to travel back up the pyramid of functions to
> really establish some error reporting. Is anyone else more comfortable doing
> that?

I'd suggest bringing questions like that to the dev list, if you tell
us what's going wrong with your patch, perhaps we can help you fix it.

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
For additional commands, e-mail: users-help@subversion.tigris.org

Re: Better patch for fixing svnadmin hotcopy handling of symlinks

[Nico Kadel-Garcia <nk...@comcast.net>]

Garrett Rooney wrote:
> On 8/7/06, Nico Kadel-Garcia <nk...@comcast.net> wrote:
>> Garrett Rooney wrote:
>>> On 8/5/06, Nico Kadel-Garcia <nk...@comcast.net> wrote:
>>>> I wrote about this before: the "svnadmin hotcopy" tool does not
>>>> successfully replicate symlinks in the repositories.
>>>>
>>>> Someone sent me a nice patch for the "repos.c" file, which added
>>>> the capability of duplicating symlinks, but I also had to add a
>>>> patch to io.c in order to actually have the function called as the
>>>> walk through the directories lists files and directories to move.
>>>>
>>>> I'm attaching a copy of the patch against the current trunk for
>>>> subversion, I'll also include it in the issue tracker page I
>>>> created.
>>>
>>> The patch seems mostly reasonable, but the bit where you fprintf to
>>> stderr in the case of something you don't understand is just wrong.
>>> We can't just spew stuff to stderr in a library function like that,
>>> there's no way to tell if anyone's even looking at stderr, this
>>> could be embedded in a GUI app or something.  If we NEED a way to
>>> warn without erroring, we have to do it via some sort of
>>> callback/baton pair, otherwise treat it as an error and return an
>>> svn_error_t just like any other problem.  Or just look at the other
>>> potential cases and figure out if they need to be handled, if not
>>> just ignore them.
>>
>> A reported error is *fine* with me! I wasn't sure what best to
>> inject, and I'm always ware of functions relying on being called
>> only with correct arguments. There should have been a reported error
>> in the first place, rather than a silently failed function. Do you
>> know the best syntax for that return?
>
> Just return an svn_error_t, like every other error return in that
> file...

My preliminary attempt to add such an error report is not working well: I'm 
afraid that I would have to travel back up the pyramid of functions to 
really establish some error reporting. Is anyone else more comfortable doing 
that? 

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
For additional commands, e-mail: users-help@subversion.tigris.org

Re: Better patch for fixing svnadmin hotcopy handling of symlinks

[Garrett Rooney <ro...@electricjellyfish.net>]

On 8/7/06, Nico Kadel-Garcia <nk...@comcast.net> wrote:
> Garrett Rooney wrote:
> > On 8/5/06, Nico Kadel-Garcia <nk...@comcast.net> wrote:
> >> I wrote about this before: the "svnadmin hotcopy" tool does not
> >> successfully replicate symlinks in the repositories.
> >>
> >> Someone sent me a nice patch for the "repos.c" file, which added the
> >> capability of duplicating symlinks, but I also had to add a patch to
> >> io.c in order to actually have the function called as the walk
> >> through the directories lists files and directories to move.
> >>
> >> I'm attaching a copy of the patch against the current trunk for
> >> subversion, I'll also include it in the issue tracker page I created.
> >
> > The patch seems mostly reasonable, but the bit where you fprintf to
> > stderr in the case of something you don't understand is just wrong.
> > We can't just spew stuff to stderr in a library function like that,
> > there's no way to tell if anyone's even looking at stderr, this could
> > be embedded in a GUI app or something.  If we NEED a way to warn
> > without erroring, we have to do it via some sort of callback/baton
> > pair, otherwise treat it as an error and return an svn_error_t just
> > like any other problem.  Or just look at the other potential cases and
> > figure out if they need to be handled, if not just ignore them.
>
> A reported error is *fine* with me! I wasn't sure what best to inject, and
> I'm always ware of functions relying on being called only with correct
> arguments. There should have been a reported error in the first place,
> rather than a silently failed function. Do you know the best syntax for that
> return?

Just return an svn_error_t, like every other error return in that file...

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
For additional commands, e-mail: users-help@subversion.tigris.org

Re: Better patch for fixing svnadmin hotcopy handling of symlinks

[Nico Kadel-Garcia <nk...@comcast.net>]

Garrett Rooney wrote:
> On 8/5/06, Nico Kadel-Garcia <nk...@comcast.net> wrote:
>> I wrote about this before: the "svnadmin hotcopy" tool does not
>> successfully replicate symlinks in the repositories.
>>
>> Someone sent me a nice patch for the "repos.c" file, which added the
>> capability of duplicating symlinks, but I also had to add a patch to
>> io.c in order to actually have the function called as the walk
>> through the directories lists files and directories to move.
>>
>> I'm attaching a copy of the patch against the current trunk for
>> subversion, I'll also include it in the issue tracker page I created.
>
> The patch seems mostly reasonable, but the bit where you fprintf to
> stderr in the case of something you don't understand is just wrong.
> We can't just spew stuff to stderr in a library function like that,
> there's no way to tell if anyone's even looking at stderr, this could
> be embedded in a GUI app or something.  If we NEED a way to warn
> without erroring, we have to do it via some sort of callback/baton
> pair, otherwise treat it as an error and return an svn_error_t just
> like any other problem.  Or just look at the other potential cases and
> figure out if they need to be handled, if not just ignore them.

A reported error is *fine* with me! I wasn't sure what best to inject, and 
I'm always ware of functions relying on being called only with correct 
arguments. There should have been a reported error in the first place, 
rather than a silently failed function. Do you know the best syntax for that 
return? 

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
For additional commands, e-mail: users-help@subversion.tigris.org

Re: Better patch for fixing svnadmin hotcopy handling of symlinks

[Garrett Rooney <ro...@electricjellyfish.net>]

On 8/5/06, Nico Kadel-Garcia <nk...@comcast.net> wrote:
> I wrote about this before: the "svnadmin hotcopy" tool does not successfully
> replicate symlinks in the repositories.
>
> Someone sent me a nice patch for the "repos.c" file, which added the
> capability of duplicating symlinks, but I also had to add a patch to io.c in
> order to actually have the function called as the walk through the
> directories lists files and directories to move.
>
> I'm attaching a copy of the patch against the current trunk for subversion,
> I'll also include it in the issue tracker page I created.

The patch seems mostly reasonable, but the bit where you fprintf to
stderr in the case of something you don't understand is just wrong.
We can't just spew stuff to stderr in a library function like that,
there's no way to tell if anyone's even looking at stderr, this could
be embedded in a GUI app or something.  If we NEED a way to warn
without erroring, we have to do it via some sort of callback/baton
pair, otherwise treat it as an error and return an svn_error_t just
like any other problem.  Or just look at the other potential cases and
figure out if they need to be handled, if not just ignore them.

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@subversion.tigris.org
For additional commands, e-mail: users-help@subversion.tigris.org