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

NG_PERSISTENT_BUILD_CACHE breaks when localizing #21275

Closed
1 of 15 tasks
terencehonles opened this issue Jul 3, 2021 · 3 comments · Fixed by #21286
Closed
1 of 15 tasks

NG_PERSISTENT_BUILD_CACHE breaks when localizing #21275

terencehonles opened this issue Jul 3, 2021 · 3 comments · Fixed by #21286
Labels
Milestone

Comments

@terencehonles
Copy link

🐞 Bug report

Command (mark with an x)

  • new
  • build
  • serve
  • test
  • e2e
  • generate
  • add
  • update
  • lint
  • extract-i18n
  • run
  • config
  • help
  • version
  • doc

Is this a regression?

No, NG_PERSISTENT_BUILD_CACHE is new

Description

When running NG_PERSISTENT_BUILD_CACHE if using --localize the cache path generates new cache keys for every run. I was trying to test out the new option as mentioned in #20792 but I wasn't seeing any difference in performance. I had noticed a few cache keys getting generated and while I didn't notice it corresponded to the number of runs I was testing (partially due to #21274 creating nested directories in the cache directory due to base64 encoding / not as the prefix). Wonder what the cache key being hashed was I edited the file and logged each of the following:

name: createHash('sha1')
.update(NG_VERSION.full)
.update(packageVersion)
.update(wco.projectRoot)
.update(JSON.stringify(wco.tsConfig))
.update(JSON.stringify(wco.buildOptions))
.update(supportedBrowsers.join(''))
.digest('base64'),

All of the variables were the same expect the wco.buildOptions. What I noticed was the outputPath was the following:

  • "outputPath":/tmp/angular-cli-i18n-9lmmzl"
  • "outputPath":"/tmp/angular-cli-i18n-LR9px9"

I should have noticed directly from the name that i18n was enabled and was likely related, but looking through code for outputPath I noticed that the following file had a match:

const tempPath = fs.mkdtempSync(path.join(fs.realpathSync(os.tmpdir()), 'angular-cli-i18n-'));
buildOptions.outputPath = tempPath;

Still not paying attention to the generated output path 😅 but realizing it might be due to the localization I removed the --localize flag from my command and noticed that it returned to the value I expected.

However I now receive errors that I was trying to write a file to the root directory. Thinking this was because NG_BUILD_CACHE was not specified I did so, but then realized there was a / in the cache name prefix and regardless of the cache directory specified (automatic or by me) the cache was trying to be created in the root directory.

I proposed #21274 to fix the issue not related to --localize, but I'm not sure if a constant output path should be swapped back in before JSON serialization or if there should be some other fix. I can include that in #21274 to fix both if desired.

🔬 Minimal Reproduction

Use any app which already has localization and try to run with and without NG_PERSISTENT_BUILD_CACHE and observe there is no difference after the initial run (the initial speedup is due to NG_BUILD_CACHE not being disabled)

🔥 Exception or Error

There is no exception, the caching just doesn't work.

🌍 Your Environment


> [email protected] ng /code
> ng "version"


     _                      _                 ____ _     ___
    / \   _ __   __ _ _   _| | __ _ _ __     / ___| |   |_ _|
   / △ \ | '_ \ / _` | | | | |/ _` | '__|   | |   | |    | |
  / ___ \| | | | (_| | |_| | | (_| | |      | |___| |___ | |
 /_/   \_\_| |_|\__, |\__,_|_|\__,_|_|       \____|_____|___|
                |___/
    

Angular CLI: 12.1.0
Node: 14.17.1
Package Manager: npm 6.14.13
OS: linux x64

Angular: 12.1.0
... animations, cdk, cli, common, compiler, compiler-cli, core
... forms, google-maps, language-service, localize, material
... platform-browser, platform-browser-dynamic, router

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1201.0
@angular-devkit/build-angular   12.1.0
@angular-devkit/core            12.1.0
@angular-devkit/schematics      12.1.0
@schematics/angular             12.1.0
rxjs                            6.6.7
typescript                      4.2.4

@alan-agius4 alan-agius4 added area: @angular-devkit/build-angular freq1: low Only reported by a handful of users who observe it rarely severity3: broken type: bug/fix labels Jul 3, 2021
@ngbot ngbot bot added this to the Backlog milestone Jul 3, 2021
@terencehonles
Copy link
Author

This is one possible fix including the change made in #21274

diff --git a/packages/angular_devkit/build_angular/src/webpack/configs/common.ts b/packages/angular_devkit/build_angular/src/webpack/configs/common.ts
index 8a0013d93..c7dd17499 100644
--- a/packages/angular_devkit/build_angular/src/webpack/configs/common.ts
+++ b/packages/angular_devkit/build_angular/src/webpack/configs/common.ts
@@ -518,21 +518,29 @@ function getCacheSettings(
   if (persistentBuildCacheEnabled) {
     const packageVersion = require('../../../package.json').version;
 
-    return {
-      type: 'filesystem',
-      cacheDirectory: findCachePath('angular-webpack'),
-      maxMemoryGenerations: 1,
-      // We use the versions and build options as the cache name. The Webpack configurations are too
-      // dynamic and shared among different build types: test, build and serve.
-      // None of which are "named".
-      name: createHash('sha1')
+    // Ignore the output path which is dynamic when localization is enabled
+    const originalOutputPath = wco.buildOptions.outputPath;
+    wco.buildOptions.outputPath = '';
+
+    // We use the versions and build options as the cache name. The Webpack configurations are too
+    // dynamic and shared among different build types: test, build and serve.
+    // None of which are "named".
+    const cacheName = createHash('sha1')
         .update(NG_VERSION.full)
         .update(packageVersion)
         .update(wco.projectRoot)
         .update(JSON.stringify(wco.tsConfig))
         .update(JSON.stringify(wco.buildOptions))
         .update(supportedBrowsers.join(''))
-        .digest('base64'),
+        .digest('hex');
+
+    wco.buildOptions.outputPath = originalOutputPath;
+
+    return {
+      type: 'filesystem',
+      cacheDirectory: findCachePath('angular-webpack'),
+      maxMemoryGenerations: 1,
+      name: cacheName,
     };
   }

I reviewed the other build options and none looked like ones that would be changed dynamically in the future, but I was wondering if the project's dependencies should also be hashed or if that would be already taken into account by the packages using the webpack filesystem cache?

@alan-agius4
Copy link
Collaborator

I think it would be better to generate a directory that doesn't change between runs, as in general it is advised not to share the cache between when an option changes.

Webpack already handles, dependencies versioning. See https://webpack.js.org/configuration/other-options/#immutablepaths

@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 Aug 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants