Proposal: SDK libraries may not depend on internal FIDL APIs

[Hunter Freyer]

Esteemed API Council members,

While working on platform versioning, Seb has identified some client libraries which are shipped with the partner SDK but depend on FIDL APIs which aren't. He identified the following FIDL APIs in this situation:

• fuchsia.fs
• fuchsia.hardware.pty
• fuchsia.net.name
• fuchsia.posix.*
• fuchsia.vulkan.loader

The problem is that "partner" APIs differ from "internal" APIs along two different dimensions: visibility and stability. These APIs don't fit neatly into either category; they want to be private (like "internal" APIs) but they need to be stable (like "partner" APIs) because they're being called from code that isn't updated in lock-step with the system.

The simplest way to address this situation is to get rid of this category of API and say that all APIs accessed by libraries in the "partner" SDK must also be "partner" (or stabler). This policy would be basically a "libraries" version of the policy established for tools in RFC-0169. 

An alternative would be to introduce a new "internal-visible, partner-stable" API category, but before considering that, we should have a pretty strong justification for why the existing categories are insufficient.

csuter, tamird, jbauman, brunodalbo: Do you have any concerns about elevating the APIs listed above to "partner" level?

Everyone else: Do you have any concerns about this policy in general? If not, I'll write up a quick RFC and we can work on tooling to enforce it.

Thanks,

Hunter

[John Bauman]

On Fri, Jan 6, 2023 at 11:17 AM Hunter Freyer <hjfr...@google.com> wrote:

Esteemed API Council members,

While working on platform versioning, Seb has identified some client libraries which are shipped with the partner SDK but depend on FIDL APIs which aren't. He identified the following FIDL APIs in this situation:

• fuchsia.fs
• fuchsia.hardware.pty
• fuchsia.net.name
• fuchsia.posix.*
• fuchsia.vulkan.loader

The problem is that "partner" APIs differ from "internal" APIs along two different dimensions: visibility and stability. These APIs don't fit neatly into either category; they want to be private (like "internal" APIs) but they need to be stable (like "partner" APIs) because they're being called from code that isn't updated in lock-step with the system.

The simplest way to address this situation is to get rid of this category of API and say that all APIs accessed by libraries in the "partner" SDK must also be "partner" (or stabler). This policy would be basically a "libraries" version of the policy established for tools in RFC-0169. 

An alternative would be to introduce a new "internal-visible, partner-stable" API category, but before considering that, we should have a pretty strong justification for why the existing categories are insufficient.

csuter, tamird, jbauman, brunodalbo: Do you have any concerns about elevating the APIs listed above to "partner" level?

I was planning on moving fuchsia.vulkan.loader into the SDK this quarter, so no objection from me. 

[Bruno Dal Bo]

Speaking from the perspective of fuchsia.net.name and fuchsia.posix.*, for which FDIO is a client in the SDK.

I think from a practical standpoint, being able to prevent the library from being consumed directly is beneficial until we feel it's ready for direct use by applications. For fuchsia.posix.* I do worry about unexpected usage creep if we just put it in the SDK. For fuchsia.net.name we just don't have a ready and willing user to justify it, which has been our bar for adding things to the SDK.

I also assume that we'd want more than just elevate to "partner", we'd probably want to apply FIDL versioning to them as well, right? I think applying the versioning might be beneficial for us to be able to use it to reason about safe API evolution more easily, which we've been doing without tooling help so far. But given zxio/FDIO is compiled with a HEAD SDK, I fear that the incentive quickly becomes to maintain most of the api at `added=HEAD` which is effectively the same as not having it in the SDK, unless I'm missing something.

If the tooling you're proposing would help me reason about API versioning within zxio/FDIO, I think the potential benefits outweigh the risk of usage creep. Though the best of both worlds, in my opinion, would be to have both: Libraries in the SDK can only reference APIs that exist in the SDK (stability guardrails) and I can have an "internal-visible partner-stable" API category (visibility guardrails).

--

Bruno

[Adam Barth]

On Fri, Jan 6, 2023 at 6:31 PM Bruno Dal Bo <bruno...@google.com> wrote:

Speaking from the perspective of fuchsia.net.name and fuchsia.posix.*, for which FDIO is a client in the SDK.

I think from a practical standpoint, being able to prevent the library from being consumed directly is beneficial until we feel it's ready for direct use by applications. For fuchsia.posix.* I do worry about unexpected usage creep if we just put it in the SDK. For fuchsia.net.name we just don't have a ready and willing user to justify it, which has been our bar for adding things to the SDK.

I also assume that we'd want more than just elevate to "partner", we'd probably want to apply FIDL versioning to them as well, right? I think applying the versioning might be beneficial for us to be able to use it to reason about safe API evolution more easily, which we've been doing without tooling help so far. But given zxio/FDIO is compiled with a HEAD SDK, I fear that the incentive quickly becomes to maintain most of the api at `added=HEAD` which is effectively the same as not having it in the SDK, unless I'm missing something.

+Sébastien Marchand is also working on building the SDK shared libraries at specific target API levels to address this issue.  We'll eventually have N copies of libfdio.so in the SDK, one for every supported API level in that SDK.  Those copies will be built against that API level.

[Hunter Freyer]

On Fri, Jan 6, 2023 at 1:47 PM Adam Barth <aba...@google.com> wrote:

On Fri, Jan 6, 2023 at 6:31 PM Bruno Dal Bo <bruno...@google.com> wrote:

Speaking from the perspective of fuchsia.net.name and fuchsia.posix.*, for which FDIO is a client in the SDK.

I think from a practical standpoint, being able to prevent the library from being consumed directly is beneficial until we feel it's ready for direct use by applications. For fuchsia.posix.* I do worry about unexpected usage creep if we just put it in the SDK. For fuchsia.net.name we just don't have a ready and willing user to justify it, which has been our bar for adding things to the SDK.

I also assume that we'd want more than just elevate to "partner", we'd probably want to apply FIDL versioning to them as well, right?

Correct - properly versioning these FIDL libraries is the main motivation here.

[Sébastien Marchand]

On Fri, Jan 6, 2023 at 1:48 PM Hunter Freyer <hjfr...@google.com> wrote:

On Fri, Jan 6, 2023 at 1:47 PM Adam Barth <aba...@google.com> wrote:

On Fri, Jan 6, 2023 at 6:31 PM Bruno Dal Bo <bruno...@google.com> wrote:

Speaking from the perspective of fuchsia.net.name and fuchsia.posix.*, for which FDIO is a client in the SDK.

I think from a practical standpoint, being able to prevent the library from being consumed directly is beneficial until we feel it's ready for direct use by applications. For fuchsia.posix.* I do worry about unexpected usage creep if we just put it in the SDK. For fuchsia.net.name we just don't have a ready and willing user to justify it, which has been our bar for adding things to the SDK.

There is at least one user in practice: at least one of the shared libraries that ship with the SDK depend on fuchsia.net.name.

 

I also assume that we'd want more than just elevate to "partner", we'd probably want to apply FIDL versioning to them as well, right?

Correct - properly versioning these FIDL libraries is the main motivation here.

 

I think applying the versioning might be beneficial for us to be able to use it to reason about safe API evolution more easily, which we've been doing without tooling help so far. But given zxio/FDIO is compiled with a HEAD SDK, I fear that the incentive quickly becomes to maintain most of the api at `added=HEAD` which is effectively the same as not having it in the SDK, unless I'm missing something.

+Sébastien Marchand is also working on building the SDK shared libraries at specific target API levels to address this issue.  We'll eventually have N copies of libfdio.so in the SDK, one for every supported API level in that SDK.  Those copies will be built against that API level.

Yes, that is in fact one of the main motivations here. We want to be able to build the shared libraries for multiple API levels (and include them in the SDK) and this requires 2 things:

• Availability annotations: If we compile a shared library for API level N then only the only FIDL APIs that exist at API level N will be usable by that library. None of the libraries listed in Hunter's email have availability annotation, so it's currently impossible to build the SDK shared libraries for an API level != HEAD.
• ABI guarantees: we need to make sure that a version of a shared library built for an earlier version of the platform will keep working on a newer system. I.e., the FIDL APIs that this library depends on should still exist on this newer version of the platform.

Moving all the libraries mentioned above to the "partner" SDK category solves these 2 problems. If we don't do this then we'll have to create a new SDK category that has the same constraints as the "partner" one (requires availability annotations, uses golden files, doesn't allow breaking changes...) without exposing them to the SDK. It'll require some additional work to do this and it'll create some additional complexity in our build system, it'd be nice to avoid this if we can.

Note that we'll soon make it mandatory to use availability annotations for all the APIs that are part of the partner SDK (https://bugs.fuchsia.dev/p/fuchsia/issues/detail?id=114251)

[Sébastien Marchand]

Here's how you can check where the dependency to your library is coming from:

- patch in this CL : https://fuchsia-review.git.corp.google.com/c/fuchsia/+/780498

- undo the change to your library (remove the "@available" annotation).

- set 'override_target_api_level = 11` in your build gn args (I recommend building for core.x64)

- build with "fx build build/sdk:sdk_versioned_shared_libraries"

This will tell you what causes a dependency on your library. It's not an exhaustive list, ninja will stop at the first error usually. You can get more granularity by building each shared library listed here one by one: https://fuchsia-review.git.corp.google.com/c/fuchsia/+/780498/6/build/sdk/BUILD.gn#43

It could probably help reduce the number of libraries that should be added to the partner SDK, it's really likely that some dependencies are simply due to some unneeded `include` lines for some targets (we can probably fix some of these issues by using forward declarations in these cases). My approach has been really mechanical, I've looked at which FIDL libraries are part of the build graph of the shared libraries but I haven't checked if these dependencies could be removed.

[Tamir Duberstein]

I think we should add the necessary SDK category (or alternative machinery).

Even if all these APIs were ready for SDK inclusion (as Bruno pointed out, that's not obviously so) we may want this mechanism in the future; in its absence, FIDL authors will be nudged toward SDK inclusion in future cases, which I think is not what we want.

On Sun, Jan 8, 2023 at 10:25 PM Chris Suter <csu...@google.com> wrote:

For fuchsia.fs, no.

[Chris Suter]

On Sat, Jan 7, 2023 at 6:55 AM Sébastien Marchand <sebma...@google.com> wrote:

For fuchsia.fs, no.

[Sébastien Marchand]

On Mon, Jan 9, 2023 at 11:12 AM Tamir Duberstein <tam...@google.com> wrote:

I think we should add the necessary SDK category (or alternative machinery).

Even if all these APIs were ready for SDK inclusion (as Bruno pointed out, that's not obviously so) we may want this mechanism in the future; in its absence, FIDL authors will be nudged toward SDK inclusion in future cases, which I think is not what we want.

Maybe, but just to be clear: these APIs will have exactly the same constraints as the partner APIs in terms of support. E.g. you won't be able to change their signature and we'll have to provide ABI compatibility windows for all these APIs (by enforcing the use of the `legacy=true` FIDL tags). So the main thing that we'll get out of the new SDK category is that these APIs won't be exposed to the SDK users (which might be a good motivation to add this category, I just want this to be clear).

[Hunter Freyer]

On Mon, Jan 9, 2023 at 11:30 AM Sébastien Marchand <sebma...@google.com> wrote:

On Mon, Jan 9, 2023 at 11:12 AM Tamir Duberstein <tam...@google.com> wrote:

I think we should add the necessary SDK category (or alternative machinery).

Even if all these APIs were ready for SDK inclusion (as Bruno pointed out, that's not obviously so) we may want this mechanism in the future; in its absence, FIDL authors will be nudged toward SDK inclusion in future cases, which I think is not what we want.

 

Maybe, but just to be clear: these APIs will have exactly the same constraints as the partner APIs in terms of support. E.g. you won't be able to change their signature and we'll have to provide ABI compatibility windows for all these APIs (by enforcing the use of the `legacy=true` FIDL tags). So the main thing that we'll get out of the new SDK category is that these APIs won't be exposed to the SDK users (which might be a good motivation to add this category, I just want this to be clear).

Yeah, can you elaborate on why including these APIs in the SDK is "not what we want"? Or why an API would be ready for use via an SDK-shipped library, but not ready for direct client use?

Thanks,

Hunter

[Tamir Duberstein]

Bruno outlined the reasons above.

We may want to use the FIDL in a library whose interfaces we support while maintaining the freedom to change the FIDL without worrying about OOT clients.

There are also cases in which the FIDL is quite complex to hold correctly (e.g. fast datagram sockets) so we want to do our best to steer clients away from direct use and toward the libraries we provide.

[Bruno Dal Bo]

 

Maybe, but just to be clear: these APIs will have exactly the same constraints as the partner APIs in terms of support. E.g. you won't be able to change their signature and we'll have to provide ABI compatibility windows for all these APIs (by enforcing the use of the `legacy=true` FIDL tags). So the main thing that we'll get out of the new SDK category is that these APIs won't be exposed to the SDK users (which might be a good motivation to add this category, I just want this to be clear).

Yeah I think the evolvability constraints are a strict positive that we want to have in everything that is supporting SDK.

To echo what tamir said, from the perspective of fuchsia.posix.*, those set of APIs are created explicitly to support the POSIX socket family of APIs, which has its own stability guarantees. We expect OOT users to use sockets exclusively through the POSIX API. The APIs as exist today are a) very hard to hold and b) somewhat optimized for FDIO as a client. We expect, someday, we'll have a socket API that doesn't go through POSIX. It is unclear if that's going to be fuchsia.posix.* or something different entirely. Meanwhile, ensuring this API doesn't accumulate users is an interesting feature for us. And, as I mentioned before, from a purely practical standpoint it's a lot easier to evolve something that I know all the clients for (FDIO at versions N...N-M in this case).

Ffor as long as our engineering contract to evolve APIs in the partner SDK is that we (the Fuchsia team) update the clients out of deprecated APIs, there's a clear benefit to keeping it hidden from direct consumers - it reduces the effort to evolve it. Then, imho, the cost-benefit analysis of having the extra SDK category depends on how long we expect to uphold this contract. Speaking from the Netstack team's perspective, I'd also like for there to exist a very intentional conversation when we get to the first client that wants to use a fuchsia-native socket API, as opposed to risking that happening "organically" through creep on fuchsia.posix.*.

If we can bear the cost of the extra SDK category I'd really like to have it.

On Mon, Jan 9, 2023 at 9:09 AM Tamir Duberstein <tam...@google.com> wrote:

Bruno outlined the reasons above.

We may want to use the FIDL in a library whose interfaces we support while maintaining the freedom to change the FIDL without worrying about OOT clients.

There are also cases in which the FIDL is quite complex to hold correctly (e.g. fast datagram sockets) so we want to do our best to steer clients away from direct use and toward the libraries we provide.

On Mon, Jan 9, 2023 at 11:52 AM Hunter Freyer <hjfr...@google.com> wrote:

On Mon, Jan 9, 2023 at 11:30 AM Sébastien Marchand <sebma...@google.com> wrote:

On Mon, Jan 9, 2023 at 11:12 AM Tamir Duberstein <tam...@google.com> wrote:

I think we should add the necessary SDK category (or alternative machinery).

Even if all these APIs were ready for SDK inclusion (as Bruno pointed out, that's not obviously so) we may want this mechanism in the future; in its absence, FIDL authors will be nudged toward SDK inclusion in future cases, which I think is not what we want.

--

Bruno

[Wez]

How does a client using fuchsia.posix.* directly differ from one using the protocol via FDIO, given that in both cases the component is built against a specific version of that protocol?

Is the argument that with the constraint that the protocol only be used via FDIO, we can more easily audit (and/or compatibility test) older FDIO versions as a proxy for whether changes are compatible with older clients? 

--
You received this message because you are subscribed to the Google Groups "api-council" group.
To unsubscribe from this group and stop receiving emails from it, send an email to api-council...@fuchsia.dev.
To view this discussion on the web visit https://groups.google.com/a/fuchsia.dev/d/msgid/api-council/CACOR3hKU1_wmGF9j4ZmAx26m0R4mOyOVqTReAXt%2BDRwZRW_9EQ%40mail.gmail.com.

[Bruno Dal Bo]

How does a client using fuchsia.posix.* directly differ from one using the protocol via FDIO, given that in both cases the component is built against a specific version of that protocol?

That the Netstack team knows and owns the client code.
 

Is the argument that with the constraint that the protocol only be used via FDIO, we can more easily audit (and/or compatibility test) older FDIO versions as a proxy for whether changes are compatible with older clients? 

Yes that's certainly a big part of it, but I also think the protocol's ergonomics and readiness for SDK inclusion - notably documentation - play an important role here. This protocol was not intended for direct use and I think one that does would have a largely different shape.

--

Bruno

[Sébastien Marchand]

From an EngProd perspective, adding a new SDK category is relatively trivial. I think that the main concern here is that it makes the overall versioning story a little bit more complex because API authors now have one more thing to reason about. It's a small thing, but these small things add up and make the overall system hard to understand.

OTOH, if not creating this category impacts the quality of our partner SDK (e.g. because now SDK users have multiple APIs they can use to achieve the same goals) then we should maybe accept this cost. We'll eventually have a lot more SDK users than SDK contributors, optimizing for the users seems like the right thing to do.

I'm not in the API council so I'll defer the decision to someone else.

[Adam Barth]

Sounds like we should add a new SDK category to address the use case described in this thread.

Adam

[Sébastien Marchand]

SG. I'll start working on that. I'll also update the RFC to mention this new category. Any suggestions for the name? Here's some ideas:

- internal_versioned

- internal_visible

- internal_with_compat

[Adam Barth]

On Mon, Jan 9, 2023 at 8:49 PM Sébastien Marchand <sebma...@google.com> wrote:

SG. I'll start working on that. I'll also update the RFC to mention this new category. Any suggestions for the name? Here's some ideas:

- internal_versioned

- internal_visible

- internal_with_compat

How about "prebuilts" to indicate that it can be used in prebuilts?

[Bruno Dal Bo]

From those 3 I prefer internal_versioned.

How about something like "sdk_support" or "internal_library_support" or some mix of those ideas?

--

Bruno

[Hunter Freyer]

Kk, it seems like everybody can live with the introduction of a new category. If that's not the case, please let me know.

On the name bikeshedding front: I think it ought to include the word "partner", since really this is a modification of the "partner" SDK category. Perhaps "partner_private" or "partner_hidden"? In the future we might want to use the same technique for the "public" SDK, so that version could be oxymoronically named "public_private" or "public_hidden".

Borrowing from C++ conventions there's sometimes "foo.h" and "foo_impl.h"... so, "partner_impl"? 

Thanks,

Hunter

[Sébastien Marchand]

On Mon, Jan 9, 2023 at 4:03 PM Hunter Freyer <hjfr...@google.com> wrote:

Kk, it seems like everybody can live with the introduction of a new category. If that's not the case, please let me know.

On the name bikeshedding front: I think it ought to include the word "partner", since really this is a modification of the "partner" SDK category. Perhaps "partner_private" or "partner_hidden"? In the future we might want to use the same technique for the "public" SDK, so that version could be oxymoronically named "public_private" or "public_hidden".

We could maybe write the category definition first and derive the name from it.

I'd describe this category (here: https://fuchsia.dev/fuchsia-src/contribute/governance/rfcs/0165_sdk_categories#implementation) as: "supported for use within the versioned artifacts that are released in the partner SDK" (or maybe we should say "shared libraries" directly instead of "artifacts"). 

I agree that "partner" should be part of the name. We might have an equivalent category for the public SDK one day. "partner_private" sounds clear enough IMO.

I'm not sure about using the term "prebuilts". "prebuilts" usually refer to prebuilts components that are checked-in in the source tree afaik. These components can't use these APIs directly, but they can use a version of the shared libraries that uses these APIs. "prebuilts" could also refer to the "prebuilt shared libraries released with the SDK", in which case using "prebuilt" is fine, but there's a risk for confusion here and it'd be nice to avoid it I think.

[Jeremy Manson]

BTW, folks, just a note to say that I suspect people writing CTF tests and tooling will end up using this extensively.  We've already been asked about it by some teams as an alternative to publishing their APIs, as I've been encouraging them to include their APIs in the SDK.

It will create some work for us on the CTF team to make sure that the API checks work appropriately.  @Kendal Harland we should track this work closely.

Thanks!

Jeremy

To view this discussion on the web visit https://groups.google.com/a/fuchsia.dev/d/msgid/api-council/CAPONGC04dh6HBy8d7NF9fepOD%2Bwx4o-L4UW7r_mhooY24LWmPw%40mail.gmail.com.

[Sébastien Marchand]

FYI, I'll move all the APIs listed in the original in this thread to the new partner_internal SDK category for now. If some of these should be elevated to "partner" then we'll do this, but that's a separate discussion at this point.

[Sébastien Marchand]

(resharing an update sent to another thread about this)

Quick update: I've landed a CL to introduce the "partner_internal" SDK category and I've prepared a CL to add all the libraries used in our shared libraries to it (I'll send it for review tomorrow).

With this we can build all the shared libraries for API level 11 (via the `sdk_versioned_shared_libraries` GN target and the `override_target_api_level = 11` GN arg). The next step is to setup a set of builders that build this target for all the SDK supported API levels >= 11 (I'm not fixing the past, so we don't worry about anything < 11). We'll run this in CQ and CI to prevent regressions. ( +Anthony Fandrianto will help me with this).

Then, we'll add this as a sub-build of the normal SDK build and we'll make the SDK build recipe merge the versioned shared libraries to the SDK archive.