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.

Sign up for GitHub

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

Jump to bottom

Windows Github Actions now uses MSYS2 #3901

Merged
merged 1 commit into from
Jul 19, 2022
Merged

Windows Github Actions now uses MSYS2 #3901

merged 1 commit into from
Jul 19, 2022

Conversation

jasagredo
Copy link
Contributor

@jasagredo jasagredo commented Jul 15, 2022 •
edited
Loading

Description

Windows Github actions CI makes use of chocolatey and some hydra artifacts to run. This PR unifies the approach for Windows by using MSYS2.

Done in the same style as this PR in cardano-node

Checklist

  • Branch
    • Commit sequence broadly makes sense
    • Commits have useful messages
    • New tests are added if needed and existing tests are updated
    • If this branch changes Consensus and has any consequences for downstream repositories or end users, said changes must be documented in interface-CHANGELOG.md
    • If this branch changes Network and has any consequences for downstream repositories or end users, said changes must be documented in interface-CHANGELOG.md
    • If serialization changes, user-facing consequences (e.g. replay from genesis) are confirmed to be intentional.
  • Pull Request
    • Self-reviewed the diff
    • Useful pull request description at least containing the following information:
      • What does this PR change?
      • Why these changes were needed?
      • How does this affect downstream repositories and/or end-users?
      • Which ticket does this PR close (if any)? If it does, is it linked?
    • Reviewer requested

@jasagredo jasagredo self-assigned this Jul 15, 2022
@jasagredo jasagredo force-pushed the jasagredo/fix-windows-ci-MSYS2 branch 2 times, most recently from d3174c8 to e4c66cb Compare July 15, 2022 09:17
@jasagredo jasagredo changed the title Windows Github Actions now usews MSYS2 Windows Github Actions now uses MSYS2 Jul 15, 2022
@jasagredo jasagredo marked this pull request as ready for review July 15, 2022 10:40
@jasagredo jasagredo requested review from coot, newhoggy and a team as code owners July 15, 2022 10:40
@jasagredo jasagredo requested review from amesgen, dnadales and nfrisby and removed request for a team and newhoggy July 15, 2022 10:40
amesgen
amesgen reviewed Jul 15, 2022
View reviewed changes
Copy link
Member

@amesgen amesgen left a comment

Choose a reason for hiding this comment

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

Looks wonderful!

Only other thing: Do you have evidence that caching is working? In e4c66cb (latest commit at time of writing), all dependencies are still built from scratch.

.github/workflows/build.yml Show resolved Hide resolved
@jasagredo jasagredo force-pushed the jasagredo/fix-windows-ci-MSYS2 branch from e4c66cb to 132f3e7 Compare July 15, 2022 14:53
amesgen
amesgen approved these changes Jul 18, 2022
View reviewed changes
Copy link
Member

@amesgen amesgen left a comment

Choose a reason for hiding this comment

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

Hmm, unfortunate that runner.os doesn't work in the shell: expression 🤔

Either way, LGTM!

coot
coot approved these changes Jul 18, 2022
View reviewed changes
Copy link
Contributor

@coot coot left a comment

Choose a reason for hiding this comment

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

LGTM

@coot
Copy link
Contributor

coot commented Jul 18, 2022

@jasagredo thanks for taking care of it. Btw, what triggered moving away of chocolatey? It was a bit flaky at times, or was it something else?

@jasagredo
Copy link
Contributor Author

jasagredo commented Jul 18, 2022 •
edited
Loading

@jasagredo thanks for taking care of it. Btw, what triggered moving away of chocolatey? It was a bit flaky at times, or was it something else?

chocolatey was somewhat flaky, Also installing libsodium manually (sed replacing things) and also with libsecp256k1 felt more complex than doing the normal thing of make install. I think this new flow is much more uniform now

cc @coot (I'm not sure if quote replying in github notifies the author)

@jasagredo jasagredo force-pushed the jasagredo/fix-windows-ci-MSYS2 branch 2 times, most recently from 4073904 to 4eb7719 Compare July 18, 2022 08:34
@amesgen amesgen mentioned this pull request Jul 18, 2022
Merged
9 tasks
@jasagredo jasagredo force-pushed the jasagredo/fix-windows-ci-MSYS2 branch 6 times, most recently from 99ca11a to adc93fb Compare July 18, 2022 14:04
@jasagredo jasagredo force-pushed the jasagredo/fix-windows-ci-MSYS2 branch 2 times, most recently from 18ccfd9 to 9df6a5a Compare July 18, 2022 14:22
@jasagredo
Copy link
Contributor Author

bors r+

iohk-bors bot added a commit that referenced this pull request Jul 18, 2022
@iohk-bors @jasagredo
Merge #3901
5f3d810 
3901: Windows Github Actions now uses MSYS2 r=Jasagredo a=Jasagredo

# Description

Windows Github actions CI makes use of chocolatey and some hydra artifacts to run. This PR unifies the approach for Windows [by using MSYS2](https://github.com/msys2/setup-msys2).

Done in the same style as [this PR in `cardano-node`](IntersectMBO/cardano-node#4184)



Co-authored-by: Javier Sagredo <jasataco@gmail.com>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 18, 2022

Timed out.

@jasagredo
Copy link
Contributor Author

bors retry

iohk-bors bot added a commit that referenced this pull request Jul 18, 2022
@iohk-bors @jasagredo
Merge #3901
b280a50 
3901: Windows Github Actions now uses MSYS2 r=Jasagredo a=Jasagredo

# Description

Windows Github actions CI makes use of chocolatey and some hydra artifacts to run. This PR unifies the approach for Windows [by using MSYS2](https://github.com/msys2/setup-msys2).

Done in the same style as [this PR in `cardano-node`](IntersectMBO/cardano-node#4184)



Co-authored-by: Javier Sagredo <jasataco@gmail.com>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 19, 2022

Timed out.

@jasagredo jasagredo force-pushed the jasagredo/fix-windows-ci-MSYS2 branch from e08246f to 29aa6bb Compare July 19, 2022 08:58
@jasagredo jasagredo force-pushed the jasagredo/fix-windows-ci-MSYS2 branch from 29aa6bb to 7f07c40 Compare July 19, 2022 09:21
@jasagredo jasagredo merged commit 3305447 into master Jul 19, 2022
@iohk-bors iohk-bors bot deleted the jasagredo/fix-windows-ci-MSYS2 branch July 19, 2022 09:42
@amesgen amesgen mentioned this pull request Jul 19, 2022
Merged
9 tasks
iohk-bors bot added a commit that referenced this pull request Jul 19, 2022
@iohk-bors @amesgen
Merge #3911
76408fb 
3911: Fix GH Actions Windows CI caching r=amesgen a=amesgen

# Description

In #3901, the store path used for caching was `C:\cabal\config\store` instead of the correct `C:\cabal\store`. This was due a mix up of the path separators (`/` vs `\`).

Also, we bump the cache version as we want to override the previous (empty) cache.

For evidence that this works, see [this run](https://github.com/input-output-hk/ouroboros-network/runs/7417675263?check_suite_focus=true), in particular how the `Build dependencies` and the `Build projects [build]` finish almost instantly.



Co-authored-by: Alexander Esgen <alexander.esgen@iohk.io>
iohk-bors bot added a commit that referenced this pull request Jul 20, 2022
@iohk-bors @amesgen
Merge #3911
2445682 
3911: Fix GH Actions Windows CI caching r=amesgen a=amesgen

# Description

In #3901, the store path used for caching was `C:\cabal\config\store` instead of the correct `C:\cabal\store`. This was due a mix up of the path separators (`/` vs `\`).

Also, we bump the cache version as we want to override the previous (empty) cache.

For evidence that this works, see [this run](https://github.com/input-output-hk/ouroboros-network/runs/7417675263?check_suite_focus=true), in particular how the `Build dependencies` and the `Build projects [build]` finish almost instantly.



Co-authored-by: Alexander Esgen <alexander.esgen@iohk.io>
nfrisby pushed a commit that referenced this pull request Jul 21, 2022
@jasagredo @nfrisby
Merge pull request #3901 from input-output-hk/jasagredo/fix-windows-c…
c0fff78 
…i-MSYS2

Windows Github Actions now uses MSYS2
nfrisby pushed a commit that referenced this pull request Jul 21, 2022
@iohk-bors @amesgen @nfrisby
Merge #3911
e51cbdd 
3911: Fix GH Actions Windows CI caching r=amesgen a=amesgen

# Description

In #3901, the store path used for caching was `C:\cabal\config\store` instead of the correct `C:\cabal\store`. This was due a mix up of the path separators (`/` vs `\`).

Also, we bump the cache version as we want to override the previous (empty) cache.

For evidence that this works, see [this run](https://github.com/input-output-hk/ouroboros-network/runs/7417675263?check_suite_focus=true), in particular how the `Build dependencies` and the `Build projects [build]` finish almost instantly.



Co-authored-by: Alexander Esgen <alexander.esgen@iohk.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amesgen amesgen amesgen approved these changes

@coot coot coot approved these changes

@dnadales dnadales Awaiting requested review from dnadales

@nfrisby nfrisby Awaiting requested review from nfrisby

Assignees

@jasagredo jasagredo

Labels
Windows
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jasagredo @coot @amesgen