Skip to content
This repository has been archived by the owner on Jan 7, 2023. It is now read-only.

sync: update CI config files #17

Merged
merged 12 commits into from
Feb 1, 2022
Merged

sync: update CI config files #17

merged 12 commits into from
Feb 1, 2022

Conversation

web3-bot
Copy link
Collaborator

@web3-bot web3-bot commented Aug 27, 2021

Syncing to commit protocol/.github@68389df.

@marten-seemann
Copy link

marten-seemann commented Aug 27, 2021

I hadn't seen that @Stebalien had already rebased this.

@Stebalien
Copy link
Member

Ah, and the 32bit tests are failing because we don't have CGO.

@Stebalien
Copy link
Member

And the windows tests are failing because we don't have openssl.

@marten-seemann
Copy link

marten-seemann commented Aug 27, 2021

And the windows tests are failing because we don't have openssl.

This is the second time in a single day that the feature request came up to disable Windows tests (the other one is: filecoin-shipyard/js-lotus-client-schema#16 (comment)).

@Stebalien
Copy link
Member

Well... I'd rather install openssl, if we can.

@marten-seemann
Copy link

Ah, I thought that's not possible. We could use a custom setup step for that.

@Stebalien
Copy link
Member

Yep. Working on it.

@Stebalien Stebalien force-pushed the web3-bot/sync branch 4 times, most recently from 81aab19 to d9a5a89 Compare August 27, 2021 23:06
@Stebalien
Copy link
Member

Ok, so the pc (pkg-config) file still isn't getting installed. I think we need to move to msys2 for windows (https://github.com/msys2/setup-msys2). Really, we can probably switch to this by default to make our lives easier.

@marten-seemann
Copy link

Do we need to do this for all shells that run on Windows, or is it sufficient to do that in the repo-specific setup step?

@Stebalien
Copy link
Member

Do we need to do this for all shells that run on Windows, or is it sufficient to do that in the repo-specific setup step?

It should be possible to do this in the setup only but:

  1. It's a bit annoying because if conditionals don't work inside "composite" actions. So we'd need to use the "default" bash, check if we're running on windows, then launch a msys bash shell script.
  2. I'm not sure what environment variables we need to setup if we can't just run msys2 everywhere. I think it's just PATH and then something for pkg-config? But I'm not sure.

@web3-bot web3-bot force-pushed the web3-bot/sync branch 2 times, most recently from 6e29ff8 to 16193d5 Compare October 17, 2021 15:41
@BigLep BigLep requested a review from a team October 24, 2021 04:37
@aschmahmann
Copy link

Update: GH Actions now support if in composites which should make our lives easier here.

@marten-seemann
Copy link

We have a PR that changes the shell to msys2 on Windows: protocol/.github#221. It would be really valuable to try it out in this repo. @galargh, is that a task you could take? To do so, you'd create a new branch which manually updates the workflow files to the PR linked above. We could then see if the tests actually pass. That would allow us to merge protocol/.github#221 with confidence that it actually solves the problem.

@galargh
Copy link

galargh commented Nov 22, 2021

@galargh, is that a task you could take?

Sure! I will check it out and report back here and under protocol/.github#221

@galargh galargh mentioned this pull request Nov 22, 2021
@galargh
Copy link

galargh commented Feb 1, 2022

I made sure we're set up correctly for CGO in #20. Note: if there is a unified CI deploy before we merge https://github.com/protocol/.github/tree/next into master then it will try to overwrite the changes made here to go-test.yml. That PR will fail however because it won't pass the tests. So it can create a bit of noise but it will auto-resolve once the next big release of unified CI is out. Just wanted to point it out.

@galargh galargh merged commit 0fadeb4 into master Feb 1, 2022
@galargh galargh deleted the web3-bot/sync branch February 1, 2022 08:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants