From d8ba4d0183b5d68ced1ba809590349519461640c Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Thu, 27 Feb 2025 15:29:51 +0100
Subject: [PATCH 01/24] Do not use -std=c++17 when compiling C code for
test_hash
---
swh/perfecthash/Makefile | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/swh/perfecthash/Makefile b/swh/perfecthash/Makefile
index a18f924..f914302 100644
--- a/swh/perfecthash/Makefile
+++ b/swh/perfecthash/Makefile
@@ -1,4 +1,5 @@
-CFLAGS=-D_FILE_OFFSET_BITS=64 -DHASH_DEBUG -Wall -I../.. -g -std=c++17 -fprofile-arcs -ftest-coverage
+CFLAGS=-D_FILE_OFFSET_BITS=64 -DHASH_DEBUG -Wall -I../.. -g -fprofile-arcs -ftest-coverage
+CXXFLAGS=$(CFLAGS) -std=c++17
LDFLAGS=-lcmph -lgtest -lpthread -lstdc++ -lstdc++fs -fprofile-arcs -ftest-coverage
test_hash: hash.o test_hash.o
--
GitLab
From 3e8a30c314142ec1d47cc427ac04e5ee5a886c88 Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Thu, 27 Feb 2025 15:31:01 +0100
Subject: [PATCH 02/24] extension: Replace %ld by %lu in string format for
unsigned long
otherwise the compiler may complain about it.
---
swh/perfecthash/hash.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/swh/perfecthash/hash.c b/swh/perfecthash/hash.c
index 4094fa9..b3ff9ae 100644
--- a/swh/perfecthash/hash.c
+++ b/swh/perfecthash/hash.c
@@ -64,12 +64,12 @@ int shard_close(shard_t *shard) {
int shard_seek(shard_t *shard, uint64_t offset, int whence) {
if (offset > INT64_MAX) {
- printf("shard_seek: %ld > %ld (INT64_MAX)", offset, INT64_MAX);
+ printf("shard_seek: %lu > %lu (INT64_MAX)", offset, INT64_MAX);
return -1;
}
int r = fseeko(shard->f, offset, whence);
if (r < 0)
- printf("shard_seek: fseeko(%p, %ld, %d): %s\n", shard->f, offset,
+ printf("shard_seek: fseeko(%p, %lu, %d): %s\n", shard->f, offset,
whence, strerror(errno));
return r;
}
@@ -84,7 +84,7 @@ uint64_t shard_tell(shard_t *shard) {
int shard_read(shard_t *shard, void *ptr, uint64_t size) {
uint64_t read;
if ((read = fread(ptr, 1, size, shard->f)) != size) {
- printf("shard_read: read %ld instead of %ld\n", read, size);
+ printf("shard_read: read %lu instead of %lu\n", read, size);
return -1;
}
return 0;
@@ -103,7 +103,7 @@ int shard_read_uint64_t(shard_t *shard, uint64_t *ptr) {
int shard_write(shard_t *shard, const void *ptr, uint64_t nmemb) {
uint64_t wrote;
if ((wrote = fwrite(ptr, 1, nmemb, shard->f)) != nmemb) {
- printf("shard_write: wrote %ld instead of %ld\n", wrote, nmemb);
+ printf("shard_write: wrote %lu instead of %lu\n", wrote, nmemb);
return -1;
}
return 0;
@@ -165,7 +165,7 @@ int shard_magic_save(shard_t *shard) {
*/
int shard_header_print(shard_header_t *header) {
-#define PRINT(name) debug("shard_header_print: " #name " %ld\n", header->name)
+#define PRINT(name) debug("shard_header_print: " #name " %lu\n", header->name)
PRINT(version);
PRINT(objects_count);
PRINT(objects_position);
@@ -199,7 +199,7 @@ int shard_header_load(shard_t *shard) {
#undef LOAD
shard_header_print(&shard->header);
if (shard->header.version != SHARD_VERSION) {
- printf("shard_header_load: unexpected version, got %ld instead of %d\n",
+ printf("shard_header_load: unexpected version, got %lu instead of %d\n",
shard->header.version, SHARD_VERSION);
return -1;
}
@@ -309,7 +309,7 @@ int shard_hash_create(shard_t *shard) {
int shard_index_save(shard_t *shard) {
shard->header.index_position =
shard->header.objects_position + shard->header.objects_size;
- debug("shard_index_save: index_position %ld\n",
+ debug("shard_index_save: index_position %lu\n",
shard->header.index_position);
assert(shard->header.index_position == shard_tell(shard));
cmph_uint32 count = cmph_size(shard->hash);
@@ -319,7 +319,7 @@ int shard_index_save(shard_t *shard) {
for (uint64_t i = 0; i < shard->index_offset; i++) {
cmph_uint32 h =
cmph_search(shard->hash, shard->index[i].key, SHARD_KEY_LEN);
- debug("shard_index_save: i = %ld, h = %d, offset = %ld\n", i, h,
+ debug("shard_index_save: i = %lu, h = %d, offset = %lu\n", i, h,
shard->index[i].object_offset);
assert(h < count);
memcpy(index[h].key, shard->index[i].key, SHARD_KEY_LEN);
@@ -337,7 +337,7 @@ int shard_index_save(shard_t *shard) {
int shard_hash_save(shard_t *shard) {
shard->header.hash_position =
shard->header.index_position + shard->header.index_size;
- debug("shard_hash_save: hash_position %ld\n", shard->header.hash_position);
+ debug("shard_hash_save: hash_position %lu\n", shard->header.hash_position);
cmph_dump(shard->hash, shard->f);
return 0;
}
@@ -415,7 +415,7 @@ int shard_find_object(shard_t *shard, const char *key, uint64_t *object_size) {
debug("shard_find_object: h = %d\n", h);
uint64_t index_offset =
shard->header.index_position + h * sizeof(shard_index_t);
- debug("shard_find_object: index_offset = %ld\n", index_offset);
+ debug("shard_find_object: index_offset = %lu\n", index_offset);
if (shard_seek(shard, index_offset, SEEK_SET) < 0) {
printf("shard_find_object: index_offset\n");
return -1;
@@ -430,7 +430,7 @@ int shard_find_object(shard_t *shard, const char *key, uint64_t *object_size) {
printf("shard_find_object: object_offset\n");
return -1;
}
- debug("shard_find_object: object_offset = %ld\n", object_offset);
+ debug("shard_find_object: object_offset = %lu\n", object_offset);
/* Has the object been deleted? */
if (object_offset == UINT64_MAX) {
return 1;
@@ -449,7 +449,7 @@ int shard_find_object(shard_t *shard, const char *key, uint64_t *object_size) {
printf("shard_find_object: object_size\n");
return -1;
}
- debug("shard_find_object: object_size = %ld\n", *object_size);
+ debug("shard_find_object: object_size = %lu\n", *object_size);
return 0;
}
@@ -466,7 +466,7 @@ int shard_hash_load(shard_t *shard) {
printf("shard_hash_load\n");
return -1;
}
- debug("shard_hash_load: hash_position %ld\n", shard->header.hash_position);
+ debug("shard_hash_load: hash_position %lu\n", shard->header.hash_position);
shard->hash = cmph_load(shard->f);
if (shard->hash == NULL) {
printf("shard_hash_load: cmph_load\n");
--
GitLab
From 353b0e5afc6a6baadfad5b2382ecdc2a10aee850 Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Thu, 27 Feb 2025 15:42:41 +0100
Subject: [PATCH 03/24] extension: initialize index entries as "deleted"
entries
A deleted entry is stored using \x00<x32> as key and UINT64_MAX as
value. Initialize all the entries in this index with this special value
to make it clearer these are unset entries (since the computed CMPH is
not really *minimal*, the general case will have a few empty index entries,
so make it explicit these are empty using the special "deleted" value).
---
swh/perfecthash/hash.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/swh/perfecthash/hash.c b/swh/perfecthash/hash.c
index b3ff9ae..08d61d3 100644
--- a/swh/perfecthash/hash.c
+++ b/swh/perfecthash/hash.c
@@ -313,9 +313,18 @@ int shard_index_save(shard_t *shard) {
shard->header.index_position);
assert(shard->header.index_position == shard_tell(shard));
cmph_uint32 count = cmph_size(shard->hash);
+ // Note that the 'count' computed by cmph is generally bigger than the
+ // number of objects (in other word, it can be a NOT *minimal* perfect hash
+ // map)", so we have to initialize the table of index entries with explicit
+ // "invalid" entries (aka {key=0x00, offset=MAX_INT})
debug("shard_index_save: count = %d\n", count);
shard->header.index_size = count * sizeof(shard_index_t);
shard_index_t *index = (shard_index_t *)calloc(1, shard->header.index_size);
+ // initialize all the index entries as "deleted" entries by default, the
+ // actual entries will be filled just below.
+ for (uint64_t i = 0; i < count; i++) {
+ index[i].object_offset = UINT64_MAX;
+ }
for (uint64_t i = 0; i < shard->index_offset; i++) {
cmph_uint32 h =
cmph_search(shard->hash, shard->index[i].key, SHARD_KEY_LEN);
--
GitLab
From b4efc6bc782f31b4c4eebe300166884c69ac1350 Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Thu, 27 Feb 2025 15:54:39 +0100
Subject: [PATCH 04/24] docs: give more details on the shard file format
and swap benchmarks and file format chapters in the index file.
---
docs/format.rst | 55 +++++++++++++++++++++++++++++++++++++++++++++++++
docs/index.rst | 2 +-
2 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/docs/format.rst b/docs/format.rst
index 69ec0e3..be4e7f9 100644
--- a/docs/format.rst
+++ b/docs/format.rst
@@ -8,3 +8,58 @@ The Read Shard has the following structure:
* bytes \[``objects_position``, ``index_position``\[: ``objects_count`` times the size of the object (``u_int64_t``) followed by the content of the object
* bytes \[``index_position``, ``hash_position``\[: An array of index entries. The size of the array is provided by ``cmph_size`` after building the hash function. An index entry is made of the key (of ``SHARD_KEY_LEN`` bytes) and the object position (``u_int64_t``) in the range \[``objects_position``, ``index_position``\[. If the object position is ``UINT64_MAX``, this means the object has been deleted.
* bytes \[``hash_position``, ...\[: The hash function, as written by ``cmph_dump``
+
+In more details:
+
++--------------------------+------+----------------------------+
+| Section | pos | description (length) |
++==========================+======+============================+
+| **SHARD_MAGIC** | 0 | SHARD_OFFSET_MAGIC (32) |
++--------------------------+------+----------------------------+
+| **header** | 32 | Header (56) |
++--------------------------+------+----------------------------+
+| ``version`` | | uint64_t (8) |
++--------------------------+------+----------------------------+
+| ``objects_count`` | | uint64_t (8) |
++--------------------------+------+----------------------------+
+| ``objects_position`` <op>| | uint64_t (8) |
++--------------------------+------+----------------------------+
+| ``objects_size`` | | uint64_t (8) |
++--------------------------+------+----------------------------+
+| ``index_position`` <ip> | | uint64_t (8) |
++--------------------------+------+----------------------------+
+| ``index_size`` | | uint64_t (8) |
++--------------------------+------+----------------------------+
+| ``hash_position`` <hp> | | uint64_t (8) |
++--------------------------+------+----------------------------+
+| **Objects** | <op> | |
++--------------------------+------+----------------------------+
+| ``object0 size`` | | uint64_t (8) |
++--------------------------+------+----------------------------+
+| ``object0 data`` | | bytes (<object0 size>) |
++--------------------------+------+----------------------------+
+| ``object1 size`` | | uint64_t (8) |
++--------------------------+------+----------------------------+
+| ``object1 data`` | | bytes (<object1 size> |
++--------------------------+------+----------------------------+
+| ... | | |
++--------------------------+------+----------------------------+
+| **Index** | <ip> | |
++--------------------------+------+----------------------------+
+| ``object0 key`` | | SHARD_KEY_LEN (32) |
++--------------------------+------+----------------------------+
+| ``object0 offset`` | | uint64_t (8) |
++--------------------------+------+----------------------------+
+| ... | | |
++--------------------------+------+----------------------------+
+| **Hash map** | <hp> | |
++--------------------------+------+----------------------------+
+| ``hash function`` | | <as written by cmph_dump> |
++--------------------------+------+----------------------------+
+
+
+``SHARD_MAGIC`` is the constant ``SWHShard`` (with ``\x00`` padding to 32
+characters).
+
+Index entries for deleted content are using the special value
+``{key=\x00...\x00, offset=2**64-1}``.
diff --git a/docs/index.rst b/docs/index.rst
index 4916df4..c11506f 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -12,8 +12,8 @@ Reference Documentation
.. toctree::
:maxdepth: 2
- benchmarks
format
+ benchmarks
.. only:: standalone_package_doc
--
GitLab
From 2591bbc8530e31bf8820244ea5002c848fbd6f70 Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Thu, 27 Feb 2025 16:08:47 +0100
Subject: [PATCH 05/24] extension: rename extension and C files hash.{ch} as
shard.{ch}
This better reflects what this extension is doing.
---
.gitignore | 8 ++++----
pyproject.toml | 2 +-
swh/perfecthash/Makefile | 20 +++++++++----------
swh/perfecthash/__init__.py | 2 +-
swh/perfecthash/build.py | 6 +++---
swh/perfecthash/{hash.c => shard.c} | 2 +-
swh/perfecthash/{hash.h => shard.h} | 0
.../{test_hash.cpp => test_shard.cpp} | 2 +-
8 files changed, 21 insertions(+), 21 deletions(-)
rename swh/perfecthash/{hash.c => shard.c} (99%)
rename swh/perfecthash/{hash.h => shard.h} (100%)
rename swh/perfecthash/{test_hash.cpp => test_shard.cpp} (99%)
diff --git a/.gitignore b/.gitignore
index d914e9b..e959ac1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -18,7 +18,7 @@ docs/README.md
docs/Makefile.sphinx
swh/perfecthash/html
-swh/perfecthash/hash.gcda
-swh/perfecthash/hash.gcno
-swh/perfecthash/test_hash
-swh/perfecthash/test_hash.lcov
+swh/perfecthash/shard.gcda
+swh/perfecthash/shard.gcno
+swh/perfecthash/test_shard
+swh/perfecthash/test_shard.lcov
diff --git a/pyproject.toml b/pyproject.toml
index b39968e..73f1f26 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -77,7 +77,7 @@ plugins = []
# 3rd party libraries without stubs (yet)
[[tool.mypy.overrides]]
module = [
- "swh.perfecthash._hash_cffi.*",
+ "swh.perfecthash._shard_cffi.*",
]
ignore_missing_imports = true
diff --git a/swh/perfecthash/Makefile b/swh/perfecthash/Makefile
index f914302..28e2ed2 100644
--- a/swh/perfecthash/Makefile
+++ b/swh/perfecthash/Makefile
@@ -2,20 +2,20 @@ CFLAGS=-D_FILE_OFFSET_BITS=64 -DHASH_DEBUG -Wall -I../.. -g -fprofile-arcs -ftes
CXXFLAGS=$(CFLAGS) -std=c++17
LDFLAGS=-lcmph -lgtest -lpthread -lstdc++ -lstdc++fs -fprofile-arcs -ftest-coverage
-test_hash: hash.o test_hash.o
+test_shard: shard.o test_shard.o
$(CXX) -o $@ $^ $(LDFLAGS)
-hash.c: hash.h
-test_hash.o: test_hash.cpp hash.h
-test_hash.cpp: hash.h
+shard.c: shard.h
+test_shard.o: test_shard.cpp shard.h
+test_shard.cpp: shard.h
format:
- clang-format -i hash.c hash.h test_hash.cpp
+ clang-format -i shard.c shard.h test_shard.cpp
-check: test_hash
- valgrind --leak-check=full --tool=memcheck ./test_hash
- lcov -d . -c -o test_hash.lcov
- rm -fr html ; genhtml -o html test_hash.lcov
+check: test_shard
+ valgrind --leak-check=full --tool=memcheck ./test_shard
+ lcov -d . -c -o test_shard.lcov
+ rm -fr html ; genhtml -o html test_shard.lcov
clean:
- rm -f *.o test_hash
+ rm -f *.o test_shard
diff --git a/swh/perfecthash/__init__.py b/swh/perfecthash/__init__.py
index 87b29f3..d435e9b 100644
--- a/swh/perfecthash/__init__.py
+++ b/swh/perfecthash/__init__.py
@@ -9,7 +9,7 @@ from typing import NewType, Optional, Type, cast
from cffi import FFI
-from swh.perfecthash._hash_cffi import lib
+from swh.perfecthash._shard_cffi import lib
Key = NewType("Key", bytes)
diff --git a/swh/perfecthash/build.py b/swh/perfecthash/build.py
index 9741fc1..67c1cc3 100644
--- a/swh/perfecthash/build.py
+++ b/swh/perfecthash/build.py
@@ -55,11 +55,11 @@ elif platform.system() == "Darwin" and Path("/opt/local/include/cmph.h").is_file
ffibuilder.set_source(
- "swh.perfecthash._hash_cffi",
+ "swh.perfecthash._shard_cffi",
"""
- #include "swh/perfecthash/hash.h"
+ #include "swh/perfecthash/shard.h"
""",
- sources=["swh/perfecthash/hash.c"],
+ sources=["swh/perfecthash/shard.c"],
include_dirs=["."],
libraries=["cmph"],
library_dirs=library_dirs,
diff --git a/swh/perfecthash/hash.c b/swh/perfecthash/shard.c
similarity index 99%
rename from swh/perfecthash/hash.c
rename to swh/perfecthash/shard.c
index 08d61d3..bad3bfb 100644
--- a/swh/perfecthash/hash.c
+++ b/swh/perfecthash/shard.c
@@ -17,7 +17,7 @@
#include <sys/types.h>
#include <unistd.h>
-#include "swh/perfecthash/hash.h"
+#include "swh/perfecthash/shard.h"
const int shard_key_len = SHARD_KEY_LEN;
diff --git a/swh/perfecthash/hash.h b/swh/perfecthash/shard.h
similarity index 100%
rename from swh/perfecthash/hash.h
rename to swh/perfecthash/shard.h
diff --git a/swh/perfecthash/test_hash.cpp b/swh/perfecthash/test_shard.cpp
similarity index 99%
rename from swh/perfecthash/test_hash.cpp
rename to swh/perfecthash/test_shard.cpp
index 35f1dde..bcfe187 100644
--- a/swh/perfecthash/test_hash.cpp
+++ b/swh/perfecthash/test_shard.cpp
@@ -13,7 +13,7 @@
#include <unistd.h>
extern "C" {
-#include "hash.h"
+#include "shard.h"
}
using namespace std::experimental;
--
GitLab
From 9b0fc2b93879a80ae81cdc810b7e3e8d087394e8 Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Thu, 27 Feb 2025 16:17:57 +0100
Subject: [PATCH 06/24] Rename the package as swh.shard
It does not make much sense to call it perfecthash, since the aim of
this package is creating, reading and manipulating shard files (which
do use cmph to speed extracting content objects from the shard file, but
this is an implementation detail, really).
---
.gitignore | 10 +++++-----
pyproject.toml | 16 ++++++++--------
setup.py | 2 +-
swh/{perfecthash => shard}/.clang-format | 0
swh/{perfecthash => shard}/Makefile | 0
swh/{perfecthash => shard}/__init__.py | 2 +-
swh/{perfecthash => shard}/build.py | 6 +++---
swh/{perfecthash => shard}/py.typed | 0
swh/{perfecthash => shard}/shard.c | 2 +-
swh/{perfecthash => shard}/shard.h | 0
swh/{perfecthash => shard}/test_shard.cpp | 0
swh/{perfecthash => shard}/tests/__init__.py | 0
.../test_hash.py => shard/tests/test_shard.py} | 4 ++--
tox.ini | 6 +++---
14 files changed, 24 insertions(+), 24 deletions(-)
rename swh/{perfecthash => shard}/.clang-format (100%)
rename swh/{perfecthash => shard}/Makefile (100%)
rename swh/{perfecthash => shard}/__init__.py (99%)
rename swh/{perfecthash => shard}/build.py (95%)
rename swh/{perfecthash => shard}/py.typed (100%)
rename swh/{perfecthash => shard}/shard.c (99%)
rename swh/{perfecthash => shard}/shard.h (100%)
rename swh/{perfecthash => shard}/test_shard.cpp (100%)
rename swh/{perfecthash => shard}/tests/__init__.py (100%)
rename swh/{perfecthash/tests/test_hash.py => shard/tests/test_shard.py} (98%)
diff --git a/.gitignore b/.gitignore
index e959ac1..25e21a2 100644
--- a/.gitignore
+++ b/.gitignore
@@ -17,8 +17,8 @@ docs/README.md
# without using tox, generally created by the swh-env/bin/update script
docs/Makefile.sphinx
-swh/perfecthash/html
-swh/perfecthash/shard.gcda
-swh/perfecthash/shard.gcno
-swh/perfecthash/test_shard
-swh/perfecthash/test_shard.lcov
+swh/shard/html
+swh/shard/shard.gcda
+swh/shard/shard.gcno
+swh/shard/test_shard
+swh/shard/test_shard.lcov
diff --git a/pyproject.toml b/pyproject.toml
index 73f1f26..cec29f8 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -1,10 +1,10 @@
[project]
-name = "swh.perfecthash"
+name = "swh.shard"
authors = [
{name="Software Heritage developers", email="swh-devel@inria.fr"},
]
-description = "Software Heritage Perfect Hash"
+description = "Software Heritage Shard File Format"
readme = {file = "README.rst", content-type = "text/x-rst"}
requires-python = ">=3.7"
classifiers = [
@@ -20,7 +20,7 @@ dynamic = ["version", "dependencies", "optional-dependencies"]
include = ["swh.*"]
[tool.setuptools.exclude-package-data]
-"swh.perfecthash" = [".clang-format"]
+"swh.shard" = [".clang-format"]
[tool.setuptools.dynamic]
dependencies = {file = ["requirements.txt", "requirements-swh.txt"]}
@@ -29,11 +29,11 @@ dependencies = {file = ["requirements.txt", "requirements-swh.txt"]}
testing = {file = ["requirements.txt", "requirements-swh.txt", "requirements-test.txt"]}
[project.urls]
-"Homepage" = "https://gitlab.softwareheritage.org/swh/devel/swh-perfecthash"
-"Bug Reports" = "https://gitlab.softwareheritage.org/swh/devel/swh-perfecthash/-/issues"
+"Homepage" = "https://gitlab.softwareheritage.org/swh/devel/swh-shard"
+"Bug Reports" = "https://gitlab.softwareheritage.org/swh/devel/swh-shard/-/issues"
"Funding" = "https://www.softwareheritage.org/donate"
-"Documentation" = "https://docs.softwareheritage.org/devel/swh-perfecthash/"
-"Source" = "https://gitlab.softwareheritage.org/swh/devel/swh-perfecthash.git"
+"Documentation" = "https://docs.softwareheritage.org/devel/swh-shard/"
+"Source" = "https://gitlab.softwareheritage.org/swh/devel/swh-shard.git"
[build-system]
requires = ["setuptools", "setuptools-scm", "cffi"]
@@ -77,7 +77,7 @@ plugins = []
# 3rd party libraries without stubs (yet)
[[tool.mypy.overrides]]
module = [
- "swh.perfecthash._shard_cffi.*",
+ "swh.shard._shard_cffi.*",
]
ignore_missing_imports = true
diff --git a/setup.py b/setup.py
index 9811762..7720587 100755
--- a/setup.py
+++ b/setup.py
@@ -7,5 +7,5 @@
from setuptools import setup
setup(
- cffi_modules=["swh/perfecthash/build.py:ffibuilder"],
+ cffi_modules=["swh/shard/build.py:ffibuilder"],
)
diff --git a/swh/perfecthash/.clang-format b/swh/shard/.clang-format
similarity index 100%
rename from swh/perfecthash/.clang-format
rename to swh/shard/.clang-format
diff --git a/swh/perfecthash/Makefile b/swh/shard/Makefile
similarity index 100%
rename from swh/perfecthash/Makefile
rename to swh/shard/Makefile
diff --git a/swh/perfecthash/__init__.py b/swh/shard/__init__.py
similarity index 99%
rename from swh/perfecthash/__init__.py
rename to swh/shard/__init__.py
index d435e9b..697b465 100644
--- a/swh/perfecthash/__init__.py
+++ b/swh/shard/__init__.py
@@ -9,7 +9,7 @@ from typing import NewType, Optional, Type, cast
from cffi import FFI
-from swh.perfecthash._shard_cffi import lib
+from swh.shard._shard_cffi import lib
Key = NewType("Key", bytes)
diff --git a/swh/perfecthash/build.py b/swh/shard/build.py
similarity index 95%
rename from swh/perfecthash/build.py
rename to swh/shard/build.py
index 67c1cc3..ee990b9 100644
--- a/swh/perfecthash/build.py
+++ b/swh/shard/build.py
@@ -55,11 +55,11 @@ elif platform.system() == "Darwin" and Path("/opt/local/include/cmph.h").is_file
ffibuilder.set_source(
- "swh.perfecthash._shard_cffi",
+ "swh.shard._shard_cffi",
"""
- #include "swh/perfecthash/shard.h"
+ #include "swh/shard/shard.h"
""",
- sources=["swh/perfecthash/shard.c"],
+ sources=["swh/shard/shard.c"],
include_dirs=["."],
libraries=["cmph"],
library_dirs=library_dirs,
diff --git a/swh/perfecthash/py.typed b/swh/shard/py.typed
similarity index 100%
rename from swh/perfecthash/py.typed
rename to swh/shard/py.typed
diff --git a/swh/perfecthash/shard.c b/swh/shard/shard.c
similarity index 99%
rename from swh/perfecthash/shard.c
rename to swh/shard/shard.c
index bad3bfb..371e92f 100644
--- a/swh/perfecthash/shard.c
+++ b/swh/shard/shard.c
@@ -17,7 +17,7 @@
#include <sys/types.h>
#include <unistd.h>
-#include "swh/perfecthash/shard.h"
+#include "swh/shard/shard.h"
const int shard_key_len = SHARD_KEY_LEN;
diff --git a/swh/perfecthash/shard.h b/swh/shard/shard.h
similarity index 100%
rename from swh/perfecthash/shard.h
rename to swh/shard/shard.h
diff --git a/swh/perfecthash/test_shard.cpp b/swh/shard/test_shard.cpp
similarity index 100%
rename from swh/perfecthash/test_shard.cpp
rename to swh/shard/test_shard.cpp
diff --git a/swh/perfecthash/tests/__init__.py b/swh/shard/tests/__init__.py
similarity index 100%
rename from swh/perfecthash/tests/__init__.py
rename to swh/shard/tests/__init__.py
diff --git a/swh/perfecthash/tests/test_hash.py b/swh/shard/tests/test_shard.py
similarity index 98%
rename from swh/perfecthash/tests/test_hash.py
rename to swh/shard/tests/test_shard.py
index 6d6ba27..9d5cac5 100644
--- a/swh/perfecthash/tests/test_hash.py
+++ b/swh/shard/tests/test_shard.py
@@ -12,7 +12,7 @@ import time
import pytest
-from swh.perfecthash import Shard, ShardCreator
+from swh.shard import Shard, ShardCreator
logger = logging.getLogger(__name__)
@@ -217,7 +217,7 @@ def payload(request):
#
# PYTHONMALLOC=malloc valgrind --tool=memcheck .tox/py3/bin/pytest \
-# -k test_build_speed swh/perfecthash/tests/test_hash.py |& tee /tmp/v
+# -k test_build_speed swh/shard/tests/test_shard.py |& tee /tmp/v
#
def test_build_speed(request, tmpdir, payload):
start = time.time()
diff --git a/tox.ini b/tox.ini
index cd2f3c2..2749266 100644
--- a/tox.ini
+++ b/tox.ini
@@ -15,9 +15,9 @@ deps =
pytest-cov
commands =
pytest --doctest-modules \
- --cov=swh/perfecthash \
+ --cov=swh/shard \
--cov-branch \
- swh/perfecthash \
+ swh/shard \
{posargs}
[testenv:black]
@@ -31,7 +31,7 @@ commands =
allowlist_externals = make
usedevelop = true
commands =
- make -C swh/perfecthash check
+ make -C swh/shard check
[testenv:flake8]
skip_install = true
--
GitLab
From 53451d7231e7d8338af5220106c9bbc65f742c6c Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Thu, 27 Feb 2025 16:25:24 +0100
Subject: [PATCH 07/24] Migrate to pybind11 and restructure the source code
directory
Use pybind11 to wrap the cmph and shard manipulation code instead of
cffi, it makes is a bit easier to add (C/C++) features in the extension.
The extension source files have been moved to src/_shard, and the python
source files for the swh.shard package have been moved to src/swh/shard,
moving away from all other swh package structure. This is required to
prevent side effects of having the local 'swh' directory in the working
directory of the developer, thus in the sys.path (by default), breaking
the dark magic involved in the loading of the package when it is
installed in editable mode (i.e. not to break pytest when executed
directly from the source with the package being installed in editable
mode).
---
.gitignore | 14 +-
.pre-commit-config.yaml | 2 +-
CMakeLists.txt | 22 ++
build_cmph.sh | 18 --
pyproject.toml | 55 +++--
requirements-swh.txt | 1 -
requirements-test.txt | 3 -
requirements.txt | 5 -
setup.py | 11 -
{swh/shard => src/_shard}/.clang-format | 0
{swh/shard => src/_shard}/Makefile | 0
src/_shard/bindings.cpp | 228 +++++++++++++++++
{swh/shard => src/_shard}/shard.c | 54 +++-
src/_shard/shard.h | 106 ++++++++
{swh/shard => src/_shard}/test_shard.cpp | 4 +-
src/swh/shard/__init__.py | 24 ++
{swh => src/swh}/shard/py.typed | 0
{swh => src/swh}/shard/tests/__init__.py | 0
{swh => src/swh}/shard/tests/test_shard.py | 148 +++++------
swh/shard/__init__.py | 271 ---------------------
swh/shard/build.py | 70 ------
swh/shard/shard.h | 60 -----
tox.ini | 11 +-
23 files changed, 553 insertions(+), 554 deletions(-)
create mode 100644 CMakeLists.txt
delete mode 100755 build_cmph.sh
delete mode 100644 requirements-swh.txt
delete mode 100644 requirements-test.txt
delete mode 100644 requirements.txt
delete mode 100755 setup.py
rename {swh/shard => src/_shard}/.clang-format (100%)
rename {swh/shard => src/_shard}/Makefile (100%)
create mode 100644 src/_shard/bindings.cpp
rename {swh/shard => src/_shard}/shard.c (92%)
create mode 100644 src/_shard/shard.h
rename {swh/shard => src/_shard}/test_shard.cpp (99%)
create mode 100644 src/swh/shard/__init__.py
rename {swh => src/swh}/shard/py.typed (100%)
rename {swh => src/swh}/shard/tests/__init__.py (100%)
rename {swh => src/swh}/shard/tests/test_shard.py (69%)
delete mode 100644 swh/shard/__init__.py
delete mode 100644 swh/shard/build.py
delete mode 100644 swh/shard/shard.h
diff --git a/.gitignore b/.gitignore
index 25e21a2..b8fde2c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -17,8 +17,12 @@ docs/README.md
# without using tox, generally created by the swh-env/bin/update script
docs/Makefile.sphinx
-swh/shard/html
-swh/shard/shard.gcda
-swh/shard/shard.gcno
-swh/shard/test_shard
-swh/shard/test_shard.lcov
+src/_shard/html
+src/_shard/shard.gcda
+src/_shard/shard.gcno
+src/_shard/test_shard
+src/_shard/test_shard.lcov
+src/swh/shard/_version.py
+
+*.shard
+*.zst
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 0487ca5..b839c82 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -37,7 +37,7 @@ repos:
- id: mypy
name: mypy
entry: mypy
- args: [swh]
+ args: ["-p", "swh.shard"]
pass_filenames: false
language: system
types: [python]
diff --git a/CMakeLists.txt b/CMakeLists.txt
new file mode 100644
index 0000000..4495f7a
--- /dev/null
+++ b/CMakeLists.txt
@@ -0,0 +1,22 @@
+cmake_minimum_required(VERSION 3.15...3.29)
+project(${SKBUILD_PROJECT_NAME}
+ VERSION ${SKBUILD_PROJECT_VERSION}
+ LANGUAGES C CXX)
+include(FindPkgConfig)
+
+set(PYBIND11_FINDPYTHON ON)
+find_package(Python REQUIRED COMPONENTS Interpreter Development.Module)
+find_package(pybind11 CONFIG REQUIRED)
+pkg_search_module(CMPH REQUIRED cmph)
+link_directories(${CMPH_INCLUDE_DIR})
+
+# Add a library using FindPython's tooling (pybind11 also provides a helper like
+# this)
+python_add_library(_shard MODULE src/_shard/bindings.cpp src/_shard/shard.c WITH_SOABI)
+target_link_libraries(_shard PRIVATE pybind11::headers cmph ${CMPH_INCLUDE_DIR})
+
+# This is passing in the version as a define just as an example
+target_compile_definitions(_shard PRIVATE VERSION_INFO=${PROJECT_VERSION})
+
+# The install directory is the output (wheel) directory
+install(TARGETS _shard DESTINATION swh/shard)
diff --git a/build_cmph.sh b/build_cmph.sh
deleted file mode 100755
index 3177b21..0000000
--- a/build_cmph.sh
+++ /dev/null
@@ -1,18 +0,0 @@
-#!/usr/bin/env bash
-
-set -e
-
-CMPH_VERSION=2.0.2
-PREFIX="$(readlink -f $(dirname $0))/cmph"
-
-rm -rf "$PREFIX"
-mkdir "$PREFIX"
-cd "$PREFIX"
-wget https://deac-ams.dl.sourceforge.net/project/cmph/v${CMPH_VERSION}/cmph-${CMPH_VERSION}.tar.gz -O cmph.tar.gz
-tar xf cmph.tar.gz
-
-cd cmph-${CMPH_VERSION}
-
-./configure --prefix="$PREFIX"
-make -j8
-make install
diff --git a/pyproject.toml b/pyproject.toml
index cec29f8..eeaed3d 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -14,19 +14,30 @@ classifiers = [
"Operating System :: OS Independent",
"Development Status :: 3 - Alpha",
]
-dynamic = ["version", "dependencies", "optional-dependencies"]
+dynamic = ["version"]
+dependencies = [
+ "click"
+]
-[tool.setuptools.packages.find]
-include = ["swh.*"]
+[project.optional-dependencies]
+testing = [
+ "pytest >= 8.1",
+ "pytest-mock",
+]
-[tool.setuptools.exclude-package-data]
-"swh.shard" = [".clang-format"]
+[build-system]
+requires = ["scikit-build-core>=0.10", "pybind11", "setuptools_scm"]
+build-backend = "scikit_build_core.build"
-[tool.setuptools.dynamic]
-dependencies = {file = ["requirements.txt", "requirements-swh.txt"]}
+[tool.scikit-build]
+minimum-version = "build-system.requires"
+wheel.exclude = ["*.c", "*.cpp", "*.h", ".clang-format", "*.o"]
-[tool.setuptools.dynamic.optional-dependencies]
-testing = {file = ["requirements.txt", "requirements-swh.txt", "requirements-test.txt"]}
+[tool.scikit-build.wheel.packages]
+"swh" = "src/swh"
+
+[tool.scikit-build.metadata]
+version.provider = "scikit_build_core.metadata.setuptools_scm"
[project.urls]
"Homepage" = "https://gitlab.softwareheritage.org/swh/devel/swh-shard"
@@ -35,11 +46,15 @@ testing = {file = ["requirements.txt", "requirements-swh.txt", "requirements-tes
"Documentation" = "https://docs.softwareheritage.org/devel/swh-shard/"
"Source" = "https://gitlab.softwareheritage.org/swh/devel/swh-shard.git"
-[build-system]
-requires = ["setuptools", "setuptools-scm", "cffi"]
-build-backend = "setuptools.build_meta"
+# have both the 'swh-shard' and 'swh shard' commands (if swh.core is installed)
+[project.scripts]
+"swh-shard" = "swh.shard.cli:main"
-[tool.setuptools_scm]
+[project.entry-points."swh.cli.subcommands"]
+"swh.shard" = "swh.shard.cli"
+
+[tool.setuptools_scm] # Section required
+write_to = "src/swh/shard/_version.py"
fallback_version = "0.0.1"
[tool.black]
@@ -55,16 +70,6 @@ line_length = 88
force_sort_within_sections = true
known_first_party = ['swh']
-[tool.cibuildwheel]
-before-all = "yum install -y wget && ./build_cmph.sh"
-
-[[tool.cibuildwheel.overrides]]
-select = "*-musllinux*"
-before-all = "apk add wget && ./build_cmph.sh"
-
-[tool.cibuildwheel.environment]
-LD_LIBRARY_PATH = "/project/cmph/lib"
-
[tool.mypy]
namespace_packages = true
warn_unused_ignores = true
@@ -77,7 +82,8 @@ plugins = []
# 3rd party libraries without stubs (yet)
[[tool.mypy.overrides]]
module = [
- "swh.shard._shard_cffi.*",
+ "swh.shard._shard.*",
+ "swh.core.*",
]
ignore_missing_imports = true
@@ -93,7 +99,6 @@ max-line-length = 88
[tool.pytest.ini_options]
norecursedirs = "build docs .*"
-asyncio_mode = "strict"
consider_namespace_packages = true
markers = [
"setrlimit(*limits): Set resource limits for the current test",
diff --git a/requirements-swh.txt b/requirements-swh.txt
deleted file mode 100644
index 2d1bbce..0000000
--- a/requirements-swh.txt
+++ /dev/null
@@ -1 +0,0 @@
-# Add here internal Software Heritage dependencies, one per line.
diff --git a/requirements-test.txt b/requirements-test.txt
deleted file mode 100644
index 5b2b51e..0000000
--- a/requirements-test.txt
+++ /dev/null
@@ -1,3 +0,0 @@
-pytest >= 8.1
-pytest-mock
-types-cffi
diff --git a/requirements.txt b/requirements.txt
deleted file mode 100644
index 9348672..0000000
--- a/requirements.txt
+++ /dev/null
@@ -1,5 +0,0 @@
-# Add here external Python modules dependencies, one per line. Module names
-# should match https://pypi.python.org/pypi names. For the full spec or
-# dependency lines, see https://pip.readthedocs.org/en/1.1/requirements.html
-
-cffi
diff --git a/setup.py b/setup.py
deleted file mode 100755
index 7720587..0000000
--- a/setup.py
+++ /dev/null
@@ -1,11 +0,0 @@
-#!/usr/bin/env python3
-# Copyright (C) 2021-2023 The Software Heritage developers
-# 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 setuptools import setup
-
-setup(
- cffi_modules=["swh/shard/build.py:ffibuilder"],
-)
diff --git a/swh/shard/.clang-format b/src/_shard/.clang-format
similarity index 100%
rename from swh/shard/.clang-format
rename to src/_shard/.clang-format
diff --git a/swh/shard/Makefile b/src/_shard/Makefile
similarity index 100%
rename from swh/shard/Makefile
rename to src/_shard/Makefile
diff --git a/src/_shard/bindings.cpp b/src/_shard/bindings.cpp
new file mode 100644
index 0000000..7bb9123
--- /dev/null
+++ b/src/_shard/bindings.cpp
@@ -0,0 +1,228 @@
+/*
+ Copyright (C) 2025 The Software Heritage developers
+ 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
+*/
+
+#include "shard.h"
+#include <errno.h>
+#include <pybind11/pybind11.h>
+#include <pybind11/pytypes.h>
+#include <pybind11/stl.h>
+#include <string.h>
+
+namespace py = pybind11;
+
+using namespace std::string_literals;
+
+class ShardCreator {
+ public:
+ ShardCreator(const std::string &path, uint64_t n)
+ : n_entries(n), n_registered(0) {
+ this->shard = shard_init(path.c_str());
+ }
+ ~ShardCreator() { shard_destroy(this->shard); }
+ void write(py::bytes key, py::bytes object) {
+ if (n_registered >= n_entries) {
+ throw py::value_error(
+ "The declared number of objects has already been written");
+ }
+ std::string kbuf = std::string(key);
+ if (kbuf.size() != SHARD_KEY_LEN) {
+ throw std::length_error(
+ "Invalid key size: "s + std::to_string(kbuf.size()) +
+ " (expected: " + std::to_string(SHARD_KEY_LEN) + ")");
+ }
+ // Not sure whether this does a copy or not...
+ std::string sv = object;
+ errno = 0;
+ if (shard_object_write(this->shard, kbuf.c_str(), sv.c_str(),
+ sv.size()) != 0) {
+ PyErr_SetFromErrno(PyExc_OSError);
+ throw py::error_already_set();
+ }
+ n_registered++;
+ }
+ ShardCreator &enter() {
+ errno = 0;
+ if (shard_prepare(this->shard, n_entries) != 0) {
+ if (errno != 0) {
+ PyErr_SetFromErrno(PyExc_OSError);
+ throw py::error_already_set();
+ } else
+ throw std::runtime_error("shard prepare failed");
+ }
+ return *this;
+ }
+ void exit() {
+ errno = 0;
+ if (n_registered < n_entries) {
+ PyErr_SetString(
+ PyExc_RuntimeError,
+ "The number of registered objects is less than the declared "
+ "number of entries; this is not allowed.");
+ throw py::error_already_set();
+ }
+ if (shard_finalize(this->shard) < 0) {
+ if (errno == 0) {
+ PyErr_SetString(PyExc_RuntimeError,
+ "shard_finalize failed. Was there a duplicate "
+ "key by any chance?");
+ throw py::error_already_set();
+ } else {
+ PyErr_SetFromErrno(PyExc_OSError);
+ throw py::error_already_set();
+ }
+ }
+ if (shard_close(this->shard) < 0) {
+ PyErr_SetFromErrno(PyExc_OSError);
+ throw py::error_already_set();
+ }
+ }
+ shard_t *shard;
+ uint64_t n_entries;
+ uint64_t n_registered;
+};
+
+class ShardReader {
+ public:
+ ShardReader(const std::string &path) {
+ this->shard = shard_init(path.c_str());
+ errno = 0;
+ if (shard_load(this->shard) != 0) {
+ PyErr_SetFromErrno(PyExc_OSError);
+ throw py::error_already_set();
+ }
+ }
+ ~ShardReader() {
+ // beware the close method (shard_close actually) may fail (not sure
+ // how) and this is not captured here... (cannot throw an exception
+ // from the destructor in c++17)
+ close();
+ shard_destroy(this->shard);
+ this->shard = NULL;
+ }
+ int close() {
+ errno = 0;
+ int ret = shard_close(this->shard);
+ return ret;
+ }
+ py::bytes getitem(py::bytes key) {
+ // get size and position file descriptor at the beginning of the object
+ uint64_t size = getsize(key);
+ // TODO: get rid of this tmp malloc...
+ char *buf = new char[size];
+ if (shard_read_object(this->shard, buf, size) != 0)
+ throw std::runtime_error(
+ "content read failed. Shard file might be corrupted.");
+ py::bytes b = py::bytes(buf, size);
+ delete buf;
+ return b;
+ }
+ void getindex(uint64_t pos, shard_index_t &idx) {
+ if (shard_index_get(this->shard, pos, &idx) < 0) {
+ if (errno != 0)
+ PyErr_SetFromErrno(PyExc_OSError);
+ else
+ PyErr_SetString(
+ PyExc_ValueError,
+ "Cannot retrieve index; either the asked position is "
+ "out range or the index cannot be found.");
+
+ throw py::error_already_set();
+ }
+ }
+ uint64_t getsize(py::bytes key) {
+ std::string kbuf = std::string(key);
+ if (kbuf.size() != SHARD_KEY_LEN) {
+ throw std::length_error(
+ "Invalid key size: "s + std::to_string(kbuf.size()) +
+ " (expected: " + std::to_string(SHARD_KEY_LEN) + ")");
+ }
+ uint64_t size;
+ if (shard_find_object(this->shard, kbuf.data(), &size) != 0)
+ throw py::key_error("key not found");
+ return size;
+ }
+ shard_t *shard;
+};
+
+PYBIND11_MODULE(_shard, m) {
+ py::class_<ShardCreator>(m, "ShardCreator")
+ .def(py::init<const std::string &, uint64_t>())
+ .def_property_readonly("header",
+ [](ShardCreator &s) -> const shard_header_t & {
+ return s.shard->header;
+ })
+ .def("__enter__", &ShardCreator::enter)
+ .def("__exit__",
+ [](ShardCreator &s, const std::optional<py::type> &exc_type,
+ const std::optional<py::object> &exc_value,
+ const std::optional<py::object> &traceback) {
+ // TODO: handle exceptions
+ if (!exc_type)
+ s.exit();
+ })
+ .def("write", &ShardCreator::write)
+ .def("key_len", [](ShardCreator &s) { return SHARD_KEY_LEN; });
+
+ py::class_<ShardReader>(m, "ShardReader")
+ .def_property_readonly_static(
+ "key_len", [](py::object /* self */) { return SHARD_KEY_LEN; })
+ .def(py::init<const std::string &>())
+ .def("close", &ShardReader::close)
+ .def_property_readonly("header",
+ [](ShardReader &s) -> const shard_header_t & {
+ return s.shard->header;
+ })
+ .def("getindex",
+ [](ShardReader &s, uint64_t pos) -> shard_index_t {
+ shard_index_t idx;
+ s.getindex(pos, idx);
+ return idx;
+ })
+ .def("getsize", &ShardReader::getsize)
+ .def("delete",
+ [](const std::string &path, py::bytes key) {
+ std::string kbuf = std::string(key);
+ if (kbuf.size() != SHARD_KEY_LEN) {
+ throw std::length_error(
+ "Invalid key size: "s + std::to_string(kbuf.size()) +
+ " (expected: " + std::to_string(SHARD_KEY_LEN) + ")");
+ }
+ ShardReader reader(path);
+ shard_delete(reader.shard, kbuf.data());
+ })
+ .def("find",
+ [](ShardReader &s, py::bytes key) {
+ std::string kbuf = std::string(key);
+ if (kbuf.size() != SHARD_KEY_LEN) {
+ throw std::length_error(
+ "Invalid key size: "s + std::to_string(kbuf.size()) +
+ " (expected: " + std::to_string(SHARD_KEY_LEN) + ")");
+ }
+ uint64_t size;
+ if (shard_find_object(s.shard, kbuf.data(), &size) != 0)
+ throw py::key_error("key not found");
+ return size;
+ })
+ .def("__getitem__", &ShardReader::getitem)
+ .def("lookup", &ShardReader::getitem);
+
+ py::class_<shard_header_t>(m, "ShardHeader")
+ .def_readonly("version", &shard_header_t::version)
+ .def_readonly("objects_count", &shard_header_t::objects_count)
+ .def_readonly("objects_position", &shard_header_t::objects_position)
+ .def_readonly("objects_size", &shard_header_t::objects_size)
+ .def_readonly("index_position", &shard_header_t::index_position)
+ .def_readonly("index_size", &shard_header_t::index_size)
+ .def_readonly("hash_position", &shard_header_t::hash_position);
+
+ py::class_<shard_index_t>(m, "ShardIndex")
+ .def_property_readonly("key",
+ [](shard_index_t &s) -> py::bytes {
+ return py::bytes(s.key, SHARD_KEY_LEN);
+ })
+ .def_readonly("object_offset", &shard_index_t::object_offset);
+};
diff --git a/swh/shard/shard.c b/src/_shard/shard.c
similarity index 92%
rename from swh/shard/shard.c
rename to src/_shard/shard.c
index 371e92f..175980f 100644
--- a/swh/shard/shard.c
+++ b/src/_shard/shard.c
@@ -1,9 +1,12 @@
/*
- * Copyright (C) 2021-2022 The Software Heritage developers
+ * Copyright (C) 2021-2025 The Software Heritage developers
* 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
*/
+#ifdef __cplusplus
+extern "C" {
+#endif
#include <assert.h>
#include <errno.h>
@@ -17,7 +20,7 @@
#include <sys/types.h>
#include <unistd.h>
-#include "swh/shard/shard.h"
+#include "shard.h"
const int shard_key_len = SHARD_KEY_LEN;
@@ -64,7 +67,7 @@ int shard_close(shard_t *shard) {
int shard_seek(shard_t *shard, uint64_t offset, int whence) {
if (offset > INT64_MAX) {
- printf("shard_seek: %lu > %lu (INT64_MAX)", offset, INT64_MAX);
+ printf("shard_seek: %lu > %ld (INT64_MAX)", offset, INT64_MAX);
return -1;
}
int r = fseeko(shard->f, offset, whence);
@@ -112,10 +115,10 @@ int shard_write(shard_t *shard, const void *ptr, uint64_t nmemb) {
int shard_write_zeros(shard_t *shard, uint64_t size) {
#define BUF_SIZE 4096
char buf[BUF_SIZE];
- size_t bytes_written;
memset(buf, 0, BUF_SIZE);
while (size > 0) {
+ size_t bytes_written;
if ((bytes_written = fwrite(buf, 1, MIN(size, BUF_SIZE), shard->f)) ==
0) {
return -1;
@@ -244,6 +247,7 @@ int shard_header_reset(shard_header_t *header) {
int shard_object_write(shard_t *shard, const char *key, const char *object,
uint64_t object_size) {
// save key & index to later build the hash
+ debug("shard_object_write: index_offset=%lu\n", shard->index_offset);
shard_index_t *index = &shard->index[shard->index_offset];
memcpy((void *)index->key, key, SHARD_KEY_LEN);
index->object_offset = shard_tell(shard);
@@ -335,6 +339,8 @@ int shard_index_save(shard_t *shard) {
index[h].object_offset = htonq(shard->index[i].object_offset);
}
uint64_t index_size = shard->header.index_size;
+ debug("shard_index_save: save %lu index bytes at position %lu\n",
+ index_size, shard->header.index_position);
if (shard_write(shard, (void *)index, index_size) < 0) {
printf("shard_index_save\n");
return -1;
@@ -343,6 +349,29 @@ int shard_index_save(shard_t *shard) {
return 0;
}
+int shard_index_get(shard_t *shard, uint64_t pos, shard_index_t *idx) {
+ // the number of entries in the cmph map (and thus in the index) is
+ // generally larger than the number of saved objects, but we do not keep
+ // the former number in the header, so recompute from the index size)
+ if (pos > shard->header.index_size / sizeof(shard_index_t)) {
+ printf("shard_index_get: position out of range\n");
+ return -1;
+ }
+ uint64_t index_offset =
+ shard->header.index_position + pos * sizeof(shard_index_t);
+ if (shard_seek(shard, index_offset, SEEK_SET) < 0) {
+ printf("shard_index_get: index not found\n");
+ }
+ errno = 0;
+ if (shard_read(shard, idx, sizeof(shard_index_t)) < 0) {
+ printf("shard_index_get: index not found\n");
+ return -1;
+ }
+ idx->object_offset = ntohq(idx->object_offset);
+
+ return 0;
+}
+
int shard_hash_save(shard_t *shard) {
shard->header.hash_position =
shard->header.index_position + shard->header.index_size;
@@ -404,6 +433,7 @@ int shard_reset(shard_t *shard) {
}
int shard_prepare(shard_t *shard, uint64_t objects_count) {
+ debug("shard_prepare: objects=%lu\n", objects_count);
if (shard_open(shard, "w+") < 0)
return -1;
if (shard_reset(shard) < 0)
@@ -486,12 +516,18 @@ int shard_hash_load(shard_t *shard) {
int shard_load(shard_t *shard) {
debug("shard_load\n");
- if (shard_open(shard, "r") < 0)
+ if (shard_open(shard, "r") < 0) {
+ debug("Open failed\n");
return -1;
- if (shard_magic_load(shard) < 0)
+ }
+ if (shard_magic_load(shard) < 0) {
+ debug("Magic load failed\n");
return -1;
- if (shard_header_load(shard) < 0)
+ }
+ if (shard_header_load(shard) < 0) {
+ debug("Header load failed\n");
return -1;
+ }
return shard_hash_load(shard);
}
@@ -618,3 +654,7 @@ int shard_destroy(shard_t *shard) {
free(shard);
return r;
}
+
+#ifdef __cplusplus
+}
+#endif
diff --git a/src/_shard/shard.h b/src/_shard/shard.h
new file mode 100644
index 0000000..bb66114
--- /dev/null
+++ b/src/_shard/shard.h
@@ -0,0 +1,106 @@
+/*
+ * Copyright (C) 2021-2025 The Software Heritage developers
+ * 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
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <cmph.h>
+#include <cmph_types.h>
+#include <stdint.h>
+
+#define SHARD_OFFSET_MAGIC 32
+#define SHARD_OFFSET_HEADER 512
+#define SHARD_KEY_LEN 32
+extern const int shard_key_len;
+
+#define SHARD_MAGIC "SWHShard"
+#define SHARD_VERSION 1
+
+/* Shard File Format
+
+ +------------------------+------+----------------------------+
+ | SHARD_MAGIC | 0 | SHARD_OFFSET_MAGIC (32) |
+ +------------------------+------+----------------------------+
+ | *header* | 32 | (56) |
+ | version | | uint64_t (8) |
+ | objects_count | | uint64_t (8) |
+ | objects_position (op)| | uint64_t (8) |
+ | objects_size | | uint64_t (8) |
+ | index_position (ip) | | uint64_t (8) |
+ | index_size | | uint64_t (8) |
+ | hash_position (hp) | | uint64_t (8) |
+ +------------------------+------+----------------------------+
+ | *Objects* | <op> | |
+ | object0 size | | uint64_t (8) |
+ | object0 data | | <object0 size> |
+ | object1 size | | uint64_t (8) |
+ | object1 data | | <object1 size> |
+ | ... | | |
+ +------------------------+------+----------------------------+
+ | *Index* | <ip> | |
+ | object0 key | | SHARD_KEY_LEN (32) |
+ | object0 offset | | uint64_t (8) |
+ | ... | | |
+ +------------------------+------+----------------------------+
+ | *Hash map* | <hp> | |
+ | hash function | | <as written by cmph_dump> |
+ +------------------------+------+----------------------------+
+
+ */
+
+typedef struct {
+ uint64_t version;
+ uint64_t objects_count;
+ uint64_t objects_position;
+ uint64_t objects_size;
+ uint64_t index_position;
+ uint64_t index_size;
+ uint64_t hash_position;
+} shard_header_t;
+
+typedef struct {
+ char key[SHARD_KEY_LEN];
+ uint64_t object_offset;
+} shard_index_t;
+
+typedef struct {
+ char *path;
+ FILE *f;
+ shard_header_t header;
+ cmph_t *hash;
+
+ // The following fields are only used when creating the Read Shard
+ cmph_io_adapter_t *source;
+ cmph_config_t *config;
+ shard_index_t *index;
+ uint64_t index_offset;
+} shard_t;
+
+shard_t *shard_init(const char *path);
+int shard_destroy(shard_t *shard);
+int shard_close(shard_t *shard);
+
+int shard_prepare(shard_t *shard, uint64_t objects_count);
+int shard_object_write(shard_t *shard, const char *key, const char *object,
+ uint64_t object_size);
+int shard_finalize(shard_t *shard);
+
+int shard_load(shard_t *shard);
+int shard_find_object(shard_t *shard, const char *key, uint64_t *object_size);
+int shard_read_object(shard_t *shard, char *object, uint64_t object_size);
+
+int shard_index_get(shard_t *shard, const uint64_t pos, shard_index_t *idx);
+
+int shard_delete(shard_t *shard, const char *key);
+
+int shard_read(shard_t *shard, void *ptr, uint64_t size);
+int shard_seek(shard_t *shard, uint64_t offset, int whence);
+
+#ifdef __cplusplus
+}
+#endif
diff --git a/swh/shard/test_shard.cpp b/src/_shard/test_shard.cpp
similarity index 99%
rename from swh/shard/test_shard.cpp
rename to src/_shard/test_shard.cpp
index bcfe187..7e8f1ab 100644
--- a/swh/shard/test_shard.cpp
+++ b/src/_shard/test_shard.cpp
@@ -60,7 +60,7 @@ std::string gen_random(const int len) {
return tmp_s;
}
-TEST(HashTest, One) {
+TEST(ShardTest, One) {
auto tmpdir = create_temporary_directory();
filesystem::path tmpfile = tmpdir / std::string("shard");
ASSERT_GE(close(open(tmpfile.c_str(), O_CREAT, 0777)), 0);
@@ -107,7 +107,7 @@ TEST(HashTest, One) {
filesystem::remove_all(tmpdir);
}
-TEST(HashTest, Many) {
+TEST(ShardTest, Many) {
auto tmpdir = create_temporary_directory();
filesystem::path tmpfile = tmpdir / std::string("shard");
ASSERT_GE(close(open(tmpfile.c_str(), O_CREAT, 0777)), 0);
diff --git a/src/swh/shard/__init__.py b/src/swh/shard/__init__.py
new file mode 100644
index 0000000..8215dfc
--- /dev/null
+++ b/src/swh/shard/__init__.py
@@ -0,0 +1,24 @@
+# Copyright (C) 2021-2025 The Software Heritage developers
+# 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 ._shard import ShardCreator, ShardReader
+
+__all__ = ["Shard", "ShardCreator"]
+
+
+class Shard(ShardReader):
+ # for BW compat reason, implement the context manager protocol
+ def __enter__(self):
+ return self
+
+ def __exit__(self, exc_type, exc_value, traceback):
+ self.close()
+
+ def __iter__(self):
+ # iterate of the keys
+ for i in range(self.header.index_size // (32 + 8)): # KEY_LEN + uint64
+ idx = self.getindex(i)
+ if idx.object_offset < (2**64 - 1):
+ yield idx.key
diff --git a/swh/shard/py.typed b/src/swh/shard/py.typed
similarity index 100%
rename from swh/shard/py.typed
rename to src/swh/shard/py.typed
diff --git a/swh/shard/tests/__init__.py b/src/swh/shard/tests/__init__.py
similarity index 100%
rename from swh/shard/tests/__init__.py
rename to src/swh/shard/tests/__init__.py
diff --git a/swh/shard/tests/test_shard.py b/src/swh/shard/tests/test_shard.py
similarity index 69%
rename from swh/shard/tests/test_shard.py
rename to src/swh/shard/tests/test_shard.py
index 9d5cac5..871720b 100644
--- a/swh/shard/tests/test_shard.py
+++ b/src/swh/shard/tests/test_shard.py
@@ -3,6 +3,7 @@
# License: GNU General Public License version 3, or any later version
# See top-level LICENSE file for more information
+from hashlib import sha256
import logging
import os
from pathlib import Path
@@ -38,9 +39,9 @@ def setrlimit(request):
logger.info("Resulting rlimit %s (%s, %s)", which, *result)
-KEY_A = b"A" * Shard.key_len()
-KEY_B = b"B" * Shard.key_len()
-KEY_C = b"C" * Shard.key_len()
+KEY_A = b"A" * Shard.key_len
+KEY_B = b"B" * Shard.key_len
+KEY_C = b"C" * Shard.key_len
OBJECT_A = b"AAAA"
OBJECT_B = b"BBBB"
@@ -74,38 +75,44 @@ def test_creator_open_without_permission(tmpdir):
path.touch()
# Remove all permissions
path.chmod(0o000)
- shard = ShardCreator(str(path), 1)
- with pytest.raises(PermissionError, match="no-perm"):
- shard.prepare()
+ with pytest.raises(PermissionError):
+ with ShardCreator(str(path), 1):
+ pass
@pytest.mark.setrlimit((resource.RLIMIT_FSIZE, (64_000, -1)))
def test_write_above_rlimit_fsize(tmpdir):
- shard = ShardCreator(f"{tmpdir}/test-shard", 1)
- shard.prepare()
- with pytest.raises(OSError, match=r"File too large.*test-shard"):
- shard.write(b"A" * Shard.key_len(), b"A" * 72_000)
+ with pytest.raises(OSError, match=r"File too large"):
+ with ShardCreator(f"{tmpdir}/test-shard", 1) as shard:
+ shard.write(b"A" * Shard.key_len, b"A" * 72_000)
def test_write_errors_if_too_many(tmpdir):
- shard = ShardCreator(f"{tmpdir}/shard", 1)
- shard.prepare()
- shard.write(b"A" * Shard.key_len(), b"AAAA")
- with pytest.raises(ValueError):
- shard.write(b"B" * Shard.key_len(), b"BBBB")
+ with ShardCreator(f"{tmpdir}/shard", 1) as shard:
+ shard.write(b"A" * Shard.key_len, b"AAAA")
+ with pytest.raises(ValueError):
+ shard.write(b"B" * Shard.key_len, b"BBBB")
+
+
+def test_write_errors_if_not_enought(tmpdir):
+ with pytest.raises(RuntimeError):
+ with ShardCreator(f"{tmpdir}/shard", 2) as shard:
+ shard.write(b"A" * Shard.key_len, b"AAAA")
def test_write_errors_for_wrong_key_len(tmpdir):
- shard = ShardCreator(f"{tmpdir}/shard", 1)
- shard.prepare()
- with pytest.raises(ValueError):
- shard.write(b"A", b"AAAA")
+ with ShardCreator(f"{tmpdir}/shard", 1) as shard:
+ with pytest.raises(ValueError):
+ shard.write(b"A", b"AAAA")
+ # add a valid entry so the __exit__ does not raise an exception
+ shard.write(b"A" * Shard.key_len, b"AAAA")
def test_creator_context_does_not_run_finalize_on_error(tmpdir, mocker):
+ pytest.skip("Not sure how to do that...")
import contextlib
- mock_method = mocker.patch.object(ShardCreator, "finalize")
+ mock_method = mocker.patch.object(ShardCreator, "__exit__")
with contextlib.suppress(KeyError):
with ShardCreator(f"{tmpdir}/shard", 1) as _:
raise KeyError(42)
@@ -115,24 +122,20 @@ def test_creator_context_does_not_run_finalize_on_error(tmpdir, mocker):
@pytest.mark.setrlimit((resource.RLIMIT_FSIZE, (64_000, -1)))
def test_finalize_above_rlimit_fsize(tmpdir):
path = f"{tmpdir}/shard"
- shard = ShardCreator(path, 1)
- shard.prepare()
- shard.write(b"A" * Shard.key_len(), b"A" * 63_500)
with pytest.raises(OSError, match="File too large"):
- shard.finalize()
+ with ShardCreator(path, 1) as shard:
+ shard.write(b"A" * Shard.key_len, b"A" * 63_500)
def test_creator_errors_with_duplicate_key(tmpdir):
- shard = ShardCreator(f"{tmpdir}/shard", 2)
- shard.prepare()
- shard.write(b"A" * Shard.key_len(), b"AAAA")
- shard.write(b"A" * Shard.key_len(), b"AAAA")
with pytest.raises(RuntimeError, match="duplicate"):
- shard.finalize()
+ with ShardCreator(f"{tmpdir}/shard", 2) as shard:
+ shard.write(b"A" * Shard.key_len, b"AAAA")
+ shard.write(b"A" * Shard.key_len, b"BBBB")
def test_load_non_existing():
- with pytest.raises(FileNotFoundError, match="/nonexistent"):
+ with pytest.raises(FileNotFoundError):
_ = Shard("/nonexistent")
@@ -142,7 +145,7 @@ def corrupted_shard_path(tmpdir):
SHARD_OFFSET_HEADER = 512
path = f"{tmpdir}/corrupted"
with ShardCreator(path, 1) as s:
- s.write(b"A" * Shard.key_len(), b"AAAA")
+ s.write(b"A" * Shard.key_len, b"AAAA")
with open(path, "rb+") as f:
f.seek(SHARD_OFFSET_HEADER)
# replace the object size (uint64_t) by something larger than file size
@@ -152,32 +155,31 @@ def corrupted_shard_path(tmpdir):
def test_lookup_failure(corrupted_shard_path):
with Shard(corrupted_shard_path) as shard:
- with pytest.raises(RuntimeError, match=r"failed.*/corrupted"):
- shard.lookup(b"A" * Shard.key_len())
+ with pytest.raises(RuntimeError, match=r"failed.*corrupted"):
+ shard.lookup(b"A" * Shard.key_len)
def test_lookup_errors_for_wrong_key_len(tmpdir):
- shard = ShardCreator(f"{tmpdir}/shard", 1)
- shard.prepare()
with pytest.raises(ValueError):
- shard.write(b"A", b"AAAA")
+ with ShardCreator(f"{tmpdir}/shard", 1) as shard:
+ shard.write(b"A", b"AAAA")
@pytest.fixture
def shard_with_mismatched_key(tmp_path):
path = tmp_path / "mismatched"
with ShardCreator(str(path), 1) as s:
- s.write(b"A" * Shard.key_len(), b"AAAA")
+ s.write(b"A" * Shard.key_len, b"AAAA")
# Replace the key in the index
content = path.read_bytes()
- path.write_bytes(content.replace(b"A" * Shard.key_len(), b"B" * Shard.key_len()))
+ path.write_bytes(content.replace(b"A" * Shard.key_len, b"B" * Shard.key_len))
return str(path)
def test_lookup_errors_for_mismatched_key(shard_with_mismatched_key):
with Shard(shard_with_mismatched_key) as shard:
- with pytest.raises(RuntimeError, match=r"Mismatch"):
- shard.lookup(b"A" * Shard.key_len())
+ with pytest.raises(KeyError):
+ shard.lookup(b"A" * Shard.key_len)
@pytest.fixture
@@ -217,7 +219,7 @@ def payload(request):
#
# PYTHONMALLOC=malloc valgrind --tool=memcheck .tox/py3/bin/pytest \
-# -k test_build_speed swh/shard/tests/test_shard.py |& tee /tmp/v
+# -k test_build_speed swh/perfecthash/tests/test_hash.py |& tee /tmp/v
#
def test_build_speed(request, tmpdir, payload):
start = time.time()
@@ -232,11 +234,12 @@ def test_build_speed(request, tmpdir, payload):
f"total_duration {duration}"
)
#
- # According to the docs/benchmarks.rst analysis, the duration is
- # below 5 times the baseline time This assertion is here to ensure
- # we do not not regress in the future...
+ # According to the docs/benchmarks.rst analysis, the duration is below 5
+ # times the baseline time. This assertion is here to ensure we do not not
+ # regress in the future... (we use x10 to give a bit of slack otherwise the
+ # test is pretty unstable)
#
- assert duration < baseline * 5
+ assert duration < baseline * 10
def test_lookup_speed(request, tmpdir, payload):
@@ -281,42 +284,49 @@ def shard_build(request, tmpdir, payload):
objects = {}
count = 0
size = 0
+ keys = []
with open(payload, "rb") as f:
while True:
- key = f.read(Shard.key_len())
- if len(key) < Shard.key_len():
- break
- assert key not in objects
object = f.read(random.randrange(512, object_max_size))
if len(object) < 512:
break
+ key = sha256(object).digest()
objects[key] = len(object)
size += len(object)
count += 1
+ keys.append(key)
print(f"number of objects = {count}, total size = {size}")
- assert size < shard_size
+ assert size <= shard_size
start = time.time()
- shard = ShardCreator(shard_path, len(objects))
- shard.prepare()
+ with ShardCreator(shard_path, len(objects)) as shard:
+ count = 0
+ size = 0
+ with open(payload, "rb") as f:
+ for key in keys:
+ object = f.read(objects[key])
+ assert len(object) == objects[key]
+ count += 1
+ size += len(object)
+ shard.write(key, object)
+ write_duration = time.time() - start
+ start = time.time()
- count = 0
- size = 0
- with open(payload, "rb") as f:
- while True:
- key = f.read(Shard.key_len())
- if len(key) < Shard.key_len():
- break
- if key not in objects:
- break
- object = f.read(objects[key])
- assert len(object) == objects[key]
- count += 1
- size += len(object)
- shard.write(key, object)
- write_duration = time.time() - start
- start = time.time()
- shard.finalize()
build_duration = time.time() - start
return write_duration, build_duration, objects
+
+
+def test_memleak(request, tmpdir, payload):
+ "Naive test for memleak in ShardReader"
+ shard_build(request, tmpdir, payload)
+ maxrss0 = resource.getrusage(resource.RUSAGE_SELF).ru_maxrss
+ shard_file = str(tmpdir / "shard")
+ for i in range(100):
+ with Shard(shard_file) as s:
+ for key in s:
+ obj = s[key]
+ assert sha256(obj).digest() == key
+
+ maxrss1 = resource.getrusage(resource.RUSAGE_SELF).ru_maxrss
+ assert (maxrss1 - maxrss0) < 1024 # in kB
diff --git a/swh/shard/__init__.py b/swh/shard/__init__.py
deleted file mode 100644
index 697b465..0000000
--- a/swh/shard/__init__.py
+++ /dev/null
@@ -1,271 +0,0 @@
-# Copyright (C) 2021-2022 The Software Heritage developers
-# 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
-
-import os
-from types import TracebackType
-from typing import NewType, Optional, Type, cast
-
-from cffi import FFI
-
-from swh.shard._shard_cffi import lib
-
-Key = NewType("Key", bytes)
-
-
-class ShardCreator:
- def __init__(self, path: str, object_count: int):
- """Create a Shard.
-
- The file at ``path`` will be truncated if it already exists.
-
- ``object_count`` must match the number of objects that will be added
- using the :meth:`write` method. A ``RuntimeError`` will be raised
- on :meth:`finalize` in case of inconsistencies.
-
- Ideally this should be done using a ``with`` statement, as such:
-
- .. code-block:: python
-
- with ShardCreator("shard", len(objects)) as shard:
- for key, object in objects.items():
- shard.write(key, object)
-
- Otherwise, :meth:`prepare`, :meth:`write` and :meth:`finalize` must be
- called in sequence.
-
- Args:
- path: path to the Shard file or device that will be written.
- object_count: number of objects that will be written to the Shard.
- """
-
- self.ffi = FFI()
- self.path = path
- self.object_count = object_count
- self.shard = None
-
- def __enter__(self) -> "ShardCreator":
- self.prepare()
- return self
-
- def __exit__(
- self,
- exc_type: Optional[Type[BaseException]],
- exc_val: Optional[BaseException],
- exc_tb: Optional[TracebackType],
- ) -> None:
- if exc_type is not None:
- self._destroy()
- return
-
- self.finalize()
-
- def __del__(self):
- if self.shard:
- _ = lib.shard_destroy(self.shard)
-
- def _destroy(self) -> None:
- _ = lib.shard_destroy(self.shard)
- self.shard = None
-
- def prepare(self) -> None:
- """Initialize the shard.
-
- Raises:
- RuntimeError: something went wrong while creating the Shard.
- """
- assert self.shard is None, "prepare() has already been called"
-
- self.shard = lib.shard_init(self.path.encode("utf-8"))
-
- self.ffi.errno = 0
- ret = lib.shard_prepare(self.shard, self.object_count)
- if ret != 0:
- raise OSError(self.ffi.errno, os.strerror(self.ffi.errno), self.path)
- self.written_object_count = 0
-
- def finalize(self) -> None:
- """Finalize the Shard.
-
- Write the index and the perfect hash table
- that will be used to find the content of the objects from
- their key.
-
- Raises:
- RuntimeError: if the number of written objects does not match ``object_count``,
- or if something went wrong while saving.
- """
- assert self.shard, "prepare() has not been called"
-
- if self.object_count != self.written_object_count:
- raise RuntimeError(
- f"Only {self.written_object_count} objects were written "
- f"when {self.object_count} were declared."
- )
-
- self.ffi.errno = 0
- ret = lib.shard_finalize(self.shard)
- if ret != 0:
- errno = self.ffi.errno
- if errno == 0:
- raise RuntimeError(
- "shard_finalize failed. Was there a duplicate key by any chance?"
- )
- else:
- raise OSError(self.ffi.errno, os.strerror(errno), self.path)
- self._destroy()
-
- def write(self, key: Key, object: bytes) -> None:
- """Add the key/object pair to the Read Shard.
-
- Args:
- key: the unique key associated with the object.
- object: the object
-
- Raises:
- ValueError: if the key length is wrong, or if enough objects
- have already been written.
- RuntimeError: if something wrong happens when writing the object.
- """
- assert self.shard, "prepare() has not been called"
-
- if len(key) != Shard.key_len():
- raise ValueError(f"key length is {len(key)} instead of {Shard.key_len()}")
- if self.written_object_count >= self.object_count:
- raise ValueError("The declared number of objects has already been written")
-
- self.ffi.errno = 0
- ret = lib.shard_object_write(self.shard, key, object, len(object))
- if ret != 0:
- raise OSError(self.ffi.errno, os.strerror(self.ffi.errno), self.path)
- self.written_object_count += 1
-
-
-class Shard:
- """Files storing objects indexed with a perfect hash table.
-
- This class allows creating a Read Shard by adding key/object pairs
- and looking up the content of an object when given the key.
-
- This class can act as a context manager, like so:
-
- .. code-block:: python
-
- with Shard("shard") as shard:
- return shard.lookup(key)
- """
-
- def __init__(self, path: str):
- """Open an existing Read Shard.
-
- Args:
- path: path to an existing Read Shard file or device
-
- """
- self.ffi = FFI()
- self.path = path
- self.shard = lib.shard_init(self.path.encode("utf-8"))
-
- self.ffi.errno = 0
- ret = lib.shard_load(self.shard)
- if ret != 0:
- raise OSError(self.ffi.errno, os.strerror(self.ffi.errno), self.path)
-
- def __del__(self) -> None:
- if self.shard:
- _ = lib.shard_destroy(self.shard)
-
- def close(self) -> None:
- assert self.shard, "Shard has been closed already"
-
- _ = lib.shard_destroy(self.shard)
- self.shard = None
-
- def __enter__(self) -> "Shard":
- return self
-
- def __exit__(
- self,
- exc_type: Optional[Type[BaseException]],
- exc_val: Optional[BaseException],
- exc_tb: Optional[TracebackType],
- ) -> None:
- self.close()
-
- @staticmethod
- def key_len():
- return lib.shard_key_len
-
- def lookup(self, key: Key) -> bytes:
- """Fetch the object matching the key in the Read Shard.
-
- Fetching an object is O(1): one lookup in the index to obtain
- the offset of the object in the Read Shard and one read to get
- the payload.
-
- Args:
- key: the key associated with the object to retrieve.
-
- Returns:
- the object as bytes.
-
- Raises:
- KeyError: the object has been deleted
- RuntimeError: something went wrong during lookup
- """
- assert self.shard, "Shard has been closed already"
-
- if len(key) != Shard.key_len():
- raise ValueError(f"key length is {len(key)} instead of {Shard.key_len()}")
-
- self.ffi.errno = 0
- object_size_pointer = self.ffi.new("uint64_t*")
- ret = lib.shard_find_object(self.shard, key, object_size_pointer)
- if ret == 1:
- raise KeyError(key)
- elif ret < 0:
- errno = self.ffi.errno
- if errno == 0:
- raise RuntimeError(
- f"shard_find_object failed. Mismatching key for {key.hex()} in the index?"
- )
- else:
- raise OSError(self.ffi.errno, os.strerror(self.ffi.errno), self.path)
- object_size = object_size_pointer[0]
- object_pointer = self.ffi.new("char[]", object_size)
- self.ffi.errno = 0
- ret = lib.shard_read_object(self.shard, object_pointer, object_size)
- if ret != 0:
- errno = self.ffi.errno
- if errno == 0:
- raise RuntimeError(
- f"shard_read_object failed. " f"{self.path} might be corrupted."
- )
- else:
- raise OSError(errno, os.strerror(errno), self.path)
- return cast(bytes, self.ffi.unpack(object_pointer, object_size))
-
- @staticmethod
- def delete(path: str, key: Key):
- """Open the Shard file and delete the given key.
-
- The object size and data will be overwritten by zeros. The Shard
- file size and offsets are not changed for safety.
-
- Args:
- key: the key associated with the object to retrieve.
-
- Raises:
- KeyError: the object has been deleted
- RuntimeError: something went wrong during lookup
- """
- with Shard(path) as shard:
- shard._delete(key)
-
- def _delete(self, key: Key):
- ret = lib.shard_delete(self.shard, key)
- if ret == 1:
- raise KeyError(key)
- elif ret < 0:
- raise RuntimeError("shard_delete failed")
diff --git a/swh/shard/build.py b/swh/shard/build.py
deleted file mode 100644
index ee990b9..0000000
--- a/swh/shard/build.py
+++ /dev/null
@@ -1,70 +0,0 @@
-# Copyright (C) 2021-2024 The Software Heritage developers
-# 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 pathlib import Path
-import platform
-
-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_prepare(shard_t* shard, uint64_t objects_count);
-int shard_object_write(shard_t* shard, const char* key,
- const char* object, uint64_t object_size);
-int shard_finalize(shard_t* shard);
-
-int shard_load(shard_t* shard);
-int shard_find_object(shard_t *shard, const char *key, uint64_t *object_size);
-int shard_read_object(shard_t *shard, char *object, uint64_t object_size);
-
-int shard_delete(shard_t* shard, const char *key);
-
-extern const int shard_key_len;
-"""
-)
-
-library_dirs = []
-extra_compile_args = ["-D_FILE_OFFSET_BITS=64"]
-bundled_cmph = Path(__file__).parent.parent.parent / "cmph"
-if bundled_cmph.is_dir():
- library_dirs.append(str(bundled_cmph / "lib"))
- extra_compile_args.append(f"-I{bundled_cmph}/include")
-elif platform.system() == "Darwin" and Path("/usr/local/include/cmph.h").is_file():
- # ensure to find cmph on macOS if installed with Homebrew using "brew install libcmph"
- library_dirs.append("/usr/local/lib")
- extra_compile_args.append("-I/usr/local/include")
-elif platform.system() == "Darwin" and Path("/opt/local/include/cmph.h").is_file():
- # ensure to find cmph on macOS if installed with MacPorts using "port install cmph"
- library_dirs.append("/opt/local/lib")
- extra_compile_args.append("-I/opt/local/include")
-
-
-ffibuilder.set_source(
- "swh.shard._shard_cffi",
- """
- #include "swh/shard/shard.h"
- """,
- sources=["swh/shard/shard.c"],
- include_dirs=["."],
- libraries=["cmph"],
- library_dirs=library_dirs,
- extra_compile_args=extra_compile_args,
-) # library name, for the linker
-
-if __name__ == "__main__":
- ffibuilder.compile(verbose=True)
diff --git a/swh/shard/shard.h b/swh/shard/shard.h
deleted file mode 100644
index f54de88..0000000
--- a/swh/shard/shard.h
+++ /dev/null
@@ -1,60 +0,0 @@
-/*
- * Copyright (C) 2021-2022 The Software Heritage developers
- * 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
- */
-
-#include <cmph.h>
-#include <cmph_types.h>
-#include <stdint.h>
-
-#define SHARD_OFFSET_MAGIC 32
-#define SHARD_OFFSET_HEADER 512
-#define SHARD_KEY_LEN 32
-extern const int shard_key_len;
-
-#define SHARD_MAGIC "SWHShard"
-#define SHARD_VERSION 1
-
-typedef struct {
- uint64_t version;
- uint64_t objects_count;
- uint64_t objects_position;
- uint64_t objects_size;
- uint64_t index_position;
- uint64_t index_size;
- uint64_t hash_position;
-} shard_header_t;
-
-typedef struct {
- char key[SHARD_KEY_LEN];
- uint64_t object_offset;
-} shard_index_t;
-
-typedef struct {
- char *path;
- FILE *f;
- shard_header_t header;
- cmph_t *hash;
-
- // The following fields are only used when creating the Read Shard
- cmph_io_adapter_t *source;
- cmph_config_t *config;
- shard_index_t *index;
- uint64_t index_offset;
-} shard_t;
-
-shard_t *shard_init(const char *path);
-int shard_destroy(shard_t *shard);
-
-int shard_prepare(shard_t *shard, uint64_t objects_count);
-int shard_object_write(shard_t *shard, const char *key, const char *object,
- uint64_t object_size);
-int shard_finalize(shard_t *shard);
-
-int shard_load(shard_t *shard);
-int shard_find_object(shard_t *shard, const char *key, uint64_t *object_size);
-int shard_read_object(shard_t *shard, char *object, uint64_t object_size);
-
-int shard_delete(shard_t *shard, const char *key);
\ No newline at end of file
diff --git a/tox.ini b/tox.ini
index 2749266..a319fd3 100644
--- a/tox.ini
+++ b/tox.ini
@@ -8,16 +8,15 @@ envlist =
c
[testenv]
-usedevelop = true
extras =
testing
deps =
pytest-cov
commands =
pytest --doctest-modules \
- --cov=swh/shard \
+ --cov={envsitepackagesdir}/swh/shard \
--cov-branch \
- swh/shard \
+ {envsitepackagesdir}/swh/shard \
{posargs}
[testenv:black]
@@ -25,13 +24,13 @@ skip_install = true
deps =
black==25.1.0
commands =
- {envpython} -m black --check swh
+ {envpython} -m black --check src/swh/shard
[testenv:c]
allowlist_externals = make
usedevelop = true
commands =
- make -C swh/shard check
+ make -C src/_shard check
[testenv:flake8]
skip_install = true
@@ -50,7 +49,7 @@ extras =
deps =
mypy==1.15.0
commands =
- mypy swh
+ mypy -p swh.shard
# build documentation outside swh-environment using the current
# git HEAD of swh-docs, is executed on CI for each diff to prevent
--
GitLab
From 5603bf2f6648ba85660f9f309290eeb463945207 Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Thu, 27 Feb 2025 16:27:22 +0100
Subject: [PATCH 08/24] Add a cli tool to manipulate shard files
Currently, it allows to:
- read the header of the shard file,
- list entries in the shard file (as a list of {key: length}),
- get an object from a shard file,
- create a shard file from a list of files.
---
src/swh/shard/cli.py | 143 ++++++++++++++++++++++++++++++++
src/swh/shard/tests/test_cli.py | 103 +++++++++++++++++++++++
2 files changed, 246 insertions(+)
create mode 100644 src/swh/shard/cli.py
create mode 100644 src/swh/shard/tests/test_cli.py
diff --git a/src/swh/shard/cli.py b/src/swh/shard/cli.py
new file mode 100644
index 0000000..efc2989
--- /dev/null
+++ b/src/swh/shard/cli.py
@@ -0,0 +1,143 @@
+# Copyright (C) 2025 The Software Heritage developers
+# 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
+
+import logging
+
+import click
+
+# WARNING: do not import unnecessary things here to keep cli startup time under
+# control
+
+
+logger = logging.getLogger(__name__)
+
+CONTEXT_SETTINGS = dict(help_option_names=["-h", "--help"])
+
+try:
+ # make this cli usable both from the swh.core's 'swh' cli group and from
+ # direct swh-shard command (since swh-shard does not depend on swh.core)
+ from swh.core.cli import swh
+
+ cli_group = swh.group
+except (ImportError, ModuleNotFoundError):
+ cli_group = click.group
+
+
+@cli_group(name="shard", context_settings=CONTEXT_SETTINGS)
+@click.pass_context
+def shard_cli_group(ctx):
+ """Software Heritage Shard tools."""
+
+
+@shard_cli_group.command("info")
+@click.argument("shard", required=True, nargs=-1)
+@click.pass_context
+def shard_info(ctx, shard):
+ "Display shard file information"
+
+ from swh.shard import Shard
+
+ for shardfile in shard:
+ with Shard(shardfile) as s:
+ h = s.header
+ click.echo(f"Shard {shardfile}")
+ click.echo(f"├─version: {h.version}")
+ click.echo(f"├─objects: {h.objects_count}")
+ click.echo(f"│ ├─position: {h.objects_position}")
+ click.echo(f"│ └─size: {h.objects_size}")
+ click.echo("├─index")
+ click.echo(f"│ ├─position: {h.index_position}")
+ click.echo(f"│ └─size: {h.index_size}")
+ click.echo("└─hash")
+ click.echo(f" └─position: {h.hash_position}")
+
+
+@shard_cli_group.command("create")
+@click.argument("shard", required=True)
+@click.argument("files", metavar="files", required=True, nargs=-1)
+@click.option(
+ "--sorted/--no-sorted",
+ "sort_files",
+ default=False,
+ help=(
+ "Sort files by inversed filename before adding them to the shard; "
+ "it may help having better compression ratio when compressing "
+ "the shard file"
+ ),
+)
+@click.pass_context
+def shard_create(ctx, shard, files, sort_files):
+ "Create a shard file from given files"
+
+ import hashlib
+ import sys
+
+ from swh.shard import ShardCreator
+
+ files = list(files)
+ if files == ["-"]:
+ # read file names from stdin
+ files = [fname.strip() for fname in sys.stdin.read().splitlines()]
+ click.echo(f"There are {len(files)} entries")
+ hashes = set()
+ files_to_add = {}
+ for fname in files:
+ try:
+ data = open(fname, "rb").read()
+ except OSError:
+ continue
+ sha256 = hashlib.sha256(data).digest()
+ if sha256 not in hashes:
+ files_to_add[fname] = sha256
+ hashes.add(sha256)
+ click.echo(f"after deduplication: {len(files_to_add)} entries")
+
+ with ShardCreator(shard, len(files_to_add)) as shard:
+ it = files_to_add.items()
+ if sort_files:
+ it = sorted(it, key=lambda x: x[0][-1::-1])
+ for fname, sha256 in it:
+ data = open(fname, "rb").read()
+ shard.write(sha256, data)
+ click.echo("Done")
+
+
+@shard_cli_group.command("ls")
+@click.argument("shard", required=True)
+@click.pass_context
+def shard_list(ctx, shard):
+ "List objects in a shard file"
+
+ from swh.shard import Shard
+
+ with Shard(shard) as s:
+ for key in s:
+ size = s.getsize(key)
+ click.echo(f"{key.hex()}: {size} bytes")
+
+
+@shard_cli_group.command("get")
+@click.argument("shard", required=True)
+@click.argument("keys", required=True, nargs=-1)
+@click.pass_context
+def shard_get(ctx, shard, keys):
+ "List objects in a shard file"
+
+ from swh.shard import Shard
+
+ with Shard(shard) as s:
+ for key in keys:
+ click.echo(s[bytes.fromhex(key)], nl=False)
+
+
+def main():
+ # Even though swh() sets up logging, we need an earlier basic logging setup
+ # for the next few logging statements
+ logging.basicConfig()
+ return shard_cli_group(auto_envvar_prefix="SWH")
+
+
+if __name__ == "__main__":
+ main()
diff --git a/src/swh/shard/tests/test_cli.py b/src/swh/shard/tests/test_cli.py
new file mode 100644
index 0000000..0ba98b4
--- /dev/null
+++ b/src/swh/shard/tests/test_cli.py
@@ -0,0 +1,103 @@
+# Copyright (C) 2025 The Software Heritage developers
+# 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 hashlib import sha256
+
+from click.testing import CliRunner
+import pytest
+
+from swh.shard import Shard, ShardCreator, cli
+
+
+@pytest.fixture
+def small_shard(tmp_path):
+ with ShardCreator(str(tmp_path / "small.shard"), 16) as shard:
+ for i in range(16):
+ shard.write(bytes.fromhex(f"{i:-064X}"), bytes((65 + i,)) * 42)
+ return tmp_path / "small.shard"
+
+
+def test_cli():
+ runner = CliRunner()
+ result = runner.invoke(cli.shard_cli_group)
+ assert result.exit_code == 0
+ assert "Software Heritage Shard tools" in result.output
+
+
+def test_cli_info(small_shard):
+ runner = CliRunner()
+ result = runner.invoke(cli.shard_info, [str(small_shard)])
+ assert result.exit_code == 0
+ assert (
+ result.output
+ == f"""\
+Shard {small_shard}
+├─version: 1
+├─objects: 16
+│ ├─position: 512
+│ └─size: 800
+├─index
+│ ├─position: 1312
+│ └─size: 680
+└─hash
+ └─position: 1992
+"""
+ )
+
+
+def test_cli_ls(small_shard):
+ runner = CliRunner()
+ result = runner.invoke(cli.shard_list, [str(small_shard)])
+ assert result.exit_code == 0
+ assert (
+ result.output
+ == """\
+000000000000000000000000000000000000000000000000000000000000000c: 42 bytes
+0000000000000000000000000000000000000000000000000000000000000005: 42 bytes
+000000000000000000000000000000000000000000000000000000000000000e: 42 bytes
+0000000000000000000000000000000000000000000000000000000000000001: 42 bytes
+0000000000000000000000000000000000000000000000000000000000000004: 42 bytes
+000000000000000000000000000000000000000000000000000000000000000d: 42 bytes
+0000000000000000000000000000000000000000000000000000000000000000: 42 bytes
+000000000000000000000000000000000000000000000000000000000000000a: 42 bytes
+0000000000000000000000000000000000000000000000000000000000000006: 42 bytes
+0000000000000000000000000000000000000000000000000000000000000009: 42 bytes
+0000000000000000000000000000000000000000000000000000000000000003: 42 bytes
+0000000000000000000000000000000000000000000000000000000000000008: 42 bytes
+000000000000000000000000000000000000000000000000000000000000000f: 42 bytes
+000000000000000000000000000000000000000000000000000000000000000b: 42 bytes
+0000000000000000000000000000000000000000000000000000000000000002: 42 bytes
+0000000000000000000000000000000000000000000000000000000000000007: 42 bytes
+"""
+ )
+
+
+def test_cli_get(small_shard):
+ runner = CliRunner()
+ for i in range(16):
+ result = runner.invoke(cli.shard_get, [str(small_shard), f"{i:-064x}"])
+ assert result.exit_code == 0
+ assert result.output == chr(65 + i) * 42
+
+
+def test_cli_create(tmp_path):
+ runner = CliRunner()
+
+ files = []
+ hashes = []
+ for i in range(16):
+ f = tmp_path / f"file_{i}"
+ data = f"file {i}".encode()
+ f.write_bytes(data)
+ files.append(str(f))
+ hashes.append(sha256(data).digest())
+ shard = tmp_path / "shard"
+ result = runner.invoke(cli.shard_create, [str(shard), *files])
+ assert result.exit_code == 0
+ assert result.output.strip().endswith("Done")
+ with Shard(str(shard)) as s:
+ assert s.header.objects_count == 16
+ # check stored sha256 digests are as expected
+ assert sorted(list(s)) == sorted(hashes)
--
GitLab
From 9b30e63feca987cecb37781ffa134f445c6555d8 Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Thu, 27 Feb 2025 16:28:49 +0100
Subject: [PATCH 09/24] Update the README file with a "Quick Start" section
---
README.rst | 94 ++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 85 insertions(+), 9 deletions(-)
diff --git a/README.rst b/README.rst
index 7c33c18..450adf7 100644
--- a/README.rst
+++ b/README.rst
@@ -1,12 +1,17 @@
-Perfect Hash table for Software Heritage Object Storage
-=======================================================
+Shard File Format for the Software Heritage Object Storage
+==========================================================
-A perfect hash table for software heritage object storage.
+This module implement the support and tooling to manipulate SWH Shard files
+based on a perfect hash table, typically used by the software heritage object
+storage.
-Build dependencies
-------------------
+It is both a Python extension that can be used as a library to manuipulate SWH
+shard files, and a set of command line tools.
-This packages uses cffi to build the wrapper around the cmph minimal perfect
+Quick Start
+-----------
+
+This packages uses pybind11 to build the wrapper around the cmph minimal perfect
hashmap library. To build the binary extension, in addition to the python
development tools, you will need cmph, gtest and valgrind. On de Debian
system, you can install these using:
@@ -15,8 +20,79 @@ system, you can install these using:
sudo apt install build-essential python3-dev libcmph-dev libgtest-dev valgrind lcov
-Then you should be able to build the binary extension:
-.. code-block:: shell
+Command Line Tool
+~~~~~~~~~~~~~~~~~
+
+You may use several methods to install swh-shard, e.g. using `uv`_ or `pip`_.
+
+For example:
+
+.. code-block:: console
+
+ $ uv tool install swh-shard
+ [...]
+ Installed 1 executable: swh-shard
+
+ $ swh-shard
+ Usage: swh-shard [OPTIONS] COMMAND [ARGS]...
+
+ Software Heritage Shard tools.
+
+ Options:
+ -C, --config-file FILE Configuration file.
+ -h, --help Show this message and exit.
+
+ Commands:
+ create Create a shard file from given files
+ get List objects in a shard file
+ info Display shard file information
+ ls List objects in a shard file
+
+Then you can create a shard file from local files:
+
+.. code-block:: console
+
+ $ swh-shard create volume.shard *.py
+ There are 3 entries
+ after deduplication: 3 entries
+ Done
+
+This will use the sha256 checksum of each file content given as argument as key
+in the shard file.
+
+Then you can check the header of the shard file:
+
+.. code-block:: console
+
+ $ swh-shard info volume.shard
+ Shard volume.shard
+ ├─version: 1
+ ├─objects: 3
+ │ ├─position: 512
+ │ └─size: 5633
+ ├─index
+ │ ├─position: 6145
+ │ └─size: 440
+ └─hash
+ └─position: 6585
+
+List the content of a shard:
+
+.. code-block:: console
+
+ $ swh-shard ls volume.shard
+ 8bb71bce4885c526bb4114295f5b2b9a23a50e4a8d554c17418d1874b1a233ac: 834 bytes
+ 06340a7a5fa9e18d72a587a69e4dc7e79f4d6a56632ea6900c22575dc207b07f: 4210 bytes
+ d39790a3af51286d2d10d73e72e2447cf97b149ff2d8e275b200a1ee33e4a3c5: 565 bytes
+
+And retrieve an object from a shard:
+
+.. code-block:: console
+
+ $ swh-shard get volume.shard 06340a7a5fa9e18d72a587a69e4dc7e79f4d6a56632ea6900c22575dc207b07f | sha256sum
+ 06340a7a5fa9e18d72a587a69e4dc7e79f4d6a56632ea6900c22575dc207b07f -
+
- python -m build
+.. _`uv`: https://docs.astral.sh/uv/
+.. _`pip`: https://pip.pypa.io/
--
GitLab
From ec3b26f5391489efc4ee06fa6ea21b7e68df1971 Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Fri, 28 Feb 2025 12:06:26 +0100
Subject: [PATCH 10/24] cli: add a 'swh-shard delete' command
---
README.rst | 13 +++-
src/swh/shard/cli.py | 60 +++++++++++++++++
src/swh/shard/tests/test_cli.py | 112 ++++++++++++++++++++++++++++++++
3 files changed, 184 insertions(+), 1 deletion(-)
diff --git a/README.rst b/README.rst
index 450adf7..f3e0bd4 100644
--- a/README.rst
+++ b/README.rst
@@ -86,13 +86,24 @@ List the content of a shard:
06340a7a5fa9e18d72a587a69e4dc7e79f4d6a56632ea6900c22575dc207b07f: 4210 bytes
d39790a3af51286d2d10d73e72e2447cf97b149ff2d8e275b200a1ee33e4a3c5: 565 bytes
-And retrieve an object from a shard:
+Retrieve an object from a shard:
.. code-block:: console
$ swh-shard get volume.shard 06340a7a5fa9e18d72a587a69e4dc7e79f4d6a56632ea6900c22575dc207b07f | sha256sum
06340a7a5fa9e18d72a587a69e4dc7e79f4d6a56632ea6900c22575dc207b07f -
+And delete one or more objects from a shard:
+
+.. code-block:: console
+
+ $ swh-shard delete volume.shard 06340a7a5fa9e18d72a587a69e4dc7e79f4d6a56632ea6900c22575dc207b07f
+ About to remove these objects from the shard file misc/volume.shard
+ 06340a7a5fa9e18d72a587a69e4dc7e79f4d6a56632ea6900c22575dc207b07f (4210 bytes)
+ Proceed? [y/N]: y
+ Deleting objects from the shard [####################################] 100%
+ Done
+
.. _`uv`: https://docs.astral.sh/uv/
.. _`pip`: https://pip.pypa.io/
diff --git a/src/swh/shard/cli.py b/src/swh/shard/cli.py
index efc2989..01438cd 100644
--- a/src/swh/shard/cli.py
+++ b/src/swh/shard/cli.py
@@ -132,6 +132,66 @@ def shard_get(ctx, shard, keys):
click.echo(s[bytes.fromhex(key)], nl=False)
+@shard_cli_group.command("delete")
+@click.argument("shard", required=True)
+@click.argument("keys", required=True, nargs=-1)
+@click.option(
+ "--confirm/--no-confirm",
+ default=True,
+ help="Ask for confirmation before performing the deletion",
+)
+@click.pass_context
+def shard_delete(ctx, shard, keys, confirm):
+ """Delete objects from a shard file
+
+ Keys to delete from the shard file are expected to be given as hex
+ representation. If there is only one argument '-', then read the list of
+ keys from stdin. Implies --no-confirm.
+
+ If at least one key is missing or invalid, the whole process is aborted.
+
+ """
+ import sys
+
+ if keys == ("-",):
+ keys = sys.stdin.read().split()
+ confirm = False
+ if len(set(keys)) < len(keys):
+ click.fail("There are duplicate keys, aborting")
+
+ from swh.shard import Shard
+
+ obj_size = {}
+ with Shard(shard) as s:
+ for key in keys:
+ try:
+ obj_size[key] = s.getsize(bytes.fromhex(key))
+ except ValueError:
+ click.secho(f"{key}: key is invalid", fg="red")
+ except KeyError:
+ click.secho(f"{key}: key not found", fg="red")
+ if len(obj_size) < len(keys):
+ raise click.ClickException(
+ "There have been errors for at least one key, aborting"
+ )
+ click.echo(f"About to remove these objects from the shard file {shard}")
+ for key in keys:
+ click.echo(f"{key} ({obj_size[key]} bytes)")
+ if confirm:
+ click.confirm(
+ click.style(
+ "Proceed?",
+ fg="yellow",
+ bold=True,
+ ),
+ abort=True,
+ )
+ with click.progressbar(keys, label="Deleting objects from the shard") as barkeys:
+ for key in barkeys:
+ Shard.delete(shard, bytes.fromhex(key))
+ click.echo("Done")
+
+
def main():
# Even though swh() sets up logging, we need an earlier basic logging setup
# for the next few logging statements
diff --git a/src/swh/shard/tests/test_cli.py b/src/swh/shard/tests/test_cli.py
index 0ba98b4..2042634 100644
--- a/src/swh/shard/tests/test_cli.py
+++ b/src/swh/shard/tests/test_cli.py
@@ -101,3 +101,115 @@ def test_cli_create(tmp_path):
assert s.header.objects_count == 16
# check stored sha256 digests are as expected
assert sorted(list(s)) == sorted(hashes)
+
+
+def test_cli_delete_one_abort(small_shard):
+ runner = CliRunner()
+ key_num = 5
+ key = f"{key_num:-064X}"
+ result = runner.invoke(
+ cli.shard_delete,
+ [str(small_shard), key],
+ input="n\n",
+ )
+ assert result.exit_code == 1, result.output
+ assert "Proceed? [y/N]" in result.output
+ assert "Aborted!" in result.output
+
+ result = runner.invoke(cli.shard_get, [str(small_shard), key])
+ assert result.exit_code == 0
+ assert result.output == chr(65 + key_num) * 42
+
+
+def test_cli_delete_invalid_key_abort(small_shard):
+ runner = CliRunner()
+ keys = [f"{i:-064x}" for i in range(5)]
+ keys.append("00" * 16)
+ result = runner.invoke(
+ cli.shard_delete,
+ [str(small_shard), *keys],
+ )
+ assert result.exit_code == 1, result.output
+ assert "key is invalid" in result.output
+ assert "aborting" in result.output
+
+
+def test_cli_delete_unknown_key_abort(small_shard):
+ runner = CliRunner()
+ keys = [f"{i:-064x}" for i in range(5)]
+ keys.append("01" * 32)
+ result = runner.invoke(
+ cli.shard_delete,
+ [str(small_shard), *keys],
+ )
+ assert result.exit_code == 1, result.output
+ assert "key not found" in result.output
+ assert "aborting" in result.output
+
+
+@pytest.mark.parametrize("key_nums", [(5,), (1, 3, 5), tuple(range(16))])
+def test_cli_delete_confirm(small_shard, key_nums):
+ runner = CliRunner()
+ keys = [f"{key_num:-064x}" for key_num in key_nums]
+ result = runner.invoke(
+ cli.shard_delete,
+ [str(small_shard), *keys],
+ input="y\n",
+ )
+ assert result.exit_code == 0, result.output
+ assert "Proceed? [y/N]" in result.output
+ assert "Done" in result.output
+
+ result = runner.invoke(cli.shard_list, [str(small_shard)])
+ assert result.exit_code == 0
+ for i in range(16):
+ key = f"{i:-064x}"
+ if i in key_nums:
+ assert key not in result.output
+ else:
+ assert key in result.output
+
+
+@pytest.mark.parametrize("key_nums", [(5,), (1, 3, 5), tuple(range(16))])
+def test_cli_delete_from_stdin(small_shard, key_nums):
+ runner = CliRunner()
+ keys = [f"{key_num:-064x}" for key_num in key_nums]
+ result = runner.invoke(
+ cli.shard_delete,
+ [str(small_shard), "-"],
+ input="\n".join(keys),
+ )
+ assert result.exit_code == 0, result.output
+ assert "Proceed? [y/N]" not in result.output
+ assert "Done" in result.output
+
+ result = runner.invoke(cli.shard_list, [str(small_shard)])
+ assert result.exit_code == 0
+ for i in range(16):
+ key = f"{i:-064x}"
+ if i in key_nums:
+ assert key not in result.output
+ else:
+ assert key in result.output
+
+
+def test_cli_delete_one_no_confirm(small_shard):
+ runner = CliRunner()
+ key_num = 5
+ key = f"{key_num:-064x}"
+ result = runner.invoke(
+ cli.shard_delete,
+ ["--no-confirm", str(small_shard), key],
+ )
+ assert result.exit_code == 0, result.output
+ assert "Proceed? [y/N]" not in result.output
+ assert "Done" in result.output
+
+ result = runner.invoke(cli.shard_list, [str(small_shard)])
+ assert result.exit_code == 0
+ for i in range(16):
+ key = f"{i:-064x}"
+ if i == key_num:
+ assert key not in result.output
+ else:
+ assert key in result.output
--
GitLab
From 7206a1ff02bdefd08c7bf315813003c1dfc4486a Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Fri, 28 Feb 2025 12:23:59 +0100
Subject: [PATCH 11/24] cli: use click.Path argument type for the 'shard'
argument
---
src/swh/shard/cli.py | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/src/swh/shard/cli.py b/src/swh/shard/cli.py
index 01438cd..cd683ca 100644
--- a/src/swh/shard/cli.py
+++ b/src/swh/shard/cli.py
@@ -32,7 +32,9 @@ def shard_cli_group(ctx):
@shard_cli_group.command("info")
-@click.argument("shard", required=True, nargs=-1)
+@click.argument(
+ "shard", required=True, nargs=-1, type=click.Path(exists=True, dir_okay=False)
+)
@click.pass_context
def shard_info(ctx, shard):
"Display shard file information"
@@ -55,7 +57,9 @@ def shard_info(ctx, shard):
@shard_cli_group.command("create")
-@click.argument("shard", required=True)
+@click.argument(
+ "shard", required=True, type=click.Path(exists=False, dir_okay=False, writable=True)
+)
@click.argument("files", metavar="files", required=True, nargs=-1)
@click.option(
"--sorted/--no-sorted",
@@ -72,10 +76,14 @@ def shard_create(ctx, shard, files, sort_files):
"Create a shard file from given files"
import hashlib
+ import os
import sys
from swh.shard import ShardCreator
+ if os.path.exists(shard):
+ raise click.ClickException(f"Shard file {shard} already exists. Aborted!")
+
files = list(files)
if files == ["-"]:
# read file names from stdin
@@ -105,7 +113,7 @@ def shard_create(ctx, shard, files, sort_files):
@shard_cli_group.command("ls")
-@click.argument("shard", required=True)
+@click.argument("shard", required=True, type=click.Path(exists=True, dir_okay=False))
@click.pass_context
def shard_list(ctx, shard):
"List objects in a shard file"
@@ -119,7 +127,7 @@ def shard_list(ctx, shard):
@shard_cli_group.command("get")
-@click.argument("shard", required=True)
+@click.argument("shard", required=True, type=click.Path(exists=True, dir_okay=False))
@click.argument("keys", required=True, nargs=-1)
@click.pass_context
def shard_get(ctx, shard, keys):
@@ -133,7 +141,9 @@ def shard_get(ctx, shard, keys):
@shard_cli_group.command("delete")
-@click.argument("shard", required=True)
+@click.argument(
+ "shard", required=True, type=click.Path(exists=True, dir_okay=False, writable=True)
+)
@click.argument("keys", required=True, nargs=-1)
@click.option(
"--confirm/--no-confirm",
--
GitLab
From 62b005c1b6460133bd8c55685639ef36350f67ef Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Fri, 28 Feb 2025 12:26:08 +0100
Subject: [PATCH 12/24] cli: add progress bars to the 'swh-shard create'
command
---
README.rst | 2 ++
src/swh/shard/cli.py | 26 ++++++++++++++------------
2 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/README.rst b/README.rst
index f3e0bd4..c95bf37 100644
--- a/README.rst
+++ b/README.rst
@@ -55,7 +55,9 @@ Then you can create a shard file from local files:
$ swh-shard create volume.shard *.py
There are 3 entries
+ Checking files to add [####################################] 100%
after deduplication: 3 entries
+ Adding files to the shard [####################################] 100%
Done
This will use the sha256 checksum of each file content given as argument as key
diff --git a/src/swh/shard/cli.py b/src/swh/shard/cli.py
index cd683ca..2746dec 100644
--- a/src/swh/shard/cli.py
+++ b/src/swh/shard/cli.py
@@ -91,24 +91,26 @@ def shard_create(ctx, shard, files, sort_files):
click.echo(f"There are {len(files)} entries")
hashes = set()
files_to_add = {}
- for fname in files:
- try:
- data = open(fname, "rb").read()
- except OSError:
- continue
- sha256 = hashlib.sha256(data).digest()
- if sha256 not in hashes:
- files_to_add[fname] = sha256
- hashes.add(sha256)
+ with click.progressbar(files, label="Checking files to add") as bfiles:
+ for fname in bfiles:
+ try:
+ data = open(fname, "rb").read()
+ except OSError:
+ continue
+ sha256 = hashlib.sha256(data).digest()
+ if sha256 not in hashes:
+ files_to_add[fname] = sha256
+ hashes.add(sha256)
click.echo(f"after deduplication: {len(files_to_add)} entries")
with ShardCreator(shard, len(files_to_add)) as shard:
it = files_to_add.items()
if sort_files:
it = sorted(it, key=lambda x: x[0][-1::-1])
- for fname, sha256 in it:
- data = open(fname, "rb").read()
- shard.write(sha256, data)
+ with click.progressbar(it, label="Adding files to the shard") as items:
+ for fname, sha256 in items:
+ data = open(fname, "rb").read()
+ shard.write(sha256, data)
click.echo("Done")
--
GitLab
From 1cb397084959c46520bb9d7ceff4781ae375c7cc Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Fri, 28 Feb 2025 16:41:07 +0100
Subject: [PATCH 13/24] Restore cibuildwheel tooling
Adapt the cmake file to make it work under cibuildwheel.
Also fix the ShardReader::getitem() to make it buildable on 32 bits
systems.
---
CMakeLists.txt | 28 +++++++++++++++++++++++++---
build_cmph.sh | 25 +++++++++++++++++++++++++
pyproject.toml | 12 +++++++++++-
src/_shard/bindings.cpp | 17 +++++++++++++----
4 files changed, 74 insertions(+), 8 deletions(-)
create mode 100755 build_cmph.sh
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 4495f7a..97d976d 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -4,16 +4,38 @@ project(${SKBUILD_PROJECT_NAME}
LANGUAGES C CXX)
include(FindPkgConfig)
+set(CMAKE_CXX_STANDARD 17)
+set(CMAKE_CXX_STANDARD_REQUIRED ON)
+
set(PYBIND11_FINDPYTHON ON)
find_package(Python REQUIRED COMPONENTS Interpreter Development.Module)
find_package(pybind11 CONFIG REQUIRED)
-pkg_search_module(CMPH REQUIRED cmph)
-link_directories(${CMPH_INCLUDE_DIR})
+pkg_search_module(CMPH cmph)
+
+if (NOT CMPH_MODULE_NAME)
+ find_path(CMPH_INCLUDEDIR cmph.h
+ HINTS
+ cmph/include
+ )
+ find_library(CMPH_LIBRARY NAMES cmph libcmph
+ HINTS
+ cmph/lib
+ )
+endif()
+
+if (NOT CMPH_INCLUDEDIR)
+ message(FATAL_ERROR "cmph not found")
+else()
+ message(STATUS "cmph include dir: ${CMPH_INCLUDEDIR}")
+ message(STATUS "cmph lib: ${CMPH_LDFLAGS}")
+endif()
+
+include_directories(${CMPH_INCLUDEDIR})
# Add a library using FindPython's tooling (pybind11 also provides a helper like
# this)
python_add_library(_shard MODULE src/_shard/bindings.cpp src/_shard/shard.c WITH_SOABI)
-target_link_libraries(_shard PRIVATE pybind11::headers cmph ${CMPH_INCLUDE_DIR})
+target_link_libraries(_shard PRIVATE pybind11::headers ${CMPH_LDFLAGS})
# This is passing in the version as a define just as an example
target_compile_definitions(_shard PRIVATE VERSION_INFO=${PROJECT_VERSION})
diff --git a/build_cmph.sh b/build_cmph.sh
new file mode 100755
index 0000000..fec78a0
--- /dev/null
+++ b/build_cmph.sh
@@ -0,0 +1,25 @@
+#!/usr/bin/env bash
+# Copyright (C) 2021-2025 The Software Heritage developers
+# 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
+#
+# This script is used by cibuildwheel to install and compile the cmph library
+# when building manylinux wheels
+
+set -e
+
+CMPH_VERSION=2.0.2
+PREFIX="$(readlink -f $(dirname $0))/cmph"
+
+rm -rf "$PREFIX"
+mkdir "$PREFIX"
+cd "$PREFIX"
+wget https://deac-ams.dl.sourceforge.net/project/cmph/v${CMPH_VERSION}/cmph-${CMPH_VERSION}.tar.gz -O cmph.tar.gz
+tar xf cmph.tar.gz
+
+cd cmph-${CMPH_VERSION}
+
+./configure --prefix="$PREFIX"
+make -j8
+make install
diff --git a/pyproject.toml b/pyproject.toml
index eeaed3d..f3d8c21 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -6,7 +6,7 @@ authors = [
description = "Software Heritage Shard File Format"
readme = {file = "README.rst", content-type = "text/x-rst"}
-requires-python = ">=3.7"
+requires-python = ">=3.9"
classifiers = [
"Programming Language :: Python :: 3",
"Intended Audience :: Developers",
@@ -103,3 +103,13 @@ consider_namespace_packages = true
markers = [
"setrlimit(*limits): Set resource limits for the current test",
]
+
+[tool.cibuildwheel]
+before-all = "yum install -y wget && ./build_cmph.sh"
+
+[[tool.cibuildwheel.overrides]]
+select = "*-musllinux*"
+before-all = "apk add wget && ./build_cmph.sh"
+
+[tool.cibuildwheel.environment]
+LD_LIBRARY_PATH = "/project/cmph/lib"
diff --git a/src/_shard/bindings.cpp b/src/_shard/bindings.cpp
index 7bb9123..a591e00 100644
--- a/src/_shard/bindings.cpp
+++ b/src/_shard/bindings.cpp
@@ -7,6 +7,7 @@
#include "shard.h"
#include <errno.h>
+#include <limits.h>
#include <pybind11/pybind11.h>
#include <pybind11/pytypes.h>
#include <pybind11/stl.h>
@@ -111,12 +112,20 @@ class ShardReader {
py::bytes getitem(py::bytes key) {
// get size and position file descriptor at the beginning of the object
uint64_t size = getsize(key);
- // TODO: get rid of this tmp malloc...
- char *buf = new char[size];
+ if (size > (uint64_t)SSIZE_MAX) {
+ PyErr_SetString(PyExc_ValueError,
+ "Object size overflows python bytes max size "
+ "(are you still using a 32bits system?)");
+ throw py::error_already_set();
+ }
+ ssize_t bufsize = size;
+ // TODO: get rid of this tmp malloc... maybe return a buffer instead of
+ // a bytes would help...
+ char *buf = new char[bufsize];
if (shard_read_object(this->shard, buf, size) != 0)
throw std::runtime_error(
- "content read failed. Shard file might be corrupted.");
- py::bytes b = py::bytes(buf, size);
+ "Content read failed. Shard file might be corrupted.");
+ py::bytes b = py::bytes(buf, bufsize);
delete buf;
return b;
}
--
GitLab
From dbd2d0d065c66953d7994d71933cd5c74748dd50 Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Mon, 3 Mar 2025 16:31:23 +0100
Subject: [PATCH 14/24] bindings: get rid of the temp malloc/memcpy to feed the
returned bytes in SharReader::getitem()
---
src/_shard/bindings.cpp | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/_shard/bindings.cpp b/src/_shard/bindings.cpp
index a591e00..c6ecd18 100644
--- a/src/_shard/bindings.cpp
+++ b/src/_shard/bindings.cpp
@@ -119,14 +119,14 @@ class ShardReader {
throw py::error_already_set();
}
ssize_t bufsize = size;
- // TODO: get rid of this tmp malloc... maybe return a buffer instead of
- // a bytes would help...
- char *buf = new char[bufsize];
+ // instantiate a py::bytes of required size
+ py::bytes b = py::bytes(NULL, bufsize);
+ // string_view.data() returns a const pointer, so enforce the cast to a
+ // char* (yep, that's not nice...)
+ char *buf = (char *)std::string_view(b).data();
if (shard_read_object(this->shard, buf, size) != 0)
throw std::runtime_error(
"Content read failed. Shard file might be corrupted.");
- py::bytes b = py::bytes(buf, bufsize);
- delete buf;
return b;
}
void getindex(uint64_t pos, shard_index_t &idx) {
--
GitLab
From 2617ed0146a7726a5ee19e3ed6b86ea3465eb9a4 Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Fri, 21 Mar 2025 10:29:06 +0100
Subject: [PATCH 15/24] bindings: Several improvements
- Pass py::bytes parameters as const reference when applicable
- Use C* includes
- Use string_views when possible (prevent copies)
---
src/_shard/bindings.cpp | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/src/_shard/bindings.cpp b/src/_shard/bindings.cpp
index c6ecd18..7c6e266 100644
--- a/src/_shard/bindings.cpp
+++ b/src/_shard/bindings.cpp
@@ -6,12 +6,12 @@
*/
#include "shard.h"
-#include <errno.h>
-#include <limits.h>
+#include <cerrno>
+#include <climits>
+#include <cstring>
#include <pybind11/pybind11.h>
#include <pybind11/pytypes.h>
#include <pybind11/stl.h>
-#include <string.h>
namespace py = pybind11;
@@ -24,21 +24,20 @@ class ShardCreator {
this->shard = shard_init(path.c_str());
}
~ShardCreator() { shard_destroy(this->shard); }
- void write(py::bytes key, py::bytes object) {
+ void write(const py::bytes key, const py::bytes object) {
if (n_registered >= n_entries) {
throw py::value_error(
"The declared number of objects has already been written");
}
- std::string kbuf = std::string(key);
+ std::string_view kbuf = key;
if (kbuf.size() != SHARD_KEY_LEN) {
throw std::length_error(
"Invalid key size: "s + std::to_string(kbuf.size()) +
" (expected: " + std::to_string(SHARD_KEY_LEN) + ")");
}
- // Not sure whether this does a copy or not...
- std::string sv = object;
+ std::string_view sv = object;
errno = 0;
- if (shard_object_write(this->shard, kbuf.c_str(), sv.c_str(),
+ if (shard_object_write(this->shard, kbuf.data(), sv.data(),
sv.size()) != 0) {
PyErr_SetFromErrno(PyExc_OSError);
throw py::error_already_set();
@@ -109,7 +108,7 @@ class ShardReader {
int ret = shard_close(this->shard);
return ret;
}
- py::bytes getitem(py::bytes key) {
+ py::bytes getitem(const py::bytes key) {
// get size and position file descriptor at the beginning of the object
uint64_t size = getsize(key);
if (size > (uint64_t)SSIZE_MAX) {
@@ -142,7 +141,7 @@ class ShardReader {
throw py::error_already_set();
}
}
- uint64_t getsize(py::bytes key) {
+ uint64_t getsize(const py::bytes key) {
std::string kbuf = std::string(key);
if (kbuf.size() != SHARD_KEY_LEN) {
throw std::length_error(
@@ -193,8 +192,8 @@ PYBIND11_MODULE(_shard, m) {
})
.def("getsize", &ShardReader::getsize)
.def("delete",
- [](const std::string &path, py::bytes key) {
- std::string kbuf = std::string(key);
+ [](const std::string &path, const py::bytes key) {
+ std::string_view kbuf = key;
if (kbuf.size() != SHARD_KEY_LEN) {
throw std::length_error(
"Invalid key size: "s + std::to_string(kbuf.size()) +
@@ -204,8 +203,8 @@ PYBIND11_MODULE(_shard, m) {
shard_delete(reader.shard, kbuf.data());
})
.def("find",
- [](ShardReader &s, py::bytes key) {
- std::string kbuf = std::string(key);
+ [](ShardReader &s, const py::bytes key) {
+ std::string_view kbuf = key;
if (kbuf.size() != SHARD_KEY_LEN) {
throw std::length_error(
"Invalid key size: "s + std::to_string(kbuf.size()) +
--
GitLab
From c300fe2700cfb906cc719322bc07c00f33753abf Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Tue, 4 Mar 2025 10:15:11 +0100
Subject: [PATCH 16/24] Use format macro constants for printf logging when
applicable
---
src/_shard/shard.c | 50 ++++++++++++++++++++++++++++------------------
1 file changed, 31 insertions(+), 19 deletions(-)
diff --git a/src/_shard/shard.c b/src/_shard/shard.c
index 175980f..0c2654c 100644
--- a/src/_shard/shard.c
+++ b/src/_shard/shard.c
@@ -11,6 +11,7 @@ extern "C" {
#include <assert.h>
#include <errno.h>
#include <fcntl.h>
+#include <inttypes.h>
#include <limits.h>
#include <memory.h>
#include <string.h>
@@ -67,13 +68,14 @@ int shard_close(shard_t *shard) {
int shard_seek(shard_t *shard, uint64_t offset, int whence) {
if (offset > INT64_MAX) {
- printf("shard_seek: %lu > %ld (INT64_MAX)", offset, INT64_MAX);
+ printf("shard_seek: %" PRIu64 " > %" PRId64 " (INT64_MAX)", offset,
+ INT64_MAX);
return -1;
}
int r = fseeko(shard->f, offset, whence);
if (r < 0)
- printf("shard_seek: fseeko(%p, %lu, %d): %s\n", shard->f, offset,
- whence, strerror(errno));
+ printf("shard_seek: fseeko(%p, %" PRIu64 ", %d): %s\n", shard->f,
+ offset, whence, strerror(errno));
return r;
}
@@ -87,7 +89,8 @@ uint64_t shard_tell(shard_t *shard) {
int shard_read(shard_t *shard, void *ptr, uint64_t size) {
uint64_t read;
if ((read = fread(ptr, 1, size, shard->f)) != size) {
- printf("shard_read: read %lu instead of %lu\n", read, size);
+ printf("shard_read: read %" PRIu64 " instead of %" PRIu64 "\n", read,
+ size);
return -1;
}
return 0;
@@ -106,7 +109,8 @@ int shard_read_uint64_t(shard_t *shard, uint64_t *ptr) {
int shard_write(shard_t *shard, const void *ptr, uint64_t nmemb) {
uint64_t wrote;
if ((wrote = fwrite(ptr, 1, nmemb, shard->f)) != nmemb) {
- printf("shard_write: wrote %lu instead of %lu\n", wrote, nmemb);
+ printf("shard_write: wrote %" PRIu64 " instead of %" PRIu64 "\n", wrote,
+ nmemb);
return -1;
}
return 0;
@@ -168,7 +172,8 @@ int shard_magic_save(shard_t *shard) {
*/
int shard_header_print(shard_header_t *header) {
-#define PRINT(name) debug("shard_header_print: " #name " %lu\n", header->name)
+#define PRINT(name) \
+ debug("shard_header_print: " #name " %" PRIu64 "\n", header->name)
PRINT(version);
PRINT(objects_count);
PRINT(objects_position);
@@ -202,7 +207,8 @@ int shard_header_load(shard_t *shard) {
#undef LOAD
shard_header_print(&shard->header);
if (shard->header.version != SHARD_VERSION) {
- printf("shard_header_load: unexpected version, got %lu instead of %d\n",
+ printf("shard_header_load: unexpected version, got %" PRIu64
+ " instead of %d\n",
shard->header.version, SHARD_VERSION);
return -1;
}
@@ -247,7 +253,8 @@ int shard_header_reset(shard_header_t *header) {
int shard_object_write(shard_t *shard, const char *key, const char *object,
uint64_t object_size) {
// save key & index to later build the hash
- debug("shard_object_write: index_offset=%lu\n", shard->index_offset);
+ debug("shard_object_write: index_offset=%" PRIu64 "\n",
+ shard->index_offset);
shard_index_t *index = &shard->index[shard->index_offset];
memcpy((void *)index->key, key, SHARD_KEY_LEN);
index->object_offset = shard_tell(shard);
@@ -313,7 +320,7 @@ int shard_hash_create(shard_t *shard) {
int shard_index_save(shard_t *shard) {
shard->header.index_position =
shard->header.objects_position + shard->header.objects_size;
- debug("shard_index_save: index_position %lu\n",
+ debug("shard_index_save: index_position %" PRIu64 "\n",
shard->header.index_position);
assert(shard->header.index_position == shard_tell(shard));
cmph_uint32 count = cmph_size(shard->hash);
@@ -332,14 +339,16 @@ int shard_index_save(shard_t *shard) {
for (uint64_t i = 0; i < shard->index_offset; i++) {
cmph_uint32 h =
cmph_search(shard->hash, shard->index[i].key, SHARD_KEY_LEN);
- debug("shard_index_save: i = %lu, h = %d, offset = %lu\n", i, h,
- shard->index[i].object_offset);
+ debug("shard_index_save: i = %" PRIu64 ", h = %d, offset = %" PRIu64
+ "\n",
+ i, h, shard->index[i].object_offset);
assert(h < count);
memcpy(index[h].key, shard->index[i].key, SHARD_KEY_LEN);
index[h].object_offset = htonq(shard->index[i].object_offset);
}
uint64_t index_size = shard->header.index_size;
- debug("shard_index_save: save %lu index bytes at position %lu\n",
+ debug("shard_index_save: save %" PRIu64 " index bytes at position %" PRIu64
+ "\n",
index_size, shard->header.index_position);
if (shard_write(shard, (void *)index, index_size) < 0) {
printf("shard_index_save\n");
@@ -375,7 +384,8 @@ int shard_index_get(shard_t *shard, uint64_t pos, shard_index_t *idx) {
int shard_hash_save(shard_t *shard) {
shard->header.hash_position =
shard->header.index_position + shard->header.index_size;
- debug("shard_hash_save: hash_position %lu\n", shard->header.hash_position);
+ debug("shard_hash_save: hash_position %" PRIu64 "\n",
+ shard->header.hash_position);
cmph_dump(shard->hash, shard->f);
return 0;
}
@@ -433,7 +443,7 @@ int shard_reset(shard_t *shard) {
}
int shard_prepare(shard_t *shard, uint64_t objects_count) {
- debug("shard_prepare: objects=%lu\n", objects_count);
+ debug("shard_prepare: objects=%" PRIu64 "\n", objects_count);
if (shard_open(shard, "w+") < 0)
return -1;
if (shard_reset(shard) < 0)
@@ -454,7 +464,7 @@ int shard_find_object(shard_t *shard, const char *key, uint64_t *object_size) {
debug("shard_find_object: h = %d\n", h);
uint64_t index_offset =
shard->header.index_position + h * sizeof(shard_index_t);
- debug("shard_find_object: index_offset = %lu\n", index_offset);
+ debug("shard_find_object: index_offset = %" PRIu64 "\n", index_offset);
if (shard_seek(shard, index_offset, SEEK_SET) < 0) {
printf("shard_find_object: index_offset\n");
return -1;
@@ -469,7 +479,7 @@ int shard_find_object(shard_t *shard, const char *key, uint64_t *object_size) {
printf("shard_find_object: object_offset\n");
return -1;
}
- debug("shard_find_object: object_offset = %lu\n", object_offset);
+ debug("shard_find_object: object_offset = %" PRIu64 "\n", object_offset);
/* Has the object been deleted? */
if (object_offset == UINT64_MAX) {
return 1;
@@ -488,7 +498,7 @@ int shard_find_object(shard_t *shard, const char *key, uint64_t *object_size) {
printf("shard_find_object: object_size\n");
return -1;
}
- debug("shard_find_object: object_size = %lu\n", *object_size);
+ debug("shard_find_object: object_size = %" PRIu64 "\n", *object_size);
return 0;
}
@@ -505,7 +515,8 @@ int shard_hash_load(shard_t *shard) {
printf("shard_hash_load\n");
return -1;
}
- debug("shard_hash_load: hash_position %lu\n", shard->header.hash_position);
+ debug("shard_hash_load: hash_position %" PRIu64 "\n",
+ shard->header.hash_position);
shard->hash = cmph_load(shard->f);
if (shard->hash == NULL) {
printf("shard_hash_load: cmph_load\n");
@@ -594,7 +605,8 @@ int shard_delete(shard_t *shard, const char *key) {
printf("shard_delete: object_size\n");
return -1;
}
- debug("shard_delete: filling object size and data (len: %lu) with zeros\n",
+ debug("shard_delete: filling object size and data (len: %" PRIu64
+ ") with zeros\n",
object_size);
if (shard_seek(shard, object_offset, SEEK_SET) < 0) {
printf("shard_delete: object_offset fill\n");
--
GitLab
From 288daf6f80488acb82069edcb34c280c8bbe50c6 Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Tue, 4 Mar 2025 11:28:21 +0100
Subject: [PATCH 17/24] Better check for malloc failure when allocating index
tables
Also explicitly validate the given objects_count to be below reasonable
threshold.
---
src/_shard/shard.c | 18 +++++++++++++++++-
src/_shard/shard.h | 1 +
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/src/_shard/shard.c b/src/_shard/shard.c
index 0c2654c..b129b53 100644
--- a/src/_shard/shard.c
+++ b/src/_shard/shard.c
@@ -329,8 +329,13 @@ int shard_index_save(shard_t *shard) {
// map)", so we have to initialize the table of index entries with explicit
// "invalid" entries (aka {key=0x00, offset=MAX_INT})
debug("shard_index_save: count = %d\n", count);
+ shard_index_t *index =
+ (shard_index_t *)calloc(count, sizeof(shard_index_t));
+ if (index == NULL) {
+ printf("shard_index_save: could not allocate memory for the index");
+ return -1;
+ }
shard->header.index_size = count * sizeof(shard_index_t);
- shard_index_t *index = (shard_index_t *)calloc(1, shard->header.index_size);
// initialize all the index entries as "deleted" entries by default, the
// actual entries will be filled just below.
for (uint64_t i = 0; i < count; i++) {
@@ -443,7 +448,14 @@ int shard_reset(shard_t *shard) {
}
int shard_prepare(shard_t *shard, uint64_t objects_count) {
+ // this is used only when creating a new shard
debug("shard_prepare: objects=%" PRIu64 "\n", objects_count);
+ if (objects_count > SHARD_MAX_OBJECTS) {
+ printf("shard_prepare: objects_count too big: %" PRIu64
+ " exceeds max value %" PRIu64,
+ objects_count, SHARD_MAX_OBJECTS);
+ return -1;
+ }
if (shard_open(shard, "w+") < 0)
return -1;
if (shard_reset(shard) < 0)
@@ -451,6 +463,10 @@ int shard_prepare(shard_t *shard, uint64_t objects_count) {
shard->header.objects_count = objects_count;
shard->index =
(shard_index_t *)malloc(sizeof(shard_index_t) * objects_count);
+ if (shard->index == NULL) {
+ printf("shard_prepare: cannot allocate memory for the index");
+ return -1;
+ }
return 0;
}
diff --git a/src/_shard/shard.h b/src/_shard/shard.h
index bb66114..14f1dae 100644
--- a/src/_shard/shard.h
+++ b/src/_shard/shard.h
@@ -16,6 +16,7 @@ extern "C" {
#define SHARD_OFFSET_MAGIC 32
#define SHARD_OFFSET_HEADER 512
#define SHARD_KEY_LEN 32
+#define SHARD_MAX_OBJECTS (SIZE_MAX / (SHARD_KEY_LEN + sizeof(shard_index_t)))
extern const int shard_key_len;
#define SHARD_MAGIC "SWHShard"
--
GitLab
From 47264890f044d881e6d04c6041103352222566b4 Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Tue, 18 Mar 2025 13:12:34 +0100
Subject: [PATCH 18/24] Disable strict editable mode
This is not supported by scikit-build-core but remains the default for
swh-environment, so explicitly disable it for this package.
---
pyproject.toml | 1 +
1 file changed, 1 insertion(+)
diff --git a/pyproject.toml b/pyproject.toml
index f3d8c21..460c42a 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -32,6 +32,7 @@ build-backend = "scikit_build_core.build"
[tool.scikit-build]
minimum-version = "build-system.requires"
wheel.exclude = ["*.c", "*.cpp", "*.h", ".clang-format", "*.o"]
+strict-config = false
[tool.scikit-build.wheel.packages]
"swh" = "src/swh"
--
GitLab
From ede2c5aa2d4efb6efede43aab9bec20912a76a83 Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Fri, 21 Mar 2025 10:30:52 +0100
Subject: [PATCH 19/24] Fix the lcov call for lcov>=2 for the check target
Debian bookwork comes with lcov 1.16 but recent Ubuntu and (future)
trixie ship lcov >= 2 which required and extra argument.
---
src/_shard/Makefile | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/_shard/Makefile b/src/_shard/Makefile
index 28e2ed2..07219bd 100644
--- a/src/_shard/Makefile
+++ b/src/_shard/Makefile
@@ -1,6 +1,13 @@
CFLAGS=-D_FILE_OFFSET_BITS=64 -DHASH_DEBUG -Wall -I../.. -g -fprofile-arcs -ftest-coverage
CXXFLAGS=$(CFLAGS) -std=c++17
LDFLAGS=-lcmph -lgtest -lpthread -lstdc++ -lstdc++fs -fprofile-arcs -ftest-coverage
+LCOVFLAGS=
+LCOVV2:=$(shell expr `lcov -v | awk '{print $4}' | cut -f1 -d. ` \>= 2)
+
+# seems lcov>=2 requires this for the check to pass
+ifeq "$(LCOV2)" "1"
+ LCOVFLAGS += --ignore-errors mismatch
+endif
test_shard: shard.o test_shard.o
$(CXX) -o $@ $^ $(LDFLAGS)
@@ -14,7 +21,7 @@ format:
check: test_shard
valgrind --leak-check=full --tool=memcheck ./test_shard
- lcov -d . -c -o test_shard.lcov
+ lcov $(LCOVFLAGS) -d . -c -o test_shard.lcov
rm -fr html ; genhtml -o html test_shard.lcov
clean:
--
GitLab
From e0a3980b786179c9a3dcc19de51c824b8b229bc9 Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Fri, 21 Mar 2025 10:30:39 +0100
Subject: [PATCH 20/24] cli: Use context managers for reading files
---
src/swh/shard/cli.py | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/swh/shard/cli.py b/src/swh/shard/cli.py
index 2746dec..3110736 100644
--- a/src/swh/shard/cli.py
+++ b/src/swh/shard/cli.py
@@ -94,13 +94,13 @@ def shard_create(ctx, shard, files, sort_files):
with click.progressbar(files, label="Checking files to add") as bfiles:
for fname in bfiles:
try:
- data = open(fname, "rb").read()
+ with open(fname, "rb") as f:
+ sha256 = hashlib.sha256(f.read()).digest()
+ if sha256 not in hashes:
+ files_to_add[fname] = sha256
+ hashes.add(sha256)
except OSError:
continue
- sha256 = hashlib.sha256(data).digest()
- if sha256 not in hashes:
- files_to_add[fname] = sha256
- hashes.add(sha256)
click.echo(f"after deduplication: {len(files_to_add)} entries")
with ShardCreator(shard, len(files_to_add)) as shard:
@@ -109,8 +109,8 @@ def shard_create(ctx, shard, files, sort_files):
it = sorted(it, key=lambda x: x[0][-1::-1])
with click.progressbar(it, label="Adding files to the shard") as items:
for fname, sha256 in items:
- data = open(fname, "rb").read()
- shard.write(sha256, data)
+ with open(fname, "rb") as f:
+ shard.write(sha256, f.read())
click.echo("Done")
--
GitLab
From d3d4d4672abca3261a691fb8c0add36d752e5f51 Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Fri, 21 Mar 2025 10:33:49 +0100
Subject: [PATCH 21/24] cli: Add a --skip-removed option to the ls cli command
---
src/swh/shard/cli.py | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/src/swh/shard/cli.py b/src/swh/shard/cli.py
index 3110736..3b9acac 100644
--- a/src/swh/shard/cli.py
+++ b/src/swh/shard/cli.py
@@ -13,6 +13,9 @@ import click
logger = logging.getLogger(__name__)
+# marker of a deleted/non-populated index entry
+NULLKEY = b"\x00" * 32
+
CONTEXT_SETTINGS = dict(help_option_names=["-h", "--help"])
try:
@@ -115,16 +118,22 @@ def shard_create(ctx, shard, files, sort_files):
@shard_cli_group.command("ls")
+@click.option("--skip-removed", default=False, is_flag=True)
@click.argument("shard", required=True, type=click.Path(exists=True, dir_okay=False))
@click.pass_context
-def shard_list(ctx, shard):
+def shard_list(ctx, skip_removed, shard):
"List objects in a shard file"
from swh.shard import Shard
with Shard(shard) as s:
for key in s:
- size = s.getsize(key)
+ if skip_removed and key == NULLKEY:
+ continue
+ try:
+ size = s.getsize(key)
+ except KeyError:
+ size = "N/A"
click.echo(f"{key.hex()}: {size} bytes")
--
GitLab
From 9c907cf9b85887019cfc1af7688355731bef11dd Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Wed, 9 Apr 2025 10:56:49 +0200
Subject: [PATCH 22/24] build: Use cmake to download and build cmph directly
Also static link libcmph in the python so extension so there is no need
for shipping libcmph.so with it (makes 'auditwheel repair' life easier).
This should help (!) cibuildwheel to run smoothly...
---
CMakeLists.txt | 44 ++++++++++++++++++++++++++++----------------
build_cmph.sh | 25 -------------------------
pyproject.toml | 11 +----------
3 files changed, 29 insertions(+), 51 deletions(-)
delete mode 100755 build_cmph.sh
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 97d976d..7bcb131 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -10,32 +10,44 @@ set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(PYBIND11_FINDPYTHON ON)
find_package(Python REQUIRED COMPONENTS Interpreter Development.Module)
find_package(pybind11 CONFIG REQUIRED)
-pkg_search_module(CMPH cmph)
+set(CMPH_REQUIRED_VERSION 2.0.2)
+
+pkg_search_module(CMPH cmph=${CMPH_REQUIRED_VERSION})
if (NOT CMPH_MODULE_NAME)
- find_path(CMPH_INCLUDEDIR cmph.h
- HINTS
- cmph/include
- )
- find_library(CMPH_LIBRARY NAMES cmph libcmph
- HINTS
- cmph/lib
+ message(STATUS "cmph not found; it will be downloaded and compiled")
+ include(ExternalProject)
+ ExternalProject_Add(
+ cmph
+ URL https://downloads.sourceforge.net/project/cmph/v${CMPH_REQUIRED_VERSION}/cmph-${CMPH_REQUIRED_VERSION}.tar.gz
+ URL_HASH SHA1=143ddd4a9ba0b0dad8f0d0e573a4a3af463030c1
+ PREFIX ${CMAKE_CURRENT_BINARY_DIR}/cmph-${CMPH_REQUIRED_VERSION}
+
+ BUILD_IN_SOURCE ON
+ # don't ask, don't tell...
+ CONFIGURE_COMMAND autoreconf -i && ./configure --prefix=${CMAKE_CURRENT_BINARY_DIR}/cmph-${CMPH_REQUIRED_VERSION} --with-pic
+ BUILD_BYPRODUCTS ${CMAKE_CURRENT_BINARY_DIR}/cmph-${CMPH_REQUIRED_VERSION}/lib/libcmph.a
)
-endif()
+ ExternalProject_Get_Property(cmph install_dir)
+ set(CMPH_INCLUDEDIR "${install_dir}/include")
+ set(CMPH_STATIC_LIB "${install_dir}/lib/libcmph.a")
+ file(MAKE_DIRECTORY ${CMPH_INCLUDEDIR})
+
+ add_library(libcmph STATIC IMPORTED GLOBAL)
+ add_dependencies(libcmph cmph)
-if (NOT CMPH_INCLUDEDIR)
- message(FATAL_ERROR "cmph not found")
-else()
- message(STATUS "cmph include dir: ${CMPH_INCLUDEDIR}")
- message(STATUS "cmph lib: ${CMPH_LDFLAGS}")
+ set_target_properties(libcmph PROPERTIES IMPORTED_LOCATION ${CMPH_STATIC_LIB})
+ set_target_properties(libcmph PROPERTIES INTERFACE_INCLUDE_DIRECTORIES ${CMPH_INCLUDEDIR})
+ set(CMPH_LIBRARIES libcmph)
endif()
-include_directories(${CMPH_INCLUDEDIR})
+message(STATUS "cmph include dir: ${CMPH_INCLUDEDIR}")
+message(STATUS "cmph lib: ${CMPH_LIBRARIES}")
# Add a library using FindPython's tooling (pybind11 also provides a helper like
# this)
python_add_library(_shard MODULE src/_shard/bindings.cpp src/_shard/shard.c WITH_SOABI)
-target_link_libraries(_shard PRIVATE pybind11::headers ${CMPH_LDFLAGS})
+target_link_libraries(_shard PRIVATE pybind11::headers ${CMPH_LIBRARIES})
# This is passing in the version as a define just as an example
target_compile_definitions(_shard PRIVATE VERSION_INFO=${PROJECT_VERSION})
diff --git a/build_cmph.sh b/build_cmph.sh
deleted file mode 100755
index fec78a0..0000000
--- a/build_cmph.sh
+++ /dev/null
@@ -1,25 +0,0 @@
-#!/usr/bin/env bash
-# Copyright (C) 2021-2025 The Software Heritage developers
-# 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
-#
-# This script is used by cibuildwheel to install and compile the cmph library
-# when building manylinux wheels
-
-set -e
-
-CMPH_VERSION=2.0.2
-PREFIX="$(readlink -f $(dirname $0))/cmph"
-
-rm -rf "$PREFIX"
-mkdir "$PREFIX"
-cd "$PREFIX"
-wget https://deac-ams.dl.sourceforge.net/project/cmph/v${CMPH_VERSION}/cmph-${CMPH_VERSION}.tar.gz -O cmph.tar.gz
-tar xf cmph.tar.gz
-
-cd cmph-${CMPH_VERSION}
-
-./configure --prefix="$PREFIX"
-make -j8
-make install
diff --git a/pyproject.toml b/pyproject.toml
index 460c42a..74acf8d 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -33,6 +33,7 @@ build-backend = "scikit_build_core.build"
minimum-version = "build-system.requires"
wheel.exclude = ["*.c", "*.cpp", "*.h", ".clang-format", "*.o"]
strict-config = false
+# build.verbose = true
[tool.scikit-build.wheel.packages]
"swh" = "src/swh"
@@ -104,13 +105,3 @@ consider_namespace_packages = true
markers = [
"setrlimit(*limits): Set resource limits for the current test",
]
-
-[tool.cibuildwheel]
-before-all = "yum install -y wget && ./build_cmph.sh"
-
-[[tool.cibuildwheel.overrides]]
-select = "*-musllinux*"
-before-all = "apk add wget && ./build_cmph.sh"
-
-[tool.cibuildwheel.environment]
-LD_LIBRARY_PATH = "/project/cmph/lib"
--
GitLab
From 58e8c05796e953e06e2bcdab441837c311af0c57 Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Wed, 9 Apr 2025 11:11:49 +0200
Subject: [PATCH 23/24] tests: fix the (naive) test_memleak for pypy
With pypy, the max used RSS behaves quite differently from CPython, so
deal with it... This test is pretty useless anyway, a proper valgrind-based
check should be used instead...
---
src/swh/shard/tests/test_shard.py | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/swh/shard/tests/test_shard.py b/src/swh/shard/tests/test_shard.py
index 871720b..c0134c0 100644
--- a/src/swh/shard/tests/test_shard.py
+++ b/src/swh/shard/tests/test_shard.py
@@ -320,13 +320,17 @@ def shard_build(request, tmpdir, payload):
def test_memleak(request, tmpdir, payload):
"Naive test for memleak in ShardReader"
shard_build(request, tmpdir, payload)
- maxrss0 = resource.getrusage(resource.RUSAGE_SELF).ru_maxrss
shard_file = str(tmpdir / "shard")
+ maxrss = [resource.getrusage(resource.RUSAGE_SELF).ru_maxrss]
for i in range(100):
with Shard(shard_file) as s:
for key in s:
obj = s[key]
assert sha256(obj).digest() == key
-
- maxrss1 = resource.getrusage(resource.RUSAGE_SELF).ru_maxrss
- assert (maxrss1 - maxrss0) < 1024 # in kB
+ maxrss.append(resource.getrusage(resource.RUSAGE_SELF).ru_maxrss)
+ # on pypy, the used rss can still grow significantly during the first gew
+ # iterations, but should remain under a reasonable threshold
+ assert (maxrss[-1] - maxrss[1]) < 100 * 1024 # in kB
+ # but there should be none for the last iterations
+ for i in range(99, 90, -1):
+ assert (maxrss[i] - maxrss[i - 1]) == 0
--
GitLab
From 8fba3adff990f1a99bb62dd95b9574e2dfdaec09 Mon Sep 17 00:00:00 2001
From: David Douard <david.douard@sdfa3.org>
Date: Wed, 9 Apr 2025 13:44:25 +0200
Subject: [PATCH 24/24] tests: fix tests for pypy
Exception handling in pybind11 is a bit mangled on pypy, see
https://github.com/pybind/pybind11/issues/4075
Also the order of objects in a shard file may not be the same on all
execution backends, so make test_cli_ls aware of that.
---
src/swh/shard/tests/test_cli.py | 39 ++++++++++++++-----------------
src/swh/shard/tests/test_shard.py | 15 ++++++++----
2 files changed, 29 insertions(+), 25 deletions(-)
diff --git a/src/swh/shard/tests/test_cli.py b/src/swh/shard/tests/test_cli.py
index 2042634..845b727 100644
--- a/src/swh/shard/tests/test_cli.py
+++ b/src/swh/shard/tests/test_cli.py
@@ -51,27 +51,24 @@ def test_cli_ls(small_shard):
runner = CliRunner()
result = runner.invoke(cli.shard_list, [str(small_shard)])
assert result.exit_code == 0
- assert (
- result.output
- == """\
-000000000000000000000000000000000000000000000000000000000000000c: 42 bytes
-0000000000000000000000000000000000000000000000000000000000000005: 42 bytes
-000000000000000000000000000000000000000000000000000000000000000e: 42 bytes
-0000000000000000000000000000000000000000000000000000000000000001: 42 bytes
-0000000000000000000000000000000000000000000000000000000000000004: 42 bytes
-000000000000000000000000000000000000000000000000000000000000000d: 42 bytes
-0000000000000000000000000000000000000000000000000000000000000000: 42 bytes
-000000000000000000000000000000000000000000000000000000000000000a: 42 bytes
-0000000000000000000000000000000000000000000000000000000000000006: 42 bytes
-0000000000000000000000000000000000000000000000000000000000000009: 42 bytes
-0000000000000000000000000000000000000000000000000000000000000003: 42 bytes
-0000000000000000000000000000000000000000000000000000000000000008: 42 bytes
-000000000000000000000000000000000000000000000000000000000000000f: 42 bytes
-000000000000000000000000000000000000000000000000000000000000000b: 42 bytes
-0000000000000000000000000000000000000000000000000000000000000002: 42 bytes
-0000000000000000000000000000000000000000000000000000000000000007: 42 bytes
-"""
- )
+ assert set(result.output.strip().splitlines()) == {
+ "0000000000000000000000000000000000000000000000000000000000000000: 42 bytes",
+ "0000000000000000000000000000000000000000000000000000000000000001: 42 bytes",
+ "0000000000000000000000000000000000000000000000000000000000000002: 42 bytes",
+ "0000000000000000000000000000000000000000000000000000000000000003: 42 bytes",
+ "0000000000000000000000000000000000000000000000000000000000000004: 42 bytes",
+ "0000000000000000000000000000000000000000000000000000000000000005: 42 bytes",
+ "0000000000000000000000000000000000000000000000000000000000000006: 42 bytes",
+ "0000000000000000000000000000000000000000000000000000000000000007: 42 bytes",
+ "0000000000000000000000000000000000000000000000000000000000000008: 42 bytes",
+ "0000000000000000000000000000000000000000000000000000000000000009: 42 bytes",
+ "000000000000000000000000000000000000000000000000000000000000000a: 42 bytes",
+ "000000000000000000000000000000000000000000000000000000000000000b: 42 bytes",
+ "000000000000000000000000000000000000000000000000000000000000000c: 42 bytes",
+ "000000000000000000000000000000000000000000000000000000000000000d: 42 bytes",
+ "000000000000000000000000000000000000000000000000000000000000000e: 42 bytes",
+ "000000000000000000000000000000000000000000000000000000000000000f: 42 bytes",
+ }
def test_cli_get(small_shard):
diff --git a/src/swh/shard/tests/test_shard.py b/src/swh/shard/tests/test_shard.py
index c0134c0..5cfa057 100644
--- a/src/swh/shard/tests/test_shard.py
+++ b/src/swh/shard/tests/test_shard.py
@@ -7,6 +7,7 @@ from hashlib import sha256
import logging
import os
from pathlib import Path
+import platform
import random
import resource
import time
@@ -18,6 +19,9 @@ from swh.shard import Shard, ShardCreator
logger = logging.getLogger(__name__)
+PYPY = platform.python_implementation() == "PyPy"
+
+
@pytest.fixture(scope="function", autouse=True)
def setrlimit(request):
marker = request.node.get_closest_marker("setrlimit")
@@ -75,7 +79,8 @@ def test_creator_open_without_permission(tmpdir):
path.touch()
# Remove all permissions
path.chmod(0o000)
- with pytest.raises(PermissionError):
+ exc_cls = RuntimeError if PYPY else PermissionError
+ with pytest.raises(exc_cls):
with ShardCreator(str(path), 1):
pass
@@ -135,7 +140,8 @@ def test_creator_errors_with_duplicate_key(tmpdir):
def test_load_non_existing():
- with pytest.raises(FileNotFoundError):
+ exc_cls = RuntimeError if PYPY else FileNotFoundError
+ with pytest.raises(exc_cls):
_ = Shard("/nonexistent")
@@ -239,7 +245,8 @@ def test_build_speed(request, tmpdir, payload):
# regress in the future... (we use x10 to give a bit of slack otherwise the
# test is pretty unstable)
#
- assert duration < baseline * 10
+ k = 100 if PYPY else 10
+ assert duration < baseline * k
def test_lookup_speed(request, tmpdir, payload):
@@ -333,4 +340,4 @@ def test_memleak(request, tmpdir, payload):
assert (maxrss[-1] - maxrss[1]) < 100 * 1024 # in kB
# but there should be none for the last iterations
for i in range(99, 90, -1):
- assert (maxrss[i] - maxrss[i - 1]) == 0
+ assert (maxrss[i] - maxrss[i - 1]) == 0, maxrss[i] - maxrss[i - 1]
--
GitLab