inspector_protocol/templates/TypeBuilder_h.template breaks with Jinja 2.9
Reported by
w...@extrahop.com,
Jul 15 2017
|
||||
Issue descriptionUserAgent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:54.0) Gecko/20100101 Firefox/54.0 Steps to reproduce the problem: 1. Attempt to build V8 on a system with Jinja 2.9 (it appears Jinja is not bundled with V8 even if it is bundled with Chromium?) What is the expected behavior? Successful build What went wrong? Build failure. See https://bugs.gentoo.org/show_bug.cgi?id=605140 for example. Did this work before? N/A Chrome version: Channel: n/a OS Version: Flash Version: inspector_protocol/templates/TypeBuilder_h.template is using Jinja assignments incorrectly. I believe the fix looks like this (line numbers are from the version of inspector_protocol in V8 6.0.186): @@ -110,12 +110,8 @@ public: public: enum { NoFieldsSet = 0, - {% set count = 0 %} - {% for property in type.properties %} - {% if not(property.optional) %} - {% set count = count + 1 %} - {{property.name | to_title_case}}Set = 1 << {{count}}, - {% endif %} + {% for property in type.properties|rejectattr("optional") %} + {{property.name | to_title_case}}Set = 1 << {{loop.index}}, {% endfor %} AllFieldsSet = ( {%- for property in type.properties %} The Jinja documentation (at least for 2.9) suggests that this code was always buggy: Scoping Behavior Please keep in mind that it is not possible to set variables inside a block and have them show up outside of it. This also applies to loops. The only exception to that rule are if statements which do not introduce a scope. As a result the following template is not going to do what you might expect: {% set iterated = false %} {% for item in seq %} {{ item }} {% set iterated = true %} {% endfor %} {% if not iterated %} did not iterate {% endif %} It is not possible with Jinja syntax to do this. Instead use alternative constructs like the loop else block or the special loop variable: {% for item in seq %} {{ item }} {% else %} did not iterate {% endfor %} Sorry for the cursory bug report. It's late on a Friday...
,
Jul 17 2017
,
Jul 17 2017
,
Oct 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/deps/inspector_protocol/+/8cb7a4f50ff7d5b1b7f2e5df0542dc577c88bdc3 commit 8cb7a4f50ff7d5b1b7f2e5df0542dc577c88bdc3 Author: Alexey Kozyatinskiy <kozyatinskiy@chromium.org> Date: Wed Oct 11 21:32:17 2017 [inspector_protocol] fixed compatibility with latest jinja 2.9.6 Bug: chromium:743313 Change-Id: I70e06837aca6acef5c0d658f46f66df97223097a R=dgozman@chromium.org [modify] https://crrev.com/8cb7a4f50ff7d5b1b7f2e5df0542dc577c88bdc3/templates/TypeBuilder_h.template
,
Oct 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/744b49ef0d1b72de2f55994c38ff0d4dea6a2d10 commit 744b49ef0d1b72de2f55994c38ff0d4dea6a2d10 Author: Alexey Kozyatinskiy <kozyatinskiy@chromium.org> Date: Wed Oct 11 22:52:17 2017 Roll third_party/inspector_protocol to 8cb7a4f50ff7d5b1b7f2e5df0542dc577c88bdc3 This roll includes: - [inspector_protocol] fixed compatibility with latest jinja 2.9.6 - [inspector_protocol] removed unused variable - Follow up on alph's review comments. - Provide default escape implementation for latin and wide strings. - Allow escaping utf8 strings in embedders that operate std::string. - Upload inspector_protocol changes to Gerrit by default - [inspector_protocol] Fix building with non-ASCII paths - [inspector_protocol] added StringUtil::toDouble method as requirement - Add const char* overloads to ErrorSupport BUG= chromium:743313 R=dgozman@chromium.org Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ic81a62c638bf592ae65c84055d53d926e50715ac Reviewed-on: https://chromium-review.googlesource.com/713538 Reviewed-by: Dmitry Gozman <dgozman@chromium.org> Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org> Cr-Commit-Position: refs/heads/master@{#48477} [modify] https://crrev.com/744b49ef0d1b72de2f55994c38ff0d4dea6a2d10/src/inspector/string-util.cc [modify] https://crrev.com/744b49ef0d1b72de2f55994c38ff0d4dea6a2d10/src/inspector/string-util.h [modify] https://crrev.com/744b49ef0d1b72de2f55994c38ff0d4dea6a2d10/third_party/inspector_protocol/CodeGenerator.py [modify] https://crrev.com/744b49ef0d1b72de2f55994c38ff0d4dea6a2d10/third_party/inspector_protocol/README.v8 [modify] https://crrev.com/744b49ef0d1b72de2f55994c38ff0d4dea6a2d10/third_party/inspector_protocol/lib/DispatcherBase_cpp.template [modify] https://crrev.com/744b49ef0d1b72de2f55994c38ff0d4dea6a2d10/third_party/inspector_protocol/lib/DispatcherBase_h.template [modify] https://crrev.com/744b49ef0d1b72de2f55994c38ff0d4dea6a2d10/third_party/inspector_protocol/lib/ErrorSupport_cpp.template [modify] https://crrev.com/744b49ef0d1b72de2f55994c38ff0d4dea6a2d10/third_party/inspector_protocol/lib/ErrorSupport_h.template [modify] https://crrev.com/744b49ef0d1b72de2f55994c38ff0d4dea6a2d10/third_party/inspector_protocol/lib/Values_cpp.template [modify] https://crrev.com/744b49ef0d1b72de2f55994c38ff0d4dea6a2d10/third_party/inspector_protocol/lib/Values_h.template [modify] https://crrev.com/744b49ef0d1b72de2f55994c38ff0d4dea6a2d10/third_party/inspector_protocol/templates/TypeBuilder_cpp.template [modify] https://crrev.com/744b49ef0d1b72de2f55994c38ff0d4dea6a2d10/third_party/inspector_protocol/templates/TypeBuilder_h.template
,
Oct 12 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/79699738fbe646f3a68c3f33b97544ed83b37a72 commit 79699738fbe646f3a68c3f33b97544ed83b37a72 Author: Alexey Kozyatinskiy <kozyatinskiy@chromium.org> Date: Thu Oct 12 14:43:21 2017 Roll third_party/inspector_protocol to 8cb7a4f50ff7d5b1b7f2e5df0542dc577c88bdc3 This roll includes: - [inspector_protocol] fixed compatibility with latest jinja 2.9.6 - [inspector_protocol] removed unused variable - [inspector_protocol] Fix building with non-ASCII paths R=dgozman@chromium.org Bug: chromium:743313 Change-Id: I998a02f6c97ebc82ff5de5d202b5532d3aea5066 Reviewed-on: https://chromium-review.googlesource.com/714236 Reviewed-by: Dmitry Gozman <dgozman@chromium.org> Reviewed-by: Alex Clarke <alexclarke@chromium.org> Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org> Cr-Commit-Position: refs/heads/master@{#508347} [modify] https://crrev.com/79699738fbe646f3a68c3f33b97544ed83b37a72/headless/lib/browser/devtools_api/domain_types_h.template [modify] https://crrev.com/79699738fbe646f3a68c3f33b97544ed83b37a72/third_party/inspector_protocol/CodeGenerator.py [modify] https://crrev.com/79699738fbe646f3a68c3f33b97544ed83b37a72/third_party/inspector_protocol/README.chromium [modify] https://crrev.com/79699738fbe646f3a68c3f33b97544ed83b37a72/third_party/inspector_protocol/templates/TypeBuilder_h.template
,
Oct 12 2017
|
||||
►
Sign in to add a comment |
||||
Comment 1 by msrchandra@chromium.org
, Jul 17 2017