Proposal: move the source code for rust-selectors into servo/servo

[Bobby Holley]

To be clear, I support the continued presence of rust-selectors on crates.io.
My proposal here applies only to the canonical repository from which the
publish happens - regular consumers of rust-selectors would not notice a
difference. Anyone _contributing_ to rust-selectors would need to write a
PR against servo/servo instead of servo/rust-selectors, which means their
change would run against servo's CI (which is probably a good thing).

So far, the guiding principle for crate modularity has been "if there is a
non-Servo consumer of the crate, publish it on crates.io and split the code
into a separate repository". I think the first part of this is great. I
think we have gone slightly too far on the second, specifically in the case
of rust-selectors.

Giving crates a dedicated repository reduces friction for non-Servo
contributors, which is a good thing. But it can also add a lot of friction
for Servo contributors, which is not good. There's an inherent tradeoff,
and while I generally support a bias towards being contributor-friendly, I
think there are some cases where the tradeoff doesn't make sense.

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 [1]. In that time, there have also been
268 other commits by Servo contributors, and 39 crate publishes, each of
which requires futzing around with Cargo dependencies, and many of which
require extra coordination for breaking changes.

Here's a run-of-the-mill story of how this costs us:

This past weekend, I decided to quickly hack up a fix to an architectural
issue we have with selector matching and DOM mutations. I normally avoid
fixing bugs that require touching rust-selectors (for the reasons described
below), but since I was working on the special stylo incubator repository
(which happens to have a vendored copy of rust-selectors), I decided to try
it.

The development experience was great: I was able to iterate easily on my
changes, including multiple interdependent changes to servo and
rust-selectors across several iterations of code review. I was also able to
perform try pushes (on stylo builds) with a single command.

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. 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, 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, and then open a PR on
rust-selectors, and then land it there, and then publish to crates.io, and
then land my servo PR.

This is super painful, and a large disincentive for me to make
rust-selectors better - instead, it incentives me to hack around issues at
the callsites whenever possible. I don't think that's a good thing.

bholley

[1] https://github.com/servo/rust-selectors/graphs/contributors , see
marvelm

[Anthony Ramine]

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?

Regards.

> Le 7 févr. 2017 à 22:47, Bobby Holley <bobby...@gmail.com> a écrit :
>
> <snip/>

[Simon Sapin]

It’s a trade-off.

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 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.

(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.

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.

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.

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?

> 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.


--
Simon Sapin

[David Bruant]

Le 08/02/2017 à 12:39, Simon Sapin a écrit :
> 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.

There is a solution for this sort of problem in the npm ecosystem :
https://greenkeeper.io/

It's a bot that you hook into your project. It finds the dependencies
and whenever the dependency releases a new version, greenkeeper sends a
Pull Request like this one : https://github.com/sudweb/2015/pull/84
With CI, all the tests are run instantly so you know whether using the
new dependency will break something.

It only works on npm packages/repos, but it doesn't seem shockingly hard
to copy the design and apply to rust crates/repos.

This could be a solution to the problem "there is a new dependency
version, let's open a PR about it" and "and 39 crate publishes, each of
which requires futzing around with Cargo dependencies". The bot would be
doing all this.

David

[Bobby Holley]

On Wed, Feb 8, 2017 at 2:28 AM, Anthony Ramine <n.o...@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...@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-...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-servo
>

[Simon Sapin]

On 08/02/17 19:42, Bobby Holley wrote:
> 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."

No. If a contributor makes a change that is breaking, I or someone else
on the Servo team would take care of updating Servo for them.

> 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 agree with your cost / benefit analysis. But as I mentioned before, my
reluctance is also based on principle.

> But please consider that it causes me (and some others) a lot of pain.

I believe you, but to be honest I don’t understand. (To me, contributing
to Gecko also requires jumping through a number of (different) hoops.)


I’m inclined to concede, but I’d like to hear from other people before
hitting the button.

--
Simon Sapin

[Jack Moffitt]

Let's allow until Friday afternoon for feedback to come in, but I
don't think it makes sense to delay this decision.

jack.

[Simon Sapin]

On 08/02/17 22:31, Jack Moffitt wrote:
> Let's allow until Friday afternoon for feedback to come in, but I
> don't think it makes sense to delay this decision.

I approved https://github.com/servo/servo/pull/15447 and it has already
landed. Oh well.

I’ve pushed a new branch to https://github.com/servo/rust-selectors and
made it the default. It only has a readme file that says the source is
now at https://github.com/servo/servo/tree/master/components/selectors.
The master branch and issue / PR tracker is still there if we need it.

--
Simon Sapin

[Anthony Ramine]

I'm not against the decision if we are willing to revert it in the future (which is why I didn't reply immediately following your reply), but I have a few things to say nonetheless.

> Le 8 févr. 2017 à 19:42, Bobby Holley <bobby...@gmail.com> a écrit :
>
> On Wed, Feb 8, 2017 at 2:28 AM, Anthony Ramine <n.o...@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

How does the discussion mean you don't have to split? All I see is three patches, none of them making it obvious how the selectors API changed. This also means that the commit messages will never ever be written from the POV of selectors, and always from the POV of servo, thus quite limiting any lingering hope to have external contributors now, given it will not be as discoverable as before what is happening in the crate at the commit level.

In general, I posit that Mozilla completely, utterly, lost against V8 in the embedded department in a huge part because of this mono-repository thing where everything is muddled together.

We can't quantify the missed opportunities when merging things together in a single repository.

> On Wed, Feb 8, 2017 at 3:39 AM, Simon Sapin <simon...@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."

As Simon says in his own reply, no we never block people because they did a breaking change that requires action on Servo's side. That's the main reason why things are split IMO. Servo shouldn't be a special burden for unrelated contributors.

>> (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.

Which you could have updated yourself, after the first failing CI pass.

>> 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.

Please don't call that heroic, that's just me doing my job. I believe such janitoring is a mandatory and crucial part of doing FOSS, and it's beyond me why people would rather do cross-project commits, where the commit messages often don't make sense when considering one of the moving parts in isolation. But well, YMMV.

Also, there is nothing fancy about "spread across 2 PRs", given how easy it is to make a PR (as opposed to doing stuff in m-c and bugzilla), and KiChjang could have just amended mine instead of making a new one altogether.

All in all, I spent like 15 minutes on that, the rest was CI doing its own time-consuming job and KiChjang changing the expectations on his own PR.

Regards.

[Jan de Mooij]

On Thu, Feb 9, 2017 at 10:51 AM, Anthony Ramine <n.o...@gmail.com> wrote:

> In general, I posit that Mozilla completely, utterly, lost against V8 in
> the embedded department in a huge part because of this mono-repository
> thing where everything is muddled together.
>

(Sorry for going offtopic, but I don't think it's that black and white. We
do/did have standalone SpiderMonkey source releases for instance. I think a
bigger problem was that we had an API from the C era that was (and still
is) hard to use correctly (with no or outdated documentation) and it
changed all the time, while V8 started from scratch with a cleaner C++ API
and had the "new, cool, most advanced JS engine" marketing.)

Jan

[Jan-Erik Rediger]

I do know that there was once a discussion between the folks behind
greenkeeper.io and Rustaceans to support Rust/crates.io as well.

I can check with those people to see if that went anywhere or what would
be needed to make it happen.

On Wed, Feb 08, 2017 at 06:23:47PM +0100, David Bruant wrote:
> Le 08/02/2017 à 12:39, Simon Sapin a écrit :

> > 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.

> There is a solution for this sort of problem in the npm ecosystem :
> https://greenkeeper.io/
>
> It's a bot that you hook into your project. It finds the dependencies and
> whenever the dependency releases a new version, greenkeeper sends a Pull
> Request like this one : https://github.com/sudweb/2015/pull/84
> With CI, all the tests are run instantly so you know whether using the new
> dependency will break something.
>
> It only works on npm packages/repos, but it doesn't seem shockingly hard to
> copy the design and apply to rust crates/repos.
>
> This could be a solution to the problem "there is a new dependency version,
> let's open a PR about it" and "and 39 crate publishes, each of
> which requires futzing around with Cargo dependencies". The bot would be
> doing all this.
>
> David

[James Graham]

On 09/02/17 09:51, Anthony Ramine wrote:

>> 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
>
> How does the discussion mean you don't have to split? All I see is
> three patches, none of them making it obvious how the selectors API
> changed. This also means that the commit messages will never ever be
> written from the POV of selectors, and always from the POV of servo,
> thus quite limiting any lingering hope to have external contributors
> now, given it will not be as discoverable as before what is happening
> in the crate at the commit level.
>

> In general, I posit that Mozilla completely, utterly, lost against V8
> in the embedded department in a huge part because of this
> mono-repository thing where everything is muddled together.
>

> We can't quantify the missed opportunities when merging things
> together in a single repository.

I don't really have any skin in this game, but I will note that the
web-platform-tests have been much more successful since we allowed
people to use them in the same repository as their browser code, even
though it does mean we get a lot of commits with useless commit messages
like "tests". So it's not always the case that the unquantifiable
benefits of multiple repositories always outweigh the unquantifiable
benefits of a single (from the point of view of a specific group of
developers working on a larger project) repository.

[Michael Howell]

WPT still has its own repo; the Servo repo just does an occasional
bidirectional sync. The equivalent for rust-selectors would be if the
rust-selectors repo was kept and just occasionally synced with Servo (like
Servo is doing for M-C, anyway).

[James Graham]

On 09/02/17 16:18, Michael Howell wrote:
> WPT still has its own repo; the Servo repo just does an occasional
> bidirectional sync. The equivalent for rust-selectors would be if the
> rust-selectors repo was kept and just occasionally synced with Servo (like
> Servo is doing for M-C, anyway).

Sure, but my point was that making the workflow for regular contributors
as painless as possible (by allowing them to work in a "monorepo" setup)
vastly increased the number of contributions we got and was a huge win
for the project. Of course the specifics of wpt demand a complex 2 way
sync architecture to support that, since there are multiple downstream
projects using the tests.

[Michael Howell]

Could we do the same thing for rust-selectors? And should we?

[Jack Moffitt]

> In general, I posit that Mozilla completely, utterly, lost against V8 in the embedded department in a huge part because of this mono-repository thing where everything is muddled together.

I don't think anyone is disagreeing with you on this point. The plan
is as it always has been - to split out anything sensible to its own
repo and publish to crates.io.

The key traits of rust-selectors leading to this exception are that it
was split out for 2 years with little community involvement in
development (but a reasonable amount of usage) and (we hope temporary)
high synchronization burden for the active developers.

We should continue to split out things as the default, since we have
no way to know which things will spawn healthy communities a priori.
This is being proposed as an exception to our policy given some unique
circumstances.

> As Simon says in his own reply, no we never block people because they did a breaking change that requires action on Servo's side. That's the main reason why things are split IMO. Servo shouldn't be a special burden for unrelated contributors.

You are correct to point out that this will change for rust-selectors
when it is in-tree. The practical consequence based on current data is
basically zero, since all active developers are working with the
must-be-in-sync-with-servo-and-gecko rule already.

jack.

[Jack Moffitt]

I think the complexity of the wpt-style setup is overkill, but I think
we could do something to alleviate the community participation issue.
We could perhaps keep the rust-selectors repo around and following the
Servo in-tree one. PRs could still be done by external contributors
the rust-selectors repo, and the current maintainers could merge those
to Servo on their behalf and manually close the PRs on rust-selectors.
Xiph.org does this for its repos as they are hosted on git.xiph.org
but they allow PRs on GitHub.

This requires someone to write a script that will take the correct
history and make new commits in rust-selectors periodically, but I
think this would solve the expected minimal impact of this proposal.

This would also mean in the future we could just delete rust-selectors
from inside Servo without any reverse move required.

jack.

[Gregory Szorc]

> On Feb 9, 2017, at 01:51, Anthony Ramine <n.o...@gmail.com> wrote:
>
> I'm not against the decision if we are willing to revert it in the future (which is why I didn't reply immediately following your reply), but I have a few things to say nonetheless.
>
>> Le 8 févr. 2017 à 19:42, Bobby Holley <bobby...@gmail.com> a écrit :
>>
>> On Wed, Feb 8, 2017 at 2:28 AM, Anthony Ramine <n.o...@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
>
> How does the discussion mean you don't have to split? All I see is three patches, none of them making it obvious how the selectors API changed. This also means that the commit messages will never ever be written from the POV of selectors, and always from the POV of servo, thus quite limiting any lingering hope to have external contributors now, given it will not be as discoverable as before what is happening in the crate at the commit level.
>

> In general, I posit that Mozilla completely, utterly, lost against V8 in the embedded department in a huge part because of this mono-repository thing where everything is muddled together.
>

> We can't quantify the missed opportunities when merging things together in a single repository.

A monorepo does not intrinsically infringe on software freedoms and flexibility: engineering culture/decisions and (deficient) tools do.

The main reasonable concerns against a monorepo from where I sit are that the VCS and tools around it don't scale or handle the scenario better. We can address this somewhat by exporting/importing directories between a monorepo and standalone repositories. This is the model we're using with Servo in the Firefox repo. That handles the VCS scaling problems. There are still some issues around keeping things in sync (single writable master is critical to avoid divergence) and workflows around things like history rewriting on Pull Requests as part of landing. But these can also be mitigated (and aren't unique to monorepos).

>
>>> On Wed, Feb 8, 2017 at 3:39 AM, Simon Sapin <simon...@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."
>

> As Simon says in his own reply, no we never block people because they did a breaking change that requires action on Servo's side. That's the main reason why things are split IMO. Servo shouldn't be a special burden for unrelated contributors.
>

[Jack Moffitt]

To followup, the core team is in favor of this change, and since it
already landed, there's nothing more to do.

I do think it's useful to decide what to do about the old repo. Do we
put a big sign on it pointing to servo/servo? Or do we keep up synced
to take PRs?

jack.

[Bobby Holley]

Simon took the "big sign" approach, which seems fine to me:
https://github.com/servo/rust-selectors

In a few months, gps' tooling may give us the ability to keep the separate
repo synced up automatically, which would let us have our cake and eat it
too.

bholley

[Simon Sapin]

On 11/02/17 01:09, Bobby Holley wrote:
> Simon took the "big sign" approach, which seems fine to me:
> https://github.com/servo/rust-selectors

Yes, as mentioned in another part of this thread:

I’ve pushed a new branch to https://github.com/servo/rust-selectors and
made it the default. It only has a readme file that says the source is
now at https://github.com/servo/servo/tree/master/components/selectors.

The master branch and issue / PR tracker are still there if we need them.

--
Simon Sapin