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

OdbBackend: initial groundwork #942

Merged
merged 1 commit into from
Oct 12, 2019
Merged

OdbBackend: initial groundwork #942

merged 1 commit into from
Oct 12, 2019

Conversation

ddevault
Copy link
Contributor

@ddevault ddevault commented Sep 16, 2019

The rough plan is to make the OdbBackend type a fairly straightforward mirror of the odb_backend in libgit2. I'll then be adding LooseOdbBackend and PackOdbBackend, and MempackOdbBackend, which will inherit from OdbBackend and set the underlying odb_backend pointer, which should Just Werk based on the approach I have set out so far.

The more interesting case is that of user-created OdbBackend implementations. I'm not really sure what to do for this yet. The first inch of this work is done in the form of the read call raising NotImplementedException if the C implementation is NULL, then through some kind of magic I assume I'll be able to set these to function pointers for some kind of proxy into Python code with some kind of magic base class. Or something. ¯\_(ツ)_/¯

"Read raw object data from this odb backend.\n");

PyObject *
OdbBackend_read(OdbBackend *self, PyObject *py_hex)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note, in keeping with the libgit2 API design, I have no intention of refactoring Odb_read to go through this code for the sake of DRY.


/* XXX: This assumes the default libgit2 allocator is in use and will
* probably segfault and/or destroy the universe otherwise */
free(data);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

💩

src/odb.c Outdated Show resolved Hide resolved
src/odb.c Show resolved Hide resolved
@jdavid
Copy link
Member

jdavid commented Sep 20, 2019

looks good
just made a couple of small comments regarding error checking and handling

@ddevault
Copy link
Contributor Author

I think the basics are in good shape here. I'm going to implement the rest of the backend interface next and then figure out how to make new backends in Python in a future pull request.

src/odb.c Outdated Show resolved Hide resolved
src/odb_backend.c Outdated Show resolved Hide resolved
src/odb_backend.c Outdated Show resolved Hide resolved
src/odb_backend.c Show resolved Hide resolved
src/odb_backend.c Outdated Show resolved Hide resolved
src/odb_backend.c Outdated Show resolved Hide resolved
@jdavid
Copy link
Member

jdavid commented Oct 9, 2019

I've fixed the build errors in AppVeyor, now there're some test failures related to this work.

@ddevault ddevault changed the title [WIP] OdbBackend: initial groundwork OdbBackend: initial groundwork Oct 11, 2019
@ddevault
Copy link
Contributor Author

I've addressed the feedback and I think that this is mergable, except for the build failure. I looked into it and I don't really understand why it's occuring. By my reckoning, this should affect a lot of tests. Can you take a look and tell me what you think?

@jdavid jdavid merged commit 6bb2fa2 into libgit2:master Oct 12, 2019
@jdavid
Copy link
Member

jdavid commented Oct 12, 2019

We've seen these errors before.

Looking for a little bit I've found commit 00dd78b which fixed 7 errors (6 of them were PermissionErrors): https://ci.appveyor.com/project/techtonik/pygit2/build/1.0.12/tests

Commit af983b9 is likely related as well.

@ddevault ddevault deleted the odb branch October 12, 2019 13:29
@ddevault
Copy link
Contributor Author

Thanks!

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.

None yet

2 participants