You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Paul Burba <pt...@gmail.com> on 2012/10/30 21:19:35 UTC
Re: [Bug] [Subversion 1.7] svn blame doesn't work for locally
modified files
On Fri, Oct 14, 2011 at 6:49 AM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> [ switching list, please drop users@ from further replies ]
>
> Konstantin Kolinko wrote on Fri, Oct 14, 2011 at 11:43:14 +0400:
>> 2011/10/14 Andrey Paramonov <pa...@acdlabs.ru>:
>> > On 13.10.2011 22:31, Ryan Schmidt wrote:
>> >>
>> >> On Oct 13, 2011, at 06:51, Andrey Paramonov wrote:
>> >>
>> >>>>> I believe Subversion can automagically translate line ending sequences
>> >>>>> when transferring data to and from server. I use Windows, so I have CRLF
>> >>>>> sequences at the ends of the lines in my working copy. The question is: what
>> >>>>> is the EOL sequence on the server?
>> >>
>> >>
>> >> On Oct 13, 2011, at 07:06, Andrey Paramonov wrote:
>> >>>
>> >>> On 13.10.2011 15:54, Daniel Shahaf wrote:
>> >>>>
>> >>>> What properties are set on your file?
>> >>>
>> >>> svn proplist returns nothing.
>> >>
>> >> If the svn:eol-style property is not set on the file, then Subversion does
>> >> not modify the file in any way as it's stored in the repository. The EOL
>> >> sequence (and every other byte in the file) is the same on the server as it
>> >> is on the client.
>> >>
>> >> If, on the other hand, svn:eol-style is set to any valid value, then
>> >> Subversion stores the file in the repository with LF line endings, and on
>> >> checkout or export, the client translates the line endings to the style
>> >> requested in the svn:eol-style property.
>> >>
>> >
>> > By looking through the code in libsvn_client/blame.c I see that in
>> > svn_client_blame5 working copy version of the file is unconditionally
>> > normalized to have "\n" EOLs (by call to svn_subst_stream_translated).
>> > However, it seems that svn_ra_get_file_revs2 doesn't do EOL normalization,
>> > at least in my case (absent svn:eol-style).
>>
>> Confirming that the issue reproduces for me on Windows using a file
>> that does not have svn:eol-style property.
>>
>> The svn blame filename@BASE, as mentioned earlier, works correctly,
>> blaming unmodified base version of the file.
>>
>>
>> If a file has svn:eol-style=native the issue does not reproduce
>> and blame works correctly.
>>
>>
>
> @Both, thanks for the analysis. Could someone file a bug report and/or
> send a patch, too? If you have a build environment, does the patch
> below fix the problem?
Hi All,
Mike Pilato is tying off loose ends for the upcoming 1.8 release and
asked me to check this patch for issue #4034 on my Windows machine.
The patch doesn't appear to make any difference:
With three files, with three different eols as per their names:
trunk@1403813>svn st -v
9 9 pburba .
9 9 pburba file-with-CR-eols
9 9 pburba file-with-CRLF-eols
9 9 pburba file-with-LF-eols
All three files have the same contents but for eols:
trunk@1403813>type file-with-CRLF-eols
A
B
C
D
E
F
Blame works fine with no local mods as expected:
trunk@1403813>svn blame file-with-CR-eols
9 pburba A
9 pburba B
9 pburba C
9 pburba D
9 pburba E
9 pburba F
trunk@1403813>svn blame file-with-CRLF-eols
9 pburba A
9 pburba B
9 pburba C
9 pburba D
9 pburba E
9 pburba F
trunk@1403813>svn blame file-with-LF-eols
9 pburba A
9 pburba B
9 pburba C
9 pburba D
9 pburba E
9 pburba F
I edited line #3 in each file like this:
trunk@1403813>svn diff file-with-CRLF-eols
Index: file-with-CRLF-eols
===================================================================
--- file-with-CRLF-eols (revision 9)
+++ file-with-CRLF-eols (working copy)
@@ -1,6 +1,6 @@
A
B
-C
+C-mod
D
E
F
Without Daniels patch we see the bug described previously in the thread:
trunk@1403813>svn blame file-with-CR-eols
- - A
- - B
- - C-mod
- - D
- - E
- - F
trunk@1403813>svn blame file-with-CRLF-eols
- - A
- - B
- - C-mod
- - D
- - E
- - F
trunk@1403813>svn blame file-with-LF-eols
9 pburba A
9 pburba B
- - C-mod
9 pburba D
9 pburba E
9 pburba F
But with Daniel's patch I see no difference in the blame output:
trunk@1403813>svn blame file-with-CR-eols
- - A
- - B
- - C-mod
- - D
- - E
- - F
trunk@1403813>svn blame file-with-CRLF-eols
- - A
- - B
- - C-mod
- - D
- - E
- - F
trunk@1403813>svn blame file-with-LF-eols
9 pburba A
9 pburba B
- - C-mod
9 pburba D
9 pburba E
9 pburba F
--
Paul T. Burba
CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development
Skype: ptburba
> [[[
> Index: subversion/libsvn_client/blame.c
> ===================================================================
> --- subversion/libsvn_client/blame.c (revision 1182771)
> +++ subversion/libsvn_client/blame.c (working copy)
> @@ -473,6 +473,9 @@
> svn_io_file_del_on_pool_cleanup,
> filepool, filepool));
>
> + cur_stream = svn_subst_stream_translated(cur_stream, "\n", TRUE, NULL,
> + FALSE, filepool);
> +
> /* Get window handler for applying delta. */
> svn_txdelta_apply(last_stream, cur_stream, NULL, NULL,
> frb->currpool,
> ]]]
>
>> >
>> > This explains why it works on *nix regardless of whether svn:eol-style is
>> > set. On *nix, the data has "\n" EOLs in the repository. I believe you'll be
>> > able to reproduce the problem on *nix too if you commit a file with CRLF
>> > line endings. Also, I see that svn blame works correctly for modified files
>> > on my Windows system for files having LF line endings.
>> >
>> > I'm not sure what's the best way to resolve the issue. For me, just removing
>> > the call to svn_subst_stream_translated would fix the problem, I think. But
>> > it wouldn't always be TRT in presence of svn:eol-style.
>> >
>> > Anyway, please consider reverting svn blame behavior without explicit
>> > @REVISION as the last resort for 1.7.1.
>> >
>>
>> Best regards,
>> Konstantin Kolinko
Re: [Bug] [Subversion 1.7] svn blame doesn't work for locally
modified files
Posted by Paul Burba <pt...@gmail.com>.
On Tue, Oct 30, 2012 at 6:08 PM, Julian Foad <ju...@btopenworld.com> wrote:
> Paul Burba wrote:
>
>> Mike Pilato is tying off loose ends for the upcoming 1.8 release and
>> asked me to check this patch for issue #4034 on my Windows machine.
>>
>> The patch doesn't appear to make any difference:
>>
>> With three files, with three different eols as per their names:
> [...]
>
> You don't appear to mention anything about setting svn:eol-style properties for these files. I assume that would be important.
Sorry, I neglected to mention that each file had svn:eol-style set as
per its name:
trunk@1403813>svn pl -vR
Properties on 'file-with-CR-eols':
svn:eol-style
CR
Properties on 'file-with-CRLF-eols':
svn:eol-style
CRLF
Properties on 'file-with-LF-eols':
svn:eol-style
LF
--
Paul T. Burba
CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development
Skype: ptburba
> (For this reply I've removed users@ and all other persons from the CC, leaving just you and dev@.)
>
> - Julian
Re: [Bug] [Subversion 1.7] svn blame doesn't work for locally modified files
Posted by Julian Foad <ju...@btopenworld.com>.
Paul Burba wrote:
> Mike Pilato is tying off loose ends for the upcoming 1.8 release and
> asked me to check this patch for issue #4034 on my Windows machine.
>
> The patch doesn't appear to make any difference:
>
> With three files, with three different eols as per their names:
[...]
You don't appear to mention anything about setting svn:eol-style properties for these files. I assume that would be important.
(For this reply I've removed users@ and all other persons from the CC, leaving just you and dev@.)
- Julian