On 21/01/25 15:31, Mark Jamsek wrote:
> Stefan Sperling <stsp@stsp.name> wrote:
>> On Tue, Jan 21, 2025 at 11:02:43AM +0100, Stefan Sperling wrote:
>>> Our cleanup implementation is only safe to use after everything
>>> referenced has been packed into a single pack file. Otherwise, the
>>> algorithm we use for checking pack redundancy might remove small
>>> packs which contain objects that other objects depend on.
>>>
>>> The easy fix for this issue is to have 'gotadmin cleanup' create the
>>> required pack file before cleaning up, making cleanup safe by default.
>>> This happens to be what 'git gc' does as well.
> This will be good so I can remove gotadmin pack from my mirrors'
> gotsync script
>
>> Two fixes on top:
>>
>> 1) pack progress output should go to stdout, not stderr.
>>
>> 2) cleanup -n output was inaccurate since pack generation was skipped.
>>   Instead, create a pack file, figure out the dry-run numbers to show,
>>   and then delete the generated pack again. Not ideal, but this is the
>>   easiest way to keep cleanup -n working.
>>
>> ok?
> The diff reads good and I think the simple solution is the way to go;
> regress is also happy.
>
> ok for me with a couple small nits inline; I've included a diff below
> that you can apply on top and commit if in agreement


Sorry for the delay, and thanks for improving on this.


ok op@ for both diffs; good catches Mark!


Thanks!