Update container image

[nathanweeks]

The jekyll/jekyll Docker image (GitHub repository envygeeks/jekyll-docker) is apparently unmaintained. Aside from lacking software environment updates and aarch64 builds, its jekyll wrapper script changes ownership of jekyll-generated in the bind-mounted volume, which seems to cause fatal errors on startup in my environment using Rancher Desktop:

% docker compose up
...
jekyll-starter-legumeinfo-jekyll-1  | chown: .jekyll-cache: Permission denied
jekyll-starter-legumeinfo-jekyll-1 exited with code 1

This PR suggests using a maintained container image like bretfisher/jekyll-serve.

Additionally:

• Use more-strict YAML & Liquid parsing options to catch bugs in the starter & theme
• Remove jekyll-feed plugin in anticipation of https://github.com/legumeinfo/jekyll-theme-legumeinfo/issues/63

If anything is controversial, I can revert / break into smaller PRs.

[alancleary]

Thanks for the PR @nathanweeks. I find it interesting that the bretfisher images don't have versions. It has me thinking that maybe Jekyll shouldn't be provided by the image in the first place, i.e. we want the Jekyll version installed in containers to match what's in the Gemfile and having it preinstalled via a third-party image undermines this. Perhaps we should just use a ruby base image and have Jekyll installed by the Gemfile, either in our own Dockerfile or directly in docker-compose.yml. This would be analogous to our various JS/TS projects that use an NPM base image. For example, GCV's Dockerfile uses the NPM image and installs the version of Angular it needs at build time using the package.json file. What do you think?

[nathanweeks]

Thanks for the PR @nathanweeks. I find it interesting that the bretfisher images don't have versions. It has me thinking that maybe Jekyll shouldn't be provided by the image in the first place, i.e. we want the Jekyll version installed in containers to match what's in the Gemfile and having it preinstalled via a third-party image undermines this. Perhaps we should just use a ruby base image and have Jekyll installed by the Gemfile, either in our own Dockerfile or directly in docker-compose.yml. This would be analogous to our various JS/TS projects that use an NPM base image. For example, GCV's Dockerfile uses the NPM image and installs the version of Angular it needs at build time using the package.json file. What do you think?

There's sort of an implicit jekyll versioning in the CalVer-tagged images (https://hub.docker.com/r/bretfisher/jekyll-serve/tags), as Jekyll releases are few & far between. Regardless, though, I agree that would be fine to define a custom Dockerfile (or dockerfile_inline section in the Compose file) that, e.g., does a bundle install on build.

[alancleary]

I agree that would be fine to define a custom Dockerfile (or dockerfile_inline section in the Compose file) that, e.g., does a bundle install on build.

Now that I think about it, a Dockerfile is probably the way to go so we can build/use the container without Docker Compose. Although there should definitely still be a compose.yml file. Do you mind tackling this? I seem to be destined to endlessly squashing bugs as I try and get the 1.0.0 release out the door!

[nathanweeks]

Minimal example ruby:3.2 based Dockerfile that installs dependencies specified in Gemfile(.lock) & compose.yml (renamed per Compose spec recommendations) that implements separate services for building (only) and building/serving the site. The build-only could be used, e.g., for CI/CD workflows outside of GitHub Actions.

FWIW, I removed jekyll from the starter site's Gemfile, as it's already listed as a dependency in jekyll-theme-legumeinfo's gemspec, but it can be retained in the Gemfile if preferred.

[alancleary]

Thanks for making those changes @nathanweeks. I think the Dockerfile should have a default entrypoint. What do you think of having jekyll serve as the default since that's the default service in the compose.yml file?

Regarding specifying jekyll as a dependency in the site or theme, I wasn't sure what the best practice is here so I had a look at the Jekyll Themes documentation. This makes me think the theme should specify a range of Jekyll versions it's compatible with (like in the minima theme) and the starter site should specify a specifc Jekyll version it wants installed, e.g. ~4.3. What do you think?

[nathanweeks]

Thanks for making those changes @nathanweeks. I think the Dockerfile should have a default entrypoint. What do you think of having jekyll serve as the default since that's the default service in the compose.yml file?

Adding bundle exec jekyll serve as the ENTRYPOINT would be fine.

Regarding specifying jekyll as a dependency in the site or theme, I wasn't sure what the best practice is here so I had a look at the Jekyll Themes documentation. This makes me think the theme should specify a range of Jekyll versions it's compatible with (like in the minima theme) and the starter site should specify a specifc Jekyll version it wants installed, e.g. ~4.3. What do you think?

Sounds reasonable. I've updated the PR to implement both; please let me know if this wasn't quite what was intended.

[alancleary]

That all looks good. One final question: should the Dockerfile also expose the ports 4000 and 35729 so there's also a default port if none is specified?

[nathanweeks]

That all looks good. One final question: should the Dockerfile also expose the ports 4000 and 35729 so there's also a default port if none is specified?

EXPOSE is usually treated as metadata-only by container engines (though I'm aware of at least one PaaS---Cloud Foundry---that uses it if it's specified), but it's fine to add for documentation/information purposes. I'll push a commit to do so.

[nathanweeks]

Actually, I guess only port 4000 would be exposed with the default bundle exec jekyll serve entrypoint (without --livereload). Is that OK?

[nathanweeks]

Alternatively, the value of the jekyll service's command property (--incremental --livereload --profile --trace --host 0.0.0.0) could be moved to the ENTRYPOINT directive, and EXPOSE 35729 added. I have no strong preference either way.