Scripts should handle Docker-related errors more gracefully

[Alhadis]

Problem Description (copied from #4054)

I ran into a few issues when testing an update to the add-grammar script:

1. Docker wasn't installed, so I had to sudo apt-get install docker

2. Docker still wasn't installed, so I had to double-check what the hell the docker package was, and found out the correct name is actually docker-ce (??!)

3. The correct Docker was installed, but it complained about inadequate permissions:

Warning: failed to get default registry endpoint from daemon (Got permission denied while trying to connect to the Docker daemon socket at unix:///var/run/docker.sock: Get http://%2Fvar%2Frun%2Fdocker.sock/v1.35/info: dial unix /var/run/docker.sock: connect: permission denied). Using system default: https://index.docker.io/v1/
Got permission denied while trying to connect to the Docker daemon socket at unix:///var/run/docker.sock: Post http://%2Fvar%2Frun%2Fdocker.sock/v1.35/images/create?fromImage=linguist%2Fgrammar-compiler&tag=latest: dial unix /var/run/docker.sock: connect: permission denied

This makes for some pretty unfriendly UX; the scripts should ideally be updated to deal with these hurdles. Even bailing early in add-grammar would be preferable to bailing halfway, because each of the above three errors left the user with half-registered grammar modules...

Bear in mind I've never used Docker before, and I'm largely ignorant to how/why it's used. So this presents a pretty good example of how The Average User might feel when preparing a PR for adding a new language to Linguist.

[Alhadis]

Something else which could be improved in the UX area: installing dependencies. From a fresh Ubuntu install:

λ sudo gem install github-linguist
Fetching: charlock_holmes-0.7.5.gem (100%)
Building native extensions.  This could take a while...
ERROR:  Error installing github-linguist:
    ERROR: Failed to build gem native extension.

    current directory: /var/lib/gems/2.3.0/gems/charlock_holmes-0.7.5/ext/charlock_holmes
/usr/bin/ruby2.3 -r ./siteconf20180313-5065-1seejfp.rb extconf.rb
mkmf.rb can't find header files for ruby at /usr/lib/ruby/include/ruby.h

extconf failed, exit code 1

Gem files will remain installed in /var/lib/gems/2.3.0/gems/charlock_holmes-0.7.5 for inspection.
Results logged to /var/lib/gems/2.3.0/extensions/x86_64-linux/2.3.0/charlock_holmes-0.7.5/gem_make.out

I had to Google what it was even referring to, and an SO question provided the answer:

Modern era update [...] In case that you are using ruby 2.0 or 2.2:

sudo apt-get install ruby-dev
sudo apt-get install ruby2.0-dev
sudo apt-get install ruby2.2-dev
sudo apt-get install ruby2.3-dev

or, generic way:

sudo apt-get install ruby`ruby -e 'puts RUBY_VERSION[/\d+\.\d+/]'`-dev

@lildude Should README.md be updated to account for this? Or should this be filed to brianmario/charlock_holmes?

[Alhadis]

Another hurdle in the installation process, solved by Googling error feedback and finding this answer:

sudo apt-get install zlib1g-dev

Which brings my total command history to all this:

λ sudo gem install github-linguist
*ERROR*
λ sudo apt-get install ruby-dev
λ sudo gem install github-linguist
*ERROR*
λ sudo apt-get install libicu-dev
λ sudo gem install github-linguist
*ERROR*
λ sudo apt-get install cmake pkg-config libicu-dev
λ sudo gem install github-linguist
*ERROR*
λ sudo apt-get install zlib1g-dev
λ sudo gem install github-linguist

... I don't know whether this should be addressed by Linguist, or by each individual Gem that it depends on (since there are dependencies of dependencies being required here...)

[lildude]

@lildude Should README.md be updated to account for this? Or should this be filed to brianmario/charlock_holmes?

I think it might be pertinent to document it in our own README as we document the requirement for the dependencies of things like rugged and charlock_holmes. The zlib1g-dev is new to me.

I'm sorting out the docker side of things in a PR I'm working on right now as adding a grammar is currently broken following the updating of Licensed.

[Alhadis]

I'm sorting out the docker side of things in a PR I'm working on right now as adding a grammar is currently broken following the updating of Licensed.

Already started. :wink:

Question, though: why is Docker even being used? I had to comment out the respective Docker-related lines in add-grammar in #4093, and I had literally zero problems running tests and everything else...

[lildude]

Already started. 😉

Woah! You're doing a whole refactor... 🙇 I'm going for the quick-n-nasty "whoopsie, we missed this spot" fix 😁

Question, though: why is Docker even being used? I had to comment out the respective Docker-related lines in add-grammar in #4093, and I had literally zero problems running tests and everything else...

To quote @vmg in https://github.com/github/linguist/pull/3915:

The compiler is being distributed as a Docker image, linguist/grammars-compiler:latest on Docker.io. The image is self-contained (duh) and includes all the dependencies, most notably NPM and the CSON parser it needs to run. package.json has been removed from this repository because eek.

We've already got dependency hell as you've encountered and this extracts some of that hell for us.

[Alhadis]

Woah! You're doing a whole refactor... :bowing_man:

I'm adding support for rolling back filesystem changes in case of an error. I'm refactoring things to use an object-oriented approach, to save the script from becoming spaghetti-code. :wink:

We've already got dependency hell as you've encountered and this extracts some of that hell for us.

Understood. However, I've added a switch that dictates whether Docker is used for compiling the grammar or not. It's off by default (I was going to bring that up for discussion in the resulting PR), but I can remove it if you want (or invert the logic so it disables Docker rather than enables).

[lildude]

I'm pretty sure we need the grammar compiler to always run when adding or replacing grammars, right @vmg?

[Alhadis]

Oh! Another thing: I had to install the golang package when adding the grammar and running tests... and that compiled things without any issues at all.

Still unsure why Docker hates me but likes everyone else. :cry:

[Alhadis]

Actually, I think I'm gonna refactor script/list-grammars as well, so all our helper scripts are referencing the same classes. Half of the GrammarList class is leaning that way already, so it only makes sense to collate everything. :grinning:

I'm sticking these under a new directory at script/helpers, unless you prefer another location.

[lildude]

I'm sticking these under a new directory at script/helpers, unless you prefer another location.

It's as good a place as any.

[stale[bot]]

This issue has been automatically marked as stale because it has not had activity in a long time. If this issue is still relevant and should remain open, please reply with a short explanation (e.g. "I have checked the code and this issue is still relevant because ___."). Thank you for your contributions.

[stale[bot]]

This issue has been automatically closed because it has not had activity in a long time. Please feel free to reopen it or create a new issue.