Christian Couder <christian.couder@xxxxxxxxx> writes: > This could be useful if, for example, some large blobs take a lot of > precious space on fast storage while they are rarely accessed. It could > make sense to move them into a separate cheaper, though slower, storage. > > In other use cases it might make sense to put all the blobs into > separate storage. Minor nit. Aren't the above two the same use case? > This is done by running two `git pack-objects` commands. The first one > is run with `--filter=<filter-spec>`, using the specified filter. It > packs objects while omitting the objects specified by the filter. > Then another `git pack-objects` command is launched using > `--stdin-packs`. We pass it all the previously existing packs into its > stdin, so that it will pack all the objects in the previously existing > packs. But we also pass into its stdin, the pack created by the previous > `git pack-objects --filter=<filter-spec>` command as well as the kept > packs, all prefixed with '^', so that the objects in these packs will be > omitted from the resulting pack. When I started reading the paragraph, the first question that came to my mind was if these two pack-objects processes can and should be run in parallel, which is answered in the part near the end of the paragraph. It may be a good idea to start the paragraph with "by running `git pack-objects` command twice in a row" or something to make it clear that one should (and cannot be) run before the other completes. In fact, isn't the call site of write_filtered_pack() in this patch a bit too early? The subprocess that runs with "--stdin-packs" is started and told about the names of the pack we are going to create, and it does not start processing until it reads everything (i.e. we run fclose(in) in the write_filtered_pack() function), but the loop over "names" string list in the caller that moves the tempfiles to their final filenames comes after the call to close_object_store() we see in the post context of the call to write_filtered_pack() that is new in this patch. The "--stdin-packs" one is told to exclude objects that appear in these packs, so if the main process is a bit slow to finalize the packfiles it created (and told the "--stdin-packs" process about), it will not lead to repository corruption---just some objects are included in the packfiles "--stdin-packs" one creates even though they do not have to. So it does not sound like a huge problem to me, but still it somehow looks wrong. Am I misreading the code? Thanks.
