add buildpack ordering rfc #22
Conversation
Co-authored-by: Arjun Sreedharan <[email protected]>
| - paketo-community/ruby | ||
| - paketo-buildpacks/httpd | ||
| - paketo-buildpacks/nginx | ||
| - paketo-buildpacks/procfile |
There was a problem hiding this comment.
Procfile should be last, otherwise staticfile apps can't have a Procfile and detect
There was a problem hiding this comment.
I think it would only matter in the case we bundle staticfile and procfile into one order group. Right now statcifile doesn't know anything about Procfiles so people shouldn't include them in their app.
There was a problem hiding this comment.
Would we change the order later if we added Procfile support to the Staticfile group?
There was a problem hiding this comment.
Yeah that would be the idea, as it would fail the validator we dreamed up at that point. I do generally agree with you that we might want to just keep the old ordering though.
| - If any of buildpack B's order groups are a subset of any of buildpack A's order groups, then buildpack A must come before buildpack B in the builder order. | ||
| - An implementation buildpack can be defined as an order group of itself. | ||
|
|
||
| Thus, our exiting buildpacks would be ordered as follows: |
There was a problem hiding this comment.
Would be great to understand our motivation from moving away from the CF ordering: https://github.com/cloudfoundry/cf-deployment/blob/41a60509ffd73662e6fac059ff39ef86b913e48e/cf-deployment.yml#L874-L896
Many of those are ordered carefully to support common app configurations. E.g., node is below Ruby and .NET Core, because Ruby and .NET Core apps often have package.json.
There was a problem hiding this comment.
Right, we were trying to come up with a more universal rule that could be validated, without a ton of knowledge of the individual languages, and I didn't know enough about the old ordering to compare its benefits.
That being said those are cases we didn't think about and it might be too hard or impossible to make a language agnostic rue in the way we did.
There was a problem hiding this comment.
I'm skeptical that this can be solved with a mechanical lexical ordering. I believe that we're currently in a situation where the detection criteria are completely disjoint and precedence doesn't matter, but I don't believe that that is a stable assumption to make for the long term.
|
|
||
| - paketo-buildpacks/dotnet-core | ||
| - paketo-buildpacks/go | ||
| - paketo-buildpacks/java |
There was a problem hiding this comment.
I think Java triggers on any class files being present. Might want to push this further down? @nebhale
There was a problem hiding this comment.
Not just any class files the way it was in CF. It's much more selective these days (hallelujah for modularity and the build plan) and is now existence of any of the following:
<APPLICATION_ROOT>/build.gradle<APPLICATION_ROOT>/pom.xml<APPLICATION_ROOT>/build.sbt<APPLICATION_ROOT>/META-INF/MANIFEST.MF<APPLICATION_ROOT>/WEB-INF/<APPLICATION_ROOT>/<APPLICATION_NAME>/bin/<APPLICATION_NAME>
So it can probably go nearly anywhere right now, but I'm not sure that'd be true forever. Lexically ordering these doesn't solve the fundamental issue of overlapping detection criteria and precedence.
|
@paketo-buildpacks/content Please review |
|
|
||
| We propose the following rule to order buildpacks in the builder's `Detect Order`: | ||
|
|
||
| - All meta buildpacks should be come before implementation buildpacks, and sort each group by buildpaco-id. |
There was a problem hiding this comment.
| - All meta buildpacks should be come before implementation buildpacks, and sort each group by buildpaco-id. | |
| - All meta buildpacks should be come before implementation buildpacks, and sort each group by buildpack-id. |
|
|
||
| - paketo-buildpacks/dotnet-core | ||
| - paketo-buildpacks/go | ||
| - paketo-buildpacks/java |
There was a problem hiding this comment.
Not just any class files the way it was in CF. It's much more selective these days (hallelujah for modularity and the build plan) and is now existence of any of the following:
<APPLICATION_ROOT>/build.gradle<APPLICATION_ROOT>/pom.xml<APPLICATION_ROOT>/build.sbt<APPLICATION_ROOT>/META-INF/MANIFEST.MF<APPLICATION_ROOT>/WEB-INF/<APPLICATION_ROOT>/<APPLICATION_NAME>/bin/<APPLICATION_NAME>
So it can probably go nearly anywhere right now, but I'm not sure that'd be true forever. Lexically ordering these doesn't solve the fundamental issue of overlapping detection criteria and precedence.
| - If any of buildpack B's order groups are a subset of any of buildpack A's order groups, then buildpack A must come before buildpack B in the builder order. | ||
| - An implementation buildpack can be defined as an order group of itself. | ||
|
|
||
| Thus, our exiting buildpacks would be ordered as follows: |
There was a problem hiding this comment.
I'm skeptical that this can be solved with a mechanical lexical ordering. I believe that we're currently in a situation where the detection criteria are completely disjoint and precedence doesn't matter, but I don't believe that that is a stable assumption to make for the long term.
|
After discussion we decided the following ordering will be less likely to break apps that rely on the current order: We don't think that there is a hard and fast rule that we can follow to make sure the ordering is correct, and any new buildpack will need careful consideration. |
This reverts commit f18af4b.
This is an rfc with a proposed method of ordering buildpacks.
Co-authored-by: Arjun Sreedharan [email protected]