You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2015/09/17 23:00:36 UTC
svn commit: r1703689 - in /subversion/trunk/subversion:
libsvn_client/merge.c tests/cmdline/merge_automatic_tests.py
Author: rhuijben
Date: Thu Sep 17 21:00:36 2015
New Revision: 1703689
URL: http://svn.apache.org/viewvc?rev=1703689&view=rev
Log:
Following up on r1703688 fix conflicts reported on merging deletes of
files that have an svn:eol-style 'CR' or 'CRLF' property.
* subversion/libsvn_client/merge.c
(files_same_p): Use a stream in repository form instead of one with '\n'
newlines to compare against the repository.
* subversion/tests/cmdline/merge_automatic_tests.py
(merge_delete_crlf_file): Remove XFail marker. Tweak some comments.
Modified:
subversion/trunk/subversion/libsvn_client/merge.c
subversion/trunk/subversion/tests/cmdline/merge_automatic_tests.py
Modified: subversion/trunk/subversion/libsvn_client/merge.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1703689&r1=1703688&r2=1703689&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/merge.c (original)
+++ subversion/trunk/subversion/libsvn_client/merge.c Thu Sep 17 21:00:36 2015
@@ -2349,17 +2349,47 @@ files_same_p(svn_boolean_t *same,
{
svn_stream_t *mine_stream;
svn_stream_t *older_stream;
- svn_opt_revision_t working_rev = { svn_opt_revision_working, { 0 } };
+ svn_string_t *special = svn_hash_gets(working_props, SVN_PROP_SPECIAL);
+ svn_string_t *eol_style = svn_hash_gets(working_props, SVN_PROP_EOL_STYLE);
+ svn_string_t *keywords = svn_hash_gets(working_props, SVN_PROP_KEYWORDS);
/* Compare the file content, translating 'mine' to 'normal' form. */
- if (svn_prop_get_value(working_props, SVN_PROP_SPECIAL) != NULL)
+ if (special != NULL)
SVN_ERR(svn_subst_read_specialfile(&mine_stream, mine_abspath,
scratch_pool, scratch_pool));
else
- SVN_ERR(svn_client__get_normalized_stream(&mine_stream, wc_ctx,
- mine_abspath, &working_rev,
- FALSE, TRUE, NULL, NULL,
- scratch_pool, scratch_pool));
+ SVN_ERR(svn_stream_open_readonly(&mine_stream, mine_abspath,
+ scratch_pool, scratch_pool));
+
+ if (!special && (eol_style || keywords))
+ {
+ apr_hash_t *kw = NULL;
+ const char *eol = NULL;
+ svn_subst_eol_style_t style;
+
+ /* We used to use svn_client__get_normalized_stream() here, but
+ that doesn't work in 100% of the cases because it doesn't
+ convert EOLs to the repository form; just to '\n'.
+ */
+
+ if (eol_style)
+ {
+ svn_subst_eol_style_from_value(&style, &eol, eol_style->data);
+
+ if (style == svn_subst_eol_style_native)
+ eol = SVN_SUBST_NATIVE_EOL_STR;
+ else if (style != svn_subst_eol_style_fixed
+ && style != svn_subst_eol_style_none)
+ return svn_error_create(SVN_ERR_IO_UNKNOWN_EOL, NULL, NULL);
+ }
+
+ if (keywords)
+ SVN_ERR(svn_subst_build_keywords3(&kw, keywords->data, "", "",
+ "", 0, "", scratch_pool));
+
+ mine_stream = svn_subst_stream_translated(
+ mine_stream, eol, FALSE, kw, FALSE, scratch_pool);
+ }
SVN_ERR(svn_stream_open_readonly(&older_stream, older_abspath,
scratch_pool, scratch_pool));
Modified: subversion/trunk/subversion/tests/cmdline/merge_automatic_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/merge_automatic_tests.py?rev=1703689&r1=1703688&r2=1703689&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/merge_automatic_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/merge_automatic_tests.py Thu Sep 17 21:00:36 2015
@@ -1352,7 +1352,6 @@ def merge_to_copy_and_add(sbox):
'merge', '--reintegrate', '^/A',
sbox.ospath('A3'))
-@XFail()
def merge_delete_crlf_file(sbox):
"merge the deletion of a strict CRLF file"
@@ -1374,7 +1373,7 @@ def merge_delete_crlf_file(sbox):
sbox.simple_commit('A') # r2
- # Merge the addition of the file
+ # Merge the addition of the files
svntest.actions.run_and_verify_svn(None, [],
'merge', '^/A', sbox.ospath('AA'))
sbox.simple_commit('AA') # r3
@@ -1384,6 +1383,7 @@ def merge_delete_crlf_file(sbox):
sbox.simple_update('') # Make single revision r4
+ # And now merge the deletes
expected_output = svntest.verify.UnorderedOutput([
'--- Merging r3 through r4 into \'%s\':\n' % sbox.ospath('AA'),
'D %s\n' % sbox.ospath('AA/cr'),
Re: svn commit: r1703689 - in /subversion/trunk/subversion:
libsvn_client/merge.c tests/cmdline/merge_automatic_tests.py
Posted by Branko Čibej <br...@apache.org>.
On 27.10.2015 20:48, Branko Čibej wrote:
> On 26.10.2015 16:03, Ivan Zhakov wrote:
>> Do you know why we do not convert eol style/collapse keywords for
>> special files? I know that it's not related to this commit, but may be
>> you know rationale behind this behavior.
> The contents of special files are not user-defined and semantically, the
> contents aren't text but control strings interpreted by the Subversion
> client. There's a pretty good case for not allowing svn:eol-style or
> svn:keywords properties on such files in the first place.
... but since IIRC we do allow them, ignoring them is the best option.
-- Brane
Re: svn commit: r1703689 - in /subversion/trunk/subversion:
libsvn_client/merge.c tests/cmdline/merge_automatic_tests.py
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 30 October 2015 at 13:40, Bert Huijben <be...@qqmail.nl> wrote:
>> -----Original Message-----
>> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
>> Sent: vrijdag 30 oktober 2015 10:52
>> To: Branko Čibej <br...@apache.org>
>> Cc: dev@subversion.apache.org
>> Subject: Re: svn commit: r1703689 - in /subversion/trunk/subversion:
>> libsvn_client/merge.c tests/cmdline/merge_automatic_tests.py
>
>
>> >>> + SVN_ERR(svn_stream_open_readonly(&mine_stream,
>> mine_abspath,
>> >>> + scratch_pool, scratch_pool));
>> >>> +
>> >>> + if (!special && (eol_style || keywords))
>> >> Hi Bert,
>> >>
>> >> Do you know why we do not convert eol style/collapse keywords for
>> >> special files? I know that it's not related to this commit, but may be
>> >> you know rationale behind this behavior.
>> >
>> > The contents of special files are not user-defined and semantically, the
>> > contents aren't text but control strings interpreted by the Subversion
>> > client. There's a pretty good case for not allowing svn:eol-style or
>> > svn:keywords properties on such files in the first place.
>> >
>> This is makes sense. Thanks for the explanation.
>
> I have nothing to add to the previous comments. Thanks Branko and Ivan.
>
>
> Maybe I should note that I reviewed other similar code when I applied this patch in Berlin. There are a few not 100% correct cases around 'svn cat' and blame, but non that can really affect our behavior.
> (And all of these worked the same way since at least 1.5.x)
>
I suspected the same. I wondered that this logic does not exists as
some API function. May be it worth to factor out this code? I'm going
to put my +1 for backport anyway.
--
Ivan Zhakov
RE: svn commit: r1703689 - in /subversion/trunk/subversion: libsvn_client/merge.c tests/cmdline/merge_automatic_tests.py
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Ivan Zhakov [mailto:ivan@visualsvn.com]
> Sent: vrijdag 30 oktober 2015 10:52
> To: Branko Čibej <br...@apache.org>
> Cc: dev@subversion.apache.org
> Subject: Re: svn commit: r1703689 - in /subversion/trunk/subversion:
> libsvn_client/merge.c tests/cmdline/merge_automatic_tests.py
> >>> + SVN_ERR(svn_stream_open_readonly(&mine_stream,
> mine_abspath,
> >>> + scratch_pool, scratch_pool));
> >>> +
> >>> + if (!special && (eol_style || keywords))
> >> Hi Bert,
> >>
> >> Do you know why we do not convert eol style/collapse keywords for
> >> special files? I know that it's not related to this commit, but may be
> >> you know rationale behind this behavior.
> >
> > The contents of special files are not user-defined and semantically, the
> > contents aren't text but control strings interpreted by the Subversion
> > client. There's a pretty good case for not allowing svn:eol-style or
> > svn:keywords properties on such files in the first place.
> >
> This is makes sense. Thanks for the explanation.
I have nothing to add to the previous comments. Thanks Branko and Ivan.
Maybe I should note that I reviewed other similar code when I applied this patch in Berlin. There are a few not 100% correct cases around 'svn cat' and blame, but non that can really affect our behavior.
(And all of these worked the same way since at least 1.5.x)
Thanks for reviewing,
Bert
Re: svn commit: r1703689 - in /subversion/trunk/subversion:
libsvn_client/merge.c tests/cmdline/merge_automatic_tests.py
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 27 October 2015 at 22:48, Branko Čibej <br...@apache.org> wrote:
> On 26.10.2015 16:03, Ivan Zhakov wrote:
>> On 18 September 2015 at 00:00, <rh...@apache.org> wrote:
>>> Author: rhuijben
>>> Date: Thu Sep 17 21:00:36 2015
>>> New Revision: 1703689
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1703689&view=rev
>>> Log:
>>> Following up on r1703688 fix conflicts reported on merging deletes of
>>> files that have an svn:eol-style 'CR' or 'CRLF' property.
>>>
>> [...]
>>
>>> Modified: subversion/trunk/subversion/libsvn_client/merge.c
>>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1703689&r1=1703688&r2=1703689&view=diff
>>> ==============================================================================
>>> --- subversion/trunk/subversion/libsvn_client/merge.c (original)
>>> +++ subversion/trunk/subversion/libsvn_client/merge.c Thu Sep 17 21:00:36 2015
>>> @@ -2349,17 +2349,47 @@ files_same_p(svn_boolean_t *same,
>>> {
>>> svn_stream_t *mine_stream;
>>> svn_stream_t *older_stream;
>>> - svn_opt_revision_t working_rev = { svn_opt_revision_working, { 0 } };
>>> + svn_string_t *special = svn_hash_gets(working_props, SVN_PROP_SPECIAL);
>>> + svn_string_t *eol_style = svn_hash_gets(working_props, SVN_PROP_EOL_STYLE);
>>> + svn_string_t *keywords = svn_hash_gets(working_props, SVN_PROP_KEYWORDS);
>>>
>>> /* Compare the file content, translating 'mine' to 'normal' form. */
>>> - if (svn_prop_get_value(working_props, SVN_PROP_SPECIAL) != NULL)
>>> + if (special != NULL)
>>> SVN_ERR(svn_subst_read_specialfile(&mine_stream, mine_abspath,
>>> scratch_pool, scratch_pool));
>>> else
>>> - SVN_ERR(svn_client__get_normalized_stream(&mine_stream, wc_ctx,
>>> - mine_abspath, &working_rev,
>>> - FALSE, TRUE, NULL, NULL,
>>> - scratch_pool, scratch_pool));
>>> + SVN_ERR(svn_stream_open_readonly(&mine_stream, mine_abspath,
>>> + scratch_pool, scratch_pool));
>>> +
>>> + if (!special && (eol_style || keywords))
>> Hi Bert,
>>
>> Do you know why we do not convert eol style/collapse keywords for
>> special files? I know that it's not related to this commit, but may be
>> you know rationale behind this behavior.
>
> The contents of special files are not user-defined and semantically, the
> contents aren't text but control strings interpreted by the Subversion
> client. There's a pretty good case for not allowing svn:eol-style or
> svn:keywords properties on such files in the first place.
>
This is makes sense. Thanks for the explanation.
--
Ivan Zhakov
Re: svn commit: r1703689 - in /subversion/trunk/subversion:
libsvn_client/merge.c tests/cmdline/merge_automatic_tests.py
Posted by Branko Čibej <br...@apache.org>.
On 26.10.2015 16:03, Ivan Zhakov wrote:
> On 18 September 2015 at 00:00, <rh...@apache.org> wrote:
>> Author: rhuijben
>> Date: Thu Sep 17 21:00:36 2015
>> New Revision: 1703689
>>
>> URL: http://svn.apache.org/viewvc?rev=1703689&view=rev
>> Log:
>> Following up on r1703688 fix conflicts reported on merging deletes of
>> files that have an svn:eol-style 'CR' or 'CRLF' property.
>>
> [...]
>
>> Modified: subversion/trunk/subversion/libsvn_client/merge.c
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1703689&r1=1703688&r2=1703689&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/libsvn_client/merge.c (original)
>> +++ subversion/trunk/subversion/libsvn_client/merge.c Thu Sep 17 21:00:36 2015
>> @@ -2349,17 +2349,47 @@ files_same_p(svn_boolean_t *same,
>> {
>> svn_stream_t *mine_stream;
>> svn_stream_t *older_stream;
>> - svn_opt_revision_t working_rev = { svn_opt_revision_working, { 0 } };
>> + svn_string_t *special = svn_hash_gets(working_props, SVN_PROP_SPECIAL);
>> + svn_string_t *eol_style = svn_hash_gets(working_props, SVN_PROP_EOL_STYLE);
>> + svn_string_t *keywords = svn_hash_gets(working_props, SVN_PROP_KEYWORDS);
>>
>> /* Compare the file content, translating 'mine' to 'normal' form. */
>> - if (svn_prop_get_value(working_props, SVN_PROP_SPECIAL) != NULL)
>> + if (special != NULL)
>> SVN_ERR(svn_subst_read_specialfile(&mine_stream, mine_abspath,
>> scratch_pool, scratch_pool));
>> else
>> - SVN_ERR(svn_client__get_normalized_stream(&mine_stream, wc_ctx,
>> - mine_abspath, &working_rev,
>> - FALSE, TRUE, NULL, NULL,
>> - scratch_pool, scratch_pool));
>> + SVN_ERR(svn_stream_open_readonly(&mine_stream, mine_abspath,
>> + scratch_pool, scratch_pool));
>> +
>> + if (!special && (eol_style || keywords))
> Hi Bert,
>
> Do you know why we do not convert eol style/collapse keywords for
> special files? I know that it's not related to this commit, but may be
> you know rationale behind this behavior.
The contents of special files are not user-defined and semantically, the
contents aren't text but control strings interpreted by the Subversion
client. There's a pretty good case for not allowing svn:eol-style or
svn:keywords properties on such files in the first place.
-- Brane
Re: svn commit: r1703689 - in /subversion/trunk/subversion:
libsvn_client/merge.c tests/cmdline/merge_automatic_tests.py
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On 18 September 2015 at 00:00, <rh...@apache.org> wrote:
> Author: rhuijben
> Date: Thu Sep 17 21:00:36 2015
> New Revision: 1703689
>
> URL: http://svn.apache.org/viewvc?rev=1703689&view=rev
> Log:
> Following up on r1703688 fix conflicts reported on merging deletes of
> files that have an svn:eol-style 'CR' or 'CRLF' property.
>
[...]
> Modified: subversion/trunk/subversion/libsvn_client/merge.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1703689&r1=1703688&r2=1703689&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/merge.c (original)
> +++ subversion/trunk/subversion/libsvn_client/merge.c Thu Sep 17 21:00:36 2015
> @@ -2349,17 +2349,47 @@ files_same_p(svn_boolean_t *same,
> {
> svn_stream_t *mine_stream;
> svn_stream_t *older_stream;
> - svn_opt_revision_t working_rev = { svn_opt_revision_working, { 0 } };
> + svn_string_t *special = svn_hash_gets(working_props, SVN_PROP_SPECIAL);
> + svn_string_t *eol_style = svn_hash_gets(working_props, SVN_PROP_EOL_STYLE);
> + svn_string_t *keywords = svn_hash_gets(working_props, SVN_PROP_KEYWORDS);
>
> /* Compare the file content, translating 'mine' to 'normal' form. */
> - if (svn_prop_get_value(working_props, SVN_PROP_SPECIAL) != NULL)
> + if (special != NULL)
> SVN_ERR(svn_subst_read_specialfile(&mine_stream, mine_abspath,
> scratch_pool, scratch_pool));
> else
> - SVN_ERR(svn_client__get_normalized_stream(&mine_stream, wc_ctx,
> - mine_abspath, &working_rev,
> - FALSE, TRUE, NULL, NULL,
> - scratch_pool, scratch_pool));
> + SVN_ERR(svn_stream_open_readonly(&mine_stream, mine_abspath,
> + scratch_pool, scratch_pool));
> +
> + if (!special && (eol_style || keywords))
Hi Bert,
Do you know why we do not convert eol style/collapse keywords for
special files? I know that it's not related to this commit, but may be
you know rationale behind this behavior.
--
Ivan Zhakov