sds / overcommit

A fully configurable and extendable Git hook manager
MIT License
3.92k stars 281 forks source link

Emoji in command output seems to break #781

Closed clemens closed 2 years ago

clemens commented 2 years ago

The latest release seems to have an issue when emojis (or more general: UTF-8 characters) are used in a command's output.

Here's the error i'm getting:

Checks for vulnerable versions of 3rd party packages.......[NpmAudit] FAILED
Hook raised unexpected error
"\xF0" on US-ASCII
/var/lib/gems/2.5.0/gems/overcommit-0.59.0/lib/overcommit/subprocess.rb:105:in `convert'
/var/lib/gems/2.5.0/gems/overcommit-0.59.0/lib/overcommit/subprocess.rb:105:in `to_utf8'
/var/lib/gems/2.5.0/gems/overcommit-0.59.0/lib/overcommit/subprocess.rb:55:in `spawn'
/var/lib/gems/2.5.0/gems/overcommit-0.59.0/lib/overcommit/utils.rb:187:in `execute'
/var/lib/gems/2.5.0/gems/overcommit-0.59.0/lib/overcommit/hook_context/base.rb:33:in `execute_hook'
/var/lib/gems/2.5.0/gems/overcommit-0.59.0/lib/overcommit/hook_loader/plugin_hook_loader.rb:85:in `run'
/var/lib/gems/2.5.0/gems/overcommit-0.59.0/lib/overcommit/hook/base.rb:47:in `block in run_and_transform'
/var/lib/gems/2.5.0/gems/overcommit-0.59.0/lib/overcommit/utils.rb:260:in `with_environment'
/var/lib/gems/2.5.0/gems/overcommit-0.59.0/lib/overcommit/hook/base.rb:47:in `run_and_transform'
/var/lib/gems/2.5.0/gems/overcommit-0.59.0/lib/overcommit/hook_runner.rb:162:in `run_hook'
/var/lib/gems/2.5.0/gems/overcommit-0.59.0/lib/overcommit/hook_runner.rb:98:in `block in consume'
/var/lib/gems/2.5.0/gems/overcommit-0.59.0/lib/overcommit/hook_runner.rb:94:in `loop'
/var/lib/gems/2.5.0/gems/overcommit-0.59.0/lib/overcommit/hook_runner.rb:94:in `consume'

The command looks like this:

NpmAudit:
  description: 'Checks for vulnerable versions of 3rd party packages'
  enabled: true
  command: ['bin/npm-audit']

The binary looks like this:

#!/bin/bash

$(npm bin)/better-npm-audit audit

The output from the command is this:

Screenshot 2022-04-29 at 11 52 56


I'm assuming this is related to https://github.com/sds/overcommit/pull/766.

OS: Debian 10 in Docker container overcommit version: 0.59.0 (it works fine in 0.58.0)

clemens commented 2 years ago

Here's how I've worked around it for now:

#!/bin/bash

$(npm bin)/better-npm-audit audit > /dev/null && echo "All good!"
Drowze commented 2 years ago

I debugged this a bit but I'm still not sure about the proper fix... It also doesn't help that #766 didn't add a test documenting the intended behaviour of to_utf8 method 🤔

Here's a minimal Dockerfile to reproduce the bug: _(note: the debian image on this Dockerfile does not define any locale; Encoding.locale_charmap returns ANSI_X3.4-1968 and the code breaks on gems/overcommit-0.59.0/lib/overcommit/subprocess.rb:105:in 'convert')_

FROM debian

RUN apt update -y && apt install -y ruby git && gem install bundler overcommit
WORKDIR some-dir

# setup a git repo with a utf-8 commit message
RUN git init && \
  git config --local user.name "Overcommit Tester" && \
  git config --local user.email "overcommit@example.com" && \
  git commit --allow-empty -m "🤝 UTF-8 text"

# add and install a pre-commit hook that checks a commit message
RUN echo "PreCommit:\n  CapitalizedSubject:\n    enabled: true" > .overcommit.yml && overcommit --install

# amend the commit to trigger the pre-commit hook; this raises an error
RUN git commit --amend --no-edit

If I understood it correctly, v0.59 breaks on utf8 strings for any unix system that does not define an utf8 locale.