I'm curious about the new 'validate_inherited_mergeinfo' parameter of
svn_ra_get_mergeinfo2(), which is the public API that calls
svn_ra__vtable_t.get_mergeinfo().
/* ...
* If the mergeinfo for any path is inherited and
* @a *validate_inherited_mergeinfo is TRUE, then request that the server
* validate the mergeinfo in @a *catalog, so it contains only merge source
* path-revisions that actually exist in repository. If validation is
* requested and the server supports it, then set
* @a *validate_inherited_mergeinfo to TRUE on return. Set it to FALSE
* in all other cases.
* ... */
This was introduced in r1035389, with the log message: "If the server is
asked to validate inherited mergeinfo, enable it to communicate back to
the client that it was actually able to do this."
Exactly one caller passes '*validate_inherited_mergeinfo' as TRUE, which
is get_invalid_inherited_mergeinfo() in libsvn_client. That function
then returns the output value from that parameter through its own
separate output parameter '*inherited_validated'.
Hyrum noticed that having an in-out parameter behave like this is a bit
ugly (all callers have to have a variable to pass the address of, even
if they are not interested in it), and a bit of a trap for the unwary,
although of course it is not terribly complex. But this made me wonder
whether it's the best way to handle this kind of information.
There are two other APIs closely related to this one:
svn_fs_get_mergeinfo2() and svn_repos_fs_get_mergeinfo2(). Each of
those has a similar parameter except that in those cases the parameter
is input-only.
Question to the experts: Could this feedback of 'OK, it was validated'
be handled instead by means of the "server capabilities" mechanism, and
if so would that be better? I mean something like having a new
capability meaning "server can validate inherited mergeinfo", and
changing that in-out parameter of svn_ra_get_mergeinfo2() to an
input-only parameter, and then the caller can assume its request for
"validation" will be honoured if that server capability is present.
Alternatively, shall we simply change the function prototype to have one
input argument and one optional output argument, like this:
/* ...
* If the mergeinfo for any path is inherited and
* @a validate_inherited_mergeinfo is TRUE, then request that the
* server validate the mergeinfo in @a *catalog, so it contains
* only merge source path-revisions that actually exist in the
* repository.
*
* If @a inherited_mergeinfo_was_validated is non-NULL, then if
* @a validate_inherited_mergeinfo is TRUE and the server did
* in fact "validate" it, set @a *inherited_mergeinfo_was_validated
* to TRUE, else to FALSE. */
* ... */
svn_boolean_t *inherited_mergeinfo_was_validated,
svn_boolean_t validate_inherited_mergeinfo,
?
- Julian
Received on 2011-06-28 18:46:47 CEST