Re: [PATCH v2 2/6] object-file.c: add dry_run mode for write_loose_object()
- From
Jiang Xin <worldhello.net@gmail.com>
- Date
- Nov 18, 2021, 05:42 UTC
- Message-ID
- <CANYiYbHvpVH7CvhGOUZojVkXd7eztC+wQ_R8=Ta9hPwjjAvWoQ@mail.gmail.com>
- In-Reply-To
- <20211112094010.73468-2-chiyutianyi@gmail.com>
On Fri, Nov 12, 2021 at 5:42 PM Han Xin <chiyutianyi@gmail.com> wrote:
Show 5 quoted lines
> > 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- }Show 96 quoted lines
> 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
>