Skip to content

Limit ucontext linking to arm32* and s390x arches#366

Merged
tianon merged 1 commit intodocker-library:masterfrom
infosiftr:more-context
Oct 28, 2021
Merged

Limit ucontext linking to arm32* and s390x arches#366
tianon merged 1 commit intodocker-library:masterfrom
infosiftr:more-context

Conversation

@yosifkit
Copy link
Member

And keep libucontext-dev on those arches to ensure rubygems work. (~249kb)

Fixes #365

@keithlayne
Copy link

This looks good, but this seems like it's pretty easy to test with just gem install <something small with native extensions>. I confess I found your test infra in the oi repo a little confusing. Should this be easy to write?

@tianon
Copy link
Member

tianon commented Oct 27, 2021

@keithlayne yes, adding that would be really easy, and I was about to go do exactly that, but got a little stuck on which native extension we should use (it'd be awesome if there was a "hello world" of native extensions we could use that we know won't ever be pre-compiled, but I couldn't find anything -- maybe you know better 👀)

@keithlayne
Copy link

I'm mostly out of the ruby world, and I never wrote a single native extension, so no. But immediately after I wrote that I was wondering basically the same thing. I thought of several ways it could be goofy. Maybe it's just a matter of generating the makefile or whatever and trying to run it for a no-op extension? I wonder if that could be bundled up into a very few files...

@tianon
Copy link
Member

tianon commented Oct 27, 2021

Looks like the magic incantation is --platform ruby to force it to build from source. I'll cook up a basic integration test for this real fast. 😅

(The gem I ran into this problem on was eventmachine, which seems like a pretty lightweight one to use -- if someone reading this later knows a better lightweight "hello-world" native extension example, please @ me! 😄)

@bpo
Copy link

bpo commented Oct 27, 2021

@tianon I think you'll find bcrypt lighter/simpler than eventmachine for a quick test like this.

@tianon
Copy link
Member

tianon commented Oct 27, 2021

Indeed; just tested and it builds way faster!! Thanks @bpo! ❤️ 👀

Edit: it also appears to be easier to use/debug because it isn't hiding the compiler errors in a log file like eventmachine does 👀

Edit 2x: and it doesn't need g++!! 🤘 🥳

@keithlayne
Copy link

@tianon because I was already down this path...if you run this on ruby:2.7.4-alpine3.1.4 on amd64, it fails as expected.

Possibly the bundle command would be fiddly and want to be interactive on version changes, but this is very much the "hello world":

bundle gem --ext  --no-coc --no-mit --test=none  test
cd test
sed -i''  '/none/d' Gemfile
sed -i'' -e 's/spec\.summary.*$/spec.summary = "bogus"/' \
  -e 's/spec\.authors.*$/spec.authors = ["bogus"]/'  \
  -e '/TODO/d' -e '/metadata/d' \
  test.gemspec
apk add make gcc musl-dev git
bundle
rake build

@keithlayne
Copy link

Downside of the above is that a) you already have your test 😁 and b) it doesn't cover jruby.

@tianon
Copy link
Member

tianon commented Oct 28, 2021

Yeah, bcrypt does just fine (and feels less fiddly and less likely to be broken by future changes to Bundler -- at most maybe we'll have to update the version at some point).

I've got docker-library/official-images#11192 merged now with that new test and restarted the builds here so it'll run it and validate that @yosifkit's fix is solid. 👍

@tianon tianon merged commit 74ff4b2 into docker-library:master Oct 28, 2021
@tianon tianon deleted the more-context branch October 28, 2021 00:24
docker-library-bot added a commit to docker-library-bot/official-images that referenced this pull request Oct 28, 2021
Changes:

- docker-library/ruby@74ff4b2: Merge pull request docker-library/ruby#366 from infosiftr/more-context
- docker-library/ruby@bda17e3: Limit ucontext linking to specific arches
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.

New build failure on 2.7.4-alpine-amd64 (maybe revisit #364?)

4 participants