Skip to content

Add missing newline when writing a symbolic ref. #490

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

Merged
merged 2 commits into from
Jul 23, 2016

Conversation

bertwesarg
Copy link
Contributor

No description provided.

@@ -313,7 +313,7 @@ def set_reference(self, ref, logmsg=None):

lfd = LockedFD(fpath)
fd = lfd.open(write=True, stream=True)
fd.write(write_value.encode('ascii'))
fd.write(write_value.encode('ascii') + '\n')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Python 3 compat, prefer to use b'\n' here, since this should be a byte, not a unicode string.

@Byron
Copy link
Member

Byron commented Jul 23, 2016

@bertwesarg Thanks for your contribution. May I ask why this is needed ? When creating refs using plain git, I don't see newlines either.
In case this is a bug you are fixing, you could add a test that goes green with this patch.
Thanks for the clarification.

@Byron Byron added this to the v2.0.8 - Bugfixes milestone Jul 23, 2016
@bertwesarg
Copy link
Contributor Author

bertwesarg commented Jul 23, 2016

Hmm, while I still use an ancient git 2.1.4 it does add a newline:

$ git init .
$ hexdump -C .git/HEAD 
00000000  72 65 66 3a 20 72 65 66  73 2f 68 65 61 64 73 2f  |ref: refs/heads/|
00000010  6d 61 73 74 65 72 0a                              |master.|
00000017

And git master should still do this:

https://github.com/git/git/blob/master/refs/files-backend.c#L2740

Though I noticed this because the git prompt provided by git-prompt.sh did not worked anymore, because it uses the shell read builtin, which does not work if the file does not has a trailing newline:

https://github.com/git/git/blob/master/contrib/completion/git-prompt.sh#L433

$ __git_ps1 '%s\n'
master
$ truncate -s 22 .git/HEAD
$ __git_ps1 '%s\n'
$

@Byron
Copy link
Member

Byron commented Jul 23, 2016

Thanks for the clarification and the thorough research, great to have this contribution merged now! I feel a bit silly that I was unable to 'see' the newline at the end of the symbolic ref with vim, but will certainly put hexdump on my map.

@Byron Byron merged commit a4ad7ce into gitpython-developers:master Jul 23, 2016
@bertwesarg bertwesarg deleted the patch-1 branch July 24, 2016 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants