X Tutup
The Wayback Machine - https://web.archive.org/web/20220504033752/https://github.com/nodejs/node-v8/pull/181
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

GitHub Action to lint upstream v8 #181

Closed
wants to merge 20 commits into from
Closed

Conversation

Copy link

@cclauss cclauss commented Oct 20, 2020

% flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics

./v8/tools/dev/update-compile-commands.py:55:25: F821 undefined name 'Error'
    if code != 0: raise Error("gn gen failed")
                        ^
./v8/tools/clusterfuzz/js_fuzzer/tools/minimize.py:26:1: F631 assertion is always true, perhaps remove parentheses?
assert(len(sys.argv) > 1, 'Need to specify minimizer path.')
^
1     F631 assertion is always true, perhaps remove parentheses?
1     F821 undefined name 'Error'
2
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

nodejs-ci and others added 20 commits Oct 19, 2020
Original commit message:

    [testrunner] delete ancient junit compatible format support

    Testrunner has ancient support for JUnit compatible XML output.

    This CL removes this old feature.

    R=mstarzinger@chromium.org,jgruber@chromium.org,jkummerow@chromium.org
    CC=​machenbach@chromium.org

    Bug: v8:8728
    Change-Id: I7e1beb011dbaec3aa1a27398a5c52abdd778eaf0
    Reviewed-on: https://chromium-review.googlesource.com/c/1430065
    Reviewed-by: Jakob Gruber <jgruber@chromium.org>
    Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
    Commit-Queue: Tamer Tas <tmrts@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#59045}

Refs: v8/v8@bd019bd

PR-URL: nodejs/node#32116
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Patch V8 (compiler/js-heap-broker.cc) to remove the use of an optional
property, which is a fairly new C++ feature, since that requires a newer
XCode version than the minimum requirement in BUILDING.md and thus
breaks CI.

PR-URL: nodejs/node#32116
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Fixes a compilation issue on some platforms

PR-URL: nodejs/node#32116
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This should be semver-patch since actual invocation is version
conditional.

PR-URL: nodejs/node#32116
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
There is a bug in the most recent version of VS2015 that affects v8.h
and therefore prevents compilation of addons.

Refs: https://stackoverflow.com/q/38378693

PR-URL: nodejs/node#32116
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs/node#26685
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>

PR-URL: nodejs/node#32116
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Patch V8 (wasm/wasm-module.cc) to remove const qualifier from type
passed to template call of `OwnedVector::Of`. Xcode 8 can't convert
'OwnedVector<unsigned char>' to 'OwnedVector<const unsigned char>' when
returning from a function (which is likely a bug on Xcode, considering
this worked on the prior version of Xcode as well as newer versions).
This workaround shouldn't affect the application, since the const
qualifier is preserved in the AsmJsOffsetInformation::encoded_offset_.

There's also a V8 test passing a const-qualified type to ::Of, but since
we don't test V8 on Xcode 8, it should be fine to leave it as is.

Signed-off-by: Matheus Marchini <mmarchini@netflix.com>

PR-URL: nodejs/node#32116
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This commit updates V8's gen-postmortem-metadata.py script
to fix SmartOS compilation for V8 8.4.

PR-URL: nodejs/node#33579
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
This commit updates V8's gen-postmortem-metadata.py script to
fix SmartOS compilation with V8 8.5.

PR-URL: nodejs/node#35415
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
PR-URL: nodejs/node#35415
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
PR-URL: nodejs/node#35415
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
until 718fbb89ef9079c142cf252488a571b620493e6f
% __flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics__
```
./v8/tools/dev/update-compile-commands.py:55:25: F821 undefined name 'Error'
    if code != 0: raise Error("gn gen failed")
                        ^
./v8/tools/clusterfuzz/js_fuzzer/tools/minimize.py:26:1: F631 assertion is always true, perhaps remove parentheses?
assert(len(sys.argv) > 1, 'Need to specify minimizer path.')
^
1     F631 assertion is always true, perhaps remove parentheses?
1     F821 undefined name 'Error'
2
```
@cclauss cclauss added the dependencies label Oct 20, 2020
@nodejs-ci nodejs-ci force-pushed the canary branch 2 times, most recently from a55f8ad to d808dee Compare Oct 22, 2020
@ryzokuken
Copy link

@ryzokuken ryzokuken commented Oct 22, 2020

I'm not sure if linting upstream python code is the best idea, since

  1. A lot of Google's python tooling still depends on Python 2.
  2. The project uses Python only for tooling and not for any actual source code.

@cclauss
Copy link
Author

@cclauss cclauss commented Oct 22, 2020

Both of the issues raised by these tests are equally problematic on both Python 2 and Python 3.

Those two statements above are contradictory. If Google writes its tooling for legacy Python and we run Google's tooling on both Python 2 and Python 3 then we can be broken. The whole idea behind canary is that you want to be warned about problems before they cause injury. The bottom line is that the code is not linted upstream yet we rely on it. An ounce of prevention...

@nodejs-ci nodejs-ci force-pushed the canary branch 5 times, most recently from 3d093d3 to 5209090 Compare Oct 27, 2020
@nodejs-ci nodejs-ci force-pushed the canary branch 3 times, most recently from ccf61f1 to fe151d3 Compare Mar 2, 2021
@nodejs-ci nodejs-ci force-pushed the canary branch 8 times, most recently from 0b1c92e to 66d2bc0 Compare Mar 14, 2021
@nodejs-ci nodejs-ci force-pushed the canary branch 7 times, most recently from f90a975 to f3309ba Compare Mar 21, 2021
@nodejs-ci nodejs-ci force-pushed the canary branch 7 times, most recently from 27ab352 to b0028e1 Compare Mar 28, 2021
@nodejs-ci nodejs-ci force-pushed the canary branch 3 times, most recently from 7ecd31a to 2a3b66c Compare Mar 31, 2021
@cclauss cclauss closed this Mar 31, 2021
@cclauss cclauss deleted the GitHub-Action-to-lint-upstream-v8 branch Mar 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants
X Tutup