| Message ID | 20190615100702.20762-1-chriscool@tuxfamily.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Test oidmap | expand |
On Sat, Jun 15, 2019 at 12:06:58PM +0200, Christian Couder wrote: > Unlike hashmap that has t/helper/test-hashmap.c and t/t0011-hashmap.sh > oidmap has no specific test. The goal of this small patch series is to > change that and also improve oidmap a bit while at it. > > Changes compared to V3 are the following: > > - removed "hash" command in test-oidmap.c and "hash" test in > t0016-oidmap.sh as suggested by Peff, > > - added patch 4/4 which does the same as above in test-hashmap.c and > t0011-hashmap.sh as suggested by Peff. This version looks good to me. I do think that sha1hash() will eventually go away in favor of oidhash(), but we can approach that separately, and convert oidmap along with everyone else. It looks like we are close to being able to do that now. Grepping for sha1hash shows just about everybody dereferencing an oid object, except for the call in pack-objects.c. And skimming the callers there, they all appear to have an oid object, too. -Peff
On Wed, Jun 19, 2019 at 05:42:13PM -0400, Jeff King wrote: > I do think that sha1hash() will eventually go away in favor of > oidhash(), but we can approach that separately, and convert oidmap along > with everyone else. > > It looks like we are close to being able to do that now. Grepping for > sha1hash shows just about everybody dereferencing an oid object, except > for the call in pack-objects.c. And skimming the callers there, > they all appear to have an oid object, too. Actually, it does get a little tangled, as our khash implementation also uses sha1hash (though IMHO that should become oidhash, too). There's also some preparatory work needed in pack-objects, which I've pushed up to: https://github.com/peff/git jk/drop-sha1hash I got interrupted and may try to resume this cleanup later; mostly I wanted to post something now so you did not duplicate what I'd already done. -Peff
On Thu, Jun 20, 2019 at 12:09 AM Jeff King <peff@peff.net> wrote: > > On Wed, Jun 19, 2019 at 05:42:13PM -0400, Jeff King wrote: > > > I do think that sha1hash() will eventually go away in favor of > > oidhash(), but we can approach that separately, and convert oidmap along > > with everyone else. Yeah, deprecating it is a different topic. > > It looks like we are close to being able to do that now. Grepping for > > sha1hash shows just about everybody dereferencing an oid object, except > > for the call in pack-objects.c. And skimming the callers there, > > they all appear to have an oid object, too. > > Actually, it does get a little tangled, as our khash implementation also > uses sha1hash (though IMHO that should become oidhash, too). There's > also some preparatory work needed in pack-objects, which I've pushed up > to: > > https://github.com/peff/git jk/drop-sha1hash > > I got interrupted and may try to resume this cleanup later; mostly I > wanted to post something now so you did not duplicate what I'd already > done. Thanks for your work on that, Christian.
Unlike hashmap that has t/helper/test-hashmap.c and t/t0011-hashmap.sh oidmap has no specific test. The goal of this small patch series is to change that and also improve oidmap a bit while at it. Changes compared to V3 are the following: - removed "hash" command in test-oidmap.c and "hash" test in t0016-oidmap.sh as suggested by Peff, - added patch 4/4 which does the same as above in test-hashmap.c and t0011-hashmap.sh as suggested by Peff. Previous versions on the mailing list: V3: https://public-inbox.org/git/20190612232425.12149-1-chriscool@tuxfamily.org/ V2: https://public-inbox.org/git/20190611082325.28878-1-chriscool@tuxfamily.org/ V1: https://public-inbox.org/git/20190609044907.32477-1-chriscool@tuxfamily.org/ This patch series on GitHub: https://github.com/chriscool/git/commits/oidmap Christian Couder (4): t/helper: add test-oidmap.c t: add t0016-oidmap.sh oidmap: use sha1hash() instead of static hash() function test-hashmap: remove 'hash' command Makefile | 1 + oidmap.c | 13 +---- t/helper/test-hashmap.c | 9 +-- t/helper/test-oidmap.c | 126 ++++++++++++++++++++++++++++++++++++++++ t/helper/test-tool.c | 1 + t/helper/test-tool.h | 1 + t/t0011-hashmap.sh | 9 --- t/t0016-oidmap.sh | 84 +++++++++++++++++++++++++++ 8 files changed, 217 insertions(+), 27 deletions(-) create mode 100644 t/helper/test-oidmap.c create mode 100755 t/t0016-oidmap.sh