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

preliminary ci.yml #9

Merged
merged 36 commits into from
May 25, 2022
Merged

preliminary ci.yml #9

merged 36 commits into from
May 25, 2022

Conversation

eightfilms
Copy link
Contributor

@eightfilms eightfilms commented May 23, 2022

Things/checks to add:

  • clippy
  • rustfmt
  • tests

@mitchmindtree mitchmindtree mentioned this pull request May 23, 2022
3 tasks
@eightfilms
Copy link
Contributor Author

eightfilms commented May 23, 2022

Added clippy and fmt for now as checks but these will fail until #8 is merged (no Cargo.toml yet).

@eightfilms eightfilms mentioned this pull request May 23, 2022
@Voxelot
Copy link
Member

Voxelot commented May 23, 2022

Maybe target this PR into the other one rather than the master branch, so that we know the initial PR to master will pass CI.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@eightfilms eightfilms changed the base branch from master to binggh/mvp May 23, 2022 23:27
@eightfilms eightfilms marked this pull request as ready for review May 24, 2022 02:49
Base automatically changed from binggh/mvp to master May 25, 2022 00:43
@eightfilms
Copy link
Contributor Author

eightfilms commented May 25, 2022

Hmm, now testing this on the test repo and the cross images are running into build errors which seem to stem from OpenSSL. Seems to be a long-standing issue and point of discussion over at cross: cross-rs/cross#229

Workaround seems to be to use a custom image and install the necessary packages(?) Trying this now.

cc: @Voxelot, if you have suggestions from doing this in fuel-core

@Voxelot
Copy link
Member

Voxelot commented May 25, 2022

Ah interesting, it may be worth investigating a way to eliminate the dependency on OpenSSL given most libraries support webpki/rustls these days. I did have to do custom docker images for fuel-core but it was painful.

@eightfilms
Copy link
Contributor Author

Ah, okay 👍 Seems possible to build with rustls instead so I think I'll work towards that, thanks @Voxelot!

@eightfilms
Copy link
Contributor Author

Ah, okay 👍 Seems possible to build with rustls instead so I think I'll work towards that, thanks @Voxelot!

Turns out curl-rust allows disabling openssl features and enabling rustls instead (link):

curl = { version = "0.4" default-features = false, features = ["rustls"] }

Builds seem to be working fine in the test repo.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@mitchmindtree mitchmindtree left a comment

Choose a reason for hiding this comment

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

Lgtm!

Copy link
Contributor

@mitchmindtree mitchmindtree left a comment

Choose a reason for hiding this comment

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

Oh, pending @Voxelot's fix - good spot

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@Voxelot
Copy link
Member

Voxelot commented May 25, 2022

should be g2g, but let's see a test release fully work before merging

@eightfilms
Copy link
Contributor Author

Is there supposed to be some way to verify the developer(s) of the releases? Couldn't run the x86_64 mac binary myself at first, then tried a bit later and it worked somehow

@Voxelot
Copy link
Member

Voxelot commented May 25, 2022

We'll need to get an apple developer certificate enabled in CI to resolve that. For now we'll need to find some kind of workaround to help users requests the permissions they need to run the binary on mac.

cc @adlerjohn https://docs.github.com/en/actions/deployment/deploying-xcode-applications/installing-an-apple-certificate-on-macos-runners-for-xcode-development

@adlerjohn
Copy link
Contributor

If it's not needed for this PR, can you file an issue and assign it to me to follow-up on adding the cert?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

4 participants