You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Sander Striker <st...@apache.org> on 2003/02/14 15:44:58 UTC
Diff plugins
Hi,
I was thinking about the diff plugins... I think we can use the
same scheme as what we do with libsvn_auth, create diff providers.
The default providers will be our internal ones, others can be
added by third parties.
Something like:
svn_diff_provider_t *svn_diff_provider_obtain(const char *mime_type, ...);
svn_error_t *svn_diff_provider_register(svn_diff_provider_t *provider,
int priority,
apr_pool_t *pool);
struct svn_diff_provider_t
{
svn_boolean_t (*match_fn)(const char *mime_type);
svn_error_t * (*diff_fn)(apr_file_t *output_file,
const char *path1, const char *path2,
const char *label1, const char *label2,
const char *mime-type; /* XXX: provider could match
multiple mime-types. */
svn_config_t *config /* XXX: maybe?? */
apr_pool_t *pool);
};
The default chain would look like:
-> internal_diff_provider -> internal_binary_diff_provider
(matches text/*) (matches *)
The internal_diff_provider would provide unified diff output like we
are used to today (no -x ... yet, we could add that later).
The internal_binary_diff_provider would just output: X an Y differ/are
the same.
Now (if the user defined some config option?) we could insert
external_diff_provider in front of the chain. Implementing
external_diff_provider is probably just moving svn_io_run_diff
and adding a match function. This isolates the entire return code of 0, 1 or
2 mess in the external_diff_provider.
Idea (making the above problem worse ;): let the external_diff_provider
read a config file which maps { mime-type : path/to/diff_of_choice }.
The match function the simply checks if there is an entry in the map.
Thoughts? Anyone got time to implement this? I'll take care of the
internal_diff_provider if need be.
Sander
PS. Since I don't have the time to implement this idea, my commits to the
issue-405-internal-diff branch do not accomodate for this yet.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: svn diff uses internal diff!, WAS: RE: Diff plugins
Posted by Sander Striker <st...@apache.org>.
> From: Greg Stein [mailto:gstein@lyra.org]
> Sent: Sunday, February 16, 2003 10:59 PM
> On Sun, Feb 16, 2003 at 11:24:26AM +0100, Sander Striker wrote:
>> I don't think you are seeing the big picture here. Merging could, next to
>> 'merge', happen on 'update' and 'switch'. Then it operates on local mods
>> that aren't backed up...
>
> Then why'd you ask if you aren't even comfortable switching it? :-)
Well, I am, but am not so sure if everyone else is. Just being nice ;)
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn diff uses internal diff!, WAS: RE: Diff plugins
Posted by Greg Stein <gs...@lyra.org>.
On Sun, Feb 16, 2003 at 11:24:26AM +0100, Sander Striker wrote:
> > From: Greg Stein [mailto:gstein@lyra.org]
> > Sent: Sunday, February 16, 2003 6:12 AM
>
> > On Sat, Feb 15, 2003 at 01:46:26AM +0100, Sander Striker wrote:
> >>...
> >> Ok, we can get rid of diff-cmd and SVN_CLIENT_DIFF right now. The diff3
> >> stuff is a bit different. I haven't merged over the internal merge code
> >> yet because there is no opt out (yet). I'm guessing we will need a
> >> merge-cmd option (and we will need a merge_fn once we move to the provider
> >> scheme). Anyway, thoughts on whether we should move to internal or hold
> >> off on it until we have an opt-out?
> >
> > Switch it. The opt-out isn't required; we're only asking for it for safety's
> > sake. Worst case, somebody can always get the two files and diff them.
>
> I don't think you are seeing the big picture here. Merging could, next to
> 'merge', happen on 'update' and 'switch'. Then it operates on local mods
> that aren't backed up...
Then why'd you ask if you aren't even comfortable switching it? :-)
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn diff uses internal diff!
Posted by Branko Čibej <br...@xbc.nu>.
Sander Striker wrote:
>>From: Philip Martin
>>Sent: Sunday, February 16, 2003 2:32 PM
>>
>>
>
>[...]
>
>
>>I have no problem with the internal diff/merge being enabled as the
>>default right now, but until we have more regression tests I think we
>>should provide an option to allow a paranoid user to disable use of
>>the internal library and fall back on the external programs.
>>
>>
>
>I've enabled it now. Paranoid users can disable it by setting diff3-cmd
>in .subversion/config. I was hoping that I also had the --diff3-cmd
>on the commandline covered, but Garrett told me otherwise. It seems that
>the config hash is not passed through libsvn_wc/log.c. Internally it
>creates a new config hash and passes that to svn_wc_merge, discarding the
>overrides.
>
>
The regression tests on Windows pass (yes, I did remember to coment out
the diff settings in the config file).
--
Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn diff uses internal diff!
Posted by Philip Martin <ph...@codematters.co.uk>.
"Sander Striker" <st...@apache.org> writes:
> I was hoping that I also had the --diff3-cmd on the commandline
> covered, but Garrett told me otherwise.
I think it would be a good idea to tweak the log message for r4910 to
reflect this.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: svn diff uses internal diff!
Posted by Sander Striker <st...@apache.org>.
> From: Philip Martin
> Sent: Sunday, February 16, 2003 2:32 PM
[...]
> I have no problem with the internal diff/merge being enabled as the
> default right now, but until we have more regression tests I think we
> should provide an option to allow a paranoid user to disable use of
> the internal library and fall back on the external programs.
I've enabled it now. Paranoid users can disable it by setting diff3-cmd
in .subversion/config. I was hoping that I also had the --diff3-cmd
on the commandline covered, but Garrett told me otherwise. It seems that
the config hash is not passed through libsvn_wc/log.c. Internally it
creates a new config hash and passes that to svn_wc_merge, discarding the
overrides.
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn diff uses internal diff!, WAS: RE: Diff plugins
Posted by Philip Martin <ph...@codematters.co.uk>.
Greg Stein <gs...@lyra.org> writes:
> On Sat, Feb 15, 2003 at 01:46:26AM +0100, Sander Striker wrote:
> >...
> > Ok, we can get rid of diff-cmd and SVN_CLIENT_DIFF right now. The diff3
> > stuff is a bit different. I haven't merged over the internal merge code
> > yet because there is no opt out (yet). I'm guessing we will need a
> > merge-cmd option (and we will need a merge_fn once we move to the provider
> > scheme). Anyway, thoughts on whether we should move to internal or hold
> > off on it until we have an opt-out?
>
> Switch it. The opt-out isn't required; we're only asking for it for safety's
> sake. Worst case, somebody can always get the two files and diff them. Or
> just wait a few days for the functionality to arrive.
I think that's a bit premature. We need some more merge regression
tests, in particular more extensive conflict detection. We need to
make sure that the internal merge is not going to lose local
modifications during an update. I'm thinking of scenarios like: I add
four new lines to a file, the update is going to merge two new lines
that happen to exactly match two of my new lines. In that sort of
case it's important that internal merge doesn't destroy my local
change. The straightforward way to "backup" my local mods before
running update is to use diff, but if that uses the internal diff and
has a bug then the "backup" may fail.
I have no reason to believe that there is any failure to detect
conflicts with the current internal merge, but without a regression
test such a problem could get introduced at any time.
I have no problem with the internal diff/merge being enabled as the
default right now, but until we have more regression tests I think we
should provide an option to allow a paranoid user to disable use of
the internal library and fall back on the external programs.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: svn diff uses internal diff!, WAS: RE: Diff plugins
Posted by Sander Striker <st...@apache.org>.
> From: Greg Stein [mailto:gstein@lyra.org]
> Sent: Sunday, February 16, 2003 6:12 AM
> On Sat, Feb 15, 2003 at 01:46:26AM +0100, Sander Striker wrote:
>>...
>> Ok, we can get rid of diff-cmd and SVN_CLIENT_DIFF right now. The diff3
>> stuff is a bit different. I haven't merged over the internal merge code
>> yet because there is no opt out (yet). I'm guessing we will need a
>> merge-cmd option (and we will need a merge_fn once we move to the provider
>> scheme). Anyway, thoughts on whether we should move to internal or hold
>> off on it until we have an opt-out?
>
> Switch it. The opt-out isn't required; we're only asking for it for safety's
> sake. Worst case, somebody can always get the two files and diff them.
I don't think you are seeing the big picture here. Merging could, next to
'merge', happen on 'update' and 'switch'. Then it operates on local mods
that aren't backed up...
> Or just wait a few days for the functionality to arrive.
Working on it.
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn diff uses internal diff!, WAS: RE: Diff plugins
Posted by Greg Stein <gs...@lyra.org>.
On Sat, Feb 15, 2003 at 01:46:26AM +0100, Sander Striker wrote:
>...
> Ok, we can get rid of diff-cmd and SVN_CLIENT_DIFF right now. The diff3
> stuff is a bit different. I haven't merged over the internal merge code
> yet because there is no opt out (yet). I'm guessing we will need a
> merge-cmd option (and we will need a merge_fn once we move to the provider
> scheme). Anyway, thoughts on whether we should move to internal or hold
> off on it until we have an opt-out?
Switch it. The opt-out isn't required; we're only asking for it for safety's
sake. Worst case, somebody can always get the two files and diff them. Or
just wait a few days for the functionality to arrive.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: svn diff uses internal diff!, WAS: RE: Diff plugins
Posted by Sander Striker <st...@apache.org>.
> From: Sander Striker [mailto:striker@apache.org]
> Sent: Saturday, February 15, 2003 1:46 AM
>> From: cmpilato@collab.net [mailto:cmpilato@collab.net]
>> Sent: Friday, February 14, 2003 5:00 PM
>
>> Speaking of diff, how close are we to having internal diff, man?
[...]
>> I can't wait until we can lose such configuration and compile-time
>> things as diff-cmd, diff3-cmd, diff3-has-program-arg, SVN_CLIENT_DIFF,
>> SVN_CLIENT_DIFF3, SVN_DIFF3_HAS_DIFF_PROGRAM_ARG, the 'config' params
> > to svn_io_run_diff(), svn_io_run_diff3(), svn_wc_merge() ...
>
> Ok, we can get rid of diff-cmd and SVN_CLIENT_DIFF right now. The diff3
> stuff is a bit different. I haven't merged over the internal merge code
> yet because there is no opt out (yet). I'm guessing we will need a
> merge-cmd option (and we will need a merge_fn once we move to the provider
> scheme). Anyway, thoughts on whether we should move to internal or hold
> off on it until we have an opt-out?
Btw, internal merge is currently only one touched file:
subversion/libsvn_wc/merge.c (on branches/issue-405-internal-diff/).
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
svn diff uses internal diff!, WAS: RE: Diff plugins
Posted by Sander Striker <st...@apache.org>.
> From: cmpilato@collab.net [mailto:cmpilato@collab.net]
> Sent: Friday, February 14, 2003 5:00 PM
> Speaking of diff, how close are we to having internal diff, man?
Very close... as in, we have it.
'svn diff' uses internal diff to generate the diff. -x is useless (for
now).
'svn diff --diff-cmd /path/to/diff' uses /path/to/diff to generate the
diff. You can make this permanent by setting diff-cmd = /path/to/diff
in the [helpers] section of ~/.subversion/config. -x passes arguments
to the external diff command.
> I can't wait until we can lose such configuration and compile-time
> things as diff-cmd, diff3-cmd, diff3-has-program-arg, SVN_CLIENT_DIFF,
> SVN_CLIENT_DIFF3, SVN_DIFF3_HAS_DIFF_PROGRAM_ARG, the 'config' params
> to svn_io_run_diff(), svn_io_run_diff3(), svn_wc_merge() ...
Ok, we can get rid of diff-cmd and SVN_CLIENT_DIFF right now. The diff3
stuff is a bit different. I haven't merged over the internal merge code
yet because there is no opt out (yet). I'm guessing we will need a
merge-cmd option (and we will need a merge_fn once we move to the provider
scheme). Anyway, thoughts on whether we should move to internal or hold
off on it until we have an opt-out?
> The list of cleanups surely goes on and one.
*nod*
> Make it happen, Sander. I'm begging you.
I'm trying, I'm trying. Only so many hours in a day...
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: Diff plugins
Posted by Sander Striker <st...@apache.org>.
> From: Sander Striker [mailto:striker@apache.org]
> Sent: Friday, February 14, 2003 5:12 PM
> Bleck! Forget it, I just see a few problems (API changes after updating, bah).
> I'll commit a fix for that shortly.
Retracting that. The changes are exactly what was holding up merging back.
I'll commit after dinner.
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: Diff plugins
Posted by Sander Striker <st...@apache.org>.
> From: Sander Striker [mailto:striker@apache.org]
> Sent: Friday, February 14, 2003 5:07 PM
>> From: cmpilato@collab.net [mailto:cmpilato@collab.net]
>> Sent: Friday, February 14, 2003 5:00 PM
>> Make it happen, Sander. I'm begging you.
>
> Right now it only needs a fix for passing a --diff-cmd argument down
> to svn_io_run_diff. Otherwise it's mostly done:
>
> 4 files touched, switch them (or fix the above bug, I don't have the
> time to do it now, and merge it to trunk):
>
> In /branches/issue-405-internal-diff/
>
> 3 for internal diff:
> subversion/clients/cmdline/cl.h
> subversion/clients/cmdline/main.c
> subversion/libsvn_client/diff.c
>
> 1 for internal merge:
> subversion/libsvn_wc/merge.c
Bleck! Forget it, I just see a few problems (API changes after updating, bah).
I'll commit a fix for that shortly.
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: Diff plugins
Posted by Sander Striker <st...@apache.org>.
> From: cmpilato@collab.net [mailto:cmpilato@collab.net]
> Sent: Friday, February 14, 2003 5:00 PM
> "Sander Striker" <st...@apache.org> writes:
>
>> Hi,
>>
>>
>> I was thinking about the diff plugins... I think we can use the
>> same scheme as what we do with libsvn_auth, create diff providers.
>
> Speaking of diff, how close are we to having internal diff, man?
>
> I can't wait until we can lose such configuration and compile-time
> things as diff-cmd, diff3-cmd, diff3-has-program-arg, SVN_CLIENT_DIFF,
> SVN_CLIENT_DIFF3, SVN_DIFF3_HAS_DIFF_PROGRAM_ARG, the 'config' params
> to svn_io_run_diff(), svn_io_run_diff3(), svn_wc_merge() ...
>
> The list of cleanups surely goes on and one.
>
> Make it happen, Sander. I'm begging you.
Right now it only needs a fix for passing a --diff-cmd argument down
to svn_io_run_diff. Otherwise it's mostly done:
4 files touched, switch them (or fix the above bug, I don't have the
time to do it now, and merge it to trunk):
In /branches/issue-405-internal-diff/
3 for internal diff:
subversion/clients/cmdline/cl.h
subversion/clients/cmdline/main.c
subversion/libsvn_client/diff.c
1 for internal merge:
subversion/libsvn_wc/merge.c
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Diff plugins
Posted by cm...@collab.net.
"Sander Striker" <st...@apache.org> writes:
> Hi,
>
>
> I was thinking about the diff plugins... I think we can use the
> same scheme as what we do with libsvn_auth, create diff providers.
Speaking of diff, how close are we to having internal diff, man?
I can't wait until we can lose such configuration and compile-time
things as diff-cmd, diff3-cmd, diff3-has-program-arg, SVN_CLIENT_DIFF,
SVN_CLIENT_DIFF3, SVN_DIFF3_HAS_DIFF_PROGRAM_ARG, the 'config' params
to svn_io_run_diff(), svn_io_run_diff3(), svn_wc_merge() ...
The list of cleanups surely goes on and one.
Make it happen, Sander. I'm begging you.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org