On Thu, 2010-12-30, Prabhu Gnana Sundar wrote:
[...]
> After a few discussions about the inconsistent behaviour of 'svn diff'
> in different scenarios, Kamesh suggested that let 'svn diff' be done
> against the copy-source by default, making use of the copyfrom info.
>
> Now this patch would do 'diff' against the copy-source by default,
> whereas the prevailing default behaviour(of showing a *modified copy* as
> all adds) can be achieved by using the '--show-copies-as-adds'
> switch(that already exists in the code-base!).
I like this change, in principle.
We will have to decide whether it is backwards-compatible enough or
whether we need to provide an additional compatibility mode. In order
to help us decide, it would be very useful if you could provide a
summary of the behavioural changes. For example, maybe some tables
something like this would be a good way to summarize the changes:
Type of diff shown BEFORE this patch, WITHOUT --show-copies-as-adds:
+-------------------+---------------+----------------------+-----------+
| | copied only | copied and modified | ... |
+-------------------+---------------+----------------------+-----------+
| | | | |
| WC-WC diff | | | |
| | | | |
| WC-repo diff | | | |
| | | | |
| repo-repo diff | | | |
| | | | |
+-------------------+---------------+----------------------+-----------+
Type of diff shown BEFORE this patch, WITH --show-copies-as-adds:
+-------------------+---------------+----------------------+-----------+
Type of diff shown AFTER this patch, WITHOUT --show-copies-as-adds:
+-------------------+---------------+----------------------+-----------+
Type of diff shown AFTER this patch, WITH --show-copies-as-adds:
+-------------------+---------------+----------------------+-----------+
Or whatever rows and columns best convey the information.
Are there any differences with different (old) versions of Subversion
server? (I seem to recall that copy-from data was not always supplied,
but I am not sure of the details.)
> I was quickly in to it and after spending some time in the process I
> came to know that several existing 'test cases' *failed* due to the
> change in the behaviour of the 'svn diff'. Later found that the 'wc
> editor' needs to be taught to handle this behaviour (handling the
> copyfrom info and retrieving it from the repository as against working
> copy text-base).
>
> This took quite some amount of time...
>
> See [1] below of the mailer.py usecase which tries to get the same
> output as my patch do.
>
> May be that's the behaviour of the 'diff' that is prefered ? :)
>
> I have attached the patch and the log message with this mail. Please
> review and respond.
I only had a very quick look at the patch. The first thing that catches
my eye is this:
> * subversion/libsvn_wc/diff.c
> (): included the svn_ra.h header file.
> (): persisted the ra_session, copyfrom file, copyfrom path and pristine properties
> in the edit_baton.
> (make_edit_baton) : introduced the ra_session.
> (get_file_from_ra): newly added to fetch file from repository.
> (add_file) : support copyfrom by making use of the copyfrom info.
> (apply_textdelta) : makes use of the copyfrom_temp file making use of the
> copyfrom info.
> (close_file) : makes use of the copyfrom info and tweaked the file_changed to
> display the copyfrom revision number.
> (svn_wc_get_diff_editor6) : introduced the ra_session.
> pass NULL for ra_session to make_edit_baton.
Isn't it a layering violation for libsvn_wc to know about libsvn_ra?
Maybe this needs to use callbacks or something, so that all the RA
knowledge remains in libsvn_client.
- Julian
Received on 2011-01-05 12:42:33 CET