On Wed, Feb 2, 2022 at 5:28 AM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote: > > > On Thu, Jan 20 2022, Han Xin wrote: > > > From: Han Xin <hanxin.hx@xxxxxxxxxxxxxxx> > > > > Changes since v8: > > * Rename "assert_no_loose ()" into "test_no_loose ()" in > > "t5329-unpack-large-objects.sh". Remove "assert_no_pack ()" and use > > "test_dir_is_empty" instead. > > > > * Revert changes to "create_tmpfile()" and error handling is now in > > "start_loose_object_common()". > > > > * Remove "finalize_object_file_with_mtime()" which seems to be an overkill > > for "write_loose_object()" now. > > > > * Remove the commit "object-file.c: remove the slash for directory_size()", > > it can be in a separate patch if necessary. > > > > Han Xin (4): > > unpack-objects: low memory footprint for get_data() in dry_run mode > > object-file.c: refactor write_loose_object() to several steps > > object-file.c: add "stream_loose_object()" to handle large object > > unpack-objects: unpack_non_delta_entry() read data in a stream > > > > Ævar Arnfjörð Bjarmason (1): > > object-file API: add a format_object_header() function > > I sent > https://lore.kernel.org/git/cover-00.10-00000000000-20220201T144803Z-avarab@xxxxxxxxx/ > today which suggests splitting out the 5/5 cleanup you'd integrated. > > I then rebased these patches of yours on top of that, the result is > here: > https://github.com/avar/git/tree/han-xin-avar/unpack-loose-object-streaming-9 > > The range-diff to your version is below. There's a few unrelated > fixes/nits in it. > > I think with/without basing this on top of my series above your patches > here look good with the nits pointed out in the diff below addressed > (and some don't need to be). I.e. the dependency on it is rather > trivial, and the two could be split up. > > What do you think is a good way to proceed? I could just submit the > below as a proposed v10 if you'd like & agree... > Yes, thanks for the suggestions, and I'm glad you're happy to do so. Thanks. -Han Xin > 1: 553a9377eb3 ! 1: 61fcfe7b840 unpack-objects: low memory footprint for get_data() in dry_run mode > @@ Commit message > unpack-objects: low memory footprint for get_data() in dry_run mode > > As the name implies, "get_data(size)" will allocate and return a given > - size of memory. Allocating memory for a large blob object may cause the > + amount of memory. Allocating memory for a large blob object may cause the > system to run out of memory. Before preparing to replace calling of > "get_data()" to unpack large blob objects in latter commits, refactor > "get_data()" to reduce memory footprint for dry_run mode. > @@ Commit message > > Suggested-by: Jiang Xin <zhiyou.jx@xxxxxxxxxxxxxxx> > Signed-off-by: Han Xin <hanxin.hx@xxxxxxxxxxxxxxx> > + Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> > > ## builtin/unpack-objects.c ## > @@ builtin/unpack-objects.c: static void use(int bytes) > @@ t/t5328-unpack-large-objects.sh (new) > + > +test_no_loose () { > + glob=dest.git/objects/?? && > -+ echo "$glob" >expect && > -+ eval "echo $glob" >actual && > ++ echo $glob >expect && > ++ echo "$glob" >actual && > + test_cmp expect actual > +} > + I have a small doubt with this, it works fine with dash, but not others like zsh. Wouldn't it be better to do compatibility, or would it introduce other issues that I don't know? Thanks. -Han Xin
