On Tue, Dec 07, 2021 at 02:46:50AM +0000, Neeraj Singh via GitGitGadget wrote:
> From: Neeraj Singh <neerajsi@xxxxxxxxxxxxx>
[snip]
> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> index c23d01de7dc..c32534c13b4 100644
> --- a/builtin/index-pack.c
> +++ b/builtin/index-pack.c
> @@ -1286,7 +1286,7 @@ static void conclude_pack(int fix_thin_pack, const char *curr_pack, unsigned cha
> nr_objects - nr_objects_initial);
> stop_progress_msg(&progress, msg.buf);
> strbuf_release(&msg);
> - finalize_hashfile(f, tail_hash, 0);
> + finalize_hashfile(f, tail_hash, FSYNC_COMPONENT_PACK, 0);
> hashcpy(read_hash, pack_hash);
> fixup_pack_header_footer(output_fd, pack_hash,
> curr_pack, nr_objects,
> @@ -1508,7 +1508,7 @@ static void final(const char *final_pack_name, const char *curr_pack_name,
> if (!from_stdin) {
> close(input_fd);
> } else {
> - fsync_or_die(output_fd, curr_pack_name);
> + fsync_component_or_die(FSYNC_COMPONENT_PACK, output_fd, curr_pack_name);
> err = close(output_fd);
> if (err)
> die_errno(_("error while closing pack file"));
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index 857be7826f3..916c55d6ce9 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -1204,11 +1204,13 @@ static void write_pack_file(void)
> * If so, rewrite it like in fast-import
> */
> if (pack_to_stdout) {
> - finalize_hashfile(f, hash, CSUM_HASH_IN_STREAM | CSUM_CLOSE);
> + finalize_hashfile(f, hash, FSYNC_COMPONENT_NONE,
> + CSUM_HASH_IN_STREAM | CSUM_CLOSE);
It doesn't have any effect here given that we don't sync at all when
writing to stdout, but I wonder whether we should set up the component
correctly regardless of that such that it makes for a less confusing
read.
[snip]
> diff --git a/config.c b/config.c
> index c3410b8a868..29c867aab03 100644
> --- a/config.c
> +++ b/config.c
> @@ -1213,6 +1213,73 @@ static int git_parse_maybe_bool_text(const char *value)
> return -1;
> }
>
> +static const struct fsync_component_entry {
> + const char *name;
> + enum fsync_component component_bits;
> +} fsync_component_table[] = {
> + { "loose-object", FSYNC_COMPONENT_LOOSE_OBJECT },
> + { "pack", FSYNC_COMPONENT_PACK },
> + { "pack-metadata", FSYNC_COMPONENT_PACK_METADATA },
> + { "commit-graph", FSYNC_COMPONENT_COMMIT_GRAPH },
> + { "objects", FSYNC_COMPONENTS_OBJECTS },
> + { "default", FSYNC_COMPONENTS_DEFAULT },
> + { "all", FSYNC_COMPONENTS_ALL },
> +};
> +
> +static enum fsync_component parse_fsync_components(const char *var, const char *string)
> +{
> + enum fsync_component output = 0;
> +
> + if (!strcmp(string, "none"))
> + return output;
> +
> + while (string) {
> + int i;
> + size_t len;
> + const char *ep;
> + int negated = 0;
> + int found = 0;
> +
> + string = string + strspn(string, ", \t\n\r");
> + ep = strchrnul(string, ',');
> + len = ep - string;
> +
> + if (*string == '-') {
> + negated = 1;
> + string++;
> + len--;
> + if (!len)
> + warning(_("invalid value for variable %s"), var);
> + }
> +
> + if (!len)
> + break;
> +
> + for (i = 0; i < ARRAY_SIZE(fsync_component_table); ++i) {
> + const struct fsync_component_entry *entry = &fsync_component_table[i];
> +
> + if (strncmp(entry->name, string, len))
> + continue;
> +
> + found = 1;
> + if (negated)
> + output &= ~entry->component_bits;
> + else
> + output |= entry->component_bits;
> + }
> +
> + if (!found) {
> + char *component = xstrndup(string, len);
> + warning(_("unknown %s value '%s'"), var, component);
> + free(component);
> + }
> +
> + string = ep;
> + }
> +
> + return output;
> +}
> +
> int git_parse_maybe_bool(const char *value)
> {
> int v = git_parse_maybe_bool_text(value);
> @@ -1490,6 +1557,13 @@ static int git_default_core_config(const char *var, const char *value, void *cb)
> return 0;
> }
>
> + if (!strcmp(var, "core.fsync")) {
> + if (!value)
> + return config_error_nonbool(var);
> + fsync_components = parse_fsync_components(var, value);
> + return 0;
> + }
> +
> if (!strcmp(var, "core.fsyncmethod")) {
> if (!value)
> return config_error_nonbool(var);
> @@ -1503,7 +1577,7 @@ static int git_default_core_config(const char *var, const char *value, void *cb)
> }
>
> if (!strcmp(var, "core.fsyncobjectfiles")) {
> - fsync_object_files = git_config_bool(var, value);
> + warning(_("core.fsyncobjectfiles is deprecated; use core.fsync instead"));
> return 0;
> }
Shouldn't we continue to support this for now such that users can
migrate from the old, deprecated value first before we start to ignore
it?
Patrick
> diff --git a/csum-file.c b/csum-file.c
> index 26e8a6df44e..59ef3398ca2 100644
> --- a/csum-file.c
> +++ b/csum-file.c
> @@ -58,7 +58,8 @@ static void free_hashfile(struct hashfile *f)
> free(f);
> }
>
> -int finalize_hashfile(struct hashfile *f, unsigned char *result, unsigned int flags)
> +int finalize_hashfile(struct hashfile *f, unsigned char *result,
> + enum fsync_component component, unsigned int flags)
> {
> int fd;
>
> @@ -69,7 +70,7 @@ int finalize_hashfile(struct hashfile *f, unsigned char *result, unsigned int fl
> if (flags & CSUM_HASH_IN_STREAM)
> flush(f, f->buffer, the_hash_algo->rawsz);
> if (flags & CSUM_FSYNC)
> - fsync_or_die(f->fd, f->name);
> + fsync_component_or_die(component, f->fd, f->name);
> if (flags & CSUM_CLOSE) {
> if (close(f->fd))
> die_errno("%s: sha1 file error on close", f->name);
> diff --git a/csum-file.h b/csum-file.h
> index 291215b34eb..0d29f528fbc 100644
> --- a/csum-file.h
> +++ b/csum-file.h
> @@ -1,6 +1,7 @@
> #ifndef CSUM_FILE_H
> #define CSUM_FILE_H
>
> +#include "cache.h"
> #include "hash.h"
>
> struct progress;
> @@ -38,7 +39,7 @@ int hashfile_truncate(struct hashfile *, struct hashfile_checkpoint *);
> struct hashfile *hashfd(int fd, const char *name);
> struct hashfile *hashfd_check(const char *name);
> struct hashfile *hashfd_throughput(int fd, const char *name, struct progress *tp);
> -int finalize_hashfile(struct hashfile *, unsigned char *, unsigned int);
> +int finalize_hashfile(struct hashfile *, unsigned char *, enum fsync_component, unsigned int);
> void hashwrite(struct hashfile *, const void *, unsigned int);
> void hashflush(struct hashfile *f);
> void crc32_begin(struct hashfile *);
> diff --git a/environment.c b/environment.c
> index f9140e842cf..09905adecf9 100644
> --- a/environment.c
> +++ b/environment.c
> @@ -42,6 +42,7 @@ const char *git_hooks_path;
> int zlib_compression_level = Z_BEST_SPEED;
> int pack_compression_level = Z_DEFAULT_COMPRESSION;
> enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT;
> +enum fsync_component fsync_components = FSYNC_COMPONENTS_DEFAULT;
> size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE;
> size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT;
> size_t delta_base_cache_limit = 96 * 1024 * 1024;
> diff --git a/midx.c b/midx.c
> index 837b46b2af5..882f91f7d57 100644
> --- a/midx.c
> +++ b/midx.c
> @@ -1406,7 +1406,8 @@ static int write_midx_internal(const char *object_dir,
> write_midx_header(f, get_num_chunks(cf), ctx.nr - dropped_packs);
> write_chunkfile(cf, &ctx);
>
> - finalize_hashfile(f, midx_hash, CSUM_FSYNC | CSUM_HASH_IN_STREAM);
> + finalize_hashfile(f, midx_hash, FSYNC_COMPONENT_PACK_METADATA,
> + CSUM_FSYNC | CSUM_HASH_IN_STREAM);
> free_chunkfile(cf);
>
> if (flags & (MIDX_WRITE_REV_INDEX | MIDX_WRITE_BITMAP))
> diff --git a/object-file.c b/object-file.c
> index eb972cdccd2..9d9c4a39e85 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -1809,8 +1809,7 @@ int hash_object_file(const struct git_hash_algo *algo, const void *buf,
> /* Finalize a file on disk, and close it. */
> static void close_loose_object(int fd)
> {
> - if (fsync_object_files)
> - fsync_or_die(fd, "loose object file");
> + fsync_component_or_die(FSYNC_COMPONENT_LOOSE_OBJECT, fd, "loose object file");
> if (close(fd) != 0)
> die_errno(_("error when closing loose object file"));
> }
> diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
> index 9c55c1531e1..c16e43d1669 100644
> --- a/pack-bitmap-write.c
> +++ b/pack-bitmap-write.c
> @@ -719,7 +719,8 @@ void bitmap_writer_finish(struct pack_idx_entry **index,
> if (options & BITMAP_OPT_HASH_CACHE)
> write_hash_cache(f, index, index_nr);
>
> - finalize_hashfile(f, NULL, CSUM_HASH_IN_STREAM | CSUM_FSYNC | CSUM_CLOSE);
> + finalize_hashfile(f, NULL, FSYNC_COMPONENT_PACK_METADATA,
> + CSUM_HASH_IN_STREAM | CSUM_FSYNC | CSUM_CLOSE);
>
> if (adjust_shared_perm(tmp_file.buf))
> die_errno("unable to make temporary bitmap file readable");
> diff --git a/pack-write.c b/pack-write.c
> index a5846f3a346..51812cb1299 100644
> --- a/pack-write.c
> +++ b/pack-write.c
> @@ -159,9 +159,9 @@ const char *write_idx_file(const char *index_name, struct pack_idx_entry **objec
> }
>
> hashwrite(f, sha1, the_hash_algo->rawsz);
> - finalize_hashfile(f, NULL, CSUM_HASH_IN_STREAM | CSUM_CLOSE |
> - ((opts->flags & WRITE_IDX_VERIFY)
> - ? 0 : CSUM_FSYNC));
> + finalize_hashfile(f, NULL, FSYNC_COMPONENT_PACK_METADATA,
> + CSUM_HASH_IN_STREAM | CSUM_CLOSE |
> + ((opts->flags & WRITE_IDX_VERIFY) ? 0 : CSUM_FSYNC));
> return index_name;
> }
>
> @@ -281,8 +281,9 @@ const char *write_rev_file_order(const char *rev_name,
> if (rev_name && adjust_shared_perm(rev_name) < 0)
> die(_("failed to make %s readable"), rev_name);
>
> - finalize_hashfile(f, NULL, CSUM_HASH_IN_STREAM | CSUM_CLOSE |
> - ((flags & WRITE_IDX_VERIFY) ? 0 : CSUM_FSYNC));
> + finalize_hashfile(f, NULL, FSYNC_COMPONENT_PACK_METADATA,
> + CSUM_HASH_IN_STREAM | CSUM_CLOSE |
> + ((flags & WRITE_IDX_VERIFY) ? 0 : CSUM_FSYNC));
>
> return rev_name;
> }
> @@ -390,7 +391,7 @@ void fixup_pack_header_footer(int pack_fd,
> the_hash_algo->final_fn(partial_pack_hash, &old_hash_ctx);
> the_hash_algo->final_fn(new_pack_hash, &new_hash_ctx);
> write_or_die(pack_fd, new_pack_hash, the_hash_algo->rawsz);
> - fsync_or_die(pack_fd, pack_name);
> + fsync_component_or_die(FSYNC_COMPONENT_PACK, pack_fd, pack_name);
> }
>
> char *index_pack_lockfile(int ip_out, int *is_well_formed)
> diff --git a/read-cache.c b/read-cache.c
> index f3986596623..f3539681f49 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -3060,7 +3060,7 @@ static int do_write_index(struct index_state *istate, struct tempfile *tempfile,
> return -1;
> }
>
> - finalize_hashfile(f, istate->oid.hash, CSUM_HASH_IN_STREAM);
> + finalize_hashfile(f, istate->oid.hash, FSYNC_COMPONENT_NONE, CSUM_HASH_IN_STREAM);
> if (close_tempfile_gently(tempfile)) {
> error(_("could not close '%s'"), get_tempfile_path(tempfile));
> return -1;
> --
> gitgitgadget
>
Attachment:
signature.asc
Description: PGP signature
