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.

Sign up for GitHub

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

Jump to bottom

Allow bundling with local govuk_frontend_toolkit #1035

Closed
wants to merge 4 commits into from
Closed

Allow bundling with local govuk_frontend_toolkit #1035

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8dd9c20
Allow bundling with local govuk_frontend_toolkit
NickColley May 10, 2017
e367bad
Add comment explaining sudo use
NickColley Jul 18, 2017
995e55d
Fix output of finding installed location
maxgds Sep 15, 2017
111450c
Merge pull request #1145 from alphagov/fix-startup-for-govuk-frontend…
maxgds Sep 15, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion Gemfile
View file
Open in desktop
Expand Up @@ -41,7 +41,12 @@ group :development, :test do
end

gem 'plek', '1.11.0'
gem 'govuk_frontend_toolkit', '~> 6.0.3'

if ENV['GOVUK_FRONTEND_TOOLKIT_DEV']
gem 'govuk_frontend_toolkit', path: "./tmp/govuk_frontend_toolkit_gem_dev"
else
gem 'govuk_frontend_toolkit', '~> 6.0.3'
end

if ENV['GOVUK_TEMPLATE_DEV']
gem 'govuk_template', path: "../govuk_template"
Expand Down
3 changes: 3 additions & 0 deletions README.md
View file
Open in desktop
Expand Up @@ -53,6 +53,9 @@ are wrapped around feedback pages, run `bowl feedback
in a separate terminal. Following local edits to `static`, restarting just
`feedback` should be sufficient.

#### Testing a local version of govuk_frontend_toolkit
To test any local updates to [govuk_frontend_toolkit](https://github.com/alphagov/govuk_frontend_toolkit) you can use `./startup.sh --test-govuk-frontend-toolkit`

### Running the test suite

`bundle exec rake` runs the test suite.
Expand Down
28 changes: 27 additions & 1 deletion startup.sh
View file
Open in desktop
@@ -1,4 +1,30 @@
#!/bin/bash
#!/bin/bash

bundle install

if [[ $1 == "--test-govuk-frontend-toolkit" ]] ; then
# Find out where it the gem is installed
installed_location="$(bundle show govuk_frontend_toolkit | sed -1p)"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line failed for me on the dev VM for a couple of reasons.

  1. sed -1p is not supported on the dev VM, on the assumption that 1p is the sed command we want to run it should be sed '1p'.

  2. The dev VM is currently pinned to a version of bundler (1.15.1) that shows a warning message if the version of bundler is older than the most recent, and this appears on stdout so it's included in the message. We can disable this with an environment variable BUNDLE_DISABLE_VERSION_CHECK.

Putting this all together I had to change this to:

`BUNDLE_DISABLE_VERSION_CHECK=yes bundle show govuk_frontend_toolkit | sed '1p'`

However, this outputs the path twice for some reason:

/usr/lib/rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/govuk_frontend_toolkit-7.0.1
/usr/lib/rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/govuk_frontend_toolkit-7.0.1

If I drop the sed part entirely then it only outputs the path once, which feels like it's what we're after:

/usr/lib/rbenv/versions/2.2.3/lib/ruby/gems/2.2.0/gems/govuk_frontend_toolkit-7.0.1

temporary_location="./tmp/govuk_frontend_toolkit_gem_dev"

# Remove any existing tmp file
rm -rf ${temporary_location}

# Copy the old assets aside
# Using sudo here since the installed location has elevated permissions
sudo cp -r ${installed_location} ${temporary_location}

# Remove current submoduled assets
rm -rf ${temporary_location}/app/assets

# Symlink the local
sudo ln -rs ../govuk_frontend_toolkit ${temporary_location}/app/assets
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my local govuk_frontend_toolkit checkout I've been running the tests so I have a node_modules folder which is copied over as part of this command. This folder is huge, and contains coffee script assets. This has two repercussions:

  1. the requests to static take a very long time as they cache all the files in node_modules
  2. the requests to static fail because we don't include the coffee script gem, but there are coffee script files in the node_modules folder which is part of the assets path now and sprockets breaks when trying to load a coffee script processor for them.

If we add a rm -rf ${temporary_location}/app/assets/node_modules this gets round the problem, but it means we've deleted them from the local checkout to which is pretty surprising. If we change ln -rs to cp -r as well that works, but we'd have to keep remembering to run this script whenever we make a local change to govuk_frontend_toolkit.

Not sure what the best solution is TBH.


export GOVUK_FRONTEND_TOOLKIT_DEV=true

bundle install

echo "Testing local govuk_frontend_toolkit"
fi

bundle exec rails s -p 3013