New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 731881 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 738611



Sign in to add a comment

Migrate from Vulcanize to polymer-bundler

Project Member Reported by dpa...@chromium.org, Jun 9 2017

Issue description

Vulcanize [1] has been superseeded by polymer-bundler [2]. We should try to migrate our https://cs.chromium.org/chromium/src/chrome/browser/resources/vulcanize_gn.py script to use polymer-bundler.

One of the blockers of such migration was https://github.com/Polymer/polymer-bundler/issues/479, which has been addressed.

Other complications I am expecting are differences in path processing between Windows and Linux (for which we had to add workarounds in existing script).  

[1] https://github.com/Polymer/polymer-bundler/tree/1.x
[2] https://github.com/Polymer/polymer-bundler/tree/master
 

Comment 1 by dpa...@chromium.org, Jun 13 2017

Status update on known issues that are blocking polymer-bundler usage in Chromium,

 - https://github.com/Polymer/polymer-bundler/issues/479, fixed.
 - https://github.com/Polymer/polymer-bundler/issues/541, fixed.
 - https://github.com/Polymer/polymer-bundler/issues/548, in progress.

Comment 2 by dpa...@chromium.org, Jun 13 2017

Forgot one more:
 - out-request-list flag equivalent missing from polymer-bundler (no Github issue yet).

Comment 3 by dpa...@chromium.org, Jun 13 2017

Additional blocking Github issues, related to out-request-list flag:
- https://github.com/Polymer/polymer-bundler/issues/549
- https://github.com/Polymer/polymer-bundler/issues/550

Comment 4 by dpa...@chromium.org, Jun 15 2017

Issues  #549  and  #550  above have been addressed. Newly discovered blocking issue https://github.com/Polymer/polymer-bundler/issues/555.

Comment 5 by dpa...@chromium.org, Jun 16 2017

FYI one remaining blocker is https://github.com/Polymer/polymer-bundler/issues/556. That bug report is not actually a bug in polymer-bundler, instead it is just a by-design difference in behavior compared to Vulcanize.

Per bendanb's suggestion, I am going to try generating the main and lazy modules at the same time (single invocation of polymer-bundler), as opposed to two separate invocations of vulcanize. Will update this thread with findings.

Comment 6 by dpa...@chromium.org, Jun 17 2017

Current progress is at https://codereview.chromium.org/2936333002, where I managed to replace multiple calls to vulcanize with a single call to polymer-bundler. There is still more work to be done (updating depfile correctly), but overall, if this works, I think the end result is simpler than the previous code.

Also, discovered https://github.com/Polymer/polymer-bundler/issues/560, which is causing the md_bookmarks page to not build correctly.
Status update: I was able to get all green tests (on Windows too!) while using polymer-bundler (at https://codereview.chromium.org/2936333002). There are still two remaining issues being addressed:

1) https://github.com/Polymer/polymer-analyzer/issues/697 (minimize polymer-analyzer deps).
2) https://github.com/Polymer/polymer-bundler/issues/574 (don't silently succeed when references to missing files are encountered).

Comment 8 by dpa...@chromium.org, Jul 18 2017

Blocking: 738611
Project Member

Comment 9 by bugdroid1@chromium.org, Jul 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1e6a87f5d3ae459cb14031af36c340349eaff40b

commit 1e6a87f5d3ae459cb14031af36c340349eaff40b
Author: dpapad <dpapad@chromium.org>
Date: Sat Jul 22 00:22:25 2017

WebUI: Replace vulcanize with polymer-bundler.

Summary of changes needed for the migration:
 - Excluded URLs behavior is different than Vulcanize, need to explicitly list
   all URLs to be excluded (unlike Vulcanize, transitive deps of an excluded
   URL are not implicitly excluded).
 - Excluded URLs need to match exactly, unlike Vulcanize where foo.html would
   match chrome://some-path/foo.html.
 - Input HTML files need to have
   <html><head>...</head><body>...</body></html> tags.
 - A single invocation of poylmer-bundler produces both "basic" and
   "lazy" modules (achieved by using the --shell flag), unlike Vulcanize
   which required two separate invocations.
 - Using new --manifest-out instead of previous --out-request-list flag to
   generate depfiles for Ninja.
 - Using --rewrite-urls-in-templates to preserve previous rewriting behavior.
 - polymer-bundler does not blow up when it can't find a file (unlike Vulcanize,
   so check output manifest for any reported "missing" dependencies).
 - Removed some Windows specific hacks that don't seem necessary anymore.

BUG= 731881 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2936333002
Cr-Commit-Position: refs/heads/master@{#488821}

[modify] https://crrev.com/1e6a87f5d3ae459cb14031af36c340349eaff40b/chrome/browser/resources/md_bookmarks/BUILD.gn
[modify] https://crrev.com/1e6a87f5d3ae459cb14031af36c340349eaff40b/chrome/browser/resources/md_bookmarks/bookmarks.html
[modify] https://crrev.com/1e6a87f5d3ae459cb14031af36c340349eaff40b/chrome/browser/resources/md_downloads/BUILD.gn
[modify] https://crrev.com/1e6a87f5d3ae459cb14031af36c340349eaff40b/chrome/browser/resources/md_history/BUILD.gn
[modify] https://crrev.com/1e6a87f5d3ae459cb14031af36c340349eaff40b/chrome/browser/resources/md_history/lazy_load.html
[modify] https://crrev.com/1e6a87f5d3ae459cb14031af36c340349eaff40b/chrome/browser/resources/settings/BUILD.gn
[modify] https://crrev.com/1e6a87f5d3ae459cb14031af36c340349eaff40b/chrome/browser/resources/settings/i18n_setup.html
[modify] https://crrev.com/1e6a87f5d3ae459cb14031af36c340349eaff40b/chrome/browser/resources/settings/lazy_load.html
[modify] https://crrev.com/1e6a87f5d3ae459cb14031af36c340349eaff40b/chrome/browser/resources/vulcanize.gni
[modify] https://crrev.com/1e6a87f5d3ae459cb14031af36c340349eaff40b/chrome/browser/resources/vulcanize_gn.py
[modify] https://crrev.com/1e6a87f5d3ae459cb14031af36c340349eaff40b/chrome/test/data/webui/polymer_browser_test_base.js
[modify] https://crrev.com/1e6a87f5d3ae459cb14031af36c340349eaff40b/docs/optimizing_web_uis.md
[modify] https://crrev.com/1e6a87f5d3ae459cb14031af36c340349eaff40b/third_party/node/README.chromium
[add] https://crrev.com/1e6a87f5d3ae459cb14031af36c340349eaff40b/third_party/node/chromium_polymer_bundler.patch
[delete] https://crrev.com/af3918157d6cc35918a3ae3fc9cefc722a6f31ad/third_party/node/chromium_vulcanize.patch
[modify] https://crrev.com/1e6a87f5d3ae459cb14031af36c340349eaff40b/third_party/node/node_modules.py
[modify] https://crrev.com/1e6a87f5d3ae459cb14031af36c340349eaff40b/third_party/node/node_modules.tar.gz.sha1
[modify] https://crrev.com/1e6a87f5d3ae459cb14031af36c340349eaff40b/third_party/node/package.json
[modify] https://crrev.com/1e6a87f5d3ae459cb14031af36c340349eaff40b/third_party/node/update_npm_deps

The migration from Vulcanize to polymer-bundler has landed. Hoping that everything works as expected and nothing gets reverted.

There are still some follow up cleanups, so I'll keep this bug open until those land too.

In total, 10 bugs/issues were discovered and fixed (9 in polymer-bundler, 1 in polymer-analyzer). Many thanks to https://github.com/usergenic for addressing all of these!

Summarizing the fixes involved to get to this point below:

--redirect not working
https://github.com/Polymer/polymer-bundler/issues/479

--strip-comments misses comments
https://github.com/Polymer/polymer-bundler/issues/541

--exlude should honor JS and CSS, not just HTML
https://github.com/Polymer/polymer-bundler/issues/548

--manifest-out should report all inlined files not just HTML
https://github.com/Polymer/polymer-bundler/issues/549

--manifest-out should work when only one root HTML file is provided.
https://github.com/Polymer/polymer-bundler/issues/550

--out-dir flag does not handle absolute paths
https://github.com/Polymer/polymer-bundler/issues/559

--out-dir is ignored when only one HTML input is provided.
https://github.com/Polymer/polymer-bundler/issues/560

Missing files are silently ignored
https://github.com/Polymer/polymer-bundler/issues/574

--shell: HTML bundles should not link back to the Shell
https://github.com/Polymer/polymer-bundler/issues/471

Remove typescript from 'dependencies'
https://github.com/Polymer/polymer-analyzer/issues/697
Remaining cleanup is at https://chromium-review.googlesource.com/c/596525. Will close this bug once it lands.
Project Member

Comment 12 by bugdroid1@chromium.org, Aug 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/48736c20653020a5715de2ce37309648e655ac3b

commit 48736c20653020a5715de2ce37309648e655ac3b
Author: dpapad <dpapad@chromium.org>
Date: Fri Aug 11 18:00:06 2017

WebUI: Move polymer-css-build invocation within vulcanize_gn.py.

After the migration from vulcanize to polymer-bundler, there is no longer a need
to have polymer-css-build invocation as a separate GN action.

Bug:  731881 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ib73ef9cb85e8ad33f84cd934137e4c9682ab5ef2
Reviewed-on: https://chromium-review.googlesource.com/596525
Commit-Queue: Demetrios Papadopoulos (OOO till Sept 11th) <dpapad@chromium.org>
Reviewed-by: Dan Beam (no longer on Chrome) <dbeam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493804}
[modify] https://crrev.com/48736c20653020a5715de2ce37309648e655ac3b/chrome/browser/resources/md_bookmarks/BUILD.gn
[modify] https://crrev.com/48736c20653020a5715de2ce37309648e655ac3b/chrome/browser/resources/md_downloads/BUILD.gn
[modify] https://crrev.com/48736c20653020a5715de2ce37309648e655ac3b/chrome/browser/resources/md_history/BUILD.gn
[delete] https://crrev.com/af1d285f81dff2617a49ffa85139069e5fa31e33/chrome/browser/resources/polymer_css_build_gn.py
[modify] https://crrev.com/48736c20653020a5715de2ce37309648e655ac3b/chrome/browser/resources/settings/BUILD.gn
[modify] https://crrev.com/48736c20653020a5715de2ce37309648e655ac3b/chrome/browser/resources/vulcanize_gn.py

Project Member

Comment 13 by bugdroid1@chromium.org, Sep 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/96def51ced68727b0966af5c86fc8e5f4fe37e25

commit 96def51ced68727b0966af5c86fc8e5f4fe37e25
Author: dpapad <dpapad@chromium.org>
Date: Wed Sep 13 18:49:37 2017

WebUI cleanup: Rename "vulcanize" to "optimize_webui" in various places.

Vulcanize was a tool that was used to optimize WebUI pages, and that was
reflected in the naming in multiple places (file names, runtime flags,
function names, comments, GN actions).

The tool has since been replaced with another tool called polymer-bundler.
On top of that, other tools are used in addition to polymer-bundler (for
example crisper, polymer-css-build and uglify).

Using "vulcanize" in terminology is confusing as it is just a remnant of a
previous state of the code, and does not make sense anymore.

Bug:  731881 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I402faacf96213b66a8045a852fe4daddb0c83cc0
Reviewed-on: https://chromium-review.googlesource.com/662360
Reviewed-by: calamity <calamity@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501704}
[modify] https://crrev.com/96def51ced68727b0966af5c86fc8e5f4fe37e25/chrome/browser/resources/PRESUBMIT.py
[modify] https://crrev.com/96def51ced68727b0966af5c86fc8e5f4fe37e25/chrome/browser/resources/md_bookmarks/BUILD.gn
[modify] https://crrev.com/96def51ced68727b0966af5c86fc8e5f4fe37e25/chrome/browser/resources/md_downloads/BUILD.gn
[modify] https://crrev.com/96def51ced68727b0966af5c86fc8e5f4fe37e25/chrome/browser/resources/md_history/BUILD.gn
[rename] https://crrev.com/96def51ced68727b0966af5c86fc8e5f4fe37e25/chrome/browser/resources/optimize_webui.gni
[rename] https://crrev.com/96def51ced68727b0966af5c86fc8e5f4fe37e25/chrome/browser/resources/optimize_webui.py
[rename] https://crrev.com/96def51ced68727b0966af5c86fc8e5f4fe37e25/chrome/browser/resources/optimize_webui_test.py
[modify] https://crrev.com/96def51ced68727b0966af5c86fc8e5f4fe37e25/chrome/browser/resources/settings/BUILD.gn

Project Member

Comment 14 by bugdroid1@chromium.org, Sep 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d0ef1a9772ee27480412f33148f6a94572f20b05

commit d0ef1a9772ee27480412f33148f6a94572f20b05
Author: dpapad <dpapad@chromium.org>
Date: Mon Sep 18 19:32:12 2017

WebUI cleanup: Rename use_vulcanize GN flag to optimize_webui

Vulcanize tool has been replaced with polymer-bundler. But also several
other optimization tools are invoked when this flag is used
(polymer-css-build, crisper, uglify), therefore the more generic
"optimize_webui" name seems more appropriate. This is addressing an
already existing TODO in the code.

Bug:  731881 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I0d25b1306141ef01f4d40a0cb9c0b79cccb6837f
Reviewed-on: https://chromium-review.googlesource.com/663215
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Dan Beam (no longer on Chrome) <dbeam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502639}
[modify] https://crrev.com/d0ef1a9772ee27480412f33148f6a94572f20b05/chrome/browser/BUILD.gn
[modify] https://crrev.com/d0ef1a9772ee27480412f33148f6a94572f20b05/chrome/browser/browser_resources.grd
[modify] https://crrev.com/d0ef1a9772ee27480412f33148f6a94572f20b05/chrome/browser/resources/BUILD.gn
[modify] https://crrev.com/d0ef1a9772ee27480412f33148f6a94572f20b05/chrome/browser/resources/settings/settings.html
[modify] https://crrev.com/d0ef1a9772ee27480412f33148f6a94572f20b05/chrome/browser/ui/webui/md_bookmarks/md_bookmarks_ui.cc
[modify] https://crrev.com/d0ef1a9772ee27480412f33148f6a94572f20b05/chrome/browser/ui/webui/md_downloads/md_downloads_ui.cc
[modify] https://crrev.com/d0ef1a9772ee27480412f33148f6a94572f20b05/chrome/browser/ui/webui/md_history_ui.cc
[modify] https://crrev.com/d0ef1a9772ee27480412f33148f6a94572f20b05/chrome/browser/ui/webui/settings/md_settings_ui.cc
[modify] https://crrev.com/d0ef1a9772ee27480412f33148f6a94572f20b05/chrome/common/BUILD.gn
[modify] https://crrev.com/d0ef1a9772ee27480412f33148f6a94572f20b05/chrome/common/features.gni
[modify] https://crrev.com/d0ef1a9772ee27480412f33148f6a94572f20b05/docs/optimizing_web_uis.md

Status: Fixed (was: Started)

Sign in to add a comment