-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RFC for prepublication dependencies #1969
Changes from 3 commits
86b811a
95cf2b4
8c15693
aefa890
5ac3974
fe22a91
a09a0c5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,395 @@ | ||
- Feature Name: prepublish | ||
- Start Date: 2017-03-22 | ||
- RFC PR: (leave this empty) | ||
- Rust Issue: (leave this empty) | ||
|
||
# Summary | ||
[summary]: #summary | ||
|
||
This RFC proposes the concept of *augmenting sources* for Cargo. Sources can be | ||
enhanced with new versions of a crate that did not exist or have existing | ||
versions replaced. Dependency resolution will work *as if* these additional | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. additional or replacement crates |
||
crates actually existed in the original source. | ||
|
||
One primary feature enabled by this is the ability to "prepublish" a crate to | ||
crates.io. Prepublication makes it possible to perform integration testing | ||
within a large crate graph before publishing anything to crates.io, and without | ||
requiring dependencies to be switched from the crates.io index to git branches. | ||
It can, to a degree, simulate an "atomic" change across a large number of crates | ||
and repositories, which can then actually be landed in a piecemeal, non-atomic | ||
fashion. | ||
|
||
# Motivation | ||
[motivation]: #motivation | ||
|
||
Large Rust projects often end up pulling in dozens or hundreds of crates from | ||
crates.io, and those crates often depend on each other as well. If the project | ||
author wants to contribute a change to one of the crates nestled deep in the | ||
graph (say, `xml-rs`), they face a couple of related challenges: | ||
|
||
- Before submitting a PR upstream to `xml-rs`, they will usually want to try | ||
integrating the change within their project, to make sure it actually meets | ||
their needs and doesn't lead to unexpected problems. That might involve a | ||
*cascade* of changes if several crates in the graph depend on `xml-rs`. How do | ||
they go about this kind of integration work prior to sending a PR? | ||
|
||
- If the change to the upstream `xml-rs` crate is breaking (would require a new | ||
major version), it's vital to carefully track which other crates in the graph | ||
have successfully been updated to this version, and which ones are still at | ||
the old version (and can stay there). This issue is related to the notion of | ||
[private dependencies](https://github.com/rust-lang/cargo/issues/2064), which | ||
should have a separate RFC in the near future. | ||
|
||
- Once they're satisfied with the change to `xml-rs` (and any other intermediate | ||
crates), they'll need to make PRs and request a new publication to | ||
crates.io. But they would like to cleanly continue local development in the | ||
meantime, with an easy migration as each PR lands and each crate is published. | ||
|
||
## The Goldilocks problem | ||
|
||
It's likely the a couple of Cargo's existing features have already come to mind | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. s/It's likely the a/It's likely that a/ |
||
as potential solutions to the challenges above. But the existing features suffer | ||
from a Goldilocks problem: | ||
|
||
- You might reach for git (or even path) dependencies. That would mean, for | ||
example, switching an `xml-rs` dependency in your crate graph from crates.io | ||
to point at, for example, a forked copy on github. The problem is that **this | ||
approach does not provide enough dependency unification**: if other parts of | ||
the crate graph refer to the crates.io version of `xml-rs`, it is treated as | ||
an entirely separate library and thus compiled separately. That in turn means | ||
that two crates in the graph using these distinct versions won't be able to | ||
talk to each other about `xml-rs` types (even when those types are identical). | ||
|
||
- You might think that `[replace]` was designed precisely for the use case | ||
above. But **it provides too much dependency unification**: it reroutes *all* | ||
uses of a particular existing crate version to new source for the crate, even | ||
if there are breaking changes involved. The feature is designed for surgical | ||
patching of specific dependency versions. | ||
|
||
Prepublication dependencies add another tool to this arsenal, with just the | ||
right amount of dependency unification: the precise amount you'd get after | ||
publication to crates.io. | ||
|
||
# Detailed design | ||
[design]: #detailed-design | ||
|
||
The design itself is relatively straightforward. The Cargo.toml file will | ||
support a new section for augmenting a source of crates: | ||
|
||
```toml | ||
[augment.crates-io] | ||
xml-rs = { path = "path/to/fork" } | ||
``` | ||
|
||
The listed dependencies have the same syntax as the normal `[dependencies]` | ||
section, but they must all come form a different source than the source being | ||
augmented. For example you can't augment crates.io with other crates from | ||
crates.io! Cargo will load the crates and extract the version information for | ||
each dependency's name, supplementing the source specified with the version it | ||
finds. If the same name/version pair *already* exists in the source being | ||
augmented, then this will act just like `[replace]`, replacing its source with | ||
the one specified in the `[augment]` section. | ||
|
||
Like `[replace]`, the `[augment]` section is only taken into account for the | ||
root crate (or workspace root); allowing it to accumulate anywhere in the crate | ||
dependency graph creates intractable problems for dependency resolution. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure what this paragraph means exactly-- does it mean that a dependent crate's There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct yeah, |
||
|
||
The sub-table of `[augment]` (where `crates-io` is used above) is used to | ||
specify the source that's being augmented. Cargo will know ahead of time one | ||
identifier, literally `crates-io`, but otherwise this field will currently be | ||
interpreted as a URL of a source. The name `crates-io` will correspond to the | ||
crates.io index, and other urls, such as git repositories, may also be specified | ||
for augmentation. Eventually it's intended we'll grow support for multiple | ||
registries here with their own identifiers, but for now just literally | ||
`crates-io` and other URLs are allowed. | ||
|
||
## Examples | ||
|
||
It's easiest to see how the feature works by looking at a few examples. | ||
|
||
Let's imagine that `xml-rs` is currently at version `0.9.1` on crates.io, and we | ||
have the following dependency setup: | ||
|
||
- Crate `foo` lists dependency `xml-rs = "0.9.0"` | ||
- Crate `bar` lists dependency `xml-rs = "0.9.1"` | ||
- Crate `baz` lists dependency `xml-rs = "0.8.0"` | ||
- Crate `servo` has `foo`, `bar` and `baz` as dependencies. | ||
|
||
With this setup, the dependency graph for Servo will contain *two* versions of | ||
`xml-rs`: `0.9.1` and `0.8.0`. That's because minor versions are coalesced; | ||
`0.9.1` is considered a minor release against `0.9.0`, while `0.9.0` and `0.8.0` | ||
are incompatible. | ||
|
||
### Scenario: augmenting with a bugfix | ||
|
||
Let's say that while developing `foo` we've got a lock file pointing to `xml-rs` | ||
`0.8.0`, and there's an `0.8.0` branch of `xml-rs` that hasn't been touched | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "a" not "an" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wasn't There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
since it was published. We then find a bug in the 0.8.0 publication of `xml-rs` | ||
which we'd like to fix. | ||
|
||
First we'll check out `foo` locally and implement what we believe is a fix for | ||
this bug, and next, we change `Cargo.toml` for `foo`: | ||
|
||
```toml | ||
[augment.crates-io] | ||
xml-rs = { path = "../xml-rs" } | ||
``` | ||
|
||
When compiling `foo`, Cargo will resolve the `xml-rs` dependency to `0.8.0`, | ||
as it did before, but that version's been replaced with our local copy. The | ||
local path dependency, which has version 0.8.0, takes precedence over the | ||
version found in the registry. | ||
|
||
Once we've confirmed a fix bug we then continue to run tests in `xml-rs` itself, | ||
and then we'll send a PR to the main `xml-rs` repo. This then leads us to the | ||
next section where a new version of `xml-rs` comes into play! | ||
|
||
### Scenario: prepublishing a new minor version | ||
|
||
Now, suppose that `foo` needs some changes to `xml-rs`, but we want to check | ||
that all of Servo compiles before pushing the changes through. | ||
|
||
First, we change `Cargo.toml` for `foo`: | ||
|
||
```toml | ||
[augment.crates-io] | ||
xml-rs = { git = "https://github.com/aturon/xml-rs", branch = "0.9.2" } | ||
|
||
[dependencies] | ||
xml-rs = "0.9.2" | ||
``` | ||
|
||
For `servo`, we also need to record the prepublication, but don't need to modify | ||
or introduce any `xml-rs` dependencies; it's enough to be using the fork of | ||
`foo`, which we would be anyway: | ||
|
||
```toml | ||
[augment.crates-io] | ||
xml-rs = { git = "https://github.com/aturon/xml-rs", branch = "0.9.2" } | ||
|
||
[dependencies] | ||
foo = { git = "https://github.com/aturon/foo", branch = "fix-xml" } | ||
``` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I understand this example-- why would we use That is, why wouldn't servo's Cargo.toml look like this?
Also it says "For There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Heh this was actually my third question asked originally. It's correct yes that your toml also works, they're equivalent constructions. Depending directly on git is possible if Sounds like a good clarification though, I'll add it. |
||
|
||
With this setup: | ||
|
||
- When compiling `foo`, Cargo will resolve the `xml-rs` dependency to `0.9.2`, and | ||
retrieve the source from the specified git branch. | ||
|
||
- When compiling `servo`, Cargo will again resolve *two* versions of `xml-rs`, | ||
this time `0.9.2` and `0.8.0`, and for the former it will use the source from | ||
the git branch. | ||
|
||
The Cargo.toml files that needed to be changed here span from the crate that | ||
actually cares about the new version (`foo`) upward to the root of the crate we | ||
want to do integration testing for (`servo`); no sibling crates needed to be | ||
changed. | ||
|
||
Once `xml-rs` version `0.9.2` is actually published, we can remove the | ||
`[augment]` sections, and Cargo will warn us that this needs to be done. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That this might need to be done, if our patch has been merged into the actually published 0.9.2... although didn't you say we can't guarantee we'd get a warning? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yes sorry I forgot to update that when updating the RFC, I'll correct this to account that no warnings will be emitted and that this may or may not be all that needs to be done. |
||
|
||
### Scenario: prepublishing a new major version | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: this heading says major version but shows changing a minor version change of 0.9.0 to 0.10.0... I think it could be fixed by saying "prepublishing a new version incompatible with current version requirements" since you might have version requirements like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Eh to me it's mincing words, I view 0.9 -> 0.10 as a new major version, but regardless I'll update the text or use literal 1.0 and 2.0 versions instead. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. THE SPEC IS CLEAR There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: "prepublishing a breaking change" |
||
|
||
What happens if `foo` instead needs to make a breaking change to `xml-rs`? The | ||
workflow is identical. For `foo`: | ||
|
||
```toml | ||
[augment.crates-io] | ||
xml-rs = { git = "https://github.com/aturon/xml-rs", branch = "0.10.0" } | ||
|
||
[dependencies] | ||
xml-rs = "0.10.0" | ||
``` | ||
|
||
For `servo`: | ||
|
||
```toml | ||
[augment.crates-io] | ||
xml-rs = { git = "https://github.com/aturon/xml-rs", branch = "0.10.0" } | ||
|
||
[dependencies] | ||
foo = { git = "https://github.com/aturon/foo", branch = "fix-xml" } | ||
``` | ||
|
||
However, when we compile, we'll now get *three* versions of `xml-rs`: `0.8.0`, | ||
`0.9.1` (retained from the previous lockfile), and `0.10.0`. Assuming that | ||
`xml-rs` is a public dependency used to communicate between `foo` and `bar` this | ||
will result in a compilation error, since they are using distinct versions of | ||
`xml-rs`. To fix that, we'll need to update `bar` to also use the new, `0.10.0` | ||
prepublication version of `xml-rs`. | ||
|
||
(Note that a | ||
[private dependency](https://github.com/rust-lang/cargo/issues/2064) distinction | ||
would help catch this issue at the Cargo level and give a maximally informative | ||
error message). | ||
|
||
## Impact on `Cargo.lock` | ||
|
||
Usage of `[augment]` will perform backwards-incompatible modifications to | ||
`Cargo.lock`, meaning that usage of `[augment]` will prevent previous versions | ||
of Cargo from interpreting the lock file. Cargo will unconditionally resolve all | ||
entries in the `[augment]` section to precise dependencies, encoding them all in | ||
the lock file whether they're used or not. | ||
|
||
Dependencies formed on crates listed in `[augment]` will then be listed directly | ||
in Cargo.lock, eschewing the actual dependency entirely. For example let's say | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unless there's another crate that depends on the un-augmented registry sourced crate, like xml-rs 0.9.1 in the previous example, right? This part:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm no I think as written this is accurate, albeit confusing. Let's say that you depend on That process means that xml-rs 0.9.1 isn't mentioned at all in the Cargo.toml, nor is xml-rs 0.9.2 from the registry (as it doesn't exist). Only xml-rs 0.9.2 in a git repo is mentioned in the lock file. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, what i mean is, in the previous example, we originally had:
which resulted in the lock file containing Then foo needed a breaking change, so it needed to go to So it is not the case that if you augment, the diff of Cargo.lock will always show an addition of the augment and a removal of the original, it might only show an addition of the augment if another crate is still depending on the original. Again, I might be being overly pedantic. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are correct yeah. I see "eschewing the actual dependency" in terms of "xml-rs 0.10.0 from crates.io" is not listed in the lock file (because it doesn't exist). In any case I'll see if I can update the wording here to be more clear. |
||
we depend on `env_logger` but we're using `[augment]` to depend on a git version | ||
of the `log` crate, a dependency of `env_logger`. First we'll have our | ||
`Cargo.toml` including: | ||
|
||
```toml | ||
# Cargo.toml | ||
[dependencies] | ||
env_logger = "0.4" | ||
``` | ||
|
||
With that we'll find this in `Cargo.lock`, notably everything comes from | ||
crates.io | ||
|
||
```toml | ||
# Cargo.lock | ||
[[package]] | ||
name = "env_logger" | ||
version = "0.4.2" | ||
source = "registry+https://github.com/rust-lang/crates.io-index" | ||
dependencies = [ | ||
"log 0.3.7 (registry+https://github.com/rust-lang/crates.io-index)", | ||
] | ||
|
||
[[package]] | ||
name = "log" | ||
version = "0.3.7" | ||
source = "registry+https://github.com/rust-lang/crates.io-index" | ||
``` | ||
|
||
Next up we'll add our `[augment]` section to crates.io: | ||
|
||
```toml | ||
# Cargo.toml | ||
[augment.crates-io] | ||
log = { git = 'https://github.com/rust-lang-nursery/log' } | ||
``` | ||
|
||
and that will generate a lock file that looks (roughly) like: | ||
|
||
```toml | ||
# Cargo.lock | ||
[[package]] | ||
name = "env_logger" | ||
version = "0.4.2" | ||
source = "registry+https://github.com/rust-lang/crates.io-index" | ||
dependencies = [ | ||
"log 0.3.7 (git+https://github.com/rust-lang-nursery/log)", | ||
] | ||
|
||
[[package]] | ||
name = "log" | ||
version = "0.3.7" | ||
source = "git+https://github.com/rust-lang-nursery/log#cb9fa28812ac27c9cadc4e7b18c221b561277289" | ||
``` | ||
|
||
Notably `log` from crates.io *is not mentioned at all here*, and crucially so! | ||
Additionally Cargo has the fully resolved version of the `log` augmentation | ||
available to it, down to the sha of what to check out. | ||
|
||
When Cargo rebuilds from this `Cargo.lock` it will not query the registry for | ||
versions of `log`, instead seeing that there's an exact dependency on the git | ||
repository (from the `Cargo.lock`) and the repository is listed as an | ||
augmentation, so it'll follow that pointer. | ||
|
||
## Impact on `[replace]` | ||
|
||
The `[augment]` section in the manifest can in many ways be seen as a "replace | ||
2.0". It is, in fact, strictly more expressive than the current `[replace]` | ||
section! For example these two sections are equivalent: | ||
|
||
```toml | ||
[replace] | ||
'log:0.3.7' = { git = 'https://github.com/rust-lang-nursery/log' } | ||
|
||
# is the same as... | ||
|
||
[augment.crates-io] | ||
log = { git = 'https://github.com/rust-lang-nursery/log' } | ||
``` | ||
|
||
This is not accidental! The intial development of the `[augment]` feature was | ||
actually focused on prepublishing dependencies and was called `[prepublish]`, | ||
but while discussing it a conclusion was reached that `[prepublish]` already | ||
allowed replacing existing versions in a registry, but issued a warning when | ||
doing so. It turned out that without a warning we ended up having a full-on | ||
`[replace]` replacement! | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah this sentence makes me think the discussion of warnings on line 189 was supposed to have been removed... |
||
|
||
At this time, though, it is not planned to deprecate the `[replace]` section, | ||
nor remove it. After the `[augment]` section is implemented, if it ends up | ||
working out this may change. If after a few cycles on stable the `[augment]` | ||
section seems to be working well we can issue an official deprecation for | ||
`[replace]`, printing a warning if it's still used. | ||
|
||
Documentation, however, will immediately being to recommend `[augment]` over | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. changed* |
||
`[replace]`. | ||
|
||
# How We Teach This | ||
[how-we-teach-this]: #how-we-teach-this | ||
|
||
Augmentation is a feature intended for large-scale projects spanning many repos | ||
and crates, where you want to make something like an atomic change across the | ||
repos. As such, it should likely be explained in a dedicated section for | ||
large-scale Cargo usage, which would also include build system integration and | ||
other related topics. | ||
|
||
The mechanism itself is straightforward enough that a handful of examples (as in | ||
this RFC) is generally enough to explain it. In the docs, these examples should | ||
be spelled out in greater detail. | ||
|
||
Most notably, however, the [overriding dependenices][over] section of Cargo's | ||
documentation will be rewritten to primarily mention `[augment]`, but | ||
`[replace]` will be mentioned still with a recommendation to use `[augment]` | ||
instead if possible. | ||
|
||
[over]: http://doc.crates.io/specifying-dependencies.html#overriding-dependencies | ||
|
||
# Drawbacks | ||
[drawbacks]: #drawbacks | ||
|
||
This feature adds yet another knob around where, exactly, Cargo is getting its | ||
source and version information. In particular, it's basically deprecating | ||
`[replace]` if it works out, and it's typically a shame to deprecate major | ||
stable features. | ||
|
||
Fortunately, because these features are intended to be relatively rarely used, | ||
checked in even more rarely, are only used for very large projects, and cannot | ||
be published to crates.io, the knobs are largely invisible to the vast majority | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct me if I'm wrong, but I think this is the first point in the RFC now that mentions not being able to publish crates that use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm so I meant to remove this but it's actually an interesting discussion point. Should crates with @carols10cents do you have an opinion one way or the other about whether crates with augments should be banned from publication? Note that the augment sections will always be ignored regardless. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If
Uhhh what about with binary crates and how Cargo.lock interacts with cargo install? rust-lang/cargo#2263 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah this is one possible use case I see for not denying publications with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO, we should rule out There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was there a decision made here? I don't see why (at least) path-based patches shouldn't be allowed. This would allow me to resolve rwf2/Rocket#513, for example. |
||
of Cargo users, who are unaffected by them. | ||
|
||
# Alternatives | ||
[alternatives]: #alternatives | ||
|
||
The primary alternative for addressing the motivation of this RFC would be to | ||
loosen the restrictions around `[replace]`, allowing it to arbitrarily change | ||
the version of the crate being replaced. | ||
|
||
As explained in the motivation section, however, such an approach does not fully | ||
address the desired workflow, for a few reasons: | ||
|
||
- It does not make it possible to track which crates in the dependency graph | ||
have successfully upgraded to a new major version of the replaced dependency, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How do we track which crates in the dependency graph have successfully upgraded to a new major version of the replaced dependency using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes this would be determined through Cargo.lock. The lock file will show a dependency on a crate added through I'll look to expanding this section. |
||
which could have the effect of masking important *behavioral* breaking changes | ||
(that still allow the crates to compile). | ||
|
||
- It does not provide an easy-to-understand picture of what the crates will look | ||
like after the relevant dependencies are published. In particular, you can't | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, even with Am I just being a bit too literal in my interpretation of "will" here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I think the motivation of this example is basically "replace doesn't work for Servo's workflow of publishing changes", and this was an attempt to articulate that. In that sense "will" is "will likely" most likely, but I'll try to clarify. |
||
use the usual resolution algorithm to understand what's going on with version | ||
resolution. | ||
|
||
- It does not provide any warning when the replaced crate has actually been | ||
published to the index, which could lead to silent divergences depending on | ||
which root crate you're compiling. | ||
|
||
# Unresolved questions | ||
[unresolved]: #unresolved-questions | ||
|
||
- It would be extremely helpful to provide a first-class workflow for forking a | ||
dependency and making the necessary changes to Cargo.toml for prepublication, | ||
and for fixing things up when publication actually occurs. That shouldn't be | ||
hard to do, but is out of scope for this RFC. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My brain is having trouble parsing this sentence, and I'm not sure what it's saying:
I think this is supposed to expand out to mean:
Or perhaps:
The less likely but still possible reading is something like:
due to the ambiguous subject of the second clause and the verbs that don't quite match...