Đoàn Trần Công Danh <congdanhqx@xxxxxxxxx> writes: > On 2020-05-09 14:24:29+0000, Derrick Stolee via GitGitGadget <gitgitgadget@xxxxxxxxx> wrote: >> From: Derrick Stolee <dstolee@xxxxxxxxxxxxx> >> >> +test_expect_success 'repack respects repack.packKeptObjects=false' ' >> + test_when_finished rm -f dup/.git/objects/pack/*keep && >> + ( >> + cd dup && >> + ls .git/objects/pack/*idx >idx-list && > > I think ls(1) is an overkill. > I think: > > echo .git/objects/pack/*idx > > is more efficient. When there is no file whose name ends with idx, what happens? $ ls *idx && echo OK ls: cannot access '*idx': No such file or directory $ echo *idx && echo OK *idx OK >> + test_line_count = 5 idx-list && >> + ls .git/objects/pack/*.pack | sed "s/\.pack/.keep/" >keep-list && > > Likewise. Likewise. >> + for keep in $(cat keep-list) >> + do >> + touch $keep || return 1 > > Is this intended? > Since touch(1) accepts multiple files as argument. Good suggestion, but doesn't .keep file record why the pack is kept in real life (i.e. not an empty file)? >> + done && >> + git multi-pack-index repack --batch-size=0 && >> + ls .git/objects/pack/*idx >idx-list && >> + test_line_count = 5 idx-list && >> + test-tool read-midx .git/objects | grep idx >midx-list && >> + test_line_count = 5 midx-list && >> + THIRD_SMALLEST_SIZE=$(test-tool path-utils file-size .git/objects/pack/*pack | sort -n | head -n 3 | tail -n 1) && > > This line is overly long. > Should we write test-tool's output to temp file and process it? > > And I think either > > sed -n '3{p;q}' > > or: > > sed -n 3p > > is cleaner than > > head -n 3 | tail -n 1 "sed -n 3p" is the only valid way to write it ;-)