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

Write commit messages to temporary files #36

Closed
jaraco opened this issue Jan 16, 2014 · 9 comments
Closed

Write commit messages to temporary files #36

jaraco opened this issue Jan 16, 2014 · 9 comments

Comments

@jaraco
Copy link
Contributor

jaraco commented Jan 16, 2014

When trying to invoke bumpversion using Windows on Python 3, it fails with this traceback:

C:\Users\jaraco\yg\pan [default tip]> bumpversion.pya minor
Traceback (most recent call last):
  File "c:\python\scripts\bumpversion.pya", line 9, in <module>
    load_entry_point('bumpversion==0.3.8', 'console_scripts', 'bumpversion')()
  File "C:\Program Files\Python34\lib\site-packages\bumpversion-0.3.8-py3.4.egg\bumpversion\__init__.py", line 579, in main
  File "C:\Program Files\Python34\lib\site-packages\bumpversion-0.3.8-py3.4.egg\bumpversion\__init__.py", line 163, in commit
  File "C:\Program Files\Python34\lib\subprocess.py", line 605, in check_output
    with Popen(*popenargs, stdout=PIPE, **kwargs) as process:
  File "C:\Program Files\Python34\lib\subprocess.py", line 848, in __init__
    restore_signals, start_new_session)
  File "C:\Program Files\Python34\lib\subprocess.py", line 1078, in _execute_child
    args = list2cmdline(args)
  File "C:\Program Files\Python34\lib\subprocess.py", line 661, in list2cmdline
    needquote = (" " in arg) or ("\t" in arg) or not arg
TypeError: Type str doesn't support the buffer API

This issue is due in part to the encoding of messages as bytes even though subprocess expects them to be strings.

Simply removing the encoding works around the TypeError and the program runs to completion, but the commit message has a "?" character in place of the "→". On further investigation, it appears the honoring of that character may be a bug with mercurial and not bumpversion or subprocess.

I'm able to run this simple script under Python 3 and it outputs the message properly:

# -*- coding: utf-8 -*-

import subprocess

msg = "A \u2192 B"
msg2 = "A → B"
assert msg == msg2
subprocess.call(["Powershell", "-C", "echo '" + msg + "'"])

Furthermore, I've confirmed that if I pass the msg to another Python 3 subprocess, the character in sys.argv is in fact unicode. If I pass the msg to a Python 2 subprocess, the character is lost and replaced by a '?'.

Overall, I believe the proper thing to do is remove the encoding of the string before passing to subprocess.

However, I suspect that will only work on Python 3 due to Python 1759845, so perhaps bumpversion should only do the encoding on Python 2.

@peritus
Copy link
Owner

peritus commented Jan 16, 2014

Ok, wow. Now it's python 3.4 and windows ;)

I currently test this piece of software on travis against python 3.2 and 3.3, but on that's on linux. If you're able to execute the tests in that environment (should be pretty standard py.test + tox), i'd be very interested in any logs.

@peritus
Copy link
Owner

peritus commented Jan 16, 2014

Also, message is configurable. If you like to use it productively in that environment, you could just set a message that reads "->" instead of "→".

If you manage do send a pull request that doesn't break py2/3 on linux, i'd be happy to merge that.

@jaraco
Copy link
Contributor Author

jaraco commented Jan 16, 2014

Indeed the custom message is a simple workaround. Perhaps we'll use that for now. As time permits, I'll work on a patch. Don't feel obliged to work on this - I more wanted to report it so the issue was captured.

@peritus
Copy link
Owner

peritus commented Jan 26, 2014

Few more thoughts on this:

In the tests at https://github.com/peritus/bumpversion/blob/master/tests.py#L16 I do

environ['HGENCODING'] = 'UTF-8'

but stayed away from including that outside of the tests for some reason I don't recall anymore.

On the other hand, I could write the message to an UTF-8 encoded file then tell git/mercurial to read the message from there. That should be the safest on all platforms.

@jaraco
Copy link
Contributor Author

jaraco commented Jan 26, 2014

Given that Mercurial is trapped on Python 2 and Python 2 is unlikely to support proper Unicode command-line arguments, perhaps writing the message to a file would in fact be a suitable workaround. Great idea.

@jaraco
Copy link
Contributor Author

jaraco commented Feb 12, 2014

Very nice. Thank you.

@peritus
Copy link
Owner

peritus commented Feb 12, 2014

Somehow this breaks travis but not my vagrant environment. Need to investigate.

@peritus peritus reopened this Feb 12, 2014
@jaraco
Copy link
Contributor Author

jaraco commented Feb 12, 2014

Looks like an issue with the git config and no initialized username. Perhaps the environment changed and this commit isn't relevant.

@peritus
Copy link
Owner

peritus commented Feb 14, 2014

Travis is green again: aa89889

@peritus peritus closed this as completed Feb 14, 2014
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