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

Running ngcc in postinstall. Angular CLI should not run as part of the build step #38216

Closed
Rush opened this issue Jul 24, 2020 · 12 comments
Closed

Comments

@Rush
Copy link

Rush commented Jul 24, 2020

🐞 bug report / feature request

This issue is a follow up to #35000

Affected Package

@angular/compiler-cli

Is this a regression?

Maybe - #35000

Description

The advice in #35000 (comment) from @Toxicable was to set up a postinstall hook like so:

If you run ngcc yourself beforehand then the angular cli wont run it as part of it's build step.
That should solve you issue of building multiple apps in parallel

"postinstall": "ngcc  --create-ivy-entry-points --properties es2015 browser module main",

the angular cli wont run it as part of it's build step. <- this is the key part I would expect to happen but it doesn't seem to be the case. as I see ngcc being run in my build for every webpack build

Another process, with id 44, is currently running ngcc.
--
5 | Waiting up to 25s for it to finish.
6 | Another process, with id 124, is currently running ngcc.
7 | Waiting up to 25s for it to finish.
8 | Another process, with id 44, is currently running ngcc.
9 | Waiting up to 25s for it to finish.
10 | Another process, with id 117, is currently running ngcc.
11 | Waiting up to 25s for it to finish.
12 | Another process, with id 58, is currently running ngcc.
13 | Waiting up to 25s for it to finish.
14 | Another process, with id 44, is currently running ngcc.
15 | Waiting up to 25s for it to finish.

Unfortunately when CI is busy with other work the max retries is used up and the build fails. We have about 10 parallel builds of different flavors of our Angular App so there is A LOT of parallelism going on.

Would it be possible to disable NGCC in ngtools/webpack builds when the dependencies have already been compiled before? @Toxicable @petebacondarwin @AndrewKushnir @gkalpak

🌍 Your Environment

Angular Version:
Angular: 9.1.0

@gkalpak
Copy link
Member

gkalpak commented Jul 24, 2020

It is hard to tell what might be going on. Any chance you could put together a reproduction, @Rush?

@gkalpak gkalpak added comp: ngcc needs reproduction This issue needs a reproduction in order for the team to investigate further labels Jul 24, 2020
@ngbot ngbot bot added this to the needsTriage milestone Jul 24, 2020
@Rush
Copy link
Author

Rush commented Jul 24, 2020

@gkalpak ok will do!

Rush added a commit to Rush/ngcc-issue-repro that referenced this issue Jul 24, 2020
@Rush
Copy link
Author

Rush commented Jul 24, 2020

@gkalpak Please see this repo for a reproducible test case https://github.com/Rush/ngcc-issue-repro/blob/master/README.md

> npm install
## RUNS ngcc  --create-ivy-entry-points --properties es2015 browser module main

> ./start-builds.sh 

> ngcc-issue-repro@0.0.0 build5 /home/rush/code/angular-bug/ngcc-issue-repro
> BUILD_NAME=build1 webpack


> ngcc-issue-repro@0.0.0 build2 /home/rush/code/angular-bug/ngcc-issue-repro
> BUILD_NAME=build1 webpack


> ngcc-issue-repro@0.0.0 build3 /home/rush/code/angular-bug/ngcc-issue-repro
> BUILD_NAME=build1 webpack


> ngcc-issue-repro@0.0.0 build1 /home/rush/code/angular-bug/ngcc-issue-repro
> BUILD_NAME=build1 webpack


> ngcc-issue-repro@0.0.0 build4 /home/rush/code/angular-bug/ngcc-issue-repro
> BUILD_NAME=build1 webpack

Another process, with id 31479, is currently running ngcc.
Waiting up to 250s for it to finish.
Another process, with id 31479, is currently running ngcc.
Waiting up to 250s for it to finish.
Another process, with id 31479, is currently running ngcc.
Waiting up to 250s for it to finish.
Another process, with id 31479, is currently running ngcc.
Waiting up to 250s for it to finish.
Another process, with id 31472, is currently running ngcc.
Waiting up to 250s for it to finish.
Another process, with id 31472, is currently running ngcc.
Waiting up to 250s for it to finish.
Another process, with id 31472, is currently running ngcc.
Waiting up to 250s for it to finish.
Another process, with id 31458, is currently running ngcc.
Waiting up to 250s for it to finish.
Another process, with id 31458, is currently running ngcc.
Waiting up to 250s for it to finish.
Another process, with id 31451, is currently running ngcc.
Waiting up to 250s for it to finish

Please let me know if you have any questions or need additional clarity.

@Rush
Copy link
Author

Rush commented Jul 27, 2020

With Angular 10, it seems to have worsened. We did an upgrade PR and now the CI doesn't even pass in a single run (CI worker completely free).


Error: Lock found, but no process with PID 117 seems to be running.
--
5 | (If you are sure no ngcc process is running then you should delete the lock-file at
Waiting up to 2500s for it to finish.
--
6 | (If you are sure no ngcc process is running then you should delete the lock-file at /drone/src/node_modules/@angular/compiler-cli/ngcc/__ngcc_lock_file__.)
7 | Error: Lock found, but no process with PID 44 seems to be running.
8 | (If you are sure no ngcc process is running then you should delete the lock-file at /drone/src/node_modules/@angular/compiler-cli/ngcc/__ngcc_lock_file__.)
9 | at new TimeoutError (/drone/src/node_modules/@angular/compiler-cli/ngcc/src/locking/async_locker.js:25:51)
10 | at AsyncLocker.<anonymous> (/drone/src/node_modules/@angular/compiler-cli/ngcc/src/locking/async_locker.js:118:47)
11 | at step (/drone/src/node_modules/@angular/compiler-cli/node_modules/tslib/tslib.js:140:27)
12 | at Object.next (/drone/src/node_modules/@angular/compiler-cli/node_modules/tslib/tslib.js:121:57)
13 | at /drone/src/node_modules/@angular/compiler-cli/node_modules/tslib/tslib.js:114:75
14 | at new Promise (<anonymous>)
15 | at Object.__awaiter (/drone/src/node_modules/@angular/compiler-cli/node_modules/tslib/tslib.js:110:16)
16 | at AsyncLocker.create (/drone/src/node_modules/@angular/compiler-cli/ngcc/src/locking/async_locker.js:75:28)
17 | at AsyncLocker.<anonymous> (/drone/src/node_modules/@angular/compiler-cli/ngcc/src/locking/async_locker.js:58:59)
18 | at step (/drone/src/node_modules/@angular/compiler-cli/node_modules/tslib/tslib.js:140:27)

@gkalpak
Copy link
Member

gkalpak commented Jul 29, 2020

@Rush, based on your last comment I wonder whether you have accidentally cached a stale node_modules/@angular/compiler-cli/ngcc/__ngcc_lock_file__ file, which prevents future ngcc runs.

Other than that, everything seems to work as expected. If you have run ngcc already, the ngcc processes will indeed be spawned (thus you will get the "Waiting for other process" messages), but they should exist quickly and essentially not impacting the overall build time in a noticeable way. (See also #37903 (comment) for some more details.)

Is this different from what you are seeing?

@Rush
Copy link
Author

Rush commented Jul 29, 2020

I wonder whether you have accidentally cached a stale

Good theory. Unfortunately I checked it by disabling cache, removing node_modules before, removing all __ngcc_lock_file__ before build starts and the issues are exactly the same.

Is this different from what you are seeing?

Definitely not what I'm seeing.

In Angular 9 - the builds are timing out before ngcc gets unblocked (if CI machine is busy running multiple builds)
In Angular 10 - builds are crashing due to processes not being found to be running.

We'll try to provide a better test case for Angular 10 which shows the crash.

@gkalpak my point is that if files are pre-generated shouldn't ngcc not run at all? It seems it would be nice to be able to delete it completely.

Anyway - will try harder to produce a better reproducible test case.

@gkalpak
Copy link
Member

gkalpak commented Jul 30, 2020

my point is that if files are pre-generated shouldn't ngcc not run at all?

Neither ngcc nor the CLI are in a position to know that you have already run ngcc (with the appropriate options). So, the CLI has to trigger ngcc and ngcc has to analyze your dependencies. As explained in #37903 (comment), we strive for this phase to be as fast as possible for the no-op case (i.e. when the dependencies have already been processed), but we can't eliminate it (without complicating ngcc significantly due to the various edgecases that we would need to handle).

It seems it would be nice to be able to delete it completely.

You might want to open a feature request on the CLI repo.

In the meantime, it would be interesting to try "manually" disabling the ngcc invocation by the CLI and see what happens with the build. For example, you could modify ngtools/webpack/src/ngcc_processor.ts#L46 (found in node_modules/@ngtools/webpack/src/ngcc_process.js) to make its process() method a no-op.

However, the idea with the current way things work is that you should not need to worry about ngcc (and let the CLI handle it for you) even when you have parallel builds. The behavior you are seeing is indeed quite strange. I would love to get to the bottom of it (but I'm afraid that won't be easy without getting our hands on a reproduction).

@Rush
Copy link
Author

Rush commented Aug 12, 2020

Thanks for pointing me to the "process" function. It's actually very easy to make process() a no-op. Check this out:
image

Setting BAZEL_TARGET: "1" in the CI made all the builds pass with flying colors!

It looks like some people already had a need to forcefully disable NGCC... I wonder why that env option has not been set to something generic like NGCC_FORCE_DISABLE=1?

// Under Bazel when running in sandbox mode parts of the filesystem is read-only.
--
38 | if (process.env.BAZEL_TARGET) {
39 | return;
40 | }
41 | // Skip if node_modules are read-only
42 | const corePackage = this.tryResolvePackage('@angular/core', this._nodeModulesDirectory);
43 | if (corePackage && isReadOnlyFile(corePackage)) {
44 | return;
45 | }

So .. if under Bazel parts of the filesystem are read-only shouldn't the second check be sufficient for Bazel users?

Anyway .. then I had a thought I could just try to make the read-only clause to trigger ...

chmod 400 node_modules/@angular/core/package.json

image

But that did not work!

Weirdly enough BAZEL_TARGETdoes work but read-only approach does not. I don't know what to make out of it yet. Will keep testing.

@petebacondarwin
Copy link
Member

@Rush - is it that corePackage in if (corePackage && isReadOnlyFile(corePackage)) is falsy?
Also can you check what this._nodeModulesDirectory is?

@Rush
Copy link
Author

Rush commented Aug 12, 2020

@petebacondarwin Not the cleanest way to debug but here you go .. :-)

sed -i "s/this.tryResolvePackage('@angular\/core', this._nodeModulesDirectory);/this.tryResolvePackage('@angular\/core', this._nodeModulesDirectory); console.log('Angular 10 debug', this._nodeModulesDirectory, corePackage, isReadOnlyFile(corePackage));/" node_modules/@ngtools/webpack/src/ngcc_processor.js
chmod 400 node_modules/@angular/core/package.json

and it's always printing

Angular 10 debug /drone/src/node_modules /drone/src/node_modules/@angular/core/package.json false
9

And then I checked on my machine by setting chmod 400 node_modules/@angular/core/package.json, and added the following to node_modules/@ngtools/webpack/src/ngcc_processor.js

console.log('Readonly', isReadOnlyFile('node_modules/@angular/core/package.json'))
> chmod 400 node_modules/@angular/core/package.json
> node node_modules/@ngtools/webpack/src/ngcc_processor.js
Readonly true
> sudo node node_modules/@ngtools/webpack/src/ngcc_processor.js
Readonly false

Of course ... Docker is running on root. So root will always have write access, regardless of file write flags.

To summarize:

  • setting chmod 400 on @angular/core/package.json in CI is a no-go for disabling ngcc due to docker running things as root
  • setting BAZEL_TARGET is a working solution, albeit quite hacky
  • would be nice to have NGCC_FORCE_DISABLE instead of BAZEL_TARGET

And the root cause for failures is still unknown.

@petebacondarwin
Copy link
Member

Hi @Rush - I just wanted to come back to you on this.

I spent some time today trying to get some timings from the reproduction you provided (https://github.com/Rush/ngcc-issue-repro).

What I have found is that ngcc is indeed being triggered (as expected) for each of the 5 builds that are kicked off in parallel.

  • Each of these does the minimal amount of work to ascertain that there is nothing left to process. They each take a bit under 400ms to do their work. (Actually, ngcc would normally take around 90ms to do this but because of the other webpack processes running in parallel, this is 4x slower).
  • Since we cannot run ngcc in parallel they each need to be run in series and we report a message saying so for each of the subsequent builds.

What this means in practice is that your overall build (of around 90secs in this repro) is delayed by up to 400ms x 5 = 2secs.
Interestingly if I run the builds in series, I get a total build time of still around 90secs. So in this case, on my machine, there is no benefit to parallelising the builds.

I am loathe to add a NGCC_FORCE_DISABLE since it could lead to builds failing for subtle difficult to debug reasons; and since you already have a workaround (even if a bit hackish) of setting BAZEL_TARGET, I would rather leave things as they stand.

@petebacondarwin petebacondarwin added state: works-as-expected type: use-case and removed needs reproduction This issue needs a reproduction in order for the team to investigate further labels Aug 17, 2020
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Aug 17, 2020
@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 Oct 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants