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

build: fix building when there is only python3 #48462

Merged
merged 5 commits into from
Oct 25, 2023

Conversation

zcbenz
Copy link
Contributor

@zcbenz zcbenz commented Jun 15, 2023

On some environments, especially macOS, there is only python3 and no python, replacing the python to <(python) can make gyp use the correct python.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/v8-update

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. v8 engine Issues and PRs related to the V8 dependency. labels Jun 15, 2023
Copy link
Member

@gengjiawen gengjiawen left a comment

Choose a reason for hiding this comment

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

This breaks all CI. I also have this changes on my macOS too.

@targos
Copy link
Member

targos commented Jun 16, 2023

The error is:

gyp: Undefined variable python in /home/runner/work/node/node/tools/v8_gypfiles/v8.gyp while loading dependencies of /home/runner/work/node/node/node.gyp while trying to load /home/runner/work/node/node/node.gyp

@gengjiawen gengjiawen added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Jun 17, 2023
@gengjiawen gengjiawen added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 17, 2023
@github-actions github-actions bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Jun 17, 2023
@github-actions
Copy link
Contributor

Failed to start CI
- Validating Jenkins credentials
✘  Jenkins credentials invalid
https://github.com/nodejs/node/actions/runs/5295843320

@gengjiawen gengjiawen added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels Jun 17, 2023
@github-actions github-actions bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Jun 17, 2023
@github-actions
Copy link
Contributor

Failed to start CI
- Validating Jenkins credentials
✘  Jenkins credentials invalid
https://github.com/nodejs/node/actions/runs/5298312237

@gengjiawen gengjiawen added request-ci Add this label to start a Jenkins CI on a PR. request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels Jun 23, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 23, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@gengjiawen
Copy link
Member

gengjiawen commented Jun 23, 2023

broken on windows arm64 cc @nodejs/platform-arm @StefanStojanovic

11:04:03   'C:\Python310\python_host.exe' is not recognized as an internal or external command,
11:04:03   operable program or batch file.
11:04:03 C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\MSBuild\Microsoft\VC\v160\Microsoft.CppCommon.targets(241,5): error MSB8066: Custom build for 'out\Release\\node_js2c_host.exe;src\inspector\node_protocol.pdl;src\inspector\node_protocol_config.json;deps\v8\include\js_protocol.pdl;out\Release\\obj\global_intermediate\concatenated_protocol.json;deps\openssl\openssl\util\libcrypto.num' exited with code 1. [C:\workspace\node-compile-windows\node\libnode.vcxproj]
11:04:03   gen-regexp-special-case.cc
11:04:03 C:\workspace\node-compile-windows\node\deps\v8\src\base\bits.h(448,31): warning C4146: unary minus operator applied to unsigned type, result still unsigned [C:\workspace\node-compile-windows\node\tools\v8_gypfiles\gen-regexp-special-case_host.vcxproj]
11:04:04      Creating library ..\..\out\Release\gen-regexp-special-case_host.lib and object ..\..\out\Release\gen-regexp-special-case_host.exp
11:04:04   gen-regexp-special-case_host.vcxproj -> ..\..\out\Release\\gen-regexp-special-case_host.exe
11:04:05   run_gen-regexp-special-case_action
11:04:05   'C:\Python310\python_host.exe' is not recognized as an internal or external command,
11:04:05   operable program or batch file.
11:04:05 C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\MSBuild\Microsoft\VC\v160\Microsoft.CppCommon.targets(241,5): error MSB8066: Custom build for '..\..\out\Release\\gen-regexp-special-case_host.exe' exited with code 1. [C:\workspace\node-compile-windows\node\tools\v8_gypfiles\run_gen-regexp-special-case.vcxproj]
11:04:05   run_gen-regexp-special-case_action
11:04:05   'C:\Python310\python_host.exe' is not recognized as an internal or external command,
11:04:05   operable program or batch file.
11:04:05 C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\MSBuild\Microsoft\VC\v160\Microsoft.CppCommon.targets(241,5): error MSB8066: Custom build for '..\..\out\Release\\gen-regexp-special-case_host.exe' exited with code 1. [C:\workspace\node-compile-windows\node\tools\v8_gypfiles\run_gen-regexp-special-case_host.vcxproj]
11:04:05 
11:04:05 > if errorlevel 1 (
11:04:05 if not defined project_generated echo Building Node with reused solution failed. To regenerate project files use "vcbuild projgen"  

@zcbenz
Copy link
Contributor Author

zcbenz commented Oct 24, 2023

@gengjiawen Can you trigger CI again? I don't have permission to add issue label.

@gengjiawen gengjiawen added the request-ci Add this label to start a Jenkins CI on a PR. label Oct 24, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 24, 2023
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@gengjiawen gengjiawen added the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 24, 2023
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Oct 24, 2023
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/48462
✔  Done loading data for nodejs/node/pull/48462
----------------------------------- PR info ------------------------------------
Title      build: fix building when there is only python3 (#48462)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     zcbenz:patch-8 -> nodejs:main
Labels     v8 engine, tools, needs-ci, commit-queue-squash
Commits    5
 - build: fix building when there is only python3
 - build: always set 'python' variable when running GYP
 - build: add quotes when executing binary
 - build: work around gyp hack for win arm64
 - gyp: add quotes for action's executable
Committers 1
 - Cheng Zhao 
PR-URL: https://github.com/nodejs/node/pull/48462
Reviewed-By: Luigi Pinca 
Reviewed-By: Jiawen Geng 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/48462
Reviewed-By: Luigi Pinca 
Reviewed-By: Jiawen Geng 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - build: fix building when there is only python3
   ⚠  - build: always set 'python' variable when running GYP
   ⚠  - build: add quotes when executing binary
   ⚠  - build: work around gyp hack for win arm64
   ⚠  - gyp: add quotes for action's executable
   ℹ  This PR was created on Thu, 15 Jun 2023 02:04:04 GMT
   ✔  Approvals: 2
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/48462#pullrequestreview-1482098764
   ✔  - Jiawen Geng (@gengjiawen): https://github.com/nodejs/node/pull/48462#pullrequestreview-1575981535
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2023-10-24T11:34:56Z: https://ci.nodejs.org/job/node-test-pull-request/55180/
- Querying data for job/node-test-pull-request/55180/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/6626587515

@@ -438,6 +438,7 @@ def _BuildCommandLineForRuleRaw(
# Support a mode for using cmd directly.
# Convert any paths to native form (first element is used directly).
# TODO(quote): regularize quoting path names throughout the module
command[1] = '"%s"' % command[1]
arguments = ['"%s"' % i for i in arguments]
Copy link
Member

Choose a reason for hiding this comment

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

This should also upstream to https://github.com/nodejs/gyp-next

@gengjiawen gengjiawen added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Oct 25, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 25, 2023
@nodejs-github-bot nodejs-github-bot merged commit d1ccca9 into nodejs:main Oct 25, 2023
63 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in d1ccca9

alexfernandez pushed a commit to alexfernandez/node that referenced this pull request Nov 1, 2023
PR-URL: nodejs#48462
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
targos pushed a commit that referenced this pull request Nov 11, 2023
PR-URL: #48462
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 11, 2023
PR-URL: #48462
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
@UlisesGascon UlisesGascon mentioned this pull request Dec 12, 2023
joshuafried pushed a commit to joshuafried/node that referenced this pull request Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commit-queue-failed An error occurred while landing this pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants