Re: [PATCH v2 2/6] object-file.c: add dry_run mode for write_loose_object()

[Jiang Xin <worldhello.net@gmail.com>]

On Fri, Nov 12, 2021 at 5:42 PM Han Xin <chiyutianyi@gmail.com> wrote:
>
> From: Han Xin <hanxin.hx@alibaba-inc.com>
>
> We will use "write_loose_object()" later to handle large blob object,
> which needs to work in dry_run mode.

The dry_run mode comes from "builtin/unpack-object.c", throw the
buffer read from "get_data()".
So why not add "dry_run" to "get_data()" instead?

If we have a dry_run version of get_data, such as "get_data(size,
dry_run)", we do not have to add dry_run mode for ”
write_loose_object()".

See: git grep -A5 get_data builtin/unpack-objects.c
builtin/unpack-objects.c:       void *buf = get_data(size);
builtin/unpack-objects.c-
builtin/unpack-objects.c-       if (!dry_run && buf)
builtin/unpack-objects.c-               write_object(nr, type, buf, size);
builtin/unpack-objects.c-       else
builtin/unpack-objects.c-               free(buf);
--
builtin/unpack-objects.c:               delta_data = get_data(delta_size);
builtin/unpack-objects.c-               if (dry_run || !delta_data) {
builtin/unpack-objects.c-                       free(delta_data);
builtin/unpack-objects.c-                       return;
builtin/unpack-objects.c-               }
--
builtin/unpack-objects.c:               delta_data = get_data(delta_size);
builtin/unpack-objects.c-               if (dry_run || !delta_data) {
builtin/unpack-objects.c-                       free(delta_data);
builtin/unpack-objects.c-                       return;
builtin/unpack-objects.c-               }


> Helped-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
> Signed-off-by: Han Xin <hanxin.hx@alibaba-inc.com>
> ---
>  object-file.c | 32 +++++++++++++++++++-------------
>  1 file changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/object-file.c b/object-file.c
> index 1ad2cb579c..b0838c847e 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -1880,9 +1880,10 @@ static const char *read_input_stream_from_buffer(void *data, unsigned long *len)
>
>  static int write_loose_object(const struct object_id *oid, char *hdr,
>                               int hdrlen, struct input_stream *in_stream,
> +                             int dry_run,
>                               time_t mtime, unsigned flags)
>  {
> -       int fd, ret;
> +       int fd, ret = 0;
>         unsigned char compressed[4096];
>         git_zstream stream;
>         git_hash_ctx c;
> @@ -1894,14 +1895,16 @@ static int write_loose_object(const struct object_id *oid, char *hdr,
>
>         loose_object_path(the_repository, &filename, oid);
>
> -       fd = create_tmpfile(&tmp_file, filename.buf);
> -       if (fd < 0) {
> -               if (flags & HASH_SILENT)
> -                       return -1;
> -               else if (errno == EACCES)
> -                       return error(_("insufficient permission for adding an object to repository database %s"), get_object_directory());
> -               else
> -                       return error_errno(_("unable to create temporary file"));
> +       if (!dry_run) {
> +               fd = create_tmpfile(&tmp_file, filename.buf);
> +               if (fd < 0) {
> +                       if (flags & HASH_SILENT)
> +                               return -1;
> +                       else if (errno == EACCES)
> +                               return error(_("insufficient permission for adding an object to repository database %s"), get_object_directory());
> +                       else
> +                               return error_errno(_("unable to create temporary file"));
> +               }
>         }
>
>         /* Set it up */
> @@ -1925,7 +1928,7 @@ static int write_loose_object(const struct object_id *oid, char *hdr,
>                 unsigned char *in0 = stream.next_in;
>                 ret = git_deflate(&stream, Z_FINISH);
>                 the_hash_algo->update_fn(&c, in0, stream.next_in - in0);
> -               if (write_buffer(fd, compressed, stream.next_out - compressed) < 0)
> +               if (!dry_run && write_buffer(fd, compressed, stream.next_out - compressed) < 0)
>                         die(_("unable to write loose object file"));
>                 stream.next_out = compressed;
>                 stream.avail_out = sizeof(compressed);
> @@ -1943,6 +1946,9 @@ static int write_loose_object(const struct object_id *oid, char *hdr,
>                 die(_("confused by unstable object source data for %s"),
>                     oid_to_hex(oid));
>
> +       if (dry_run)
> +               return 0;
> +
>         close_loose_object(fd);
>
>         if (mtime) {
> @@ -1996,7 +2002,7 @@ int write_object_file_flags(const void *buf, unsigned long len,
>                                   &hdrlen);
>         if (freshen_packed_object(oid) || freshen_loose_object(oid))
>                 return 0;
> -       return write_loose_object(oid, hdr, hdrlen, &in_stream, 0, flags);
> +       return write_loose_object(oid, hdr, hdrlen, &in_stream, 0, 0, flags);
>  }
>
>  int hash_object_file_literally(const void *buf, unsigned long len,
> @@ -2023,7 +2029,7 @@ int hash_object_file_literally(const void *buf, unsigned long len,
>                 goto cleanup;
>         if (freshen_packed_object(oid) || freshen_loose_object(oid))
>                 goto cleanup;
> -       status = write_loose_object(oid, header, hdrlen, &in_stream, 0, 0);
> +       status = write_loose_object(oid, header, hdrlen, &in_stream, 0, 0, 0);
>
>  cleanup:
>         free(header);
> @@ -2052,7 +2058,7 @@ int force_object_loose(const struct object_id *oid, time_t mtime)
>         data.buf = buf;
>         data.len = len;
>         hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %"PRIuMAX , type_name(type), (uintmax_t)len) + 1;
> -       ret = write_loose_object(oid, hdr, hdrlen, &in_stream, mtime, 0);
> +       ret = write_loose_object(oid, hdr, hdrlen, &in_stream, 0, mtime, 0);
>         free(buf);
>
>         return ret;
> --
> 2.33.1.44.g9344627884.agit.6.5.4
>