googlearchive/git-rv

Unable to export due to commit message

Closed this issue · 4 comments

Context: I had not pushed my changes merged from upstream, so my fork repository had some commits that were not intended for the code review. Due to one of them, git-rv export just fails.

Traceback (most recent call last):
  File "/usr/lib/python2.7/runpy.py", line 162, in _run_module_as_main
    "__main__", fname, loader, pkg_name)
  File "/usr/lib/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "__main__.py", line 28, in <module>
  File "__main__.py", line 23, in main
  File "export.py", line 218, in callback
  File "export.py", line 113, in __init__
  File "export.py", line 168, in __get_commit_message_parts
  File "utils.py", line 456, in get_user_commit_message_parts
  File "utils.py", line 358, in get_commit_message_parts
utils.GitRvException: Commit message:
'- strings are now assumed to always be Unicode\nThis fixes a bug where this caused a crash:\n\nLANG=C bin/logstash agent -e \'filter { mutate { replace => { "message"\n=> "\xe2\x98\xb9" } } } output { stdout { codec => json } }\'\n\nThe problem was that LANG=C told ruby\'s eval() what the encoding of\nstrings was by default, and since logstash requires UTF-8/unicode\nelswhere, it makes sense to require it for the config file as well.'
does not begin with the subject:
'- strings are now assumed to always be Unicode This fixes a bug where this caused a crash:'.

These are the recent log messages (the one starting with "- strings" seem to have caused the issue)

commit 522d8b029e8416dcbac795297dbc1a841b11407e
Author: Rodrigo De Castro <rdc@google.com>
Date:   Thu Sep 12 14:15:29 2013 -0700

    Add first version of BQ output plugin.

commit 29de30745138ddcb69a2b45b8ebf3e5a1c39b58a
Author: Jordan Sissel <jls@semicomplete.com>
Date:   Wed Sep 11 11:02:20 2013 -0700

    .

commit 7ca972e8d11337142fd841e6f175cd3f744b76e5
Author: Jordan Sissel <jls@semicomplete.com>
Date:   Mon Sep 9 11:01:30 2013 -0700

    - strings are now assumed to always be Unicode
    This fixes a bug where this caused a crash:

    LANG=C bin/logstash agent -e 'filter { mutate { replace => { "message"
    => "☹" } } } output { stdout { codec => json } }'

    The problem was that LANG=C told ruby's eval() what the encoding of
    strings was by default, and since logstash requires UTF-8/unicode
    elswhere, it makes sense to require it for the config file as well.

commit 45ac715e36e6b0074e7233d709eefee28b50d650
Author: Jordan Sissel <jls@semicomplete.com>
Date:   Mon Sep 9 07:35:42 2013 -0700

    - fix docs

commit f1d6d1d71cef10067180ef2d2fac401d6356483c
Author: Jordan Sissel <jls@semicomplete.com>
Date:   Sun Sep 8 21:01:47 2013 -0700

    - swear less, and never in documentation.

First, thank you @RDCastro for the interest in git-rv!

So this is not necessarily a bug, but worth discussing.

The message says that the full commit message disagrees with the subject.

The full message is

'- strings are now assumed to always be Unicode\nThis fixes...

whereas the subject is

'- strings are now assumed to always be Unicode This fixes a bug where 

The reason this occurs is because you don't have a blank line in your commit between the subject and the message, e.g.

'- strings are now assumed to always be Unicode
This fixes...

instead of

'- strings are now assumed to always be Unicode
This fixes...

This update happened in 7180948 and the related bug discussion may illuminate why it was done.

Essentially, the output of git log -1 --pretty=%s will give the subject if you have no break between your first and second line it turns the new line into a space. That space remains as a newline in the output of git log -1 --pretty=%B.

So this error message is git-rv "encouraging" you to have more canonical-looking commit messages.

This is not actually my commit message (it's from an upstream's commit), but it seems that if the goal is to encourage canonical, it could display a warning with the suggested message and not block the review.

Even if it returns an error, we could have an option to force the review even with a non-canonical message.

Ah yes, I meant to mention something similar in my much too long reply.

This is closely related to #48

Mind if I merge this into that bug? Essentially, if there are multiple commits, git-rv wants to give you a nice prompt to pick one of them. However, if ANY of those commits isn't formatted correctly, the whole export fails. This of course is nonsense, so the solution would be to have a better failure case for such commits.

Sound good? Am I missing something?

Yes, please merge them as they are very related.