Open Bug 1381629 Opened 8 years ago Updated 1 year ago

stylo: Improve the workflow for landing patches that don't touch style system code like bug 1377993

Categories

(Core :: CSS Parsing and Computation, enhancement, P4)

Product:
Core
Component:
CSS Parsing and Computation
Type:
enhancement

Tracking

()

Tracking Flags:
Tracking Status
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- ?

People

(Reporter: emilio, Unassigned)

References

Details

Attachments

(1 obsolete file)

Olli complained (with a point, IMO), that having to land https://github.com/servo/servo/pull/17758 separately in order to land bug 1377993 is stupid. I think for this kind of patches where we're not actually touching any servo-affecting code, we could afford to land directly on autoland, with the bindings updated from the taskcluster build... In particular, there are a few paths that aren't compiled in servo mode, so can't break servo's CI: * servo/components/style/gecko/* * servo/components/style/gecko_bindings/* * servo/components/style/gecko_string_cache/* * servo/ports/geckolib/* I can see how frustrating is to try to land bug 1377993... WDYT about allowing a minimal set of two-way sync for these patches? Would it be too annoying?
Probably "CSS Parsing and computation" isn't the most appropriate module, feel free to change it to whatever you think it fits more.
Blocks: stylo
(In reply to Emilio Cobos Álvarez [:emilio] from comment #0) > I can see how frustrating is to try to land bug 1377993... WDYT about > allowing a minimal set of two-way sync for these patches? Would it be too > annoying? Managing the coherency of bidirectional syncing is a known unsolved problem. We were promised a better workflow for landing changes that touch both projects, but it never materialized...
That can break Servo CI as far as it still does geckolib build...
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #3) > That can break Servo CI as far as it still does geckolib build... How would it break (without breaking Gecko), assuming we use the updated bindings from TC?
Hmmm... that's fair theoretically. There could be race condition, though. Also Gecko builds don't run test-stylo yet, which can also break Servo CI without breaking Gecko.
We could also consider, presumably, once stylo has stabilized more, to move all the Gecko only code to m-c, and treat style as a normal vendored dependency... wdyt?
I don't think treating style as a normal vendored dependency is going to work... but I guess it may make sense to not build geckolib in Servo CI so we don't need to keep the generated files in tree. We should run test-stylo in Gecko CI and just follow backout flow if Servo pr happens to break Gecko. That should simplify things significantly.
It's unknown, though, how frequently would Servo pr break Gecko if we don't have Servo CI checking geckolib build...
Why not? I think that'd be nice, eventually, specially if we manage to be able to define CSS properties from mozilla-central in the form of a toml file or something like that. It works for the WR people, and WR is also a pretty intrincate dependency... They probably have a few more regressions than stylo because we notice them earlier, but we'd annoy less everyone else working on central.
(In reply to Emilio Cobos Álvarez [:emilio] from comment #9) > It works for the WR people, and WR is also a pretty intrincate dependency... The dependency is nowhere near as tight as it is for the style system. More to the point, we've discussed the various configurations and tradeoffs endlessly in the past. The solution we came up with was to solve the problem with tooling, but resources were reassigned and the tooling never got built. So our tooling sucks, but fixing it will take time and organizational energy, and so if we want to ship 57 we're going to have to live with it. Let's focus on making the code great, and rethink the repo setup when we ship.
(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #10) > So our tooling sucks, but fixing it will take time and organizational > energy, and so if we want to ship 57 we're going to have to live with it. > Let's focus on making the code great, and rethink the repo setup when we > ship. Right, to be clear, I'm quite sure we can't change the setup right now, because how so fast stylo is moving right now. Thus the condition of: > once stylo has stabilized more I'm just saying that we should eventually get this fixed, and I personally think that going the "treat the style crate as another rust dependency" is the sanest way to do so.
(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #10) > So our tooling sucks, but fixing it will take time and organizational > energy, and so if we want to ship 57 we're going to have to live with it. > Let's focus on making the code great, and rethink the repo setup when we > ship. Right now I as a someone who thought to be a core DOM developer can't land patches changing code which is supposed to be very internal code DOM code. I do expect more changes to come while we're improving performance. Could we break Gecko development less and perhaps move the possible temporary breakage elsewhere (would it be Servo?)? (I wish I understood why Stylo doesn't live in mozilla hg and Servo could clone/merge it from there.)
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #7) > I guess it may make sense to not build geckolib in Servo CI […] > and just follow backout flow if Servo pr happens to break Gecko. That is unacceptable. Servo backouts only exist on the premise that they are exceptionally rare. (In reply to Xidorn Quan [:xidorn] UTC+10 from comment #8) > It's unknown, though, how frequently would Servo pr break Gecko if we don't > have Servo CI checking geckolib build... No, that is known. Servo pull requests that fail `mach build-geckolib` but pass every other CI job happen all the time.
(In reply to Olli Pettay [:smaug] from comment #12) > (In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #10) > > So our tooling sucks, but fixing it will take time and organizational > > energy, and so if we want to ship 57 we're going to have to live with it. > > Let's focus on making the code great, and rethink the repo setup when we > > ship. > > Right now I as a someone who thought to be a core DOM developer can't land > patches changing code which is supposed to be very internal code DOM code. You can land them, you just need to ping one of the many helpful servo developers to help you do it. > I > do expect more changes to come while we're improving performance. > Could we break Gecko development less and perhaps move the possible > temporary breakage elsewhere (would it be Servo?)? > (I wish I understood why Stylo doesn't live in mozilla hg and Servo could > clone/merge it from there.) You might find the FAQ section of https://docs.google.com/document/d/1tyNKLsgaVn1uQ69FVeM3I4RhuhSI_ibz4vYPDM85ZC4/ enlightening.
(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #2) > Managing the coherency of bidirectional syncing is a known unsolved problem. > > We were promised a better workflow for landing changes that touch both > projects, but it never materialized... > More to the point, we've discussed the various configurations and tradeoffs > endlessly in the past. The solution we came up with was to solve the problem > with tooling, but resources were reassigned and the tooling never got built. the focus was shifted from co-landing to first being able to backout servo changes that break firefox ci; that _was_ built and is now in production.
As a followup here - Glob is working on the co-lander this quarter, which will allow people to directly land cross-cutting changes to the two repositories. It will be integrated into the Phabricator/autoland setup that will be rolled out as the recommended workflow at the end of the year.
Priority: -- → P4
Severity: normal → S3
Attachment #9387436 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: