List Info

Thread: Re: svn commit: r26761 - in branches/svnpatch-diff/subversion: include libsvn_client libsvn_subr sv




Re: svn commit: r26761 - in branches/svnpatch-diff/subversion: include libsvn_client libsvn_subr sv
user name
2007-10-03 18:07:31
On 9/27/07, David Glasser <glasserdavidglasser.net>
wrote:
> Huh, I expected that you would have needed some sort of
-p argument to
> patch.

I didn't want to make patch_cmd_args[] GNU-patch specific
and I had
only tested in a top directory.  Now fixed.

> > +  if (exitcode != 0 && exitcode != 1)
> > +    {
> > +      /* This is the case when we're trying to
execute 'patch' and got
> > +       * some weird exitcode.
> > +       * XXX: I haven't figured out how to check
against the 'command
> > +       * not found' error, which returns exitcode
== 255.  Is there a
> > +       * macro somewhere to compare with?  Let's
use > 2 for now. */
> > +      if (patch_bin_guess && exitcode
> 2)
> > +        return svn_error_createf
> > +                (SVN_ERR_EXTERNAL_PROGRAM, NULL,
> > +                 _("No 'patch' program was
found in your system.  Please tryn"
> > +                   "to use --patch-cmd or
'patch-cmd' run-time configurationn"
> > +                   "option or manually use
an external tool to apply Unidiffs."));
>
> This heuristic seems not to be sound enough.  (And
again, referring to
> --patch-cmd in libsvn_client is bad.)  I would not try
to guess if the
> command existed or not (will the exec itself print an
error if it
> fails?  it does for 'svn diff --diff-cmd
asfdjkasfdjklafds'), and then
> in the svn CLI wrap that error in a warning mentioning
the ways to
> configure.

We have to specify somewhere that the error was either
raised because
the 'patch' binary we're looking for is nowhere to be found,
or
because the external program internally raised an error, so
that we
can adjust and deliver an appropriate message to the user. 
So in
r26915 I added the SVN_ERR_EXTERNAL_PROGRAM_MISSING macro
and moved
the error message handling to a more appropriate place,
subversion/svn/patch-cmd.c, that is.

Most of your review comments were fixed in r26915.  Good
catch on the
indent stuff and CLI-specific bytes in libsvn_*!

Cheers,
Charles

------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesubversion.tigris.org
For additional commands, e-mail: dev-helpsubversion.tigris.org


[1]

about | contact  Other archives ( Real Estate discussion Medical topics )