GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — build: add `--without-amaro` build flag by aduh95 · Pull Request #54136 · nodejs/node

Conversation

aduh95 commented Jul 31, 2024

Copy link

Copy Markdown

Contributor

With ./configure --without-amaro, the resulting node binary won't contain the Amaro source (which includes the SWC WASM binary). This PR does not remove the --experimental-strip-types CLI flag, when using this flag an error will be emitted when trying to load a module-typescript or commonjs-typescript module.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

marco-ippolito and ChALkeR reacted with thumbs up emoji

All reactions

👍 2 reactions


          build: add --without-amaro build flag

92cc486

aduh95 requested a review from marco-ippolito

July 31, 2024 09:55

nodejs-github-bot commented Jul 31, 2024

Copy link

Copy Markdown

Collaborator

Review requested:

@nodejs/gyp

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

nodejs-github-bot added lib / src

Issues and PRs involving general changes in the lib/ or src/ directories.

needs-ci PRs that need a full CI run. labels

Jul 31, 2024

aduh95 added 2 commits

July 31, 2024 12:52


          fixup! build: add --without-amaro build flag

2d34ab3


          fixup! build: add --without-amaro build flag

c178ac4

marco-ippolito commented Jul 31, 2024

Copy link

Copy Markdown

Member

should the flag be added to the vcbuild.bat too?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

codecov Bot commented Jul 31, 2024 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.07%. Comparing base (b4fd1fd) to head (c178ac4).
Report is 529 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #54136   +/-   ##
=======================================
  Coverage   87.06%   87.07%           
=======================================
  Files         643      643           
  Lines      181576   181576           
  Branches    34894    34891    -3     
=======================================
+ Hits       158088   158106   +18     
+ Misses      16759    16753    -6     
+ Partials     6729     6717   -12

Files with missing lines	Coverage Δ
src/node_metadata.cc	`96.42% <ø> (ø)`

... and 31 files with indirect coverage changes

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

marco-ippolito added the strip-types Issues and PRs related to TypeScript type stripping. label

Jul 31, 2024

aduh95 commented Jul 31, 2024

Copy link

Copy Markdown

Contributor Author

should the flag be added to the vcbuild.bat too?

Not my area of expertise, and no way for me to test it, but I'd say no based on the fact that HAVE_OPENSSL and node_use_openssl are not defined in vcbuild.bat

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

aduh95 added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Jul 31, 2024

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label

Jul 31, 2024

nodejs-github-bot commented Jul 31, 2024

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

anonrig approved these changes

Jul 31, 2024

View reviewed changes

marco-ippolito approved these changes

Jul 31, 2024

View reviewed changes

marco-ippolito left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

RafaelGSS approved these changes

Jul 31, 2024

View reviewed changes

nodejs-github-bot commented Aug 1, 2024

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

nodejs-github-bot commented Aug 1, 2024

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

nodejs-github-bot commented Aug 2, 2024

Copy link

Copy Markdown

Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

aduh95 added author ready

PRs with CI started, the required approvals, and no outstanding review comments.

commit-queue PRs queued for automated landing through the Commit Queue. labels

Aug 2, 2024

nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label

Aug 2, 2024

nodejs-github-bot merged commit 1e3a347 into nodejs:main

Aug 2, 2024

nodejs-github-bot commented Aug 2, 2024

Copy link

Copy Markdown

Collaborator

Landed in 1e3a347

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

aduh95 deleted the without-amaro-build-flag branch

August 2, 2024 12:04

targos pushed a commit that referenced this pull request

Aug 14, 2024


          build: add --without-amaro build flag

98fed76

PR-URL: #54136
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Rafael Gonzaga <[email protected]>

RafaelGSS mentioned this pull request

Aug 19, 2024

v22.7.0 proposal #54452

Merged

targos added the dont-land-on-v20.x label

Sep 21, 2024

zcbenz mentioned this pull request

Nov 9, 2024

build: implement node_use_amaro flag in GN build #55798

Merged

nodejs-github-bot pushed a commit that referenced this pull request

Nov 11, 2024


          build: implement node_use_amaro flag in GN build

57f23b4

PR-URL: #55798
Refs: #54136
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>

aduh95 pushed a commit that referenced this pull request

Nov 16, 2024


          build: implement node_use_amaro flag in GN build

472d55e

PR-URL: #55798
Refs: #54136
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>

ruyadorno pushed a commit that referenced this pull request

Nov 27, 2024


          build: implement node_use_amaro flag in GN build

f285e79

PR-URL: #55798
Refs: #54136
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>

ruyadorno pushed a commit that referenced this pull request

Nov 27, 2024


          build: implement node_use_amaro flag in GN build

b4e4139

PR-URL: #55798
Refs: #54136
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>

marco-ippolito pushed a commit that referenced this pull request

Jan 22, 2025


          build: implement node_use_amaro flag in GN build

8ba8b20

PR-URL: #55798
Refs: #54136
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>

marco-ippolito pushed a commit that referenced this pull request

Jan 24, 2025


          build: implement node_use_amaro flag in GN build

9f14ba8

PR-URL: #55798
Refs: #54136
Reviewed-By: Marco Ippolito <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>

Reviewers

anonrig

anonrig approved these changes

RafaelGSS

RafaelGSS approved these changes

marco-ippolito

marco-ippolito approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready

PRs with CI started, the required approvals, and no outstanding review comments.

lib / src

Issues and PRs involving general changes in the lib/ or src/ directories.

needs-ci

PRs that need a full CI run.

strip-types

Issues and PRs related to TypeScript type stripping.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

aduh95 commented Jul 31, 2024

Uh oh!

nodejs-github-bot commented Jul 31, 2024

Uh oh!

marco-ippolito commented Jul 31, 2024

Uh oh!

codecov Bot commented Jul 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

aduh95 commented Jul 31, 2024

Uh oh!

nodejs-github-bot commented Jul 31, 2024

Uh oh!

marco-ippolito left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Aug 1, 2024

Uh oh!

nodejs-github-bot commented Aug 1, 2024

Uh oh!

nodejs-github-bot commented Aug 2, 2024

Uh oh!

nodejs-github-bot commented Aug 2, 2024

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov Bot commented Jul 31, 2024 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.