On Tue, Feb 02, 2010 at 09:11:52PM +0100, Stefan Sperling wrote:
> On Tue, Feb 02, 2010 at 08:48:43PM +0100, Daniel Näslund wrote:
> > Hi Stefan!
> >
> > In match_hunk() we try to match lines from the context of the patch with
> > lines in the target. Earlier, in init_patch_target() we detect the eol
> > of the target and open streams to read from the target and to write the
> > patched result. Those streams does translation of keywords and eols.
> >
> > In match_hunk we read a line from original_text and translate it. But we
> > don't get any translation of the eols in hunk_line_translated.
>
> svn patch only repairs EOLs if the svn:eol-style enforces a fixed
> value (such as CR or CRLF). Try setting svn:eol-style to 'CRLF' on
> the patch target. Then you'll see dos-style newlines in the patched result.
>
> Admittedly, we may want to repair EOLs in more scenarios (such as
> eol-style = native).
I have a test where the target uses '\r\n' and the patch uses '\r' . The
eols are consistent within each file but we get a failure saying:
[[[
subversion/svn/patch-cmd.c:81: (apr_err=135000)
subversion/libsvn_client/patch.c:1463: (apr_err=135000)
subversion/libsvn_client/patch.c:1410: (apr_err=135000)
subversion/libsvn_client/patch.c:1100: (apr_err=135000)
subversion/libsvn_client/patch.c:830: (apr_err=135000)
subversion/libsvn_client/patch.c:799: (apr_err=135000)
subversion/libsvn_subr/subst.c:876: (apr_err=135000)
subversion/libsvn_subr/subst.c:643: (apr_err=135000)
svn: Inconsistent line ending style
]]]
What to do?
[ ] Write documentation saying we can only repair eols on targets with a
svn:eol-style prop.
[ ] Wrap the error for a better error message perhaps saying 'patch and
target uses different eol' or similar.
[ ] Adjust the code to allow for repairing eols even when there is no
svn:eol-style prop. I would go for this one if it's doable.
Before I throw myself to the mercy of the eol-translating code, I ask
you if there is something you know of hindering us from repairing eols
where there is no prop set?
In init_patch_target() we decide how the eol translation of
target->patched will be done:
[[[
eol_style_val = apr_hash_get(props, SVN_PROP_EOL_STYLE,
APR_HASH_KEY_STRING);
if (eol_style_val)
{
svn_subst_eol_style_from_value(&target->eol_style,
&target->eol_str,
eol_style_val->data);
}
else
{
/* Just use the first EOL sequence we can find in the file. */
SVN_ERR(svn_eol__detect_file_eol(&target->eol_str,
target->file, scratch_pool));
/* But don't enforce any particular EOL-style. */
target->eol_style = svn_subst_eol_style_none;
}
[...]
target->patched = svn_subst_stream_translated(
target->patched_raw,
target->eol_str,
target->eol_style ==
svn_subst_eol_style_fixed,
target->keywords, TRUE, result_pool);
]]]
You decided to not enforce any particular eol-style if we don't have a
prop. The eol_style is used for the cases where there is an eol-style
prop. Fair enough. But couldn't we do some hack where we always say that
the eol-style is fixed and use whatever eol we find in the target? I did
a quick try and got a lot of rejected hunks...
What can go wrong with this approach?
-------------------------------------
* We falsely label targets with inconsistent eols to be of type
eol_style_fixed. But after patching the eols will be consistent and
that's a good thing.
* [Placeholder for more stuff that can go wrong]
Or we could set the style to svn_subst_eol_style_native. We will end up
with native newlines in the target then, right?
What say you?
Daniel
