Makefile: various improvements

[maxim-belkin]

0. ~Use proper bundle config set --local path ... syntax instead of (incorrect) bundle config --local set path ...~ (implemented in #559).
1. ~Decide whether to use bundle or jekyll based on the presence of Gemfile or Gemfile.lock~
2. ~Use $(SHELL) to call bin/knit_lessons.sh: this lets us avoid relying on /usr/bin/env finding bash.~ (implemented in #562)
3. New bundle target for building .vendor/bundle directory with required gems: a. This is an alias for .vendor/bundle target which depends on Gemfile and Gemfile.lock b. It installs/updates bundled gems only if necessary
4. Empty Gemfile target for lessons that don't use it. a. Gemfile.lock target that creates it based on Gemfile
5. ~clean target: remove .vendor and .bundle directories and Gemfile.lock file.~ (implemented in #560)

[maxim-belkin]

@fmichonneau, I rebased this on top of the latest gh-pages. Changes here deal with Bundler and Gemfile(s). This PR also fixes an incorrect syntax for bundler config command (which should be bundle config set --local rather than bundle config --local set).

[fmichonneau]

Thanks for this Maxim! Overall, it looks good. After talking with the @carpentries/core-team-curriculum, one thing that came up is that we do want everyone to use bundler to ensure that we use the same dependencies as GitHub pages, so when the Gemfile is missing or bundler is not available, the build should fail and indicate that these components are missing. I think we also want to run bundle update regularly (every time?) to ensure that dependencies get updated when the github-pages gem gets updated.

[maxim-belkin]

we do want everyone to use bundler to ensure that we use the same dependencies as GitHub pages, so when the Gemfile is missing or bundler is not available, the build should fail and indicate that these components are missing.

Sounds reasonable. I've updated the PR accordingly.

I think we also want to run bundle update regularly (every time?) to ensure that dependencies get updated when the github-pages gem gets updated.

In the light that we want to be able to run make serve and make site offline (https://github.com/carpentries/styles/pull/451#issuecomment-591465135) and that bundle update requires an internet connection, I split bundle target into two:

1. bundle target that installs all the necessary gems into .vendor/bundle directory, and
2. update-bundle target that updates gems in .vendor/bundle directory.

make site uses gems currently installed in .vendor/bundle (if it exists) or installs them there. This way, one can run make bundle while connected to the internet, then use make site offline. Then, whenever they're online and want to update the gems, they can call make update-bundle. Does that seem reasonable? Please let me know if you think that make site has to call bundle update regardless of the internet connection -- I'll update the PR.

[maxim-belkin]

@fmichonneau, do you have any concerns or questions about this PR?

[maxim-belkin]

@fmichonneau, ping. If desired, I can factor out the fix for the bundle config command into a separate PR -- just let me know.

[fmichonneau]

Thanks for the ping @maxim-belkin, this had fell off my radar. I appreciate the flexibility that separating the update-bundle and site targets provide but I worry that people will not forget to run update-bundle regularly. Could we keep make site to run bundle update but maybe create a new target make site-offline that would not (and say that an internet connection is needed if the bundle doesn't exist)?

[maxim-belkin]

I appreciate the flexibility that separating the update-bundle and site targets provide but I worry that people will not forget to run update-bundle regularly.

update-bundle target (which runs bundle update) updates the Gemfile.lock file when GitHub updates the github-pages gem. The release cycle of this gem is irregular and isn't frequent (see https://github.com/github/pages-gem/releases), so I wouldn't worry about not running bundle update every time we build a website locally. But even when the gem is updated -- I don't expect our lessons to depend on the bleeding-edge features/gems.

Could we keep make site to run bundle update but maybe create a new target make site-offline that would not (and say that an internet connection is needed if the bundle doesn't exist)?

We can:

1. create another target that would do that
2. use some tricks to run bundle update monthly/weekly/daily
3. leave this PR as is

Personally, I think make site should be as "offline" as possible because it's used to build a website and not be 100% in sync with the github-pages gem. But if you think we need to run bundle update with every make site -- I'd be happy to do that.

[maxim-belkin]

Also, The Carpentries could remind maintainers to update the bundles (by running make update-bundle or bundle update) in monthly emails when the github-pages gem is updated.

[maxim-belkin]

@fmichonneau, I added site-offline target and made site target always update the bundle.

[maxim-belkin]

Failure (R Intro Geospatial (macOS), link) seems to be unrelated to the change.

[maxim-belkin]

I'm going to move the fix for thebundle config command into a separate PR as it seems that the review of this PR is taking a bit longer than usual.

[maxim-belkin]

@fmichonneau @zkamvar, I've implemented several improvements to this PR and I think it's ready for your reviews. Below I list the most noticeable changes:

1. Added $(info ...) to all targets to make it clear which one is being executed:

   Example: make site

   $ make site
   ==> bundler
   ==> Gemfile.lock [Gemfile]
   Fetching gem metadata from https://rubygems.org/...........
   Fetching gem metadata from https://rubygems.org/.
   Resolving dependencies....
   Writing lockfile to /Users/mbelkin/git/swcarpentry/python-novice-inflammation/Gemfile.lock
   ==> .bundle/config []
   ==> update-bundle [Gemfile.lock .bundle/config]
   ==> lesson-md []
   ==> site-offline [lesson-md index.md bundler .vendor/bundle]
   Configuration file: /Users/mbelkin/git/swcarpentry/python-novice-inflammation/_config.yml
               Source: /Users/mbelkin/git/swcarpentry/python-novice-inflammation
          Destination: /Users/mbelkin/git/swcarpentry/python-novice-inflammation/_site
    Incremental build: disabled. Enable with --incremental
         Generating... 
                       done in 3.291 seconds.
    Auto-regeneration: disabled. Use --watch to enable.
   ==> site [update-bundle site-offline]
   

   Example: make site-offline

   $ make site-offline
   ==> lesson-md []
   ==> bundler
   ==> site-offline [lesson-md index.md bundler .vendor/bundle]
   Configuration file: /Users/mbelkin/git/swcarpentry/python-novice-inflammation/_config.yml
               Source: /Users/mbelkin/git/swcarpentry/python-novice-inflammation
          Destination: /Users/mbelkin/git/swcarpentry/python-novice-inflammation/_site
    Incremental build: disabled. Enable with --incremental
         Generating... 
                       done in 3.335 seconds.
    Auto-regeneration: disabled. Use --watch to enable.
   

2. Added site-offline and serve-offline targets. These targets do not update bundled Ruby gems before building/serving a website.

3. clean target now deletes Gemfile.lock only if Git does not track it.

4. New targets related to bundler and gems:

   1. bundler: check that Bundler is installed
   2. .bundle/config: configures Bundler (bundle config set --local path .vendor/bundle)
   3. install-bundle: install Ruby gems specified in Gemfile/Gemfile.lock
   4. .vendor/bundle: same as install-bundle (but not a PHONY target)
   5. update-bundle: update Ruby gems specified in Gemfile/Gemfile.lock. Used by site and serve targets
   6. Gemfile.lock: create Gemfile.lock based on Gemfile
   7. Added index.html target (necessary for workshop-check) and combined it with index.md and Gemfile targets: each of these targets does one thing -- checks that corresponding file exists, so it makes sense to not duplicate code.
   8. Some targets use bundler as an order-only prerequisite (| bundler). This is necessary because bundler is a PHONY target and when it's used as a normal prerequisite it causes the target to be regenerated even though there were no changes.

And it looks like these changes passed all the tests while I was writing all of the above, so this is promising.