Update shared library advice now that a.out no longer is a consideration

[emaste]

documentation/content/en/books/developers-handbook/policies/chapter.adoc

326–327

other bullets do not have trailing .

[emaste]

LGTM with minor nit

[imp]

word smith

[debdrup]

Looks good to me too, let's get this in.

[jhb]

documentation/content/en/books/developers-handbook/policies/chapter.adoc

325–326

I would be tempted to expand this a bit further and subsume your new point above, maybe something like:

* If there is an incompatible change, there are a few options:

   * If the library uses versioned symbols, bump the version of affected symbols and provide compatibility versions of affected symbols under the old version.

   * If the library uses versioned symbols, but the compatibility cannot be preserved with compatibility symbols (e.g. the layout of an exported global variable changed), bump the major number.  Along with bumping the major number, old compatibility symbols should be removed.

    * If the library does not use versioned symbols, bump major number.  Consider adding a version map to begin versioning symbols when bumping the major.

325–326

[imp]

Updates from jhb and emaste

[imp]

documentation/content/en/books/developers-handbook/policies/chapter.adoc

326–327

pulled in jhb's and emaste's suggestions.

[kib]

documentation/content/en/books/developers-handbook/policies/chapter.adoc

325–326

IMO it does not provide the right attitude.

First it should emphasize that ABI should not be broken. If this is absolutely impossible, then the breakage should be done in backward-compatible manner.

For symver-ed libraries, attitude should be that we do not do changes that would require bumping the library version. If the change of the nature that makes it impossible to handle most of the cases with the application of symver, it should be heavily considered if this change worth it at all.

That said, it is not true that gobal symbols cannot be versioned, or that layout changes for them are blockers. For instance, we sometimes grow sys_errlist[] when adding new errno values.

[imp]

Strengthen the "don't bump major versions" per kib@

[imp]

documentation/content/en/books/developers-handbook/policies/chapter.adoc

325–326

Updated to reflect kib pointing out the practice is to not bump major versions where possible.

[kib]

documentation/content/en/books/developers-handbook/policies/chapter.adoc

326–356

We do allow changing the libraries ABI, otherwise FreeBSD cannot evolve. But we make concerned efforts and pay large tax to make ABI changes backward-compatible.

327

This is not what I wrote or asked for. For non-symvered libs, we have no other way than bump library version.

But for symvered, the change that requires library version bump probably should be not done (but other strong reasons must exist to even make this argument for bumping).

330

I think 'top' there is 'to'.

And the wording is strange. I suspect you mean that the version bump should be last, since from now on we would have symver?

331

Also it probably should be noted, that for third-party libraries that do not provide stable ABI or which ABI compat policies are not compatible with FreeBSD policy, we do something special. Either make them private (sqlite?) or just follow upstream (openssl?).

For third-party libs which do provide comparable ABI statibility guarantess, we use upstream tooling (e.g. symver for libstdc++ or libcxx).

[jhb]

documentation/content/en/books/developers-handbook/policies/chapter.adoc

325–326

I agree we don't want breakage. I was perhaps trying to say that there might some extreme cases where symvers alone doesn't save you. Growing sys_errlist isn't one of those, but if we had a public structure and wanted to remove fields from it, that would be painful. OTOH, in my (still WIP) patches to bump fileno in FILE to int I have gone to great lengths to preserve the ABI for fields currently abused by existing software by leaving their ABI intact even when they are hidden in the future.

326

I don't know that we always want a version map. Off the top of my head I would guess that a minority of the libraries in /lib and /usr/lib are versioned. Partly because adding versions itself is a major change requiring a bump, so instead, we should probably defer adding versions until there's another reason to bump and then at that time evaluate if for the given library, adding versions would be a good idea.

[imp]

try again to see if these changes address kib's concerns

[imp]

documentation/content/en/books/developers-handbook/policies/chapter.adoc

326

When creating new libraries, we should always do version mapped or the library must be private. for existing libraries, you may be right...

[imp]

documentation/content/en/books/developers-handbook/policies/chapter.adoc

326

That is to say, the cost of a version map for existing libraries is high. The cost for new ones that want to do ABI stability is low if done from the start. So we should encourage them strongly for FreeBSD-created libraries at least.

[kib]

documentation/content/en/books/developers-handbook/policies/chapter.adoc

325–326

This probably should say 'for project-maintained shared libraries', since we cannot guarantee ABI stability for third-party. Look at the ncurses saga.

326

This makes the library symver-ed.

326–356

I am not sure where this 'only one increment between major releases' comes from. It would makes HEAD users' live miserable in some situations.

I believe we should not punish them, who provide such valuable and otherwise limited testing of the HEAD.

[imp]

documentation/content/en/books/developers-handbook/policies/chapter.adoc

325–326

Good point of clarification.

326

Yes. I'm saying that all new libraries should be either symver'd or private unless there's a good reason not to do so.

326–356

It's the current policy that we've done since day one in the project.

Having said that, we can certainly change it since, honestly, it was tied in many ways to the OS == library version notion people had in their heads. And to avoid undue churn, especially in the era when changes were dumped incrementally. And we already bump many times for ports.

So, I agree with you it's a bad policy that we should revisit.

How about: When changing a library in a way that requires a major version bump, please be mindful of the affect this has on users upgrading and try to batch changes where possible to reduce churn.

[kib]

documentation/content/en/books/developers-handbook/policies/chapter.adoc

326–356

Fine with me.

[imp]

documentation/content/en/books/developers-handbook/policies/chapter.adoc

326–356

OK. Done, and made it a bullet list, which is easier to read, imho

[imp]

update with latest feedback... move to bullet points

[imp]

btw, I'm starting to think a new review would be good given that all the content has migrated away from the comments...

[debdrup]

I spotted a few minor nits.

Also, please take care to either consistently terminate list items with or without a period.
Right now, some list items have it and others don't.

It's outside the scope of this review, but I feel like there should be a style guide for this.

documentation/content/en/books/developers-handbook/policies/chapter.adoc

333

Did you miss a word here?

338

I don't believe listed items should start with 'However' as the indentation already indicates that it's a sub-clause of the above item.

341

A tiny bit of wordsmithing is required here.

344

Tiny typo here.

348

Using our here seems to imply that it's written from a different perspective than the rest of the article.

350

Same as above.

355

Make it clear that exp-runs should be run to catch the full impact.

[imp]

debdrub's changes

[debdrup]

Looks good to me, let's land this. :)