[rush] Incremental build should skip projects that succeed with warnings

[WanderWang]

Is this a feature or a bug?

• Feature
• Bug

What is the expected behavior?

1. create a project and add it to rush.json
2. setup the script so that it prints a warning to STDERR (e.g. console.error("Warning!");)
3. execute rush build,
4. execute rush build again

Expected behavior: The project should be skipped the second time, since nothing has changed

Actual behavior: The project gets built a second time

[octogonz]

The logic is here: ProjectTask.ts#L94

From what I understand, the package-deps.json file will not be written if either (1) the process exit code is nonzero, or (2) if there is a "warning or error" (i.e. any output written to STDERR). Because the file is not written, rush build must redo the build the next time.

Here's why I think it was designed that way:

Imagine if warnings did not invalidate the build: Suppose that you run rush build, and it succeeds with some warnings. If you run rush build again, we'd like for it to skip all the projects (because nothing has changed). But it would be misleading for Rush to report 0 warnings. Ideally, Rush should somehow remember the warnings from the previous build, so it can show them the second time (without actually invoking the build scripts). At the very least, Rush needs to remember that there /were/ warnings, to prevent a CI build from accidentally succeeding when it should have failed.

How can we implement this? A simple idea would be for the package-deps.json file to track whether the build succeeded with warnings. Perhaps we could even save the STDERR content in that file.

Thoughts?

@nickpape @iclanton

[hardfist]

But we may need some escape hatch to skip the build cache invalidate for warning build, since we may know that some tools write to stderr not for warnings(esbuild for example, use stderr to print info instead of stdout, because it already use stdout for pipe communication evanw/esbuild#1024)

[Measy]

The logic is here: ProjectTask.ts#L94

From what I understand, the package-deps.json file will not be written if either (1) the process exit code is nonzero, or (2) if there is a "warning or error" (i.e. any output written to STDERR). Because the file is not written, rush build must redo the build the next time.

Here's why I think it was designed that way:

Imagine if warnings did not invalidate the build: Suppose that you run rush build, and it succeeds with some warnings. If you run rush build again, we'd like for it to skip all the projects (because nothing has changed). But it would be misleading for Rush to report 0 warnings. Ideally, Rush should somehow remember the warnings from the previous build, so it can show them the second time (without actually invoking the build scripts). At the very least, Rush needs to remember that there /were/ warnings, to prevent a CI build from accidentally succeeding when it should have failed.

How can we implement this? A simple idea would be for the package-deps.json file to track whether the build succeeded with warnings. Perhaps we could even save the STDERR content in that file.

Thoughts?

@nickpape @iclanton

So will you eventually provide this as you mentioned? such as 'allowWarningsCache' that make package-deps.json file to track whether the build succeeded with warnings. And save the STDERR content in that file while next build to remember the warnings;

[TheBit]

@Measy this might be interesting to you

[BorysShulyak]

any news on this?

[alber70g]

Any advancement on this. It's very annoying to rebuild projects that only had warnings. The allowWarningsInSuccessfulBuild should be enough to skip rebuilding, but an extra flag like allowBuildsWithWarningsToBeCached could work as well.

We're adopting the api-extractor, and that gives a bunch of warnings, that we want to have, but not be blocking and increasing build times

[alber70g]

I just looked at the source (the path mentioned here is not up-to-date?)

https://github.com/microsoft/rushstack/blob/master/apps/rush-lib/src/logic/taskExecution/ProjectTaskRunner.ts#L356-L357

It seems there's an experimental flag? this._rushConfiguration.experimentsConfiguration.configuration .buildCacheWithAllowWarningsInSuccessfulBuild

[alber70g]

Oh well, there's a flag. Go to common/config/rush/experiments.json and enable buildCacheWithAllowWarningsInSuccessfulBuild

See also the config source

[ajmeese7]

@alber70g I enabled that flag and I'm still having the same problem. I went ahead and changed my Babel config to supress the warning, but the flag itself did not fix the issue. Is there something that I am missing? Just curious in case I encounter this problem again.