Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make-deb fails if the latest_git_commit contains unicode #13

Open
jobevers opened this issue Jul 5, 2016 · 4 comments
Open

make-deb fails if the latest_git_commit contains unicode #13

jobevers opened this issue Jul 5, 2016 · 4 comments

Comments

@jobevers
Copy link

jobevers commented Jul 5, 2016

If the latest git commit message has unicode in it, this line https://github.com/nylas/make-deb/blob/master/make_deb/__init__.py#L95 will throw a UnicodeDecode error.

jobevers added a commit to lbryio/lbry-sdk that referenced this issue Jul 5, 2016
Ran into a bug in make-deb that fails when there is utf-8 characters
in the git commit message. nylas/make-deb#13.

I wrote a patch to fix the bug and am installing that version until the
make-deb maintainers incorporate it
@msabramo
Copy link

Hmmm, I'm not seeing a problem when using git master. I did a commit with a Unicode snowman (U+2603) in the commit message:

[marca@marca-mac2 smbot2]$ git log -1 --oneline
84a40df Add ☃

[marca@marca-mac2 smbot2]$ rm -rf debian; make-deb && cat debian/rules
'debian' directory successfully placed at the root of your repository
#!/usr/bin/make -f

%:
    dh $@ --with python-virtualenv%

Am I not doing the right thing to reproduce the issue?

Or maybe the issue has already been fixed?

@msabramo
Copy link

It looks like 662c6aa fixed this issue. However, that code causes the program to fail on Python 3:

Traceback (most recent call last):
  File "/Users/marca/dev/git-repos/make-deb/bin/make-deb", line 8, in <module>
    reload(sys)

@msabramo
Copy link

OK, I reproduced it by removing 662c6aa, which worked around the problem.

I like #14 better. I would vote for reverting 662c6aa and merging #14.

@jobevers
Copy link
Author

Thanks for digging into that.

On Mon, Oct 31, 2016 at 12:46 PM, Marc Abramowitz [email protected]
wrote:

OK, I reproduced it by removing 662c6aa
662c6aa,
which worked around the problem.

I like #14 #14 better. I would
vote for reverting 662c6aa
662c6aa
and merging #14 #14.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#13 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ACRRXztWVZ9Nvu4ubOMTqXjcNlMBBuUQks5q5iligaJpZM4JFdgF
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants