Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

Add support for Heroku-22 #239

Closed
wants to merge 1 commit into from
Closed

Add support for Heroku-22 #239

wants to merge 1 commit into from

Conversation

edmorley
Copy link
Member

In order to support Heroku-22 we need to generate new binaries, so it makes sense to build the latest versions for it and the other stacks rather than something outdated.

The binary build process for this buildpack uses the default nginx version specified by ngx_mruby. As such, updating ngx_mruby from 2.2.3 to 2.2.4 means the bundled nginx version is also updated from 1.19.0 to 1.21.3:
https://github.com/matsumotory/ngx_mruby/blob/v2.2.3/nginx_version
https://github.com/matsumotory/ngx_mruby/blob/v2.2.4/nginx_version

Changes:
https://github.com/matsumotory/ngx_mruby/releases/tag/v2.2.4
https://nginx.org/en/CHANGES

GUS-W-10346704.

@edmorley edmorley self-assigned this May 18, 2022
@edmorley

This comment was marked as outdated.

@edmorley

This comment was marked as outdated.

@edmorley edmorley marked this pull request as ready for review May 18, 2022 12:38
@edmorley edmorley requested review from a team and hone May 18, 2022 12:38
In order to support Heroku-22 we need to generate new binaries, so
it makes sense to build the latest versions for it and the other stacks
rather than something outdated.

The binary build process for this buildpack uses the default `nginx`
version specified by `ngx_mruby`. As such, updating `ngx_mruby`
from `2.2.3` to `2.2.4` means the bundled `nginx` version is also
updated from `1.19.0` to `1.21.3`:
https://github.com/matsumotory/ngx_mruby/blob/v2.2.3/nginx_version
https://github.com/matsumotory/ngx_mruby/blob/v2.2.4/nginx_version

Changes:
https://github.com/matsumotory/ngx_mruby/releases/tag/v2.2.4
https://nginx.org/en/CHANGES

GUS-W-10346704.
@edmorley edmorley force-pushed the edmorley/heroku-22 branch from a4f0801 to 79f2d69 Compare May 19, 2022 06:31
@edmorley
Copy link
Member Author

So I spotted there was a typo in the Makefile which meant the wrong Docker image was used to perform the Heroku-22 build. After fixing that and rebuilding, I'm running into:

...
(cd /tmp/ngx_mruby.42Hc/mruby && rake -j 4 MRUBY_CONFIG=/tmp/ngx_mruby.42Hc/build_config.rb NGX_MRUBY_CFLAGS="" NGX_MRUBY_LDFLAGS="")
/bin/sh: 1: rake: not found

Which is due to the removal of system Ruby from Heroku-22. The whole buildpack relies upon Ruby (not just at build time), so this is going to require a bit of a different approach.

That said, part of me wonders whether it's just time to deprecate this buildpack in favour of https://github.com/heroku/heroku-buildpack-nginx ?

@edmorley edmorley removed request for a team and hone May 19, 2022 06:48
@edmorley edmorley marked this pull request as draft May 19, 2022 06:48
@edmorley
Copy link
Member Author

I've split the version bump parts of this out to #240, and will close this out for now in favour of opening a new PR for Heroku-22 once the Ruby aspect figured out.

@edmorley edmorley closed this May 19, 2022
@edmorley edmorley deleted the edmorley/heroku-22 branch May 19, 2022 06:55
@edmorley
Copy link
Member Author

edmorley commented May 19, 2022

/bin/sh: 1: rake: not found
Which is due to the removal of system Ruby from Heroku-22. The whole buildpack relies upon Ruby (not just at build time), so this is going to require a bit of a different approach.

It seems our options are either:

  1. Have this buildpack vendor Ruby (eg from the archives used by the Ruby buildpack) on Heroku-22+
  2. If Ruby not found during the build, error out and say users need to manually add the Ruby buildpack prior to this one
  3. Not support Heroku-22, and instead deprecate this buildpack in favour of the nginx buildpack
  4. Add Ruby back to the stack image for Heroku-22

@hone, thoughts? :-)

@edmorley
Copy link
Member Author

cc @heroku/languages (decision needed here ref how to handle system Ruby no longer being in the Heroku-22 stack image - see above for possible options)

@edmorley edmorley changed the title Bump ngx_mruby/nginx versions and add support for Heroku-22 Add support for Heroku-22 May 20, 2022
@joshwlewis
Copy link
Member

I'd be in favor of option 2. But I'd suggest the error messaging giving users two options: a) add the ruby buildpack, or b) switch to the nginx buildpack. For some folks, switching to the nginx buildpack might be a better choice.

@edmorley
Copy link
Member Author

@dzuelke @schneems Any thoughts? :-)

@edmorley
Copy link
Member Author

edmorley commented Jun 2, 2022

@hone (as original buildpack creator) + anyone else: As a heads up, our current thinking is to go with option (3):

Not support Heroku-22, and instead deprecate this buildpack in favour of the nginx buildpack

The plan is to have a migration guide in the README of this buildpack, that can be linked to from a Heroku-22 specific failure message during bin/compile, plus also from the Heroku-22 migration help on Dev Center. On older stacks we should also emit a warning that this buildpack is deprecated.

The migration guide in the README will suggest running a one-off dyno to trigger the script that generates the nginx config, then cat it, so that the resultant generated nginx config can be placed into the app's source ready for use by the nginx buildpack. That doesn't cover the dynamic request-rewriting parts of this buildpack, but it will should cover many of the more simple use-cases. We may also want to add some more examples of nginx configs to the nginx buildpack repo itself:
https://github.com/heroku/heroku-buildpack-nginx/tree/main/config

Lastly, we'll need to notify https://github.com/mars/create-react-app-buildpack that they'll need to migrate to the nginx buildpack too. (Edit: Filed mars/create-react-app-buildpack#200)

@edmorley
Copy link
Member Author

edmorley commented Jun 7, 2022

#243 is now open for deprecating this buildpack

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants