Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ivy):track changes across failed builds #33971

Closed
wants to merge 1 commit into from

Conversation

@alxhub
Copy link
Contributor

alxhub commented Nov 21, 2019

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@googlebot googlebot added the cla: yes label Nov 21, 2019
@alxhub alxhub force-pushed the alxhub:ngtsc/inc/errors branch from 28b060a to 6453281 Nov 21, 2019
@alxhub alxhub changed the title wip fix(ivy):track changes across failed builds Nov 21, 2019
@alxhub

This comment has been minimized.

Copy link
Contributor Author

alxhub commented Nov 21, 2019

Presubmit
Ivy Presubmit: not needed; incremental build doesn't affect g3

@JoostK
JoostK approved these changes Nov 22, 2019
Copy link
Member

JoostK left a comment

LGTM, thanks for writing the descriptive comments!

@alxhub alxhub force-pushed the alxhub:ngtsc/inc/errors branch from 6453281 to 6980cda Nov 22, 2019
Previously, our incremental build system kept track of the changes between
the current compilation and the previous one, and used its knowledge of
inter-file dependencies to evaluate the impact of each change and emit the
right set of output files.

However, a problem arose if the compiler was not able to extract a
dependency graph successfully. This typically happens if the input program
contains errors. In this case the Angular analysis part of compilation is
never executed.

If a file changed in one of these failed builds, in the next build it
appears unchanged. This means that the compiler "forgets" to emit it!

To fix this problem, the compiler needs to know the set of changes made
_since the last successful build_, not simply since the last invocation.

This commit changes the incremental state system to much more explicitly
pass information from the previous to the next compilation, and in the
process to keep track of changes across multiple failed builds, until the
program can be analyzed successfully and the results of those changes
incorporated into the emit plan.

Fixes #32214
@alxhub alxhub force-pushed the alxhub:ngtsc/inc/errors branch from 6980cda to 4ddb2d0 Nov 22, 2019
@alxhub alxhub marked this pull request as ready for review Nov 22, 2019
@alxhub alxhub requested a review from angular/fw-compiler as a code owner Nov 22, 2019
Copy link
Member

IgorMinar left a comment

great docs updates!

I'm adding lgtm to deal with codeowners issues until Joost is a member of angular/fw-compiler

@IgorMinar

This comment has been minimized.

Copy link
Member

IgorMinar commented Nov 22, 2019

merge-assistance: global approval

@matsko matsko closed this in 4cf1979 Nov 22, 2019
matsko added a commit that referenced this pull request Nov 22, 2019
Previously, our incremental build system kept track of the changes between
the current compilation and the previous one, and used its knowledge of
inter-file dependencies to evaluate the impact of each change and emit the
right set of output files.

However, a problem arose if the compiler was not able to extract a
dependency graph successfully. This typically happens if the input program
contains errors. In this case the Angular analysis part of compilation is
never executed.

If a file changed in one of these failed builds, in the next build it
appears unchanged. This means that the compiler "forgets" to emit it!

To fix this problem, the compiler needs to know the set of changes made
_since the last successful build_, not simply since the last invocation.

This commit changes the incremental state system to much more explicitly
pass information from the previous to the next compilation, and in the
process to keep track of changes across multiple failed builds, until the
program can be analyzed successfully and the results of those changes
incorporated into the emit plan.

Fixes #32214

PR Close #33971
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.