This repository has been archived by the owner on Nov 1, 2023. It is now read-only.
Buildpack compile fixes and refactoring #177
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Example build on this branch using Heroku-18: And with the not-yet-supported Heroku-20 we now get a nicer build time error and not a boot failure at runtime: |
Since with exit on error, if that line is ever reached, the exit code will always be zero anyway.
In order to prevent the build completing apparently successfully, but the app fail to boot at runtime due to stack incompatibility. At first glance this would seem unnecessary due to the stack-specific URL meaning the `curl` would 404 on supported stacks. However #165 means the Cedar-14 binary is installed on all stacks, and on Heroku-20 causes the failures at runtime seen in #166. Future PRs will fix the curl/binary handling to use stack-specific URLs, however it's still nicer to explicitly handle unsupported stacks with a clear error message than a 404.
The caching of the nginx archive isn't used in production (nothing ever writes to the cached file) or in CI. Whilst it may speed up some local development workflows slightly, on a fast connection downloading from S3 takes less than a second, so isn't worth the added `bin/compile` complexity / confusion as to behaviour in production.
- The `s3-external-1` endpoint is a legacy reference to `us-east-1`: https://stackoverflow.com/a/26622229 - The path based bucket specification is deprecated: https://aws.amazon.com/blogs/aws/amazon-s3-path-deprecation-plan-the-rest-of-the-story/
Previously the nginx version command was failing since the `nginx-$STACK` binary does not exist, resulting in output like: ``` remote: -----> Installed directory to /app/bin ``` This failure went unnoticed since `pipefail` mode is not enabled. The nginx binary path has been fixed, and the command now uses `-v` instead of `-V` since the former only output one line, avoiding the need to `head -n1`. In addition, the `cut` usage shows more of the original line in the case of no match. Fixes #174.
Enables the following bash modes: - `u`: error on undefined variables - `pipefail`: error if an earlier command in a pipe sequence exits non-zero, rather than only if the final command is non-zero See: http://redsymbol.net/articles/unofficial-bash-strict-mode/
To improve the reliability of the build. See: https://curl.haxx.se/docs/manpage.html#--retry https://curl.haxx.se/docs/manpage.html#--connect-timeout
edmorley
force-pushed
the
edmorley-cleanup
branch
from
29e8f94
to
0dfb606
Compare
dzuelke
approved these changes
Nov 9, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In order to reduce the size of the Heroku-20 support PR, I've split out some of the compile fixes/cleanup.
The most notable of these are:
See the individual commit messages (and changelog entries) for more details.
Refs W-8367040.