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.

Sign up for GitHub

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

Jump to bottom

fix(ngcc): do not lock if the target is not compiled by Angular #35057

Closed
wants to merge 3 commits into from
Closed

fix(ngcc): do not lock if the target is not compiled by Angular #35057

wants to merge 3 commits into from

Conversation

petebacondarwin
Copy link
Member

To support parallel CLI builds we instruct developers to pre-process
their node_modules via ngcc at the command line.

Despite doing this ngcc was still trying to set a lock when it was being
triggered by the CLI for packages that are not going to be processed,
since they are not compiled by Angular for instance.

This commit checks whether a target package needs to be compiled
at all before attempting to set the lock.

Fixes #35000

@petebacondarwin petebacondarwin added type: bug/fix action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release comp: ngcc labels Jan 30, 2020
@ngbot ngbot bot modified the milestone: needsTriage Jan 30, 2020
@pullapprove pullapprove bot requested a review from alxhub January 30, 2020 14:42
@petebacondarwin
Copy link
Member Author

This PR looks a lot more complicated than it is. Mostly it is just a refactoring - moving the hasProcessedTargetEntryPoint() check into the EntryPointFinder.

@petebacondarwin petebacondarwin requested review from gkalpak and removed request for alxhub January 30, 2020 14:46
@petebacondarwin petebacondarwin force-pushed the ngcc-lockfile-fix branch 2 times, most recently from b2d8894 to ed60dbe Compare January 31, 2020 13:46
@petebacondarwin
fix(ngcc): do not lock if the target is not compiled by Angular
8faa7aa 
To support parallel CLI builds we instruct developers to pre-process
their node_modules via ngcc at the command line.

Despite doing this ngcc was still trying to set a lock when it was being
triggered by the CLI for packages that are not going to be processed,
since they are not compiled by Angular for instance.

This commit checks whether a target package needs to be compiled
at all before attempting to set the lock.

Fixes angular#35000
@IgorMinar IgorMinar modified the milestones: needsTriage, v9-candidates Feb 2, 2020
gkalpak
gkalpak approved these changes Feb 3, 2020
View reviewed changes
Copy link
Member

@gkalpak gkalpak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of minor (and few super-minor) comments. Otherwise lgtm 👌

packages/compiler-cli/ngcc/src/entry_point_finder/targeted_entry_point_finder.ts Outdated Show resolved Hide resolved
packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts Outdated Show resolved Hide resolved
packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts Outdated Show resolved Hide resolved
packages/compiler-cli/ngcc/src/main.ts Outdated Show resolved Hide resolved
packages/compiler-cli/ngcc/test/entry_point_finder/targeted_entry_point_finder_spec.ts Outdated Show resolved Hide resolved
packages/compiler-cli/ngcc/test/entry_point_finder/targeted_entry_point_finder_spec.ts Show resolved Hide resolved
packages/compiler-cli/ngcc/test/entry_point_finder/targeted_entry_point_finder_spec.ts Outdated Show resolved Hide resolved
packages/compiler-cli/ngcc/test/entry_point_finder/targeted_entry_point_finder_spec.ts Outdated Show resolved Hide resolved
packages/compiler-cli/ngcc/test/entry_point_finder/targeted_entry_point_finder_spec.ts Outdated Show resolved Hide resolved
packages/compiler-cli/ngcc/test/entry_point_finder/targeted_entry_point_finder_spec.ts Outdated Show resolved Hide resolved
@petebacondarwin petebacondarwin added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Feb 3, 2020
@ngbot
Copy link

ngbot bot commented Feb 3, 2020

I see that you just added the PR action: merge label, but the following checks are still failing:
    failure status "ci/circleci: test_docs_examples_ivy" is failing
    pending missing required status "ci/circleci: publish_snapshot"

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

mhevery pushed a commit that referenced this pull request Feb 3, 2020
@petebacondarwin @mhevery
fix(ngcc): do not lock if the target is not compiled by Angular (#35057)
c30c518 
To support parallel CLI builds we instruct developers to pre-process
their node_modules via ngcc at the command line.

Despite doing this ngcc was still trying to set a lock when it was being
triggered by the CLI for packages that are not going to be processed,
since they are not compiled by Angular for instance.

This commit checks whether a target package needs to be compiled
at all before attempting to set the lock.

Fixes #35000

PR Close #35057
@mhevery mhevery closed this in 3d4067a Feb 3, 2020
@petebacondarwin petebacondarwin deleted the ngcc-lockfile-fix branch February 3, 2020 16:51
gkalpak added a commit to gkalpak/angular that referenced this pull request Feb 3, 2020
@gkalpak
refactor(ngcc): remove unused function
ba4cae4 
Since angular#35057, the `markNonAngularPackageAsProcessed()` function is no
longer used and can be removed.
@gkalpak gkalpak mentioned this pull request Feb 3, 2020
Closed
mhevery pushed a commit that referenced this pull request Feb 3, 2020
@gkalpak @mhevery
refactor(ngcc): remove unused function (#35122)
c2ef5ac 
Since #35057, the `markNonAngularPackageAsProcessed()` function is no
longer used and can be removed.

PR Close #35122
mhevery pushed a commit that referenced this pull request Feb 3, 2020
@gkalpak @mhevery
refactor(ngcc): remove unused function (#35122)
9601b5d 
Since #35057, the `markNonAngularPackageAsProcessed()` function is no
longer used and can be removed.

PR Close #35122
sonukapoor pushed a commit to sonukapoor/angular that referenced this pull request Feb 13, 2020
@petebacondarwin @sonukapoor
fix(ngcc): do not lock if the target is not compiled by Angular (angu…
2064d94 
…lar#35057)

To support parallel CLI builds we instruct developers to pre-process
their node_modules via ngcc at the command line.

Despite doing this ngcc was still trying to set a lock when it was being
triggered by the CLI for packages that are not going to be processed,
since they are not compiled by Angular for instance.

This commit checks whether a target package needs to be compiled
at all before attempting to set the lock.

Fixes angular#35000

PR Close angular#35057
sonukapoor pushed a commit to sonukapoor/angular that referenced this pull request Feb 13, 2020
@gkalpak @sonukapoor
refactor(ngcc): remove unused function (angular#35122)
8f1f108 
Since angular#35057, the `markNonAngularPackageAsProcessed()` function is no
longer used and can be removed.

PR Close angular#35122
sonukapoor pushed a commit to sonukapoor/angular that referenced this pull request Feb 17, 2020
@petebacondarwin @sonukapoor
fix(ngcc): do not lock if the target is not compiled by Angular (angu…
feb93fd 
…lar#35057)

To support parallel CLI builds we instruct developers to pre-process
their node_modules via ngcc at the command line.

Despite doing this ngcc was still trying to set a lock when it was being
triggered by the CLI for packages that are not going to be processed,
since they are not compiled by Angular for instance.

This commit checks whether a target package needs to be compiled
at all before attempting to set the lock.

Fixes angular#35000

PR Close angular#35057
sonukapoor pushed a commit to sonukapoor/angular that referenced this pull request Feb 17, 2020
@gkalpak @sonukapoor
refactor(ngcc): remove unused function (angular#35122)
7c1e8cd 
Since angular#35057, the `markNonAngularPackageAsProcessed()` function is no
longer used and can be removed.

PR Close angular#35122
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Mar 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@gkalpak gkalpak gkalpak approved these changes

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker cla: yes target: patch This PR is targeted for the next patch release type: bug/fix
Projects
None yet
Milestone
v9.x-candidates
Development

Successfully merging this pull request may close these issues.

Allow specifying custom NGCC lockfile location (cannot build in parallel with Ivy)
4 participants
@petebacondarwin @gkalpak @IgorMinar @googlebot