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

Windows CI builds #4590

Merged
merged 1 commit into from
Nov 7, 2022
Merged

Windows CI builds #4590

merged 1 commit into from
Nov 7, 2022

Conversation

newhoggy
Copy link
Contributor

@newhoggy newhoggy commented Nov 1, 2022

Resolves #4614

@newhoggy newhoggy force-pushed the newhoggy/fix-windows-builds branch 18 times, most recently from 316e01c to 60ffc02 Compare November 4, 2022 12:23
@newhoggy newhoggy changed the title Fix Windows CI builds by installing openssl library Windows CI builds Nov 4, 2022
@newhoggy newhoggy marked this pull request as ready for review November 4, 2022 14:02
@newhoggy newhoggy requested review from Jimbo4350 and a team as code owners November 4, 2022 14:02
Copy link
Contributor

@deepfire deepfire left a comment

Choose a reason for hiding this comment

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

Thank you a lot @newhoggy !

@deepfire
Copy link
Contributor

deepfire commented Nov 4, 2022

bors r+

@michaelpj
Copy link
Contributor

Can I suggest that we leave a comment explaining why this is doing what it's doing? Otherwise we will lose the knowledge and it will just make our pipelines even more obscure and hard to modify!

@deepfire
Copy link
Contributor

deepfire commented Nov 4, 2022

bors r-

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Nov 4, 2022

Canceled.

@@ -178,7 +179,7 @@ jobs:
sed -i 's|tests: False|tests: True|g' cabal.project
fi

cp .github/workflows/cabal.project.local.ci cabal.project.local
cp ".github/workflows/cabal.project.local.ci.$(uname -s)" cabal.project.local
Copy link
Contributor Author

@newhoggy newhoggy Nov 4, 2022

Choose a reason for hiding this comment

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

For Windows only, we need to pass extra flags so that gcc and ld (which ghc invokes) can find the libraries and header files.

This is necessary because unfortunately the 3rdparty msys2 system we use installs packages into D:/a/_temp/msys64/mingw64 instead of the system C:\msys64\mingw64. This applies also the secp256k1.

It's unclear why gcc and ld invoked by ghc is unable to see this alternate installation directory.

To make it easy so treat Windows differently, we have OS specific cabal.project.local.ci.* files which are copied to cabal.project.local.

The additional flags to make this work on Windows can then be put into cabal.project.local.ci.MINGW64_NT-10.0-20348

Please see the linked issue for more information.


package cardano-crypto-praos
extra-include-dirs: D:/a/_temp/msys64/mingw64/include
extra-lib-dirs: D:/a/_temp/msys64/mingw64/lib
Copy link
Contributor Author

@newhoggy newhoggy Nov 4, 2022

Choose a reason for hiding this comment

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

Any Haskell packages that fails to build because the system installation directory is missing will need an entry here. This ought to only happen with packages that have C bindings to system libraries that are only installed in the alternate package installation directory.

@@ -57,6 +57,7 @@ jobs:
mingw-w64-x86_64-toolchain
mingw-w64-x86_64-libsodium
mingw-w64-x86_64-jq
mingw-w64-x86_64-openssl
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's unclear if I need this, but I added because HsOpenSSL was the first package that was failing to build..

Once this is merged I will try to remove the openssl and see if it still builds.

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems it worked fine without installing openssl in ouroboros-network.

@michaelpj
Copy link
Contributor

@coot seems to be addressing the same problems here, perhaps worth comparing notes? IntersectMBO/ouroboros-network#4134

@coot
Copy link
Contributor

coot commented Nov 4, 2022

@newhoggy pointed me to his PR, and I copied the solution.

@newhoggy
Copy link
Contributor Author

newhoggy commented Nov 7, 2022

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Nov 7, 2022

Build succeeded:

@iohk-bors iohk-bors bot merged commit 621615b into master Nov 7, 2022
@iohk-bors iohk-bors bot deleted the newhoggy/fix-windows-builds branch November 7, 2022 08:41
iohk-bors bot added a commit that referenced this pull request Feb 4, 2023
4612: Cardano Node 1.35.6 aka Single Relay P2P release r=coot a=coot

Update `ouroboros-network-0.2.0.0`.

This also cherry-picked CI commits:
* e745ca8 from #4184
* #4513
* #4526
* #4590

Co-authored-by: Michael Peyton Jones <[email protected]>
Co-authored-by: Marcin Szamotulski <[email protected]>
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.

[BUG] - Github Actions CI for Windows is broken
4 participants