Skip to content

Conversation

@cclauss
Copy link
Contributor

This PR uses io.open(encoding="utf-8") to resolve a UnicodeDecodeError on Python 3 after the fix in #30218 has been applied.

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 the tools Issues and PRs related to the tools directory. label Nov 2, 2019
@cclausscclauss added the python PRs and issues that require attention from people who are familiar with Python. label Nov 2, 2019
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

Copy link
Member

@joyeecheungjoyeecheung left a comment

Choose a reason for hiding this comment

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

Thanks!

(And makes me wonder whether it is useful to add an dockerfile to BUILDING.md since it's pretty easy to understand what's necessary to have a working dev environment from a glance)

@nodejs-github-bot
Copy link
Collaborator

cclauss added a commit that referenced this pull request Nov 5, 2019
PR-URL: #30220 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
@cclauss
Copy link
ContributorAuthor

Landed in aa4e54e

@targos is this in time for inclusion in #30262 ?

@cclausscclauss closed this Nov 5, 2019
@cclausscclauss deleted the fix-tools-check-imports.py branch November 5, 2019 10:30
MylesBorins pushed a commit that referenced this pull request Nov 17, 2019
PR-URL: #30220 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
@BridgeARBridgeAR mentioned this pull request Nov 19, 2019
targos pushed a commit that referenced this pull request Dec 1, 2019
PR-URL: #30220 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
@BethGriggsBethGriggs mentioned this pull request Dec 9, 2019
MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
PR-URL: #30220 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
@BethGriggsBethGriggs mentioned this pull request Dec 23, 2019
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pythonPRs and issues that require attention from people who are familiar with Python.toolsIssues and PRs related to the tools directory.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@cclauss@nodejs-github-bot@lpinca@targos@joyeecheung@richardlau@BridgeAR