On Wed, Oct 9, 2019 at 7:14 PM Daniel Shahaf <d.s_at_daniel.shahaf.name> wrote:
>
> Branko Čibej wrote on Wed, Oct 09, 2019 at 15:45:32 +0200:
> > On 09.10.2019 15:00, Johan Corveleyn wrote:
> > > During that entire discussion thread the only objections raised were
> > > about "enforcing it in the repos layer / server directly". No-one
> > > objected to the proposal(s) to solve the issue via pre-commit hooks.
> >
> > Validating contents (such as line endings based on svn:eol-style) is a
> > perfect fit for hooks. Not normalising of course. :)
>
> The repos layer validates svn:* revprops in svn_repos__validate_prop().
>
> The FS layer doesn't do that validation.
>
> The result of that is that old repositories with invalid svn:* properties can
> be dump/load'd but can't be svnsync'.d
>
> ---
>
> As to separation of concerns, that's a valid point, but we should be consistent
> about what concerns belong where. The validation of svn:* props and of
> contents of files with svn:eol-style set belongs in the same layer, doesn't it?
>
> Therefore, I think there are two options:
>
> - These validation belongs in the repos layer. The simpler/lower layer that
> doesn't do such validation is the FS layer. svn:eol-style validation belongs
> in the repos layer. (And if svnsync needs to bypass it, we'll design a way
> for it to do so.)
>
> - These validations belong $elsewhere. svn:eol-style validation will be added
> $elsewhere and svn_repos__validate_prop() will be moved $elsewhere.
>
> My interpretation: I don't have an opinion yet, except that if we move the
> validation out of the repos layer, I won't be quite as sure any more what the
> difference between the FS layer and the repos layer _is_. The FS layer exposes
> a filesystem that's a 0-indexed array of trees [implemented by the DAG layer].
> The repos layer does… what, exactly, on top of that?
Thinking about it again some more, I guess I agree that this really
belongs in the same place as svn_repos__validate_prop(), and really
should be in the repos layer (or in any case, more widely /
automatically / standardly available than just a hook script, where it
depends too much on the individual sysadmin).
In fact, back in 2012 I wasn't happy with the consensus gravitating
towards "solve this in a pre-commit hook" (except as a stop-gap
measure). But the "standard pre-commit utility" seemed to be the best
attainable idea that would receive support from most devs, at the
time.
Earlier in the current thread Brane said:
"Validating contents (such as line endings based on svn:eol-style) is
a perfect fit for hooks."
But I think this kind of content validation is different from
"application-level content validation", like "follow coding style X",
or "commit message should contain an issue key", or "I want to enforce
that .c files have svn:eol-style=native". Such application-level
content rules have no effect whatsoever on the workings of SVN itself.
SVN clients don't care about those, only the applications (IDE, users,
...) do.
In contrast, having a "non-LF-normalized with svn:eol-style=native" in
the repository breaks "svn diff" (all lines shown as different) and
"svn status" (if timestamps mismatch, contents are checksummed, and
the file shows up as modified while it isn't -- causing confusion and
even worse, spurious tree conflicts).
So in my eyes this is more of an obligatory validation, to preserve
the sanity of your "svn ecosystem". A bit like the sha1 collision
protection (yes, the repository can store sha1 collisions perfectly
fine, but they wreak havoc amongst your clients / working copies if
you let them enter your repository). The "havoc" caused by an SVN-4065
file is orders of magnitude less severe than what a sha1 collision
causes (entire working copy hosed), but still, it's not nice.
(Julian: sorry for my frustrated reaction earlier, and thanks for
staying constructive, as always ... you were right, the discussion was
clearly not over yet :-))
--
Johan
Received on 2019-10-10 01:12:35 CEST