Migrate user-facing apis to null-safety

[simolus3]

Migrate packages typically imported by build authors to null-safety (part 1 /3). Closes #2920

In this PR, I migrate

• the build package
• the build_test package (or rather, the parts of that package that don't import build_resolvers)
• the scratch_space package

After the graphs package is migrated, I'll open another PR to migrate the build_resolvers package. When that one is done too, we can start to opt the tests of build and build_test into null-safety and publish, thus enabling build authors to write and test their builders with strong null-safety.
Migrating the rest of the build system is not a goal for the near future, so we won't run builds with strong null-safety.

[jakemac53]

Thanks for starting on this! As you have probably discovered this is going to be a large and somewhat complicated effort. My thoughts were that we should be able to migrate mostly in a bottom up fashion, one package at a time. I do believe it is feasible to do that, although I am not sure all of the packages are unblocked yet, and we will want to wait on that probably before publishing any of them (I think it just gets confusing otherwise, and it doesn't unblock anybody).

Tests would initially all be opted out (or run in --no-sound-null-safety mode). Maybe the latter is better.

The order would be roughly [build, build_config, build_daemon], [build_resolvers, scratch_space], build_test, build_runner_core, build_runner, build_modules, [build_web_compilers, build_vm_compilers].

Then we would go back through and enable sound null safety on all the tests, fixing things up as needed.

I think probably we would want to do all of this before we publish any of them, get them all passing tests and happy, etc. That means a somewhat long feature freeze most likely. Whenever one is migrated you would have to go add overrides for it in all the ones that depend on it.

Another option we may want to consider, is splitting this into two chunks for publishing. If we get build, build_config, build_resolvers, and build_test migrated, that should be pretty much everything any builders depend on, and most builders would at that point be unblocked from migrating. I think we could run most tests with sound null safety at that point also. We could then publish those.

The rest of the packages are more related to the build script itself, which can be tackled as a separate thing, and is honestly just a non-goal for the immediate future.

[jakemac53]

cc @natebosch

[simolus3]

Thanks for the quick feedback!

Another option we may want to consider, is splitting this into two chunks for publishing. If we get build, build_config, build_resolvers, and build_test migrated, that should be pretty much everything any builders depend on, and most builders would at that point be unblocked from migrating

From a builder-author perspective, this is what I was aiming for. I want to write and test my builders with sound null safety, but I don't care about whether the actual build runner is using null safety or not (similar to the story with package:test).
Maybe we can encourage users to run builds with weak null assertions if that helps.

build, build_test and build_resolvers have circular (dev-)dependencies, so we kind of have to migrate them at once. When that's done, we can run the majority of tests with sound null safety too.

[jakemac53]

build, build_test and build_resolvers have circular (dev-)dependencies, so we kind of have to migrate them at once. When that's done, we can run the majority of tests with sound null safety too.

They can be migrated separately - just the tests have to be opted out (or ran without sound null safety and add an ignore to the imports).

[jakemac53]

I am OK though with a larger PR that migrates them all at once, but its more difficult to manage and review etc. It is easier to have a few smaller PRs and then go back and enable sound null safety in a followup imo.

[simolus3]

Fair enough, I'll open another PR for build_resolvers and to migrate the tests later. I already migrated most of build, build_test and scratch_space though, if it's not too much work for you I'd prefer to not split those up.

And unfortunately I had to add lots of dependency overrides now: Since _test wants all build packages from path, they all need to support the latest analyzer and glob, and due to breaking changes I couldn't really expand the version range. So the overrides are kind of starting to cascade now...

[jakemac53]

Ya I am fine with keeping this PR as is at this point and not splitting it up.

[jakemac53]

And unfortunately I had to add lots of dependency overrides now: Since _test wants all build packages from path, they all need to support the latest analyzer and glob, and due to breaking changes I couldn't really expand the version range. So the overrides are kind of starting to cascade now...

Ya, this mono_repo is definitely more than a bit crazy like that

[simolus3]

I think we might want to put this on hold until the test packages support the latest analyzer. The dependency conflicts are getting out of hand.

[jakemac53]

I think we might want to put this on hold until the test packages support the latest analyzer. The dependency conflicts are getting out of hand.

sgtm

[simolus3]

I'll wait for #3003 to land before fixing conflicts (otherwise I'd effectively have to do it twice). Apart from that, I think this should be ready now.

[jakemac53]

I'll wait for #3003 to land before fixing conflicts (otherwise I'd effectively have to do it twice). Apart from that, I think this should be ready now.

Sounds good, we probably also want to actually do a round of releases as well before merging and releasing this.

[simolus3]

Sounds good, we probably also want to actually do a round of releases as well before merging and releasing this.

Definitely. And if we end up doing a build: 2.0.0 release for this, we might want to integrate other breaking changes too.

[jakemac53]

@simolus3 we should be ready to proceed here if you want to merge with master and I can take another pass over the pr

[jakemac53]

Note that our tests red on master right now, I am working on fixing them.

[jakemac53]

Re-running the tests now, should go green

[jakemac53]

@simolus3 I think maybe you need to merge with master again to get the test fixes?

[simolus3]

Just for completeness, I want to point out that this package is not fully migrated yet (and shouldn't be released as-is). We're still blocked by build_resolvers, which is blocked by graphs.

[jakemac53]

Understood, did you plan to look at those or should one of us (either is fine, just want to not duplicate work :) )

[simolus3]

One of you would have to take a look at dart-archive/graphs#52. As soon as that package is migrated I'm happy to work on build_resolvers and the rest of build_test.

[NarHakobyan]

are you planning to release?

[jakemac53]

Probably sometime next week, there is another small breaking change we are rolling as a part of this release (#1899) but its going to take a little bit to roll that out (and I am OOO for the rest of this week).