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

agd enforce Node.js version #9623

Merged
merged 2 commits into from
Jul 1, 2024
Merged

Conversation

mhofman
Copy link
Member

@mhofman mhofman commented Jul 1, 2024

refs: #9622

Description

Enforce the Node.js version in agd builder. Reverts #9619

This enforces LTS range, aka ^18.12 or ^20.9 through a regexp similar to the golang version check.

Security Considerations

None

Scaling Considerations

None

Documentation Considerations

We'll need to update release notes for validators to use agd build instead of the former yarn install and yarn build as that may still fail. If they still chose to go that route, they're on their own.

Testing Considerations

CI covers agd build

Upgrade Considerations

Smooth the transition away from previously supported Node v16 in upgrade-15

@mhofman mhofman requested a review from michaelfig July 1, 2024 01:57
repoconfig.sh Outdated Show resolved Hide resolved
scripts/agd-builder.sh Show resolved Hide resolved
scripts/agd-builder.sh Outdated Show resolved Hide resolved
Copy link

cloudflare-workers-and-pages bot commented Jul 1, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: 5f01bef
Status: ✅  Deploy successful!
Preview URL: https://afe57c7e.agoric-sdk.pages.dev
Branch Preview URL: https://mhofman-9622-node-version-ch.agoric-sdk.pages.dev

View logs

@mhofman mhofman added the force:integration Force integration tests to run on PR label Jul 1, 2024
@mhofman mhofman force-pushed the mhofman/9622-node-version-check branch from 3876ed0 to 5f01bef Compare July 1, 2024 08:12
@mhofman mhofman added the automerge:rebase Automatically rebase updates, then merge label Jul 1, 2024
@mergify mergify bot merged commit 4f70e66 into master Jul 1, 2024
77 of 82 checks passed
@mergify mergify bot deleted the mhofman/9622-node-version-check branch July 1, 2024 08:27
mhofman pushed a commit that referenced this pull request Jul 1, 2024
refs: #9622

## Description
Enforce the Node.js version in agd builder. Reverts #9619

This enforces LTS range, aka `^18.12` or `^20.9` through a regexp similar to the golang version check.

### Security Considerations
None

### Scaling Considerations
None

### Documentation Considerations
We'll need to update release notes for validators to use `agd build` instead of the former `yarn install` and `yarn build` as that may still fail. If they still chose to go that route, they're on their own.

### Testing Considerations
CI covers `agd build`

### Upgrade Considerations
Smooth the transition away from previously supported Node v16 in upgrade-15
mergify bot added a commit that referenced this pull request Jul 1, 2024
refs: #9622

## Description
#9623 claimed to enforce Node.js versions ^18.12 or ^20.9, but due to a typo allows arbitrarily high versions to pass the check. This corrects it to enforce that the major version is exactly 18 or 20.

### Security Considerations
None.

### Scaling Considerations
n/a

### Documentation Considerations
Matches the [README](https://github.com/Agoric/agoric-sdk?tab=readme-ov-file#prerequisites).

### Testing Considerations
Checked by integration testing in CI.

### Upgrade Considerations
Will need an update for supporting Node.js v22 when that enters LTS (ref #9265).
gibson042 pushed a commit that referenced this pull request Jul 1, 2024
refs: #9622

## Description
#9623 claimed to enforce Node.js versions ^18.12 or ^20.9, but due to a typo allows arbitrarily high versions to pass the check. This corrects it to enforce that the major version is exactly 18 or 20.

### Security Considerations
None.

### Scaling Considerations
n/a

### Documentation Considerations
Matches the [README](https://github.com/Agoric/agoric-sdk?tab=readme-ov-file#prerequisites).

### Testing Considerations
Checked by integration testing in CI.

### Upgrade Considerations
Will need an update for supporting Node.js v22 when that enters LTS (ref #9265).
gibson042 added a commit that referenced this pull request Jul 1, 2024
Rebase todo

```
# Branch fix-vow-include-vat-js-in-package-files-9607-
label base-fix-vow-include-vat-js-in-package-files-9607-
pick b6ffa6f fix(vow): include vat.js in package files
label fix-vow-include-vat-js-in-package-files-9607-
reset base-fix-vow-include-vat-js-in-package-files-9607-
merge -C a3826e9 fix-vow-include-vat-js-in-package-files-9607- # fix(vow): include vat.js in package files (#9607)

# Pull Request #9601
pick 6bc363b fix(cosmos): only allow snapshot export at latest height (#9601)

# Branch Force-xsnap-rebuild-9618-
label base-Force-xsnap-rebuild-9618-
pick 467435a fix(xsnap): force rebuild if build config changes
pick a22772e fix(agd): check xsnap was rebuilt
pick 2b53896 feat(xsnap): force rebuild if binary version mismatch
label Force-xsnap-rebuild-9618-
reset base-Force-xsnap-rebuild-9618-
merge -C 78b6fec Force-xsnap-rebuild-9618- # Force xsnap rebuild (#9618)

# Branch agd-enforce-Node-js-version-9623-
label base-agd-enforce-Node-js-version-9623-
#pick 4b35caf revert fix: typescript-estree does not support Node.js LTS (#9619)
pick 5f01bef fix(agd): force own node.js version check
label agd-enforce-Node-js-version-9623-
reset base-agd-enforce-Node-js-version-9623-
merge -C 4f70e66 agd-enforce-Node-js-version-9623- # agd enforce Node.js version (#9623)

# Branch gibson-9623-followup
label base-gibson-9623-followup
pick 6102eb9 fix: Disallow Node.js major version >20
label gibson-9623-followup
reset base-gibson-9623-followup
merge -C caaec05 gibson-9623-followup # (upstream/master) fix: Disallow Node.js major version >20 (#9630)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge force:integration Force integration tests to run on PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants