Skip to content

feat: enable macos build#22

Draft
tony-go wants to merge 15 commits into
mainfrom
enable-macos
Draft

feat: enable macos build#22
tony-go wants to merge 15 commits into
mainfrom
enable-macos

Conversation

@tony-go

@tony-go tony-go commented May 23, 2023

Copy link
Copy Markdown
Collaborator

No description provided.

@tony-go tony-go self-assigned this May 23, 2023
@tony-go tony-go force-pushed the enable-macos branch 3 times, most recently from 0a62b8a to c6cbc94 Compare May 23, 2023 10:48
@tony-go

tony-go commented May 23, 2023

Copy link
Copy Markdown
Collaborator Author

Current status:

  • It works locally 🎉
  • I face a path issue with openssl in the CI (same as Macos build #12)

@tony-go tony-go force-pushed the enable-macos branch 2 times, most recently from 8d43b0c to e55a183 Compare May 23, 2023 13:20
@tony-go tony-go force-pushed the enable-macos branch 2 times, most recently from 9f55e84 to 32ac9a6 Compare May 23, 2023 13:58
@tony-go

tony-go commented May 23, 2023

Copy link
Copy Markdown
Collaborator Author

Update: build passes on CI but the first test crashes. I have to investigate.

Maybe a first hint:

ld: warning: dylib (/usr/local/opt/openssl@3/lib/libcrypto.dylib) was built for newer macOS version (12.0) than being linked (10.13)

@G-Ray

G-Ray commented Nov 11, 2023

Copy link
Copy Markdown
Owner

@tony-go I made CI build passes on branch enable-macos-1.
Could you please merge it to your branch, rebase to main, and see how it goes ?

@tony-go

tony-go commented Nov 13, 2023

Copy link
Copy Markdown
Collaborator Author

lease merge it to your bran

Don't follow you up on this. Why should we use that branch if you are a build that passes on another branch?

If you used this branch as a base, please open a PR basis on this one.

@G-Ray

G-Ray commented Nov 15, 2023

Copy link
Copy Markdown
Owner

lease merge it to your bran

Don't follow you up on this. Why should we use that branch if you are a build that passes on another branch?

If you used this branch as a base, please open a PR basis on this one.

Ok you are right, let's review #33
🤗

@G-Ray

G-Ray commented Nov 20, 2023

Copy link
Copy Markdown
Owner

Now that #33 is merged, could you please rebase this branch on main, and see if Windows CI passes ?

@tony-go

tony-go commented Dec 4, 2023

Copy link
Copy Markdown
Collaborator Author

hey @G-Ray

With your new clone function, normally, I just have to:

  • npm i
  • npm test
  • npm run rebuild
    ??

The "install": "node-gyp-build "node scripts/build-transmission.js""` not happen on my side.

@tony-go

tony-go commented Dec 4, 2023

Copy link
Copy Markdown
Collaborator Author

That is the patch I add to do to make it work:

index 97c752e..fc8ad63 100644
--- a/package.json
+++ b/package.json
@@ -6,7 +6,7 @@
   "license": "GPL-3.0",
   "main": "index.js",
   "scripts": {
-    "install": "node-gyp-build \"node scripts/build-transmission.js\"",
+    "postInstall": "node-gyp-build \"node scripts/build-transmission.js\"",
     "test": "standard && node --test test.js",
     "prebuild": "prebuildify --napi --strip",
     "prebuild-arm64": "prebuildify --napi --strip --tag-armv"

Should I raise a PR?

@G-Ray

G-Ray commented Dec 5, 2023

Copy link
Copy Markdown
Owner

That is the patch I add to do to make it work:

index 97c752e..fc8ad63 100644
--- a/package.json
+++ b/package.json
@@ -6,7 +6,7 @@
   "license": "GPL-3.0",
   "main": "index.js",
   "scripts": {
-    "install": "node-gyp-build \"node scripts/build-transmission.js\"",
+    "postInstall": "node-gyp-build \"node scripts/build-transmission.js\"",
     "test": "standard && node --test test.js",
     "prebuild": "prebuildify --napi --strip",
     "prebuild-arm64": "prebuildify --napi --strip --tag-armv"

Should I raise a PR?

So as we discussed, one just need to be sure deleting thebuild/ or prebuilds folder(s), if it exist 🤗

npm install --build-from-source might work too

@G-Ray G-Ray force-pushed the main branch 2 times, most recently from a0cc8c8 to a589f52 Compare July 28, 2024 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants