-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add to CI builds for mac, linux, windows #1
Comments
I'm a bit confused. Why do you want this to be build for 3 different platforms if this is essentially a Node.js package? |
This is an Electron app (like VSCode) so the build actually generates native binaries |
Well, first of all I'm getting installation failures on linux when running
Apparently you can get that on Ubuntu from But I'd rather use a Nix shell. |
yeah libpcsclite-dev is needed on Linux, same as Status Desktop. How was it solved there? |
I tried using
It's just the libraries. |
Ah, but the dev package does have it:
|
Managed to fix that, but now I'm getting:
|
What Node.JS version is recommended for building this? I see Electron 21 was using NodeJS 16: |
I tried using But it doesn't seem to work. |
BTW, why is this |
@choppu is the developer of the SDK and this app (this repo is a fork) |
Hi, I will check and let you know |
I can see there is
|
But this file references it as upper case: import { SCP02Channel } from "./SCP02-channel";
import { SCP02Keys } from "./scp02-keys"; Which seems like a mistake to me. |
I found the issue, I'm fixing it now |
Fixed. By the way, I use v18.16.1 of nodejs |
Thanks for the fix @choppu, now |
great! |
Now I can run
Which is obviously because I'm on NixOS, but why the hell does this intall |
How the fuck is this a thing? https://www.npmjs.com/package/7zip-bin |
This appears to be coming from And it appears to be a source of other issues: Why would you ever install binary tools via NPM... |
latest electron-builder seems to not use 7zip on linux. I will try to update to the latest version and will let you know soon. |
That would be cool, but if it's too much of a pain in the ass please tell me and I'll work around it. |
Absolutely no problem. I have updated electron-builder to the latest stable version. I don't have linux, so can't check if this removes all dependencies from 7zip |
Thank you, I will test this after I finish my meetings. |
Initial build on Jenkins CI:
|
Here are some actual build errors:
I'm not getting this locally. |
This appears to be a known issue: |
And now it fails at signing:
Do we need to sign this? |
Apparently you can disable signing:
|
Okay, now what is this now:
Why does it want a GitHub token? |
It seems like it's trying to publish the app to GitHub:
But I don't want it to do that. |
Apparently you can set But where? |
I'm checking why it is trying to publish. |
Apparently you set it in |
And there we have it: https://ci.infra.status.im/job/keycard-certify/job/platforms/job/macos/job/x86_64/18/ Now, the Windows part will be tricky. |
Nice to hear that it works now. In any case, I have added the publish never option. |
I found out that |
I added it as a command line flag because in theory the "publish" section should be an array of publishing providers whereas the flag says when you want to publish (never, always, on tag, etc). Of course setting |
The "side effect" of disabling publishing is exactly what we want. |
Ok, updated as requested. |
In a perfect world I'd use Windows with WSL2 and then we could also use Nix on there, but I don't have that currently working. @guylouis how important is the Windows platform? Because that's going to be the one that requires the most work. |
by the way, since I am on Windows I can provide a windows binary if/when needed if it is not important to have automatic builds for it |
Another major question is: how important is signing for this? Is this for internal use only or will this be published to external users? Because signing setup is usually more tricky than just getting the build to work. |
Ok to not have automatic build for windows! |
Signing is nice to have but not mandatory since the app is for internal use |
I will be working on Nix-based builds for Windows desktop app soon, and while doing that I will look into using WSL2 + Nix. If that turns out to work well then we will also get Windows builds for this for free. |
Looking good: https://ci.infra.status.im/job/keycard-certify/job/manual/5/ But we're missing the
I wonder if we can fix this. |
Apparently it's conttrolled by the
Which in turn uses
Which we already have set: |
Well, I just changed I'm assuming SNAP is using |
Okay, this looks pretty good: https://ci.infra.status.im/job/keycard-certify/job/manual/10/ Next we test the GitHub release. |
I tried the mac build above and got the following error message. Seems related to the build with some dependancies on nix. cc @jakubgs A javascript error happened in the main process
|
Please use triple backticks for pasting logs in markdown. And it's not a dependency on Nix, it's a dependency on |
seems similar to https://github.com/orgs/Homebrew/discussions/3038#discussion-3911704 it might be that nix links to the runtime of the LLVM compiler it is managing and not the system one. The difference being, apparently, that the system lib is called |
Yeah, I was reading that. Why do Macs always have to be different. |
Actually, I've tested the build and it appears to work on Windows too: https://ci.infra.status.im/job/keycard-certify/job/platforms/job/windows/job/x86_64/2/ Nice. |
Here's the PR: If need be we can add signing later. |
Could the infra team setup CI for this project so that it is built for mac, linux, windows?
The command to build is
npm dist
.Releases builds are enough, no need for PR builds
cc @bitgamma
The text was updated successfully, but these errors were encountered: