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.

Sign up for GitHub

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

Jump to bottom

deps: exclude linker scripts for windows builds #42973

Closed
wants to merge 2 commits into from
Closed

deps: exclude linker scripts for windows builds #42973

wants to merge 2 commits into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented May 5, 2022 •
edited
Loading

This commit ignore fips.ld and legacy.ld linker scripts for Windows
environments using msbuild.

Fixes: #40854

@nodejs-github-bot nodejs-github-bot added dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run. openssl Issues and PRs related to the OpenSSL dependency. labels May 5, 2022
@mhdawson
Copy link
Member

mhdawson commented May 5, 2022

This commit ignore fips.ld and legacy.ld linker scripts for Windows

Just wondering how ignoring those files adds the provider? There are so many files updated it's hard to tell from the PR itself.

@danbev
Copy link
Contributor Author

danbev commented May 6, 2022

Just wondering how ignoring those files adds the provider? There are so many files updated it's hard to tell from the PR itself

This PR is not about adding a provider. The first commit the actual content of the changes in this PR and then we need to regenerate the OpenSSL header files.

@danbev
deps: exclude linker scripts for windows builds
1ecf3c4 
This commit ignore fips.ld and legacy.ld linker scripts for Windows
environments.

Fixes: nodejs#40854
@danbev
deps: regenerate OpenSSL archs files
4eb4d0e
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/44028/

This was referenced May 17, 2022
Open
Open
Open
Open
Open
Open
@danbev
Copy link
Contributor Author

danbev commented May 23, 2022

@mhdawson I've taken a closer look at this and my understanding is that the linker scripts are not used with msbuild and can therefore be removed the from sources list in the .gyp files for these environments.

@mhdawson
Copy link
Member

@danbev ok thanks for the update.

mhdawson
mhdawson approved these changes May 25, 2022
View reviewed changes
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/44232/

@danbev
Copy link
Contributor Author

danbev commented May 30, 2022

Landed in 2f44273, and c3a127d.

@danbev danbev closed this May 30, 2022
danbev added a commit that referenced this pull request May 30, 2022
@danbev
deps: exclude linker scripts for windows builds
c3a127d 
This commit ignore fips.ld and legacy.ld linker scripts for Windows
environments.

PR-URL: #42973
Fixes: #40854
Reviewed-By: Michael Dawson <midawson@redhat.com>
danbev added a commit that referenced this pull request May 30, 2022
@danbev
deps: regenerate OpenSSL archs files
2f44273 
PR-URL: #42973
Reviewed-By: Michael Dawson <midawson@redhat.com>
@danbev danbev deleted the fips.ld_legacy.ld_win branch May 30, 2022 06:48
bengl pushed a commit that referenced this pull request May 30, 2022
@danbev @bengl
deps: exclude linker scripts for windows builds
ecacc3a 
This commit ignore fips.ld and legacy.ld linker scripts for Windows
environments.

PR-URL: #42973
Fixes: #40854
Reviewed-By: Michael Dawson <midawson@redhat.com>
bengl pushed a commit that referenced this pull request May 30, 2022
@danbev @bengl
deps: regenerate OpenSSL archs files
35250bf 
PR-URL: #42973
Reviewed-By: Michael Dawson <midawson@redhat.com>
@github-actions github-actions bot mentioned this pull request May 31, 2022
Open
16 tasks
@bengl bengl mentioned this pull request May 31, 2022
Merged
@github-actions github-actions bot mentioned this pull request Jun 1, 2022
Open
17 tasks
F3n67u pushed a commit to F3n67u/node that referenced this pull request Jun 1, 2022
@danbev @F3n67u
deps: exclude linker scripts for windows builds
24cfada 
This commit ignore fips.ld and legacy.ld linker scripts for Windows
environments.

PR-URL: nodejs#42973
Fixes: nodejs#40854
Reviewed-By: Michael Dawson <midawson@redhat.com>
F3n67u pushed a commit to F3n67u/node that referenced this pull request Jun 1, 2022
@danbev @F3n67u
deps: regenerate OpenSSL archs files
a52b8f1 
PR-URL: nodejs#42973
Reviewed-By: Michael Dawson <midawson@redhat.com>
This was referenced Jun 2, 2022
Open
Open
@danielleadams
Copy link
Member

danielleadams commented Jun 26, 2022 •
edited
Loading

@danbev is this able to be landed in v16.x or is there a backport PR for this? It didn't land cleanly in v16.x-staging for the v16.15.2 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mhdawson mhdawson mhdawson approved these changes

Assignees
No one assigned
Labels
dependencies Pull requests that update a dependency file. needs-ci PRs that need a full CI run. openssl Issues and PRs related to the OpenSSL dependency.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

OpenSSL fips.ld and legacy.ld missing for Windows
5 participants
@danbev @mhdawson @nodejs-github-bot @danielleadams @targos