★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/31095Nouvelle 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

build: switch realpath to pwd #31095

Closed
wants to merge 3 commits into from
Closed

build: switch realpath to pwd #31095

wants to merge 3 commits into from

Conversation

@bcoe
Copy link
Member

bcoe commented Dec 26, 2019

Our test coverage linux machine doesn't have the realpath bin, which I believe comes in as part of coreutils?

This became an issue in #30954, which I believe caused realpath to start getting invoked on these machines for the first time.

I believe we can simply swap out the use of realpath to pwd, for our purposes.

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
@bcoe
build: switch realpath to pwd
Loading status checks…
e6b9f68
@bcoe bcoe requested review from addaleax, jasnell, gabrielschulhof and Trott Dec 26, 2019
@bcoe
whoops, that should have been ld.implicit.script.lld
Loading status checks…
6c878a9
@Trott

This comment has been minimized.

Sign in to view
Copy link
Member

Trott commented Dec 26, 2019

Our test coverage linux machine doesn't have the realpath bin, which I believe comes in as part of coreutils?

@nodejs/build
@Trott

This comment has been minimized.

Sign in to view
Copy link
Member

Trott commented Dec 26, 2019

I believe we can simply swap out the use of realpath to pwd, for our purposes.

@nodejs/python @nodejs/build-files
@addaleax
addaleax approved these changes Dec 26, 2019
View changes
Copy link
Member

addaleax left a comment

Do we need the prefix at all? Other parts of node.gypi also refer to source tree files using relative paths, so I’d expect that to work too
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
Copy link

nodejs-github-bot commented Dec 26, 2019

CI: https://ci.nodejs.org/job/node-test-pull-request/27960/
@bcoe bcoe force-pushed the bcoe:build-realpath branch to 6c878a9 Dec 26, 2019
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
Copy link

nodejs-github-bot commented Dec 26, 2019 •
edited by gabrielschulhof

CI: https://ci.nodejs.org/job/node-test-pull-request/27962/ Result was: ⚠
@bcoe

This comment has been minimized.

Sign in to view
Copy link
Member Author

bcoe commented Dec 26, 2019

@addaleax having experimented with our CI, it does seem that the path provided to the linker needs to be absolute; I've reverted to the prior pwd approach, and it seems to be working like a charm. I think pwd will be more universally available out of the gate, than realpath.
@gabrielschulhof
gabrielschulhof approved these changes Dec 26, 2019
View changes
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
Copy link

nodejs-github-bot commented Dec 26, 2019

CI: https://ci.nodejs.org/job/node-test-pull-request/27965/
@bcoe
force build
Loading status checks…
7bc04bf
@nodejs-github-bot

This comment has been minimized.

Sign in to view
Copy link

nodejs-github-bot commented Dec 26, 2019

CI: https://ci.nodejs.org/job/node-test-pull-request/27966/
@nodejs-github-bot

This comment has been minimized.

Sign in to view
Copy link

nodejs-github-bot commented Dec 26, 2019

CI: https://ci.nodejs.org/job/node-test-pull-request/27968/
@BridgeAR
BridgeAR approved these changes Dec 27, 2019
View changes
@BridgeAR

This comment has been minimized.

Sign in to view
Copy link
Member

BridgeAR commented Dec 27, 2019

Maybe fast track this to fix the coverage builds?
gabrielschulhof pushed a commit that referenced this pull request Dec 27, 2019
@bcoe Gabriel Schulhof
build: switch realpath to pwd
Loading status checks…
ce0fb0f 
PR-URL: #31095
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@gabrielschulhof

This comment has been minimized.

Sign in to view
Copy link

gabrielschulhof commented Dec 27, 2019

Landed in ce0fb0f.
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this pull request Dec 28, 2019
@bcoe Gabriel Schulhof
build: switch realpath to pwd
816bbae 
PR-URL: nodejs#31095
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this pull request Dec 31, 2019
@bcoe Gabriel Schulhof
build: switch realpath to pwd
40afe3d 
PR-URL: nodejs#31095
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
BridgeAR added a commit that referenced this pull request Jan 3, 2020
@bcoe @BridgeAR
build: switch realpath to pwd
Unverified
The committer email address is not verified.
Learn about signing commits
ca05a5b 
PR-URL: #31095
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@BridgeAR BridgeAR mentioned this pull request Jan 7, 2020
Merged
MylesBorins added a commit that referenced this pull request Jan 8, 2020
@bcoe @MylesBorins
build: switch realpath to pwd
Verified
This commit was signed with a verified signature.
MylesBorins Myles Borins
GPG key ID: 933B01F40B5CA946 Learn about signing commits
dea40f8 
Backport-PR-URL: #31063
PR-URL: #31095
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@targos targos mentioned this pull request Jan 15, 2020
Closed
BethGriggs added a commit that referenced this pull request Feb 6, 2020
@bcoe @BethGriggs
build: switch realpath to pwd
Verified
This commit was signed with a verified signature.
BethGriggs Bethany Nicolle Griggs
GPG key ID: D7062848A1AB005C Learn about signing commits
e67b360 
Backport-PR-URL: #31063
PR-URL: #31095
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@MylesBorins MylesBorins mentioned this pull request Feb 8, 2020
Merged
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this pull request Mar 4, 2020
@bcoe Gabriel Schulhof
build: switch realpath to pwd
fd8d347 
PR-URL: nodejs#31095
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this pull request Mar 6, 2020
@bcoe Gabriel Schulhof
build: switch realpath to pwd
ed5e50d 
PR-URL: nodejs#31095
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
saitonakamura pushed a commit to saitonakamura/node that referenced this pull request Apr 7, 2020
@bcoe Michael Bashurov
build: switch realpath to pwd
7f1431a 
PR-URL: nodejs#31095
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this pull request Apr 9, 2020
@bcoe Gabriel Schulhof
build: switch realpath to pwd
17fa34c 
PR-URL: nodejs#31095
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@addaleax addaleax

@gabrielschulhof gabrielschulhof

@BridgeAR BridgeAR

@jasnell jasnell

@Trott Trott

Assignees
No one assigned
Labels
build fast-track
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
@bcoe @Trott @nodejs-github-bot @BridgeAR @gabrielschulhof @addaleax
You can’t perform that action at this time.