You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Julian Foad <ju...@apache.org> on 2018/09/12 19:21:26 UTC

API review for 1.11; do we need to mark new APIs as experimental?

The following public APIs are new or revved since 1.10:

svn_client_layout_func_t
svn_client_layout_list
  - New for 'viewspec' output; marked 'experimental'.

svn_client_diff7
svn_client_diff_peg7
  - Adds a 'pretty_print_mergeinfo' option.

svn_client_revert4
svn_wc_revert6
  - Adds an 'added_keep_local' option.

svn_client_conflict_option_id_t
  - 3 new enumerators.

svn_client_shelf_*
  - Marked 'experimental'.

SVN_OPT_MAX_PARAGRAPHS
svn_opt_subcommand_desc3_t
svn_opt_get_canonical_subcommand3
svn_opt_get_option_from_code3
svn_opt_subcommand_takes_option4
svn_opt_print_generic_help3
svn_opt_subcommand_help4
svn_opt_print_help5
  - For multi-paragraph command-line help text.


** QUESTION **

We recently decided that only APIs released in an LTS release will be subject to our compatibility guarantees. As 1.11 is not an LTS release, the above APIs will not be subject to those guarantees (until they appear in an LTS).

Do we need to do anything in the source code to emphasize that, for the ones that are not already marked 'experimental'? If so, what? Mark them 'experimental' in addition to their existing annotations (which probably would mean we'd want to turn off the warnings about use of experimental APIs in our own builds), or something else?

We should certainly mention this explicitly in the 1.11 release notes.

-- 
- Julian

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Branko Čibej wrote on Mon, 17 Sep 2018 12:57 +0200:
> A corollary of the above is that we should never have aliases for
> experimental bits in another namespace: for example, if there is an 'svn
> x-shelve' command it should not have an 'svn shelve' alias, since that
> could lead users to believe that the feature is no longer experimental.
> 
> Consequently, releasing experimental features to the stable set implies
> removing associated functions, types, commands, etc. from the
> experimental namespace — and yes, this *will* create ABI
> incompatibilities. If we decide that we cannot allow that and that ABI
> compatibility is more important even for experimental stuff, we can keep
> dummy functions that return APR_ENOTIMPL (we don't have to invent a new
> error code).

I agree that when an experimental feature becomes stable, it would be
good to remove it from the x- namespace in order to reinforce the
distinction that "a feature is stable iff it isn't in the experimental
namespace".  (In other words: to prevent brand dilution of svn_x_*() as
a container of experimental features.)

However, while the whole point of experimental features is to provide
little or no promises of forward compatibility and provision of upgrade
paths, I suspect it won't be uncommon for an svn_x_foo() to be graduated
to a stable svn_foo() with little or no incompatible changes to its
signature and semantics.  In such cases, could we keep the svn_x_*()
name around for one minor release cycle, with a warning, in order to
provide a smoother upgrade path?

That is:

    in 1.11:
        svn_error_t *svn_x_foo(SOME_SIGNATURE)
        % svn x-foo
        % 
    .
    in 1.12:
        svn_error_t *svn_x_foo(SOME_SIGNATURE) __attribute__((warning("svn_x_foo renamed to svn_foo; the old name will be removed in 1.13")));
	svn_error_t *svn_foo(SOME_SIGNATURE);
        % svn x-foo
        svn: warning: 'x-foo' renamed to 'foo'; 'x-foo' will be removed in 1.13
        % svn foo
        % 
    .
    in 1.13:
	svn_error_t *svn_foo(SOME_SIGNATURE);
        % svn x-foo
        svn: Unknown subcommand: 'x-foo'
        % svn foo
        % 

Again, this would only be the case if svn_x_foo()'s signature did not
change when the function was promoted to stable.  We'd still reserve the
right to remove svn_x_foo() with no upgrade path, with or without adding
an svn_foo() that has a different signature to svn_x_foo().  (But as
Greg said, we shouldn't change svn_x_foo()'s signature, to preserve ABI
compatibility.)

Cheers,

Daniel

P.S. It goes without saying that that __attribute__() usage would be
wrapped by a macro that replaces the equivalent syntax for the local
compiler understands.

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Julian Foad <ju...@apache.org>.
Julian Foad wrote:
> [...] The users have no idea about that unless 
> the developer takes care to tell them, [...]

A further point. As a library supplier, our responsibility for APIs is *only* to the downstream developer. If that developer chooses to use our experimental APIs to build a product which does a useful job, it is not necessary that the developer must tell the users that it is uses some APIs that we declared as experimental. As long as that developer takes full responsibility for maintaining their application -- which in the extreme could involve maintaining their own copy of svn libs -- they are perfectly entitled to declare their application as stable.

As a supplier of the command-line application "svn" we have a different responsibility to its direct users, but that is a separate responsibility.

-- 
- Julian

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Wed, 19 Sep 2018 18:11 +0100:
> There was some good discussion on IRC today about the rules for 
> experimental APIs.
> http://colabti.org/irclogger/irclogger_log/svn-dev?date=2018-09-19
> 
> I'll get ideas from this incorporated in the wiki page tomorrow.
> https://cwiki.apache.org/confluence/display/SVN/LTS+and+regular+releases
> 

My notes from today are:

1. (see my email from earlier today about LTS promises not applying to
   experimental APIs)

2. Should we promise forward compatibility of experimental features
   within a minor line?  Idea: Promise 'best effort' compatibility,
   i.e., we'll try not to break compatibility gratuitously, but we
   reserve the right to do so if needed, plus some promises on what
   the worst-case failure is (e.g., no bricking of apps due to runtime
   linker failure, no toasters growing arms).  Compare how 1.A.0-rcN
   is not guaranteed to be forward-compatible with 1.A.0 GA, but often is.

3. Could we document on the wiki the _goals_ of the policy?  E.g.,
   "Don't brick apps through runtime linker failing after", "Don't break
   ABI compatibility through incompatible changes to signatures", etc..
   Our promises should be sorted by scenario: downgrade within a minor
   line; upgrade within a minor line; upgrade across minor lines within
   a major line.

4. Question: Should we promise ABI backwards compatibility within a
   single minor line?  In other words, would we permit new experimental
   functions to be added in a patch release 1.A.B where B>0?

Cheers,

Daniel

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Julian Foad <ju...@apache.org>.
There was some good discussion on IRC today about the rules for experimental APIs.
http://colabti.org/irclogger/irclogger_log/svn-dev?date=2018-09-19

I'll get ideas from this incorporated in the wiki page tomorrow.
https://cwiki.apache.org/confluence/display/SVN/LTS+and+regular+releases

Anybody want to attend an IRC meeting about it -- a time when several of us get online together to thrash out the concerns and agree on a result (which we then bring to this list)? If so, when? I'm available Thursday, Friday, Monday, 08:00 to 16:00 UTC roughly. My starting suggestions are 14:00 or 15:00 UTC tomorrow or Friday.

-- 
- Julian

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Wed, 19 Sep 2018 13:46 +0100:
> I have written my proposal and rationale at
> https://cwiki.apache.org/confluence/display/SVN/LTS+and+regular+releases

About LTS v. experimental: LTS promises shouldn't apply to experimental
API's; an experimental API is always covered by the support promises of
a non-LTS release, even when it appears in an LTS release.

Rationale: Experimental features are a moving target.  We shouldn't tie
down our resources with supporting a 2-years-old prototype of some feature
that has since then been revised or removed.

---

That means our net promise for LTS releases would be: critical bugfixes
for: until the next release, for experimental APIs; two years or until
the next LTS, for everything else.  I think this reinforces the need for
experimental APIs to be exposed only on an opt-in basis.  They certainly
should be in a separate namespace and may also need some explicit opt-in
as discussed elsethread.

Cheers,

Daniel

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Julian Foad <ju...@apache.org>.
I have written my proposal and rationale at
https://cwiki.apache.org/confluence/display/SVN/LTS+and+regular+releases

Summary of my latest thoughts:
* Apps using experimental APIs should be packaged declaring a single-version dependency. Then there is no need to provide error-return stub versions of dead APIs.
* We should treat what we are calling "experimental" APIs and what we have previously called "private" APIs exactly the same.
* In order to give packagers flexibility about packaging experimental APIs, we should put them in separate header files, and perhaps there is more that we should do too.

Please read and comment (here and/or there).

-- 
- Julian

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Mon, 17 Sep 2018 15:47 +0100:
> Daniel Shahaf wrote:
> > [...] I suspect it won't be uncommon for an svn_x_foo() to be graduated
> > to a stable svn_foo() with little or no incompatible changes to its
> > signature and semantics.  In such cases, could we keep the svn_x_*()
> > name around for one minor release cycle, with a warning, in order to
> > provide a smoother upgrade path?
> 
> I was wondering about the same. I don't think we should promise to do 
> so, although we could do it opportunistically on the occasions when the 
> graduating API is in fact totally compatible.
> 

Agreed.

> But what for? For the API source-code user, the downstream application 
> needs to be keeping up with changes in the experimental API, and this 
> change would be easy to follow, and this would just delay when they have 
> to do it. So that doesn't seem a good reason.
> 

For an API user, if we remove the old symbol name then they *must*
update their source code in order to be able to upgrade to the newer
libsvn.  Continuing to provide the symbol for one minor release gives
them more time to upgrade their code.

I was thinking not of client maintainers here, but of sysadmins
installing new libsvn packages and the failure modes of any bindings
scripts they may have.  It's one thing to install a new libsvn and get a
new warning on stderr; quite another to install a new libsvn and find
that the script dies with APR_ENOTIMPL until somebody does
s/svn_x_foo/svn_foo/ in the script's source code.

In short, I am saying that _if_ we can provide an API- and ABI-
compatible stub for one release cycle, that would make it easier for
API users to upgrade their calls and/or to support multiple libsvn
versions in parallel.

Does this answer your questions?

Cheers,

Daniel

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Julian Foad <ju...@apache.org>.
Daniel Shahaf wrote:
> [...] I suspect it won't be uncommon for an svn_x_foo() to be graduated
> to a stable svn_foo() with little or no incompatible changes to its
> signature and semantics.  In such cases, could we keep the svn_x_*()
> name around for one minor release cycle, with a warning, in order to
> provide a smoother upgrade path?

I was wondering about the same. I don't think we should promise to do so, although we could do it opportunistically on the occasions when the graduating API is in fact totally compatible.

But what for? For the API source-code user, the downstream application needs to be keeping up with changes in the experimental API, and this change would be easy to follow, and this would just delay when they have to do it. So that doesn't seem a good reason.

So is it for ABI compatibility?

> (... as Greg said, we shouldn't change svn_x_foo()'s signature, to
> preserve ABI compatibility.)

What degree of ABI compatibility are we asking for? Greg's example had a "mid-LTS" client findings its libs updated to the next LTS version (*). So are we asking that a client built against 1.11 should still link and run against 1.14-LTS libs in which this API has graduated to stable; and the 1.11 experimental features are not expected to still work then, but other features should still work? And the 1.11 experimental features should then fail cleanly, not nastily?

(* I think we're now dismissing the idea that we'd release new stable APIs only in an LTS release and are deciding instead that all minor releases should follow the same rules. But a similar example could apply to any minor version in which experimental so let's continue.)

For that kind of ABI compatibility our first thought might be that we will need to use a subset of the kind of API version bumping and deprecation mechanism that we use for stable APIs. On a per-function basis, (if I recall C ABI rules correctly), functions would need this versioning whenever their signature changes, while structure definitions would not need versioning, if previous functions are to just return an error.

But that's not right. That leaves an overall API in which a 1.11 client with 1.12 libs will find that some functions still function (e.g. shelve your changes) while other functions now return failure (e.g. trying to unshelve). That's now a nasty failure mode.

The correct way to keep ABI-compatible failure stubs would be to apply version numbers to the whole API, not separately on each individual function.

Or are was asking for the same as that, but only across the one minor version where the experimental API graduates to stable, say from 1.13 to 1.14, and no such guarantee for any other jump?


Daniel Shahaf wrote:
> Julian Foad wrote on Mon, 17 Sep 2018 12:36 +0100:
> > Branko Čibej wrote:
> > > dummy functions that return APR_ENOTIMPL [...].
> > 
> > Not worth it. A client written against the experimental API is unlikely 
> > to handle that error gracefully (keeping all other functions working) 
> 
> How so?  A client app written against *any* API needs to be written to
> handle unexpected errors [...]

It depends. For simple, isolated, user-requested operation like "shelve this", if a client like TortoiseSVN receives an error it is going to just fail that operation and carry on with everything else.

But I was thinking a complex client is likely to do stuff like gather information about the WC state (e.g. including shelves, for example) and then display all the information. If a function throws an unexpected, unknown error during the information gathering, it is likely to give up at that point and not go on to display all the other available information.

Or a complex client may build some other operation on top of shelving, and then that higher level operation may not fail cleanly.

So making each individual API function call return "not implemented" doesn't mean the application overall will gracefully drop support for that feature.


> I think our choice is between always returning some error code, as
> above, and removing the function entirely, in which case the runtime
> linker will prevent the application from starting, so the user won't be
> able to use even non-experimental functions.  As much as I like "If you
> have to fail, fail early", bricking an app because a shared library it
> depends on was upgraded strikes me as an undesirable outcome.

But maybe we still want those stubs because that's better than not being able to run the app at all.

Or is there a better way to make an app that uses experimental APIs but won't break when libs are upgraded removing the experimental API entry points?

-- 
- Julian

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Nathan Hartman <ha...@gmail.com>.
In order to strike a balance between:

* Developing new features across the more frequent release
  schedule

* Allowing developers to offer these features early, and users to
  opt in to them

* Avoiding bricking a program as described by Daniel (program
  fails to start after DLLs are upgraded)

* Avoiding the hilarity scenario described by Greg (an API with
  formerly three parameters now takes five. Two of these will be
  undefined values from the stack.

* Avoiding keeping every defunct experimental API in the
  namespace forever

* (Did I miss anything else that should be in this list?)

I suggest the following:

* When an experimental API is declared stable:

  It is moved to the stable namespace. The experimental name is
  kept as an alias for the duration of that major version (e.g.,
  Subversion 1.x).

* When an experimental API is declared defunct:

  The Subversion project should define, as part of its API/ABI
  stability rules, a length of time after which defunct
  experimental APIs are removed.

  During that length of time, the function unconditionally returns
  an error (whichever error code that ends up being) and produces a
  compile-time deprecation warning, which lists the date.

  All such APIs are kept in a single source file, sorted by their
  scheduled destruction date. That significantly reduces the burden
  of maintenance and opens up an opportunity to automate the
  process.

  The next release after that date will not contain the removed
  APIs.

  I suggest one year as the length of time.

* When an experimental API changes (Greg's scenario), the names
  are changed to protect the innocent, and the old name is treated
  the same as a defunct experimental API described above.

Clients that use experimental APIs *should* implement the pattern
of requiring the user to opt in (e.g., by checking a box in a
preference dialog) and notify the user that this feature may break
or be removed in the near future. This should be stated in
Subversion's docs for downstream developers' consideration.

Rationale: That gives downstream developers, OS distributors,
system administrators, and end users a reasonable window of time
during which client and library upgrades will not break each other;
it provides a defined expectation that all parties can plan
against; and it avoids forever accumulating a perpetually growing
list of defunct APIs.

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Mon, 17 Sep 2018 12:36 +0100:
> Branko Čibej wrote:
> > Consequently, releasing experimental features to the stable set implies
> > removing [the experimental API] — and yes, this *will* create ABI
> > incompatibilities. If we decide that we cannot allow that and that ABI
> > compatibility is more important even for experimental stuff, we can keep
> > dummy functions that return APR_ENOTIMPL [...].
> 
> Not worth it. A client written against the experimental API is unlikely 
> to handle that error gracefully (keeping all other functions working) 

How so?  A client app written against *any* API needs to be written to
handle unexpected errors returned from that API, due to forward
compatibility (the runtime libraries might be newer than the compile-time
libraries).  APR_ENOTIMPL is, at worst, just another not-specifically-handled
err->apr_err value.

I think our choice is between always returning some error code, as
above, and removing the function entirely, in which case the runtime
linker will prevent the application from starting, so the user won't be
able to use even non-experimental functions.  As much as I like "If you
have to fail, fail early", bricking an app because a shared library it
depends on was upgraded strikes me as an undesirable outcome.

> unless it is a rather trivial client or was specifically designed and 
> *tested* against that scenario.

Cheers,

Daniel

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Julian Foad <ju...@apache.org>.
Branko Čibej wrote:
> The proposal to put experimental APIs (and command-line features) into a
> separate namespace is a variant of this off-by-default approach. [...]
> we can't exclude the experimental bits from the ABI [...]. Instead, we force
> downstream developers and users to consciously choose to use
> experimental features, with all that the choice implies.

Using an "x-" namespace for the API identifiers only makes developers aware. Imagine the developer of GUI "svn-x" releases a version of "svn-x" that uses experimental APIs. The users have no idea about that unless the developer takes care to tell them, perhaps by releasing it in a separate distribution channel, or naming it appropriately, or putting in-app messages marking which features depend on the experimental feature APIs.

I agree that (as you said) it is that developer's responsibility to communicate that status appropriately to the users, but let's not assume it will always be obvious to the users.

The "off-by-default" approach goes further than that. It would rather map to Subversion as requiring the user to (for example) set an environment variable ("SVN_ENABLE_EXPERIMENTAL") or install an extra package ("libsvn-experimental") without which the application would either fail to run at all or fail (gracefully or not) to provide those features. We certainly could implement this approach if we wanted to, in various ways.

Now, that applies only to applications that use separately distributed svn libs. TortoiseSVN doesn't -- it bundles its own build of svn libs -- and so its users wouldn't be affected by changes to our experimental APIs, only its developers would see the changes when they try to build and test a new version.

> A corollary of the above is that we should never have aliases for
> experimental bits in another namespace: for example, if there is an 'svn
> x-shelve' command it should not have an 'svn shelve' alias, since that
> could lead users to believe that the feature is no longer experimental.

I have to agree, yes.

I would quite like some users to be able to type just "svn shelve" when testing the experimental feature, but the proper way to enable that would be to allow the user to install or define aliases mapping "shelve" to "x-shelve" rather than to have these built in.

So I suppose I had better propose a patch that removes the non-x aliases.

> Consequently, releasing experimental features to the stable set implies
> removing [the experimental API] — and yes, this *will* create ABI
> incompatibilities. If we decide that we cannot allow that and that ABI
> compatibility is more important even for experimental stuff, we can keep
> dummy functions that return APR_ENOTIMPL [...].

Not worth it. A client written against the experimental API is unlikely to handle that error gracefully (keeping all other functions working) unless it is a rather trivial client or was specifically designed and *tested* against that scenario.

> As to what should be marked experimental, I maintain that my definition
> elsewhere in this thread matches our original reason for introducing the
> concept in the first place: to allow for developing experimental
> features across multiple minor releases in a way that lets users test
> them and give us feedback, yet allows us to avoid the burden of
> maintaining permanent API/ABI compatibility for defunct functionality.

Agreed.

-- 
- Julian

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Branko Čibej <br...@apache.org>.
On 16.09.2018 22:24, Julian Foad wrote:
> In this thread we have a lot of suggestions and disagreements. I started looking for existing practices that we could copy, and here is the first thing I found. The top three hits in a DuckDuckGo search for "experimental APIs" [1] have in common the use of a run-time guard that prevents ordinary stable client software from accidentally using them by requiring an explicit acknowledgement action.
>
> Chrome: you must enable Experimental Extension APIs in your browser, and "the Chrome Web Store doesn't allow you to upload items that use experimental APIs".
>
> Windows: "By default, these APIs are disabled at runtime and calling them will result in a runtime exception."
>
> OpenStack: "clients must include a specific HTTP header, X-OpenStack-Manila-API-Experimental".
>
> That's not something we've considered before. We could think of ways to implement such a thing, but, before we go there, even just having the possibility in mind may help us think more clearly about what we wish to achieve.


The proposal to put experimental APIs (and command-line features) into a
separate namespace is a variant of this off-by-default approach. Since
Subversion is a set of libraries, we can't exclude the experimental bits
from the ABI (or rather, we can, but doing so would defeat the purpose
of having experimental bits in the first place). Instead, we force
downstream developers and users to consciously choose to use
experimental features, with all that the choice implies.

A corollary of the above is that we should never have aliases for
experimental bits in another namespace: for example, if there is an 'svn
x-shelve' command it should not have an 'svn shelve' alias, since that
could lead users to believe that the feature is no longer experimental.

Consequently, releasing experimental features to the stable set implies
removing associated functions, types, commands, etc. from the
experimental namespace — and yes, this *will* create ABI
incompatibilities. If we decide that we cannot allow that and that ABI
compatibility is more important even for experimental stuff, we can keep
dummy functions that return APR_ENOTIMPL (we don't have to invent a new
error code).

As to what should be marked experimental, I maintain that my definition
elsewhere in this thread matches our original reason for introducing the
concept in the first place: to allow for developing experimental
features across multiple minor releases in a way that lets users test
them and give us feedback, yet allows us to avoid the burden of
maintaining permanent API/ABI compatibility for defunct functionality.

-- Brane


Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Julian Foad <ju...@apache.org>.
In this thread we have a lot of suggestions and disagreements. I started looking for existing practices that we could copy, and here is the first thing I found. The top three hits in a DuckDuckGo search for "experimental APIs" [1] have in common the use of a run-time guard that prevents ordinary stable client software from accidentally using them by requiring an explicit acknowledgement action.

Chrome: you must enable Experimental Extension APIs in your browser, and "the Chrome Web Store doesn't allow you to upload items that use experimental APIs".

Windows: "By default, these APIs are disabled at runtime and calling them will result in a runtime exception."

OpenStack: "clients must include a specific HTTP header, X-OpenStack-Manila-API-Experimental".

That's not something we've considered before. We could think of ways to implement such a thing, but, before we go there, even just having the possibility in mind may help us think more clearly about what we wish to achieve.


[1] https://developer.chrome.com/extensions/experimental , https://docs.microsoft.com/en-us/windows/uwp/whats-new/experimental-apis , https://docs.openstack.org/manila/latest/contributor/experimental_apis.html
-- 
- Julian

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Nathan Hartman wrote on Sun, 16 Sep 2018 15:04 -0400:
> I think there is no reason to add a unique error code for each removed
> function because those would accumulate forever as well. Instead,
> I'd create only one systemwide error code for this purpose.

Agreed.  "One systemwide error code" is what I meant to say.  Thanks for
catching that.  (When I wrote SVN_ERR_FOO I overlooked that 'foo' was
not a free variable in that paragraph.)

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Branko Čibej <br...@apache.org>.
On 16.09.2018 21:04, Nathan Hartman wrote:
> Returning an error code is probably a better option but many such
> functions may accumulate over time -- unless they are only kept for
> some predefined length of time and then ultimately removed.

I disagree. "Experimental" means "not for production" and also "may
vanish at any time, even in a patch release." If some downstream
developer decides to publish a tool that depends on experimental APIs,
and we remove those APIs or modify them, causing the downstream dev's
users to see breakage: that is *not* our problem. It's the problem of
whoever decided to ignore our "experimental" designation.

Typically we should use the "experimental" tag only for things that our
command-line tools use for feature development that spans more than one
release, and for nothing else.

-- Brane

P.S.: This reminds me that our svn_mtcc_ API is private, yet used by
svnmucc; it should be public and tagged experimental instead, our tools
really shouldn't depend on private APIs.


Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Nathan Hartman <ha...@gmail.com>.
On Sun, Sep 16, 2018 at 2:09 PM Stefan Sperling <st...@elego.de> wrote:

> I'm not sure. Exposing these APIs as private ones looked like a simple
> solution to me. On the other hand, maybe "experimental" is a designation
> which is easier for downstream users to understand as opposed to "private"
> which already has an implied meaning of "don't ever use this".
>
> In any case, we should consider declaring experimental APIs in
> dedicated header files, and maybe even in a separate directory.
> That might reduce the potential for confusion between "public"
> and "experimental".
>

There's a difference between *developer* "users" downstream and
actual users of software that uses the svn libs.

Developer users understand the difference between "private,"
"experimental," "public," etc.

Actual users shouldn't need to understand that, or be exposed to
mysterious breakage (or worse, corruption) that results from an API
changing because of an upgrade.

I think the experimental APIs should have the x_ prefix and version
number postfix that increases as development progresses; and when
an API is "blessed" the function is renamed to remove the x_ prefix
and all x_ versions of the experimental function are either removed
or made to unconditionally return an error code.

Removal means client software built against the libs will not link
(dynamically or statically), causing users to see breakage.

Returning an error code is probably a better option but many such
functions may accumulate over time -- unless they are only kept for
some predefined length of time and then ultimately removed.

I think there is no reason to add a unique error code for each removed
function because those would accumulate forever as well. Instead,
I'd create only one systemwide error code for this purpose.

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Stefan Sperling <st...@elego.de>.
On Sun, Sep 16, 2018 at 05:13:47PM +0100, Julian Foad wrote:
> Stefan Sperling wrote:
> > I'm starting to wonder what the point of public experimental APIs really is.
> > [...]
> > What about just making such APIs private and available under include/private?
> 
> subversion/include/private/README says:
> [[[
> Header files in this private/ directory are for internal APIs shared
> across Subversion's implementation.  They are not part of the public
> API, nor are they ever copied into or under the include/ directory
> (e.g. by the installation process).
> ]]]
> 
> In other words, we don't expose the "private" header files to downstream software.

Thanks for pointing this out. I wasn't aware of that.

Indeed it looks like the private/ directory is not a good place for
experimental public APIs, at least according to our existing policy.
 
> But maybe we should. Is there any practical reason not to? The argument that
> other people should not see them because we don't think they're good enough
> may not be a good argument. The argument that other people will start to
> depend on them if they are available (ignoring our warnings), and will then
> have a bad experience when we break them later, may also not be a good
> argument.

I'm not sure. Exposing these APIs as private ones looked like a simple
solution to me. On the other hand, maybe "experimental" is a designation
which is easier for downstream users to understand as opposed to "private"
which already has an implied meaning of "don't ever use this".

In any case, we should consider declaring experimental APIs in
dedicated header files, and maybe even in a separate directory.
That might reduce the potential for confusion between "public"
and "experimental".

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Julian Foad <ju...@apache.org>.
Stefan Sperling wrote:
> I'm starting to wonder what the point of public experimental APIs really is.
> [...]
> What about just making such APIs private and available under include/private?

subversion/include/private/README says:
[[[
Header files in this private/ directory are for internal APIs shared
across Subversion's implementation.  They are not part of the public
API, nor are they ever copied into or under the include/ directory
(e.g. by the installation process).
]]]

In other words, we don't expose the "private" header files to downstream software.

But maybe we should. Is there any practical reason not to? The argument that other people should not see them because we don't think they're good enough may not be a good argument. The argument that other people will start to depend on them if they are available (ignoring our warnings), and will then have a bad experience when we break them later, may also not be a good argument.

For reference, our documentation on the matter is located here:
https://subversion.apache.org/docs/community-guide/conventions.html#interface-visibility
and
https://subversion.apache.org/docs/community-guide/conventions.html#other-conventions

The Book doesn't mention the "private" category in its API section here:
http://svnbook.red-bean.com/en/1.8/svn.developer.usingapi.html

-- 
- Julian

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Stefan Sperling <st...@elego.de>.
On Sun, Sep 16, 2018 at 12:30:24PM +0000, Daniel Shahaf wrote:
> Thanks for elaborating on that, Greg.  I agree that the scenario you
> describe is something we should design to prevent.
> 
> In a nutshell, I think the point is that we promise ABI forward
> compatibility should deliver that even for experimental entry points.

I'm starting to wonder what the point of public experimental APIs really is.
They seem to just muddle up our simple public API rules for no good reason.

It seems to actual goal here is to encourage third party authors to integrate
experimental features in their clients for users to test. Is there more to it?

What about just making such APIs private and available under include/private?
There is no technical reason why third party clients would be unable to link
those functions into their program if they wish to. The private nature makes
it very clear that these APIs cannot be relied upon after upgrades.

That model is very simple to understand so I would encourage us to not
invent anything else if there is no urgent need. So far, I don't see how
adding more exceptions and rules to our API guidelines improves anything.

Note that our own 'svn' client is already calling private APIs:

shelf-cmd.c:      SVN_ERR(svn_client__get_diff_summarize_callbacks(&diff_processor,
shelf-cmd.c:      SVN_ERR(svn_client__get_diff_writer_svn(
shelf-cmd.c:  SVN_ERR(svn_client__shelf_diff(shelf_version, "",
util.c:  err = svn_client__assert_homogeneous_target_type(targets);

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Greg Stein wrote on Sun, 16 Sep 2018 04:16 -0500:
> On Sat, Sep 15, 2018 at 8:48 AM Greg Stein <gs...@gmail.com> wrote:
> >...
> 
> > No no no... I agree with Brane above. It is confusing, and if people
> > mistakenly mix/match releases things will Just Break. Mysteriously. And
> > horribly. And possibly data-destructively.
> >
> 
> To clarify the above a bit:
> 
> Consider if the mid-LTS had an entrypoint with 3 parameters, [...] In
> the LTS, the entrypoint takes 5 parameters. So the function reads
> wonky stuff from the stack. Hilarity ensues.

Thanks for elaborating on that, Greg.  I agree that the scenario you
describe is something we should design to prevent.

In a nutshell, I think the point is that we promise ABI forward
compatibility should deliver that even for experimental entry points.
I can think of two alternative ways to achieve this:

1. An API function marked experimental may not have its signature
   changed within a single major release line.  The function may,
   however, be removed at any 1.x.0 release.

2. An API function marked experimental may neither be removed nor have
   its signature change within a single major release line (just like a
   non-experimental function); however, its implementation _may_ be
   changed in any minor (1.x.0) release to error out unconditionally,
   ideally with an error code documented in the first minor release that
   function appeared in (for forward compatibility).

   For example, If 1.11.0 adds an svn_x_foo() function, we'll also add
   an SVN_ERR_FOO error code and document that in a future minor
   release, svn_x_foo()'s implementation may do nothing and return
   SVN_ERR_FOO, and that a non-svn_x_* variant of svn_x_foo() may or may
   not be added at that time.

Would that work?

Cheers,

Daniel

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Nathan Hartman <ha...@gmail.com>.
On Sun, Sep 16, 2018 at 5:17 AM Greg Stein <gs...@gmail.com> wrote:

> On Sat, Sep 15, 2018 at 8:48 AM Greg Stein <gs...@gmail.com> wrote:
> >...
>
>> No no no... I agree with Brane above. It is confusing, and if people
>> mistakenly mix/match releases things will Just Break. Mysteriously. And
>> horribly. And possibly data-destructively.
>>
>
> To clarify the above a bit:
>
> Consider if the mid-LTS had an entrypoint with 3 parameters, so a client
> coded to that to (also) experiment with the feature set. So Alan has got
> his client installed, trying out this new "shelve" feature, running against
> the mid-LTS release that has been on the system for the past year.
>
> The following week, Brian, his system administrator upgrades to the LTS
> release, for the admin's sanity. "What? LTS releases? YAY! Less work for
> me."
>
> In the LTS, the entrypoint takes 5 parameters. So the function reads wonky
> stuff from the stack. Hilarity ensues.
>
> Brian now gets a call from Alan: "I was working with $client, doing all my
> normal Subversion work. But now it seems that my working copy is corrupted.
> What happened?"
>
> The hilarity ends. Alan is told he can't use his nifty client. Alan gets
> angry. Brian bangs his head on his desk, and wonders how 18 years of random
> Subversion upgrades were always safe. But not this week. Why did it break
> this week? And it was an LTS release!?! Why....
>

If the function name begins with x_ until the API stabilizes, that should
prevent the 3 parameter/5 parameter hilarity described above, no? The
client should fail to start due to dynamic linking failure.

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Greg Stein <gs...@gmail.com>.
On Sat, Sep 15, 2018 at 8:48 AM Greg Stein <gs...@gmail.com> wrote:
>...

> No no no... I agree with Brane above. It is confusing, and if people
> mistakenly mix/match releases things will Just Break. Mysteriously. And
> horribly. And possibly data-destructively.
>

To clarify the above a bit:

Consider if the mid-LTS had an entrypoint with 3 parameters, so a client
coded to that to (also) experiment with the feature set. So Alan has got
his client installed, trying out this new "shelve" feature, running against
the mid-LTS release that has been on the system for the past year.

The following week, Brian, his system administrator upgrades to the LTS
release, for the admin's sanity. "What? LTS releases? YAY! Less work for
me."

In the LTS, the entrypoint takes 5 parameters. So the function reads wonky
stuff from the stack. Hilarity ensues.

Brian now gets a call from Alan: "I was working with $client, doing all my
normal Subversion work. But now it seems that my working copy is corrupted.
What happened?"

The hilarity ends. Alan is told he can't use his nifty client. Alan gets
angry. Brian bangs his head on his desk, and wonders how 18 years of random
Subversion upgrades were always safe. But not this week. Why did it break
this week? And it was an LTS release!?! Why....

Cheers,
-g

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Greg Stein <gs...@gmail.com>.
On Thu, Sep 13, 2018 at 10:24 AM Branko Čibej <br...@apache.org> wrote:

> On 13.09.2018 17:11, Julian Foad wrote:
> > Julian Foad wrote:
> >> [...] Are we saying now
> >> that they need not be specifically marked if we feel they are pretty
> >> safe? If we say that, then marking specific APIs as "experimental" in a
> >> regular release signifies only that we consider them more experimental
> >> (less stable) than others.
> >>
> >> That might be fine. Anyone developing against a regular release is
> >> necessarily developing against new (experimental) APIs, so maybe no
> >> explicit warning mechanism is necessary.
> > I have gone ahead with producing a release candidate 1.11.0-rc1 with
> things just as they are. It currently seems OK to me. If we decide we need
> to change this, we can.
>
>
> I've been thinking about this idea of having different compatibility
> rules for LTS and regular releases. For 18 years now we've promised and
> maintained fairly simple rules of API and ABI compatibility; so much so
> that they're implied in and used by our own code. I've come to the
> conclusion that changing these rules just because we're changing how
> many releases we support would be a very bad idea indeed.
>

Being absent from that discussion ... oh geez.

No no no... I agree with Brane above. It is confusing, and if people
mistakenly mix/match releases things will Just Break. Mysteriously. And
horribly. And possibly data-destructively. And in 18 years, we've never
allowed for API/ABI breakage. Never.

We've been able to be hard-core with our compatibility guarantees for 18
years. Why stop?

-g

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Branko Čibej <br...@apache.org>.
On 13.09.2018 17:11, Julian Foad wrote:
> Julian Foad wrote:
>> [...] Are we saying now 
>> that they need not be specifically marked if we feel they are pretty 
>> safe? If we say that, then marking specific APIs as "experimental" in a 
>> regular release signifies only that we consider them more experimental 
>> (less stable) than others.
>>
>> That might be fine. Anyone developing against a regular release is 
>> necessarily developing against new (experimental) APIs, so maybe no 
>> explicit warning mechanism is necessary.
> I have gone ahead with producing a release candidate 1.11.0-rc1 with things just as they are. It currently seems OK to me. If we decide we need to change this, we can.


I've been thinking about this idea of having different compatibility
rules for LTS and regular releases. For 18 years now we've promised and
maintained fairly simple rules of API and ABI compatibility; so much so
that they're implied in and used by our own code. I've come to the
conclusion that changing these rules just because we're changing how
many releases we support would be a very bad idea indeed.

Furthermore, the "experimental" designator should be used only for APIs
that, for one reason or another, cannot be private, yet we know that
we're not prepared to support them yet. Saying that everything in a
regular release is experimental would just sow confusion; it would tell
users not to rely on the bug fixes and new features in those releases.

The fact that the experimental designation currently isn't visible at
the ABI level is a separate problem and we shouldn't conflate it with
the meaning of a released API.

-- Brane


Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Julian Foad <ju...@apache.org>.
Julian Foad wrote:
> [...] Are we saying now 
> that they need not be specifically marked if we feel they are pretty 
> safe? If we say that, then marking specific APIs as "experimental" in a 
> regular release signifies only that we consider them more experimental 
> (less stable) than others.
> 
> That might be fine. Anyone developing against a regular release is 
> necessarily developing against new (experimental) APIs, so maybe no 
> explicit warning mechanism is necessary.

I have gone ahead with producing a release candidate 1.11.0-rc1 with things just as they are. It currently seems OK to me. If we decide we need to change this, we can.

-- 
- Julian

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Julian Foad <ju...@apache.org>.
Julian Foad wrote:
> Daniel Shahaf wrote:
>> To be concrete, I propose that we have exactly two kinds of public APIs:
>> 
>> [[[
>> - Standard public APIs are supported through the end of the major
>>   release line (1.x, 2.x, etc) they appear in.
>> 
>> - Experimental public APIs are named svn_x_* and are supported through
>>   the end of the minor release line (1.11.x, 1.12.x, etc) they appear
>>   in.  A particular API may make stronger promises in its docstring, but
>>   not weaker.  (If we ever need to make even weaker a promise than this,
>>   APIs covered by it should be in a different namespace.)
>> 
>>   Experimental public APIs may be graduated to non-experimental status
>>   at any minor release.
>> ]]]
> 
> I would like to add, after a brief IRC discussion with brane and yourself:
> 
> - Each experimental public API SHOULD also have the "SVN_EXPERIMENTAL" 
> function decorator (where applicable) and the "@warning EXPERIMENTAL" 
> Doxygen tag. (These MUST NOT be used on a standard public API.)
> 
> - The 'svn' command-line UI SHOULD also use an "x-" prefix for 
> subcommands (and "--x-" for options). However it MAY also supply aliases 
> without the prefix.
> 
>> And I further propose that we implement this convention on the 1.11.x
>> branch before 1.11.0.
> 
> I will prepare a patch, but not commit it until we have waited a bit 
> longer for consensus, and I will get ready to roll another RC.

I filed this proposal as issue https://issues.apache.org/jira/browse/SVN-4775 "Add x- prefix to experimental APIs".

I have attached the required changes as two patches (one for viewspec, one for shelving) to https://issues.apache.org/jira/browse/SVN-4775 .

Can anyone give further thoughts on whether this is definitely what we should do for 1.11?

-- 
- Julian

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Julian Foad <ju...@apache.org>.
Hi Daniel. Your argument makes sense.

Daniel Shahaf wrote:
> To be concrete, I propose that we have exactly two kinds of public APIs:
> 
> [[[
> - Standard public APIs are supported through the end of the major
>   release line (1.x, 2.x, etc) they appear in.
> 
> - Experimental public APIs are named svn_x_* and are supported through
>   the end of the minor release line (1.11.x, 1.12.x, etc) they appear
>   in.  A particular API may make stronger promises in its docstring, but
>   not weaker.  (If we ever need to make even weaker a promise than this,
>   APIs covered by it should be in a different namespace.)
> 
>   Experimental public APIs may be graduated to non-experimental status
>   at any minor release.
> ]]]

I would like to add, after a brief IRC discussion with brane and yourself:

- Each experimental public API SHOULD also have the "SVN_EXPERIMENTAL" function decorator (where applicable) and the "@warning EXPERIMENTAL" Doxygen tag. (These MUST NOT be used on a standard public API.)

- The 'svn' command-line UI SHOULD also use an "x-" prefix for subcommands (and "--x-" for options). However it MAY also supply aliases without the prefix.

> And I further propose that we implement this convention on the 1.11.x
> branch before 1.11.0.

I will prepare a patch, but not commit it until we have waited a bit longer for consensus, and I will get ready to roll another RC.

-- 
- Julian

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Thu, Sep 13, 2018 at 09:44:47 +0100:
> Daniel Shahaf wrote:
> > Julian Foad wrote:
> >> We recently decided that only APIs released in an LTS release will be 
> >> subject to our compatibility guarantees. As 1.11 is not an LTS release, 
> >> the above APIs will not be subject to those guarantees (until they 
> >> appear in an LTS).
> >> 
> >> Do we need to do anything in the source code to emphasize that, for the 
> >> ones that are not already marked 'experimental'? If so, what? Mark them 
> >> 'experimental' in addition to their existing annotations (which probably 
> >> would mean we'd want to turn off the warnings about use of experimental 
> >> APIs in our own builds), or something else?
> > 
> > I'm not sure all of these APIs need to be marked experimental.  For
> > example, svn_client_layout_list() is new functionality that we might not
> > want to support forever, but the 'pretty_print_mergeinfo' change sounds
> > pretty safe.
> 
> How do you square that with the proposal in the thread "API compatibility promises in light of biannual releases" where you replied "+1" to the following?
> 

I'm trying to strike a balance between two extremes.

On the one hand, under our compatibility rules, releasing an API is a
one-way street: it compels us to support it indefinitely.  On the other
hand, if an API _is_ stable, we needn't sit on the formality of waiting
for an LTS release to declare it so.

Case in point, the addition of PRETTY_PRINT_MERGEINFO to
svn_client_diff7().  The new behaviour is to *disable* a special-case
code path; I can hardly see a situation in which continuing to support
such a disabling would be a maintenance burden, and therefore, I see no
reason why svn_client_diff7() shouldn't be released under the standard
compatibility promise we've used through 1.10.

Second case in point, the shelving logic.  In 1.10 and 1.11 we're
releasing early forms of shelving for consumption by users and API
consumers.  We're hoping this will result in feedback that will allow
us to write the API better the first time around — but this is only
possible if what we *are* releasing in 1.10 and 1.11 doesn't get the
same compatibility promises as everything else.

Frequent minor releases are a good thing, and releasing APIs under
reduced compatibility promises at first (and strengthening the promises
subsequently) is also a good thing, but I think I'm coming to agree with
what Brane said downthread, that these two good things should neither be
conflated nor coupled.  Some releases receive bugfixes for ×4 times as
long as others.  Some releases see APIs being bestowed upon the "Will be
supported through the end of time^Wthe 1.x line" badge (either new APIs,
or ones that were previously released under reduced forward compatibility
promises).  These can and should be two independent aspects of a release,
shouldn't they?

Thanks for asking.

To be concrete, I propose that we have exactly two kinds of public APIs:

[[[
- Standard public APIs are supported through the end of the major
  release line (1.x, 2.x, etc) they appear in.

- Experimental public APIs are named svn_x_* and are supported through
  the end of the minor release line (1.11.x, 1.12.x, etc) they appear
  in.  A particular API may make stronger promises in its docstring, but
  not weaker.  (If we ever need to make even weaker a promise than this,
  APIs covered by it should be in a different namespace.)

  Experimental public APIs may be graduated to non-experimental status
  at any minor release.
]]]

And I further propose that we implement this convention on the 1.11.x
branch before 1.11.0.

Thanks for asking.

Cheers,

Daniel

> > However, a process of marking all new APIs as experimental in
> > regular releases, and ensuring we review them for LTS releases,
> > might be a good way to satisfy both Semantic Versioning and
> > experimental APIs.
> 
> There, the proposal was that *all* new APIs (which I would assume
> includes revved ones) should be regarded as experimental and marked as
> such until "blessed" by appearing in an LTS release. Are we saying now
> that they need not be specifically marked if we feel they are pretty
> safe? If we say that, then marking specific APIs as "experimental" in
> a regular release signifies only that we consider them more
> experimental (less stable) than others.
> 
> That might be fine. Anyone developing against a regular release is
> necessarily developing against new (experimental) APIs, so maybe no
> explicit warning mechanism is necessary.
> 
> -- - Julian

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Julian Foad <ju...@apache.org>.
Daniel Shahaf wrote:
> Julian Foad wrote:
>> We recently decided that only APIs released in an LTS release will be 
>> subject to our compatibility guarantees. As 1.11 is not an LTS release, 
>> the above APIs will not be subject to those guarantees (until they 
>> appear in an LTS).
>> 
>> Do we need to do anything in the source code to emphasize that, for the 
>> ones that are not already marked 'experimental'? If so, what? Mark them 
>> 'experimental' in addition to their existing annotations (which probably 
>> would mean we'd want to turn off the warnings about use of experimental 
>> APIs in our own builds), or something else?
> 
> I'm not sure all of these APIs need to be marked experimental.  For
> example, svn_client_layout_list() is new functionality that we might not
> want to support forever, but the 'pretty_print_mergeinfo' change sounds
> pretty safe.

How do you square that with the proposal in the thread "API compatibility promises in light of biannual releases" where you replied "+1" to the following?

> However, a process of marking all new APIs as experimental in regular 
> releases, and ensuring we review them for LTS releases, might be a good 
> way to satisfy both Semantic Versioning and experimental APIs.

There, the proposal was that *all* new APIs (which I would assume includes revved ones) should be regarded as experimental and marked as such until "blessed" by appearing in an LTS release. Are we saying now that they need not be specifically marked if we feel they are pretty safe? If we say that, then marking specific APIs as "experimental" in a regular release signifies only that we consider them more experimental (less stable) than others.

That might be fine. Anyone developing against a regular release is necessarily developing against new (experimental) APIs, so maybe no explicit warning mechanism is necessary.

-- 
- Julian

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Branko Čibej <br...@apache.org>.
On 12.09.2018 22:16, Daniel Shahaf wrote:
> Julian Foad wrote on Wed, 12 Sep 2018 20:21 +0100:
>> The following public APIs are new or revved since 1.10:
>>
>> svn_client_layout_func_t
>> svn_client_layout_list
>>   - New for 'viewspec' output; marked 'experimental'.
>>> svn_client_diff7
>> svn_client_diff_peg7
>>   - Adds a 'pretty_print_mergeinfo' option.
>>
>> svn_client_revert4
>> svn_wc_revert6
>>   - Adds an 'added_keep_local' option.
>>
>> svn_client_conflict_option_id_t
>>   - 3 new enumerators.
>>
>> svn_client_shelf_*
>>   - Marked 'experimental'.
>>
>> SVN_OPT_MAX_PARAGRAPHS
>> svn_opt_subcommand_desc3_t
>> svn_opt_get_canonical_subcommand3
>> svn_opt_get_option_from_code3
>> svn_opt_subcommand_takes_option4
>> svn_opt_print_generic_help3
>> svn_opt_subcommand_help4
>> svn_opt_print_help5
>>   - For multi-paragraph command-line help text.
>>
>>
>> ** QUESTION **
>>
>> We recently decided that only APIs released in an LTS release will be 
>> subject to our compatibility guarantees. As 1.11 is not an LTS release, 
>> the above APIs will not be subject to those guarantees (until they 
>> appear in an LTS).
>>
>> Do we need to do anything in the source code to emphasize that, for the 
>> ones that are not already marked 'experimental'? If so, what? Mark them 
>> 'experimental' in addition to their existing annotations (which probably 
>> would mean we'd want to turn off the warnings about use of experimental 
>> APIs in our own builds), or something else?
> I'm not sure all of these APIs need to be marked experimental.  For
> example, svn_client_layout_list() is new functionality that we might not
> want to support forever, but the 'pretty_print_mergeinfo' change sounds
> pretty safe.
>
> Separately, regarding how to mark an API experimental, can we put them
> into an svn_x_* namespace, e.g., svn_x_client_foo()?  When we're ready
> to declare an API stable we'll add svn_client_foo() (no "x_") and keep
> svn_x_client_foo() as a function wrapping svn_client_foo().  This way we
> can keep ABI and API compatibility for the svn_x_ name, if we want, and
> API consumers will have a clear warning sign about the experimental
> status: right in the name of the function, as opposed to some compiler
> diagnostic.


If we invent an svn_x_ namespace, we should never keep these names
available once the API is "blessed" by an LTS release. Otherwise the
experimental designation loses its purpose.


> Simply slapping an SVN_EXPERIMENTAL decorator on an API does nothing
> to alert bindings users to its experimental status; and we don't want
> bindings users to have a mistaken impression that an experimental API is
> supported, or we'd find ourselves having to support an API we didn't
> intend to.

I wonder if we can make the SVN_EXPERIMENTAL macro add appropriate
prefixes to the function name. That would leave us with *one* mechanism
to tag experimental functionality and remove that tag when it's no
longer needed.

-- Brane

Re: API review for 1.11; do we need to mark new APIs as experimental?

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Wed, 12 Sep 2018 20:21 +0100:
> The following public APIs are new or revved since 1.10:
> 
> svn_client_layout_func_t
> svn_client_layout_list
>   - New for 'viewspec' output; marked 'experimental'.
> > svn_client_diff7
> svn_client_diff_peg7
>   - Adds a 'pretty_print_mergeinfo' option.
> 
> svn_client_revert4
> svn_wc_revert6
>   - Adds an 'added_keep_local' option.
> 
> svn_client_conflict_option_id_t
>   - 3 new enumerators.
> 
> svn_client_shelf_*
>   - Marked 'experimental'.
> 
> SVN_OPT_MAX_PARAGRAPHS
> svn_opt_subcommand_desc3_t
> svn_opt_get_canonical_subcommand3
> svn_opt_get_option_from_code3
> svn_opt_subcommand_takes_option4
> svn_opt_print_generic_help3
> svn_opt_subcommand_help4
> svn_opt_print_help5
>   - For multi-paragraph command-line help text.
> 
> 
> ** QUESTION **
> 
> We recently decided that only APIs released in an LTS release will be 
> subject to our compatibility guarantees. As 1.11 is not an LTS release, 
> the above APIs will not be subject to those guarantees (until they 
> appear in an LTS).
> 
> Do we need to do anything in the source code to emphasize that, for the 
> ones that are not already marked 'experimental'? If so, what? Mark them 
> 'experimental' in addition to their existing annotations (which probably 
> would mean we'd want to turn off the warnings about use of experimental 
> APIs in our own builds), or something else?

I'm not sure all of these APIs need to be marked experimental.  For
example, svn_client_layout_list() is new functionality that we might not
want to support forever, but the 'pretty_print_mergeinfo' change sounds
pretty safe.

Separately, regarding how to mark an API experimental, can we put them
into an svn_x_* namespace, e.g., svn_x_client_foo()?  When we're ready
to declare an API stable we'll add svn_client_foo() (no "x_") and keep
svn_x_client_foo() as a function wrapping svn_client_foo().  This way we
can keep ABI and API compatibility for the svn_x_ name, if we want, and
API consumers will have a clear warning sign about the experimental
status: right in the name of the function, as opposed to some compiler
diagnostic.

Simply slapping an SVN_EXPERIMENTAL decorator on an API does nothing
to alert bindings users to its experimental status; and we don't want
bindings users to have a mistaken impression that an experimental API is
supported, or we'd find ourselves having to support an API we didn't
intend to.

> We should certainly mention this explicitly in the 1.11 release notes.

Cheers,

Daniel