Changeset View
Standalone View
swh/perfecthash/build.py
- This file was added.
| # Copyright (C) 2021 The Software Heritage developers | |||||
ardumont: same remark about headers. | |||||
| # See the AUTHORS file at the top-level directory of this distribution | |||||
| # License: GNU General Public License version 3, or any later version | |||||
| # See top-level LICENSE file for more information | |||||
| from cffi import FFI | |||||
| ffibuilder = FFI() | |||||
| # cdef() expects a single string declaring the C types, functions and | |||||
| # globals needed to use the shared object. It must be in valid C syntax. | |||||
| # | |||||
| # The following is only the necessary part parsed by cffi to generate python bindings. | |||||
| # | |||||
| ffibuilder.cdef( | |||||
| """ | |||||
| typedef struct shard_t shard_t; | |||||
| shard_t* shard_init(const char* path); | |||||
| int shard_destroy(shard_t* shard); | |||||
| int shard_create(shard_t* shard, size_t objects_count); | |||||
Done Inline ActionsIsn't this ^ redundant with the header file hash.h? If not, i gather that's some of those tiny details about using cffi stuff that escapes me ;) ardumont: Isn't this ^ redundant with the header file `hash.h`?
If not, i gather that's some of those… | |||||
Done Inline ActionsIt is not redundant and that confused me too at first :-) This is parsed by cffi and it does python binding based on it. Only what needs to be exposed is copy/pasted here. There would be a way to avoid the duplication but I'm not sure it's worth the effort: if there is an inconsistency it shows at compile time: no risk of divergence. dachary: It is not redundant and that confused me too at first :-) This is parsed by cffi and it does… | |||||
Done Inline ActionsThen, the right effort needed is just to update a bit the existing comment, one extra sentence like this would do: The following is only the necessary part parsed by cffi to generate python bindings. What do you think? ardumont: Then, the right effort needed is just to update a bit the existing comment, one extra sentence… | |||||
| int shard_object_write(shard_t* shard, const char* key, | |||||
| const char* object, size_t object_size); | |||||
| int shard_save(shard_t* shard); | |||||
| int shard_load(shard_t* shard); | |||||
| int shard_lookup_object_size(shard_t *shard, const char *key, size_t *object_size); | |||||
Done Inline Actionsis there some funky indent business here? douardda: is there some funky indent business here? | |||||
Done Inline ActionsYeah, fixed! dachary: Yeah, fixed! | |||||
| int shard_lookup_object(shard_t *shard, char *object, size_t object_size); | |||||
| """ | |||||
| ) | |||||
| ffibuilder.set_source( | |||||
| "_hash_cffi", | |||||
Done Inline ActionsShouldn't this be prefixed by swh.perfecthash (and the module name be swh.perfecthash._hash_cffi)? olasd: Shouldn't this be prefixed by `swh.perfecthash` (and the module name be `swh.perfecthash. | |||||
Done Inline ActionsI'm unclear about the requirements of ffibuilder: it works as it is therefore I conclude it should not be different. But I can try with prefixing the module with swh.prefecthash if you think it is important. dachary: I'm unclear about the requirements of ffibuilder: it works as it is therefore I conclude it… | |||||
Done Inline ActionsI mean, this looks like this is creating a "non-namespaced" extension module called _hash_cffi (which would be installed at the root of the python modules installation path), which sounds wrong. olasd: I mean, this looks like this is creating a "non-namespaced" extension module called… | |||||
Done Inline ActionsGood catch: it is exactly like that and I overlooked it. I'm surprised it did not just break with tox. Fixed! dachary: Good catch: it is exactly like that and I overlooked it. I'm surprised it did not just break… | |||||
Not Done Inline Actionsnitpick: shouldn't it be called swh.perfecthash._hash? that's how the stdlib does it (eg. https://archive.softwareheritage.org/swh:1:cnt:92a71e14c480bdcab1fd43ecbfcd70bb6e23f0d5;origin=https://github.com/python/cpython;visit=swh:1:snp:3326130e6e26bc726591d44c04ec4d8d77cb20e8;anchor=swh:1:rev:fa26245a1c1aa938cce391348d6bd879da357522;path=/Lib/random.py;lines=58 ) vlorentz: nitpick: shouldn't it be called `swh.perfecthash._hash`? that's how the stdlib does it (eg. | |||||
Done Inline ActionsInteresting. Since you're asking my preference would be to keep the name as it is but that's because I'm lazy. I'd be happy to change it if you think it's better though: I'm not that lazy ;-) dachary: Interesting. Since you're asking my preference would be to keep the name as it is but that's… | |||||
Not Done Inline Actionsheh, it doesn't matter vlorentz: heh, it doesn't matter | |||||
| """ | |||||
| #include "swh/perfecthash/hash.h" | |||||
| """, | |||||
| sources=["swh/perfecthash/hash.c"], | |||||
| include_dirs=["."], | |||||
| libraries=["cmph"], | |||||
| ) # library name, for the linker | |||||
| if __name__ == "__main__": | |||||
| ffibuilder.compile(verbose=True) | |||||
same remark about headers.