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

Deprecate beta CNB support #1445

Merged
merged 2 commits into from
Apr 22, 2024
Merged

Deprecate beta CNB support #1445

merged 2 commits into from
Apr 22, 2024

Conversation

schneems
Copy link
Contributor

CNB support was introduced into this buildpack as an experiment. The experiment is over, anyone wanting to use the official CNB (still in "preview" support) should use https://github.com/heroku/buildpacks-ruby

CNB support was introduced into this buildpack as an experiment. The experiment is over, anyone wanting to use the official CNB (still in "preview" support) should use https://github.com/heroku/buildpacks-ruby
@schneems schneems force-pushed the schneems/deprecate-cnb branch from 20fb84f to f8027e9 Compare April 22, 2024 16:43
@schneems schneems marked this pull request as ready for review April 22, 2024 16:50
@schneems schneems requested a review from a team as a code owner April 22, 2024 16:50
Copy link
Member

@edmorley edmorley left a comment

Choose a reason for hiding this comment

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

I think you could remove bin/build entirely (and the CNB subsets of buildpack.toml) now, given that the CNB in this repo implements Buildpack API 0.2, which has not been supported by our builders since last year:


https://github.com/heroku/cnb-builder-images#available-images
https://github.com/buildpacks/lifecycle#supported-apis

Copy link
Member

@joshwlewis joshwlewis left a comment

Choose a reason for hiding this comment

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

Looks good to me. Left some optional suggestions about making this sound less like we're abandoning CNB altogether and more like the functionality has moved.

CHANGELOG.md Outdated Show resolved Hide resolved
bin/build Outdated Show resolved Hide resolved
@schneems
Copy link
Contributor Author

@edmorley I'm up for removing all this code, but the Ruby integrations run deep and I'm trying to get codon unblocked on heroku-24, however I've got no idea how people might be using this TBH, just that they might be. I know waypoint use(d?) heroku CNBs before we implemented ours in rust, but I'm not sure what mechanism they were using.

I saw this code while working on adding shellcheck so I can fix ruby bootstrapping on heroku-24 (the vendored version logic is incorrect on that stack, but all the logic lives in bash).

In Ruby-land I like to deprecate, release, and then remove where possible. I was thinking of doing that here.

@schneems schneems merged commit 397b8f8 into main Apr 22, 2024
3 checks passed
@schneems schneems deleted the schneems/deprecate-cnb branch April 22, 2024 17:29
@edmorley
Copy link
Member

edmorley commented Apr 22, 2024

@edmorley I'm up for removing all this code, but the Ruby integrations run deep and I'm trying to get codon unblocked on heroku-24, however I've got no idea how people might be using this TBH, just that they might be. I know waypoint use(d?) heroku CNBs before we implemented ours in rust, but I'm not sure what mechanism they were using.

Every single third party I've found (over several hours of searching, as part of heroku/cnb-builder-images#429 (comment)) uses our buildpacks from the CNB registry or via cnb-shim (which uses the classic buildpack from the classic buildpack registry's S3 bucket).

There is also no CNB publishing pipeline in this repo any more, so no one will execute the deprecation warning.

However, there is no harm in doing it this way if you prefer - I just wanted to make you aware that in my opinion it was already safe to delete, should you wish to do so :-)

@edmorley
Copy link
Member

I know waypoint use(d?) heroku CNBs before we implemented ours in rust, but I'm not sure what mechanism they were using.

They use the builder image:
https://github.com/search?q=repo%3Ahashicorp%2Fwaypoint+%22heroku%2Fbuildpacks%3A20%22&type=code

xref:
hashicorp/waypoint#4911

@heroku-linguist heroku-linguist bot mentioned this pull request Apr 23, 2024
schneems pushed a commit that referenced this pull request Jun 13, 2024
The buildpack in this repo is primarily a classic Heroku buildpack,
however, as part of the initial exploration into CNBs had experimental
CNB support added some time ago.

However, the maintained Ruby CNB now exists in a separate repo:
https://github.com/heroku/buildpacks-ruby

The experimental CNB support in this repo doesn't actually work any more
since the buildpack API version it implements (v0.2) isn't supported by
modern `lifecycle` versions - and attempts at building encounter this
error:

```
ERROR: failed to set API for Buildpack 'heroku/[email protected]': buildpack API version '0.2' is incompatible with the lifecycle
```

In addition, it's not even possible to use this repo with a CNB build
without having cloned it locally, since:
1. The CNB parts are no longer published
2. The old CNB release assets were deleted a year or so ago:
   https://github.com/heroku/heroku-buildpack-ruby/releases
3. Pack/lifecycle doesn't support cloning Git URLs.
4. Attempting to use a buildpack URL pointing at the GitHub gzip archive
   fails due to GitHub's nesting of the repo inside a subdirectory:

```
$ pack build --builder heroku/builder:22 --buildpack https://github.com/heroku/heroku-buildpack-ruby/archive/refs/heads/main.tar.gz ruby-test
...
Downloading from https://github.com/heroku/heroku-buildpack-ruby/archive/refs/heads/main.tar.gz
93.2 KB/-1 B
ERROR: failed to build: downloading buildpack: extracting from https://github.com/heroku/heroku-buildpack-ruby/archive/refs/heads/main.tar.gz: reading buildpack: reading buildpack.toml: could not find entry path 'buildpack.toml': not exist
```

Given both the Buildpack API error, and the GitHub URL issues no one is
using the CNB implementation here, and so its dead code that should be
removed to prevent confusion (eg over where the CNB lives).

A deprecation warning was previously added in #1445 (though as above
it's unlikely anyone even saw that message, since the build was already
erroring before it gets that far).
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.

3 participants