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

bin/configure could be broken up #1440

Closed
jagthedrummer opened this issue May 6, 2024 · 2 comments · Fixed by #1478
Closed

bin/configure could be broken up #1440

jagthedrummer opened this issue May 6, 2024 · 2 comments · Fixed by #1478

Comments

@jagthedrummer
Copy link
Contributor

The bin/configure script does a lot of things and it's not really possible (or at least not easy) to run only a portion of the script. It might be nice to extract the various pieces out into separate scripts that could be run individually.

The biggest problem with the current arrangement is that we first do a lot of work to try to detect and install dependencies, before moving on to making changes to the app config and setting up git. We don't gracefully handle dependency detection on all systems, and when that fails it can end up blocking people from being able to finish making the config changes needed.

So, in addition to extracting the bits into separate scripts we might also revisit the order in which we run those individual scripts. We might want to do config tweaking first, and then move on to dependency installation.

Related to #1435

@jagthedrummer
Copy link
Contributor Author

Starting to take a look at what all happens in bin/configure and am just going to kind of live-tweet my findings here.

The first thing we do is make sure that a couple of utility gems are installed and required.

puts ""
puts `gem install colorize`
puts ""
puts ""
puts `gem install activesupport`
puts ""
require 'colorize'
require 'active_support'

Then we define several utility functions that we use later in the script.

def replace_in_file(file, before, after, target_regexp = nil)
puts "Replacing in '#{file}'."
if target_regexp
target_file_content = ""
File.open(file).each_line do |l|
l.gsub!(before, after) if !!l.match(target_regexp)
l if !!l.match(target_regexp)
target_file_content += l
end
else
target_file_content = File.open(file).read
target_file_content.gsub!(before, after)
end
File.open(file, "w+") do |f|
f.write(target_file_content)
end
end
def stream(command, prefix = " ")
puts ""
IO.popen(command) do |io|
while (line = io.gets) do
puts "#{prefix}#{line}"
end
end
puts ""
end
def ask(string)
puts string.blue
return gets.strip
end
def not_installed?(package)
`brew info #{package} | grep "Not installed"`.strip.length > 0
end
def check_package(package)
if not_installed?(package)
puts "#{package} is not installed via Homebrew. Try running `brew install #{package}`.".red
input = ask "Try proceeding without #{package}? [y/n]"
if input.downcase[0] == "n"
exit
end
else
puts "#{package} is installed via Homebrew.".green
end
end

Then we check that the version of ruby on the local system matches what we expect. If we don't have the right version we don't just bail out automatically, we give the user the option to try to continue with whatever ruby version they're using.

# Unless the shell's current version of Ruby is the same as what the application requires, we should flag it.
# rbenv produces strings like "3.1.2" while rvm produces ones like "ruby-3.1.2", so we account for that here.
required_ruby = `cat ./.ruby-version`.strip.gsub(/^ruby-/, "")
actual_ruby = `ruby -v`.strip
message = "Bullet Train requires Ruby #{required_ruby} and `ruby -v` returns #{actual_ruby}."
if actual_ruby.include?(required_ruby)
puts message.green
else
puts message.red
input = ask "Try proceeding with with Ruby #{actual_ruby} anyway? [y/n]"
if input.downcase[0] == "n"
exit
end
end

Next we check whether homebrew is installed. We do this check even on Windows and Linux, which is unnecessary because those platforms don't have homebrew. This should be moved to be a part of the MacOS only conditional branch below.

if `brew info 2> /dev/null`.length > 0
puts "Homebrew is installed.".green
else
puts "You don't have Homebrew installed. This isn't necessarily a problem, you might not even be on macOS, but we can't check your dependencies without it.".red
input = ask "Try proceeding without Homebrew? [y/n]"
if input.downcase[0] == "n"
exit
end
end

Now we start a case statement where we do various things depending on your local OS.

case Gem::Platform.local.os

On darwin (MacOS) we seem to expect you to have homebrew installed and we just check to see if a few particular libraries are installed via homebrew. This is less than ideal because people may have installed these packages in some other way. Ideally we should check for their existence on the system and not for their existence inside homebrew. (Maybe there's an opportunity to treat darwin the same way that we treat linux? That would allow us to remove some code and simplify.)

when "darwin"
check_package("postgresql@14")
check_package("redis")
check_package("icu4c")

Continuing with the OS case statement we have the block for handling linux. Here we test for things in a variety of ways. For postgresql and libicu we use the output of dpkg to determine whether we think they're installed. For postgresql we go on to do an additional check by running psql --version to check the specific version. For libicu we don't do an additional check beyond dpkg. For redis we don't check dpkg at all and instead only check the output of running redis-cli --version.

Relying on dpkg has many of the same drawbacks as relying on homebrew on MacOS. Namely if people have installed these things in other ways then we can't see them. If they're on a Linux variant that doesn't have dpkg then our initial checks are likely to fail. Again, the ideal scenario would be checking for their existence on the system and not in a particular package manager. For postgresqul and redis this should be easy since there's a CLI command that we can use. I'm not sure how we'd detect libicu in a platform-and-package-manager-independent way.

bullet_train/bin/configure

Lines 95 to 132 in f4a32e7

when "linux"
system_packages = `dpkg -l | grep '^ii'`.split("\n").map do |package_information|
package_information.split("\s")[1]
end
psql_installed = system_packages.include?("postgresql")
if psql_installed
psql_version = `psql --version`.split("\s")[2]
if psql_version.match?(/^14/)
puts "You have PostgreSQL 14 installed.".green
else
puts "You have PostgreSQL installed, but you're using v#{psql_version} and not v14.".red
input = ask "Try proceeding without PostgreSQL 14? [y/n]"
if input.downcase[0] == "n"
exit
end
end
else
puts "You don't have PostgreSQL installed. Please see the installation instructions at https://ubuntu.com/server/docs/databases-postgresql".red
exit
end
# TODO: system_packages should include `redis`.
# https://github.com/bullet-train-co/bullet_train/issues/1330
begin
redis_version = `redis-cli --version`.chomp.split("\s").last
puts "Redis #{redis_version} is installed.".green
rescue
puts "You don't have Redis installed. Please see the installation instructions at https://redis.io/docs/getting-started/installation/install-redis-on-linux/ .".red
exit
end
if system_packages.select{|pkg| pkg.match?(/^libicu/)}.any?
puts "You have icu4c installed.".green
else
puts "You don't have icu4c installed. Please run `sudo apt-get install libicu-dev`.".red
exit
end

To close out the OS case statement we have a block for platforms that are neither darwin or linux (does that basically mean Windows-only?). Here we list the required packages and give people the option to either proceed or not.

bullet_train/bin/configure

Lines 133 to 144 in f4a32e7

else
puts "We currently don't support this platform to check if you have the following libraries installed:".red
puts "1. PostgreSQL"
puts "2. Redis"
puts "3. icu4c"
puts ""
puts "Please ensure they are installed before proceeding."
input = ask "Proceed? [y/n]"
if input.downcase[0] == "n"
exit
end
end

Next we try to determine which version of node is on the system. We do this by checking the output of node -v, so we're not relying on a particular OS or package manager.

bullet_train/bin/configure

Lines 146 to 164 in f4a32e7

required_node = `cat ./.nvmrc`.strip
actual_node = begin
`node -v`.strip.gsub("v", "")
rescue
:not_found
end
message = "Bullet Train requires Node.js #{required_node} and `node -v` returns #{actual_node}."
if actual_node == :not_found
puts "You don't have Node installed. We can't proceed without it. Try `brew install node`.".red
exit
elsif Gem::Version.new(actual_node) >= Gem::Version.new(required_node)
puts message.green
else
puts message.red
input = ask "Try proceeding with Node #{actual_node} anyway? [y/n]"
if input.downcase[0] == "n"
exit
end
end

Next we check for yarn, again by using the CLI. However we don't check the yarn version. We recently began to specify yarn version 4.2.1 (#1445) so we probably want to robustify this check a little bit.

bullet_train/bin/configure

Lines 166 to 171 in f4a32e7

if `yarn -v 2> /dev/null`.length > 0
puts "Yarn is installed.".green
else
puts "You don't have Yarn installed. We can't proceed without it. Try `brew install yarn` or see the installation instructions at https://yarnpkg.com/getting-started/install .".red
exit
end

Next is a block that is currently commented out, with a note about uncommenting it. Should we have already done that?

bullet_train/bin/configure

Lines 173 to 182 in f4a32e7

# TODO: Uncomment this when the enable bulk invitations JS is implemented.
# Enable the bulk invitations configuration.
# bt_config_lines = File.open("config/initializers/bullet_train.rb").readlines
# new_lines = bt_config_lines.map do |line|
# if line.match?("config.enable_bulk_invitations")
# line.gsub!(/#\s*/, "")
# end
# line
# end
# File.write("config/initializers/bullet_train.rb", bt_config_lines.join)

Next is a block where we ask you if you want to push your new project to GitHub.

bullet_train/bin/configure

Lines 184 to 188 in f4a32e7

puts "Next, let's push your application to GitHub."
puts "If you would like to use another service like Gitlab to manage your repository,"
puts "you can opt out of this step and set up the repository manually."
puts "(If you're not sure, we suggest going with GitHub)"
skip_github = ask "Continue setting up with GitHub? [y/n]"

After asking about GitHub there's a long block where we do various things.

First we try to change the name of the remote for the starter repo from origin to bullet-train. We should probably do this no matter how you answer the GitHub question.

bullet_train/bin/configure

Lines 190 to 199 in f4a32e7

if `git remote | grep bullet-train`.strip.length > 0
puts "Repository already has a \`bullet-train`\ remote.".yellow
else
if `git remote | grep origin`.strip.length > 0
puts "Renaming repository `origin` remote to `bullet-train`.".green
`git remote rename origin bullet-train`
else
puts "Repository has no `origin` remote, but also no `bullet-train` remote. Did something go wrong?".red
end
end

Then comes a block where we pop open a web browser for you so you can create a new repo on GitHub. Once you've done that you're supposed to paste the https variant of the repo URL. Once you're into this block there doesn't seem to be any way out of it short of aborting the whole script with Ctl-c or something.

bullet_train/bin/configure

Lines 201 to 219 in f4a32e7

if `git remote | grep origin`.strip.length > 0
puts "Repository already has a \`origin`\ remote.".yellow
else
ask "Hit <Return> and we'll open a browser to GitHub where you can create a new repository. When you're done, copy the SSH path from the new repository and return here. We'll ask you to paste it to us in the next step."
command = if Gem::Platform.local.os == "linux"
"xdg-open"
else
"open"
end
`#{command} https://github.com/new`
ssh_path = ask "OK, what was the SSH path? (It should look like `[email protected]:your-account/your-new-repo.git`.)"
while ssh_path == ""
puts "You must provide a path for your new repository.".red
ssh_path = ask "What was the SSH path? (It should look like `[email protected]:your-account/your-new-repo.git`.)"
end
puts "Setting repository's `origin` remote to `#{ssh_path}`.".green
puts `git remote add origin #{ssh_path}`.chomp
end

Then to conclude the GitHub-repo-setup block we grab the name of your current branch, and then push it to your new repo as the main branch. This seems fairly presumptuous to me. If you cloned the starter repo, and then checked out a new branch (git checkout -b run-bin-configure or something) before running bin/configure then you'll end up with that local branch pushed to your repo as main. I certainly wouldn't want that, and I'm having trouble imagining a scenario in which that would be useful. We should probably do a vanilla git push if we can.

bullet_train/bin/configure

Lines 221 to 224 in f4a32e7

local_branch = `git branch | grep "*"`.split.last
puts "Pushing repository to `origin`.".green
stream "git push origin #{local_branch}:main 2>&1"

After the GitHub stuff we then run bundle install and yarn install.

bullet_train/bin/configure

Lines 227 to 231 in f4a32e7

puts "Running `bundle install`.".green
stream "bundle install"
puts "Running `yarn install`.".green
stream "yarn install"

Next we ask for the name of your application. Then we munge that to create a few variants ("My Test App" => "my_test_app" => "mytestapp" => etc...). Then we do string substitution in a bunch of files to replace instances of "Untitled Application" or "untitled_application" with the new values.

bullet_train/bin/configure

Lines 233 to 265 in f4a32e7

human = ask "What is the name of your new application in title case? (e.g. \"Some Great Application\")"
while human == ""
puts "You must provide a name for your application.".red
human = ask "What is the name of your new application in title case? (e.g. \"Some Great Application\")"
end
require "active_support/inflector"
variable = ActiveSupport::Inflector.parameterize(human.gsub("-", " "), separator: '_')
environment_variable = ActiveSupport::Inflector.parameterize(human.gsub("-", " "), separator: '_').upcase
class_name = variable.classify
kebab_case = variable.tr("_", "-")
connected_name = variable.gsub("_", "") # i.e. `bullettrain` as opposed to `bullet_train`
puts ""
puts "Replacing instances of \"Untitled Application\" with \"#{human}\" throughout the codebase.".green
replace_in_file("./.circleci/config.yml", "untitled_application", variable)
replace_in_file("./config/application.rb", "untitled_application", connected_name)
replace_in_file("./config/database.yml", "untitled_application", variable)
replace_in_file("./config/database.yml", "UNTITLED_APPLICATION", environment_variable)
replace_in_file("./config/cable.yml", "untitled_application", variable)
replace_in_file("./config/initializers/session_store.rb", "untitled_application", variable)
replace_in_file("./config/environments/production.rb", "untitled_application", variable)
replace_in_file("./config/application.rb", "UntitledApplication", class_name)
replace_in_file("./config/locales/en/application.en.yml", "Untitled Application", human, /name/)
replace_in_file("./config/locales/en/application.en.yml", "untitled_application", variable)
replace_in_file("./config/locales/en/application.en.yml", "untitled application", human.downcase, /keywords/)
replace_in_file("./config/locales/en/user_mailer.en.yml", "Untitled Application", human)
replace_in_file("./zapier/package.json", "untitled-application", kebab_case)
replace_in_file("./zapier/package.json", "Untitled Application", human)
replace_in_file("./app/views/api/v1/open_api/index.yaml.erb", "Untitled Application", human)
replace_in_file("./app.json", "Untitled Application", human)
replace_in_file("./.redocly.yaml", "untitled_application", variable)

Next up is a block that I don't think is ever run. Due to the way that we capture the skip_github variable higher up in the script it is always populated with either y or n and so doing unless skip_github always evaluates to unless true and so we skip the block.

Allegedly this block tries to change the repo link on the "deploy to render" button in README.example.md, but only if you did not setup GitHub. I think it would probably be better to ask the user if they intend to use Render and then proceed from there. If they don't plan to use Render we should remove the button entirely. And if they do intend to use it we can swap out the repo URLs.

bullet_train/bin/configure

Lines 269 to 273 in f4a32e7

unless skip_github
original_repo_link = "https://github.com/bullet-train-co/bullet_train"
new_repo_link = ask "What is the link to your repository? We will use this to enable the one-click deploy to Render button for your application."
replace_in_file("README.example.md", original_repo_link, new_repo_link, /repo=#{original_repo_link}/)
end

Then we move README.example.md to overwrite README.md (so that the new project doesn't contain the full "how to get started with Bullet Train" stuff that's in the main README.md).

bullet_train/bin/configure

Lines 275 to 276 in f4a32e7

puts "Moving `./README.example.md` to `./README.md`.".green
puts `mv ./README.example.md ./README.md`.chomp

Then we remove a file that is specific to the BT starter repo. There are probably a few more files we could remove here. Specifically some of the workflow files that are for internal core team processes.

puts `rm .github/FUNDING.yml`.chomp

Then we do one more string substitution. It's not clear to me why this one would need to happen after overwriting README.md. It seems like we could make the change in README.example.md before overwriting.

bullet_train/bin/configure

Lines 280 to 281 in f4a32e7

# We can only do this after the README is moved into place.
replace_in_file("./README.md", "Untitled Application", human)

Now we have another block related to GitHub setup. Again the seeming intention here differs from what actually happens, again due to how skip_github is captured. What happens is that we always tell you that you should commit your changes. It seems the intention is that if you did setup GitHub that we should automatically add all the changes, commit them, and then push them to your repo. This again explicitly pushes whatever your current local branch is to main on the repo. Personally I'm not sure we should add, commit and push for people. At the very least we should ask.

  • Do you want us to commit the changes made by this script? y/n?
  • Do you want us to push those changes to your repo? y/n?

bullet_train/bin/configure

Lines 283 to 291 in f4a32e7

if skip_github
puts ""
puts "Make sure you save your changes with Git.".yellow
else
puts "Committing all these changes to the repository.".green
stream "git add -A"
stream "git commit -m \"Run configuration script.\""
stream "git push origin #{local_branch}:main"
end

And finally we output some messages.

bullet_train/bin/configure

Lines 293 to 297 in f4a32e7

puts ""
puts "OK, we're done, but at some point you should edit `config/locales/en/application.en.yml`!".yellow
puts ""
puts "Next you can run `bin/setup`, then `bin/dev` to spawn a local instance, and then you can navigate to http://localhost:3000 to access your new application.".green
puts ""

I think broadly this script only does a few big-picture things, but they're currently kind of jumbled up in confusing ways.

  • check for system-level dependencies like postgresql, redis, etc...
  • ruby version checking and bundle install
  • node/yarn version checking, and yarn install
  • "Untitled Application" => "My Awesome Project" type string substitution and related tasks to change it from being a copy of the starter repo into a "real" application
  • GitHub setup

I think ideally we should be able to separate these things out into a collection of smaller scripts (bin/check_dependencies, bin/check_ruby, etc...) so that it would be possible to run any one of them at a time. We can tie them all together by calling them from within bin/configure.

It's also probably worth thinking about the order in which this all happens. For instance, is it really necessary to know that all the dependencies are installed before we do string substitution and setup GitHub? Is it realistic to to do the string substitution first, then GitHub, then local dependency checking?

To put it a different way: Should you be blocked from starting a new app and pushing it to GitHub just because you don't have all of your local dependencies satisfied? (And satisfied via the package managers that we know how to interrogate?)

@jagthedrummer
Copy link
Contributor Author

jagthedrummer commented May 21, 2024

All of the current open issues around bin/configure. Just to have easy reference:

#1435 - bin/configure assumes Debian based system

#1412 - bin/configure should probably rename origin even if you skip GitHub setup

#1388 - Repo link in README is not replaced

#1116 - bin/setup failing for me based on Node version.

#1262 - Add Tunnelmole as an open source alternative to ngrok

#814 - bin/configure behaves differently for y/n questions when ENTER is pressed (without answering y or n)

#515 - rails not found in bin/configure step

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 a pull request may close this issue.

1 participant