diff --git a/Documentation/RelNotes/2.52.0.adoc b/Documentation/RelNotes/2.52.0.adoc index 6f8579bdeed781..959c8afe15f8a8 100644 --- a/Documentation/RelNotes/2.52.0.adoc +++ b/Documentation/RelNotes/2.52.0.adoc @@ -20,6 +20,10 @@ UI, Workflows & Features * "git refs exists" that works like "git show-ref --exists" has been added. + * "repo info" learns a short-hand option "-z" that is the same as + "--format=nul", and learns to report the objects format used in the + repository. + Performance, Internal Implementation, Development Support etc. -------------------------------------------------------------- @@ -55,6 +59,10 @@ Performance, Internal Implementation, Development Support etc. which items are still on the queue (an unacceptable alternative is to reserve one object flag bits). + * The bulk-checkin code used to depend on a file-scope static + singleton variable, which has been updated to pass an instance + throughout the callchain. + Fixes since v2.51 ----------------- @@ -164,6 +172,11 @@ including security updates, are included in this release. which has been disabled in Gitlab CI. (merge 608cf5b793 ps/gitlab-ci-disable-windows-monitoring later to maint). + * A broken or malicious "git fetch" can say that it has the same + object for many many times, and the upload-pack serving it can + exhaust memory storing them redundantly, which has been corrected. + (merge 88a2dc68c8 ps/upload-pack-oom-protection later to maint). + * Other code cleanup, docfix, build fix, etc. (merge 823d537fa7 kh/doc-git-log-markup-fix later to maint). (merge cf7efa4f33 rj/t6137-cygwin-fix later to maint). @@ -182,3 +195,6 @@ including security updates, are included in this release. (merge 2f4bf83ffc km/alias-doc-markup-fix later to maint). (merge b0d97aac19 kh/doc-markup-fixes later to maint). (merge f9a6705d9a tc/t0450-harden later to maint). + (merge c25651aefd ds/midx-write-fixes later to maint). + (merge 069c15d256 rs/object-name-extend-abbrev-len-update later to maint). + (merge bf5c224537 mm/worktree-doc-typofix later to maint). diff --git a/Documentation/config/worktree.adoc b/Documentation/config/worktree.adoc index 5e35c7d018aecd..9e3f84f748c4aa 100644 --- a/Documentation/config/worktree.adoc +++ b/Documentation/config/worktree.adoc @@ -15,5 +15,5 @@ worktree.useRelativePaths:: different locations or environments. Defaults to "false". + Note that setting `worktree.useRelativePaths` to "true" implies enabling the -`extension.relativeWorktrees` config (see linkgit:git-config[1]), +`extensions.relativeWorktrees` config (see linkgit:git-config[1]), thus making it incompatible with older versions of Git. diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc index 2870828d936192..209afd1b6152be 100644 --- a/Documentation/git-repo.adoc +++ b/Documentation/git-repo.adoc @@ -8,7 +8,7 @@ git-repo - Retrieve information about the repository SYNOPSIS -------- [synopsis] -git repo info [--format=(keyvalue|nul)] [...] +git repo info [--format=(keyvalue|nul)] [-z] [...] DESCRIPTION ----------- @@ -18,7 +18,7 @@ THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE. COMMANDS -------- -`info [--format=(keyvalue|nul)] [...]`:: +`info [--format=(keyvalue|nul)] [-z] [...]`:: Retrieve metadata-related information about the current repository. Only the requested data will be returned based on their keys (see "INFO KEYS" section below). @@ -40,6 +40,8 @@ supported: between the key and the value and using a NUL character after each value. This format is better suited for being parsed by another applications than `keyvalue`. Unlike in the `keyvalue` format, the values are never quoted. ++ +`-z` is an alias for `--format=nul`. INFO KEYS --------- @@ -53,6 +55,9 @@ values that they return: `layout.shallow`:: `true` if this is a shallow repository, otherwise `false`. +`object.format`:: + The object format (hash algorithm) used in the repository. + `references.format`:: The reference storage format. The valid values are: + diff --git a/builtin/add.c b/builtin/add.c index 0235854f8099c4..740c7c45817828 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -389,6 +389,7 @@ int cmd_add(int argc, char *seen = NULL; char *ps_matched = NULL; struct lock_file lock_file = LOCK_INIT; + struct odb_transaction *transaction; repo_config(repo, add_config, NULL); @@ -574,7 +575,7 @@ int cmd_add(int argc, string_list_clear(&only_match_skip_worktree, 0); } - begin_odb_transaction(); + transaction = begin_odb_transaction(repo->objects); ps_matched = xcalloc(pathspec.nr, 1); if (add_renormalize) @@ -593,7 +594,7 @@ int cmd_add(int argc, if (chmod_arg && pathspec.nr) exit_status |= chmod_pathspec(repo, &pathspec, chmod_arg[0], show_only); - end_odb_transaction(); + end_odb_transaction(transaction); finish: if (write_locked_index(repo->index, &lock_file, diff --git a/builtin/repo.c b/builtin/repo.c index 8c6e7f42aba107..bbb0966f2d2284 100644 --- a/builtin/repo.c +++ b/builtin/repo.c @@ -9,7 +9,7 @@ #include "shallow.h" static const char *const repo_usage[] = { - "git repo info [--format=(keyvalue|nul)] [...]", + "git repo info [--format=(keyvalue|nul)] [-z] [...]", NULL }; @@ -38,6 +38,12 @@ static int get_layout_shallow(struct repository *repo, struct strbuf *buf) return 0; } +static int get_object_format(struct repository *repo, struct strbuf *buf) +{ + strbuf_addstr(buf, repo->hash_algo->name); + return 0; +} + static int get_references_format(struct repository *repo, struct strbuf *buf) { strbuf_addstr(buf, @@ -49,6 +55,7 @@ static int get_references_format(struct repository *repo, struct strbuf *buf) static const struct field repo_info_fields[] = { { "layout.bare", get_layout_bare }, { "layout.shallow", get_layout_shallow }, + { "object.format", get_object_format }, { "references.format", get_references_format }, }; @@ -112,26 +119,40 @@ static int print_fields(int argc, const char **argv, return ret; } +static int parse_format_cb(const struct option *opt, + const char *arg, int unset UNUSED) +{ + enum output_format *format = opt->value; + + if (opt->short_name == 'z') + *format = FORMAT_NUL_TERMINATED; + else if (!strcmp(arg, "nul")) + *format = FORMAT_NUL_TERMINATED; + else if (!strcmp(arg, "keyvalue")) + *format = FORMAT_KEYVALUE; + else + die(_("invalid format '%s'"), arg); + + return 0; +} + static int repo_info(int argc, const char **argv, const char *prefix, struct repository *repo) { - const char *format_str = "keyvalue"; - enum output_format format; + enum output_format format = FORMAT_KEYVALUE; struct option options[] = { - OPT_STRING(0, "format", &format_str, N_("format"), - N_("output format")), + OPT_CALLBACK_F(0, "format", &format, N_("format"), + N_("output format"), + PARSE_OPT_NONEG, parse_format_cb), + OPT_CALLBACK_F('z', NULL, &format, NULL, + N_("synonym for --format=nul"), + PARSE_OPT_NONEG | PARSE_OPT_NOARG, + parse_format_cb), OPT_END() }; argc = parse_options(argc, argv, prefix, options, repo_usage, 0); - if (!strcmp(format_str, "keyvalue")) - format = FORMAT_KEYVALUE; - else if (!strcmp(format_str, "nul")) - format = FORMAT_NUL_TERMINATED; - else - die(_("invalid format '%s'"), format_str); - return print_fields(argc, argv, repo, format); } diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index 7ae7c82b6c05a6..28124b324d2641 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -584,6 +584,7 @@ static void unpack_all(void) { int i; unsigned char *hdr = fill(sizeof(struct pack_header)); + struct odb_transaction *transaction; if (get_be32(hdr) != PACK_SIGNATURE) die("bad pack file"); @@ -599,12 +600,12 @@ static void unpack_all(void) progress = start_progress(the_repository, _("Unpacking objects"), nr_objects); CALLOC_ARRAY(obj_list, nr_objects); - begin_odb_transaction(); + transaction = begin_odb_transaction(the_repository->objects); for (i = 0; i < nr_objects; i++) { unpack_one(i); display_progress(progress, i + 1); } - end_odb_transaction(); + end_odb_transaction(transaction); stop_progress(&progress); if (delta_list) diff --git a/builtin/update-index.c b/builtin/update-index.c index 2380f3ccd68c8c..2ba2d29c959fac 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -77,7 +77,7 @@ static void report(const char *fmt, ...) * objects invisible while a transaction is active, so flush the * transaction here before reporting a change made by update-index. */ - flush_odb_transaction(); + flush_odb_transaction(the_repository->objects->transaction); va_start(vp, fmt); vprintf(fmt, vp); putchar('\n'); @@ -940,6 +940,7 @@ int cmd_update_index(int argc, strbuf_getline_fn getline_fn; int parseopt_state = PARSE_OPT_UNKNOWN; struct repository *r = the_repository; + struct odb_transaction *transaction; struct option options[] = { OPT_BIT('q', NULL, &refresh_args.flags, N_("continue refresh even when index needs update"), @@ -1130,7 +1131,7 @@ int cmd_update_index(int argc, * Allow the object layer to optimize adding multiple objects in * a batch. */ - begin_odb_transaction(); + transaction = begin_odb_transaction(the_repository->objects); while (ctx.argc) { if (parseopt_state != PARSE_OPT_DONE) parseopt_state = parse_options_step(&ctx, options, @@ -1213,7 +1214,7 @@ int cmd_update_index(int argc, /* * By now we have added all of the new objects */ - end_odb_transaction(); + end_odb_transaction(transaction); if (split_index > 0) { if (repo_config_get_split_index(the_repository) == 0) diff --git a/bulk-checkin.c b/bulk-checkin.c index b2809ab0398136..124c49306769a5 100644 --- a/bulk-checkin.c +++ b/bulk-checkin.c @@ -19,11 +19,7 @@ #include "object-file.h" #include "odb.h" -static int odb_transaction_nesting; - -static struct tmp_objdir *bulk_fsync_objdir; - -static struct bulk_checkin_packfile { +struct bulk_checkin_packfile { char *pack_tmp_name; struct hashfile *f; off_t offset; @@ -32,27 +28,36 @@ static struct bulk_checkin_packfile { struct pack_idx_entry **written; uint32_t alloc_written; uint32_t nr_written; -} bulk_checkin_packfile; +}; + +struct odb_transaction { + struct object_database *odb; + + int nesting; + struct tmp_objdir *objdir; + struct bulk_checkin_packfile packfile; +}; -static void finish_tmp_packfile(struct strbuf *basename, - const char *pack_tmp_name, - struct pack_idx_entry **written_list, - uint32_t nr_written, - struct pack_idx_option *pack_idx_opts, +static void finish_tmp_packfile(struct odb_transaction *transaction, + struct strbuf *basename, unsigned char hash[]) { + struct bulk_checkin_packfile *state = &transaction->packfile; + struct repository *repo = transaction->odb->repo; char *idx_tmp_name = NULL; - stage_tmp_packfiles(the_repository, basename, pack_tmp_name, - written_list, nr_written, NULL, pack_idx_opts, hash, - &idx_tmp_name); - rename_tmp_packfile_idx(the_repository, basename, &idx_tmp_name); + stage_tmp_packfiles(repo, basename, state->pack_tmp_name, + state->written, state->nr_written, NULL, + &state->pack_idx_opts, hash, &idx_tmp_name); + rename_tmp_packfile_idx(repo, basename, &idx_tmp_name); free(idx_tmp_name); } -static void flush_bulk_checkin_packfile(struct bulk_checkin_packfile *state) +static void flush_bulk_checkin_packfile(struct odb_transaction *transaction) { + struct bulk_checkin_packfile *state = &transaction->packfile; + struct repository *repo = transaction->odb->repo; unsigned char hash[GIT_MAX_RAWSZ]; struct strbuf packname = STRBUF_INIT; @@ -69,17 +74,17 @@ static void flush_bulk_checkin_packfile(struct bulk_checkin_packfile *state) CSUM_HASH_IN_STREAM | CSUM_FSYNC | CSUM_CLOSE); } else { int fd = finalize_hashfile(state->f, hash, FSYNC_COMPONENT_PACK, 0); - fixup_pack_header_footer(the_hash_algo, fd, hash, state->pack_tmp_name, + fixup_pack_header_footer(repo->hash_algo, fd, hash, state->pack_tmp_name, state->nr_written, hash, state->offset); close(fd); } - strbuf_addf(&packname, "%s/pack/pack-%s.", repo_get_object_directory(the_repository), - hash_to_hex(hash)); - finish_tmp_packfile(&packname, state->pack_tmp_name, - state->written, state->nr_written, - &state->pack_idx_opts, hash); + strbuf_addf(&packname, "%s/pack/pack-%s.", + repo_get_object_directory(transaction->odb->repo), + hash_to_hex_algop(hash, repo->hash_algo)); + + finish_tmp_packfile(transaction, &packname, hash); for (uint32_t i = 0; i < state->nr_written; i++) free(state->written[i]); @@ -90,18 +95,18 @@ static void flush_bulk_checkin_packfile(struct bulk_checkin_packfile *state) strbuf_release(&packname); /* Make objects we just wrote available to ourselves */ - reprepare_packed_git(the_repository); + reprepare_packed_git(repo); } /* * Cleanup after batch-mode fsync_object_files. */ -static void flush_batch_fsync(void) +static void flush_batch_fsync(struct odb_transaction *transaction) { struct strbuf temp_path = STRBUF_INIT; struct tempfile *temp; - if (!bulk_fsync_objdir) + if (!transaction->objdir) return; /* @@ -113,7 +118,8 @@ static void flush_batch_fsync(void) * to ensure that the data in each new object file is durable before * the final name is visible. */ - strbuf_addf(&temp_path, "%s/bulk_fsync_XXXXXX", repo_get_object_directory(the_repository)); + strbuf_addf(&temp_path, "%s/bulk_fsync_XXXXXX", + repo_get_object_directory(transaction->odb->repo)); temp = xmks_tempfile(temp_path.buf); fsync_or_die(get_tempfile_fd(temp), get_tempfile_path(temp)); delete_tempfile(&temp); @@ -123,20 +129,21 @@ static void flush_batch_fsync(void) * Make the object files visible in the primary ODB after their data is * fully durable. */ - tmp_objdir_migrate(bulk_fsync_objdir); - bulk_fsync_objdir = NULL; + tmp_objdir_migrate(transaction->objdir); + transaction->objdir = NULL; } -static int already_written(struct bulk_checkin_packfile *state, struct object_id *oid) +static int already_written(struct odb_transaction *transaction, + struct object_id *oid) { /* The object may already exist in the repository */ - if (odb_has_object(the_repository->objects, oid, + if (odb_has_object(transaction->odb, oid, HAS_OBJECT_RECHECK_PACKED | HAS_OBJECT_FETCH_PROMISOR)) return 1; /* Might want to keep the list sorted */ - for (uint32_t i = 0; i < state->nr_written; i++) - if (oideq(&state->written[i]->oid, oid)) + for (uint32_t i = 0; i < transaction->packfile.nr_written; i++) + if (oideq(&transaction->packfile.written[i]->oid, oid)) return 1; /* This is a new object we need to keep */ @@ -235,13 +242,15 @@ static int stream_blob_to_pack(struct bulk_checkin_packfile *state, } /* Lazily create backing packfile for the state */ -static void prepare_to_stream(struct bulk_checkin_packfile *state, +static void prepare_to_stream(struct odb_transaction *transaction, unsigned flags) { + struct bulk_checkin_packfile *state = &transaction->packfile; if (!(flags & INDEX_WRITE_OBJECT) || state->f) return; - state->f = create_tmp_packfile(the_repository, &state->pack_tmp_name); + state->f = create_tmp_packfile(transaction->odb->repo, + &state->pack_tmp_name); reset_pack_idx_option(&state->pack_idx_opts); /* Pretend we are going to write only one object */ @@ -250,11 +259,11 @@ static void prepare_to_stream(struct bulk_checkin_packfile *state, die_errno("unable to write pack header"); } -static int deflate_blob_to_pack(struct bulk_checkin_packfile *state, - struct object_id *result_oid, - int fd, size_t size, - const char *path, unsigned flags) +int index_blob_bulk_checkin(struct odb_transaction *transaction, + struct object_id *result_oid, int fd, size_t size, + const char *path, unsigned flags) { + struct bulk_checkin_packfile *state = &transaction->packfile; off_t seekback, already_hashed_to; struct git_hash_ctx ctx; unsigned char obuf[16384]; @@ -268,21 +277,21 @@ static int deflate_blob_to_pack(struct bulk_checkin_packfile *state, header_len = format_object_header((char *)obuf, sizeof(obuf), OBJ_BLOB, size); - the_hash_algo->init_fn(&ctx); + transaction->odb->repo->hash_algo->init_fn(&ctx); git_hash_update(&ctx, obuf, header_len); /* Note: idx is non-NULL when we are writing */ if ((flags & INDEX_WRITE_OBJECT) != 0) { CALLOC_ARRAY(idx, 1); - prepare_to_stream(state, flags); + prepare_to_stream(transaction, flags); hashfile_checkpoint_init(state->f, &checkpoint); } already_hashed_to = 0; while (1) { - prepare_to_stream(state, flags); + prepare_to_stream(transaction, flags); if (idx) { hashfile_checkpoint(state->f, &checkpoint); idx->offset = state->offset; @@ -300,7 +309,7 @@ static int deflate_blob_to_pack(struct bulk_checkin_packfile *state, BUG("should not happen"); hashfile_truncate(state->f, &checkpoint); state->offset = checkpoint.offset; - flush_bulk_checkin_packfile(state); + flush_bulk_checkin_packfile(transaction); if (lseek(fd, seekback, SEEK_SET) == (off_t) -1) return error("cannot seek back"); } @@ -309,7 +318,7 @@ static int deflate_blob_to_pack(struct bulk_checkin_packfile *state, return 0; idx->crc32 = crc32_end(state->f); - if (already_written(state, result_oid)) { + if (already_written(transaction, result_oid)) { hashfile_truncate(state->f, &checkpoint); state->offset = checkpoint.offset; free(idx); @@ -323,7 +332,7 @@ static int deflate_blob_to_pack(struct bulk_checkin_packfile *state, return 0; } -void prepare_loose_object_bulk_checkin(void) +void prepare_loose_object_bulk_checkin(struct odb_transaction *transaction) { /* * We lazily create the temporary object directory @@ -331,15 +340,16 @@ void prepare_loose_object_bulk_checkin(void) * callers may not know whether any objects will be * added at the time they call begin_odb_transaction. */ - if (!odb_transaction_nesting || bulk_fsync_objdir) + if (!transaction || transaction->objdir) return; - bulk_fsync_objdir = tmp_objdir_create(the_repository, "bulk-fsync"); - if (bulk_fsync_objdir) - tmp_objdir_replace_primary_odb(bulk_fsync_objdir, 0); + transaction->objdir = tmp_objdir_create(transaction->odb->repo, "bulk-fsync"); + if (transaction->objdir) + tmp_objdir_replace_primary_odb(transaction->objdir, 0); } -void fsync_loose_object_bulk_checkin(int fd, const char *filename) +void fsync_loose_object_bulk_checkin(struct odb_transaction *transaction, + int fd, const char *filename) { /* * If we have an active ODB transaction, we issue a call that @@ -348,7 +358,7 @@ void fsync_loose_object_bulk_checkin(int fd, const char *filename) * before renaming the objects to their final names as part of * flush_batch_fsync. */ - if (!bulk_fsync_objdir || + if (!transaction || !transaction->objdir || git_fsync(fd, FSYNC_WRITEOUT_ONLY) < 0) { if (errno == ENOSYS) warning(_("core.fsyncMethod = batch is unsupported on this platform")); @@ -356,36 +366,38 @@ void fsync_loose_object_bulk_checkin(int fd, const char *filename) } } -int index_blob_bulk_checkin(struct object_id *oid, - int fd, size_t size, - const char *path, unsigned flags) +struct odb_transaction *begin_odb_transaction(struct object_database *odb) { - int status = deflate_blob_to_pack(&bulk_checkin_packfile, oid, fd, size, - path, flags); - if (!odb_transaction_nesting) - flush_bulk_checkin_packfile(&bulk_checkin_packfile); - return status; -} + if (!odb->transaction) { + CALLOC_ARRAY(odb->transaction, 1); + odb->transaction->odb = odb; + } -void begin_odb_transaction(void) -{ - odb_transaction_nesting += 1; + odb->transaction->nesting += 1; + + return odb->transaction; } -void flush_odb_transaction(void) +void flush_odb_transaction(struct odb_transaction *transaction) { - flush_batch_fsync(); - flush_bulk_checkin_packfile(&bulk_checkin_packfile); + if (!transaction) + return; + + flush_batch_fsync(transaction); + flush_bulk_checkin_packfile(transaction); } -void end_odb_transaction(void) +void end_odb_transaction(struct odb_transaction *transaction) { - odb_transaction_nesting -= 1; - if (odb_transaction_nesting < 0) + if (!transaction || transaction->nesting == 0) BUG("Unbalanced ODB transaction nesting"); - if (odb_transaction_nesting) + transaction->nesting -= 1; + + if (transaction->nesting) return; - flush_odb_transaction(); + flush_odb_transaction(transaction); + transaction->odb->transaction = NULL; + free(transaction); } diff --git a/bulk-checkin.h b/bulk-checkin.h index 7246ea58dcf348..ac8887f476b496 100644 --- a/bulk-checkin.h +++ b/bulk-checkin.h @@ -5,13 +5,20 @@ #define BULK_CHECKIN_H #include "object.h" +#include "odb.h" -void prepare_loose_object_bulk_checkin(void); -void fsync_loose_object_bulk_checkin(int fd, const char *filename); +struct odb_transaction; + +void prepare_loose_object_bulk_checkin(struct odb_transaction *transaction); +void fsync_loose_object_bulk_checkin(struct odb_transaction *transaction, + int fd, const char *filename); /* - * This creates one packfile per large blob unless bulk-checkin - * machinery is "plugged". + * This writes the specified object to a packfile. Objects written here + * during the same transaction are written to the same packfile. The + * packfile is not flushed until the transaction is flushed. The caller + * is expected to ensure a valid transaction is setup for objects to be + * recorded to. * * This also bypasses the usual "convert-to-git" dance, and that is on * purpose. We could write a streaming version of the converting @@ -24,8 +31,8 @@ void fsync_loose_object_bulk_checkin(int fd, const char *filename); * binary blobs, they generally do not want to get any conversion, and * callers should avoid this code path when filters are requested. */ -int index_blob_bulk_checkin(struct object_id *oid, - int fd, size_t size, +int index_blob_bulk_checkin(struct odb_transaction *transaction, + struct object_id *oid, int fd, size_t size, const char *path, unsigned flags); /* @@ -35,20 +42,20 @@ int index_blob_bulk_checkin(struct object_id *oid, * and objects are only visible after the outermost transaction * is complete or the transaction is flushed. */ -void begin_odb_transaction(void); +struct odb_transaction *begin_odb_transaction(struct object_database *odb); /* * Make any objects that are currently part of a pending object * database transaction visible. It is valid to call this function * even if no transaction is active. */ -void flush_odb_transaction(void); +void flush_odb_transaction(struct odb_transaction *transaction); /* * Tell the object database to make any objects from the * current transaction visible if this is the final nested * transaction. */ -void end_odb_transaction(void); +void end_odb_transaction(struct odb_transaction *transaction); #endif diff --git a/cache-tree.c b/cache-tree.c index 66ef2becbe01a4..d225554eedd920 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -474,6 +474,7 @@ static int update_one(struct cache_tree *it, int cache_tree_update(struct index_state *istate, int flags) { + struct odb_transaction *transaction; int skip, i; i = verify_cache(istate, flags); @@ -489,10 +490,10 @@ int cache_tree_update(struct index_state *istate, int flags) trace_performance_enter(); trace2_region_enter("cache_tree", "update", the_repository); - begin_odb_transaction(); + transaction = begin_odb_transaction(the_repository->objects); i = update_one(istate->cache_tree, istate->cache, istate->cache_nr, "", 0, &skip, flags); - end_odb_transaction(); + end_odb_transaction(transaction); trace2_region_leave("cache_tree", "update", the_repository); trace_performance_leave("cache_tree_update"); if (i < 0) diff --git a/midx-write.c b/midx-write.c index 1e99f6a26e9e91..c73010df6d3a4f 100644 --- a/midx-write.c +++ b/midx-write.c @@ -1,5 +1,3 @@ -#define DISABLE_SIGN_COMPARE_WARNINGS - #include "git-compat-util.h" #include "abspath.h" #include "config.h" @@ -24,6 +22,7 @@ #define BITMAP_POS_UNKNOWN (~((uint32_t)0)) #define MIDX_CHUNK_FANOUT_SIZE (sizeof(uint32_t) * 256) #define MIDX_CHUNK_LARGE_OFFSET_WIDTH (sizeof(uint64_t)) +#define NO_PREFERRED_PACK (~((uint32_t)0)) extern int midx_checksum_valid(struct multi_pack_index *m); extern void clear_midx_files_ext(struct odb_source *source, const char *ext, @@ -104,7 +103,7 @@ struct write_midx_context { unsigned large_offsets_needed:1; uint32_t num_large_offsets; - int preferred_pack_idx; + uint32_t preferred_pack_idx; int incremental; uint32_t num_multi_pack_indexes_before; @@ -261,7 +260,7 @@ static void midx_fanout_sort(struct midx_fanout *fanout) static void midx_fanout_add_midx_fanout(struct midx_fanout *fanout, struct multi_pack_index *m, uint32_t cur_fanout, - int preferred_pack) + uint32_t preferred_pack) { uint32_t start = m->num_objects_in_base, end; uint32_t cur_object; @@ -275,7 +274,7 @@ static void midx_fanout_add_midx_fanout(struct midx_fanout *fanout, end = m->num_objects_in_base + ntohl(m->chunk_oid_fanout[cur_fanout]); for (cur_object = start; cur_object < end; cur_object++) { - if ((preferred_pack > -1) && + if ((preferred_pack != NO_PREFERRED_PACK) && (preferred_pack == nth_midxed_pack_int_id(m, cur_object))) { /* * Objects from preferred packs are added @@ -365,7 +364,8 @@ static void compute_sorted_entries(struct write_midx_context *ctx, preferred, cur_fanout); } - if (-1 < ctx->preferred_pack_idx && ctx->preferred_pack_idx < start_pack) + if (ctx->preferred_pack_idx != NO_PREFERRED_PACK && + ctx->preferred_pack_idx < start_pack) midx_fanout_add_pack_fanout(&fanout, ctx->info, ctx->preferred_pack_idx, 1, cur_fanout); @@ -841,7 +841,7 @@ static int write_midx_bitmap(struct write_midx_context *ctx, uint32_t commits_nr, unsigned flags) { - int ret, i; + int ret; uint16_t options = 0; struct bitmap_writer writer; struct pack_idx_entry **index; @@ -868,7 +868,7 @@ static int write_midx_bitmap(struct write_midx_context *ctx, * this order). */ ALLOC_ARRAY(index, pdata->nr_objects); - for (i = 0; i < pdata->nr_objects; i++) + for (uint32_t i = 0; i < pdata->nr_objects; i++) index[i] = &pdata->objects[i].idx; bitmap_writer_init(&writer, ctx->repo, pdata, @@ -889,7 +889,7 @@ static int write_midx_bitmap(struct write_midx_context *ctx, * happens between bitmap_writer_build_type_index() and * bitmap_writer_finish(). */ - for (i = 0; i < pdata->nr_objects; i++) + for (uint32_t i = 0; i < pdata->nr_objects; i++) index[ctx->pack_order[i]] = &pdata->objects[i].idx; bitmap_writer_select_commits(&writer, commits, commits_nr); @@ -910,8 +910,7 @@ static int write_midx_bitmap(struct write_midx_context *ctx, return ret; } -static int fill_packs_from_midx(struct write_midx_context *ctx, - const char *preferred_pack_name, uint32_t flags) +static int fill_packs_from_midx(struct write_midx_context *ctx) { struct multi_pack_index *m; @@ -919,29 +918,10 @@ static int fill_packs_from_midx(struct write_midx_context *ctx, uint32_t i; for (i = 0; i < m->num_packs; i++) { - ALLOC_GROW(ctx->info, ctx->nr + 1, ctx->alloc); - - /* - * If generating a reverse index, need to have - * packed_git's loaded to compare their - * mtimes and object count. - * - * If a preferred pack is specified, need to - * have packed_git's loaded to ensure the chosen - * preferred pack has a non-zero object count. - */ - if (flags & MIDX_WRITE_REV_INDEX || - preferred_pack_name) { - if (prepare_midx_pack(m, m->num_packs_in_base + i)) { - error(_("could not load pack")); - return 1; - } - - if (open_pack_index(m->packs[i])) - die(_("could not open index for %s"), - m->packs[i]->pack_name); - } + if (prepare_midx_pack(m, m->num_packs_in_base + i)) + return error(_("could not load pack")); + ALLOC_GROW(ctx->info, ctx->nr + 1, ctx->alloc); fill_pack_info(&ctx->info[ctx->nr++], m->packs[i], m->pack_names[i], m->num_packs_in_base + i); @@ -1045,15 +1025,17 @@ static int write_midx_internal(struct odb_source *source, struct repository *r = source->odb->repo; struct strbuf midx_name = STRBUF_INIT; unsigned char midx_hash[GIT_MAX_RAWSZ]; - uint32_t i, start_pack; + uint32_t start_pack; struct hashfile *f = NULL; struct lock_file lk; struct tempfile *incr; - struct write_midx_context ctx = { 0 }; + struct write_midx_context ctx = { + .preferred_pack_idx = NO_PREFERRED_PACK, + }; int bitmapped_packs_concat_len = 0; int pack_name_concat_len = 0; int dropped_packs = 0; - int result = 0; + int result = -1; const char **keep_hashes = NULL; struct chunkfile *cf; @@ -1107,14 +1089,12 @@ static int write_midx_internal(struct odb_source *source, error(_("could not load reverse index for MIDX %s"), hash_to_hex_algop(get_midx_checksum(m), m->source->odb->repo->hash_algo)); - result = 1; goto cleanup; } ctx.num_multi_pack_indexes_before++; m = m->base_midx; } - } else if (ctx.m && fill_packs_from_midx(&ctx, preferred_pack_name, - flags) < 0) { + } else if (ctx.m && fill_packs_from_midx(&ctx)) { goto cleanup; } @@ -1150,17 +1130,20 @@ static int write_midx_internal(struct odb_source *source, */ if (!want_bitmap) clear_midx_files_ext(source, "bitmap", NULL); + result = 0; goto cleanup; } } - if (ctx.incremental && !ctx.nr) + if (ctx.incremental && !ctx.nr) { + result = 0; goto cleanup; /* nothing to do */ + } if (preferred_pack_name) { - ctx.preferred_pack_idx = -1; + ctx.preferred_pack_idx = NO_PREFERRED_PACK; - for (i = 0; i < ctx.nr; i++) { + for (size_t i = 0; i < ctx.nr; i++) { if (!cmp_idx_or_pack_name(preferred_pack_name, ctx.info[i].pack_name)) { ctx.preferred_pack_idx = i; @@ -1168,14 +1151,21 @@ static int write_midx_internal(struct odb_source *source, } } - if (ctx.preferred_pack_idx == -1) + if (ctx.preferred_pack_idx == NO_PREFERRED_PACK) warning(_("unknown preferred pack: '%s'"), preferred_pack_name); } else if (ctx.nr && (flags & (MIDX_WRITE_REV_INDEX | MIDX_WRITE_BITMAP))) { - struct packed_git *oldest = ctx.info[ctx.preferred_pack_idx].p; + struct packed_git *oldest = ctx.info[0].p; ctx.preferred_pack_idx = 0; + /* + * Attempt opening the pack index to populate num_objects. + * Ignore failiures as they can be expected and are not + * fatal during this selection time. + */ + open_pack_index(oldest); + if (packs_to_drop && packs_to_drop->nr) BUG("cannot write a MIDX bitmap during expiration"); @@ -1185,11 +1175,12 @@ static int write_midx_internal(struct odb_source *source, * pack-order has all of its objects selected from that pack * (and not another pack containing a duplicate) */ - for (i = 1; i < ctx.nr; i++) { + for (size_t i = 1; i < ctx.nr; i++) { struct packed_git *p = ctx.info[i].p; if (!oldest->num_objects || p->mtime < oldest->mtime) { oldest = p; + open_pack_index(oldest); ctx.preferred_pack_idx = i; } } @@ -1201,22 +1192,26 @@ static int write_midx_internal(struct odb_source *source, * objects to resolve, so the preferred value doesn't * matter. */ - ctx.preferred_pack_idx = -1; + ctx.preferred_pack_idx = NO_PREFERRED_PACK; } } else { /* * otherwise don't mark any pack as preferred to avoid * interfering with expiration logic below */ - ctx.preferred_pack_idx = -1; + ctx.preferred_pack_idx = NO_PREFERRED_PACK; } - if (ctx.preferred_pack_idx > -1) { + if (ctx.preferred_pack_idx != NO_PREFERRED_PACK) { struct packed_git *preferred = ctx.info[ctx.preferred_pack_idx].p; + + if (open_pack_index(preferred)) + die(_("failed to open preferred pack %s"), + ctx.info[ctx.preferred_pack_idx].pack_name); + if (!preferred->num_objects) { error(_("cannot select preferred pack %s with no objects"), preferred->pack_name); - result = 1; goto cleanup; } } @@ -1224,7 +1219,7 @@ static int write_midx_internal(struct odb_source *source, compute_sorted_entries(&ctx, start_pack); ctx.large_offsets_needed = 0; - for (i = 0; i < ctx.entries_nr; i++) { + for (size_t i = 0; i < ctx.entries_nr; i++) { if (ctx.entries[i].offset > 0x7fffffff) ctx.num_large_offsets++; if (ctx.entries[i].offset > 0xffffffff) @@ -1234,10 +1229,10 @@ static int write_midx_internal(struct odb_source *source, QSORT(ctx.info, ctx.nr, pack_info_compare); if (packs_to_drop && packs_to_drop->nr) { - int drop_index = 0; + size_t drop_index = 0; int missing_drops = 0; - for (i = 0; i < ctx.nr && drop_index < packs_to_drop->nr; i++) { + for (size_t i = 0; i < ctx.nr && drop_index < packs_to_drop->nr; i++) { int cmp = strcmp(ctx.info[i].pack_name, packs_to_drop->items[drop_index].string); @@ -1255,10 +1250,8 @@ static int write_midx_internal(struct odb_source *source, } } - if (missing_drops) { - result = 1; + if (missing_drops) goto cleanup; - } } /* @@ -1268,7 +1261,7 @@ static int write_midx_internal(struct odb_source *source, * pack_perm[old_id] = new_id */ ALLOC_ARRAY(ctx.pack_perm, ctx.nr); - for (i = 0; i < ctx.nr; i++) { + for (size_t i = 0; i < ctx.nr; i++) { if (ctx.info[i].expired) { dropped_packs++; ctx.pack_perm[ctx.info[i].orig_pack_int_id] = PACK_EXPIRED; @@ -1277,7 +1270,7 @@ static int write_midx_internal(struct odb_source *source, } } - for (i = 0; i < ctx.nr; i++) { + for (size_t i = 0; i < ctx.nr; i++) { if (ctx.info[i].expired) continue; pack_name_concat_len += strlen(ctx.info[i].pack_name) + 1; @@ -1304,7 +1297,6 @@ static int write_midx_internal(struct odb_source *source, if (ctx.nr - dropped_packs == 0) { error(_("no pack files to index.")); - result = 1; goto cleanup; } @@ -1324,13 +1316,13 @@ static int write_midx_internal(struct odb_source *source, incr = mks_tempfile_m(midx_name.buf, 0444); if (!incr) { error(_("unable to create temporary MIDX layer")); - return -1; + goto cleanup; } if (adjust_shared_perm(r, get_tempfile_path(incr))) { error(_("unable to adjust shared permissions for '%s'"), get_tempfile_path(incr)); - return -1; + goto cleanup; } f = hashfd(r->hash_algo, get_tempfile_fd(incr), @@ -1407,7 +1399,6 @@ static int write_midx_internal(struct odb_source *source, midx_hash, &pdata, commits, commits_nr, flags) < 0) { error(_("could not write multi-pack bitmap")); - result = 1; clear_packing_data(&pdata); free(commits); goto cleanup; @@ -1421,6 +1412,9 @@ static int write_midx_internal(struct odb_source *source, * have been freed in the previous if block. */ + if (ctx.num_multi_pack_indexes_before == UINT32_MAX) + die(_("too many multi-pack-indexes")); + CALLOC_ARRAY(keep_hashes, ctx.num_multi_pack_indexes_before + 1); if (ctx.incremental) { @@ -1430,18 +1424,18 @@ static int write_midx_internal(struct odb_source *source, if (!chainf) { error_errno(_("unable to open multi-pack-index chain file")); - return -1; + goto cleanup; } if (link_midx_to_chain(ctx.base_midx) < 0) - return -1; + goto cleanup; get_split_midx_filename_ext(source, &final_midx_name, midx_hash, MIDX_EXT_MIDX); if (rename_tempfile(&incr, final_midx_name.buf) < 0) { error_errno(_("unable to rename new multi-pack-index layer")); - return -1; + goto cleanup; } strbuf_release(&final_midx_name); @@ -1449,7 +1443,7 @@ static int write_midx_internal(struct odb_source *source, keep_hashes[ctx.num_multi_pack_indexes_before] = xstrdup(hash_to_hex_algop(midx_hash, r->hash_algo)); - for (i = 0; i < ctx.num_multi_pack_indexes_before; i++) { + for (uint32_t i = 0; i < ctx.num_multi_pack_indexes_before; i++) { uint32_t j = ctx.num_multi_pack_indexes_before - i - 1; keep_hashes[j] = xstrdup(hash_to_hex_algop(get_midx_checksum(m), @@ -1457,7 +1451,7 @@ static int write_midx_internal(struct odb_source *source, m = m->base_midx; } - for (i = 0; i < ctx.num_multi_pack_indexes_before + 1; i++) + for (uint32_t i = 0; i <= ctx.num_multi_pack_indexes_before; i++) fprintf(get_lock_file_fp(&lk), "%s\n", keep_hashes[i]); } else { keep_hashes[ctx.num_multi_pack_indexes_before] = @@ -1473,9 +1467,10 @@ static int write_midx_internal(struct odb_source *source, clear_midx_files(source, keep_hashes, ctx.num_multi_pack_indexes_before + 1, ctx.incremental); + result = 0; cleanup: - for (i = 0; i < ctx.nr; i++) { + for (size_t i = 0; i < ctx.nr; i++) { if (ctx.info[i].p) { close_pack(ctx.info[i].p); free(ctx.info[i].p); @@ -1488,7 +1483,7 @@ static int write_midx_internal(struct odb_source *source, free(ctx.pack_perm); free(ctx.pack_order); if (keep_hashes) { - for (i = 0; i < ctx.num_multi_pack_indexes_before + 1; i++) + for (uint32_t i = 0; i <= ctx.num_multi_pack_indexes_before; i++) free((char *)keep_hashes[i]); free(keep_hashes); } diff --git a/object-file.c b/object-file.c index 2bc36ab3ee8cbf..bc15af42450949 100644 --- a/object-file.c +++ b/object-file.c @@ -674,7 +674,7 @@ static void close_loose_object(struct odb_source *source, goto out; if (batch_fsync_enabled(FSYNC_COMPONENT_LOOSE_OBJECT)) - fsync_loose_object_bulk_checkin(fd, filename); + fsync_loose_object_bulk_checkin(source->odb->transaction, fd, filename); else if (fsync_object_files > 0) fsync_or_die(fd, filename); else @@ -852,7 +852,7 @@ static int write_loose_object(struct odb_source *source, static struct strbuf filename = STRBUF_INIT; if (batch_fsync_enabled(FSYNC_COMPONENT_LOOSE_OBJECT)) - prepare_loose_object_bulk_checkin(); + prepare_loose_object_bulk_checkin(source->odb->transaction); odb_loose_path(source, &filename, oid); @@ -941,7 +941,7 @@ int stream_loose_object(struct odb_source *source, int hdrlen; if (batch_fsync_enabled(FSYNC_COMPONENT_LOOSE_OBJECT)) - prepare_loose_object_bulk_checkin(); + prepare_loose_object_bulk_checkin(source->odb->transaction); /* Since oid is not determined, save tmp file to odb path. */ strbuf_addf(&filename, "%s/", source->path); @@ -1253,18 +1253,26 @@ int index_fd(struct index_state *istate, struct object_id *oid, * Call xsize_t() only when needed to avoid potentially unnecessary * die() for large files. */ - if (type == OBJ_BLOB && path && would_convert_to_git_filter_fd(istate, path)) + if (type == OBJ_BLOB && path && would_convert_to_git_filter_fd(istate, path)) { ret = index_stream_convert_blob(istate, oid, fd, path, flags); - else if (!S_ISREG(st->st_mode)) + } else if (!S_ISREG(st->st_mode)) { ret = index_pipe(istate, oid, fd, type, path, flags); - else if ((st->st_size >= 0 && (size_t) st->st_size <= repo_settings_get_big_file_threshold(istate->repo)) || - type != OBJ_BLOB || - (path && would_convert_to_git(istate, path))) + } else if ((st->st_size >= 0 && + (size_t)st->st_size <= repo_settings_get_big_file_threshold(istate->repo)) || + type != OBJ_BLOB || + (path && would_convert_to_git(istate, path))) { ret = index_core(istate, oid, fd, xsize_t(st->st_size), type, path, flags); - else - ret = index_blob_bulk_checkin(oid, fd, xsize_t(st->st_size), path, - flags); + } else { + struct odb_transaction *transaction; + + transaction = begin_odb_transaction(the_repository->objects); + ret = index_blob_bulk_checkin(transaction, + oid, fd, xsize_t(st->st_size), + path, flags); + end_odb_transaction(transaction); + } + close(fd); return ret; } diff --git a/object-name.c b/object-name.c index 732056ff5e305b..1e0118e8a66cfd 100644 --- a/object-name.c +++ b/object-name.c @@ -696,10 +696,9 @@ static inline char get_hex_char_from_oid(const struct object_id *oid, return hex[oid->hash[pos >> 1] & 0xf]; } -static int extend_abbrev_len(const struct object_id *oid, void *cb_data) +static int extend_abbrev_len(const struct object_id *oid, + struct min_abbrev_data *mad) { - struct min_abbrev_data *mad = cb_data; - unsigned int i = mad->init_len; while (mad->hex[i] && mad->hex[i] == get_hex_char_from_oid(oid, i)) i++; diff --git a/odb.h b/odb.h index 51fe8a5a929f16..bd7374f92f492b 100644 --- a/odb.h +++ b/odb.h @@ -92,6 +92,7 @@ struct odb_source { struct packed_git; struct cached_object_entry; +struct odb_transaction; /* * The object database encapsulates access to objects in a repository. It @@ -102,6 +103,13 @@ struct object_database { /* Repository that owns this database. */ struct repository *repo; + /* + * State of current current object database transaction. Only one + * transaction may be pending at a time. Is NULL when no transaction is + * configured. + */ + struct odb_transaction *transaction; + /* * Set of all object directories; the main directory is first (and * cannot be NULL after initialization). Subsequent directories are diff --git a/read-cache.c b/read-cache.c index 06ad74db2286ae..229b8ef11c9a74 100644 --- a/read-cache.c +++ b/read-cache.c @@ -3947,6 +3947,7 @@ int add_files_to_cache(struct repository *repo, const char *prefix, const struct pathspec *pathspec, char *ps_matched, int include_sparse, int flags) { + struct odb_transaction *transaction; struct update_callback_data data; struct rev_info rev; @@ -3972,9 +3973,9 @@ int add_files_to_cache(struct repository *repo, const char *prefix, * This function is invoked from commands other than 'add', which * may not have their own transaction active. */ - begin_odb_transaction(); + transaction = begin_odb_transaction(repo->objects); run_diff_files(&rev, DIFF_RACY_IS_MODIFIED); - end_odb_transaction(); + end_odb_transaction(transaction); release_revisions(&rev); return !!data.add_errors; diff --git a/t/t1900-repo.sh b/t/t1900-repo.sh index a69c715357e26b..2beba67889af25 100755 --- a/t/t1900-repo.sh +++ b/t/t1900-repo.sh @@ -63,6 +63,12 @@ test_expect_success 'setup remote' ' test_repo_info 'shallow repository = true is retrieved correctly' \ 'git clone --depth 1 "file://$PWD/remote"' 'shallow' 'layout.shallow' 'true' +test_repo_info 'object.format = sha1 is retrieved correctly' \ + 'git init --object-format=sha1' 'sha1' 'object.format' 'sha1' + +test_repo_info 'object.format = sha256 is retrieved correctly' \ + 'git init --object-format=sha256' 'sha256' 'object.format' 'sha256' + test_expect_success 'values returned in order requested' ' cat >expect <<-\EOF && layout.bare=false @@ -92,4 +98,16 @@ test_expect_success 'git-repo-info aborts when requesting an invalid format' ' test_cmp expect actual ' +test_expect_success '-z uses nul-terminated format' ' + printf "layout.bare\nfalse\0layout.shallow\nfalse\0" >expected && + git repo info -z layout.bare layout.shallow >actual && + test_cmp expected actual +' + +test_expect_success 'git repo info uses the last requested format' ' + echo "layout.bare=false" >expected && + git repo info --format=nul -z --format=keyvalue layout.bare >actual && + test_cmp expected actual +' + test_done diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh index 4e5e882989fc45..93f319a4b29fbb 100755 --- a/t/t5319-multi-pack-index.sh +++ b/t/t5319-multi-pack-index.sh @@ -989,6 +989,23 @@ test_expect_success 'repack --batch-size=0 repacks everything' ' ) ' +test_expect_success EXPENSIVE 'repack/expire with many packs' ' + cp -r dup many && + ( + cd many && + + for i in $(test_seq 1 100) + do + test_commit extra$i && + git maintenance run --task=loose-objects || return 1 + done && + + git multi-pack-index write && + git multi-pack-index repack && + git multi-pack-index expire + ) +' + test_expect_success 'repack --batch-size= repacks everything' ' ( cd dup2 && @@ -1083,7 +1100,10 @@ test_expect_success 'load reverse index when missing .idx, .pack' ' mv $idx.bak $idx && mv $pack $pack.bak && - git cat-file --batch-check="%(objectsize:disk)" err && + test_grep "could not load pack" err ) ' diff --git a/t/t5530-upload-pack-error.sh b/t/t5530-upload-pack-error.sh index 558eedf25a4c9b..d40292cfb7b48f 100755 --- a/t/t5530-upload-pack-error.sh +++ b/t/t5530-upload-pack-error.sh @@ -4,8 +4,6 @@ test_description='errors in upload-pack' . ./test-lib.sh -D=$(pwd) - corrupt_repo () { object_sha1=$(git rev-parse "$1") && ob=$(expr "$object_sha1" : "\(..\)") && @@ -21,11 +19,7 @@ test_expect_success 'setup and corrupt repository' ' test_tick && echo changed >file && git commit -a -m changed && - corrupt_repo HEAD:file - -' - -test_expect_success 'fsck fails' ' + corrupt_repo HEAD:file && test_must_fail git fsck ' @@ -40,17 +34,12 @@ test_expect_success 'upload-pack fails due to error in pack-objects packing' ' ' test_expect_success 'corrupt repo differently' ' - git hash-object -w file && - corrupt_repo HEAD^^{tree} - -' - -test_expect_success 'fsck fails' ' + corrupt_repo HEAD^^{tree} && test_must_fail git fsck ' -test_expect_success 'upload-pack fails due to error in rev-list' ' +test_expect_success 'upload-pack fails due to error in rev-list' ' printf "%04xwant %s\n%04xshallow %s00000009done\n0000" \ $(($hexsz + 10)) $(git rev-parse HEAD) \ $(($hexsz + 12)) $(git rev-parse HEAD^) >input && @@ -59,7 +48,6 @@ test_expect_success 'upload-pack fails due to error in rev-list' ' ' test_expect_success 'upload-pack fails due to bad want (no object)' ' - printf "%04xwant %s multi_ack_detailed\n00000009done\n0000" \ $(($hexsz + 29)) $(test_oid deadbeef) >input && test_must_fail git upload-pack . output 2>output.err && @@ -69,7 +57,6 @@ test_expect_success 'upload-pack fails due to bad want (no object)' ' ' test_expect_success 'upload-pack fails due to bad want (not tip)' ' - oid=$(echo an object we have | git hash-object -w --stdin) && printf "%04xwant %s multi_ack_detailed\n00000009done\n0000" \ $(($hexsz + 29)) "$oid" >input && @@ -80,7 +67,6 @@ test_expect_success 'upload-pack fails due to bad want (not tip)' ' ' test_expect_success 'upload-pack fails due to error in pack-objects enumeration' ' - printf "%04xwant %s\n00000009done\n0000" \ $((hexsz + 10)) $(git rev-parse HEAD) >input && test_must_fail git upload-pack . /dev/null 2>output.err && @@ -105,18 +91,48 @@ test_expect_success 'upload-pack tolerates EOF just after stateless client wants test_cmp expect actual ' -test_expect_success 'create empty repository' ' - - mkdir foo && - cd foo && - git init - -' - test_expect_success 'fetch fails' ' + git init foo && + test_must_fail git -C foo fetch .. main +' - test_must_fail git fetch .. main +test_expect_success 'upload-pack ACKs repeated non-commit objects repeatedly (protocol v0)' ' + commit_id=$(git rev-parse HEAD) && + tree_id=$(git rev-parse HEAD^{tree}) && + test-tool pkt-line pack >request <<-EOF && + want $commit_id + 0000 + have $tree_id + have $tree_id + 0000 + EOF + git upload-pack --stateless-rpc . actual && + depacketize actual.raw && + grep ^ACK actual.raw >actual.acks && + cat >expect <<-EOF && + ACK $tree_id + ACK $tree_id + EOF + test_cmp expect actual.acks +' +test_expect_success 'upload-pack ACKs repeated non-commit objects once only (protocol v2)' ' + commit_id=$(git rev-parse HEAD) && + tree_id=$(git rev-parse HEAD^{tree}) && + test-tool pkt-line pack >request <<-EOF && + command=fetch + object-format=$(test_oid algo) + 0001 + want $commit_id + have $tree_id + have $tree_id + 0000 + EOF + GIT_PROTOCOL=version=2 git upload-pack . actual && + depacketize actual.raw && + grep ^ACK actual.raw >actual.acks && + echo "ACK $tree_id" >expect && + test_cmp expect actual.acks ' test_done diff --git a/upload-pack.c b/upload-pack.c index 91fcdcad9b5b48..f78fabc1e1089a 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -476,20 +476,17 @@ static void create_pack_file(struct upload_pack_data *pack_data, static int do_got_oid(struct upload_pack_data *data, const struct object_id *oid) { - int we_knew_they_have = 0; struct object *o = parse_object_with_flags(the_repository, oid, PARSE_OBJECT_SKIP_HASH_CHECK | PARSE_OBJECT_DISCARD_TREE); if (!o) die("oops (%s)", oid_to_hex(oid)); + if (o->type == OBJ_COMMIT) { struct commit_list *parents; struct commit *commit = (struct commit *)o; - if (o->flags & THEY_HAVE) - we_knew_they_have = 1; - else - o->flags |= THEY_HAVE; + if (!data->oldest_have || (commit->date < data->oldest_have)) data->oldest_have = commit->date; for (parents = commit->parents; @@ -497,11 +494,13 @@ static int do_got_oid(struct upload_pack_data *data, const struct object_id *oid parents = parents->next) parents->item->object.flags |= THEY_HAVE; } - if (!we_knew_they_have) { - add_object_array(o, NULL, &data->have_obj); - return 1; - } - return 0; + + if (o->flags & THEY_HAVE) + return 0; + o->flags |= THEY_HAVE; + + add_object_array(o, NULL, &data->have_obj); + return 1; } static int got_oid(struct upload_pack_data *data,