Skip to content

Conversation

@cclauss
Copy link
Contributor

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

@nodejs-github-botnodejs-github-bot added post-mortem Issues and PRs related to the post-mortem diagnostics of Node.js. v8 engine Issues and PRs related to the V8 dependency. labels Nov 2, 2019
@cclausscclauss mentioned this pull request Nov 2, 2019
3 tasks
@nodejs-github-bot
Copy link
Collaborator

@targos
Copy link
Member

do you want me to upstream the change?

@cclauss
Copy link
ContributorAuthor

cclauss commented Nov 2, 2019

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.

@cclausscclauss mentioned this pull request Nov 2, 2019
1 task
@cclausscclauss added the python PRs and issues that require attention from people who are familiar with Python. label Nov 2, 2019
@targos
Copy link
Member

https://chromium-review.googlesource.com/c/v8/v8/+/1895560

@cclauss
Copy link
ContributorAuthor

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

@targos
Copy link
Member

CL landed. I'll update this to be a cherry pick

Copy link
Member

@legendecaslegendecas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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 <[email protected]> Commit-Queue: Michaël Zasso <[email protected]> Cr-Commit-Position: refs/heads/master@{#64717} Refs: v8/v8@a7dffcd
@targostargosforce-pushed the fix-gen-postmortem-metadata.py branch from f4122ca to c21d28eCompareNovember 3, 2019 16:11
@nodejs-github-bot
Copy link
Collaborator

@cclauss
Copy link
ContributorAuthor

Landed in c80771d

@cclausscclauss closed this Nov 4, 2019
@cclausscclauss deleted the fix-gen-postmortem-metadata.py branch November 4, 2019 10:39
cclauss added a commit that referenced this pull request Nov 4, 2019
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 <[email protected]> Commit-Queue: Michaël Zasso <[email protected]> Cr-Commit-Position: refs/heads/master@{#64717} Refs: v8/v8@a7dffcd PR-URL: #30218 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
targos pushed a commit that referenced this pull request Nov 5, 2019
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 <[email protected]> Commit-Queue: Michaël Zasso <[email protected]> Cr-Commit-Position: refs/heads/master@{#64717} Refs: v8/v8@a7dffcd PR-URL: #30218 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
@targostargos mentioned this pull request Nov 5, 2019
targos pushed a commit that referenced this pull request Nov 8, 2019
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 <[email protected]> Commit-Queue: Michaël Zasso <[email protected]> Cr-Commit-Position: refs/heads/master@{#64717} Refs: v8/v8@a7dffcd PR-URL: #30218 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
targos pushed a commit to targos/node that referenced this pull request Nov 8, 2019
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 <[email protected]> Commit-Queue: Michaël Zasso <[email protected]> Cr-Commit-Position: refs/heads/master@{#64717} Refs: v8/v8@a7dffcd PR-URL: nodejs#30218 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
targos pushed a commit that referenced this pull request Nov 10, 2019
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 <[email protected]> Commit-Queue: Michaël Zasso <[email protected]> Cr-Commit-Position: refs/heads/master@{#64717} Refs: v8/v8@a7dffcd PR-URL: #30218 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
targos pushed a commit that referenced this pull request Nov 10, 2019
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 <[email protected]> Commit-Queue: Michaël Zasso <[email protected]> Cr-Commit-Position: refs/heads/master@{#64717} Refs: v8/v8@a7dffcd PR-URL: #30218 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
@targostargos mentioned this pull request Nov 10, 2019
targos pushed a commit that referenced this pull request Nov 11, 2019
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 <[email protected]> Commit-Queue: Michaël Zasso <[email protected]> Cr-Commit-Position: refs/heads/master@{#64717} Refs: v8/v8@a7dffcd PR-URL: #30218 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
targos pushed a commit to targos/node that referenced this pull request Nov 17, 2019
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 <[email protected]> Commit-Queue: Michaël Zasso <[email protected]> Cr-Commit-Position: refs/heads/master@{#64717} Refs: v8/v8@a7dffcd PR-URL: nodejs#30218 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 21, 2019
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 <[email protected]> Commit-Queue: Michaël Zasso <[email protected]> Cr-Commit-Position: refs/heads/master@{#64717} Refs: v8/v8@a7dffcd Backport-PR-URL: #30513 PR-URL: #30218 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
@BridgeARBridgeAR mentioned this pull request Nov 21, 2019
targos pushed a commit to targos/node that referenced this pull request Dec 5, 2019
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 <[email protected]> Commit-Queue: Michaël Zasso <[email protected]> Cr-Commit-Position: refs/heads/master@{#64717} Refs: v8/v8@a7dffcd PR-URL: nodejs#30218 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
MylesBorins pushed a commit to targos/node that referenced this pull request Jan 7, 2020
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 <[email protected]> Commit-Queue: Michaël Zasso <[email protected]> Cr-Commit-Position: refs/heads/master@{#64717} Refs: v8/v8@a7dffcd PR-URL: nodejs#30218 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 8, 2020
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 <[email protected]> Commit-Queue: Michaël Zasso <[email protected]> Cr-Commit-Position: refs/heads/master@{#64717} Refs: v8/v8@a7dffcd Backport-PR-URL: #30109 PR-URL: #30218 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
@targostargos mentioned this pull request Jan 15, 2020
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
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 <[email protected]> Commit-Queue: Michaël Zasso <[email protected]> Cr-Commit-Position: refs/heads/master@{#64717} Refs: v8/v8@a7dffcd Backport-PR-URL: #30109 PR-URL: #30218 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Feb 8, 2020
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

post-mortemIssues and PRs related to the post-mortem diagnostics of Node.js.pythonPRs and issues that require attention from people who are familiar with Python.v8 engineIssues and PRs related to the V8 dependency.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@cclauss@nodejs-github-bot@targos@cjihrig@joyeecheung@devnexen@richardlau@legendecas