On Wed, Feb 8, 2017 at 2:28 AM, Anthony Ramine <n.ox...@gmail.com> wrote:
> Could you please push the code you wanted to test on your GH's fork, > so we can at least see what prompted this discussion? > I haven't split it out yet, because I'm still hoping that the outcome of this discussion is that I won't have to. The unified code changes are in part 3 of https://bugzilla.mozilla.org/show_bug.cgi?id=1336646 On Wed, Feb 8, 2017 at 3:39 AM, Simon Sapin <simon.sa...@exyr.org> wrote: > It’s a trade-off. > Totally. My request here is that we evaluate this tradeoff with some consideration for the realities of the crate in question. I'm totally happy to bias towards "separate repo" if the tradeoff is at all unclear. > > It’s true that historically we’ve been more eager than necessary to put > every crate going to crates.io in its own repository. (See for example > github.com/servo/webrender_traits, since then merged into webrender.) > > I understand that juggling multiple repositories makes things complicated > for Servo contributors. But I think that it’s not too terrible once you > figure out the workflow, and that we can improve it both with tooling (in > Cargo replacements/overrides support) and in our own habits/conventions. > On the other hand, keeping / moving back a library in servo/servo that is > used outside makes life more difficult for contributors Right. It makes life harder for some people and easier for others. But if we evaluate the aggregate pain on both sides, I posit that the tradeoff _thus far_ has not been worth it (see below about future contributions). > since they need to clone/checkout a much larger repository, CI takes much > longer, is more prone to unrelated intermittent failures, and the CI queue > can be more busy. > Won't they need to do this anyway? At least assuming we accept your proposal below that: "Whenever a PR to rust-selectors (and other repositories where we think that’s appropriate) makes breaking changes, we don’t land it until there’s also a corresponding PR to update Servo for these changes." > (Though this has gotten better now that we don’t need anymore to retry > more often than not because of intermittent.) > > On 07/02/17 22:47, Bobby Holley wrote: > >> rust-selectors was split out into a separate crate and repository in 2015. >> Since then, there has been one push (with 2 commits) by a contributor that >> does not also contribute to servo >> > > But we don’t know how many people will want to contribute in the future, > and how many would be discouraged if it’s in servo/servo. > We have two years of historical data. That does not predict the future, it's at least some indication of the volume we might expect in the near term. I also think that even if the direct benefits in this particular case are > small, doing this is part of being a "good citizen" in the Rust ecosystem. > > > But now I'm ready to land these changes, and things get tricky. The latest >> version of rust-selectors is several breaking versions ahead of the one >> used by servo, so now I need to either do these updates for code changes >> I'm not familiar with, or block my patch indefinitely until someone else >> does it: I can't make progress until that "lock" is released. >> > > In this case, servo/rust-selectors had two breaking changes ahead of Servo: > > * One added variants to a public enum. Since Servo doesn’t match values of > that enum, no change at all was required. > > * The other changed a parameter in a method of a public trait from Foo to > &Foo. The fix was to add & in a few places in Servo. > There were also text expectations that needed adjustment. > But yes, you couldn’t know how much effort this update would take until > you tried it. You could have asked, though. We have people in multiple time > zones who could help with this, some of whom reviewed the relevant changes. > I had a feeling that it might be more than I bargained for. And was I wrong? It took 2 people 6 attempts (spread across 2 PRs) and 9 hours to get it landed. Anthony and Keith were heroic, but that is certainly a lot of friction just to put the tree in a state where I can start trying to land my patch. > > The reason the update was not in Servo yet was that I wanted to land it > with tests that are not written yet. It is landed now (with an open issue > for the tests). Thanks to Nox and KiChjang for doing it while I was > sleeping. > > > So here is a proposal to avoid this situation in the future: > > Whenever a PR to rust-selectors (and other repositories where we think > that’s appropriate) makes breaking changes, we don’t land it until there’s > also a corresponding PR to update Servo for these changes. > > What do you think? > Per above, it seems like if we agree that Servo is a tier-1 consumer of rust-selectors, then contributors will need to figure out how to pull Servo anyway (and do more work once they do!). > > > And then I still want to run my combined (servo+rust-selectors) >> changes through servo try before landing and publishing the >> rust-selectors change, which means that I need to go push a special >> branch, >> > > You’ll need a branch anyway to make a PR. > > > and fuss with with the cargo manifests to reference the temporary >> branch, and then push that to servo/servo, and then trigger a try >> push, >> > > I think we can and should improve tooling here. One command to add a > replacement/override, one command to start a try build. > > > [...] and then publish to crates.io, [...] >> > > I think we should make Travis-CI publish to crates.io when we merge a > version number change to master. > These would reduce some of the friction, but not the overall problem of coordinating interdependent changes with a separate repository. Let's take a step back here. I think the crates.io ecosystem is wonderful, and am not trying to undermine it. I voluntarily split out the atomic_refcell code I wrote from the style system into a separate crate and git repo because I thought that was the right thing to do. I also have a lot on my plate and a deadline breathing down my neck. I knew this discussion would take time, but I chose to bring it up because the rust-selectors separation is _such_ a point of friction for me that I honestly believe it will save me time overall. I understand that other people don't mind this friction as much as I do. But please consider that it causes me (and some others) a lot of pain. Here's a compromise proposal: Right now the Servo style system is under intense development by a lot of Servo hackers because we're trying to ship stylo (and make the style system production-grade in the process). That means we have a large quantity of known pain on the horizon, but that it will taper off at some point in the future (hopefully before the end of the year). What about landing #15447 for now, and then potentially undoing it once stylo development settles down? I will personally commit to moving it back when that happens, assuming that's what Simon and the rest of the community decide is best. bholley > -- > Simon Sapin > > _______________________________________________ > dev-servo mailing list > dev-servo@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-servo > _______________________________________________ dev-servo mailing list dev-servo@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-servo
