On Tue, Feb 23, 2010 at 08:57:26PM +0100, Daniel Näslund wrote:
> 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:
[..]
> 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?
Oups, lost the thread there. The eol-style was only used to check for
style_fixed, and if so translate eols. The question remains; why
can't we translate files when there is no no eol-style prop. Is it worth
investigating further?
Daniel
