optimize_webui: Bundled HTML/JS files are not always updated when source files are changed. |
||||||||||||
Issue descriptionRepro steps: 1) Ensure you have optimized_webui=true in GN (this is also the default setting). 2) Delete any previously generated print preview files (if they exist) rm out/<out_folder>/gen/chrome/browser/resources/print_preview/print_preview_resources.unpak 3) Execute the following ninja -C out/<out_folder> chrome/browser/resources/print_preview:unpak -j1 -v -d explain You should see a log like the following indicating that unpack_pak.py is executing. [5/6] python ../../chrome/browser/resources/unpack_pak.py Also the folder deleted at step 2 should now exist again. 4) Open chrome/browser/resources/print_preview/new/destination_dialog.html and add the following string before line 108 (see [1]), and save <div>foobar</div> 5) Execute step 3 again. Notice that unpack_pak.py is not triggered, even though it should, since it depends "flattened_resources", which in turn should regenerate every time a source file is affected, see [2]. This is currently affecting the new Print preview, but also potentially Settings. [1] https://cs.chromium.org/chromium/src/chrome/browser/resources/print_preview/new/destination_dialog.html?l=108 [2] https://cs.chromium.org/chromium/src/chrome/browser/resources/print_preview/BUILD.gn?l=34
,
May 16 2018
FWIW, thestig@ has not been able to repro, and I am only able to repro on my Linux machine, but not on my Mac. Either there is something wrong with my setup, or there is some important info in the repro steps that is currently missing (will keep looking).
,
May 17 2018
+aee as FYI. This is the issue I was facing when manually testing a CL of yours latel.
,
Jun 6 2018
@scottchen: I am suspecting this issue, is the reason [1] passed tryjobs, and later started failing on dbg bots after landing. [1] https://chromium-review.googlesource.com/c/chromium/src/+/1081243
,
Jun 6 2018
this is definitely also affecting settings (hit me yesterday while working on https://chromium-review.googlesource.com/c/chromium/src/+/1088161)
,
Jun 6 2018
I think I just ran into a case of this, after patching a CL and ran the build, the bundled HTML file did not update.
,
Jun 6 2018
Since this seems to not be just my machine's issue, I am raising the priority, since the consequences of this bug are pretty significant.
,
Jun 6 2018
I am wondering if this is a GN regression, but don't know how to bisect GN issues. Simplified repro steps: 1) use optimized_webui=true in GN flags. 2) Build chrome ninja -C out/gchrome/ chrome -j700 -l50 3) touch chrome/browser/resources/settings/settings_shared_css.html 4) Build chrome again (step 2) Expected: It should re-trigger the following GN targets https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/BUILD.gn?l=15,37,46 Actual: It does not (at least not all of them).
,
Jun 7 2018
,
Jun 7 2018
,
Jun 7 2018
Small correction. The correct name of the GN flag is optimize_webui (no 'd').
,
Jun 7 2018
,
Jun 7 2018
Performed a local bisection and it seems that the issue is caused by https://chromium-review.googlesource.com/c/chromium/src/+/1085699 (r564311). Modified repro steps from comment#8: At step 4 run the following command instead (it's sufficient to demonstrate the issue). ninja -C out/gchrome chrome/browser/resources/settings:build -j1 -v -d explain Pasting output before r564311 below: ninja -C out/gchrome chrome/browser/resources/settings:build -j1 -v -d explain ninja: Entering directory `out/gchrome' ninja explain: restat of output gen/chrome/browser/resources/settings/flattened_resources_stamp.d.stamp older than most recent input ../../chrome/browser/resources/settings/settings_shared_css.html (1528333540 vs 1528333553) ninja explain: gen/chrome/browser/resources/settings/flattened_resources_stamp.d.stamp is dirty ninja explain: gen/chrome/browser/resources/settings/grit/settings_resources.h is dirty ninja explain: gen/chrome/browser/resources/settings/grit/settings_resources_map.cc is dirty ninja explain: gen/chrome/browser/resources/settings/grit/settings_resources_map.h is dirty ninja explain: gen/chrome/browser/resources/settings/settings_resources.pak is dirty ninja explain: gen/chrome/browser/resources/settings/settings_resources.pak.info is dirty ninja explain: obj/chrome/browser/resources/settings/flattened_resources_grit.stamp is dirty ninja explain: gen/chrome/browser/resources/settings/grit/settings_resources_map.cc is dirty ninja explain: gen/chrome/browser/resources/settings/grit/settings_resources_map.cc is dirty ninja explain: gen/chrome/browser/resources/settings/grit/settings_resources_map.h is dirty ninja explain: gen/chrome/browser/resources/settings/grit/settings_resources.h is dirty ninja explain: obj/chrome/browser/resources/settings/flattened_resources_grit.stamp is dirty ninja explain: obj/chrome/browser/resources/settings/flattened_resources/settings_resources_map.o is dirty ninja explain: obj/chrome/browser/resources/settings/flattened_resources.stamp is dirty ninja explain: obj/chrome/browser/resources/settings/flattened_resources_grit.stamp is dirty ninja explain: gen/chrome/browser/resources/settings/settings_resources.unpak/unpack.stamp is dirty ninja explain: obj/chrome/browser/resources/settings/unpak.stamp is dirty ninja explain: gen/chrome/browser/resources/settings/vulcanized.html is dirty ninja explain: gen/chrome/browser/resources/settings/lazy_load.vulcanized.html is dirty ninja explain: gen/chrome/browser/resources/settings/crisper.js is dirty ninja explain: gen/chrome/browser/resources/settings/lazy_load.crisper.js is dirty ninja explain: obj/chrome/browser/resources/settings/build.stamp is dirty [1/9] python ../../tools/grit/grit.py -i ../../chrome/browser/resources/settings/settings_resources.grd build -o gen/chrome/browser/resources/settings --depdir . --depfile gen/chrome/browser/resources/settings/flattened_resources_stamp.d --write-only-new=1 --depend-on-stamp -D _google_chrome -E CHROMIUM_BUILD=google_chrome -D desktop_linux -D toolkit_views -D use_aura -D use_nss_certs -D enable_app_list=false -D enable_background_mode=true -D enable_background_contents=true -D enable_extensions=true -D enable_google_now=false -D enable_hangout_services_extension=true -D enable_plugins=true -D enable_print_preview=true -D enable_printing=true -D enable_service_discovery=true -D enable_vr=true -D mac_views_browser=false -D safe_browsing_mode=1 -D optimize_webui=true -f ../../tools/gritsettings/resource_ids --assert-file-list=obj/chrome/browser/resources/settings/flattened_resources_expected_outputs.txt [2/7] touch obj/chrome/browser/resources/settings/flattened_resources_grit.stamp [3/7] touch obj/chrome/browser/resources/settings/flattened_resources.inputdeps.stamp [4/7] python ../../chrome/browser/resources/unpack_pak.py --out_folder gen/chrome/browser/resources/settings/settings_resources.unpak --pak_file gen/chrome/browser/resources/settings/settings_resources.pak [5/7] touch obj/chrome/browser/resources/settings/unpak.stamp [6/7] python ../../chrome/browser/resources/optimize_webui.py --host settings --input gen/chrome/browser/resources/settings/settings_resources.unpak --out_folder gen/chrome/browser/resources/settings --depfile gen/chrome/browser/resources/settings/build.d --html_in_files settings.html lazy_load.html --html_out_files vulcanized.html lazy_load.vulcanized.html --js_out_files crisper.js lazy_load.crisper.js --insert_in_head \<base\ href=\"chrome://settings\"\> [7/7] touch obj/chrome/browser/resources/settings/build.stamp Pasting output after r564311 ninja -C out/gchrome chrome/browser/resources/settings:build -j1 -v -d explain ninja: Entering directory `out/gchrome' ninja explain: restat of output gen/chrome/browser/resources/settings/flattened_resources_stamp.d.stamp older than most recent input ../../chrome/browser/resources/settings/settings_shared_css.html (1528333375 vs 1528333394) ninja explain: gen/chrome/browser/resources/settings/grit/settings_resources_map.cc is dirty ninja explain: gen/chrome/browser/resources/settings/grit/settings_resources_map.cc is dirty ninja explain: gen/chrome/browser/resources/settings/grit/settings_resources_map.h is dirty ninja explain: gen/chrome/browser/resources/settings/grit/settings_resources.h is dirty ninja explain: gen/chrome/browser/resources/settings/flattened_resources_stamp.d.stamp is dirty ninja explain: gen/chrome/browser/resources/settings/grit/settings_resources.h is dirty ninja explain: gen/chrome/browser/resources/settings/grit/settings_resources_map.cc is dirty ninja explain: gen/chrome/browser/resources/settings/grit/settings_resources_map.h is dirty ninja explain: gen/chrome/browser/resources/settings/settings_resources.pak is dirty ninja explain: gen/chrome/browser/resources/settings/settings_resources.pak.info is dirty ninja explain: obj/chrome/browser/resources/settings/flattened_resources/settings_resources_map.o is dirty ninja explain: obj/chrome/browser/resources/settings/flattened_resources.stamp is dirty ninja explain: gen/chrome/browser/resources/settings/settings_resources.unpak/unpack.stamp is dirty ninja explain: obj/chrome/browser/resources/settings/unpak.stamp is dirty ninja explain: gen/chrome/browser/resources/settings/vulcanized.html is dirty ninja explain: gen/chrome/browser/resources/settings/lazy_load.vulcanized.html is dirty ninja explain: gen/chrome/browser/resources/settings/crisper.js is dirty ninja explain: gen/chrome/browser/resources/settings/lazy_load.crisper.js is dirty ninja explain: obj/chrome/browser/resources/settings/build.stamp is dirty [1/8] python ../../tools/grit/grit.py -i ../../chrome/browser/resources/settings/settings_resources.grd build -o gen/chrome/browser/resources/settings --depdir . --depfile gen/chrome/browser/resources/settings/flattened_resources_stamp.d --write-only-new=1 --depend-on-stamp -D _google_chrome -E CHROMIUM_BUILD=google_chrome -D desktop_linux -D toolkit_views -D use_aura -D use_nss_certs -D enable_app_list=false -D enable_background_mode=true -D enable_background_contents=true -D enable_extensions=true -D enable_google_now=false -D enable_hangout_services_extension=true -D enable_plugins=true -D enable_print_preview=true -D enable_printing=true -D enable_service_discovery=true -D enable_vr=true -D mac_views_browser=false -D safe_browsing_mode=1 -D optimize_webui=true -f ../../tools/gritsettings/resource_ids --assert-file-list=obj/chrome/browser/resources/settings/flattened_resources_expected_outputs.txt [2/2] touch obj/chrome/browser/resources/settings/flattened_resources_grit.stamp
,
Jun 7 2018
This was a reland of the GN change made when this bug was first filed (which is why it wasn't reproducible for a while). GN is now stricter about targets depending on their inputs. Having a look now to see what's going on in this case.
,
Jun 7 2018
This should fix it: https://chromium-review.googlesource.com/c/chromium/src/+/1090231 Thanks for the clear repro steps!
,
Jun 7 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cc27bfe031641c2fb273cbc136750c38b5ba8361 commit cc27bfe031641c2fb273cbc136750c38b5ba8361 Author: Andrew Grieve <agrieve@chromium.org> Date: Thu Jun 07 03:15:03 2018 Fix optimize_webui not being re-run when source files change The "unpak" rule was forgetting to list the .pak as an input. TBR=dpapad@chromium.org # Fix build Bug: 843789 Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation Change-Id: I373aef5f1245d1be07640c35b2c5ee3595f04efe Reviewed-on: https://chromium-review.googlesource.com/1090231 Reviewed-by: agrieve <agrieve@chromium.org> Commit-Queue: agrieve <agrieve@chromium.org> Cr-Commit-Position: refs/heads/master@{#565162} [modify] https://crrev.com/cc27bfe031641c2fb273cbc136750c38b5ba8361/chrome/browser/resources/optimize_webui.gni
,
Jun 7 2018
,
Jun 7 2018
I synced passed r565162 and I am still able to reproduce the problem (same repro steps as comment #13). Will re-open this bug until I can verify that the problem is fixed.
,
Jun 7 2018
Ok, so I can actually verify the fix. The reason I was not able to verify originally was that using "touch" is not sufficient, but actually modifying the file contents is necessary. Thanks agrieve!
,
Jun 7 2018
|
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by dpa...@chromium.org
, May 16 2018