search

crystal-lang / distribution-scripts

40 stars 24 forks source link

Build libgc in alpine image to enable MT builds #63

Closed Blacksmoke16 closed 3 years ago

Blacksmoke16 commented 4 years ago

~One question I have left is what to do about LIBRARY_PATH? Given I think both paths need to be used?~

~Added built libgc path to LIBRARY_PATH in build image.~

LIBRARY_PATH is no longer required apparently due to #69.

Blacksmoke16 commented 4 years ago

Bump.

Blacksmoke16 commented 4 years ago

Would be great if this made it in time for https://github.com/crystal-lang/crystal/pull/9317 in order to enable MT builds with 0.35.0.

\cc @bcardiff @RX14 @waj

Blacksmoke16 commented 4 years ago

@RX14 @bcardiff @straight-shoota This should be good for another review.

straight-shoota commented 4 years ago

I suppose the build tools should be removed from the docker image after building libgc?

tducasse commented 3 years ago

Hey just FYI, I was looking for an "official" alpine crystal image and stumbled onto this PR.

I was trying to build a static binary for Crystalline to solve this issue. It seems like yaml-dev on Alpine doesn't give the right static libs, so I had to add yaml-static as well.

Not sure if it is useful, but since this is not merged yet, I thought I'd share 😄

caspiano commented 3 years ago

Bump :)

straight-shoota commented 3 years ago

This needs conflict resolution.

I'm also a bit confused now about the Ubuntu dockerfile changes. Description only talks about alpine. And a patched library should be shipped with the debian package, so rebuilding seems unnecessary here.

caspiano commented 3 years ago

@Blacksmoke16, I made a PR to your repo that I believe resolves the comment above by @straight-shoota

Blacksmoke16/distribution-scripts/pull/1

Blacksmoke16 commented 3 years ago

@caspiano Thanks! Merged and fixed up two things I noticed. Should be good for another review now.

Disclaimer: I didn't test it locally yet.

caspiano commented 3 years ago

This would be awesome to have with 1.1.0 😉 What's currently in the way of a merge? I cannot wait to run my crystal services with MT 🤤

straight-shoota commented 3 years ago

I cannot wait to run my crystal services with MT 🤤

Well, you can already do that with older releases, just need to provide the patched library.

caspiano commented 3 years ago

I normally run alpine images, I'd either have to build an image with the patch (like this) or switch to the ubuntu images. This is a great change, thanks @Blacksmoke16!