Skip to content

Commit ae8bb3d

Browse files
authored
Cleanup and refactoring (heroku#1600)
* Delete unused file This code is unused * Refactor Lockfile Error Previously, the existence check for a `Gemfile.lock` was enforced by creating a "language pack" that is checked and executed only if the Gemfile.lock file does not exist. The location for this logic doesn't need to be tied to a lanugage pack. * Dont emit warning (from grep) when Gemfile.lock does not exist Fixes this output: ``` remote: -----> Building on the Heroku-24 stack remote: -----> Using buildpack: https://github.com/heroku/heroku-buildpack-ruby#schneems/every-bundler-version remote: -----> Ruby app detected remote: grep: /tmp/build_cdf9bd69/Gemfile.lock: No such file or directory remote: remote: ! remote: ! Gemfile.lock required. Please check it in. remote: ! remote: ! Push rejected, failed to compile Ruby app. ``` * Remove false documentation Neither detect or release use Ruby for logic: ``` #!/bin/sh APP_DIR=$1 if [ -f "$APP_DIR/Gemfile" ]; then echo "Ruby" exit 0 else exit 1 fi $ cat bin/release #!/usr/bin/env bash set -euo pipefail BUILD_DIR="${1:-}" RELEASE_FILE="${BUILD_DIR}/tmp/heroku-buildpack-release-step.yml" # Whilst the release file is always written by the buildpack, some apps use # third-party slug cleaner buildpacks to remove this and other files, so we # cannot assume it still exists by the time the release step runs. if [[ -f "${RELEASE_FILE}" ]]; then cat "${RELEASE_FILE}" fi ``` The cache path is a required argument * Rename `build_path` to `app_path` From the perspective of the buildpack the build happens in the application path. The word "build" is overloaded and can be confusing. The application directory has more specificity. * Remove stray comment * Remove unused env var * Remove whitespace * Remove unused variable * Prefer kwargs * Standardize on Pathname as the language pack input * Extract Dir.chdir(app_path) The expectation is that the language pack is called in the application directory. We can do this before it is invoked, remove extra calls sprinkled throughout. * Update HEREDOC to ideomatic format The `<<~` heredoc is indentation aware and was not available when a lot of these `<<-` heredocs were added. * Whitespace
1 parent 5ec402b commit ae8bb3d

16 files changed

Lines changed: 190 additions & 225 deletions

bin/support/bash_functions.sh

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ detect_needs_java()
7474
if which_java; then
7575
return $skip_java_install
7676
fi
77-
grep "(jruby " "$gemfile_lock" --quiet
77+
78+
grep "(jruby " "$gemfile_lock" --quiet &> /dev/null
7879
}
7980

8081
# Runs another buildpack against the build dir

bin/support/ruby_compile

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,12 @@ HerokuBuildReport.set_global(
1919
).tap(&:clear!)
2020

2121
begin
22+
app_path = Pathname(ARGV[0])
23+
cache_path = Pathname(ARGV[1])
24+
Dir.chdir(app_path)
25+
2226
LanguagePack::ShellHelpers.initialize_env(ARGV[2])
23-
if pack = LanguagePack.detect(ARGV[0], ARGV[1])
27+
if pack = LanguagePack.detect(app_path: app_path, cache_path: cache_path)
2428
pack.topic("Compiling #{pack.name}")
2529
pack.compile
2630
end

bin/support/ruby_test-compile

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,11 @@ require "language_pack/test"
1919
include LanguagePack::ShellHelpers
2020

2121
begin
22-
if pack = LanguagePack.detect(ARGV[0], ARGV[1])
22+
app_path = Pathname(ARGV[0])
23+
cache_path = Pathname(ARGV[1])
24+
Dir.chdir(app_path)
25+
26+
if pack = LanguagePack.detect(app_path: app_path, cache_path: cache_path)
2327
LanguagePack::ShellHelpers.initialize_env(ARGV[2])
2428
pack.topic("Setting up Test for #{pack.name}")
2529
pack.compile

lib/heroku_build_report.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ def capture(metrics = {})
3636
return if key.nil? || key.to_s.strip.empty?
3737

3838
key = key&.strip
39-
raise "Key cannot be empty (#{key.inspect} => #{value})" if key.nil? || key.empty?
39+
raise "Key cannot be empty (#{key.inspect} => #{value})" if key.nil? || key.empty?
4040

4141
# Don't serialize complex values by accident
4242
if complex_object?(value)

lib/language_pack.rb

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,24 @@
11
require "pathname"
22
require 'benchmark'
33

4+
require 'language_pack/shell_helpers'
5+
46
# General Language Pack module
57
module LanguagePack
68
module Helpers
79
end
810

911
# detects which language pack to use
10-
# @param [Array] first argument is a String of the build directory
11-
# @return [LanguagePack] the {LanguagePack} detected
12-
def self.detect(*args)
13-
Dir.chdir(args.first)
12+
def self.detect(app_path:, cache_path:)
13+
if !File.exist?("Gemfile.lock")
14+
raise BuildpackError.new("Gemfile.lock required. Please check it in.")
15+
end
1416

15-
pack = [ NoLockfile, Rails8, Rails7, Rails6, Rails5, Rails42, Rails41, Rails4, Rails3, Rails2, Rack, Ruby ].detect do |klass|
17+
pack = [ Rails8, Rails7, Rails6, Rails5, Rails42, Rails41, Rails4, Rails3, Rails2, Rack, Ruby ].detect do |klass|
1618
klass.use?
1719
end
1820

19-
return pack ? pack.new(*args) : nil
21+
return pack ? pack.new(app_path: app_path, cache_path: cache_path) : nil
2022
end
2123
end
2224

@@ -25,7 +27,6 @@ def self.detect(*args)
2527

2628
require 'heroku_build_report'
2729

28-
require 'language_pack/shell_helpers'
2930
require "language_pack/helpers/plugin_installer"
3031
require "language_pack/helpers/stale_file_cleaner"
3132
require "language_pack/helpers/rake_runner"
@@ -39,12 +40,10 @@ def self.detect(*args)
3940
require "language_pack/rack"
4041
require "language_pack/rails2"
4142
require "language_pack/rails3"
42-
require "language_pack/disable_deploys"
4343
require "language_pack/rails4"
4444
require "language_pack/rails41"
4545
require "language_pack/rails42"
4646
require "language_pack/rails5"
4747
require "language_pack/rails6"
4848
require "language_pack/rails7"
4949
require "language_pack/rails8"
50-
require "language_pack/no_lockfile"

lib/language_pack/base.rb

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
require "language_pack/fetcher"
1010

1111
Encoding.default_external = Encoding::UTF_8 if defined?(Encoding)
12-
ENV["BPLOG_PREFIX"] = "buildpack.ruby"
1312

1413
# abstract class that all the Ruby based Language Packs inherit from
1514
class LanguagePack::Base
@@ -21,23 +20,19 @@ class LanguagePack::Base
2120
MULTI_ARCH_STACKS = ["heroku-24"]
2221
KNOWN_ARCHITECTURES = ["amd64", "arm64"]
2322

24-
attr_reader :build_path, :cache, :stack
23+
attr_reader :app_path, :cache, :stack
2524

26-
# changes directory to the build_path
27-
# @param [String] the path of the build dir
28-
# @param [String] the path of the cache dir this is nil during detect and release
29-
def initialize(build_path, cache_path = nil)
30-
@build_path = build_path
25+
def initialize(app_path: , cache_path: )
26+
@app_path = app_path
3127
@stack = ENV.fetch("STACK")
3228
@cache = LanguagePack::Cache.new(cache_path)
3329
@metadata = LanguagePack::Metadata.new(@cache)
3430
@bundler_cache = LanguagePack::BundlerCache.new(@cache, @stack)
35-
@id = Digest::SHA1.hexdigest("#{Time.now.to_f}-#{rand(1000000)}")[0..10]
3631
@fetchers = {:buildpack => LanguagePack::Fetcher.new(VENDOR_URL) }
3732
@arch = get_arch
3833
@report = HerokuBuildReport::GLOBAL
3934

40-
Dir.chdir build_path
35+
Dir.chdir app_path
4136
end
4237

4338
def get_arch
@@ -54,7 +49,7 @@ def get_arch
5449
arch
5550
end
5651

57-
def self.===(build_path)
52+
def self.===(app_path)
5853
raise "must subclass"
5954
end
6055

@@ -134,7 +129,7 @@ def setup_language_pack_environment
134129
end
135130

136131
def add_to_profiled(string, filename: "ruby.sh", mode: "a")
137-
profiled_path = "#{build_path}/.profile.d/"
132+
profiled_path = "#{app_path}/.profile.d/"
138133

139134
FileUtils.mkdir_p profiled_path
140135
File.open("#{profiled_path}/#{filename}", mode) do |file|

lib/language_pack/disable_deploys.rb

Lines changed: 0 additions & 17 deletions
This file was deleted.

lib/language_pack/no_lockfile.rb

Lines changed: 0 additions & 16 deletions
This file was deleted.

lib/language_pack/rack.rb

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,4 @@ def setup_profiled(**args)
3838
super(**args)
3939
set_env_default "RACK_ENV", "production"
4040
end
41-
4241
end
43-

lib/language_pack/rails2.rb

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ def self.use?
1414
return is_rails2
1515
end
1616

17-
def initialize(*args)
18-
super(*args)
17+
def initialize(app_path: , cache_path: )
18+
super(app_path: app_path, cache_path: cache_path)
1919
@rails_runner = LanguagePack::Helpers::RailsRunner.new
2020
end
2121

@@ -57,11 +57,11 @@ def compile
5757

5858
def best_practice_warnings
5959
if env("RAILS_ENV") != "production"
60-
warn(<<-WARNING)
61-
You are deploying to a non-production environment: #{ env("RAILS_ENV").inspect }.
62-
This is not recommended.
63-
See https://devcenter.heroku.com/articles/deploying-to-a-custom-rails-environment for more information.
64-
WARNING
60+
warn(<<~WARNING)
61+
You are deploying to a non-production environment: #{ env("RAILS_ENV").inspect }.
62+
This is not recommended.
63+
See https://devcenter.heroku.com/articles/deploying-to-a-custom-rails-environment for more information.
64+
WARNING
6565
end
6666
super
6767
end
@@ -84,5 +84,4 @@ def setup_profiled(**args)
8484
set_env_default key, value
8585
end
8686
end
87-
8887
end

0 commit comments

Comments
 (0)