★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/30218Nouvelle recherche | Portail wanayoo
Skip to content

/node

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

v8: load_objects_from_file() using utf-8 #30218

Closed
wants to merge 1 commit into from
Closed

v8: load_objects_from_file() using utf-8 #30218

wants to merge 1 commit into from
+4 −3

Conversation

@cclauss
Copy link
Contributor

cclauss commented Nov 2, 2019

This PR uses io.open(encoding="utf-8") to resolve a UnicodeDecodeError reported at nodejs/code-and-learn#99 (comment)

This can be replicated by running docker build . using the following:

Dockerfile 
FROM ubuntu:bionic

# Install tools
RUN apt-get update
RUN apt-get install -y build-essential python3 python3-pip ninja-build make g++ vim git sudo

# Expose ports used by net tests
ENV NODE_COMMON_PORT=12346
EXPOSE 12346
# debug port used by v8 debugger
EXPOSE 5858
# debug port used by v8 insepctor
EXPOSE 9229

# Set up user node-dev: the tests fail when run by the root
RUN adduser --disabled-password --gecos '' node-dev
RUN adduser node-dev sudo
RUN echo '%sudo ALL=(ALL) NOPASSWD:ALL' >> /etc/sudoers
USER node-dev
WORKDIR /home/node-dev
ENV HOME /home/node-dev

# Clone the code and build
RUN git clone --depth=1 https://github.com/nodejs/node.git
WORKDIR /home/node-dev/node
RUN python3 -m pip install setuptools
RUN python3 ./configure --ninja
ENV BUILD_WITH ninja
RUN make -j2 test
Checklist
@richardlau
richardlau approved these changes Nov 2, 2019
View changes
@cclauss cclauss mentioned this pull request Nov 2, 2019
Closed
3 of 3 tasks complete
@nodejs-github-bot

This comment has been minimized.

Sign in to view
Copy link

nodejs-github-bot commented Nov 2, 2019

CI: https://ci.nodejs.org/job/node-test-pull-request/26290/
@targos

This comment has been minimized.

Sign in to view
Copy link
Member

targos commented Nov 2, 2019

do you want me to upstream the change?
@cclauss

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

cclauss commented Nov 2, 2019 •
edited

Yes, please. Should we wait for our Jenkins tests to pass first? Your call.

It works on the Dockerfile when I do a COPY gen-postmortem-metadata.py ./deps/v8/tools/ with these mods.
@cclauss cclauss mentioned this pull request Nov 2, 2019
Closed
1 of 1 task complete
@cclauss cclauss added the Python label Nov 2, 2019
This was referenced Nov 2, 2019
Merged
Open
@targos

This comment has been minimized.

Sign in to view
Copy link
Member

targos commented Nov 2, 2019

https://chromium-review.googlesource.com/c/v8/v8/+/1895560
@devnexen
devnexen approved these changes Nov 3, 2019
View changes
@cclauss

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

cclauss commented Nov 3, 2019

Nice! Any hints on how to get that arm-fanned to pass?
@targos

This comment has been minimized.

Sign in to view
Copy link
Member

targos commented Nov 3, 2019

CL landed. I'll update this to be a cherry pick
@cjihrig
cjihrig approved these changes Nov 3, 2019
View changes
@legendecas
legendecas approved these changes Nov 3, 2019
View changes
Copy link
Member

legendecas left a comment

LGTM
deps/v8/tools/gen-postmortem-metadata.py Outdated Show resolved Hide resolved
@cclauss @targos
deps: V8: cherry-pick a7dffcd767be
Verified
This commit was signed with a verified signature.
targos Michaël Zasso
GPG key ID: 770F7A9A5AE15600 Learn about signing commits
Loading status checks…
c21d28e 
Original commit message:

    [postmortem] Load files using utf-8 to support Python 3

    Change-Id: I174d38cc33210c07d1a7596627e1b2d21bb06313
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1895560
    Reviewed-by: Mathias Bynens <mathias@chromium.org>
    Commit-Queue: Michaël Zasso <mic.besace@gmail.com>
    Cr-Commit-Position: refs/heads/master@{#64717}

Refs: v8/v8@a7dffcd
@targos targos force-pushed the cclauss:fix-gen-postmortem-metadata.py branch from f4122ca to c21d28e Nov 3, 2019
@nodejs-github-bot

This comment has been minimized.

Sign in to view
Copy link

nodejs-github-bot commented Nov 3, 2019

CI: https://ci.nodejs.org/job/node-test-pull-request/26301/
@joyeecheung
joyeecheung approved these changes Nov 3, 2019
View changes
@cclauss

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

cclauss commented Nov 4, 2019

Landed in c80771d
@cclauss cclauss closed this Nov 4, 2019
@cclauss cclauss deleted the cclauss:fix-gen-postmortem-metadata.py branch Nov 4, 2019
cclauss added a commit that referenced this pull request Nov 4, 2019
@cclauss
deps: V8: cherry-pick a7dffcd767be
Loading status checks…
c80771d 
Original commit message:

    [postmortem] Load files using utf-8 to support Python 3

    Change-Id: I174d38cc33210c07d1a7596627e1b2d21bb06313
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1895560
    Reviewed-by: Mathias Bynens <mathias@chromium.org>
    Commit-Queue: Michaël Zasso <mic.besace@gmail.com>
    Cr-Commit-Position: refs/heads/master@{#64717}

Refs: v8/v8@a7dffcd

PR-URL: #30218
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
targos added a commit to targos/node that referenced this pull request Nov 4, 2019
@cclauss @targos
deps: V8: cherry-pick a7dffcd767be
Verified
This commit was signed with a verified signature.
targos Michaël Zasso
GPG key ID: 770F7A9A5AE15600 Learn about signing commits
68e602f 
Original commit message:

    [postmortem] Load files using utf-8 to support Python 3

    Change-Id: I174d38cc33210c07d1a7596627e1b2d21bb06313
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1895560
    Reviewed-by: Mathias Bynens <mathias@chromium.org>
    Commit-Queue: Michaël Zasso <mic.besace@gmail.com>
    Cr-Commit-Position: refs/heads/master@{#64717}

Refs: v8/v8@a7dffcd

PR-URL: nodejs#30218
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
targos added a commit that referenced this pull request Nov 5, 2019
@cclauss @targos
deps: V8: cherry-pick a7dffcd767be
Verified
This commit was signed with a verified signature.
targos Michaël Zasso
GPG key ID: 770F7A9A5AE15600 Learn about signing commits
46c9194 
Original commit message:

    [postmortem] Load files using utf-8 to support Python 3

    Change-Id: I174d38cc33210c07d1a7596627e1b2d21bb06313
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1895560
    Reviewed-by: Mathias Bynens <mathias@chromium.org>
    Commit-Queue: Michaël Zasso <mic.besace@gmail.com>
    Cr-Commit-Position: refs/heads/master@{#64717}

Refs: v8/v8@a7dffcd

PR-URL: #30218
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
@targos targos mentioned this pull request Nov 5, 2019
Merged
addaleax added a commit to targos/node that referenced this pull request Nov 5, 2019
@cclauss @addaleax
deps: V8: cherry-pick a7dffcd767be
Verified
This commit was signed with a verified signature.
addaleax Anna Henningsen
GPG key ID: 9C63F3A6CD2AD8F9 Learn about signing commits
7fe8be1 
Original commit message:

    [postmortem] Load files using utf-8 to support Python 3

    Change-Id: I174d38cc33210c07d1a7596627e1b2d21bb06313
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1895560
    Reviewed-by: Mathias Bynens <mathias@chromium.org>
    Commit-Queue: Michaël Zasso <mic.besace@gmail.com>
    Cr-Commit-Position: refs/heads/master@{#64717}

Refs: v8/v8@a7dffcd

PR-URL: nodejs#30218
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
targos added a commit that referenced this pull request Nov 8, 2019
@cclauss @targos
deps: V8: cherry-pick a7dffcd767be
Verified
This commit was signed with a verified signature.
targos Michaël Zasso
GPG key ID: 770F7A9A5AE15600 Learn about signing commits
f88fdd0 
Original commit message:

    [postmortem] Load files using utf-8 to support Python 3

    Change-Id: I174d38cc33210c07d1a7596627e1b2d21bb06313
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1895560
    Reviewed-by: Mathias Bynens <mathias@chromium.org>
    Commit-Queue: Michaël Zasso <mic.besace@gmail.com>
    Cr-Commit-Position: refs/heads/master@{#64717}

Refs: v8/v8@a7dffcd

PR-URL: #30218
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
targos added a commit to targos/node that referenced this pull request Nov 8, 2019
@cclauss @targos
deps: V8: cherry-pick a7dffcd767be
Verified
This commit was signed with a verified signature.
targos Michaël Zasso
GPG key ID: 770F7A9A5AE15600 Learn about signing commits
8c86c37 
Original commit message:

    [postmortem] Load files using utf-8 to support Python 3

    Change-Id: I174d38cc33210c07d1a7596627e1b2d21bb06313
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1895560
    Reviewed-by: Mathias Bynens <mathias@chromium.org>
    Commit-Queue: Michaël Zasso <mic.besace@gmail.com>
    Cr-Commit-Position: refs/heads/master@{#64717}

Refs: v8/v8@a7dffcd

PR-URL: nodejs#30218
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
targos added a commit to targos/node that referenced this pull request Nov 8, 2019
@cclauss @targos
deps: V8: cherry-pick a7dffcd767be
Verified
This commit was signed with a verified signature.
targos Michaël Zasso
GPG key ID: 770F7A9A5AE15600 Learn about signing commits
10af006 
Original commit message:

    [postmortem] Load files using utf-8 to support Python 3

    Change-Id: I174d38cc33210c07d1a7596627e1b2d21bb06313
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1895560
    Reviewed-by: Mathias Bynens <mathias@chromium.org>
    Commit-Queue: Michaël Zasso <mic.besace@gmail.com>
    Cr-Commit-Position: refs/heads/master@{#64717}

Refs: v8/v8@a7dffcd

PR-URL: nodejs#30218
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
targos added a commit that referenced this pull request Nov 10, 2019
@cclauss @targos
deps: V8: cherry-pick a7dffcd767be
Verified
This commit was signed with a verified signature.
targos Michaël Zasso
GPG key ID: 770F7A9A5AE15600 Learn about signing commits
6df1e25 
Original commit message:

    [postmortem] Load files using utf-8 to support Python 3

    Change-Id: I174d38cc33210c07d1a7596627e1b2d21bb06313
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1895560
    Reviewed-by: Mathias Bynens <mathias@chromium.org>
    Commit-Queue: Michaël Zasso <mic.besace@gmail.com>
    Cr-Commit-Position: refs/heads/master@{#64717}

Refs: v8/v8@a7dffcd

PR-URL: #30218
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
targos added a commit that referenced this pull request Nov 10, 2019
@cclauss @targos
deps: V8: cherry-pick a7dffcd767be
Verified
This commit was signed with a verified signature.
targos Michaël Zasso
GPG key ID: 770F7A9A5AE15600 Learn about signing commits
ad4e217 
Original commit message:

    [postmortem] Load files using utf-8 to support Python 3

    Change-Id: I174d38cc33210c07d1a7596627e1b2d21bb06313
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1895560
    Reviewed-by: Mathias Bynens <mathias@chromium.org>
    Commit-Queue: Michaël Zasso <mic.besace@gmail.com>
    Cr-Commit-Position: refs/heads/master@{#64717}

Refs: v8/v8@a7dffcd

PR-URL: #30218
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
@targos targos mentioned this pull request Nov 10, 2019
Merged
targos added a commit that referenced this pull request Nov 11, 2019
@cclauss @targos
deps: V8: cherry-pick a7dffcd767be
Verified
This commit was signed with a verified signature.
targos Michaël Zasso
GPG key ID: 770F7A9A5AE15600 Learn about signing commits
b1529c6 
Original commit message:

    [postmortem] Load files using utf-8 to support Python 3

    Change-Id: I174d38cc33210c07d1a7596627e1b2d21bb06313
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1895560
    Reviewed-by: Mathias Bynens <mathias@chromium.org>
    Commit-Queue: Michaël Zasso <mic.besace@gmail.com>
    Cr-Commit-Position: refs/heads/master@{#64717}

Refs: v8/v8@a7dffcd

PR-URL: #30218
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
targos added a commit to targos/node that referenced this pull request Nov 17, 2019
@cclauss @targos
deps: V8: cherry-pick a7dffcd767be
Verified
This commit was signed with a verified signature.
targos Michaël Zasso
GPG key ID: 770F7A9A5AE15600 Learn about signing commits
493311b 
Original commit message:

    [postmortem] Load files using utf-8 to support Python 3

    Change-Id: I174d38cc33210c07d1a7596627e1b2d21bb06313
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1895560
    Reviewed-by: Mathias Bynens <mathias@chromium.org>
    Commit-Queue: Michaël Zasso <mic.besace@gmail.com>
    Cr-Commit-Position: refs/heads/master@{#64717}

Refs: v8/v8@a7dffcd

PR-URL: nodejs#30218
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
MylesBorins added a commit that referenced this pull request Nov 21, 2019
@cclauss @MylesBorins
deps: V8: cherry-pick a7dffcd767be
Verified
This commit was signed with a verified signature.
MylesBorins Myles Borins
GPG key ID: 933B01F40B5CA946 Learn about signing commits
5a1ad57 
Original commit message:

    [postmortem] Load files using utf-8 to support Python 3

    Change-Id: I174d38cc33210c07d1a7596627e1b2d21bb06313
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1895560
    Reviewed-by: Mathias Bynens <mathias@chromium.org>
    Commit-Queue: Michaël Zasso <mic.besace@gmail.com>
    Cr-Commit-Position: refs/heads/master@{#64717}

Refs: v8/v8@a7dffcd

Backport-PR-URL: #30513
PR-URL: #30218
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Nov 21, 2019
Merged
targos added a commit to targos/node that referenced this pull request Dec 5, 2019
@cclauss @targos
deps: V8: cherry-pick a7dffcd767be
Verified
This commit was signed with a verified signature.
targos Michaël Zasso
GPG key ID: 770F7A9A5AE15600 Learn about signing commits
4044ccd 
Original commit message:

    [postmortem] Load files using utf-8 to support Python 3

    Change-Id: I174d38cc33210c07d1a7596627e1b2d21bb06313
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1895560
    Reviewed-by: Mathias Bynens <mathias@chromium.org>
    Commit-Queue: Michaël Zasso <mic.besace@gmail.com>
    Cr-Commit-Position: refs/heads/master@{#64717}

Refs: v8/v8@a7dffcd

PR-URL: nodejs#30218
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
targos added a commit to targos/node that referenced this pull request Dec 5, 2019
@cclauss @targos
deps: V8: cherry-pick a7dffcd767be
Verified
This commit was signed with a verified signature.
targos Michaël Zasso
GPG key ID: 770F7A9A5AE15600 Learn about signing commits
5726545 
Original commit message:

    [postmortem] Load files using utf-8 to support Python 3

    Change-Id: I174d38cc33210c07d1a7596627e1b2d21bb06313
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1895560
    Reviewed-by: Mathias Bynens <mathias@chromium.org>
    Commit-Queue: Michaël Zasso <mic.besace@gmail.com>
    Cr-Commit-Position: refs/heads/master@{#64717}

Refs: v8/v8@a7dffcd

PR-URL: nodejs#30218
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
targos added a commit to targos/node that referenced this pull request Dec 6, 2019
@cclauss @targos
deps: V8: cherry-pick a7dffcd767be
Verified
This commit was signed with a verified signature.
targos Michaël Zasso
GPG key ID: 770F7A9A5AE15600 Learn about signing commits
ebc557f 
Original commit message:

    [postmortem] Load files using utf-8 to support Python 3

    Change-Id: I174d38cc33210c07d1a7596627e1b2d21bb06313
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1895560
    Reviewed-by: Mathias Bynens <mathias@chromium.org>
    Commit-Queue: Michaël Zasso <mic.besace@gmail.com>
    Cr-Commit-Position: refs/heads/master@{#64717}

Refs: v8/v8@a7dffcd

PR-URL: nodejs#30218
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@legendecas legendecas

@cjihrig cjihrig

@joyeecheung joyeecheung

@devnexen devnexen

@richardlau richardlau

@sam-github sam-github

Assignees
No one assigned
Labels
Python V8 Engine post-mortem
Projects
None yet
Milestone
No milestone
8 participants
@cclauss @nodejs-github-bot @targos @cjihrig @joyeecheung @devnexen @richardlau @legendecas
You can’t perform that action at this time.