-
Notifications
You must be signed in to change notification settings - Fork 113
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
Rewrite build system for new versions and Heroku stacks #14
Conversation
This is the first part of some work I've been doing today. Started as a review of what was here and... kind of spiraled. I'm also looking at making changes to The goal here was to make it easy to create builds for new versions of jemalloc. The next step that I'm working on is being able to use those builds and easily switch between them. From a high level this codebase is two things: heroku buildpack for downloading compiled binaries and tooling to build those binaries. It should be possible to quickly and easily make binaries available based on version without needing to update the buildpack. I'm thinking we can use a set of releases (one per stack) that hosts the binaries for each version of jemalloc (I have a proof of concept on my fork). For switching between versions, I've been able to have a |
In |
4461dd3
to
71ac02c
Compare
This allows new versions to be built for uploading by running `make VERSION=5.0.1` rather than updating the hard coded version in multiple places. I've also added support for building against either the legacy cedar-14 architecture or heroku-16. While versions built on cedar-14 appear to run on heroku-16, the newer stack should also mean a newer and better tool chain. I've removed the Dockerfile and switched to using Heroku's docker images directly as this removes the need for cleaning them up afterwards. It possibly makes debugging harder but having build.sh available should avoid some of that. Fixes #2 Fixes #7 Fixes #13
71ac02c
to
81173ed
Compare
👍 on the environment variables to control the build and if jemalloc should be used. That makes testing very easy too. Using |
I've fleshed out what I was thinking and it's sitting on master of my fork. We're slowly testing it on a couple apps and already have it running on Dead Man's Snitch. Going to clean up some of the fork specific changes and get another PR up for consideration. |
@gaffneyc I was trying out your folk, and I noticed the builds for jemalloc 3.6.0 do not include a |
@csuhta Interesting, I will take a look. I'm not modifying the sources at all so I'm guessing it's not part of the 3.x line. I'll see about getting a work around in place. Going to take some time today to upstream my changes as well. |
@gaffneyc I created two bundles with working config scripts for 3.6.0: https://img.scryfall.com/build/jemalloc/cedar-14/jemalloc-3.6.0.tar.bz2 |
Was there a config change you needed to make for it to be generated or did you add the file directly? |
I manually created them |
Gotcha. It looks like jemalloc-config was added in 4.0.0. Is it worth support any versions older than 4.0.0 other than 3.6.0? |
It may be possible to hardcode it as /app/vendor/jemalloc/lib/libjemalloc.so as it's a symlink to the specific revision. Since slugs are compiled independently on each code change, there should only ever be one version installed in /app/vendor/jemalloc |
@csuhta I've pushed gaffneyc@6564f30 which should fix the issue with 3.6.0. I was able to remove the need for jemalloc-config entirely. |
|
||
.PHONY: cedar-14 heroku-16 | ||
|
||
# Build for cedar stack (deprecated) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cedar-14 isn't deprecated, can just remove this comment
Thank you so much for this.
I like this, but can leave for another day/PR. |
Maybe also for a different PR, but to use any of this properly, we'll have to change the way this line works in bin/compile, correct? |
I've been planning to create a PR for what's on the master branch of my fork but have been pressed for time this last week. Thinking that PR would supersede this one and probably #11. |
Yes, it will end up looking something like this: https://github.com/gaffneyc/heroku-buildpack-jemalloc/blob/6306663a60b49aadc2f2907141028f07029d59e7/bin/compile#L18 with releases configured like this: https://github.com/gaffneyc/heroku-buildpack-jemalloc/releases |
Took a couple minutes and submitted #18 which supersedes this one. |
This allows new versions to be built for uploading by running
make VERSION=5.0.1
rather than updating the hard coded version inmultiple places. I've also added support for building against either the
legacy cedar-14 architecture or heroku-16. While versions built on
cedar-14 appear to run on heroku-16, the newer stack should also mean a
newer and better tool chain.
I've removed the Dockerfile and switched to using Heroku's docker images
directly as this removes the need for cleaning them up afterwards. It
possibly makes debugging harder but having build.sh available should
avoid some of that.
Fixes #2
Fixes #7
Fixes #13