Johan Corveleyn wrote:
Julian Foad wrote:
Johan Corveleyn wrote:
I think that was the conclusion from those threads. I.e. it would be
best if we developed a standard "svnhooks" program that can be invoked
from the pre-commit hook (and not try to implement this directly in
the repos layer). At least, after those svnhooks suggestions no-one
objected, so I assumed silent consensus about that way forward :-) ...
Silence meant "errm... what?"
I disagree. On this list silence means "no objections". If someone
wanted to say "errm... what?", they should have mailed it.
Sorry, I was too harsh there.
Nevertheless, there are hundreds of issues and mail threads that are
incomplete but don't end with someone saying so.
My reading of the issue ended with the impression that there should be a
standard supplied "hooks program", but with no definition of what its
scope/purpose/role should be, so unclear what we should add to it, in
what contexts the svn server would execute it (like, svnadmin load?),
why it should be an external program, which server operators would
reasonably want to replace this program with a different one in what
kinds of situation, etc.
In that very thread, 7 years ago, it seemed quite clear that consensus
was gravitating towards "don't solve it in the repos layer, but in a
pre-commit hook":
* Daniel first posted a simple script for pre-commit hook validation:
https://svn.haxx.se/dev/archive-2012-12/0180.shtml
[...]
* Ivan suggested to create a separate program for it "svnhooks", with
this concrete "rule" as a first use case for a more general tool:
https://svn.haxx.se/dev/archive-2012-12/0202.shtml
* Ivan specified his idea a bit further here:
https://svn.haxx.se/dev/archive-2012-12/0217.shtml
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.
Sound pretty consensus-y to me.
Thanks for summarizing the thread. I had only skimmed through bits of
it. In particular I hadn't seen where Ivan said the program would
implement hooks for "standard recommended polices", and I hadn't picked
up on it looking so consensus-y.
That means we have to design it in such a way that only "client commits"
are affected, and "server tools" such as sump/load are not, because we
can't have them suddenly start failing to load existing repository data.
Yes, and hooks can do that. You're arriving at the same conclusion as
the thread 7 years ago.
Are "svnrdump" and "svnsync" client-layer or repos-layer tools, for
these purposes? We don't yet have a formal way to distinguish and use
"repos-layer" tools through our client-server (RA) interface. So at the
moment I suppose we might say that ("de facto") all interactions through
the RA interface are deemed to be client-layer interactions. We could
consider an enhancement to enable "repos-layer" interactions to be
performed over RA with suitable authorization. We probably ought to
file that as a future enhancement issue.
Currently we have "svn commit" and other client-layer operations, vs.
"svnadmin load" as the main repos-layer (server side) interaction.
"svnadmin load" has these options:
--use-pre-commit-hook
--use-post-commit-hook
--normalize-props
--bypass-prop-validation
To me, this looks like a crude attempt to allow it to support both
client-layer and repos-layer modes, but with no overall mode switch so
the user has to think about the implications of each individual sub-switch.
We never spelled out the role of commit hooks. Therefore presumably
some commit hooks are used for client-side purposes (enforcing rules
like LF normalization and log message formats, etc.) and some for
server-side purposes (logging, backups, mirroring to svnsync, etc.).
The option to enable or disable all commit hooks seems misguided:
instead it would seem more appropriate to categorize hooks into client
and server roles, and have "svnadmin load" apply only those in the
server role.
Is two roles of hooks something it makes sense to introduce? Or could
we clarify the current situation, document it?
It looks to me like "enforcement of the standard svn client's rules" is
an option we would ideally like to provide to server operators. How
would this be defined more precisely? How should we design it to cope
with different client versions, whose rules have changed a few times?
You're pulling this way out of scope.
I'm unpicking a load of unspoken assumptions.
Issue SVN-4065 is very concrete
and specific, and a concrete proposal was made on how best to solve
it.
You're using it to open a much broader architectural discussion. Which
is fine of course, and I think quite interesting. But that shouldn't
block progress for SVN-4065 in the way which was proposed 7 years ago,
and which still seems the best way (via a utility program that can be
invoked from hooks, where our hooks still have the same semantics /
confusions / shortcomings / ... as today).
OK, so a valid option is to treat this as "just another pre-commit
hook". If the suggestion was just to write and supply another hook
script, it would be reasonable to ignore all that: we don't need to
think about the system semantics, as it's just like any other pre-commit
hook.
The talk about building the required hook functionality into a new
compiled program led to me wondering more widely about the design. If
we're going to combine multiple "standard" hooks into one configurable
program, that sort of thing does need to be discussed.
It seems to me that combining multiple hooks into one configurable
program is orthogonal to the actual issue. The only reason it came up,
AFAICT, is because of possibly premature concern about efficiency.
That being said, maybe that svnhooks utility program (Ivan's proposal)
could be used to introduce a way to separate those different roles
that hooks serve. Ivan hinted a bit in that direction in his post
there. From https://svn.haxx.se/dev/archive-2012-12/0217.shtml:
svnhooks configuration file has separate section for each policy. For example:
[[[
[check-eol-normalization]
enable = yes
[rev-propchange]
enable = yes
users = svnsync
[edit-log-message]
enable = yes
users = admin, @author
]]]
I agree this should be thought through, with a good design (a "users"
field might not cut it -- perhaps something else). It's clear that the
proposal needs further design work.
Thanks.
I didn't mean to prevent a simple task from being done.
- Julian
