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

0.23.3 fails to compile on Fedora 23 i686 #597

Closed
pypingou opened this issue Feb 22, 2016 · 9 comments · Fixed by #599
Closed

0.23.3 fails to compile on Fedora 23 i686 #597

pypingou opened this issue Feb 22, 2016 · 9 comments · Fixed by #599

Comments

@pypingou
Copy link

I have recently been trying to update the Fedora package in F23 to 0.23.3 since Fedora already have libgit2 0.23.4 I thought this should work fine.

Build on x86_64 is fine, but i686 fails to build, or rather fails on the unit-tests, I would appreciate some help to figure out what's going on.

The build is at: http://koji.fedoraproject.org/koji/taskinfo?taskID=13024033
The log for i686 is at: https://kojipkgs.fedoraproject.org//work/tasks/4036/13024036/build.log

There is apparently something odd going on with encoding and somehow -dirty becomes \x08. No idea why though.

@pypingou
Copy link
Author

Note the code was added commit 57507982 and line which might be responsible may be: 99dfce9#diff-57507982bc2ad39925ca8f387e172b29R720 but I don't see anything wrong w/ it, so I wonder if the problem isn't lower down the stack, no idea.

@carlosmn
Copy link
Member

It looks like the string might not be kept alive long enough, so libgit2 ends up reading nonsense. This looks like a dupe of #592 though.

@pypingou
Copy link
Author

Indeed it's a dupe, should I close this one or do we prefer to close the other one?

@pypingou
Copy link
Author

Any idea on how we could help debugging this?

@carlosmn
Copy link
Member

There's looking at the code to see if we make sure we keep a reference to the unmanaged string. Then there's also a way to build python so it doesn't pool allocations and you can throw valgrind and friends at it.

@carlosmn
Copy link
Member

This should keep the pointer alive.

diff --git a/pygit2/repository.py b/pygit2/repository.py
index f8a6cad..d7ca58e 100644
--- a/pygit2/repository.py
+++ b/pygit2/repository.py
@@ -716,8 +716,10 @@ class Repository(_Repository):
                 format_options.abbreviated_size = abbreviated_size
             if always_use_long_format is not None:
                 format_options.always_use_long_format = always_use_long_format
+            dirty_ptr = None
             if dirty_suffix:
-                format_options.dirty_suffix = ffi.new('char[]', to_bytes(dirty_suffix))
+                dirty_ptr = ffi.new('char[]', to_bytes(dirty_suffix))
+                format_options.dirty_suffix = dirty_ptr

             buf = ffi.new('git_buf *', (ffi.NULL, 0))

@pypingou
Copy link
Author

thanks I'll test this patch :)

@pypingou
Copy link
Author

Worked perfectly, thanks! http://koji.fedoraproject.org/koji/taskinfo?taskID=13116069

@pypingou
Copy link
Author

So as far as I am concerned this can be closed as fixed, I could build 0.23.3 for Fedora 23, 24 and rawhide successfully on all arches (i686, x86_64 and arm).

If you submit the patch for inclusion, I can vouch that it fixed the unit-tests for us.

Many thanks for your help!

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 a pull request may close this issue.

2 participants