Closed Bug 1496059 Opened Last year Closed Last year

Clean up the MSVC jobs we run in CI

Categories

(Firefox Build System :: General, enhancement)

enhancement
Not set

Tracking

(firefox64 fixed)

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: RyanVM, Assigned: RyanVM)

Details

Attachments

(4 files)

Now that we're confirmed to be shipping Fx63 with clang-cl enabled, I'd like to clean up to MSVC jobs we run in CI.

1.) Turn off PGO for the opt builds. These builds will be running to ensure correctness, not performance. Let's save the build machine time and turn off PGO.
2.) Turn off tests. These builds aren't perf-critical anymore and Talos uses physical machine resources. Additionally, we don't typically run tests on builds we don't ship binaries for (good examples would be MinGW and Linux base-toolchain builds).
3.) Turn the builds on for every push in CI. Running them only m-c has predictably led to a number of build bustage bugs being filed long after the fact when they could have been caught much earlier if running on integration branches.
4.) Make the builds Tier 1. If we intend to keep MSVC supported and working long-term (and it sounds like that's the current plan), breaking the build should lead to an immediate backout.
(In reply to Joel Maher ( :jmaher ) (UTC-4) from comment #1)
> so we are not going to have pgo anymore?

Only on the -msvc builds, which aren't what we ship anymore. The clang-cl builds we ship will continue to have PGO builds.
For posterity we should record the intent in a bit more detail.

> If we intend to keep MSVC supported and working
> long-term (and it sounds like that's the current plan),

Why is that? Off the top of my head, the experimental AArch64 builds currently depend on MSVC, but presumably at some point the clang-cl bugs blocking that will be resolved. (Or maybe we'll stop doing those builds.)

Are we intending to keep MSVC as an escape hatch for developers who hit an LLVM bug or annoyance? Are we trying to leave ourselves open to a future where MSVC makes great advances and we want to switch back? Other reasons that I've missed? We should have an answer to "why don't you turn them off altogether?".

> These builds will be running to ensure correctness

This seems to be at odds with "Turn off tests." It's pretty much a given that anything we don't test will become broken quite quickly. Should the statement be reduced to "ensure buildability"?

I am not saying that I'm opposed to doing any of items 1-4. I just want us to be clear about what our purpose and expectations are.
(In reply to David Major [:dmajor] from comment #3)
> Why is that? Off the top of my head, the experimental AArch64 builds
> currently depend on MSVC, but presumably at some point the clang-cl bugs
> blocking that will be resolved. (Or maybe we'll stop doing those builds.)
> 
> Are we intending to keep MSVC as an escape hatch for developers who hit an
> LLVM bug or annoyance? Are we trying to leave ourselves open to a future
> where MSVC makes great advances and we want to switch back? Other reasons
> that I've missed? We should have an answer to "why don't you turn them off
> altogether?".

Right now, "AAarch64 builds need it" seems like a valid reason to keep them working and supported. Once we're successfully building AArch64 on clang-cl, we can certainly revisit whether we should support MSVC at all. But that seems more like a follow-up concern rather than an immediate one.

> This seems to be at odds with "Turn off tests." It's pretty much a given
> that anything we don't test will become broken quite quickly. Should the
> statement be reduced to "ensure buildability"?

Yes, "correctness" was a poor choice of words on my part.
I think "we have not made the decision to *un*support MSVC, so we will ensure that Firefox continues to build with it until such time as we do" is fine. It's a decision that merits some discussion and we're just punting that down the road for a while. If keeping the MSVC builds working turns out to be a large drag we can certainly have that discussion!
My hunch is that disabling tests will precipitate an un-supporting of MSVC sooner than we'd have otherwise planned, when the builds become unusable even for local debugging due to an accumulation of bugs. In other words, the actions we take now are not independent of the actions we take down the road.

I'm completely fine with saying we'll just deal with that as it happens, as long as we're explicit about it.
That seems fine. There are lots of ways to build Firefox that we don't test in CI, or don't test exhaustively, that still work for some people. We just don't make any guarantees about them.

I will say that I think we should probably leave these at tier-2, since tier-1 has historically meant "a thing we fully support":
https://developer.mozilla.org/en-US/docs/Mozilla/Supported_build_configurations
I had proposed Tier 1 since that's what the linux64-base-toolchains builds are, but could live with Tier 2. And maybe we should revisit whether those builds are properly tiered in a different bug :)
I meant to say that gcc on Linux is the perfect comparison here: we have builds in CI to ensure that it keeps building, but we're not running tests on those builds so it's entirely possible that Firefox could be broken in strange ways and we would not notice.
(In reply to Ryan VanderMeulen [:RyanVM] from comment #8)
> I had proposed Tier 1 since that's what the linux64-base-toolchains builds
> are, but could live with Tier 2. And maybe we should revisit whether those
> builds are properly tiered in a different bug :)

In light of that (written at the same time as my comment, humorously) Tier-1 seems reasonable.
Pushed by rvandermeulen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f464ecbaeeda
Don't use PGO for Windows opt-msvc builds now that they're not being shipped. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/1ade03649066
Turn off tests for Windows MSVC builds. r=froydnj,ahal,jmaher
https://hg.mozilla.org/integration/autoland/rev/40af17ba0d2c
Run Windows MSVC builds on all branches. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/a9cb7624c15d
Make Windows MSVC builds Tier 1. r=froydnj
You need to log in before you can comment on or make changes to this bug.