RFC for Public/Private Dependencies

This RFC introduces the concept of a public/private separation of crate dependencies to aid the ecosystem to explicitly deal with dependencies that themselves become part of the public API of a crate.
This is work in progress.
Rendered

Question: if your crate depends on multiple versions of the same dependency, does the compiled binary just have multiple copies of the same generated code? How does this affect name resolution in the compiler? Or linking? Also, is the binary proportionally larger?

Public dependencies shouldn't just be restricted to public APIs. For instance, most -sys crates can only be used once due to C's linkage rules. Noting libc as a public dependency would have made may have limited the impact of the libc-ocolypse. It would also be convenient to have a notion of a "unique" crate for -sys crates and for #[allocator] crates but I think that feature is orthogonal, and complex enough that it should be left as a future extension.

@bbatha libc does not have the single-version constraint. The libc-pocalypse pain was just the normal issue of crates trying to bridge libc 0.1 and 0.2 APIs.

If dependencies default to private, and you can't export types from private dependencies, then this introduces a compatibility break.

(That said, I'd love to see a change like this.)

@joshtriplett that's why initially it will only be a compiler/cargo publish warning.

I think this is a real problem and this seems like a good solution, but it is a pretty big change. Even though you're starting with a warning, everyone is going to get the warning & be compelled to fix them. This is a huge burst of churn & the drawbacks section should at least note this as a drawback.

Is the crate using a dependency the right place to declare it?
It could be a declaration in the dependency, similar to the link attribute (i.e. a dependency would mark itself as intended to be shared and exist only once [within any subtree of dependencies?]). IHO that would be a significant improvement in usability, because it frees all users of the crate from understanding the concept and marking the crate usage appropriately.
Perhaps it could even be a declaration per type or module? e.g. an Error type may be intended to be re-exported, but other types should not, e.g. pub(globally unique) or pub(no re-export).

Does it have to be explicit at all? Can cargo/rustc notice the situation and emit a warning? (and you could silence the warning with either allow attribute or some cargo.toml flag)

warning: you have two copies of foolib, because bar requires 1.x and baz requires 2.x

I'm assuming duplication of dependencies is generally unexpected and undesirable (because it's not a thing in C, and it bloats executables), so users may want to avoid it even if it doesn't break any APIs.

@pornel I think your proposal misses the point of this one. The point isn't to ensure that a dependency is used only once in the entire dependency graph, but to provide cargo with additional metadata that can then be used to potentially implement things like hinted at in the "Unresolved questions" section, or just improve the error messages that currently happen at version splits.

I'm assuming duplication of dependencies is generally unexpected

No, it's completely reasonable to have that for private dependencies.
You're right that this situation could potentially be detected by the compiler, however IMO this might turn out to be too complex (or slow for certain operations) to implement since now your dependency management (e.g. cargo update) depends on the sourcecode as well, not just the metadata.

Some related discussion: rust-lang/cargo#2064

👍 to the general idea. Some random comments:

I think assuming private dependencies by default is an interesting choice: probably the right one, but not obviously so. I'd be curious to see some measurements on how often deps in practice are private vs public (I don't really know).
I'd like to hear a lot more details about what rules the compiler will use when issuing a warning.
This also seems related to the "private type in public API" rules (which have been a perennial point of controversy). I don't want to join the two topics together per se, but it seems to me that they are related in purpose. The existing rules aim to ensure that private structs are not exposed or usable outside your crate, and the same is true of the external deps.

I don't want to join the two topics together per se, but it seems to me that they are related in purpose.

I'm afraid we might. But maybe this isn't so bad. Comparability guarantees in conjunction wit this can be a clear and clearly-motivated way (even if those are only "guarantees" with warnings-as-errors) to evaluate those rules.

@mark-i-m i missed the question but the RFC changes nothing (so far) about how multiple dependencies functionally work in Rust. You can already end up with multiple versions of the same crate for as long as major versions are involved.

Thanks for writing up this RFC @mitsuhiko! Here's some thoughts I had while reading:

I believe @eternaleye is correct where the fact that a dependency is either public or private needs to be encoded into a registry's index. This is sort of like how the "kind" of dependency (build, dev, etc) is encoded today. This is I believe a very easy change, but is likely worth mentioning.
The RFC explicitly calls out how on a publish you'll get warnings, but this is happening on all rustc compilations, right? (that's what I'd like to happen, at least).
I, like @nikomatsakis, wouldn't mind seeing some more detail on the compiler side of things. Both in terms of what the argument looks like that Cargo is passing but also the way the compiler is detecting these apis.
The error messages here may run a risk of being confusing in some situations. For example let's say we've got A -> B -> C where B has a public dependency on C. This means that A can reexport some of C's types, but if you're a Cargo user then C could be way far down in the compilation graph and totally unknown. Do you think the compiler should go the extra mile and detect that C was exported transitively through B and tell A that B needs to be a public dependency? Put another way, if B is listed as a public dependency of A and then A reexported some of C's types, would the compiler warn/error about that?
Currently there's no mention of crate resolution in Cargo, but I find that to be a crucial aspect of public/private dependencies. Cargo will specifically at least need to start rejecting crate graph resolutions where two versions of a crate are publicly reachable to one another. This in turn would fix almost all of the woes brought up during https://github.com/rust-lang/cargo/issues/2064 because Cargo will deterministically reach a statisfiable and compilable graph.

I think it's also worth mentioning in general some use cases of this RFC to mitigate pain seen in the ecosystem today. Note that this is mostly just an example, I'd encourage discussion of precisely what's happening here to happen elsewhere so this thread can continue to focus solely on this RFC. I consider public/private dependencies to be a big part of the answer to the major version upgrade of dependencies like libc. Once this change is implemented and propagated, I would feel confident about bumping the libc crate to 1.0.
When libc reaches 1.0 then many crates will need to bump their major version as libc is a public dependency. This RFC will allow us to precisely identify what set of crates need to be bumped, transitively! So immediately we'll have precise knowledge of how much of the ecosystem needs to be upgraded. After that Cargo will also reject any attempt to compile a graph which contains both libc 0.2 and libc 1.0 which would fail to compile. This is the crucial part of crate resolution I mentioned in the last bullet above. If Cargo determines that types from libc 0.2 can reach libc 1.0 then it will reject the resolution before we even start running the compiler. This means that error messages are immediate, high quality, and actionable.
Overall public/private dependencies empowers crate authors to release new major versions and:

Precisely know the impact of how many crates need to have a major version bump
Prevent any weird compiler error messages by having two versions of the library reach each other
Allow consumers to know exactly what crates need to be updated so they know where to file bugs and/or focus efforts

Note that public/private dependencies does not reduce the amount of work that needs to be done as part of a major version upgrade, it simply makes the experience of discovering the work before/after the major bump realistic. Similarly it enables authors to have precise knowledge of when they're done upgrading, because it's when Cargo starts to invoke the compiler!

Please remember to keep an eye on compatibility, especially if there are to be changes to the crates.io index.

@alexcrichton

The error messages here may run a risk of being confusing in some situations....

I'm very interested in this bullet but having trouble following it.

@Ericson2314
Oh let me clarify as well (even though it sounds like you've understood now).
Let's say we have three crates, A, B, and C. A depends on B, and B depends on C. We then have a few situations:

A reexports none of B, transitively reexporting none of C. In this case the compiler should issue no warnings.
A reexports some of B. If the compiler is informed that A publicly depends on B, then no warnings are issued. Otherwise, warnings are issued. In other words, this situation would require B to be tagged with public = true in A's Cargo.toml.
A reexports some of C's API. Because A doesn't depend on C this must transitively happen through B. In this example it means that B publicly depends on C. What happens in this case?

If the compiler is informed that A publicly depends on B, does the compiler warn? It seems like a simple implementation in the compiler would warn about this, but would end up having not very good ux.
Would the compiler require that it's informed of A publicly depending on C? This seems like it's difficult for Cargo to do so.

I was basically mostly wondering about the last case. How the compiler makes heads or tails of it and what error message it prints. If A didn't publicly depend on anything then I'd guess that a simple implementation would cause the compiler to warn "A needs to publicly depend on C". That's not quite accurate, though, as A needs to publicly depend on B, but the compiler also has to make headers or tails of that.

@alexcrichton
As to the last case, would it be possible for the compiler to trace where the exported APIs are coming from? Thinking through it in my head, it seems to me that the compiler would have enough information to create a chain of re-exports/exports for each aspect of an API. If the compiler could figure out this chain, it would then be as simple as finding the first crate in the chain that is exporting an API (other than the crate being compiled, that is) and issuing a warning that states that that crate needs to be marked as a public dependency.
I suppose that an issue could arise if two dependencies re-export the same lower API, but in that case marking either, or both, as a public dependency should solve that problem. In fact, the right option may be to make both public dependencies, but I can't say I have thought very hard about this.

@alexcrichton This relates to reexporting vs merely exposing.
On one hand, B's reexports are bonafide part of B's API, so no warning is even needed. A isn't doing anything wrong.
On the other hand, since B effective interface may magically depend on C, it's unsafe B to have anything but an exact-version dependency on C, because otherwise B is unable to uphold its compatibility guarantees even if C does:
Let <: be the crate interface compatibility relation. Let both crates "on their own" preserve comparability: C-1.0 <: C-1.1 and ∀c B-1.0(c) <: V-1.1(c), where Foo(Bar) is Foo instantiated with Bar. Those two alone don't mean B-1.0(C-1.1) <: B-1.1(C-1.0), yet both of those valid instantiation assuming B has a normal C = "^1.0" dependency. A, even without a transitive reexport, can indeed rely on that last being true.
While it is certainly is possible to transitively reexport, absent some typeof operator, it's impossible to transitively expose without reexports, which means A would not be able to rely on things like B-1.0(C-1.1) <: B-1.1(C-1.0) when reexports are not allowed. This is why mere exposing is a far safer thing for crates to do, avoiding this quagmire.
[Note as each std is always tied to a single version of core, there are no potholes here for #1133.]

@alexcrichton @Ericson2314 This is a fascinating issue, thanks for digging into it!
I want to try to give a slightly more concrete version of (what I understand to be) @Ericson2314's concern. Suppose we have:

A depends on:

B 1.0 which publicly depends on C ^1.0
D 1.0, which has no dependencies

When A is initially built, it resolves to B 1.0 and C 1.1.

Because C's APIs are available to A via re-exports in B, A effectively depends on C 1.1 now, even though B only claims to depend on C ^1.0
In particular, the code in A might depend on APIs only available in C 1.1
However, if A is a library, we don't check in any lockfile for it, so this information is lost.

Now we change A to depend on D 1.1, which depends on C =1.0

A fresh copy of A, when built, will now resolve the crate graph to B 1.0, D 1.1, C 1.0
But now A may suddenly fail to compile, because it was implicitly depending on C 1.1 features via B.

BTW, I think that the problem here is that D is not using a "semver-open" constraint (in the sense I spelled out here).
On the other hand, I think that if everyone is using semver-open constraints, given Cargo's greedy resolution strategy (which picks the biggest version possible), it should all work out.
And I think semver-open constraints are by far the most common. It'd be good to analyze how many crates in the ecosystem have non-semver-open constraints, and why.

It'd be good to analyze how many crates in the ecosystem have non-semver-open constraints, and why.

Clap explicitly recommends a tilde constraint as a way of dealing with minimum-Rust compatibility.

Thanks for clarifying @aturon, but I think I may be failing to see how this is relevant to this RFC? I think it's highly relevant to #1890 in terms of defining what's a breaking change and decision on this RFC will affect the outcome of that RFC, but I'm not sure how it's directly related to public/private dependencies.
The semantics of crate resolution are not specified in this RFC so I'm also not sure if we're talking about a concrete proposal or not.
My current understanding of the relevance of this issue looks like:

Let's say that D privately depends on C. Cargo could allow this resolution or it could also not. The types from C 1.0 cannot reach C 1.1 because D does not leak types from C 1.0. Technically this can be allowed, but Cargo could also arbitrarily say that within one semver track (e.g. 1..) only one version is allowed (like it does today).
Let's say that D publicly depends on C. It seems in all possible versions of this RFC Cargo should reject such a resolution. It's possible for types from C 1.1 to reach C 1.0, an incompatibility, so Cargo would reject the resolution.

The only question here to me at least seems to be whether Cargo maximally allows duplication if it's technically feasible.

There are some packages which can't really follow semver. For example, the gitlab crate encodes the version of Gitlab that is supported (Gitlab does not provide stable hook entities[1] and can break on any given release). Rather than basically having a major version bump for any minor bump in Gitlab itself, instead, we just do 0.$gitlab.$patch and recommend an exact dependency on it since we can find "oops, that's actually nullable" at any time. What do we do for projects whose API is at the mercy of some external service that may not adhere to semver?
[1]I have issues open requesting a more stable API for hooks, but they're not prioritized.

@aturon it's not solved by assuming only semver-open. If B 1.0 depends on C >= 1, < 3 and D 1.1 depends on C ^1, A might accidentally depend on C ^2 and then fail with the D upgrade.

Forgive me if I missing something crucial...
@Ericson2314

Those two alone don't mean B-1.0(C-1.1) <: B-1.1(C-1.0), yet both of those are valid instantiation assuming B has a normal C = "^1.0" dependency

I got lost here. Assuming C-1.0 <: C-1.1 and ∀c : B-1.0(c) <: V-1.1(c), is it not true that ∀ c,d : (c <: d) => (B-1.0(c) <: V-1.1(d))? Can you give an example of how this could be false?
@aturon I am not sure I followed your example:

When A is initially built, it resolves to B 1.0 and C 1.1.

Because C's APIs are available to A via re-exports in B, A effectively depends on C 1.1 now, even though B only claims to depend on C ^1.0
In particular, the code in A might depend on APIs only available in C 1.1
However, if A is a library, we don't check in any lockfile for it, so this information is lost.

I am guessing that this is the same confusion as for my questions to @Ericson2314. If A depends on features of C 1.1 reexported through B, then B must depend on C 1.1 (not just ^1.0). Otherwise, B could not re-export them and compile, right? Is it possible for C to change in a way that B can still compile, but A can't? Am I missing something?

@Ericson2314 Argh, good point.
@alexcrichton This all potentially bears on your original question about transitive public dependencies. The point is that when crate A depends on crate B which publicly depends on crate C, that induces an implicit dependency of A on a particular minimal version of C (whatever it first resolves to). Since that information isn't explicit in the toml, future situations can invalidate it and produce compiler errors, rather than dependency resolution errors.
Let me sketch one -- probably too extreme -- way that it could interact with this RFC. We could require that, if you depend on crate B that has public dependency C, you must add a dependency for C yourself, stating the version requirements you intend. (And that dependency must be public if your dependency on B is).
There's a certain appeal to that: I think of public dependencies as trying to give a clear heads up for when a transitive dependency is not encapsulated, and thus your clients need to know/care about it. This would be one way to surface precisely that information. Subsequent conflicts around the version of a public dependency would thus be less surprising.
A counterpoint to all of the above is that, when you write a ^1.1 dependency, you might still get version 1.3 instead, and could easily end up using features from 1.3 that aren't in 1.1 -- another "implicit" dependency that could be broken in precisely the same way. (This makes me wonder whether crate resolution should actually prefer the largest patch release on the smallest major/minor version compatible with constraints, to help people depend on only what they state.)

@mark-i-m in your first part you are indeed orrect, the compatiblity relation is always so preserved. But the issue is a newer version of B with an older version of C.
As to your second point the issue is that if B, due to the globalness of public dep solving, sees a newer C and thus richer interface than it asked for, it is not prevented from using those extra bells and wistles.

I'm a bit uncomfortable with any fix relying on Cargo's preferences during solving, as those can be circumnavigated by the lockfile—in will in practice with users skeptical of bumping deps willy-nilly. In fact, those users would probably be most annoyed because they are trying to be cautious bumping or downgrading deps individually precisely to avoid these problems.
I think @mark-i-m's questions hint at what a more robust solution could be: constraining deps' interfaces to the intersection of the versions requested. [Optional automatic features, or something like "match cfg" from the portability lint RFC, could be used as an escape hatch.]

@aturon

Let me sketch one -- probably too extreme -- way that it could interact with this RFC. We could require that, if you depend on crate B that has public dependency C, you must add a dependency for C yourself, stating the version requirements you intend. (And that dependency must be public if your dependency on B is).

Extreme indeed! This is what I meant above by "the semantics of crate resolution are not specified in this RFC". I'm not really sure the best way to continue this conversation. I fear that much of what I'm thinking about is merely implicit in my head and not written down. I consider a major facet of public/private resolution to be how it affects crate version resolution in Cargo, and my thinking about how it affects resolution is that this situation is basically moot wrt to how resolution works. Again, very relevant for what a breaking change is, but not relevant for how Cargo works.
My rule of thumb has been that if types from a crate can possibly reach a different version of the same crate then Cargo will reject the resolution graph. "Reach" here is "somehow I can pass the types along via APIs", and that's done through public dependency edges typically. With that mindset I consider my original question more about the compiler implementation. If implemented naively I fear it will make the lint too unergonomic to use and have little effect in practice.

Hm ok, so I believe one major feature of this RFC is that it has the power to affect crate graph resolution (e.g. generation of a lock file) in Cargo. I'd like to try defining what I precisely mean by that as I think it can help frame some of the conversation here.
As a quick recap, graph resolution in Cargo is where it takes a Cargo.toml and then generates a Cargo.lock. The Cargo.lock file is a complete and deterministic resolution of the dependency graph which satisfies all the listed constraints. Cargo has a resolver module that performs this operation. Constraint resolution is an NP... er... something problem. (NP-hard? NP-complete?) It suffices to say that it's a hard problem that can involve SAT solvers and everything. As a result Cargo basically implements a depth-first search in an attempt to find a resolution of crates that fits. Cargo returns the first graph that satisfies all constraints or waits to verify all possible graphs are impossible before returning an error.
So basically Cargo is a pile of heuristics today to quickly find a graph that satisfies all the dependency constraints. Cargo's only (I think) constraint to reject a dependency graph today is that with a semver compatible track of a package (e.g. the 0.2.* series or the 1.. series) there exists only one version of a crate. For example it's disallowed to have log 0.3.1 and 0.3.2 in the same graph.
Given that information, public/private dependencies gives us an opportunity to add more contraints to what a valid resolution graph looks like. In simplified terms I believe the constraint we wish to add is "types from a crate cannot reach a different version of the same crate." For example let's say I have two dependencies, iron and hyper. If I pass a Hyper client into iron, I would like that to compile, but it requies that iron uses the same version of hyper as I do. Because types from one library could reach another library, we have to make sure that shared dependencies all have the same version.
I believe a formal description of this desired end state of affairs looks like this:

First, a resolution graph has a bunch of nodes. These nodes are "package ids" which are a triple of (name, source, version). Basically this means that different versions of the same crate are different nodes, and different sources of the same name (e.g. git and crates.io) are also different nodes.
There are directed edges between nodes. A directed edge represents a dependency. For example if A depends on B then there's a directed edge from A to B.
With public/private dependencies, we can now say that every edge is either tagged with public or private.
This means that we can have a collection of subgraphs purely connected by public dependency edges. I think the directionality of the public dependency edges within the subgraph don't matter (but we can come back to that). Each of these subgraphs basically represents an "ecosystem" of crates publicly depending on each other, and my thinking at least is that these subgraphs are "pools of public types" where if you have access to the subgraph you have access to all types within that pool of types.
Now first we can place a constraint that each of these "publicly connected subgraphs" are required to have exactly one version of all crates internally. For example each subgraph can only have one version of Hyper.
Finally, we can consider all pairs of edges coming out of each node in the resolution graph. If the two edges point to two distinct publicly connected subgraphs from above and those subgraphs contain two different versions of the same crate, we consider that an error. This basically means that if you privately depend on Hyper 0.3 and Hyper 0.4, that's an error.

Let's take a look at some scenarios from the main Cargo issue for these rules and see how they play out.
First on the bug is a situation:

r2d2_postgres version 0.9.3 depends on postgres version 0.10, while version 0.9.2 and lower depend on postgres version 0.9. Cargo would ideally pick r2d2_postgres version 0.9.2 and postgres version 0.9.6, but it instead picks r2d2_postgres version 0.9.3, which pulls in postgres version 0.10, as well as postgres version 0.9.6

Here we can say that r2d2_postgres publicly depends on postgres. We also have two dependencies for our crate, r2d2_postgres = "0.9" and postgres = "0.9". Let's first consider a graph where r2d2_postgres is selected as 0.9.3 which depends on postgres 0.10. This means we'll have two disconnected publicly connected subgraphs, one for postgres 0.9 and one for r2d2_postgres 0.9.3. Our crate, however, depends on both, so by the last rule above this graph would be rejected by Cargo.
If we instead, however, selected r2d2_postgres 0.9.2 then we'll share the postgres 0.9 dependency, creating only one publicly connected subgraph with only one version for each dependency. This will be allowed by Cargo.
Here this is basically showing that given the Cargo.toml and dependency situation Cargo will eventually reach a valid dependency graph for the underlying packages. (making this fast is a separate issue...)
Before I wrap this up I think it's worth taking a look at one more situation. A little bit farther down the thread we see an example with libc, but more specifically depending on a few major versions that are known to all be compatible.
Here let's say we create a crate that depend on both foo and bar packages in these comments. First let's consider the case where foo privately depends on libc. If both crates get libc 0.1 then that's a valid crate graph as there's no duplicate anywhere. If, however foo gets libc 0.2 and bar gets libc 0.1 that's also a valid crate graph! The publicly connected subgraph of bar does not include libc 0.2, so types from libc 0.2 cannot reach libc 0.1.
Now let's instead consider the case where foo publicly depends on libc. We'll get into the same situation as postgres above where selecting 0.1 for both libc version is totally fine but when we select 0.2 the public subgraph with bar includes libc 0.2, so our crate depends on two subgraphs, bar with libc 0.2 and then libc 0.1 itself. Those contain two versions of libc, and as a result this is disallowed.

Ok so there's basically two things I've listed above, a "gut feeling" for how I feel resolution should change, as well as a more formal description for what I think that means and some examples for how I think that plays out in practice.
@aturon or @Ericson2314 do you still feel that the situation you're outlining is still relevant? In terms of public/private dependencies?
Are there other questions about this in terms of mechanics or maybe a wrong definition? Feedback would be greatly appreciated!

@Ericson2314

As to your second point the issue is that if B, due to the globalness of public dep solving, sees a newer C and thus richer interface than it asked for, it is not prevented from using those extra bells and whistles.

constraining deps' interfaces to the intersection of the versions requested.

Ah, got it :)
I guess I never really thought about the fact that Cargo doesn't do this...

Feel free to tell me if this is off-topic, but the main problem seems to be the lack of a way to enforce that constraint. i.e. Cargo is aware of versions, but the code itself is unaware of which version it is IIUC.
So some ideas I had (in order of increasing invasiveness):

Cargo always chooses the smallest version number it can. This seems like an awful solution, since it is directly contrary to what happens now, I think.

Make it conventional for people to document modules/functions with their amount of stability. In other words, "very stable" means feature is unlikely to change across major versions, "stable" means feature is unlikely to change across minor versions, and "unstable" means it could change arbitrarily. Then, we make it conventional to use ^ only if you are importing all "very stable" things, ~ if at least one "stable" or "unstable".

Idea (2) but with the addition of rust attributes #[very_stable], #[stable], and we already have #[unstable]... Thus, the compiler or cargo could warn/error...

@alexcrichton Sorry, I'm missing where in those posts you argue why our scenario is not relevant. Are the two resolutions @aturon mentioned not possible and valid? In them only a single version of each package is used, so it is fine to assume all edges mentioned are public. The issue is going from one resolution to the other---assume we threw out the lockfile or something.
Also a few issues I think exist in your formalization. FWIW I agree with you on the fundamental principle; I would rephrase it as "no code can observe multiple versions of the same crate":

We cannot just consider public edges for our unique version subgraphs. A crate's transitive public deps may also coincide with (immediate) private deps, and those those versions must be unique too. Furthermore, the public deps of private deps also matter, as private deps are free to expose those. The actual relation showing the visible crates from a node in the graph is (Priv ∪ Id) · IPub*), where Priv is the private dependency relation/sub-graph, Pub is its public counterpart, Id is the identity relation, and * is the transitive-reflex closure operator.
(That relation was taken from https://github.com/Ericson2314/ghc-proposals/blob/private-deps/proposals/0000-private-dependencies.rst which was my WIP attempt at a similar RFC for Cabal. Haskell's allowance of orphan instances etc makes modularity and thus safe private deps harder to come by; the RFC proposes private deps not public because with Cabal today actually public is the only option.)

Some flexibility is gained by not ignoring the directionality of the edges. Consider a "W" (sub-)dependency graph where B publicly depends on A-1 and C, and D depends on A-2 and C. (For a single root, assuming something else, the root, has private deps on B and D.) Now no crate can observe both versions of A, so the plan is fine in principle, but according to your rules the {A-1,B,C,D,A-2} "W" is a connected subgraph and thus the plan is invalid.

@mark-i-m By limiting the interface I don't mean changing how crates are resolved, but actually downloading the relevant source code to properly filter the interface exposed (through intersection) while keeping the same underlying implementation as today. This is the only way to achieve my goal because the interface filtering is per edge not per node -- the same compiled crate exposes different subsets of itself to different downstream consumers. [The distinction between this and just duplicating the crate in the graph is import wrt global resources, like static muts and and C libraries which can only be linked once.]

@Ericson2314

Sorry, I'm missing where in those posts you argue why our scenario is not relevant.

Oh sure yeah let me clarify. The major takeaway I feel is that the addition of public/private dependencies should have an impact on crate graph resolution, concretely on empowering Cargo to reject otherwise "valid" crate graphs today. The rules listed above are a way for Cargo to understand public/private dependencies and reject candidate resolution graphs.
This scenario, though, about upgrading dependencies seems unrelated to this aspect. You're right in that all incarnations are valid graphs (as there's only one version of everything). The key takeaway though is that, though, it's a valid graph and Cargo can't do much else but to forge ahead. Articulated by @aturon this is super important for understand what is a breaking change, but I see it as not relevant to the impact of public/private dependencies on Cargo.
So to be clear, the situation you're describe means that adding an upper bound on a dependency is a breaking change. Do you have text in mind though you envision being added to this RFC? I may be mistaken in thinking that the "semver compatibility" story is relatively orthogonal to the definition of public/private deps!

We cannot just consider public edges for our unique version subgraphs.

Oh I think this is what I attempted to capture in my last bullet above, but probably didn't do a great job in doing so.  This is similar to the scenario with libc I outlined above though, where libc is a transitive public dep of foo (through bar) but also an immediate private dependency of foo. As a result there's a constraint that they must be the same version.

Some flexibility is gained by not ignoring the directionality of the edges.

Indeed yeah! I was having a tough time figuring out how to reason about this and figure it'd be better if I got something written down first :).
I wonder if it might be better to formalize this with semantics along the lines of tracing how types can flow through a dependency graph. Basically just define what it means for a type to flow from one crate to another, and then define that if a type can flow to two versions of a crate then it's considered an invalid resolution graph.

The situation is supposed to illustrate that reexporting (as oppose to merely exposing) public deps would cause problems. It's not a new issue, but I think it's still important to consider because it shows the limits of this proposal on its own. Likewise we want to consider this and something else like my interface filtering thing, we'd want to show how they work together to prevent the errors we care about.
And to be fair, I think it's quite feasible to enforce compatibility up to type errors modulo macros/annotation. I.e. enforce all cargo files are writen correctly such that a valid build plan will never result in a failed build due to type error. That's the goal by which I want to judge these sorts of proposals.

Oh I think this is what I attempted to capture in my last bullet above.

Sorry, I missed that and it does indeed patch things up.

I wonder if it might be better...

Right so that's indeed basically how I got that relation I copied from Cabal RFC. This "flow" of types I think is just visibility: a type is visible if it can be brought into scope [Note]. Data can flow all over the place (in general), but types are visible only along paths of dependency edges (respecting the  direction of the edges).
Obviously crates can use their own items, so the visibility relation should be reflexive. Secondly a crate is free to use everything in it's immediate dependencies, so the visibility relation should include both the private and public dependency relations. Finally, since defintions from public dependencies can be reexported, and we're assuming the maximal visibility, all of our deps' public deps, and their public deps, etc. The relation I gave is the minimum that follows these constraints.
[Note]: It's possible to export an item but not every identifier in it's type, e.g. if the unexported type came from a public deps, but we're formatting a worst-case visibility relation where stuff is maximally visibile so it suffices to assume the type of each item is as exported.

By limiting the interface I don't mean changing how crates are resolved, but actually downloading the relevant source code to properly filter the interface exposed (through intersection) while keeping the same underlying implementation as today

Ah, I see what you mean now... It makes me nervous, though. It seems like this sort of limiting of interfaces could lead to all sorts of hard-to-fix and hard-to-understand errors where you include a new crate and now cargo/rustc is complaining about some completely unrelated type/crate because they happen to have some dependency issues. But maybe this is an issue with almost any solution to these problems?

The Cargo team is planning to discuss this RFC in our meeting today, but I wanted to jot down a few notes after catching up on the thread.
First, thanks @alexcrichton for writing out your thoughts on the impact on graph resolution! I also gave it some thought, and came to the same conclusion as @Ericson2314.
Here's how I would phrase the constraint (I'm just spelling out what @Ericson2314 said here). A graph is "public dep incoherent" if:

There are packages A, B, and C in the graph, and A and B are different versions of the same crate
There exists a directed path from C to A, possibly starting with one private edge, followed by any number of public edges
There exists a directed path from C to B, possibly starting with one private edge, followed by any number of public edges

In other words, C has multiple dependencies that can talk about a crate publicly, but that depend on different versions of that crate, and hence won't be able to talk to each other. We want to rule this out.
@Ericson2314 put this more mathematically as:

The actual relation showing the visible crates from a node in the graph is (Priv ∪ Id) · Pub*), where Priv is the private dependency relation/sub-graph, Pub is its public counterpart, Id is the identity relation, and * is the transitive-reflex closure operator.

With that formulation, the constraint is that you cannot have more than one version of a crate visible to another crate in the graph.
@alexcrichton, does this formulation make sense to you?

So to be clear, the situation you're describe means that adding an upper bound on a dependency is a breaking change.

I think it's worse than that -- depending on what you mean by upper bound -- because of the example @Ericson2314 gave earlier:

If B 1.0 depends on C >= 1, < 3 and D 1.1 depends on C ^1, A might accidentally depend on C ^2 and then fail with the D upgrade.

Put differently, our normal semver constraints already impose an upper bound (the next major version). That's fine as long as everybody is using semver-based constraints within the same major version. But as soon as you allow spanning multiple major versions, it breaks. Moreover, it would be bad to call that a breaking change, since usually the motivation for such spanning is greater compatibility.
I would be interested to dig a bit more into a requirement that all transitive public dependencies must be listed as direct public dependencies, which surfaces this information. Again, for my mental model, part of the point of public dependencies is to make more clear the "effective API surface" and its versioning at the top level of each a crate. From that perspective, forcing a "flattening" of public dependencies makes good sense; it makes explicit, for every crate, exactly what that API surface is.

@alexcrichton, does this formulation make sense to you?

Yes. There's other fiddly bits like if the two edges out of C (in your example) are a build and a normal dep then that's allowed to duplicate versions (you can't take types from a build dep and give them to a runtime dep). In general though the general "two versions of types cannot reach each other" still stands, it's just further refinement of that.

But as soon as you allow spanning multiple major versions, it breaks.

I think I'm still very confused by the reason for bringing this up. I don't disagree with anything you've said, but continue to feel that anything having to do with upgrading dependencies is largely irrelevant to the RFC here? Public private dependencies will allow us to reject otherwise valid resolution graphs, but talking about breaking changes seems like it's in more appropriate for "what's a breaking change" rfc vs this.
Put another way, what is this discussion supposed to lead to? Is there any change to this RFC that would happen? Do we want to just abandon public/private entirely if we feel that it's too onerous in breaking changes? Is there a middle ground?

I would be interested to dig a bit more into a requirement that all transitive public dependencies must be listed as direct public dependencies, which surfaces this information.

I agree that theoretically this sounds nice, but I can't imagine this ever being practical. The list of dependencies in the Servo crate, for example, would be insanely long and have a huge amount of duplication with all the dependencies.
We don't require you to extern crate transitive dependencies even though the compiler is going behind your back and loading more crates?

Put another way, what is this discussion supposed to lead to? Is there any change to this RFC that would happen? Do we want to just abandon public/private entirely if we feel that it's too onerous in breaking changes? Is there a middle ground?

No, of course we don't want to abandon it. But with virtually every feature we add to the language, we try to consider the impact on evolution, and this feature is no exception. Oftentimes we can find a tweaked design that achieves the original goals while being more robust for evolution.
As to the potential interaction, something like a requirement to list transitive public deps would be a change to this RFC.
Put another way, the question is whether we want to change the definition of, or requirements around, public dependencies in order to provide a smoother evolution story.

I agree that theoretically this sounds nice, but I can't imagine this ever being practical. The list of dependencies in the Servo crate, for example, would be insanely long and have a huge amount of duplication with all the dependencies.

How sure are you about this? When I first brought up this suggestion, I labeled it a "extreme" since I had the same expectation, but remember that we're talking here about inherited public dependency chains. I honestly am not sure how many such chains exist at any given level in a project like Servo. It's possible that we could leave this as an unresolved question, build the tooling that we need anyway, and then use that to gauge what the impact would be.
There may also be some other route to cope with this issue that we haven't thought of yet, I'm not sure.

We don't require you to extern crate transitive dependencies even though the compiler is going behind your back and loading more crates?

I find this kind of argument pretty frustrating, tbh, because it's brushing aside important nuance. As I said in the original comment:

Again, for my mental model, part of the point of public dependencies is to make more clear the "effective API surface" and its versioning at the top level of each a crate.

The point is that there's a crucial distinction between your public and private dependencies in terms of the impact on your public API surface. The analogy around extern crate is akin to saying you have to list all of your transitive deps, which is ignoring the basic point I'm trying to make: listing your public deps explicitly in the first place is partly about a signal to what your clients are buying into. If we don't list transitive public deps, then a thorough client needs to do that transitive walk through all public deps to understand the constraints they're about to put on themselves. (It's a large "reasoning footprint").
I agree that this idea may be impractical, and maybe we can find other routes that work better, but to me these questions are very much relevant to the basic design, mental model, and mechanics of what public dependencies signify.

One very concrete issue with this RFC is making private the default. I'm wary because public is the strictly safer one. It's a very subtle thing, and I rather require one to be explicit either way.
Crates.io would also hereafter ban uploads that aren't explicit on the dependency type.
Unlabeled dependencies would be legacy-only, and Cargo would allow either treating them as public (the safe choice) or private (dangerous, but won't invalidate existing build plans).

@Ericson2314

One very concrete issue with this RFC is making private the default.
If I understand everything correctly, dependencies are currently effectively private. Would changing to a public-by-default model be a breaking change? I feel like it would for any crate that has dependencies that share sub-dependencies that really are private and happen to be on different versions.

If I understand everything correctly, dependencies are currently effectively private.

Yes, but unsoundly so. When confronted with either making a breaking change or letting unsoundness wallow, I rather expose a knob to the user to pick their poison.

I agree that public by default is the better choice in terms of soundness, I just worry about how much that "change" will break. I guess a cargobomb type thing that rejects all compilations that use multiple versions of the same crate could test the effect this would have.

Yes, but unsoundly so. When confronted with either making a breaking change or letting unsoundness wallow, I rather expose a knob to the user to pick their poison.

We tend to use the word "unsound" to mean it can violate one of the core guarantees of the language (e.g. memory safety, data race avoidance, impl coherence); as far as I know this isn't the case here (but I could believe it, so let me know if I'm wrong). All that can happen is users using crates which all have "correct" Cargo.tomls can have build plans for which this is no genuinely correct resolution.
It'd be helpful to avoid using 'unsound' when talking about Rust except in this narrow way. Since I'm currently trying to get up to speed on what's at stake in this RFC, I'm acutely aware of the importance of clear communication here. :-)

The Cargo team discussed this RFC during our meeting today, and I wanted to write up some notes that came out of it.

First, regarding the precise impact on resolution, we agreed that the constraints imposed should be along the lines mentioned previously; doesn't seem like there's a lot more RFC work on that topic, other than integrating it into the text.

We extensively discussed some of the examples raised in earlier comments, where there can be "invisible" dependencies that don't cause resolution failures but can create compilation errors as APIs evolve.
There were a couple of key observations here:

All of these examples rely on a common ingredient: a crate somewhere using an API that only is available in a newer version of a crate than the version listed in Cargo.toml.

We believe that this is a very common situation today; usually, people write the initial version of a dependency in their Cargo.toml, but never bother to update it as they take advantage of new features in newer versions. This works out okay because (1) Cargo will generally use the largest version it can find, compatible with constraints, and (2) upper bounds on constraints (at least within a particular minor version) are relatively rare.
That means, in particular, that Cargo.toml is not a fully accurate picture of version dependency information; in general it's a lower bound at best.
We could try to validate this information, for example on crate publication, by attempting to resolve the graph while picking the smallest versions compatible with constraints. But given minor version coalescing, even that's not enough to ensure that each individual Cargo.toml is accurate. It seems likely that this property just cannot be fully mechanically checked.

Public dependencies exacerbate the above problem, because you can end up relying on features of a "new API" from a crate you didn't even know you depended on!

We care about this issue particularly for examples involving dependency constraints spanning multiple major versions, which we expect to be common when coping with things like a new version of libc being introduced.

For this kind of example, all that's really needed is getting at least the major version of a dependency correct, which seems much easier.
In particular, @alexcrichton pointed out that if we did require you to list all of you transitive public dependencies, that would provide a reasonable check on the major versions you were using.

Given all of the above, though, there does not appear to be a "perfect", mechanically-checked solution that would always ensure that you can upgrade to a new minor version of a dependency without some possibility of breakage -- even if everyone is following semver. That makes these questions mostly ones of practical mitigation, rather than an "all or nothing" proposition.
We didn't reach any firm conclusion on the right direction here, though.

@matklad raised questions around whether we should be enforcing the graph constraints for public dependencies at all -- a point he already elaborated on.
I think it'd be a very good step to go back and lay out the precise costs/benefits for some of these options. In particular, what exactly do we gain by making this a hard error, rather than using it to steer resolution and provide more informative compiler errors?

I've raised a Cargo issue about accidentally depending on new api from minor version: https://github.com/rust-lang/cargo/issues/4100 (it seems to be mostlynorthogonal to this RFC).

We could try to validate this information, for example on crate publication, by attempting to resolve the graph while picking the smallest versions compatible with constraints. But given minor version coalescing, even that's not enough to ensure that each individual Cargo.toml is accurate. It seems likely that this property just cannot be fully mechanically checked.

I take this back. The key insight is that, while the Cargo.toml of any single crate may not directly tell the whole story, there should be a "canonical minimal resolution" which tells you the actual minimal versions required by a crate. (We could in principle also force a crate to raise the minimum version of a dependency if a higher version is implied due to transitive dependencies).

I hope there will be time for me to write up in sufficient detail why I do think there is a "'perfect, mechanically-checked solution".

@rfcbot fcp merge

Ah, there was a month but I was too busy with other things to write it up. Two concrete concerns with the proposal as written (that I'm fine with just resolving during stabilization):

Private default: I'd prefer no default forcing explicit either way, and warning but not error of existing lock files. New solver plans could have an "unsafe keep going" or similar.

#[allow(external_private_dependency)]: Makes doing what is certainly a bad thing too easy.

Turns out there are a number of aspects the cargo team has been discussing that aren't currently reflected in the RFC, so I was perhaps a bit premature. We do want to get this moving forward, though!

@rfcbot concern resolution
As I mentioned above there's currently no discussion about crate graph resolution in this RFC. We discussed this quite a bit amongst the Cargo subteam and the conclusion (so far) is that resolution graphs where types of one version of a crate can reach another version will be considered invalid. This will both (a) guide Cargo's crate graph resolution and (b) provide an error message in this situation to give some feedback to the user if it's actually an un-resolvable graph.
We've also discussed this being a warning but @wycats and @withoutboats made a convincing argument at the recent work week that this should be a hard error instead fo a warning.

@rfcbot concern publish-validate-lowest
In our discussions amongst the Cargo subteam we talked a lot about the "correctness" of a Cargo.toml. We reflected quite a bit on how today's Cargo.toml are likely incorrect specifically with respect to the version ranges depended on. For example it's common that although version ranges say "semver compatible but at least x.y.z" the "x.y.z" number is likely too low. Over time you may accidentally pull in new features of a crate without bumping "x.y.z" in the manifest.
This in turn has ramifications for public/private dependencies which relies on the correctness of the manifest in that if a public dependency isn't marked public it can lead to unusual compiler errors or not actually help guide Cargo to a successful resolution graph.
We discussed that a concrete action item here was to change the behavior of cargo publish to test against the lowest possible resolution graph rather than the highest (what Cargo greedily selects today). This will help verify lower bounds are correct.
I... unfortunately forget how this concretely ties back in to the story for public/private dependencies. @rust-lang/cargo someone else mind filling in the gap here?

We discussed that a concrete action item here was to change the behavior of cargo publish to test against the lowest possible resolution graph rather than the highest (what Cargo greedily selects today). This will help verify lower bounds are correct.

That would be helpful!  Although it's worth noting that the lowest resolution graph won't necessarily be the lower bound -- e.g. foo->(bar, baz-1.0), bar->baz-1.1, foo gets a baz-1.1 upgrade for free.

the lowest resolution graph won't necessarily be the lower bound -- e.g. foo->(bar, baz-1.0), bar->baz-1.1, foo gets a baz-1.1 upgrade for free.

If baz is a public dependency of bar (transitively) then I wonder if it would make sense to have that produce a warning during resolution.

@rfcbot reviewed
So far, the bulk of discussion revolved around the Cargo's side of things, but rustc's one seems equally important. Do we understand how compiler checking should work here (second bullet from https://github.com/rust-lang/rfcs/pull/1977#issuecomment-296168834)?

We've also discussed this being a warning but @wycats and @withoutboats made a convincing argument at the recent work week that this should be a hard error instead fo a warning.

@alexcrichton could you summarize the argument?

@matklad AFAIK no, no further discussion has happened about the compiler implementation. It's so far just 'assumed possible', although I agree that I, too, am wary of such a claim.
I also figured I'd defer to @withoutboats's explanation for errors!

@cuviper Indeed! The solution is to upcast 1.1 to have 1.0's interface just for type-checking bar. Iff semver is obeyed, this upcast is valid.

@alexcrichton

I... unfortunately forget how this concretely ties back in to the story for public/private dependencies.

So say A depends on B and C. Originally B::foo and C::foo's return types are both impl Bar, but then later the two crates use concrete Baz from crate D (a public dependency, necessarily). If A still supports the older B and C, it must never need to unify those two return types.

I also figured I'd defer to @withoutboats's explanation for errors!

We made an analogy to the orphan rules. There are plenty of cases where the orphan rules are overly conservative and this code would work fine, but then later on you or someone else would be in a lot of trouble because something out of your control has changed. Same thing here.
In particular, it seems like it becomes possible for an otherwise non-breaking change to be breaking because it exposes additional types from a dependency; if so, this would be difficult to manage correctly. I'm having difficulty coming up with a concrete example though.
There's also a "social engineering" aspect to making it an error - it encourages small & targeted crates over sprawling ones.
Lastly, if the error ends up being too restrictive, it can be reduced to a warning later, but the contrary can't be done.

I'm for the solution and starting with an error, but I'm unclear on this part:

Lastly, if the error ends up being too restrictive, it can be reduced to a warning later, but the contrary can't be done.

Isn't the current suggestion to go from no error to an error? If so, what prevents going from a warning to an error?

@shepmaster You have to opt into the error by marking your dependency public. (Possibly there will be a warning encouraging you to do this.) But upgrading a warning to an error would not involve an opt-in.

question for us to think about: if we had cargo feature gates, could we use them for this feature or does that not make sense?

@carols10cents I think that makes sense. I'd like to see all cargo features not instant-stabilized.

How would people feel about this syntax for public dependencies, instead of the one in the RFC:
[dependencies.public]
foo = "1.0.0"
bar = "1.2.0"

[dependencies.public.baz]
path = "./baz"

So they're a separate object in dependencies, rather than each having the attribute. Seems easier to use.

@withoutboats So what would this default to:
[dependencies]
foo = "1.0.0"

Would that mean foo is public or private? I think it ought to mean that it's public.

@mark-i-m There was discussion previously of using a feature gate: either all dependencies are marked as either public or private or you're not allowed to specify. For compatibility, IIRC, everything not marked would be considered public.

@mark-i-m @mathstuf The position the cargo team has taken is that unmarked dependencies are private, because that is backwards compatible.

@withoutboats one downside to your alternative is that it's not backwards compatible in the sense that projects which want to be compatible with both pre-public-dependencies Cargo and post-public-dependencies Cargo can't do that. Basically in Cargo today [dependencies.public] is an error.
An updside of public = true is that Cargo would ignore the key today, so projects could immediately start picking up the feature without worrying about upping the rust version requirement

@alexcrichton

Basically in Cargo today [dependencies.public] is an error.

Yes and no? There's currently no crate on crates.io with the name public, but is it a banned name? Might it be used in a private registry? Could someone be using it with a path dependency?
I'm not fond of how this overloads the dependencies map; this is actually one of the things @Ericson2314 , @joshtriplett , and I discussed when we originally brainstormed the Cargo schema version RFC (and part of why we did so).

What about [public.dependencies] or even [public-dependencies]?

@withoutboats That would avoid the problems I described, yes.

To me the significant feature of this difference is avoiding individual annotations, which are especially problematic because they require converting the dependency into an object when it could be a version string today.

@eternaleye oh that's true yeah, I think Cargo would ignore sub-keys so if crates were called version, path, or git they'd be in trouble, but otherwise it may work out.
To put it more succinctly, though, here's an example. This Cargo toml is interepted as Cargo today has having a dependency on both foo and bar
[dependencies]
foo = "0.1"
bar = { version = "0.1", public = true }

Whereas today Cargo interprets this manifest with a dependency on the crate called public and a crate called foo:
[dependencies]
foo = "0.1"

[dependencies.public]
bar = "0.1"

Similarly, Cargo today only understand this manifest as having a dependency on foo:
[dependencies]
foo = "0.1"

[public-dependencies]
bar = "0.1"

Now again though this isn't a showstopper or anything, just figured it's worth understanding as a consequence of using different syntax.

The forward compatibility issue is unfortunate, but I think I still prefer this syntax for its ergonomics. Would this mean that if a dependency starts using this feature, you would have to upgrade? Do we download the literal toml file, and does the API not receive any info from cargo about what version of cargo is calling it?

The forward compatibility issue is unfortunate, but I think I still prefer this syntax for its ergonomics

I think I agree yeah, I like the idea of a [public-dependencies] section a little bit better. That being said though I don't have strong preference and would be fine with public = true if the compatibility story holds more weight.

Would this mean that if a dependency starts using this feature, you would have to upgrade? Do we download the literal toml file, and does the API not receive any info from cargo about what version of cargo is calling it?

Hm yeah my going assumption to this point is that if you publish a crate using [public-dependencies] then you force all crates transitively depending on you to do a version upgrade of Rust. Given that we'd want to use [public-dependencies] in core crates to get the maximum mileage out of this it seemed like the wrong tradeoff because core crates are often the slowest to upgrade their minimum rustc requirements.
You may be right though in that older Cargo implementations wouldn't even parse the Cargo.toml for dependency information, I think they just use the manfiest. I'd want to test that, but it may make the issue not relevant! (the issue of compatibility that is)

@withoutboats: Resolution is done on the index, which is just a git repo Cargo clones. Moreover, the index does not contain the full Cargo.toml - just a distillation of certain parts of it. There's no "API" that Cargo would tell its version to, at that stage of the process.

In the index this could be represented as a flag in the deps object, so we don't have to worry about libraries upgrading forcing dependent crates to upgrade.

that resolution graphs where types of one version of a crate can reach another version will be considered invalid

Wouldn't that make @dtolnay's proposal to avoid the next libcpocalypse impossible?

Hmm, that would seem to be the case. I wonder if libc 0.3 could set some kind of compatible-with = "~0.2" setting to allow that kind of bridging to happen. You'll still get problems if something requires the new 0.3 version of some type and communicates with that type to another crate, but from what I can see, that'd be a problem with the semver-trick strategy anyways (since that's just a migration strategy rather than a solution). Or am I missing something?

@nox "Yes, but I'm not sure that's a problem"
@dtolnay's proposal makes it so that only the necessarily different types result in the failures that currently affect all cross-crate-version types, whereas this proposal prevents any cross-crate-version types from meeting in the first place by filtering out such resolutions in favor of ones that use only one version of the crate. As an example, let's assume crates.io contains the following (all deps public):

foo 1.0.0, depending on bar 1 and baz 1
bar 1.0.0, depending on baz 1
bar 1.1.0, depending on baz 2
baz 1 and 2

Under the old system, foo could pull in baz 1 and bar 1.1, which then pulls in baz 2. @dtolnay's proposal then means you only fail at build time if foo uses a type that changed between the two.
Under the new system, that entire resolution itself invalid, and foo would have instead pulled in bar 1.0.0, avoiding the problem entirely.

Ok so it sounds like the current status of this RFC is:

the publish-validate-lowest and resolution concerns still need to be addressed in the rfc
while we'd like a nicer syntax with a section for public dependencies and a section for private dependencies, in order to avoid not forcing everyone depending on a crate using this feature to upgrade rust to be able to build, we have to stay with specifying public = true on each public crate
this will use the new cargo unstable features

@mitsuhiko do you have time to make these changes to the RFC text before the impl period starts? Or would it be ok if I push some changes to your fork? (I think I already have permission to do that as a maintainer on this repo since this is a pull request)

@carols10cents i'm okay if you want to push some changes up. I won't have time to look at this before next weekend.

@withoutboats I have a question regarding this comment of yours:

@shepmaster You have to opt into the error by marking your dependency public. (Possibly there will be a warning encouraging you to do this.) But upgrading a warning to an error would not involve an opt-in.

I'm confused about whether we're talking about a compilation error or a version resolution error in this thread of conversation.  What I think you're saying, if I'm an author of a library that is currently compiling and has a dependency used in my crate's public API, is that this will happen when I upgrade to the next stable Rust+Cargo and run cargo build:

I will get a warning saying that I'm using a dependency in my public API and that I should mark it as public.
If I mark that dependency as public, I'm "opting in to the error" as you said, but there is no error, because my crate was compiling before I upgraded Rust (so I must have had a valid resolution).
If I mark that dependency with #[allow(external_private_dependency)], the warning goes away? Or did we not want to allow allowing? Or only on crate publish did we want to disallow this, someday?

On the other hand, if my crate is NOT currently compiling because type url::Url expected, got type url::Url and I heard this new version of Rust would fix it, then what happens would be:

I will get a warning saying that I'm using a dependency in my public API and that I should mark it as public.
If I mark that dependency as public, I'm "opting in to the error". Next time I run cargo build, cargo will recognize that the graph in my Cargo.lock is now an invalid graph-- will it automatically throw that away and try looking for a valid graph given the current version constraints? And then it might error if there are no possible graphs? Or do I have to cargo update if I have a lockfile, to get cargo to retry resolution? I'm assuming that we mean a resolution error here, because otherwise I'd get the same compilation error as before upgrading Rust and we're trying to get rid of that.

I may have some misunderstandings in here, I would appreciate any corrections from anyone!

If I mark that dependency as public, I'm "opting in to the error" as you said, but there is no error, because my crate was compiling before I upgraded Rust (so I must have had a valid resolution).

The possibility is that there are errors in crates that depend on you, not errors in your crate, because they won't be able to construct a crate resolution graph if they also depend on a conflicting version of url or whatever.
This means marking a dependency public is a breaking change, so you need to release a new major version once you mark all your public dependencies correctly.

The possibility is that there are errors in crates that depend on you, not errors in your crate, because they won't be able to construct a crate resolution graph if they also depend on a conflicting version of url or whatever.
This means marking a dependency public is a breaking change, so you need to release a new major version once you mark all your public dependencies correctly.

Ok. I think I get it. I was interpreting "i opt in to the error" as "it is now possible for me to get an error", but it's more like "i am opting in to bumping my major version and potentially causing errors for others". Is that right?
Everyone: I've made some changes to hopefully address the concerns. Please let me know what you think!

Is that right?

Yep! And other people opt into possibly getting errors by upgrading to the new version of your crate.

@rfcbot resolved publish-validate-lowest
Changes look great!

@rfcbot resolution
Changes look great!

@rfcbot resolved resolution

This basically means that if you privately depend on Hyper 0.3 and Hyper 0.4, that's an error.

Does that mean that it is impossible for a given type to implement serde::Serialize from both serde 0.9.x and serde 1.x.y? (Either through reexports like https://github.com/serde-rs/legacy, or with a future Cargo feature to locally rename dependent crates.)
That sounds bad :/

Maybe if you have to go through some kind of "legacy" crate meant for bridging support for multiple versions of a single crate? There could be a field for that.
Though you still would have the problem that serde_derive can only generate for a single version of serde at a time…

Why distinguish public and private dependencies if it means you can't depend on different versions of a private dependency?

This basically means that if you privately depend on Hyper 0.3 and Hyper 0.4, that's an error.

This feels like an error or a case of a bad explanation.

@alexcrichton's comment I think needs more context. What he meant was that if two versions of hyper are "exposed" to you, its an error. A dependency is exposed to you if:

It is a direct dependency of your crate, public or private.
It is a public dependency of a direct dependency or another public dependency of a direct dependency (recursively into infinite).

That is, this error is exactly what the RFC is supposed to introduce. It won't break any existing code, because dependencies are marked private by default and you can't directly depend on the same crate twice (at least not in a way that would trigger this error).
Obviously, there are use cases for "version straddling" - like implementing Serialize from both versions of serde. This needs to be supported somehow, possibly involving an opt out of this error. If its not adequately resolved in the RFC text I would say we make it an unresolved question for now.

if two versions of hyper are "exposed" to you, its an error

In @SimonSapin's examples

reexports like https://github.com/serde-rs/legacy

Legacy would treat serde 0.9 as a private dependency, so all's well here.

a future Cargo feature to locally rename dependent crates

Would depend on how it's implemented, of course, but I imagine that it could work in a similar way. The important thing would be that the renamed crate couldn't have any public dependencies of its own that then conflict. For example, if crate A v1.0 has a public dependency on Common v1.0 and A v2.0 has a public dependency on Common v2.0, then I'd hope the warning / error would be raised (unless there's something that also renames the public dependencies...)

Legacy would treat serde 0.9 as a private dependency, so all's well here.

So implementing serde09::Serialize for a public type does not make serde09 a public dependency?

So implementing serde09::Serialize for a public type does not make serde09 a public dependency?

I believe it would make the crate serde09 (version 0.9) a public dependency, but it would not make the crate serde (version 0.9) a public dependency.
This assumes that the crate performing the implementation of serde09::Serialize has marked serde09 as a public dependency (as the default is to act as a private dependency).

The real question is whether or not its sufficient for serde09 to just not marke serde a public dependency, or if we need some other mechanism here. I'm really unsure!

Merged this PR! Tracking issue is rust-lang/rust#44663. Thanks so much @mitsuhiko, @carols10cents, and everyone else for working on this RFC.
Excited to try to get this in nightly during the upcoming impl period. :tada:

The rendered RFC link gives a 404 - could it be updated to the current repository's copy please?

@alekratz

The rendered RFC link gives a 404 - could it be updated to the current repository's copy please?

Done!


tls_context error initializing configuration "invalid path: /etc/..."

Problem
Hi all,
I just want to extend my envoy with the 443 redirect but I fail to get it work.
If I run my code I get

Thank you so much for any help!! :)
Description:
My Setup:
Docker Swarm Node on digitalocean droplet
2 Python Services with 1 Angular Frontend
I Cant get it solved, maybe someone can help me.
1 In the folder "test" I deploy my docker-stack with the file docker-stack.yaml:

2 in the folder envoy I have the envoy config and the 2 certificates

front-envoy.yaml:
static_resources:
  listeners:
  - address:
      socket_address:
        address: <IP_ADDRESS>
        port_value: 443
    filter_chains:
    - filters:
      - name: envoy.http_connection_manager
        config:
          codec_type: auto
          stat_prefix: ingress_http
          route_config:
            name: local_route
            virtual_hosts:
            - name: backend
              domains:
              - "example.de"
              routes:
              - match:
                  prefix: "/api/service1/"
                route:
                  cluster: appone
              - match:
                  prefix: "/api/service2/"
                route:
                  cluster: apptwo
              - match:
                  prefix: "/"
                route:
                  cluster: frontend
          http_filters:
          - name: envoy.router
            config: {}
      tls_context:
        common_tls_context:
          tls_certificates:
            - certificate_chain:
                filename: "/etc/chain.pem"
              private_key:
                filename: "/etc/privkey.pem"

  - address:
      socket_address:
        address: <IP_ADDRESS>
        port_value: 80
    filter_chains:
    - filters:
      - name: envoy.http_connection_manager
        config:
          codec_type: auto
          stat_prefix: ingress_http
          route_config:
            virtual_hosts:
            - name: backend
              domains:
              - "example.de"
              routes:
              - match:
                  prefix: "/"
                redirect:
                  path_redirect: "/"
                  https_redirect: true
          http_filters:
          - name: envoy.router
            config: {}

  clusters:
  - name: appone
    connect_timeout: 0.25s
    type: strict_dns
    lb_policy: round_robin
    http_protocol_options: {}
    hosts:
    - socket_address:
        address: appone
        port_value: 5001
  - name: apptwo
    connect_timeout: 0.25s
    type: strict_dns
    lb_policy: round_robin
    http_protocol_options: {}
    hosts:
    - socket_address:
        address: apptwo
        port_value: 5002
  - name: frontend
    connect_timeout: 0.25s
    type: strict_dns
    lb_policy: round_robin
    http_protocol_options: {}
    hosts:
    - socket_address:
        address: frontend
        port_value: 80

admin:
  access_log_path: "/dev/null"
  address:
    socket_address:
      address: <IP_ADDRESS>
      port_value: 8001

docker-stack.yaml:
version: '3.7'

services:
  front-envoy:
    image: test/envoy:v10
    volumes:
      - ./envoy/front-envoy.yaml:/etc/front-envoy.yaml
    deploy:
      replicas: 1
      restart_policy:
        condition: on-failure
    networks:
      - envoymesh
    ports:
      - "80:80"
      - "8001:8001"

  appone:
    image: test/appone:v10
    deploy:
      endpoint_mode: dnsrr
      replicas: 2
      restart_policy:
        condition: on-failure
    networks:
      default:
      envoymesh:
        aliases:
          - appone
    depends_on:
      - front-envoy
    environment:
      - SERVICE_NAME=appone

  apptwo:
    image: test/apptwo:v10
    deploy:
      endpoint_mode: dnsrr
      replicas: 3
      restart_policy:
        condition: on-failure
    networks:
      envoymesh:
        aliases:
          - apptwo
    depends_on:
      - front-envoy
    environment:
      - SERVICE_NAME=apptwo

  frontend:
    image: test/frontend:v10
    volumes:
      - /app/node_modules
    deploy:
      endpoint_mode: dnsrr
      replicas: 2
      restart_policy:
        condition: on-failure
    networks:
      envoymesh:
        aliases:
          - frontend
    environment:
      - SERVICE_NAME=frontend
    depends_on:
      - appone
      - apptwo
      - front-envoy

networks:
  envoymesh:
    driver: overlay
    attachable: true

I think you missed to mount the certificate files. You can put smth like
- ./envoy/chain.pem:/etc/chain.pem under volumes in docker-stack.yaml

@crazyxy thank you very much! this works, but now I get "failed to load privkey"

But thank you again for your help

You should mount the private key as well.

@crazyxy  :) yes I ve done it. but get this error ;)

Is your private key file valid? I used following cmd to generate cert and private key and it works in my env
openssl req -x509 -newkey rsa:4096 -keyout key.pem -out cert.pem -days 365 -nodes -subj '/CN=localhost'

@crazyxy now evrything works. I created a new key and chain with certbot. Thank you very much for your help !!!


Create an OWNERS file with approvers and reviewers for IBM Cloud Kubeflow Deploy docs

This PR adds an OWNERS file with a list of approvers and reviewers for IBM Cloud Kubeflow Deploy docs.
Added by mirroring the OWNERS file in the IBM Cloud folder + adding 8bitmp3:
approvers:
  - animeshsingh
reviewers:
  - adrian555
  - animeshsingh
  - shawnzhu
  - 8bitmp3

@Tomcli do you want to be included too?
/assign @animeshsingh
cc @adrian555 @shawnzhu

Thanks @8bitmp3, can you also add me as the reviewer?

@8bitmp3 would you please update this OWNERS file instead of adding a new one?
https://github.com/kubeflow/website/blob/d8a6745d343d995bb2f5f8a967c75a6b1b523d54/content/en/docs/ibm/OWNERS

@shawnzhu Let's update both just in case.
@Tomcli  Added you :)

@animeshsingh Let us know what you think. Thanks!

@Bobgy @jlewi If you can assist with approving this OWNERS PR, it'd be very helpful. Then, certain IBM Cloud team members can proceed with https://github.com/kubeflow/website/pull/2163 Thanks!

/lgtm
/approve


Improve performance of VF2 scoring and add support for scoring passes

Summary
This commit makes 2 key changes to the vf2 layout pass. The first is it migrates the scoring routine to rust. When running vf2 layout and vf2 post layout we're bottle necked by the performance of the scoring of a layout since in practice scoring a large circuit ends up taking more time than the vf2_mapping() function. To address this the scoring function is migrated to rust where the iteration will be much faster. To enable this rust migration the average error map is made into an ErrorMap class which can be efficiently be accessed by reference from rust. This additionally also enables a convenient interface for future expansion of the vf2 layout passes. The VF2LayoutPass and VF2PostLayout passes will now both look for a "vf2_avg_error_map" entry in the property set which contains a ErrorMap used for scoring. If present that array will be used for scoring instead of the computing one from the target's error rates. This will enable custom analysis passes to be run pre-layout to compute or inject a custom scoring heuristic.
Details and comments
TODO:

[x] Fix test failures. Fails with faulty qubits (I assume the backend properties mapping is broken and only worked by chance before this) and fake melbourne (which had a weird broken qubit and I bet the stored payload is broken)
[x] Benchmark and potentially tune performance

I've updated the PR to use a custom hash map based class instead of a 2d numpy array to represent the average error map. This should both be much more memory efficient and also make the usage a bit clearer for people to interact with.

To show the performance improvements I threw together a small test script:
import statistics
import time

from qiskit.transpiler.passes.layout import VF2Layout
from qiskit.circuit import QuantumCircuit
from qiskit.providers.fake_provider import FakeMumbaiV2
from qiskit.converters import circuit_to_dag

qc = QuantumCircuit(7)

qc.h(0)
qc.cz(0, 1)
qc.cz(1, 2)
qc.cz(2, 3)
qc.measure_all()
dag = circuit_to_dag(qc)
backend = FakeMumbaiV2()

times = []
vf2_pass = VF2Layout(target=backend.target, max_trials=-1)
for i in range(5):
    print(f"Run {i}")
    start = time.perf_counter()
    vf2_pass.run(dag)
    stop = time.perf_counter()
    times.append(stop - start)
    print(stop - start)
print(statistics.geometric_mean(times))

This script is a worst case from a scoring perspective, it's mapping a line with 4 nodes and 3 free nodes onto the coupling graph. This means there are a lot of possible permutations for valid isomorphic mappings but each is trivial for rustworkx to calculate. It also turns off any limits in the pass so it will fully iterate through all available mappings.
Running this script with this PR applied returned:
6.825820831970144

Then running it on main:
14.780817176541099

This obviously is a best case improvement. In more realistic testing with limits set I was seeing a ~10% improvement over main with this PR applied.


My tools first namespace group does not start out expanded

Bug Report
List of tools by name space on my tools should start out with the first namespace expanded.
Expected behaviour
Expand first namespace grouping on load of page.
Actual behaviour
No namespace group is open.
Steps to reproduce the behaviour
Go to My Tools.

Fixed in PR 89 of UI


Fixes to make role work with new logstash

Some small fixes to correct issues with running the role. Bigges issue was the iteration and ignoring errors for the calls for logstash-web which doesn't exist in versions after 1.4 (which is very old).
Pointed the default URL to newes 2.1.1 release of logstash.
Future enhancements might see replacing the logstash forward with the newer filebeats.

Stouts does seem to like to hardcode versions.  I think it'd be useful to extract the logstash_version out to a variable, and use that in the logstash_url.  That way you can use a when: logstash_version >= x.x instead of those ignore_errors: yes on the tasks specific to older versions.

otherwise looks LGTM

@sairez I changed it a bit more to allow definition of logstash_version which then add the official repo fo r that version and installs it via apt rather then curling the file down.


Added onPageBeingChanged event

Hi,
Together with @thierryfornetti we have added a new event "onPageBeingChanged" that will be triggered every time the user start the scroll to a new page.
Our use case was: hide some UI elements when a page is appearing.

Hi @phil-r!
Thank you for the review!
I've just pushed the changed required.
Thank you for your time!
Gabriele


Lineage-specific alignment

We align each genome to the WH1 reference genome using minimap2 to extract any mutational difference from the reference as a "feature".  This was a reasonable approximation in the early stages of the pandemic.  However, circulating genomes can now carry more than 100 mutations away from this reference, and there is a substrantial number of recurrent mutations (homoplasies).
Since we are using PANGO lineage assignments to partition genomes into separate sets at an early stage of the analysis workflow, it should be possible to use a lineage-defining genome sequence (or even the consensus of multiple such genomes) as a lineage-specific reference.  The rest of the analysis should be the same (the feature vectors will be greatly reduced).
In fact, we are already selecting a representative genome for each lineage for building the time-scaled tree as a navigation tool.  The tricky part is that we need to have the reference genome for any given lineage available at the initial data feed processing step.

On the other hand, the shared features of genomes of a given lineage shouldn't affect the clustering (neighbor-joining) analysis, and managing thousands of reference genomes is probably more trouble than it's worth.


@slidy/svelte: number of items visible in viewport

Right now there is option to control the number of items to display in the viewport.
New property visibleGroup  should be implemented.

Implemented as groups property: eb21b94.


Bug: Changing network while connected with WalletConnect not working

I've found a problem, while I'm connected to WalletConenct network subscription is not being called when I change the network in my wallet. It does work if I'm connected with MetaMask directly.
Steps to reproduce:

I have MM on my phone
I connect through WC to my MM
I change from Ethereum Mainnet to Ethereum Optimism
network subscription is never called.

The same steps work fine if I connect directly with my MM browser extension.

@nicosampler Unfortunately this appears to be a Metamask Mobile issue :/
I will let them know to see if they can resolve this.


The presence of ivy.xml should be optional

Ivy can fetch jars that have no accompanying ivy.xml (presumably it assumes that these have no dependencies of their own). However Coursier seems to require an ivy.xml from ivy: repositories.
This is related to https://github.com/coursier/coursier/issues/755.
Repro:
Put some .jar file at /my/local/repo/foo/bar/1.2.3/bar-1.2.3.jar.
Run:
java  -cp ~/coursier.jar coursier.cli.Coursier fetch --no-default -t -r ivy:file:////my/local/repo/[organization]/[module]/[revision]/[artifact]-[revision].[ext]  foo:bar:1.2.3
Expected behavior:
/my/local/repo/foo/bar/1.2.3/bar-1.2.3.jar is downloaded
Actual behavior:
An attempt is made to fetch /my/local/repo/foo/bar/1.2.3/ivy-1.2.3.xml. This file doesn't exist, and the coursier run fails.

I wasn't aware ivy could do that… It'd be interesting to support that, yes. (PRs welcome :)


Running on ESP32

As my raspi is to far away from the plants with ble plant sensor I wondered if it would be possible to get the code on a esp32 running.

Hey! This project depends on python and a Linux host system. A totally different world.
You should check out the passive BLE capabilities in the latest releases of Home Assistant as an alternative to the project here. In addition you can program your ESP32 with ESPHome to be a HA compatible "bluetooth repeater". If I didn't have a working setup using the project here, this is probably how I would do it these days.


⚠ Processors has degraded performance

In e947859, Processors ($STATUS_URL) experienced degraded performance:

HTTP code: 200
Response time: 780 ms

Resolved: Processors performance has improved in ecb039f.


Fix highlighting for 'diff' filetype

By default, syntax/diff.vim from the neovim runtime uses highlights for Identifier and Special for added and removed lines:
hi def link diffAdded		Identifier
hi def link diffRemoved		Special

As these are both highlighted in white in monokai.nvim, it makes diff highlighting display everything as plain white.
Fix this by adding explicit highlights for diffAdded and diffRemoved in appropriate colors.

Thanks a bunch :)


Audit formulas for "build.head?" on instance methods

It is correct to use Formula#head? etc within instance methods in place of build.head?. build.head? et al. are nil in post_install.

Condenses knowledge from
https://github.com/Homebrew/legacy-homebrew/issues/37120.
Swats fly with shotgun.

@tdsmith Any more thoughts here?

@tdsmith Ping?


Generated typings do not name refrences correctly

Content stack appends Connection after all field names that are references, and also adds subfields in GraphQL.
example:
Content type field:
Reference to ContentBelt type
GraphQL request response:
"content_beltsConnection": {             "edges": [               {                 "node": {                   "title": "Collapsibles test",                   "system": {                     "uid": "blt847f4e37080902a8"                   }                 }               },] }
however the tsgen result of that is just the field name, with an array of the content type:
navigation_menu: ContentBelt[]

Hey @olafurns7 ,Support for graphQL has been added to tsgen plugin. Install the most recent version, please.
Example:- csdx tsgen -a "delivery token alias" -o "contentstack/generated.d.ts" --api-type graphql
Doc for reference:- https://www.contentstack.com/docs/developers/cli/tsgen-plugin
PR:- #82


Update Makefile

Test PR creation
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: abhay-krishna
The full list of commands accepted by this bot can be found here.
The pull request process is described here

Needs approval from an approver in each of these files:

OWNERS [abhay-krishna]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@abhay-krishna: The following test failed, say /retest to rerun all failed tests:

Test name
Commit
Details
Rerun command

eks-distro-base-presubmit
1c7c1c947dc6ab0da8442531d0fd5e4abf76e82a
link
/test eks-distro-base-presubmit

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@abhay-krishna: The following test failed, say /retest to rerun all failed tests:

Test name
Commit
Details
Rerun command

eks-distro-base-presubmit
3bb2f2cd9464bc09086481092c61f1617ca782c6
link
/test eks-distro-base-presubmit

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@abhay-krishna: The following test failed, say /retest to rerun all failed tests:

Test name
Commit
Details
Rerun command

eks-distro-base-presubmit
60920a60c23b08e688c51e57b20dfed490d260b1
link
/test eks-distro-base-presubmit

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@abhay-krishna: The following test failed, say /retest to rerun all failed tests:

Test name
Commit
Details
Rerun command

eks-distro-base-presubmit
60920a60c23b08e688c51e57b20dfed490d260b1
link
/test eks-distro-base-presubmit

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.


[Request]: Apex Legends

App to request
Apex Legends
App Store link
https://apps.apple.com/nl/app/apex-legends-mobile/id1543991460?l=en
App version
1.0.1576
Jailbreak
Unc0ver + Fugu14
Bypass tried

[ ] A-Bypass
[ ] Liberty Lite (Beta)
[ ] Hestia
[ ] Shadow
[ ] HideJB
[ ] iHide
[ ] KernBypass (iOS 12.0 - 14.2)
[X] vnodebypass
[X] Disabling tweak injection with Choicy or libhooker-configurator

Which bypass worked?
vnodebypass
Checklist before submission

[X] The app is missing from the table, or bypasses are not working anymore.
[X] I have checked through all open issues and none matched my app.
[X] I have tried at least one bypass before submission.

will test soon

currently using odysseyra1n and the game hasn't prevented me from playing. strange


Make confirm logic in volume prune be consistent with other Prune

Signed-off-by: Han Xu<EMAIL_ADDRESS>

Could you explain this PR?

Could you explain this PR?

Sorry for not explaining this PR.
Description updated.


fetchNext() does not return the first result

Package Name: @azure/cosmos
Package Version: 3.17.1
Operating system: MacOS and Azure App Services
[x] nodejs

version: 14 and 16

[ ] browser

name/version:

[x] typescript

version: 4.8.4

Is the bug related to documentation in

[ ] README.md
[ ] source code documentation
[ ] SDK API docs on https://docs.microsoft.com

Describe the bug
After investigation we found that there is as any issue with the fetchNext() method that we are using, when we only expect one database result.
Sometimes the first call to fetchNext() will return an empty array of resources with hasMoreResults = true. In order to get the first record in this situation you have to call fetchNext() multiple times until you get a result, or hasMoreResults becomes comes false.
On the other hand fetchAll() will call fetchNext() multiple times until all the results are retrieved. (There are no more results.)
So we are switching all of the findOne() and similar methods over to use fetchAll() to avoid this problem.
To Reproduce
Steps to reproduce the behavior:

Insert a bunch of records into a container, with different partition keys.
Try to query for a single document with a unique identifier that is not the id/pk of the container. (This means that the queries will be cross partition queries.)
Try Step 2 for all documents in the container, until one fails to be returned by the first call to fetchNext()
This behavior will repeatable for the document identified in Step 3

Expected behavior
The first call to fetchNext() always returns the first query result.
Actual behavior
The first call to fetchNext() does not always return the first result.
Screenshots
If applicable, add screenshots to help explain your problem.
Additional context
Add any other context about the problem here.

hi @paulsmithkc This is the expected behaviour, hasMoreResults is always true by default, it gets set to false after fetchNext if there is no more continuation!

@sajeetharan You misunderstood the issue.
Previously the first time fetchNext returned, it would include a result. Now you have to call fetchNext() multiple times to get the first result.

Reopening to investigate further.

@paulsmithkc

Previously the first time fetchNext returned, it would include a result. Now you have to call fetchNext() multiple times to get the first result.

By "Previously" you mean at an earlier time when the database had different number of entities? This is possible because the paritions might have been different. We have a gist that describes this issue here.
Could you clarify what you meant by "Previously"?

I think that gist accurately describes the problem.

Is there a reason it is labelled "secret" rather than being searchable?

Should that gist be included in the API docs, to help explain usage/expectations for fetchNext() ?


[docker] fix cgroup parsing to support rancherOS cgroup hierarchy

What does this PR do?
As per support ticket, support rancherOS's custom cgroup hierarchy:

add unit tests for cgroup -> cid parsing
fix containerIDFromCgroup logic to select last match instead of first
use regexp.FindAllString instead of regexp.FindAll to avoid []bytes and string allocations. This method returns a slice of the source string.

@shang-wang that's merged. you should update your lock on the package and build a new test binary for them


How to use na fill in mlsql?

In spark, we can user df.na.fill to handle null, but in mlsql you can't call na.fill, i wan't to realize sqlalg  to offer  na.fill semantic。

I have find the doc about how to write my sqlalg https://github.com/allwefantasy/streamingpro/blob/master/docs/docv2/contribute/estimator-transformer-in-mlsql.md

Now, the doc don't said how to run with myself  na jar, is use register ?

Make sure the package name is "streaming.dsl.mmlib.algs" and the class name starts with SQL and ends with Ext. Then the system will find it automatically.

streaming.dsl.mmlib.algs.SQLCacheExt is a transformer used to cache dataframe, take his for example.

I think na fill is not a simple sql action,it is a sql action. We may need to think more clear how to do this type of action.
Let's see this action will effect all column but not one
maybe some kind of action like train may look goods.
but keyword train may not fit to this.
should we add a keyword like transform.

@RebieKong You are right, Transformer is good for this. The keyword train is used by estimator which will generate a model after data processing. keyword run is ok and MLSQL already supports this keyword.

I code a Transformer
`package mlsqlext
import org.apache.spark.ml.param.Param
import org.apache.spark.sql.{DataFrame, SparkSession}
import org.apache.spark.sql.expressions.UserDefinedFunction
import streaming.dsl.mmlib.SQLAlg
import streaming.dsl.mmlib.algs.param.{BaseParams, WowParams}
import streaming.session.MLSQLException
class SQLNa(override val uid: String) extends SQLAlg with WowParams {
override def train(df: DataFrame, path: String, params: Map[String, String]): DataFrame = {
val exe = params.get(execute.name)

if (!execute.isValid(exe)) {
  throw new MLSQLException(s"${execute.name} should be fill or drop or replace")
}

if (exe == "fill") {
  df.na.fill("")
} else if (exe == "drop") {
  df.na.drop()
} else if(exe == "replace") {
  //df.na.replace()
}

df

}
override def load(sparkSession: SparkSession, path: String, params: Map[String, String]): Any = {
throw new RuntimeException("train is not support")
}
override def predict(sparkSession: SparkSession, _model: Any, name: String, params: Map[String, String]): UserDefinedFunction = {
null
}
final val execute: Param[String] = new Param[String](this, "execute", "fill|drop|replace", isValid = (m: String) => {
m == "fill" || m == "drop" || m == "replace"
})
def this() = this(BaseParams.randomUID())
}
and I have submit mlsql with -jars, now How i use the Transformer? For example,
include hdfs./tmp/virtual_tables/community_stream;
select
*
from community_stream as write_table ;
`

Rename you transformer to SQLNaFillExt . Then you can use like this:
run data as NaFillExt.`` where execute="fill";

greate, if we can remove `` to make this script looks much better