Re: [dev-servo] Removing shared boxes from the DOM

[Bobby Holley]

To clarify, this stuff doesn't use the new inheritance stuff that's going
into Rust, right? I assume that stuff isn't done yet?


On Thu, Dec 5, 2013 at 9:05 AM, Josh Matthews  wrote:

> For those who just want to see a before/after summary, here's an example.
>
> BEFORE: we have to handwrite conversion routines for each downcast or
> upcast we want to use. Furthermore, we have the ugly
> AbstractDocument/AbstractNode/AbstractEvent/AbstractEventTarget types, in
> addition to all of the concrete @mut SomeDOMType that are scattered
> everywhere.
>
> impl AbstractNode {
>  /// Allow consumers to upcast from derived classes.
>  pub fn from_document(doc: AbstractDocument) -> AbstractNode {
>  unsafe {
>  cast::transmute(doc)
>  }
>  }
>
>  pub fn from_eventtarget(target: AbstractEventTarget) ->
> tractNode {
>  assert!(target.is_node());
>  unsafe {
>  cast::transmute(target)
>  }
>  }
> }
>
> 788 let doctarget = AbstractEventTarget::from_document(document);
> 789 let wintarget = AbstractEventTarget::from_window(window);
> 790 window.eventtarget.dispatch_event_with_target(wintarget,
> Some(doctarget), event);
>
> 307 if child.is_element() {
> 308 do child.with_imm_element |elem| {
> 309 if callback(elem) {
> 310 elements.push(child);
> 311 }
> 312 }
> 313 }
>
>
> AFTER all casting methods are automatically generated; all that's left is
> writing the is_foo method and ensuring that we can tell the concrete type
> of a DOM object based on the root type in an inheritance chain:
>
> impl UIEventDerived for Event {
> fn is_uievent(&self) -> bool {
> self.type_id == UIEventTypeId
> }
> }
>
> let doctarget = EventTargetCast::from(document);
> let wintarget = EventTargetCast::from(window);
> window.eventtarget.dispatch_event_with_target(wintarget, Some(doctarget),
> event);
>
> if child.is_element() {
> let elem = ElementCast::to(child);
> if callback(elem) {
>   elements.push(elem);
> }
> }
>
> In my mind, the biggest improvement here is that we can actually have
> lists of JSManaged, whereas before we could only store
> ~[AbstractNode] with the handwave-y guarantee that all of the nodes should
> also be elements. I also find the new checked casts much nicer to read (and
> yes, the downcasts assert at runtime if the value passed is not an instance
> of the desired type).
>
> Cheers,
> Josh
>
>
> On 12/03/2013 03:07 AM, Josh Matthews wrote:
>
>> Ms2ger and I have been working on this on and off, and the Event
>> hierarchy is looking very nice so far:
>> https://github.com/jdm/servo/commits/jsmanaged . It even builds and
>> passes tests, so we should be able to continue converting this
>> piece-by-piece. There is an absolute minimum amount of boilerplate
>> required now, which is lovely.
>>
>> Cheers,
>> Josh
>>
>> On 11/28/2013 10:54 AM, Josh Matthews wrote:
>>
>>> I've finally got a sketch of my plans to remove all of the @mut
>>> annotations from Servo's DOM. You can see it at
>>> https://gist.github.com/jdm/7693770, but here's the breakdown of the
>>> improvements:
>>>
>>> * no more AbstractNode (\o/) - we can actually use JSManaged
>>> when we want to refer to something derived from Element now.
>>> * actually fulfils the contract that the SpiderMonkey GC owns the sole
>>> reference
>>> * no need to add as_foo methods for downcasting
>>>
>>> Breaking it down further, one of the biggest changes is in the
>>> implementation of downcasting and upcasting. Upcasting a JSManaged
>>> to a JSManaged is a statically-checked operation that should be
>>> free at runtime. This is enforced by a series of empty traits that each
>>> derived class much implement (ie. see EventTargetBase), such that any
>>> value passed to Bar::from() must be a type that implements BarBase. In a
>>> similar fashion, downcasting is also statically-checked (does the cast
>>> even make sense?), before performing a dynamic assertion (is this base
>>> class actually an instance of the derived type?). Therefore, when
>>> calling Foo::to(), the value passed must implement the FooDerived trait
>>> with an appropriate boolean is_foo method implemented.
>>>
>>> These casting changes may not look like a big improvement at first, but
>>> the important consideration is the the upcasting traits can be
>>> automatically generated completely from WebIDL definitions, so that
>>> boilerplate should be essentially free. For downcasting, each type that
>>> is on an inheritance chain will need to implement a single trait with a
>>> single boolean method that performs the required dynamic check; all
>>> other boilerplate can also be generated from WebIDL definitions, and is
>>> therefore essentially free.
>>>
>>> I welcome feedback about this sketch. For a qui

[dev-servo] loading web fonts synchronously during automation

[Bobby Holley]

Right now, every wpt test includes a stylesheet with an @font-face rule to
make the 'ahem' font available to tests. Since font loading triggers a
document-wide reflow, we end up with a non-deterministic network-driven
reflow in each and every test, which makes it harder to test layout
optimizations and increases the chances for intermittent bugs.

In [1] I'm planning to add a debug flag to load web fonts synchronously,
and set that flag in the wpt runner. Please let me know if anyone has any
objections.

Cheers,
bholley

[1] https://github.com/servo/servo/pull/8341
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] loading web fonts synchronously during automation

[Bobby Holley]

That's a great point - I'll take a look to see how doable it is.

On Wed, Nov 4, 2015 at 7:28 PM, L. David Baron  wrote:

> On Wednesday 2015-11-04 19:16 -0800, Bobby Holley wrote:
> > Right now, every wpt test includes a stylesheet with an @font-face rule
> to
> > make the 'ahem' font available to tests. Since font loading triggers a
> > document-wide reflow, we end up with a non-deterministic network-driven
> > reflow in each and every test, which makes it harder to test layout
> > optimizations and increases the chances for intermittent bugs.
> >
> > In [1] I'm planning to add a debug flag to load web fonts synchronously,
> > and set that flag in the wpt runner. Please let me know if anyone has any
> > objections.
>
> The font shouldn't even load unless it's needed (i.e., there's text
> that uses it).  I *think* that's the way things work across
> browsers, and allows inclusion of style sheets that provide a
> "library" of @font-face rules that may or may not be used.  That
> would avoid the reflow except where Ahem is actually used, which
> seems like the way things ought to work.
>
> -David
>
> --
> 𝄞   L. David Baron http://dbaron.org/   𝄂
> 𝄢   Mozilla  https://www.mozilla.org/   𝄂
>  Before I built a wall I'd ask to know
>  What I was walling in or walling out,
>  And to whom I was like to give offense.
>- Robert Frost, Mending Wall (1914)
>
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] loading web fonts synchronously during automation

[Bobby Holley]

Seems generally doable, but probably more than I want to chew right now.
I'll get an issue filed though.

Does anyone object to landing the synchronous font loading as a stopgap to
get better reftest coverage until we make the font loading logic smarter?

On Wed, Nov 4, 2015 at 8:14 PM, Bobby Holley  wrote:

> That's a great point - I'll take a look to see how doable it is.
>
>
> On Wed, Nov 4, 2015 at 7:28 PM, L. David Baron  wrote:
>
>> On Wednesday 2015-11-04 19:16 -0800, Bobby Holley wrote:
>> > Right now, every wpt test includes a stylesheet with an @font-face rule
>> to
>> > make the 'ahem' font available to tests. Since font loading triggers a
>> > document-wide reflow, we end up with a non-deterministic network-driven
>> > reflow in each and every test, which makes it harder to test layout
>> > optimizations and increases the chances for intermittent bugs.
>> >
>> > In [1] I'm planning to add a debug flag to load web fonts synchronously,
>> > and set that flag in the wpt runner. Please let me know if anyone has
>> any
>> > objections.
>>
>> The font shouldn't even load unless it's needed (i.e., there's text
>> that uses it).  I *think* that's the way things work across
>> browsers, and allows inclusion of style sheets that provide a
>> "library" of @font-face rules that may or may not be used.  That
>> would avoid the reflow except where Ahem is actually used, which
>> seems like the way things ought to work.
>>
>> -David
>>
>> --
>> 𝄞   L. David Baron http://dbaron.org/   𝄂
>> 𝄢   Mozilla  https://www.mozilla.org/   𝄂
>>  Before I built a wall I'd ask to know
>>  What I was walling in or walling out,
>>  And to whom I was like to give offense.
>>- Robert Frost, Mending Wall (1914)
>>
>
>
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] PSA: Review delegation enabled for homu

[Bobby Holley]

Thanks for working on this Manish!

As mentioned previously, I think that scoping this bit of trust to the PR
is sub-optimal. This kind of delegation is effectively unsupervised write
access to the repository, with an understanding that the committer is
trustworthy enough to ask for review on any significant not-yet-approved
changes. It doesn't really have anything to do with the particular PR, and
has everything to do with the person being granted that access.

This kind of trust is a governance issue, and making it a per-reviewer
per-PR preference introduces the following problems:
* The committer needs to establish the trust relationship with each
individual reviewer, which doesn't scale for either party.
* Reviewers need to make an important trust decision for every PR, and may
not always make decisions that are consistent with what they and others
have decided previously.
* Reviewers may not always remember to set the delegate flag.

The combination of all three of the above means that, in practice, a
contributor is likely to find themselves with carry privileges in some
situations but not others. Is that ever desirable?

bholley

On Wed, Nov 11, 2015 at 3:01 AM, Manish Goregaokar 
wrote:

> After some
> 
> discussion
> <
> http://logs.glob.uno/?c=mozilla%23servo&s=10+Nov+2015&e=10+Nov+2015&h=delegate#c298415
> >
> I've landed  and enabled review
> delegation for homu.
>
> This means that any homu command containing `delegate=USER` will give that
> user full reviewer status for the scope of that PR (i.e., they can use all
> homu commands for that PR). You can also use `delegate+` as a shortcut to
> delegate to the PR author. `delegate-` undoes delegation.
>
> Reviewers: In case of "r=me with minor nit" situations (where the author is
> trusted enough to not add our infra to a botnet or whatever), try to use
> `@bors-servo delegate+` so that the author can finish up the PR on their
> own. This should ease up the workflow for non-reviewers.
>
> You can see it in action here .
>
> Thanks,
> -Manish Goregaokar
> ___
> 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

Re: [dev-servo] PSA: Review delegation enabled for homu

[Bobby Holley]

On Wed, Nov 11, 2015 at 4:22 PM, Lars Bergstrom 
wrote:

> I've been assuming the only time that we would use `delegate` is for
> people like you (Bobby), whom we trust, but who have not gone through
> the process to become a Servo reviewer.


Sure. From a personal standpoint, my ask here is to make that decision
explicit and global, rather than leaving it per-(reviewer, PR), because I
want to remove these (minor) barriers to my productivity.

>From a project standpoint, I am trying to blaze the trail of the
committer-but-not-reviewer role, because I think it can be a useful one -
for example, Ms2ger did a ton of great work on Gecko in this capacity back
in the day. I have a personal preference for not doing a bunch of Servo
reviews right now (mostly related to the 6-8 Gecko modules where I'm
currently shirking my review obligations). And while I'm sure I could
review a few token patches and cajole people into making me a reviewer, I'd
rather advocate to improve support for this role and remove barriers that I
think are unnecessary. If that fails, I'll probably resort to cajoling. ;-)


> Honestly, I'd rather not have
> this feature at all --- I would prefer that a reviewer (even if it's
> the PR author) at least glance at all squashes / rebases in Reviewable
> to verify that nothing slipped in, because we all mess up `git rebase`
> from time to time.
>

This suggests that you trust Servo Reviewers to self-check against
mis-merges, but do not trust anyone else. I think that is unnecessarily
restrictive, and that there is a class of contributors (like myself, the
Ms2ger-of-old, and the hypothetical RyanVM-of-Servo) who can qualify for
the "merge-er" badge without qualifying for the "reviewer" badge.


> But, until we have closer to 24/7 reviewer
> coverage, like Rust does, I was in favor of adding this support so
> that you are not blocked during the 1--2AM US Pacific slot that
> appears to be both the "deadzone" for our reviewer coverage and your
> prime patch-landing time :-)
>

To be clear, #servo support is phenomenal, and Manish is basically awake
all the time anyway. It is nevertheless frustrating to be required to ask
for help to complete basic tasks when that requirement appears
ill-conceived. Imagine that you needed to ping someone in order to, say,
clobber your objdir - that restriction would not likely be a major barrier
to practical development (since Manish is always there, and you don't
clobber your objdir _that_ often), but it would still feel annoying and
unnecessary.

Put another way: I already have access to the green merge button, and
therefore write access to the repo. I don't use it because I know I'm
supposed to go through bors-servo instead, which is why it feels silly that
bors-servo is programmed to treat me as untrusted.

I haven't been a fan of "commit" access in the past because the Servo
> process tends to focus on doing a lot of before-commit work (running
> the full test suite, doing full reviews, etc.) to avoid the need to
> ever close tree or back anything out.


The bors model certainly obviates the need for manual landings (c/f the
disused green merge button). But it also creates an arbitrarily-long window
between "submitted for landing" and "landed" during which new merge
conflicts may arise, and puts the burden of resolving trivial merge
conflicts on the contributor (rather than any sort of sheriff figure). This
increases number of times that a patch author needs to rebase, and
consequently the value of being able to rebase without round-tripping
through IRC.


> But, I can definitely see both
> your argument and have heard from other open source people (especially
> consultants and employees at BigCos) that having a "commit" level of
> access that's given out much more freely is a nice thing for them to
> be able to show to their bosses or use to market themselves, at least
> in the WebKit and Blink worlds.
>

FWIW, I think that's a somewhat-unfortunate overloading that we probably
shouldn't be optimizing for.

bholley


> - Lars
>
>
>
> On Wed, Nov 11, 2015 at 5:47 PM, Bobby Holley 
> wrote:
> > Thanks for working on this Manish!
> >
> > As mentioned previously, I think that scoping this bit of trust to the PR
> > is sub-optimal. This kind of delegation is effectively unsupervised write
> > access to the repository, with an understanding that the committer is
> > trustworthy enough to ask for review on any significant not-yet-approved
> > changes. It doesn't really have anything to do with the particular PR,
> and
> > has everything to do with the person being granted that access.
> >
> > This kind of trust is a governance issue, and making it a per-reviewer
> &g

Re: [dev-servo] PSA: Review delegation enabled for homu

[Bobby Holley]

On Wed, Nov 11, 2015 at 10:59 PM, Nicholas Nethercote <
n.netherc...@gmail.com> wrote:

> On the other hand, I'm more optimistic than bholley about the per-bug
> delegation. Given that it's been implemented I'd suggest using it and
> if reviewers do tend to forget it then consider implementing a broader
> mechanism.
>


To be clear: I think it would probably work ok, but still worse than the
alternative I'm proposing (which requires no additional engineering), and I
don't see how it is advantageous, (since, per my original email, it seems
like we should give someone delegation in any given PR i.f.f. we should
give them delegation in every PR).
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] PSA: Review delegation enabled for homu

[Bobby Holley]

On Thu, Nov 12, 2015 at 10:32 PM, Manish Goregaokar 
wrote:

> There are actually two kinds of trust involved. First is the trust to not
> botnet the CI, basically
> everyone here (at least everyone with try) has that.
>

I think there's an important trust distinction between try and core repo
access - try lets you botnet the CI, but that is a very intentional,
detectable, and low-stakes compared with intentionally or unintentionally
introducing a security vulnerability or architectural flaw in the core
engine.

But I agree with you that this isn't the distinction that matters so much
here.

But that's not the actual one being discussed
> here. The second is not really a form of "trust", rather it's the level of
> familiarity involved in
> the codebase to know the areas of code where small changes may cause big
> problems.
>

Or being experienced enough to know what you don't know. Familiarity with
the code matters, but I think judgement and sensitivity to social norms
probably matter more.


> For example, I
> would probably ask for re-review on rebases of my own changes to layout
> code or bindings code. Often
> a PR is to an area of code where stuff is straightforward and there's very
> little chances for things
> to mess up, which is where delegate+ comes in. Other times it may be in an
> area which some things
> need to be done precisely, and a rebase needs review -- but it's not
> always possible to tell these
> areas apart at first glance. Reviewers have the "trust" that they know
> when review-carry is
> appropriate. The debate boils down to whether or not everyone has this
> kind of "trust".
>

Exactly. I think that limiting this trust to reviewers is too restrictive
in practice, costing us more than we realize and buying us less than we
think. But I think those arguments have been reasonably-well-fleshed-out,
so I won't belabor them here. :-)

Cheers,
bholley
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] Weird build error

[Bobby Holley]

(Yoric is the same person as David R-T)
On Dec 6, 2015 7:25 PM, "Corey Farwell"  wrote:

> Is your version of Python installed via MacPorts by chance? yoric had an
> error that was identical to this today and they use MacPorts.
> ___
> 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

Re: [dev-servo] Using Cargo in Gecko

[Bobby Holley]

I had a long conversation about the two-way sync stuff at Orlando with Alex
and some of the Servo folks.

We concluded that, long-term, we need to let Gecko hackers commit to the
vendored code and have that code automatically upstreamed. Similarly,
upstream fixes need to be automatically merged into Gecko.

Everyone seemed on board with this plan, but Alex suggested that it should
probably be a separate tool, distinct from Cargo. This tooling is going to
be pretty important, but we should probably wait on it until we've got the
other issues sorted out.

bholley

On Thu, Dec 17, 2015 at 3:04 PM, Valentin Gosu 
wrote:

> Hi folks,
>
> I just wanted to share some notes on using Cargo in Gecko. One of the
> requirements was vendoring packages in tree. It turns out I was able to do
> that with a .cargo/config file in the scope of the rust directory, which
> defines the paths for dependencies, and [http] timeout = 0, which prevents
> downloading any resources from crates.io. [1]
> Better support for this use case would be welcome, such as making sure no
> network connections are made (the index still gets refreshed), but this is
> good enough for what we need at the moment.
>
> I'm not sure if we've agreed to have a top level directory for all of the
> rust crates, but I think it makes sense. It allows for easier sharing of
> crates (between necko and media for example).
> From what I understand servo's mach has a command for two-way sync between
> web-platform-tests and upstream. This would be a nice feature for cargo, as
> most of our dependencies would come from crates.io. So identifying the
> upstream repo, and doing a diff would be super awesome - probably for other
> projects as well.
>
> I'll probably have more to share once/if I manage to use cargo withing
> Gecko's build system.
>
> Thanks!
>
> [1] https://github.com/rust-lang/cargo/issues/1926
> ___
> 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

Re: [dev-servo] Using Cargo in Gecko

[Bobby Holley]

On Thu, Dec 17, 2015 at 5:08 PM, Lars Bergstrom 
wrote:

> - How do we handle commit access? Having the upstream maintainers
> (none of whom might be current Gecko peers) correctly identified and
> doing reviews in bugzilla to ensure m-c pushes can be upstreamed seems
> challenging.
>

Auto-upstreaming+downstreaming would only work if the set of peers of the
vendored version is identical to that of the upstream canonical repo. It's
easy enough to implement this on the Gecko side (you must be a peer of the
upstream code in order to be a peer in this vendored directory (we can
enforce this with a commit hook)). On the upstream side, it more or less
requires the peers to have an understanding of how the code is used in
Gecko, and to commit to maintaining it as a Tier-1 platform. If those
things are unfeasible, we'd need to fork.


> - What about conflicts? If even half of Servo's dependency graph is
> pulled in, that will be ~30 different repos, and the odds that some of
> them will temporarily be incompatible with others means that a m-c
> commit may be against an older version of upstream and not able to be
> automatically upstreamed into master. Does everything vendored in m-c
> get a private fork in the mozilla GH organization and get code and
> policy conflicts resolved there?
>

We would probably need to partition the set of things that are
auto-upstream+downstreamed and the set of things that are more
standard-library-esque and thus forbidden to touch in the vendored
directory (again, we can enforce this with a commit hook). My sense is that
achieving this partition won't be too hard.


> - As I've heard it from the cargo team, this approach is different
> from the solutions being pursued by the other nontrivial (>= 5 million
> LOC) corporate build systems. I worry a bit about having a different
> tool stack than the rest of the ecosystem and potentially confusing
> volunteers who would like to contribute to Rust code in Gecko or have
> their projects used in Gecko.
>

Can you elaborate on this?


> That said, trying to figure out how to make the obvious extension of
> the current experience (set a `paths` override in a config file, clone
> the repo, do the work there, open a PR and land upstream and push to
> crates.io, remove the override, run the "vendor into m-c" tool to pull
> it down, and *finally* land in m-c) seems brutal even for a single
> dependency, much less something that sweeps across several of them.
>

No kidding. :-)

bholley


> The answer to any approach we come up with is probably "more tooling,"
> but I suspect that, like bholley said below, there's a lot of
> discussion to be had here around the myriad issues.
>
> This conversation is also going on in Bugzilla here:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1231764 and also has
> build peers and Cargo folks in the discussion. I believe they're not
> on dev-servo.
> - Lars
>
>
> On Thu, Dec 17, 2015 at 5:28 PM, Bobby Holley 
> wrote:
> > I had a long conversation about the two-way sync stuff at Orlando with
> Alex
> > and some of the Servo folks.
> >
> > We concluded that, long-term, we need to let Gecko hackers commit to the
> > vendored code and have that code automatically upstreamed. Similarly,
> > upstream fixes need to be automatically merged into Gecko.
> >
> > Everyone seemed on board with this plan, but Alex suggested that it
> should
> > probably be a separate tool, distinct from Cargo. This tooling is going
> to
> > be pretty important, but we should probably wait on it until we've got
> the
> > other issues sorted out.
> >
> > bholley
> >
> > On Thu, Dec 17, 2015 at 3:04 PM, Valentin Gosu 
> > wrote:
> >
> >> Hi folks,
> >>
> >> I just wanted to share some notes on using Cargo in Gecko. One of the
> >> requirements was vendoring packages in tree. It turns out I was able to
> do
> >> that with a .cargo/config file in the scope of the rust directory, which
> >> defines the paths for dependencies, and [http] timeout = 0, which
> prevents
> >> downloading any resources from crates.io. [1]
> >> Better support for this use case would be welcome, such as making sure
> no
> >> network connections are made (the index still gets refreshed), but this
> is
> >> good enough for what we need at the moment.
> >>
> >> I'm not sure if we've agreed to have a top level directory for all of
> the
> >> rust crates, but I think it makes sense. It allows for easier sharing of
> >> crates (between necko and media for example).
> >> From what I understand servo's mach has a comman

Re: [dev-servo] Using Cargo in Gecko

[Bobby Holley]

On Mon, Dec 21, 2015 at 1:52 PM,  wrote:

> Developing against external packages (including ones shared with Servo)
> and pulling in changes as needed can be an easier process than you expect,
> and the alternatives (forking or syncing) have much more serious
> consequences.
>
> The basic idea is that you work with shared dependencies from crates.io
> or github under normal circumstances. If you find you need to make a quick
> change to a dependency to keep moving, you can fork that dependency on
> Github and use your local changes instead. At the same time,  you submit
> the changes upstream as a pull request, and revert back to using the shared
> dependencies once they have been accepted.
>
> Since Gecko intends to, at minimum, share dependencies with Servo, this
> strategy should be the most seamless.
>

I don't think this is going to fly in Gecko, unfortunately.

Gecko is a monolithic repo, and everything needs to be vendored in-tree (a
non-negotiable requirement from the build peers). This means that we'll
already need an automated process for pulling in updates to the shared
code, committing them, and running them through CI.

The only remaining question, then, is whether we allow Gecko hackers to
commit directly (with appropriate review) to the vendored code, or whether
we require them hack on a local clone of the upstream.

Gecko hackers expect to be able to easily make atomic changesets that span
modules. They want to make a series of changsets that is, say, 15-deep and
spanning multiple programming languages, interactively reorder those
commits, bisect them, etc. They may grudgingly put up with barriers to that
workflow for something self-contained and infrequently-touched (like the
url parser), but requiring them to make a github fork to hack on the style
system will likely meet insurmountable opposition.

So of the aforementioned options, I'm pretty sure that only the first is
workable. The two ways to implement that are with tooling for automatic
upstreaming, or forking Servo. So I sure hope that we can figure out the
tooling. ;-)

bholley


> That said, Servo has blazed a trail as one of the first Rust projects with
> a huge tree of dependencies, and uncovered some weaknesses in Cargo as a
> result. At Mozlando, we had a great conversation with the Servo team about
> how to help them manage upstream dependencies better.
>
> ---
>
> The main improvement that we plan to make to Cargo as a result of that
> conversation is *temporary overrides*.
>
> **Temporary overrides are an important part of the design of a package
> manager like Cargo** (Bundler, the package manager I worked on in the Ruby
> ecosystem, has support for temporary overrides).
>
> Let me outline how I envision that working in Cargo.
>
> ---
>
> Workflow (using Servo as an example)
>
> When a project discovers that it needs to make a temporary patch to an
> upstream dependency, it forks the project on Github.
>
> Next, it goes into the top-level `Cargo.toml` for their project and enters
> the override:
>
> ```
> [[overrides.simd]]
>
> version = "*"
> git = "https://github.com/servo/simd";
> ```
>
> This means that any version of the `simd` crate used by Servo (directly or
> indirectly) will be overridden by the version supplied by `servo/simd`.
> This applies to crates from crates.io as well as other git crates.
>
> Next, Servo would submit a pull request to the original git repository
> that they have forked. Once the original owner has accepted the pull
> request, Servo can remove the override.
>
> Alternately, if the crate in question is owned by Servo
> (`servo/rust-freetype` for example), Servo can create a branch in
> `servo/rust-freetype` and use those changes using the same override
> mechanism:
>
> ```
> [[overrides.rust-freetype]]
>
> version = "*"
> branch = "bugfix-3561"
> ```
>
> I would also like to make this workflow nice from the command line, with
> `cargo override` or something similar.
>
> -- Yehuda
>
> > On Thu, Dec 17, 2015 at 3:04 PM, Valentin Gosu 
> > wrote:
> >
> > > Hi folks,
> > >
> > > I just wanted to share some notes on using Cargo in Gecko. One of the
> > > requirements was vendoring packages in tree. It turns out I was able
> to do
> > > that with a .cargo/config file in the scope of the rust directory,
> which
> > > defines the paths for dependencies, and [http] timeout = 0, which
> prevents
> > > downloading any resources from crates.io. [1]
> > > Better support for this use case would be welcome, such as making sure
> no
> > > network connections are made (the index still gets refreshed), but
> this is
> > > good enough for what we need at the moment.
> > >
> > > I'm not sure if we've agreed to have a top level directory for all of
> the
> > > rust crates, but I think it makes sense. It allows for easier sharing
> of
> > > crates (between necko and media for example).
> > > From what I understand servo's mach has a command for two-way sync
> between
> > > web-platform-tests and upstream. This wo

Re: [dev-servo] Using Cargo in Gecko

[Bobby Holley]

On Mon, Dec 21, 2015 at 2:21 PM, Yehuda Katz  wrote:

> On Mon, Dec 21, 2015 at 2:14 PM, Bobby Holley 
> wrote:
>
> > I don't think this is going to fly in Gecko, unfortunately.
> >
> > Gecko is a monolithic repo, and everything needs to be vendored in-tree
> (a
> > non-negotiable requirement from the build peers). This means that we'll
> > already need an automated process for pulling in updates to the shared
> > code, committing them, and running them through CI.
> >
>
> Can you say a bit more about what the requirements are here? Is the reason
> for including the code in-tree that you need to be 100% confident that
> everyone is talking about the same code? Or is it more than that?
>

The reason I've heard are:
(1) Gecko/Firefox has a very complicated releng setup, and the builders are
heavily firewalled from the outside, and not allowed to hit the network. So
adding network dependencies to the build step would require a lot of
operations work.
(2) Gecko exists on a pretty long timescale, and we want to make sure that
we can still build Firefox 50 ten years from now, even if Caro has long
migrated to some other setup.
(3) A general unease about depending on any third-party service without a
contract and SLA in order to build and ship Firefox.

There may be other reasons, or I may be getting some of these wrong. This
all comes from gps, ted, etc, so you're probably better off discussing with
them directly.
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] Using Cargo in Gecko

[Bobby Holley]

I CCed them upthread, but then the subsequent reply dropped them. ;-)

I would suggest starting a new, explicit thread with a summary of what's
been discussed thus far (rather than asking them to jump into this one),
and CCing dev-servo.

That being said, my guess is that it's going to be a mostly academic
discussion, because the workflow issues mentioned above would likely lead
us to vendoring even without the requirements from the build peers.

On Mon, Dec 21, 2015 at 4:21 PM, Yehuda Katz  wrote:

> On Mon, Dec 21, 2015 at 4:17 PM, James Graham 
> wrote:
>
> >
> > It is not immediately apparent to me if that is sufficient to meet the
> > Gecko requirements, but again, having this conversation on a list without
> > gps, ted, glandium and other build peers seems rather counterproductive
> > since they know both the requirements that we have from our current build
> > system, and the improvements that are in the pipeline for next year
> (which
> > I understand to be numerous and considered a priority). Without this
> > information it's impossible to tell if any proposed change to cargo is
> > solving the right problem.
>
>
> Can we get them here? I'm very interested in working through this with
> someone who knows all of the constraints deeply. At minimum, I would like
> Cargo to be a suitable tool for people in similar situations as much as
> possible.
>
> -- Yehuda
>
>
> >
> >
> > ___
> > 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
>
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] Using Cargo in Gecko

[Bobby Holley]

On Mon, Dec 21, 2015 at 2:47 PM, Alex Crichton 
wrote:

> I think one important aspect here is to separate out the concerns of
> vendoring code in-tree vs where that code is developed. It sounds like
> Gecko has lots of technical requirements about why and how code lives
> in-tree, but the more important bit here is how it changes over time.
>
> What Yehuda was saying about how forking or syncing having downsides I
> think is quite important for Rust code in Gecko, as this may run the
> risk of splitting the ecosystem on various boundaries (e.g. the "gecko
> rust-url" and the "upstream rust-url" or something like that). Along
> those lines I think it's very important to drill down into exactly
> what concerns there are about having these components developed in the
> Servo-style as they are today.
>

IME "Servo-style today" would already be an enormous hassle if it weren't
for the mitigating factor that most of the core browser components already
live in a monolithic repo. In the cases where some more core-ish code lives
in an external repo (like rust-selectors), making cross-coupled changes is
a huge pain, even with super-responsive maintainers like Simon.

Given that I hack on both, I am interested in solving this problem for
Servo too. :-)


> It sounds like there's some hesitation about this today, but Yehuda I
> think is making a good case that tooling can make the process quite
> easy. There's quite a lot to benefit from with having only "one source
> of truth" for all this code!
>
>
> On Mon, Dec 21, 2015 at 2:45 PM, Yehuda Katz  wrote:
> > Yehuda Katz
> > (ph) 718.877.1325
> >
> > On Mon, Dec 21, 2015 at 2:28 PM, Josh Matthews 
> > wrote:
> >
> >> On 2015-12-21 5:21 PM, Yehuda Katz wrote:
> >>
> >>> On Mon, Dec 21, 2015 at 2:14 PM, Bobby Holley 
> >>> wrote:
> >>>
> >>> I don't think this is going to fly in Gecko, unfortunately.
> >>>>
> >>>> Gecko is a monolithic repo, and everything needs to be vendored
> in-tree
> >>>> (a
> >>>> non-negotiable requirement from the build peers). This means that
> we'll
> >>>> already need an automated process for pulling in updates to the shared
> >>>> code, committing them, and running them through CI.
> >>>>
> >>>>
> >>> Can you say a bit more about what the requirements are here? Is the
> reason
> >>> for including the code in-tree that you need to be 100% confident that
> >>> everyone is talking about the same code? Or is it more than that?
> >>>
> >>>
> >> The "non-negotiable requirement from the build peers" is that the
> machines
> >> that build Firefox do not have internet access.
> >
> >
> > That's a totally reasonable requirement and one that I am very
> comfortable
> > saying that Cargo should support.
> >
> > To support this requirement, we plan to add a new command that packages
> up
> > an application into a single tarball (or equivalent), complete with all
> > dependencies, and another command that can build such a package without
> > network connectivity.
> >
> > Bundler has equivalent facilities (bundle package and bundle install
> > --deployment) that I consider indispensable for deployment scenarios, and
> > which I championed early on in the bundler process. (Heroku uses `bundle
> > install --deployment` in their buildpacks).
> >
> > -- Yehuda
> ___
> 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

Re: [dev-servo] Using Cargo in Gecko

[Bobby Holley]

Reposting this without the extraneous mailing lists. Please reply to this
one.

On Tue, Dec 22, 2015 at 9:23 AM, Bobby Holley  wrote:

> On Mon, Dec 21, 2015 at 2:47 PM, Alex Crichton 
> wrote:
>
>> I think one important aspect here is to separate out the concerns of
>> vendoring code in-tree vs where that code is developed. It sounds like
>> Gecko has lots of technical requirements about why and how code lives
>> in-tree, but the more important bit here is how it changes over time.
>>
>> What Yehuda was saying about how forking or syncing having downsides I
>> think is quite important for Rust code in Gecko, as this may run the
>> risk of splitting the ecosystem on various boundaries (e.g. the "gecko
>> rust-url" and the "upstream rust-url" or something like that). Along
>> those lines I think it's very important to drill down into exactly
>> what concerns there are about having these components developed in the
>> Servo-style as they are today.
>>
>
> IME "Servo-style today" would already be an enormous hassle if it weren't
> for the mitigating factor that most of the core browser components already
> live in a monolithic repo. In the cases where some more core-ish code lives
> in an external repo (like rust-selectors), making cross-coupled changes is
> a huge pain, even with super-responsive maintainers like Simon.
>
> Given that I hack on both, I am interested in solving this problem for
> Servo too. :-)
>
>
>> It sounds like there's some hesitation about this today, but Yehuda I
>> think is making a good case that tooling can make the process quite
>> easy. There's quite a lot to benefit from with having only "one source
>> of truth" for all this code!
>>
>>
>> On Mon, Dec 21, 2015 at 2:45 PM, Yehuda Katz  wrote:
>> > Yehuda Katz
>> > (ph) 718.877.1325
>> >
>> > On Mon, Dec 21, 2015 at 2:28 PM, Josh Matthews 
>> > wrote:
>> >
>> >> On 2015-12-21 5:21 PM, Yehuda Katz wrote:
>> >>
>> >>> On Mon, Dec 21, 2015 at 2:14 PM, Bobby Holley 
>> >>> wrote:
>> >>>
>> >>> I don't think this is going to fly in Gecko, unfortunately.
>> >>>>
>> >>>> Gecko is a monolithic repo, and everything needs to be vendored
>> in-tree
>> >>>> (a
>> >>>> non-negotiable requirement from the build peers). This means that
>> we'll
>> >>>> already need an automated process for pulling in updates to the
>> shared
>> >>>> code, committing them, and running them through CI.
>> >>>>
>> >>>>
>> >>> Can you say a bit more about what the requirements are here? Is the
>> reason
>> >>> for including the code in-tree that you need to be 100% confident that
>> >>> everyone is talking about the same code? Or is it more than that?
>> >>>
>> >>>
>> >> The "non-negotiable requirement from the build peers" is that the
>> machines
>> >> that build Firefox do not have internet access.
>> >
>> >
>> > That's a totally reasonable requirement and one that I am very
>> comfortable
>> > saying that Cargo should support.
>> >
>> > To support this requirement, we plan to add a new command that packages
>> up
>> > an application into a single tarball (or equivalent), complete with all
>> > dependencies, and another command that can build such a package without
>> > network connectivity.
>> >
>> > Bundler has equivalent facilities (bundle package and bundle install
>> > --deployment) that I consider indispensable for deployment scenarios,
>> and
>> > which I championed early on in the bundler process. (Heroku uses `bundle
>> > install --deployment` in their buildpacks).
>> >
>> > -- Yehuda
>> ___
>> 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

Re: [dev-servo] Using Cargo in Gecko

[Bobby Holley]

On Mon, Dec 21, 2015 at 6:55 PM,  wrote:

> On Thursday, December 17, 2015 at 3:29:17 PM UTC-8, Bobby Holley wrote:
> > I had a long conversation about the two-way sync stuff at Orlando with
> Alex
> > and some of the Servo folks.
> >
> > We concluded that, long-term, we need to let Gecko hackers commit to the
> > vendored code and have that code automatically upstreamed. Similarly,
> > upstream fixes need to be automatically merged into Gecko.
> >
> > Everyone seemed on board with this plan, but Alex suggested that it
> should
> > probably be a separate tool, distinct from Cargo. This tooling is going
> to
> > be pretty important, but we should probably wait on it until we've got
> the
> > other issues sorted out.
>
> Bi-directional sync in version control is a hard problem. It's possible to
> deliver something that is correct 90% of the time. But there are inevitably
> race conditions that lead to divergence, which human intervention is
> required to correct. The *only* way to prevent this is for there to be a
> single source of truth and for the repository's locking and transaction
> mechanism to prevent races.
>
> This means version control for Gecko+Servo given the apparent set of
> requirements is very difficult. As it stands, we're talking about source of
> truth living in several upstream repositories which are then vendored in
> mozilla-central. We either have a Gecko developer first committing upstream
> and then pulling down that change into mozilla-central - likely high
> latency and frustrating - OR a nasty sync/merge problem coupled with
> governance and logistical concerns/problems over who can commit to what and
> with what oversight. It's a nightmare.
>
> An alternative is to make mozilla-central the canonical repository. I'm
> almost certain the thought of this would incite riots. But I don't think
> this - or something along that vein - is as far fetched as you may think
> because there are crazy things you can do in version control.
>
> Facebook has a crazy version control setup. Internally they have a
> monorepo with tons of random projects. This includes a number of their
> popular open source projects. What they do is export sub-directories from
> their internal monorepo to e.g. GitHub. People submit pull requests and do
> the typical GitHub workflow. For some projects, they synchronize pull
> request code reviews into their internal code review tool (Phabricator).
> Comments left on Phabricator are replicated to GitHub and vice versa. When
> it comes time to merge a pull request, they don't merge it in GitHub.
> Instead, a tool grafts the branch onto their monorepo, commits it, syncs it
> back out to the GitHub project, then closes the pull request. It's a
> terrific model and something that we (Mozilla) could build if we needed to.
> Servo isn't along in wanting this. Groups like Firefox Developer Tools and
> Loop desire to live in a world where their feature behaves like a
> standalone project - independent of mozilla-central.
>

This is an interesting idea. However, for the style system case this would
involve the entirety of of the top-level Servo living in mozilla-central
(or rather, the shared monolithic mega-repo, since both projects would
likely end up doing partial checkouts). And even if the tooling were
seemless enough that Servo developers were able to transparently hack on
the github repo as they do now, we'd still need to unify the CI in order to
continue treating both projects as Tier-1. This is theoretically doable,
and possibly the right thing if Gecko and Servo start sharing lots and lots
of code, but I'm pretty sure we don't want that in the near future.

Moreover, as Simon points out, this doesn't really scale to all the other
projects that Servo depends on.

I recognize that bidirectional sync is imperfect and may require manual
resolution of conflicts (source-level, compile-time, or runtime). But I
think it makes the right trade-offs for the projects we're pursuing right
now. The devil is in the details, and I think it might be worth spinning
off a separate thread to discuss the precise mechanics of how that tool
would work. I'll do that now.

I understand why there would be visceral reactions to shackling your
> project to mozilla-central. There is a lot of process and overhead there.
> But, with some tooling and realized planned improvements to the automation
> infrastructure that e.g. reduce tree closures, I think many of these
> concerns would go away. Projects could be canonically located in
> mozilla-central, but all day-to-day development is performed in GitHub (or
> wherever). This gives us all the nice properties of the monorepo without
> t

[dev-servo] Mechanics of Bidirectional Syncing

[Bobby Holley]

Here's a strawman proposal of what I'm imagining for the sharing of the
style system between Gecko and Servo. I'm sure there's lots wrong here and
stuff that can be tweaked, but I wanted to get a slightly more concrete
proposal out there for discussion.

* In m-c, we have vendored directories for each Servo crate we use.
* For crates where we cannot come up with an identical peer list on the
Gecko and Servo side, the component is marked read-only except for upstream
updates (enforced with a push hook), and must be modified the slow way
(submitting patches upstream, and then pulling them in).
* For components with unified peers, we do the following:
** Enforce the peer set with commit hooks on both the Gecko and Servo side.
** Gecko and Servo try pushes which touch the shared code include an
additional job to run a try push on the other CI, and report the results.
If the peers enforce judicious use of try, this should catch most of the
cross-project breakage.
** Each time a push which touches shared code lands in either repository, a
corresponding push (with non-squashed changesets) is prepared and
automatically pushed to the other repository.
** If that push bounces, the repositories are marked as out-of-sync, and an
email is sent to the committer and the set of peers indicating that manual
fixup is required.
** If possible, when the repositories are known to be out of sync, a commit
hook would prevent additional changesets from landing on either side
without a magic word in the commit message.

Thoughts?
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] Mechanics of Bidirectional Syncing

[Bobby Holley]

Thinking about this a bit more, it occurs to me that style system changes
that would require simultaneous Gecko fixup in the same commit are likely
to also require simultaneous Servo fixup.

Given that, I think we probably want to vendor all of Servo, rather than
just the crates we build in Gecko. This would facilitate making breaking
API changes where we have consumers in both Gecko and Servo that need
fixing up (fixing up multiple consumers alongside the API is one of the
primary wins of mono-repos).

This would increase the traffic of Servo->Gecko vendoring updates, but we
would clearly tweak CI to no-op for those effectively-NPOTB changes.
Long-term when we get partial checkouts, we could make the default checkout
of m-c omit the Servo-only crates.

On Tue, Dec 22, 2015 at 10:15 AM, Bobby Holley 
wrote:

> Here's a strawman proposal of what I'm imagining for the sharing of the
> style system between Gecko and Servo. I'm sure there's lots wrong here and
> stuff that can be tweaked, but I wanted to get a slightly more concrete
> proposal out there for discussion.
>
> * In m-c, we have vendored directories for each Servo crate we use.
> * For crates where we cannot come up with an identical peer list on the
> Gecko and Servo side, the component is marked read-only except for upstream
> updates (enforced with a push hook), and must be modified the slow way
> (submitting patches upstream, and then pulling them in).
> * For components with unified peers, we do the following:
> ** Enforce the peer set with commit hooks on both the Gecko and Servo side.
> ** Gecko and Servo try pushes which touch the shared code include an
> additional job to run a try push on the other CI, and report the results.
> If the peers enforce judicious use of try, this should catch most of the
> cross-project breakage.
> ** Each time a push which touches shared code lands in either repository,
> a corresponding push (with non-squashed changesets) is prepared and
> automatically pushed to the other repository.
> ** If that push bounces, the repositories are marked as out-of-sync, and
> an email is sent to the committer and the set of peers indicating that
> manual fixup is required.
> ** If possible, when the repositories are known to be out of sync, a
> commit hook would prevent additional changesets from landing on either side
> without a magic word in the commit message.
>
> Thoughts?
>
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] Mechanics of Bidirectional Syncing

[Bobby Holley]

On Tue, Dec 22, 2015 at 1:59 PM, Ryan VanderMeulen <
rvandermeu...@mozilla.com> wrote:

> Can you provide some context to this discussion for those of us who know
> little to nothing about the work you're discussing?
>

This is a fork of the thread in
https://groups.google.com/forum/#!topic/mozilla.dev.servo/qESJoCIsPU8 .


> We're going to start sharing a common style system between Servo and Gecko?
>

Maybe. We are exploring the feasibility and benefits of doing so on the
code-side. While we do that, we're also discussing what the sharing might
look like.


> When? :)
>

Still very much in the exploratory phase. I would hope that we'd have a
clear-ish sense of whether we're going ahead with this by London.


-Ryan
>
> On Tue, Dec 22, 2015 at 4:23 PM, Bobby Holley 
> wrote:
>
>> Thinking about this a bit more, it occurs to me that style system changes
>> that would require simultaneous Gecko fixup in the same commit are likely
>> to also require simultaneous Servo fixup.
>>
>> Given that, I think we probably want to vendor all of Servo, rather than
>> just the crates we build in Gecko. This would facilitate making breaking
>> API changes where we have consumers in both Gecko and Servo that need
>> fixing up (fixing up multiple consumers alongside the API is one of the
>> primary wins of mono-repos).
>>
>> This would increase the traffic of Servo->Gecko vendoring updates, but we
>> would clearly tweak CI to no-op for those effectively-NPOTB changes.
>> Long-term when we get partial checkouts, we could make the default checkout
>> of m-c omit the Servo-only crates.
>>
>> On Tue, Dec 22, 2015 at 10:15 AM, Bobby Holley 
>> wrote:
>>
>>> Here's a strawman proposal of what I'm imagining for the sharing of the
>>> style system between Gecko and Servo. I'm sure there's lots wrong here and
>>> stuff that can be tweaked, but I wanted to get a slightly more concrete
>>> proposal out there for discussion.
>>>
>>> * In m-c, we have vendored directories for each Servo crate we use.
>>> * For crates where we cannot come up with an identical peer list on the
>>> Gecko and Servo side, the component is marked read-only except for upstream
>>> updates (enforced with a push hook), and must be modified the slow way
>>> (submitting patches upstream, and then pulling them in).
>>> * For components with unified peers, we do the following:
>>> ** Enforce the peer set with commit hooks on both the Gecko and Servo
>>> side.
>>> ** Gecko and Servo try pushes which touch the shared code include an
>>> additional job to run a try push on the other CI, and report the results.
>>> If the peers enforce judicious use of try, this should catch most of the
>>> cross-project breakage.
>>> ** Each time a push which touches shared code lands in either
>>> repository, a corresponding push (with non-squashed changesets) is prepared
>>> and automatically pushed to the other repository.
>>> ** If that push bounces, the repositories are marked as out-of-sync, and
>>> an email is sent to the committer and the set of peers indicating that
>>> manual fixup is required.
>>> ** If possible, when the repositories are known to be out of sync, a
>>> commit hook would prevent additional changesets from landing on either side
>>> without a magic word in the commit message.
>>>
>>> Thoughts?
>>>
>>
>>
>
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] Using Cargo in Gecko

[Bobby Holley]

On Tue, Dec 22, 2015 at 2:46 PM, Yehuda Katz  wrote:

> I want to make sure that I understand the core constraints here:
>
>1. Gecko wants to be able to reliably do builds (especially urgent ones)
>without relying on the uptime of third party services.
>2. Gecko's build bots do not have any access to the Internet.
>3. Servo will continue to make use of third-party libraries like
>rust-url that will be developed in parallel by the Rust community.
>4. Browsers need to be sure that the upstream code they are using is
>byte-for-byte compatible across builds.
>5. Gecko would like to manually audit all changes to upstream
>dependencies, and be sure that once a dependency (direct or indirect)
> has
>been audited, the relevant source code cannot change without additional
>intervention.
>6. Both Gecko and Servo need the ability to quickly (and in a
>lightweight fashion) make changes to upstream dependencies without
> needing
>to immediately convince the upstream maintainer to accept the change.
>

As a corollary to this, we want a mechanism that avoids divergent forks.
Gecko's vendoring strategy allows quick updates at the cost of drifting
towards forks (i.e. Gecko's Cairo is a totally divergent fork). Servo's
dependency strategy avoids forking at the cost of making quick updates
difficult (i.e. it's a pain to make changes that touch rust-selectors).


>7. The Rust community assumes that the Cargo package manager will be
>used to update dependencies, and packages lean on the ability for
>sophisticated dependency resolution algorithms to manage the process of
>updates.
>
> Have I missed any important constraints?
>

There's still the issue of wanting to avoid a complicated dependency on a
ecosystem that may disappear or look very different down the line, and the
added complexity in the sort term of tying into more systems. But I think
that's probably less of a "hard" constraint.


>
> -- Yehuda
>
> Yehuda Katz
> (ph) 718.877.1325
>
> On Tue, Dec 22, 2015 at 9:25 AM, Bobby Holley 
> wrote:
>
> > Reposting this without the extraneous mailing lists. Please reply to this
> > one.
> >
> > On Tue, Dec 22, 2015 at 9:23 AM, Bobby Holley 
> > wrote:
> >
> > > On Mon, Dec 21, 2015 at 2:47 PM, Alex Crichton 
> > > wrote:
> > >
> > >> I think one important aspect here is to separate out the concerns of
> > >> vendoring code in-tree vs where that code is developed. It sounds like
> > >> Gecko has lots of technical requirements about why and how code lives
> > >> in-tree, but the more important bit here is how it changes over time.
> > >>
> > >> What Yehuda was saying about how forking or syncing having downsides I
> > >> think is quite important for Rust code in Gecko, as this may run the
> > >> risk of splitting the ecosystem on various boundaries (e.g. the "gecko
> > >> rust-url" and the "upstream rust-url" or something like that). Along
> > >> those lines I think it's very important to drill down into exactly
> > >> what concerns there are about having these components developed in the
> > >> Servo-style as they are today.
> > >>
> > >
> > > IME "Servo-style today" would already be an enormous hassle if it
> weren't
> > > for the mitigating factor that most of the core browser components
> > already
> > > live in a monolithic repo. In the cases where some more core-ish code
> > lives
> > > in an external repo (like rust-selectors), making cross-coupled changes
> > is
> > > a huge pain, even with super-responsive maintainers like Simon.
> > >
> > > Given that I hack on both, I am interested in solving this problem for
> > > Servo too. :-)
> > >
> > >
> > >> It sounds like there's some hesitation about this today, but Yehuda I
> > >> think is making a good case that tooling can make the process quite
> > >> easy. There's quite a lot to benefit from with having only "one source
> > >> of truth" for all this code!
> > >>
> > >>
> > >> On Mon, Dec 21, 2015 at 2:45 PM, Yehuda Katz 
> wrote:
> > >> > Yehuda Katz
> > >> > (ph) 718.877.1325
> > >> >
> > >> > On Mon, Dec 21, 2015 at 2:28 PM, Josh Matthews <
> j...@joshmatthews.net
> > >
> > >> > wrote:
> > >> >
> > >> >> On 2015-12-21 5:21 PM, Yehuda Katz wrote:
>

Re: [dev-servo] Student Project

[Bobby Holley]

At least in Gecko, the media stack is somewhat intertwined. I know we were
planning on lifting at least the media playback pieces from Gecko into
Servo, so it's worth considering the implications there.

I've also heard it suggested several times that Gecko might be interested
in building a WebAudio implementation in Rust. So it seems like there's
some collaboration potential there, but only if we think it through and
align resources.

All in all, I think it's probably not suitable for a student project.

On Sun, Jan 10, 2016 at 1:45 PM, Ms2ger  wrote:

> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
>
> On 01/10/2016 09:48 PM, Robert O'Callahan wrote:
> > On Mon, Jan 11, 2016 at 7:04 AM, Nicolas Silva
> >  wrote:
> >
> >> Anyone interested in implementing WebAudio and/or WebRTC (in
> >> Gecko there's some overlap in the underlying infrastructure)
> >> should first spend some time discussing the architecture with
> >> Paul Adenot (look for padenot on irc). Having a competitive or
> >> even just decent WebAudio implementation is more complex than it
> >> looks, and Gecko's implementation had to go through massive
> >> rewrites before it got to a satisfying place. According to Paul
> >> it's not the kind of architecture one gets right the first time
> >> without some serious experience in audio engines. That said, it
> >> would be great to have a WebAudio implementation in rust, and I
> >> am sure the rust gamedev community would be thrilled to have
> >> something like this!
> >>
> >
> > Right.
> >
> > You'd really want to study the architecture of Gecko's
> > implementation and understand why it is the way it is before
> > attempting this. You'd also want to understand the architectural
> > changes we're still planning to make. Paul and I can help with
> > this.
> >
> > In Gecko MediaStreams and WebAudio are integrated into a single
> > real-time media graph (well, multiple graphs, one per logical
> > output channel), which has advantages but means you're designing
> > more than just WebAudio.
> >
> > If we did have a good WebAudio implementation in Servo it's one
> > piece I could imagine sharing with Gecko. But it's a lot of work to
> > get to parity.
> >
>
> FWIW, the issue () already
> says suggests talking to Paul and you, from the last time this was
> discussed.
>
> HTH
> Ms2ger
> -BEGIN PGP SIGNATURE-
>
> iQEcBAEBAgAGBQJWktEKAAoJEOXgvIL+s8n2cfoH/0JcslWVFJZwjvurKWKRz4sv
> jVWwywdFQ1TGPHIPtMTlmtbeZMkT5Y8CJ2jXY6OYv27JnQtr8JguosDe3rtQiq3x
> ahRaaWTcvfwa6elAvnusLeA24oowSXW94VGixjVW9W5VWqJgi4wN/B1WT4QUvpp7
> vVeT2jFcYQUWLjOOVnFWmvsCeZNUggsN1B6oNwlXTIWpDzXPaippPwlFPOvZUMqc
> kJAU5bsD+XA7686rL2GqGLBKyALcJx4oyEQQOhyt56PubQcR1Zy7Z5t0+oWsoKjt
> Lcimxcx9ts1Z1jqLgaZvWTQwDnEL9zvGX+7PGyw8kgH4P/TGNOXQ31fspA3FULY=
> =fhYA
> -END PGP SIGNATURE-
> ___
> 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

Re: [dev-servo] when to switch to webrender by default

[Bobby Holley]

In general, does the software-fallback path for WebRender mean "really
really slow"? If WebRender avoids dirty-rects on the grounds that painting
is free, then a GL-based software path is going to be a lot slower than a
classic software rendering path.

On Mon, Mar 28, 2016 at 10:33 AM,  wrote:

> On Monday, March 21, 2016 at 11:45:06 AM UTC-5, Jack Moffitt wrote:
> > I propose the following straw man transition plan:
> >
> > 1. Keep -c, -g, -w command line options as they are, but switch the
> > default setting to WebRender.
> > 2. Remove -g.
> > 3. Add in an llvmpipe software-only fallback and replace -c with that.
>
> I like this plan, or at least the first two steps of it. Does #1 require
> getting any additional WPT/CSS tests working, or is WebRender basically
> already there?
>
> I think #3 might need some more thought to figure out what scenarios it
> will support (printing? X11 remoting? headless mode?) to see if llvmpipe
> will support all of those scenarios. But it'd be great if it did and would
> let us reduce the number of rendering backends.
> - Lars
> ___
> 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

Re: [dev-servo] when to switch to webrender by default

[Bobby Holley]

On Mon, Mar 28, 2016 at 3:06 PM, Patrick Walton  wrote:

> Keep in mind that Servo without WR doesn't do dirty rects and invalidation
> either. (I had patches to do it but they weren't complete and bitrotted.)
>

Ok, so that helps to explain Manish's results above.


> The question in my mind is whether it'd be better to build the CPU fallback
> on top of WebRender or whether it's best to build it on top of Skia. There
> are ways to build invalidation on top of WebRender, so I'm not sure we gain
> much by keeping the old painting code around. Furthermore, WebRender 2 has
> the potential to make invalidation and dirty rects easy. It's not clear
> whether WR2 will work, but signs are positive enough that there's a good
> chance that any invalidation work we did today would be obsoleted by the
> WR2 effort. The upshot to me is that it's likely that maintaining the
> existing painting code will not prove helpful for invalidation work anyhow.
>

This makes sense to me.


>
> Patrick
> On Mar 28, 2016 1:50 PM, "Bobby Holley"  wrote:
>
> > In general, does the software-fallback path for WebRender mean "really
> > really slow"? If WebRender avoids dirty-rects on the grounds that
> painting
> > is free, then a GL-based software path is going to be a lot slower than a
> > classic software rendering path.
> >
> > On Mon, Mar 28, 2016 at 10:33 AM,  wrote:
> >
> > > On Monday, March 21, 2016 at 11:45:06 AM UTC-5, Jack Moffitt wrote:
> > > > I propose the following straw man transition plan:
> > > >
> > > > 1. Keep -c, -g, -w command line options as they are, but switch the
> > > > default setting to WebRender.
> > > > 2. Remove -g.
> > > > 3. Add in an llvmpipe software-only fallback and replace -c with
> that.
> > >
> > > I like this plan, or at least the first two steps of it. Does #1
> require
> > > getting any additional WPT/CSS tests working, or is WebRender basically
> > > already there?
> > >
> > > I think #3 might need some more thought to figure out what scenarios it
> > > will support (printing? X11 remoting? headless mode?) to see if
> llvmpipe
> > > will support all of those scenarios. But it'd be great if it did and
> > would
> > > let us reduce the number of rendering backends.
> > > - Lars
> > > ___
> > > 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
> >
> ___
> 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

[dev-servo] PSA: Custom rust-selectors no longer required to build stylo

[Bobby Holley]

Now that the initial atom infrastructure has landed, you "just" need the
stylo branches of gecko-dev and servo (along with the appropriate
.mozconfig) to build stylo and render the wikipedia test page.

I've updated the instructions at https://public.etherpad-mozilla.org/p/stylo
to reflect this.

Happy Hacking!
bholley
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] Servo to Gecko comparison chart

[Bobby Holley]

On Tue, May 17, 2016 at 7:28 PM, Shing Lyu  wrote:

> Hi Jet,
>
> Thanks. I'm aware of the Stylo project and I'm excited about it!
>

If you or anyone else wants to help out with stylo, you're most welcome to!
There's lots of important work to do, so feel free to ping me if you want
to get involved. :-)

bholley


>
>
>
>  - Shing Lyu | Mozilla Taipei
>
> 2016-05-17 23:55 GMT+08:00 Jet Villegas :
>
> > I'd recommend you follow the Stylo progress as you dig through the source
> > folders. I think the most interesting (and immediately useful) bit is the
> > convergence of Layout data structures between the 2 engines.
> >
> > https://public.etherpad-mozilla.org/p/stylo
> >
> > --Jet
> >
> >
> > On Tue, May 17, 2016 at 12:31 AM, Shing Lyu  wrote:
> >
> >> Hi all,
> >>
> >> Some Gecko developers asked for a Servo to Gecko comparison chart, so
> they
> >> can easily convert their Gecko knowledge into Servo one.
> >>
> >> Since I know so little about Servo, I create this wiki page hoping that
> >> someone can fill in the blanks:
> >>
> >> https://github.com/servo/servo/wiki/Servo-for-Gecko-Developers
> >>
> >> (I might be totally wrong on the examples. Please correct any error you
> >> see
> >> directly)
> >>
> >> I know some concepts can't be mapped cleanly, so I created a "Notes"
> >> column
> >> for you to explain the detail (or just paste a link to the
> documentation.)
> >>
> >> We might also want to add columns for other browser engines like Blink,
> >> WebKit or Edge.
> >>
> >> Cheers,
> >>  - Shing Lyu | Mozilla Taipei
> >> ___
> >> dev-servo mailing list
> >> dev-servo@lists.mozilla.org
> >> https://lists.mozilla.org/listinfo/dev-servo
> >>
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "tw-layout" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> > email to tw-layout+unsubscr...@mozilla.com.
> > To post to this group, send email to tw-lay...@mozilla.com.
> > To view this discussion on the web visit
> >
> https://groups.google.com/a/mozilla.com/d/msgid/tw-layout/CA%2B3pDCJzp9Y4VBoT7OTV64HWbYS8kdR2EgeSKgXkBv%3DY-DEWCQ%40mail.gmail.com
> > <
> https://groups.google.com/a/mozilla.com/d/msgid/tw-layout/CA%2B3pDCJzp9Y4VBoT7OTV64HWbYS8kdR2EgeSKgXkBv%3DY-DEWCQ%40mail.gmail.com?utm_medium=email&utm_source=footer
> >
> > .
> >
> ___
> 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

Re: [dev-servo] Proposed work for upcoming sharing of Servo components with Firefox

[Bobby Holley]

I just chatted with Manish a little bit to sort out some details and
misunderstandings, and I think we're much more on the same page now.

A few high-level points worth emphasizing:
* Making the Homu queue take longer would be bad and we should avoid that.
I believe the proposal would actually significantly reduce landing latency.
* The proposal does not involve hooking servo/servo up to mozilla-inbound
and its associated churn. As long as gecko development uses the
mozilla-inbound model, servo/servo would be linked to a much-more-stable
"mozilla-servo" integration repo, which will have very few backouts and
should never be closed modulo infra issues.
* Updating the code in both repos "in lockstep" is central to the plan,
because it prevents divergent forks.
* The details of how/when tests are run involve a lot of tweakable
parameters, and I am optimistic that we can adjust them to solve any pain
points.

In a bit more detail, there are basically 3 ways to manage CI:
(1) The mozilla-inbound model (land possibly-untested stuff, run CI
post-commit, perform backouts and close tree as necessary)
(2) The bors/homu model (run the full suite on the precise commit that will
be merged, and only merge it when green)
(3) The relaxed bors/homu model: Run CI in parallel on pending commits. Any
mergable commits with "recent enough" green CI runs can be merged. We do
additional periodic post-merge CI runs on the master branch to spot the
occasional intra-commit bustage.

(1) obviously sucks and (2) doesn't scale well. The proposal is to do
something like (3) for both projects [1]. This is the long-term plan for
Gecko development anyway (i.e. mandatory pre-commit testing), and relaxing
the strict homu/bors model would significantly increase responsiveness and
throughput on the servo/servo side.

The question of exactly which pre-commit tests should be run for which
commits is a heuristic one that we can tweak. For example, we may decide
that changes to components/style require pre-commit gecko test runs, but
version bumps on leaf-y dependencies are safe enough to leave to the
post-commit sanity checking. We can play with that as we go.

[1] It's worth noting that (3) is _almost_ isomorphic to what Rust does
(merging rollups of commits with green CI), which the primary difference
being whether rollups are tested pre- or post-commit.

On Wed, Jun 22, 2016 at 9:03 AM, Boris Zbarsky  wrote:

> On 6/22/16 11:17 AM, Manish Goregaokar wrote:
>
>> We don't close down the tree except for CI fires
>>
>
> Yes, I understand your model.  You don't have to explain it to me.
>
> I was just pointing out that for normal commits you prefer the model of
> test-each-thing, but for style system you would prefer the model of
> just-test-every-so-often.  I realize the reasons for that too, but I think
> we have a different evaluation of the resulting costs.  In particular
>
> If the tests fail for a PR, it is taken out of the queue until it is fixed
>> and
>> reapproved.
>>
>
> My question is what happens when a PR stalls like this for months because
> by the time it's fixed something else has broken.  I expect this to happen
> every so often when the PR is the style system merge.
>
> m-c does not have this model, and I'd also like to prevent marrying our CI
>> times to those of m-c, or being affected by m-c backouts.
>>
>
> I understand and sympathize with that.
>
> That's why I like the sync model, it concentrates all of the conflict in
>> one operation
>>
>
> At the cost of possibly making that operation impossible to complete.
>
> instead of distributing it everywhere and making every PR that touches
>> anything that affects style suffer. The syncing operation is also async
>> (ha!) -- while being performed other Servo work can continue.
>>
>
> Which is what may well make it impossible to complete the sync.
>
> Sending PRs which look like they need testing to gecko-try should catch
>> most issues,
>> the only remaining issues are when a PR (like a wrong refactor, or
>> something) which wasn't sent through gecko-try breaks stylo, or when
>> something on Gecko's vendor dir managed to land between the time the Servo
>> PR was tested and landed. I postulate these two cases will be relatively
>> rare, and these will be the only times the sync fails.
>>
>
> Right, I think our fundamental disagreement is a matter of assumptions
> about frequency.  I hope that you're right about these failures being rare,
> but expect that you're wrong and worry about the failure modes if you
> are
>
> This is actually pretty close to the proposed model in the doc, but it
>> gives a lot more leeway in when the sync should happen.
>>
>
> Well, and assumes that reviewers are decent at evaluating the "may need
> stylo testing" predicate.
>
> The proposed model is basically equivalent to this except syncs are
>> mandatorily part of any PR
>> that affects style, and can make the queue clog up. Here based on various
>> factors you can choose not to sync (e.g. if 

Re: [dev-servo] Standardizing upgrade procedures for crates.io dependencies for servo/servo

[Bobby Holley]

Is there a risk that non-overlapping version ranges in dependent Cargo.toml
files will cause multiple versions of the package to be imported? Or would
that be obvious when looking at the Cargo.lock diff?

On Thu, Jun 23, 2016 at 7:17 AM, Josh Matthews 
wrote:

> https://github.com/servo/servo/pull/11824 relies on upgrading hyper to a
> more recent version. As far as I recall, for servo/servo we have only ever
> asked people to upgrade the package version via `./mach cargo-update`; we
> haven't required (or even asked for) modifying all of our many TOML files
> to match the new version. This makes sense to me, since we have our
> Cargo.lock checked into the repository so there's no chance that a
> contributor could end up using an old version of a package. Additionally,
> we have so many TOML files that I feel it's unreasonable to ask people to
> make those changes unnecessarily.
>
> I'd like to make a policy to either always require the changes or never do
> so, rather than imposing a burden on some contributors depending on who
> reviews their PR.
>
> Cheers,
> Josh
> ___
> 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

Re: [dev-servo] Standardizing upgrade procedures for crates.io dependencies for servo/servo

[Bobby Holley]

And what about the case of major version bumps? Presumably we want to crawl
the Cargo.toml files in that case? And again, will it be obvious that needs
to happen from the Cargo.lock diff?

On Thu, Jun 23, 2016 at 9:32 AM, Manish Goregaokar 
wrote:

> We rarely use ranges, just a minimum version, and aside from major version
> differences cargo just picks the max.
>
> dev-servo@lists.mozilla.org wrote:
> > Is there a risk that non-overlapping version ranges in dependent
> Cargo.toml
> > files will cause multiple versions of the package to be imported? Or
> would
> > that be obvious when looking at the Cargo.lock diff?
> >
> > On Thu, Jun 23, 2016 at 7:17 AM, Josh Matthews 
> wrote:
> >> https://github.com/servo/servo/pull/11824 relies on upgrading hyper to
> a
> >> more recent version. As far as I recall, for servo/servo we have only
> ever
> >> asked people to upgrade the package version via `./mach cargo-update`;
> we
> >> haven't required (or even asked for) modifying all of our many TOML
> files
> >> to match the new version. This makes sense to me, since we have our
> >> Cargo.lock checked into the repository so there's no chance that a
> >> contributor could end up using an old version of a package.
> Additionally,
> >> we have so many TOML files that I feel it's unreasonable to ask people
> to
> >> make those changes unnecessarily.
> >>
> >> I'd like to make a policy to either always require the changes or never
> do
> >> so, rather than imposing a burden on some contributors depending on who
> >> reviews their PR.
> >>
> >> Cheers,
> >> Josh
> >> ___
> >> 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
>
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] Proposed work for upcoming sharing of Servo components with Firefox

[Bobby Holley]

An interesting idea, but off-topic for this thread, and probably not
helpful to debate at this moment in time.

That is to say: we are inching towards a consensus across several groups on
a complicated, important, and time-sensitive topic, so please keep the cats
in the bag for now, or at least keep them away from the pigeons. ;-)
I have a cat I would like to place among the pigeons. I'm going to start
with the brave assertion that ultimately we want Servo based components in
Firefox to go directly into Firefox release several times per week.

"That's impossible" I hear you cry but it is common practice among web
services. Fast feedback improves our ability lean and quickly get value out
of our work. It won't be easy and it will require much better tools,
infrastructure and processes than we have today. There will probably be
some culture shock.

Rust has a number of language features that make it safer to iterate
quickly compared to C++ or Javascript. The question we all need to ask
ourselves is "why not?". We can take that list of blockers and burn them
down over the coming months.

Anthony

On Thursday, 23 June 2016 16:55:04 UTC+12, Manish Goregaokar  wrote:
> Coordinating rollups across two repos will be a pain. The proposed
> automation sounds better to me.
>
> -Manish Goregaokar
>
> On Thu, Jun 23, 2016 at 10:08 AM, Michael Howell <
michaelhowell...@gmail.com
> > wrote:
>
> > If the model you're proposing is "almost isomorphic" to roll ups, then
why
> > not just use roll ups?
> >
> > On Wed, Jun 22, 2016, 09:58 Bobby Holley  wrote:
> >
> > > I just chatted with Manish a little bit to sort out some details and
> > > misunderstandings, and I think we're much more on the same page now.
> > >
> > > A few high-level points worth emphasizing:
> > > * Making the Homu queue take longer would be bad and we should avoid
> > that.
> > > I believe the proposal would actually significantly reduce landing
> > latency.
> > > * The proposal does not involve hooking servo/servo up to
mozilla-inbound
> > > and its associated churn. As long as gecko development uses the
> > > mozilla-inbound model, servo/servo would be linked to a
much-more-stable
> > > "mozilla-servo" integration repo, which will have very few backouts
and
> > > should never be closed modulo infra issues.
> > > * Updating the code in both repos "in lockstep" is central to the
plan,
> > > because it prevents divergent forks.
> > > * The details of how/when tests are run involve a lot of tweakable
> > > parameters, and I am optimistic that we can adjust them to solve any
pain
> > > points.
> > >
> > > In a bit more detail, there are basically 3 ways to manage CI:
> > > (1) The mozilla-inbound model (land possibly-untested stuff, run CI
> > > post-commit, perform backouts and close tree as necessary)
> > > (2) The bors/homu model (run the full suite on the precise commit that
> > will
> > > be merged, and only merge it when green)
> > > (3) The relaxed bors/homu model: Run CI in parallel on pending
commits.
> > Any
> > > mergable commits with "recent enough" green CI runs can be merged. We
do
> > > additional periodic post-merge CI runs on the master branch to spot
the
> > > occasional intra-commit bustage.
> > >
> > > (1) obviously sucks and (2) doesn't scale well. The proposal is to do
> > > something like (3) for both projects [1]. This is the long-term plan
for
> > > Gecko development anyway (i.e. mandatory pre-commit testing), and
> > relaxing
> > > the strict homu/bors model would significantly increase responsiveness
> > and
> > > throughput on the servo/servo side.
> > >
> > > The question of exactly which pre-commit tests should be run for which
> > > commits is a heuristic one that we can tweak. For example, we may
decide
> > > that changes to components/style require pre-commit gecko test runs,
but
> > > version bumps on leaf-y dependencies are safe enough to leave to the
> > > post-commit sanity checking. We can play with that as we go.
> > >
> > > [1] It's worth noting that (3) is _almost_ isomorphic to what Rust
does
> > > (merging rollups of commits with green CI), which the primary
difference
> > > being whether rollups are tested pre- or post-commit.
> > >
> > > On Wed, Jun 22, 2016 at 9:03 AM, Boris Zbarsky 
wrote:
> > >
> > > > On 6/22/16 11:17 AM, Manish Goregaokar wrote:
> > > >
> > > >> We don't close 

Re: [dev-servo] Proposed work for upcoming sharing of Servo components with Firefox

[Bobby Holley]

One fundamental problem with precommit testing the precise commit to be
merged is that intermittents are more costly. You either have to hold up
the pipeline while you retrigger the intermittent job, or throw away all of
the green results you got and try again later. This doesn't scale to that
~600 per-commit CI jobs that we run on mozilla-central, where it's almost
unheard of to have an entirely green run with no intermittents at all. The
nice thing about combining pre-commit testing of the changesets (but not
precise commits) with post-commit testing is that intermittents can be
retriggered without holding anything up.

At a higher level: the advantage of the proposed approach (3) is that jives
very well with the way TaskCluster and TreeHerder currently work, which
makes it a very tractable model upon which to align both mozilla-central
and servo/servo. I think we should start with that as a minimum-viable
implementation, and then tweak design and algorithms if there are obvious
improvements that we can envision.

bholley

On Thu, Aug 4, 2016 at 12:19 PM, Michael Howell 
wrote:

> 04.1000 < jgraham> I'm pretty sure every discussion I've ever seen of
> commit queues has ended with someone saying "we should binary search on the
> queue"
>
> 04.1000 < jgraham> I'm also pretty sure that zero times so far has anyone
> actually implemented that proposal :/
>
> On Thu, Aug 4, 2016 at 11:45 AM Jack Moffitt  wrote:
>
> > > Actually (though I've been busy implementing other things), I've been
> > > planning a somewhat different way to solve the scalability problems
> that
> > I
> > > called "auto rollup."
> >
> > This is more or less the direction I've thought we'd move in as well.
> > Basically try in bigger batches but have blame pinpointing stay the
> > same.
> >
> > It does share one of the same drawbacks as the pipelining proposal,
> > which is that in the case that things aren't mostly succeeding by
> > default, it makes things slower. That said I think this is worth
> > trying.
> >
> > jack.
> > ___
> > 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
>
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] Filterable list of all CSS properties and their syntax

[Bobby Holley]

This is awesome, thanks Manish!

Given our focus on testing, at this stage I think the most useful
prioritization would be reftests. We could turn on logging for
unimplemented properties (and properties values), and then see which ones
are the most common on one of shing's reftest runs.

On Mon, Aug 29, 2016 at 8:37 AM, Manish Goregaokar 
wrote:

> I included the Alexa numbers (these are not exactly accurate because
> heycam's list counts property-value pairs, not properties, I need to write
> a better script for extracting these correctly).
>
> I also added a dashboard thingy that shows all the useful property counts
> in the corner.
>
> -Manish Goregaokar
>
> On Mon, Aug 29, 2016 at 4:41 PM, Manish Goregaokar 
> wrote:
>
> > On Mon, Aug 29, 2016 at 2:23 PM, Chris Peterson 
> > wrote:
> > > Stylo has implemented exactly one property Gecko has not: column-width.
> >
> > Oh, that's because Firefox implements it as -moz-column-width. I
> > haven't tried merging the prefixes -- this information can be scraped
> > from MDN, but it's only an issue for very few properties so I didn't
> > work on that.
> >
> > The Chrome/Firefox implementation info is just what getComputedStyle
> > contains. The Servo/Stylo implementation info is gotten by having the
> > mako templates print the longhand names (this ignores any mapping to
> > prefixed values that we do in stylo).
> >
> > I'll be sure to keep this up to date. While the MDN scraping script is
> > in the repo behind that page, the way of getting property data from
> > servo/stylo is a bit fiddly right now. I'll try to clean that up and
> > check in patches to that repo, but I'm fine with just doing the
> > updating myself.
> >
> > I'll also add counters to the page listing how many properties each
> > one implements, and note down if it's a top 500 property.
> >
> > Do you plan on prioritizing these properties (like how the Wikipedia
> > ones were prioritized)? I was thinking of adding a priority field, but
> > I didn't because I don't know how to prioritize most of these
> > properties. The Alexa numbers will certainly help, though.
> >
> > Thanks,
> > -Manish Goregaokar
> >
> ___
> 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

Re: [dev-servo] Mozilla's Project Quantum and Servo

[Bobby Holley]

On Fri, Oct 28, 2016 at 4:50 AM, Emilio Cobos Álvarez 
wrote:

> Here are the initial measurements that Cameron and Bobby took initially
> for Wikipedia and the HTML spec when prototyping Stylo:
>
>  * http://people.mozilla.org/%7Ebholley/stylo-london-2016/
> wikipedia-annotated.png
>  * http://people.mozilla.org/%7Ebholley/stylo-london-2016/
> spec-annotated.png
>
> That's for a full-page restyle. Now there's a bunch of work going on to
> make incremental restyling even more awesome :)
>
> There was a spreadsheet somewhere with more numbers, and more details
> about how the measurement was done (though the rough version IIRC is:
> delaying styling until the document was loaded for Stylo, and forcing a
> full-document restyle on stock Gecko), but I can't find the link right
> now, I've CC'd them.
>

The spreadsheet is at
https://docs.google.com/spreadsheets/d/1jwoH0B6kxYUp-a_OH147xz9IQHYJJW1D9CY8F9aLb0Y/edit#gid=0
. It's pretty old and rough though. We should have better numbers on stylo
once we get incremental restyle done.

As for the original question of "how much page load time is spent in the
style system", the answer depends a lot on the page. A rough heuristic I've
heard quoted is that it's half of layout, though after looking at various
measurements I think it may be a bit less than that on average. Hard to say
though.


>
>  -- Emilio
>
> On Thu, Oct 27, 2016 at 11:35:23AM -0700, mayankleob...@gmail.com wrote:
> > > The style system determines which CSS styles apply to HTML elements.
> > > In Servo, we have a fast, parallel implementation, and the performance
> > > scales linearly with the available hardware. The Stylo effort
> > > (https://wiki.mozilla.org/Stylo) replaces the style system in Firefox
> > > with Servo’s
> >
> >
> > How much of the total page load time  is spent in the style system?And
> how much of a speedup is there with a typical 4 core CPU? Assume the Alexa
> top 100 websites.
> > I would be very interested to see the numbers.
> >
> > TIA
> > ___
> > 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

Re: [dev-servo] string-cache (string interning) is now generic

[Bobby Holley]

This is awesome, and fixes a long-running pain point. Thanks Simon!

On Thu, Nov 3, 2016 at 12:17 PM, Simon Sapin  wrote:

> # How it worked until recently
>
> Servo uses a crate called string-cache for string interning. It defines an
> `Atom` type that represents a string (it dereferences to `&str`), but it
> take 8 bytes of stack space (whereas `String` take three times that on
> 64-bit systems) and is fast to compare for equality or to hash (only
> comparing/hashing a u64 value).
>
> The memory representation of Atom is one u64, whose lower 2 bits indicate
> one of three variants:
>
> * If the string is in a set known at compile-time, the upper 32 bits are
> an index. (This is a "static atom".)
> * Otherwise, if the string 7 bytes long or shorter, it is stored inline in
> the upper 56 bits and its length is stored in the next 4 bits. (This is an
> "inline atom".)
> * Otherwise, a `String` is stored in a global hash map whose entries are
> atomically reference-counted. The atom’s value is the address of the
> corresponding entry, cast to a pointer when needed. Entries are
> memory-aligned so that the lower bits of their address is zero. (This is a
> "dynamic atom".)
>
> Creating an `Atom` from `&str` involves hashing the string and has some
> computational cost. For strings in the static set, the `atom!` macro (used
> with a string literal: `atom!("foo")`) allows doing that computation at
> compile-time. It can also be used as a pattern in a `match` expression.
>
>
> # Why we changed it
>
> Adding a string to the static set (in order to use it with the `atom!`
> macro, or just to avoid some dynamic memory allocations) required:
>
> * Having a local fork of the string-cache repository
> * Making the addition
> * Making a pull request
> * Having a reviewer approve it
> * Waiting for Travis-CI to test it and homu to merge it
> * Having a reviewer publish a new version of string-cache on crates.io
> * Updating string-cache to the newly published version
>
> In particular, I want to use static atoms for all CSS property names.
> Doing so without changing string-cache would require contributors to go
> through all this when adding support for a new CSS property in Servo.
>
> This situation also forced non-Servo users of the crate to ship in their
> binaries a large number of static strings they might not care about.
>
>
> # What changed
>
> The goal was to move the set of static strings out of the string-cache
> crate, and allow users to define their own.
>
> It turns out we even want multiple crates in the same project to define
> their own static strings. For example, html5ever uses `atom!` for element
> and attribute names, and Servo’s style crate would use them for CSS
> property names, ideally with a static set generated automatically.
>
> So from string-cache 0.3, in https://github.com/servo/strin
> g-cache/pull/178 (based on initial work by @aidanhs, thanks again!), we
> made the `string_cache::Atom` type generic. It now takes a type parameter
> which provides the set of static strings.
>
> This type is typically not used directly anymore. Instead, the
> string_cache_codegen crate is intended to be used in a build script. Given
> static strings, a type alias name, and a macro name, it generates code that
> defines:
>
> * The appropriate data structure and trait impl for the hash set of static
> strings
> * A type alias like `type Foo = string_cache::Atom;`. (This
> is the type that is typically used.)
> * A macro like the former `atom!` that takes a string literal in the
> static set and expands to a value of that type.
>
>
> An important aspect is that atoms with different static sets are different
> Rust types. One can not be used in place of the other (conversion has to go
> through `&str`), and they can not be compared for equality (but their
> dereferenced `&str`s can).
>
>
> # How it’s used in Servo
>
> https://github.com/servo/html5ever now has a new html5ever_atoms crate
> that defines:
>
> * Three atom types, each with known common values in their static sets:
> `Prefix`, `Namespace`, and `LocalName`.
> * Respective macros `namespace_prefix!`, `namespace_url!`, and
> `local_name!`.
> * The `ns!` macro and `QualName` type previously in string-cache.
>
> Since https://github.com/servo/servo/pull/14043, the Servo repository
> contains a servo_atoms crate in ./components/atoms that defines an `Atom`
> type and corresponding `atom!` macro.
>
> `servo_atoms::Atom` is now used for everything else (other than prefixes,
> namespaces, element names, and content attribute names) that was previously
> `string_cache::Atom`.
>
> Note that the static set right now is just enough to make every usage of
> `atom!` work. If you had added strings (such as common attribute values) to
> string-cache’s static list purely to avoid dynamic atoms (with their memory
> allocation and reference-counting cost), they are likely not static
> anymore. (This is because I didn’t find a way to tell which item of the old
> set sho

Re: [dev-servo] string-cache (string interning) is now generic

[Bobby Holley]

On Thu, Nov 3, 2016 at 1:30 PM, Simon Sapin  wrote:

> On 03/11/16 21:02, Boris Zbarsky wrote:
>
>> On 11/3/16 3:17 PM, Simon Sapin wrote:
>>
>>> An important aspect is that atoms with different static sets are
>>> different Rust types.
>>>
>>
>> Just to check that I understand correctly...
>>
>> Are element names atoms?  Which static set do they come from?
>> Presumably the html5ever one?
>>
>
> Yes. Previously Servo had a single Atom type that was used, among other
> things, for element names.
>
> Now there is html5ever_atoms::LocalName (which is a type alias for
> string_cache::Atom), used for
> element and content attribute names.
>
>
> When parsing CSS selectors, are tag name selectors atomized?  Which
>> static set is used for that?
>>
>
> Yes. Types from the selectors crate are generic, with a trait that has
> associated types for the string types of various components.
>
> Servo defines:
>
> type AttrValue = std::string::String;
> type Identifier = servo_atoms::Atom;
> type ClassName = servo_atoms::Atom;
> type LocalName = html5ever_atoms::LocalName;
> type NamespacePrefix = html5ever_atoms::Prefix;
>
> Stylo defines:
>
> type AttrValue = gecko_string_cache::Atom;
> type Identifier = gecko_string_cache::Atom;
> type ClassName = gecko_string_cache::Atom;
> type LocalName = gecko_string_cache::Atom;
> type NamespacePrefix = gecko_string_cache::Atom;
>
> gecko_string_cache::Atom wraps *mut nsIAtom.
>
> (I don’t know if there’s a reason for the discrepancy: interning attribute
> values in selectors in Stylo but not Servo. We can change it.)
>

I did this for stylo because those attribute values needed to be passed
across the FFI boundaries, and passing atoms is much cheaper than passing
strings. There was some mechanical reason I couldn't do it easily for
Servo, but I don't remember what it was.


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

[dev-servo] PSA: Mac debugging tips for Servo and Stylo

[Bobby Holley]

First, make sure you're running XCode 8, since the lldb in XCode 7 is
busted for Rust in various ways [1]. A while ago I set up my system with
XCode 8 beta as a workaround, and then forgot about it. XCode 8 came out a
while ago, but it doesn't auto-update.

Second, the Rust optimization level for debug Gecko builds is currently
nonzero [2] to avoid test timeouts on CI. I recommend setting it to zero in
local debug builds to avoid missing breakpoints.

bholley

[1] https://github.com/rust-lang/rust/issues/33062
[2]
http://searchfox.org/mozilla-central/rev/f5c9e9a249637c9abd88754c8963ecb3838475cb/toolkit/library/rust/Cargo.toml#23
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] stylo: Consider spliting gecko bindings (style/gecko_{bindings, string_cache}) into a separate crate / component?

[Bobby Holley]

Given that we've already done the deed and merged the crates, I think we
should hold off on resplitting them until the stylo integration is more
mature. We may find other ways to achieve additional type safety with tight
coupling between generate bindings and other style code (like the
ElementData stuff we have now), and I wouldn't want crate separation to
stand in the way of that. We can revisit this once development slows down
and the tradeoffs are clearer.

If I understand correctly, the cost here is rebuilding the entire style
crate whenever the bindings change. I think that cost is probably
acceptable for now.

On Thu, Dec 15, 2016 at 11:46 AM, Manish Goregaokar 
wrote:

> I was actually intending to expand that set as necessary, a lot of the
> owned and arc types could use this trick too. A lot of this was held back
> on the Rust compiler losing inline drop flags (which happened recently).
>
>
> I'm okay with splitting it again, but would prefer to avoid as much as
> possible :)
>
> -Manish Goregaokar
>
> On Thu, Dec 15, 2016 at 12:50 AM, Xidorn Quan  wrote:
>
> > Oh, I see what did you mean. ElementData and AtomicRefCell would be a
> > problem. I don't see anything else, though.
> >
> > - Xidorn
> >
> > On Thu, Dec 15, 2016, at 05:29 PM, Manish Goregaokar wrote:
> > > The sugar stuff is just helpers. The crossover happens in the bindings
> > > file
> > > itself, where &ServoOpaqueType gets replaced with &RealType, etc.
> > >
> > > On Dec 14, 2016 5:13 PM, "Xidorn Quan"  wrote:
> > >
> > > > On Thu, Dec 15, 2016, at 12:01 PM, Manish Goregaokar wrote:
> > > > > They used to be a different crate. I merged them so that we can do
> > > > > replacements with safer wrappers and have fewer coherence issues.
> > > >
> > > > It doesn't seem to me the safer wrappers (I suppose the ones in
> > > > gecko_bindings/sugar?) needs the bindings to stay in the style
> > > > component. We can still leave gecko mod in style component, and just
> > > > split out the bindings mods. That shouldn't cause much trouble I
> > > > suppose.
> > > >
> > > > > Perhaps we can make triggering local build time bindgen regen more
> > > > > explicit?
> > > >
> > > > You mean, probably not listing that many files in
> > > > cargo:rerun-if-changed? That probably works I guess.
> > > >
> > > > - Xidorn
> > > > ___
> > > > 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
> > ___
> > 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
>
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] PSA: Style system changes now require documentation.

[Bobby Holley]

Of the 82 commits that landed on servo/servo this weekend, 73 were
emilio's. The combined patch weighed in at 540k, with 88 files changed,
2747 insertions(+), 1001 deletions(-).

Chapeau!

On Mon, Jan 2, 2017 at 6:04 AM, Emilio Cobos Álvarez 
wrote:

> [Bcc dev-tech-layout, for potentially-interested stylo people, please
> follow-up on dev-servo]
>
> This week I've landed three PRs[1][2][3], that add documentation to most
> of the style system code, and uses the standard Rust compiler lints to
> force it on new code. Thanks Boris for filling[4], btw.
>
> There is surely documentation that needs to be expanded. Please file
> issues for underdocumented stuff, or fill the gaps if you can.
>
> The setup is as follows:
>
> All the public code needs to be documented, and this is enforced via the
> compiler (that is: no docs, no build).
>
> There are few private things that may not be caught by the lint and I've
> tried to document. There's no standard way that I know of for preventing
> non-pub code to be undocumented. Non-public code is not that common
> though, so hopefully reviewers will require it to land changes if the
> code is not straight-forward.
>
> There are a few exceptions to the docs-required lint though:
>
>  * The media_queries module, which I'm rewriting as part of bug 1325878
>(didn't want to write throwaway docs).
>
>  * The atomic_refcell module, which Bobby was rewriting.
>
>  * The attr module, which is mostly DOM-related code.
>
> Finally, there's all the property parsing code. That is a _huge_ amount
> of code, and when I went through the missing docs I found myself writing
> a lot of repetitive documentation (given they're mostly straight-forward
> representations of the spec text). That means the lint is disabled for
> those modules.
>
> I think what we may want to do there is requiring at least a spec link
> per property (and then comments as the author and reviewers see fit).
> Also, a big comment describing the current Mako setup for both Servo and
> Stylo (which is sort of weird for newcomers) would be helpful.
>
> These last two bits are not done yet, and I won't have the cycles to do
> them this week (and probably next week, because of exams), so if someone
> wants to jump in and do it, please feel free to do so!
>
> Thanks for reading, and thanks to the reviewers for the feedback and the
> fast turnaround time, which made this way easier to land than expected!
>
>  -- Emilio
>
> [1]: https://github.com/servo/servo/pull/14801
> [2]: https://github.com/servo/servo/pull/14802
> [3]: https://github.com/servo/servo/pull/14819
> [4]: https://github.com/servo/servo/issues/14765
>
> ___
> 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

Re: [dev-servo] build-time bindgen support for Stylo has landed on mozilla-inbound

[Bobby Holley]

This is a really exciting step - thanks for making it happen Nathan!

On Fri, Jan 20, 2017 at 9:46 AM, Nathan Froyd  wrote:

> Bug 1302028, build-time bindgen support for Stylo, has landed.
> Bindgen support is automatically enabled for --enable-stylo builds
> (including stylo-incubator integration builds); use
> --disable-stylo-build-bindgen if you need to disable it for any
> reason.
>
> The bindgen support assumes that you have a clang 3.9 installation and
> looks for the `llvm-config` program on your PATH to confirm that
> assumption.  If `llvm-config` is not on your PATH, you can export its
> location in your mozconfig.
>
> Since mozilla-{inbound,central} do not have a servo/ import yet, we
> landed a dummy geckoservo crate to satisfy Cargo and to enable us to
> get all the Cargo features in the correct place.  There will need to
> be some manual fixup/overriding of our dummy geckoservo crate when
> mozilla-central containing bug 1302028 merges to stylo-incubator.
>
> Please ping me on IRC or bugzilla if there are any questions.
>
> Happy hacking,
> -Nathan
> ___
> 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

[dev-servo] 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
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] Proposal: move the source code for rust-selectors into servo/servo

[Bobby Holley]

On Wed, Feb 8, 2017 at 2:28 AM, Anthony Ramine  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  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

[dev-servo] Tentatively retiring incubator for stylo development

[Bobby Holley]

Our stylo workflow thus far has required a brave soul (usually heycam or
Manishearth) to periodically merge updates from mozilla-central and
servo/servo into incubator. We are now in a place where we can (hopefully)
remove this step.

Some new developments:
* As of last week, mozilla-central has a copy of servo/servo in the
top-level servo/ directory.
* As of yesterday, new PRs that land on servo/servo get automatically
landed to integration/autoland, which the sheriffs regularly merge to and
from mozilla-central.
* Manish is currently working on |cargo vendor|-ing geckolib's dependencies
into mozilla-central. Once this is done, we should be able to turn on
linux64-stylo jobs on autoland, inbound, and mozilla-central.

So I'd like to try retiring incubator and switching to mozilla-central for
stylo development. If it doesn't work, we'll learn something, and can fall
right back to incubator until we're ready to try again.

The servo/ directory is read-only except for the sync bot, and we're still
some time from being able to modify it and have the autolander mirror the
changes to servo/servo. This means that servo-side changes still need to
land on servo/servo. If there are interdependent gecko changes, the
servo-side changes should land first. Once they hit autoland, the developer
should promptly land the gecko-side changes to minimize the interval of
bustage.

This also means that we now have shorter cycle times between the
introduction and the detection of bustage. If your patch breaks something,
please fix it immediately to keep autoland green.

In summary, here's the new workflow - I'll send out another email when the
jobs are running and we can start using it:
(1) New stylo-related changes should land on autoland and servo/servo.
(2) Developers should use mozilla-central for local development.

Comments and discussion welcome. Thanks for everyone's work on this!
bholley
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] Proposal: move the source code for rust-selectors into servo/servo

[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

On Fri, Feb 10, 2017 at 4:00 PM, Jack Moffitt  wrote:

> 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.
>
> On Thu, Feb 9, 2017 at 10:40 AM, Gregory Szorc  wrote:
> >
> >
> >> On Feb 9, 2017, at 01:51, Anthony Ramine  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  a écrit
> :
> >>>
> >>> On Wed, Feb 8, 2017 at 2:28 AM, Anthony Ramine 
> 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 
> 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
> >>>
> >>>
> >>&

Re: [dev-servo] Tentatively retiring incubator for stylo development

[Bobby Holley]

Update: This is now on autoland, so please start using the new workflow.
Thanks to Greg and Manish for pushing it over the line!

The crashtest job is currently failing - I can't tell whether it's a
one-off (in which case we should disable the crashtest and file a bug) or
whether it portends more failures that we should probably look into
directly. I'll investigate soon if nobody else does before then.

Let's try to get our test jobs green so that we can keep them tier-2.

Cheers,
bholley

On Fri, Feb 10, 2017 at 12:32 PM, Bobby Holley 
wrote:

> Our stylo workflow thus far has required a brave soul (usually heycam or
> Manishearth) to periodically merge updates from mozilla-central and
> servo/servo into incubator. We are now in a place where we can (hopefully)
> remove this step.
>
> Some new developments:
> * As of last week, mozilla-central has a copy of servo/servo in the
> top-level servo/ directory.
> * As of yesterday, new PRs that land on servo/servo get automatically
> landed to integration/autoland, which the sheriffs regularly merge to and
> from mozilla-central.
> * Manish is currently working on |cargo vendor|-ing geckolib's
> dependencies into mozilla-central. Once this is done, we should be able to
> turn on linux64-stylo jobs on autoland, inbound, and mozilla-central.
>
> So I'd like to try retiring incubator and switching to mozilla-central for
> stylo development. If it doesn't work, we'll learn something, and can fall
> right back to incubator until we're ready to try again.
>
> The servo/ directory is read-only except for the sync bot, and we're still
> some time from being able to modify it and have the autolander mirror the
> changes to servo/servo. This means that servo-side changes still need to
> land on servo/servo. If there are interdependent gecko changes, the
> servo-side changes should land first. Once they hit autoland, the developer
> should promptly land the gecko-side changes to minimize the interval of
> bustage.
>
> This also means that we now have shorter cycle times between the
> introduction and the detection of bustage. If your patch breaks something,
> please fix it immediately to keep autoland green.
>
> In summary, here's the new workflow - I'll send out another email when the
> jobs are running and we can start using it:
> (1) New stylo-related changes should land on autoland and servo/servo.
> (2) Developers should use mozilla-central for local development.
>
> Comments and discussion welcome. Thanks for everyone's work on this!
> bholley
>
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] Tentatively retiring incubator for stylo development

[Bobby Holley]

On Fri, Feb 10, 2017 at 10:04 PM, Cameron McCormack  wrote:

> On Sat, Feb 11, 2017, at 11:54 AM, Bobby Holley wrote:
> > Update: This is now on autoland, so please start using the new workflow.
> > Thanks to Greg and Manish for pushing it over the line!
>
> Great work!
>
> > The crashtest job is currently failing - I can't tell whether it's a
> > one-off (in which case we should disable the crashtest and file a bug) or
> > whether it portends more failures that we should probably look into
> > directly. I'll investigate soon if nobody else does before then.
>
> The assertion failures are due to bug 1331294 landing.  The crash I'm
> not sure about.  I'll take a look.
>

I just did a few crashtest runs locally while. Adding skip-if(stylo) to
layout/base/crashtests/404218-1.xhtml and dom/xbl/crashtests/336960-1.html
lets us get through the crashtest (thought we still have the negative
leaks). If somebody has a minute to push that to autoland and file a
followup bug to investigate (NI me please) I'd appreciate it - I haven't
had time to set up MozReview on my new machine, so I can't push to autoland
myself.


> By the way, are we waiting until the leaks on the reftest jobs are
> solved before running them on autoland?
>

Not explicitly no - I'm not entirely sure why the reftest jobs aren't
running, but we could presumably turn them on. That said, I do think we
should establish a bit better hygiene than we had on incubator in terms of
only displaying green jobs (we can make non-green jobs hidden by default on
treeherder view). Once this hits central and inbound, we want developers to
notice when they break stylo rather than just glossing over it.

Looks like there's a solution for the "negative leaks" issue in bug
1334579. I'm not sure if anyone has investigated the calc leak yet, but we
should.

bholley
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

[dev-servo] Quantum CSS (stylo) builds now on m-c

[Bobby Holley]

Hot on the heels of Quantum Render, we now have linux64-stylo jobs running
on mozilla-central. There are a few remaining issues to sort out, but we
hope to have all the visible jobs greened up within the few days. Please
reach out to me directly with any questions or concerns.

Because the style system is tightly-coupled with DOM and layout, we are
tracking the Servo project in real time. Whenever a push lands in the Servo
repository, our infrastructure automatically synthesizes a corresponding
commit to m-c's servo/ directory and pushes it to integration/autoland.
Please take a moment to appreciate how cool this is - our automation has
come a long way.

There are two important pieces of our integration story that we still lack:
(1) Running Firefox CI on Servo commits.
(2) Landing atomic changesets that span both repositories (the servo/
directory is currently read-only except for the push bot).

Until those two things happen, occasional bustage from the automatic servo
updates will be unavoidable, but we hope to keep it to a minimum. If we
keep an eye on the tree and jump on things promptly, I'm hopefully that we
can contain it to a cycle or two on autoland, and avoid having it ever
reach mozilla-central and mozilla-inbound.

Since the jobs may bust, they are Tier-2. That said, please avoid adding
any new failures to green stylo jobs on mozilla-inbound and elsewhere. Our
schedule is tight, and I'd like to minimize the time we spend firefighting.

Running automation on mozilla-central is shaping up to be a game-changer
for our productivity. Thanks to everyone who made it possible.

bholley
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] Quantum CSS (stylo) builds now on m-c

[Bobby Holley]

On Mon, Feb 13, 2017 at 11:47 AM, Chris Peterson 
wrote:

> On 2/12/2017 7:31 PM, Bobby Holley wrote:
>
>> There are two important pieces of our integration story that we still
>> lack:
>> (1) Running Firefox CI on Servo commits.
>>
>
> Here you are talking about running Firefox tests on Servo pull requests on
> GitHub before they are merged to servo/servo, right?
>

Correct.


>
> We do run Firefox tests on the Servo changes imported from GitHub to the
> autoland repo:
>

Sure, but servo-side contributors don't see those results in their try
pushes, so the bustage needs to be dealt with after the fact. This is
doable for the interim, just not entirely optimal.


>
> https://treeherder.mozilla.org/#/jobs?repo=autoland
> ___
> 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

[dev-servo] PSA: Breaking updates to geckolib dependencies will break mozilla-central

[Bobby Holley]

Now that we have the VCS sync system running, changes to servo/servo are
immediately mirrored over to mozilla-central (via the
|integration/autoland| repo). Because mozilla-central uses |cargo vendor|,
the build will fail if a non-semver-compatible version bump occurs in the
dependency chain, and somebody needs to run |cargo vendor| again on the
mozilla-central side.

We're working to automate this, but in the mean time, please ping a stylo
developer if you're going to land such a change, so that we can be ready
with the followup fix on the Gecko side. Manish, emilio, heycam, xidorn,
and myself are all good bets.

Thanks for bearing with us through these turbulent but exciting times!
bholley
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] Useful tools for current Stylo workflow

[Bobby Holley]

Nice! That test expectation script in particular looks super useful. :-)

On Sat, Feb 18, 2017 at 2:58 AM, Manish Goregaokar 
wrote:

> I updated some of my scripts to the latest workflow, thought I would share
> them.
>
> To take a cinnabar servo branch based on `tip`, and remove all changes
> related to servo, run these two commands in succession:
>
>- git filter-branch --index-filter 'git reset @ servo && git checkout @
>-- servo' --prune-empty -f -- tip..
>- g rebase -i tip -x "git reset @^ servo && git checkout @^ -- servo &&
>git add -u servo && git commit --amend -C @"
>
> I'm not sure why the second is necessary, the first one works for all
> commits but the first in the series, and the second one is able to handle
> that. Just the second one won't work because it will cause merge conflicts;
> rebase -x needs to be able to make the commit before you modify it, while
> filter-branch modifies commits in-progress.
>
>
> I also have
> https://gist.github.com/Manishearth/1d1da99296dbff10c003bd3db4bf6e58 for
> quickly setting test expectations. You can just copy the error summary into
> a file and feed it to the script, which will update all the relevant list
> files.
>
>
> -Manish Goregaokar
> ___
> 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

Re: [dev-servo] PSA: Breaking updates to geckolib dependencies will break mozilla-central

[Bobby Holley]

It likely will, yes, because style depends on serde (via app_units), which
means that Gecko's Cargo.lock will be incompatible with the Cargo.toml tree
when the change lands.

On Fri, Feb 17, 2017 at 2:51 PM, Anthony Ramine  wrote:

> I'm not sure what you mean. Do you mean for example that my serde 0.8 to
> serde 0.9 bump will break m-c?
>
> > Le 17 févr. 2017 à 23:08, Bobby Holley  a écrit :
> >
> > Now that we have the VCS sync system running, changes to servo/servo are
> > immediately mirrored over to mozilla-central (via the
> > |integration/autoland| repo). Because mozilla-central uses |cargo
> vendor|,
> > the build will fail if a non-semver-compatible version bump occurs in the
> > dependency chain, and somebody needs to run |cargo vendor| again on the
> > mozilla-central side.
> >
> > We're working to automate this, but in the mean time, please ping a stylo
> > developer if you're going to land such a change, so that we can be ready
> > with the followup fix on the Gecko side. Manish, emilio, heycam, xidorn,
> > and myself are all good bets.
> >
> > Thanks for bearing with us through these turbulent but exciting times!
> > bholley
> > ___
> > 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
>
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] Stylo binding files are now downloadable from TreeHerder

[Bobby Holley]

Sounds sensible - thanks Xidorn!

On Fri, Mar 31, 2017 at 5:44 PM, Xidorn Quan  wrote:

> With bug 1350810 [1], Gecko's CI now uploads its generated binding files
> as an artifact for stylo builds. It is listed as
> "target.stylo-bindings.zip" in Job Details of stylo build tasks [2].
>
> So you no longer need to do an independent geckolib build to update the
> binding files before creating Servo side pull request (IIRC some of
> stylo developers do this). You can simply download the binding files
> from your try push. And it makes it easy to generate diff of binding
> files without local rebuilds.
>
> I would like to propose that we use the binding files generated from
> Gecko's CI as the single source of binding files we check in to Servo
> tree, because that can eliminate difference between platforms, and thus
> minimize changes between commits, which hopefully reduces possibility of
> conflict.
>
> [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1350810
> [2] e.g.
> https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=
> 31810a9548fcede48be099fc9823fd2710616d64&selectedJob=87929391
>
> - Xidorn
> ___
> 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

Re: [dev-servo] Incremental compilation and other compile time tricks

[Bobby Holley]

That's awesome - thanks Simon!

On Wed, Apr 5, 2017 at 3:13 PM, Simon Sapin  wrote:

> On 31/03/17 19:43, Simon Sapin wrote:
>
>> # cargo check
>>
>> Remember how I said a crate needs its dependencies to be compiled before
>> it can start compiling? It actually only needs metadata (the results of
>> the analysis phase), not the generated code.
>>
>> Cargo added a `cargo check` command that skips code generation entirely
>> (except for build scripts and build-dependencies). This saves a lot of
>> time (I’ve measured up to 5× speed from `./mach clean`), though
>> obviously you don’t get an executable at the end. Still, it can help for
>> example during a refactoring: run `./mach cargo check` many times until
>> compiler error are resolved, and only then run `./mach build` and/or
>> `./mach test-unit`.
>>
>> https://github.com/rust-lang/cargo/pull/3296
>> https://github.com/servo/servo/pull/14594
>>
>
> With a couple pull requests that just landed, "./mach cargo-geckolib
> check" now also works. On a fast desktop it runs in 28 seconds after "cargo
> clean" and 19 seconds after "touch components/style/lib.rs".
>
>
> --
> 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

Re: [dev-servo] All properties implemented in Stylo!

[Bobby Holley]

Great work everybody! A huge thanks to the entire servo community who
volunteered so many hours of time to getting us across the finish line. We
couldn't have done it without so much enthusiastic help!

On Sun, May 28, 2017 at 11:07 PM, Manish Goregaokar 
wrote:

> As of this week, all supported Firefox properties, including internal-only
> properties, have been implemented in Stylo.
>
> https://manishearth.github.io/css-properties-list/?stylo=
> hide&servo=show&firefox=only&chrome=show&mdn=false&alexa=false
>
> Some of these properties are still incomplete in their implementation (the
> basic property is supported but not some specific value), or may have bugs,
> but that will get fixed as we attack the reftests.
>
>
> Thanks,
> -Manish Goregaokar
> ___
> 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

[dev-servo] custom derive for Deref/DerefMut

[Bobby Holley]

There's a lot of boilerplate involved just to make a newtype [1]. Is this
something we could add a custom derive for?

[1]
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9adf7a220ac8fa870f30a25a1ff8c71da65873fa
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] custom derive for Deref/DerefMut

[Bobby Holley]

Sorry, wrong link. I meant:
https://reviewboard.mozilla.org/r/157064/diff/3#index_header

On Thu, Jul 13, 2017 at 5:13 PM, Bobby Holley  wrote:

> There's a lot of boilerplate involved just to make a newtype [1]. Is this
> something we could add a custom derive for?
>
> [1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=
> 9adf7a220ac8fa870f30a25a1ff8c71da65873fa
>
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] custom derive for Deref/DerefMut

[Bobby Holley]

On Fri, Jul 14, 2017 at 2:24 AM, Anthony Ramine  wrote:

>
> > Le 14 juil. 2017 à 02:13, Bobby Holley  a écrit :
> >
> > There's a lot of boilerplate involved just to make a newtype [1]. Is this
> > something we could add a custom derive for?
>
> We don't use that many newtypes to justify writing a custom derive for
> that IMO. Often we don't even bother and just do wrapper.0.
>

Well, we all do that because writing 10 lines of boilerplate is usually
worse than scattering some .0s around. If there were a 1-line derive we
might use it more.


> Remove all the unneeded where clauses for E on the type definition, and on
> the Deref* impls and you have only ~10 lines of boilerplate.
>
> ___
> 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

Re: [dev-servo] custom derive for Deref/DerefMut

[Bobby Holley]

I guess? It's certainly less nice than #[derive(Deref, DerefMut)] on the
type, but maybe there are some downsides of derives that I'm not aware of.

On Sat, Jul 15, 2017 at 10:27 AM, Jim Blandy  wrote:

> A macro_rules macro wouldn't serve here?
>
> On Fri, Jul 14, 2017 at 5:26 PM, Bobby Holley 
> wrote:
>
> > On Fri, Jul 14, 2017 at 2:24 AM, Anthony Ramine 
> wrote:
> >
> > >
> > > > Le 14 juil. 2017 à 02:13, Bobby Holley  a
> > écrit :
> > > >
> > > > There's a lot of boilerplate involved just to make a newtype [1]. Is
> > this
> > > > something we could add a custom derive for?
> > >
> > > We don't use that many newtypes to justify writing a custom derive for
> > > that IMO. Often we don't even bother and just do wrapper.0.
> > >
> >
> > Well, we all do that because writing 10 lines of boilerplate is usually
> > worse than scattering some .0s around. If there were a 1-line derive we
> > might use it more.
> >
> >
> > > Remove all the unneeded where clauses for E on the type definition, and
> > on
> > > the Deref* impls and you have only ~10 lines of boilerplate.
> > >
> > > ___
> > > 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
> >
> ___
> 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

Re: [dev-servo] Categorizing Stylo reftest failures

[Bobby Holley]

Great work Manish!

On Wed, Aug 9, 2017 at 6:53 PM, Manish Goregaokar 
wrote:

> https://gist.github.com/Manishearth/086118c940ff86a6cfc573f53c508279
>
> I've now gone through the failures and categorized them as yes/no/maybe
> based on whether they should block shipping. I've filed bugs for and
> partially investigated all of the "yes" ones (and fixed some of them). Some
> of these bugs are unassigned; feel free to pick them up!
>
> The "maybe" ones are ones which I don't *think* are important but I haven't
> had a chance to properly investigate yet. I will do that soon.
>
>
> We're looking pretty good on this front, only a couple of "yes" failures
> left.
>
> There may have been additional failures introduced since I generated this
> doc (I removed new passes but did not add new failures), which  I also
> intend to go through at some point.
>
>
> Thanks,
> -Manish
> ___
> 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

Re: [dev-servo] Disallowing warnings on all crates?

[Bobby Holley]

Could we modify the pragma to include the maximum error number that is
known to be supported, and make it a compiler policy for new warnings that
might impact existing code to have monotonic numbering?

On Dec 15, 2017 10:48, "Mike Hommey"  wrote:

> On Fri, Dec 15, 2017 at 04:40:42PM +0100, Emilio Cobos Álvarez wrote:
> > Hi,
> >
> > Tigercosmos sent a patch today adding #![deny(warnings)] to a bunch of
> > crates[1].
> >
> > Just wanted to give a heads-up / ask whether there's any objection to
> > the change before going ahead and r+ it. Does it sound reasonable to
> > everyone? It may make the servo-with-rust-nightly build fail a bit
> > earlier, but probably that's good, actually.
>
> From a Firefox point of view, #![deny(warnings)] is already causing
> problems, and I don't think it's a good idea to have that in source,
> essentially for the same reason it's not a good idea to have -Werror set
> by default on C/C++ projects.
>
> Now, to be more specific about what problems we're already seeing with
> #![deny(warnings)], they are around the fact that when you bisect, the
> build fails on older code that triggers warnings that didn't exist in
> the stable rust compiler back when the code was written, but that do in
> the newer compiler, which you're using while bisecting.
>
> I'd actually go as far as saying that #![deny(warnings)] shouldn't even
> exist in rust at all for this reason, and CIs should be setting the
> equivalent to -Werror (which I think -D warnings is).
>
> Mike
> ___
> 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

[dev-servo] PSA: Avoid invoking Debug formatters in release-mode Rust

[Bobby Holley]

TL;DR: To prevent code bloat, avoid {:?} in format strings for panic!(),
unreachable!(), error!(), warn!(), and info!() for Rust code that ships in
Gecko.

Longer version:

One nice thing about Rust is that you can #[derive(Debug)] for a type, and
the compiler will generate a stringification method, which may in turn be
inductively defined in terms of the Debug implementations of member types.
This is great for logging and debugging, but the implementations can be
quite large depending on the types involved.

The linker will generally eliminate unused Debug implementations as dead
code, but can't do so if they might be invoked in release builds. The most
common way this seems to happen is in panic!() messages, where it can be
tempting to include a stringified value to make the message more
informative. It can also happen for the logging macros that don't get
compiled out of release builds, which (at least for stylo) are info!(),
warn!(), and error!() [1].

To demonstrate what's at stake here, this trivial patch eliminates more
than 80K from libxul: https://github.com/servo/servo/pull/19756

Given how easy it is to mess this up and pull tons of unnecessary code into
Firefox, and given that it's rather time-consuming to notice the problem
and track down the culprit, I think we're best off categorically avoiding
this pattern.

Comments and alternative proposals welcome.

bholley


[1]
https://searchfox.org/mozilla-central/rev/7fb999d1d39418fd331284fab909df076b967ac6/servo/ports/geckolib/Cargo.toml#21
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] PSA: Avoid invoking Debug formatters in release-mode Rust

[Bobby Holley]

(From your next message it sounds like there's no disagreement here, but I
wanted to get the reasoning written down)

On Sat, Jan 13, 2018 at 6:36 AM, Anthony Ramine  wrote:

> I would much rather prefer if we just checked that we didn't use the Debug
> impls of large types.


The issue here is that it's difficult to tell at a glance how big a Debug
impl will be, especially at the call site. Generics make this extra
complicated, and even a couple of "small" stringifiers can easily add up.

Moreover, a case-by-case analysis makes it hard to do enforcement with
tooling. That said, if the tooling supports a whitelist, I'm totally fine
with allowing exceptions when we have a good reason. I'm proposing some
tooling in bug 1430678.

> We could also just not derive them in release mode.
>

I believe debug!() statements are type-checked in release-mode compiles, so
this would cause our logging code to break.


>
> In the PR you link, AFAICT you also removed some uses that were just very
> small enums or even integers, which may not be necessary.
>
> > Le 13 janv. 2018 à 06:07, Bobby Holley  a écrit :
> >
> > Comments and alternative proposals welcome.
>
> ___
> 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

[dev-servo] Reduced Build Times for Stylo Code

[Bobby Holley]

Just a happy update that the code size reduction work we've been doing on
the style system also seems to have made an impact on build times.

Over the past week, we've removed almost half a megabyte of code from
libxul. On my linux64 ThinkStation, an optimized build of the style system
[1] seems to have gone from 3:11 to 2:30 over that same timeframe, which is
a ~20% reduction.

Hopefully this will translate to better productivity for everyone hacking
on Gecko. It's also a good reminder that code size is correlated with build
times, giving us an extra incentive to reduce it where we can [2].

Big thanks to everyone who's been helping out here, especially Anthony
Ramine (nox), whose overhaul of the serialization code likely had the
single biggest impact.

Finally, I wanted to share one trick that may not be obvious to everyone.
If you find yourself triggering rebuilds of the style system after changing
C++ header files, and you're sure that no relevant memory layouts have
actually changed, you can interrupt the Rust compilation after the C++ code
has linked, and your changes should still take effect. YMMV, but I do this
a lot.

Cheers,
bholley

[1] ./mach clobber && ./mach build && touch layout/style/nsStyleStruct.h &&
./mach build toolkit/library/rust
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1432996
___
dev-servo mailing list
dev-servo@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-servo

Re: [dev-servo] Tidy lints

[Bobby Holley]

Yeah. It's probably less important exactly what the order is and more that
there's a canonical order that a tool can detect and automatically fix
things to match. rustfmt fits the bill, so I think it makes sense to retire
that particular tidy lint.

On Sun, Aug 19, 2018 at 5:18 AM Anthony Ramine  wrote:

> I don't know, but if your question is "would you mind using a tool to
> automate that stuff instead of a lint that complains?" then my answer is
> "yes, I too don't enjoy reordering things by hand".
>
> > Le 19 août 2018 à 13:58, Emilio Cobos Álvarez  a
> écrit :
> >
> > Can we enable import reordering in rustfmt instead?
>
> ___
> 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