1977641 - `MachCommandConditions` that use `substs` fail with misleading message after `./mach clobber`

Status: REOPENED

(Firefox Build System :: Mach Core, defect, P5)

Description

[Ben Dean-Kawamura [:bdk]]

If I run ./mach rusttest after ./mach clobber I get the following error:

It looks like you tried to run a mach command from an invalid context. The rusttests
command failed to meet the following conditions:

  is_non_artifact_build - Must not be an artifact build.

If I run ./mach build export, then ./mach rusttest works again.

Comment 1

[Alex Hochheiden [:ahochheiden] [he/him]]

The problem makes sense. It's a condition for rusttests, which returns False if there is no substs, and there won't be any after a clobber, since config.status has been blown away.

I suppose we could automatically run ./mach configure if <topobjdir>/config.status is missing for all MachCommandConditions?

Comment 2

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 1977641 - Run `./mach configure` if a mach command condition can't be checked without `config.status` r?#build

Comment 3

[Alex Hochheiden [:ahochheiden] [he/him]]

Attachment: Bug 1977641 - Add new arg to `./mach format` to explicitly allow skip checking if android formatters are valid in this context r?sheehan

With an upcoming change to conditions.is_android it will require config.status to exist. If it does not
exist ./mach configure will be called. In the case where configure does not succeed, we explicitly fail.

Lando depends on ./mach format, but Lando cannot guarantee that ./mach configure will succeed (since it
depends on ./mach boostrap, and we've had issues with bootstrap failing on the landing worker and preventing
all landings). We also can't do a try/catch, since it configure fails in the try/catch, it will always try again
on subsequent landings, wasting a bunch of time on every landing.

As such, it seems like the best option is to just not check for this condition in Lando, and to do that we'll
use an explcit arg. In Lando conditions.is_android always evaluated False anyway, so this isn't changing any
behavior, just making the upcoming patch compatible with Lando.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/7585337c3cf5
https://hg.mozilla.org/integration/autoland/rev/26f008e9be22
Add new arg to `./mach format` to explicitly allow skip checking if android formatters are valid in this context r=sheehan

Comment 5

[[e-mail]]

https://hg.mozilla.org/mozilla-central/rev/26f008e9be22

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/e094d6932971
https://hg.mozilla.org/integration/autoland/rev/0d66a369dcbf
Run `./mach configure` if a mach command condition can't be checked without `config.status` r=firefox-build-system-reviewers,glandium

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/a6a559f1189b
https://hg.mozilla.org/integration/autoland/rev/c85ae0773654
Revert "Bug 1977641 - Run `./mach configure` if a mach command condition can't be checked without `config.status` r=firefox-build-system-reviewers,glandium" for causing try failures

Comment 8

[Cristina Horotan [:chorotan]]

Backed out for causing try failures

Backout link

Push with failures

Failure log
Failure log mpu

Comment 9

[Alex Hochheiden [:ahochheiden] [he/him]]

There's various contexts where we can't run ./mach configure, but MachCommandConditions are being used (and probably not being evaluated correctly, since there's no config.status present). I've concluded it's not ideal, but it's not worth hunting down and finding a workaround for these edge cases for the build team. This is a low priority for us, but a contributor would be welcome to pick this up and get it across the finish line.

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Firefox Build System::Mach Core' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.