Taylor Blau <me@xxxxxxxxxxxx> writes: > I think the reason that this succeeds is that we already have a bitmap, > and it likely reuses all of the existing bitmaps before discovering that > the pack we wrote doesn't contain all objects. Now I am confused. We were asked to write bitmap index when we are going to create an incomplete pack, and the packfile we generate with the filter will not have full set of objects, and generating a bitmap with such an incomplete knowledge of what objects are reachable from what would be a disaster, so we should turn it off. But the posted patch lacked such a "we should abort when bitmap is asked to be written while filtering" logic. Then what were we expecting for the test to fail for? > but I wonder if a more complete fix would be something like: > ... > @@ -966,6 +972,9 @@ int cmd_repack(int argc, const char **argv, const char *prefix) > if (write_bitmaps && !(pack_everything & ALL_INTO_ONE) && !write_midx) > die(_(incremental_bitmap_conflict_error)); > > + if (write_bitmaps && po_args.filter_options.choice) > + die(_(filtered_bitmap_conflict_error)); > + It sounds like the most direct fix.
