-
Notifications
You must be signed in to change notification settings - Fork 88
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
fix(secrets): Add prebuildCheck.js
script to check if prebuilds exist when building
#1788
Conversation
Signed-off-by: Trae Yelovich <[email protected]>
Signed-off-by: Trae Yelovich <[email protected]>
Signed-off-by: Trae Yelovich <[email protected]>
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## master #1788 +/- ##
=======================================
Coverage 92.82% 92.82%
=======================================
Files 423 423
Lines 7191 7191
Branches 1319 1319
=======================================
Hits 6675 6675
Misses 515 515
Partials 1 1 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Trae Yelovich <[email protected]>
preinstall.js
script to check if prebuilds exist when buildingprebuildCheck.js
script to check if prebuilds exist when building
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.
Should we update the changelog for the patch release of Secrets SDK?
I suppose the changelog check is ignoring the secrets package because of private: true
in its package.json 😋
@@ -39,7 +40,7 @@ | |||
"build": "npm run build:ts && cd src/keyring && napi build --config napi.json --js false --platform --release", | |||
"build:debug": "cd src/keyring && napi build --config napi.json --js false --platform", | |||
"build:ts": "tsc --pretty", | |||
"install": "node src/keyring/index.js || npm run rebuild", | |||
"install": "node scripts/prebuildCheck.js || npm run rebuild", | |||
"prepack": "node ../../scripts/prepareLicenses.js", | |||
"prepublishOnly": "bash scripts/prebuildify.sh", | |||
"rebuild": "npx --yes --package=@napi-rs/[email protected] -- napi build --config src/keyring/napi.json --cargo-cwd src/keyring --platform --release --js=false src/keyring", |
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.
The "rebuild" script seems to fail with newer versions of npm: sh: 1: napi: not found
This may be caused by an npm bug so not sure if we can do anything to fix it: npm/cli#6435
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! 😋
Signed-off-by: Trae Yelovich <[email protected]>
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
What It Does
This PR adds a
preinstall.js
script to the Secrets SDK install process so that it will only fail if the prebuilds are not found during installation, triggering a rebuild.Fixes #1787 - if the user's architecture/platform is not supported, Cargo is still required to build the Secrets SDK.
How to Test
First run the
prepublishOnly
script for the Secrets SDK so that it downloads the prebuilds locally. Then, try installing Zowe CLI with this branch in a Linux environment withoutlibsecret
installed. The CLI should still install without throwing any errors with the Secret SDK during installation.Review Checklist
I certify that I have: