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

Fixed bug related to dirty workdir #28

Merged
merged 1 commit into from
Dec 13, 2013
Merged

Fixed bug related to dirty workdir #28

merged 1 commit into from
Dec 13, 2013

Conversation

dbrgn
Copy link
Contributor

@dbrgn dbrgn commented Nov 29, 2013

  • describe_out[-1] may contain a newline, therefore the strip() is necessary.
  • is does identity comparison. You need == there.

@peritus
Copy link
Owner

peritus commented Nov 29, 2013

Thank you for the fix. Just out of curiosity, in which situation is this necessary ?

@dbrgn
Copy link
Contributor Author

dbrgn commented Nov 29, 2013

In the situation where the entire program crashes because my describe_out[-1] looks like dirty\n ;)

Because then it's not stripped away and the distance_to_latest_tag assignment tries to convert a commit hash to an integer.

@peritus
Copy link
Owner

peritus commented Dec 13, 2013

I want to understand how describe_out[-1] could ever look like dirty\n — is it a specific git configuration or an old version or something ? Do you have a stacktrace or a way to reproduce it ?

Also, I'd like to build a regression test against this before merging.

@dbrgn
Copy link
Contributor Author

dbrgn commented Dec 13, 2013

Well, you're running a bash command in the background. Usually any command output appends a newline at the end. Might be a git version issue.

$ git --version
git version 1.8.5.1
$ uname -a
Linux t410 3.12.3-1-ARCH #1 SMP PREEMPT Wed Dec 4 21:45:42 CET 2013 x86_64 GNU/Linux

And describe_out:

$ python
Python 2.7.6 (default, Nov 26 2013, 12:52:49) 
[GCC 4.8.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import subprocess
>>> describe_out = subprocess.check_output([
... "git",
... "describe",
... "--dirty",
... "--tags",
... "--long",
... "--abbrev=40",
... "--match=v*",
... ], stderr=subprocess.STDOUT)
>>> describe_out
'v0.3.6-1-gd9527ff2b3d409abec266ac99dbacc5faed2e1b2\n'

@peritus
Copy link
Owner

peritus commented Dec 13, 2013

Thanks, that explains it.

That line of code is never reached because at the moment at dirty workdir aborts bumpversion very early on. I eventually want to loosen that restriction, so: thanks for finding this before anyone could execute these lines.

peritus added a commit that referenced this pull request Dec 13, 2013
Fixed bug related to dirty workdir
@peritus peritus merged commit 1979798 into peritus:master Dec 13, 2013
@dbrgn
Copy link
Contributor Author

dbrgn commented Dec 13, 2013

Actually it was reached (when my directory was dirty), otherwise it wouldn't have crashed bumpversion. But I never investigated how it reached that line.

But in any case, this is now fixed and shouldn't be a problem anymore.

@peritus
Copy link
Owner

peritus commented Dec 13, 2013

Right. Thanks again.

Sent from my pocket calculator

On 13 Dec 2013, at 15:45, Danilo Bargen [email protected] wrote:

Actually it was reached (when my directory was dirty), otherwise it wouldn't have crashed bumpversion. But I never investigated how it reached that line.

But in any case, this is now fixed and shouldn't be a problem anymore.


Reply to this email directly or view it on GitHub.

@peritus
Copy link
Owner

peritus commented Jan 10, 2014

This is now part of v0.3.8. Running pip install --upgrade bumpversion should get you there.

Also: I'm sorry for realising too late that we could have met at 30C3 to nerd out about bumping versions :(

@dbrgn
Copy link
Contributor Author

dbrgn commented Jan 10, 2014

Great :)

Also: I'm sorry for realising too late that we could have met at 30C3 to nerd out about bumping versions :(

Ah. Next time maybe :)

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

Successfully merging this pull request may close these issues.

2 participants