DFE-Digital / rails-template

Rails 7 template with GOV.UK Frontend
27 stars 5 forks source link

Fix bundle with asdf #18

Closed misaka closed 1 year ago

misaka commented 1 year ago

I ran into an issue trying to run bundle with a project generated from this template, and uncovered a couple of other issues.


Running bundle from a project generated with this template fails because the pg gem fails to build.

$ bundle install
Fetching gem metadata from https://rubygems.org/.........
... normal template output removed for brevity ...
Fetching pg 1.4.4
...
Installing pg 1.4.4 with native extensions
Gem::Ext::BuildError: ERROR: Failed to build gem native extension.

    current directory: /Users/misaka/.asdf/installs/ruby/3.1.2/lib/ruby/gems/3.1.0/gems/pg-1.4.4/ext
/Users/misaka/.asdf/installs/ruby/3.1.2/bin/ruby -I /Users/misaka/.asdf/installs/ruby/3.1.2/lib/ruby/3.1.0 -r ./siteconf20221207-56729-ni1ph7.rb extconf.rb
...
Calling libpq with GVL unlocked
checking for pg_config... yes
Using config values from /Users/misaka/.asdf/shims/pg_config
No version is set for command pg_config
Consider adding one of the following versions in your config file at /Users/misaka/.tool-versions
postgres 13.5
...

The key here is No version is set for command pg_config. asdf doesn't know what version of postgres to use because bundle it's changed to a directory that isn't the project directory. This is annoying and not this template's fault, but we can add instructions on how to circumvent this.

Change 1: Update generated README

When installing the pg gem, bundle changes directory outside of this project directory, causing it lose track of which version of postgres has been selected in the project's .tool-versions file. To ensure the pg gem installs correctly, you'll want to set the version of postgres that asdf will use:

# Temporarily set the version of postgres to use to build the pg gem
ASDF_POSTGRES_VERSION=13.5 bundle install

Non-obvious failures when running without dependencies

When running the template without the dependencies available, or as in my case, incorrectly assuming that since it's using asdf it will pick up the correct dependencies, the run fails with errors that aren't obvious. This could lead users to incorrectly assuming that an incorrectly setup project is now ready.

$ rails new --force --database=postgresql --skip-bundle --skip-git --skip-jbuilder --skip-hotwire --skip-action-mailbox --skip-action-mailer --skip-action-text --asset-pipeline=propshaft --javascript=esbuild --css=sass -m DfE/rails-template/template.rb apply-for-a-juggling-licence
... normal template output removed for brevity ...
         run    bundle --quiet from "."
         run    rails css:install:sass from "."
Build into app/assets/builds
      create  app/assets/builds
      create  app/assets/builds/.keep
Remove app/assets/stylesheets/application.css so build output can take over
      remove  app/assets/stylesheets/application.css
Add stylesheet link tag in application layout
File unchanged! The supplied flag value not found!  app/views/layouts/application.html.erb
Add default package.json
      create  package.json
Add default Procfile.dev
      create  Procfile.dev
Ensure foreman is installed
         run  gem install foreman from "."
Successfully installed foreman-0.87.2
Parsing documentation for foreman-0.87.2
Done installing documentation for foreman after 0 seconds
1 gem installed
Add bin/dev to start foreman
      create  bin/dev
Install Sass
      create  app/assets/stylesheets/application.sass.scss
         run  yarn add sass from "."
No version is set for command yarn
Consider adding one of the following versions in your config file at /Users/misaka/.tool-versions
yarn 1.22.19
Add build:css script
No version is set for command npx
Consider adding one of the following versions in your config file at /Users/misaka/.tool-versions
nodejs 18.1.0
Add "scripts": { "build:css": "sass ./app/assets/stylesheets/application.sass.scss:./app/assets/builds/application.css --no-source-map --load-path=node_modules" } to your package.json
         run    rails javascript:install:esbuild from "."
Compile into app/assets/builds
       exist  app/assets/builds
   identical  app/assets/builds/.keep
Add JavaScript include tag in application layout
      insert  app/views/layouts/application.html.erb
Create default entrypoint in app/javascript/application.js
      create  app/javascript
      create  app/javascript/application.js
      append  Procfile.dev
Add bin/dev to start foreman
   identical  bin/dev
Install esbuild
         run  yarn add esbuild from "."
No version is set for command yarn
Consider adding one of the following versions in your config file at /Users/misaka/.tool-versions
yarn 1.22.19
Add build script
No version is set for command npx
Consider adding one of the following versions in your config file at /Users/misaka/.tool-versions
nodejs 18.1.0
Add "scripts": { "build": "esbuild app/javascript/*.* --bundle --sourcemap --outdir=app/assets/builds --public-path=assets" } to your package.json
        gsub    package.json
      create    bin/bundle
       chmod    bin/bundle
      remove    app/assets/stylesheets/application.css
       force    app/assets/stylesheets/application.sass.scss
       force    app/javascript/application.js
       force    app/views/layouts/application.html.erb
    generate    rspec:install
       rails    generate rspec:install
      create  .rspec
      create  spec
      create  spec/spec_helper.rb
      create  spec/rails_helper.rb
      insert    app/controllers/application_controller.rb
      create    config/initializers/govuk_formbuilder.rb
      insert    config/application.rb
      remove    config/initializers/assets.rb
    generate    controller
       rails    generate controller pages home --skip-routes
      create  app/controllers/pages_controller.rb
      invoke  erb
      create    app/views/pages
      create    app/views/pages/home.html.erb
      invoke  test_unit
      create    test/controllers/pages_controller_test.rb
      invoke  helper
      create    app/helpers/pages_helper.rb
      invoke    test_unit
       route    root to: 'pages#home'
       force    app/views/pages/home.html.erb
       force    config/locales/en.yml
      create    Dockerfile
      create    .dockerignore
         run    yarn --silent add govuk-frontend@4.0.1 from "."
No version is set for command yarn
Consider adding one of the following versions in your config file at /Users/misaka/.tool-versions
yarn 1.22.19

This happens because asdf was setup after other steps, like installing gems, yarn, etc. The only way to make this work correctly and as expected is to either have asdf configured correctly for this project already, or by setting up asdf before we do these other setup steps.

Change 2: Setup asdf first

The script now does the asdf setup before the other steps to ensure the gems and node packages get installed to the right place.

Change 3: Install required plugins using asdf

When setting up asdf, the plugins we rely on will be installed by running asdf plugin add ______ to ensure they are available to the rest of the setup process.

Change 4: Update our README

The README for this project now makes it clear that some required tools will be installed using asdf if that option is selected. It also has examples of how to check the versions of the installed tools if asdf is not used, to help make it clearer to users what's expected.

Change 5: Move adrs setup to end

Because moving the asdf setup to the beginning affects how the README file is generated, the "How the application works" section was wrongly placed in the middle of the Setup section. To fix this, the ADRs setup has been moved to the end of the script. This doesn't break the script since there are no dependencies aside from installing the rladr, which is not installed in the correct asdf Ruby instance.

misaka commented 1 year ago

After a chat with @tvararu I've double-checked that this template works for the non-asdf path. I ran it by setting the tools versions asdf uses in my shell (setting ASDF_NODEJS_VERSION, ASDF_YARN_VERSION, etc), so this is not a 100% asdf-less test, but as the test doesn't install .tools-version I think this is close enough to show that these changes haven't introduced any new issues. Any issues using non-asdf tool installations should probably be addressed in a separate PR.

misaka commented 1 year ago

I've also addressed a couple more issues relating to the asdf path:

Change 6: Create asdf section in README

Before instructions for asdf would be added as part of the h2 "Setup" section. This meant that if asdf was not installed then there would be no h2 "Setup" heading, and the h3 headings that follow for linting, etc, wouldn't be nested into an h2 nicely.

Now an h2 "Setup" heading is always added and asdf gets it's own h3 section.

Change 7: Do asdf setup if .tools-version file detected

If a .tools-version file was detected before, the entire asdf setup was aborted, meaning no instructions added to the README (which gets regenerated every run, so would be blank). Now the asdf section is added to the README and the plugins are added even if .tools-version is detected.

Change 8: Remove our .ruby-version template

Rails already installs this for us, so this is one less thing for us to worry about. Rails also uses the (more) correct format with ruby- included:

ruby-3.1.2

whereas ours was just the version, i.e. 3.1.2