2029056 - Enable autoformatting on uplift trains

Status: RESOLVED FIXED

(Conduit :: Lando, enhancement, P3)

Description

[Connor Sheehan [:sheehan]]

We currently only enable autoformatting for landings to autoland. We should be able to enable autoformatting on beta/release/esr* as well.

OTOMH, we may need to account for a lack of mach format on older branches, probably by allowing each train to define an autoformatting command, or by uplifting mach format.

Comment 1

[Connor Sheehan [:sheehan]]

Ryan, I'd like to try turning autoformatting on for landings to beta. We can let it bake for a short while to determine if there are any issues, and then make a decision about further rollout to release/esr. There may be some extra work to enable on esr/release (potential backport of mach format, or completing bug 1932005). Roll back can happen at any time by flipping a config knob in the Lando admin panel.

Is there a optimal time in the next little while where we could make this switch?

Comment 2

[Ryan VanderMeulen [:RyanVM]]

302 Donal as Beta will be under his purview for the upcoming cycle starting next week.

Comment 3

[Donal Meehan [:dmeehan]]

:sheehan, I'm ok with trying it in the next beta cycle.
You can enable it whenever you like, since there won't be any beta uplifts until next week.

Comment 4

[Connor Sheehan [:sheehan]]

I started poking at this and ran into a few hurdles:

• We run mach bootstrap to install formatters, however by default mach bootstrap uses the user's homedir for mozbuild state.
• Since we use the homedir by default, running autoformatting on an uplift train would use the autoland formatter versions, which aren't guaranteed to be backward compatible with newer versions.
• We also have the problem of bug 2030921, and the suggested fix of running rustup install stable will install the toolchain globally.

We can work around this issue by using a per-checkout MOZBUILD_STATE_PATH directory when running mach commands in the landing worker. That way toolchains are installed in a per-checkout location. Alex also suggested using ./mach artifact toolchain --from-build linux64-rust to install rust at our desired version - perhaps we could apply this approach to all our required toolchains for consistency.

I think the next steps are as follows:

• Add a custom MOZBUILD_STATE_PATH to our current autoland-based formatting.
• Add a mach artifact toolchain to worker setup to handle bug 2030921 on autoland.
• Enable autoformatting on non-autoland branches as previously described.

Comment 5

[BMO Github Automation]

Attachment: [mozilla-conduit/lando] repo: create srcdir-specific `MOZBUILD_STATE_PATH` and pass to `mach` commands (Bug 2029056) (#1176)

Comment 6

[Connor Sheehan [:sheehan]]

Discussing with :shtrom, it would be ideal if we had a mechanism to force an update of the toolchains from the admin interface. Something like a flag on the Worker model that when flipped will cause the toolchains to be refreshed the next time a job for a specific repo is picked up.

Comment 7

[BMO Github Automation]

Authored by https://github.com/cgsheeh
https://github.com/mozilla-conduit/lando/commit/374daeb05fb5229748cdd6d3188ab9a653b96140
[main] repo: create srcdir-specific MOZBUILD_STATE_PATH and pass to mach commands (Bug 2029056) r=shtrom,zeid

Comment 8

[BMO Github Automation]

Attachment: [mozilla-conduit/lando] repo: add configurable autoformat setup command sequence (Bug 2029056) (#1205)

Comment 9

[BMO Github Automation]

Authored by https://github.com/cgsheeh
https://github.com/mozilla-conduit/lando/commit/724e15eaaf654a457a167803714208770ae1e5b5
[main] repo: add configurable autoformat setup command sequence (Bug 2029056) r=shtrom,zeid

Comment 10

[Connor Sheehan [:sheehan]]

:dmeehan, the pieces are in place for us to try enabling autoformatting on beta. Can you give me the OK to enable it?

We can disable it at any time if problems arise, it's controlled by a config flag in the Lando admin.

Comment 11

[Donal Meehan [:dmeehan]]

302 to RyanVM for awareness, now that 152 is in beta

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Go for it, I guess

Comment 13

[Connor Sheehan [:sheehan]]

Okay, autoformat is enabled on beta. We will let it bake and then we can try on release/esr.

Comment 14

[Connor Sheehan [:sheehan]]

Heads up: I'm planning to enable this on release/esr as well, starting tomorrow.

Comment 15

[Connor Sheehan [:sheehan]]

We now have autoformatting running on all uplift trains.

Bug 2046117 added better tracking of changes made by autoformatting, and once bug 2046251 is landed we will have that data available in BigQuery.

Comment 16

[Connor Sheehan [:sheehan]]

I had to disable autoformatting on ESR* due to differences in mach format that hadn't been uplifted yet. Deployed Bug 1932005 to work around them, it seems to have worked for esr140 but esr115 is still having trouble. I've disabled on esr115 for now, and will fix that in a follow-up.