On 2020-05-09 10:33:30-0700, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Đ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 Yes, but I think the next line is checking for the number of lines. This is better to fail faster. (My suggestion was wrong anyway, it should be "printf "%s\\n" *idx) > >> + test_line_count = 5 idx-list && > >> + 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)? Yes, in real life, we usually provide a reason in this .keep file. But, we also allow empty file with git-index-pack --keep I think simple touch is fine for this test. Missing piece for my previous command: if `keep-list` is empty, we may want to fail fast, touch with empty list will error out (at least in my system). -- Danh