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

chore(NODE-6603): set errexit in install script and download prebuilt windows zstd #51

Merged
merged 9 commits into from
Dec 4, 2024

Conversation

baileympearson
Copy link
Contributor

@baileympearson baileympearson commented Dec 4, 2024

Description

What is changing?

Set errexit in install-zstd.sh.

This causes the install script to fail on windows because the tarball contains symlinks, which cannot be extracted. The symlink files are not source files, so the build worked without issues before this, but now that errexit is set, we cannot download and extract the zstd source code.

Instead, on windows we now download the prebuilt static library (which does not contain symlinks).

Is there new documentation needed for these changes?

What is the motivation for this change?

Double check the following

  • Ran npm run format:js && npm run format:rs script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@baileympearson baileympearson changed the title wip chore(NODE-6603): set errexit in install script and download prebuilt windows zstd Dec 4, 2024
@baileympearson baileympearson marked this pull request as ready for review December 4, 2024 20:53
download_zstd() {
mkdir -p deps/zstd
ZSTD_VERSION=$(node -p "require('./package.json')['mongodb:zstd_version']")
is_windows=$(node -p "process.platform === 'win32'")
Copy link
Contributor

Choose a reason for hiding this comment

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

clever!

@nbbeeken nbbeeken self-assigned this Dec 4, 2024
@nbbeeken nbbeeken added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Dec 4, 2024
@nbbeeken nbbeeken merged commit 0063c4e into main Dec 4, 2024
37 checks passed
@nbbeeken nbbeeken deleted the NODE-6603 branch December 4, 2024 20:59
baileympearson added a commit that referenced this pull request Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Primary Review In Review with primary reviewer, not yet ready for team's eyes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants