Note that this does not remove the need to have to write data to disc before writing it into the object database. That step is simply behind the _fromchunks() function call.

Indeed - but it only needs to write it to disk once. Using fromworkdir involves writing it to disk twice: first to the working directory, and then to the objects database.

I whipped this together as a proof of concept (I'm relatively new to Python/C bindings). But I had the chance to sleep on it, and perhaps it would be cleaner to expose Blob_create_from_callback and then expose a pure Python implementation of Repository.create_blob_fromiobase that uses Blob_create_from_callback in its implementation. What do you think?

Using fromworkdir involves writing it to disk twice: first to the working directory, and then to the objects database.

Using _fromchunks() is the same. First we have to write to a temporary file and then to the object database (they both use the same function to perform the work).

It is not possible to write create an object without knowing the size ahead of time. Anything which does not ask for the size beforehand (or for a full buffer) must store the data somewhere else before writing to the odb.

Python already has ways of making arbitrary sources look like a buffer/stream, so I'd rather use a readable stream interface rather than callbacks.

From the discussion I understand you cannot save writing the data to disk.

I have made some comments about the code, but not exhaustively, you need to thoroughly check errors and possible return values.

When @carlosmn says I'd rather use a readable stream interface rather than callbacks I understand he prefers the original proposal in this PR, and not Blob_create_from_callback. And I agree. Just fix the programming errors and it is good to me for merging.

@jdavid I addressed the review comments.

However I looked at libgit2 after @carlosmn's comments, and it does indeed create a temporary file. As a result, I'm not so sure about the relevance of this pull request. I will leave it to you guys to decide whether or not it should be merged, and happily accept a refusal.

Even if this does not speed up anything, it is still a nice API addition to be able to create a blob from a file like object.

But the unit tests are not passing with Python 2, that needs to be fixed.

Before I said supporting IOBase is enough, and that's right. But you may want to make this PR even better by supporting Python 2 files. Thought these files do not inherit from IOBase, and they don't have the readable method. If you wish to make this change, then the method should be renamed to the more general create_from_file. Otherwise, you can leave the PR the way it is, supporting Python 2 files may be the object of a future pull request.

@jdavid thanks for the kind feedback.

Looking at the link failure in the tests (and based on my extremely limited experience with Python/C interaction), is it even possible to implement it with this strategy in a way that is compatible with Python 2 and Python 3? Reading through the documentation implies this should rather be implemented at the python layer.

In fact, it would be far simpler to implement a create_blob_fromfile that worked correctly with file-like objects on Python 2 and IOBase implementations on Python 3 by doing most of the work in python code.

Okey, let make it simple:

• Rename the method to create_blob_fromfile
• Drop the type check, since this does not work on Python 2 (and pypy), we will stick to duck-typing by calling readable()
• Change METH_VARARGS to METH_O
• Add a TODO comment to the source, something like TODO Support Python 2 files
• Add a TODO comment to the source, something like TODO Type check PyIOBase_Type (only works on Python 3)

Thanks!