Skip to content

Commit

Permalink
Apply some v2.48 regression bugfixes (#5376)
Browse files Browse the repository at this point in the history
Now that most of the Git contributors are back from the holidays, there
is an influx of bug fixes. This is precisely why I held off from rushing
Git for Windows v2.48.0 out the door.

These bug fixes are mostly taken from upstream's branches; In some
cases, though, I had to apply them directly from the Git mailing list
because they did not make it into git/git yet.

I deem those bug fixes necessary to get Git for Windows into a somewhat
healthy state again.
  • Loading branch information
dscho authored and Git for Windows Build Agent committed Jan 28, 2025
2 parents 0f0741e + 664c0c6 commit 1603176
Show file tree
Hide file tree
Showing 13 changed files with 102 additions and 54 deletions.
15 changes: 8 additions & 7 deletions builtin/fetch.c
Original file line number Diff line number Diff line change
Expand Up @@ -1618,9 +1618,9 @@ static void report_set_head(const char *remote, const char *head_name,
}

static int set_head(const struct ref *remote_refs, int follow_remote_head,
const char *no_warn_branch)
const char *no_warn_branch, int mirror)
{
int result = 0, create_only, is_bare, was_detached;
int result = 0, create_only, baremirror, was_detached;
struct strbuf b_head = STRBUF_INIT, b_remote_head = STRBUF_INIT,
b_local_head = STRBUF_INIT;
const char *remote = gtransport->remote->name;
Expand Down Expand Up @@ -1655,17 +1655,17 @@ static int set_head(const struct ref *remote_refs, int follow_remote_head,

if (!head_name)
goto cleanup;
is_bare = is_bare_repository();
create_only = follow_remote_head == FOLLOW_REMOTE_ALWAYS ? 0 : !is_bare;
if (is_bare) {
baremirror = is_bare_repository() && mirror;
create_only = follow_remote_head == FOLLOW_REMOTE_ALWAYS ? 0 : !baremirror;
if (baremirror) {
strbuf_addstr(&b_head, "HEAD");
strbuf_addf(&b_remote_head, "refs/heads/%s", head_name);
} else {
strbuf_addf(&b_head, "refs/remotes/%s/HEAD", remote);
strbuf_addf(&b_remote_head, "refs/remotes/%s/%s", remote, head_name);
}
/* make sure it's valid */
if (!is_bare && !refs_ref_exists(refs, b_remote_head.buf)) {
if (!baremirror && !refs_ref_exists(refs, b_remote_head.buf)) {
result = 1;
goto cleanup;
}
Expand Down Expand Up @@ -1925,7 +1925,8 @@ static int do_fetch(struct transport *transport,
}
}
if (set_head(remote_refs, transport->remote->follow_remote_head,
transport->remote->no_warn_branch))
transport->remote->no_warn_branch,
transport->remote->mirror))
;
/*
* Way too many cases where this can go wrong
Expand Down
2 changes: 2 additions & 0 deletions grep.c
Original file line number Diff line number Diff line change
Expand Up @@ -1646,6 +1646,8 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle

bol = gs->buf;
left = gs->size;
if (left && gs->buf[left-1] == '\n')
left--;
while (left) {
const char *eol;
int hit;
Expand Down
24 changes: 16 additions & 8 deletions refs.c
Original file line number Diff line number Diff line change
Expand Up @@ -1318,13 +1318,21 @@ int ref_transaction_update(struct ref_transaction *transaction,
return 0;
}

int ref_transaction_update_reflog(struct ref_transaction *transaction,
const char *refname,
const struct object_id *new_oid,
const struct object_id *old_oid,
const char *committer_info, unsigned int flags,
const char *msg, unsigned int index,
struct strbuf *err)
/*
* Similar to`ref_transaction_update`, but this function is only for adding
* a reflog update. Supports providing custom committer information. The index
* field can be utiltized to order updates as desired. When not used, the
* updates default to being ordered by refname.
*/
static int ref_transaction_update_reflog(struct ref_transaction *transaction,
const char *refname,
const struct object_id *new_oid,
const struct object_id *old_oid,
const char *committer_info,
unsigned int flags,
const char *msg,
uint64_t index,
struct strbuf *err)
{
struct ref_update *update;

Expand Down Expand Up @@ -2805,7 +2813,7 @@ static int migrate_one_ref(const char *refname, const char *referent UNUSED, con
}

struct reflog_migration_data {
unsigned int index;
uint64_t index;
const char *refname;
struct ref_store *old_refs;
struct ref_transaction *transaction;
Expand Down
14 changes: 0 additions & 14 deletions refs.h
Original file line number Diff line number Diff line change
Expand Up @@ -771,20 +771,6 @@ int ref_transaction_update(struct ref_transaction *transaction,
unsigned int flags, const char *msg,
struct strbuf *err);

/*
* Similar to`ref_transaction_update`, but this function is only for adding
* a reflog update. Supports providing custom committer information. The index
* field can be utiltized to order updates as desired. When not used, the
* updates default to being ordered by refname.
*/
int ref_transaction_update_reflog(struct ref_transaction *transaction,
const char *refname,
const struct object_id *new_oid,
const struct object_id *old_oid,
const char *committer_info, unsigned int flags,
const char *msg, unsigned int index,
struct strbuf *err);

/*
* Add a reference creation to transaction. new_oid is the value that
* the reference should have after the update; it must not be
Expand Down
4 changes: 2 additions & 2 deletions refs/refs-internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ struct ref_update {
* when migrating reflogs and we want to ensure we carry over the
* same order.
*/
unsigned int index;
uint64_t index;

/*
* If this ref_update was split off of a symref update via
Expand Down Expand Up @@ -203,7 +203,7 @@ struct ref_transaction {
enum ref_transaction_state state;
void *backend_data;
unsigned int flags;
unsigned int max_index;
uint64_t max_index;
};

/*
Expand Down
22 changes: 16 additions & 6 deletions refs/reftable-backend.c
Original file line number Diff line number Diff line change
Expand Up @@ -942,7 +942,7 @@ struct write_transaction_table_arg {
size_t updates_nr;
size_t updates_alloc;
size_t updates_expected;
unsigned int max_index;
uint64_t max_index;
};

struct reftable_transaction_data {
Expand Down Expand Up @@ -1444,7 +1444,9 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data
* multiple entries. Each entry will contain a different update_index,
* so set the limits accordingly.
*/
reftable_writer_set_limits(writer, ts, ts + arg->max_index);
ret = reftable_writer_set_limits(writer, ts, ts + arg->max_index);
if (ret < 0)
goto done;

for (i = 0; i < arg->updates_nr; i++) {
struct reftable_transaction_update *tx_update = &arg->updates[i];
Expand Down Expand Up @@ -1766,7 +1768,9 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)
deletion_ts = creation_ts = reftable_stack_next_update_index(arg->be->stack);
if (arg->delete_old)
creation_ts++;
reftable_writer_set_limits(writer, deletion_ts, creation_ts);
ret = reftable_writer_set_limits(writer, deletion_ts, creation_ts);
if (ret < 0)
goto done;

/*
* Add the new reference. If this is a rename then we also delete the
Expand Down Expand Up @@ -2298,7 +2302,9 @@ static int write_reflog_existence_table(struct reftable_writer *writer,
if (ret <= 0)
goto done;

reftable_writer_set_limits(writer, ts, ts);
ret = reftable_writer_set_limits(writer, ts, ts);
if (ret < 0)
goto done;

/*
* The existence entry has both old and new object ID set to the
Expand Down Expand Up @@ -2357,7 +2363,9 @@ static int write_reflog_delete_table(struct reftable_writer *writer, void *cb_da
uint64_t ts = reftable_stack_next_update_index(arg->stack);
int ret;

reftable_writer_set_limits(writer, ts, ts);
ret = reftable_writer_set_limits(writer, ts, ts);
if (ret < 0)
goto out;

ret = reftable_stack_init_log_iterator(arg->stack, &it);
if (ret < 0)
Expand Down Expand Up @@ -2434,7 +2442,9 @@ static int write_reflog_expiry_table(struct reftable_writer *writer, void *cb_da
if (arg->records[i].value_type == REFTABLE_LOG_UPDATE)
live_records++;

reftable_writer_set_limits(writer, ts, ts);
ret = reftable_writer_set_limits(writer, ts, ts);
if (ret < 0)
return ret;

if (!is_null_oid(&arg->update_oid)) {
struct reftable_ref_record ref = {0};
Expand Down
1 change: 1 addition & 0 deletions reftable/reftable-error.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ enum reftable_error {

/* Misuse of the API:
* - on writing a record with NULL refname.
* - on writing a record before setting the writer limits.
* - on writing a reftable_ref_record outside the table limits
* - on writing a ref or log record before the stack's
* next_update_inde*x
Expand Down
24 changes: 14 additions & 10 deletions reftable/reftable-writer.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,17 +124,21 @@ int reftable_writer_new(struct reftable_writer **out,
int (*flush_func)(void *),
void *writer_arg, const struct reftable_write_options *opts);

/* Set the range of update indices for the records we will add. When writing a
table into a stack, the min should be at least
reftable_stack_next_update_index(), or REFTABLE_API_ERROR is returned.
For transactional updates to a stack, typically min==max, and the
update_index can be obtained by inspeciting the stack. When converting an
existing ref database into a single reftable, this would be a range of
update-index timestamps.
/*
* Set the range of update indices for the records we will add. When writing a
* table into a stack, the min should be at least
* reftable_stack_next_update_index(), or REFTABLE_API_ERROR is returned.
*
* For transactional updates to a stack, typically min==max, and the
* update_index can be obtained by inspeciting the stack. When converting an
* existing ref database into a single reftable, this would be a range of
* update-index timestamps.
*
* The function should be called before adding any records to the writer. If not
* it will fail with REFTABLE_API_ERROR.
*/
void reftable_writer_set_limits(struct reftable_writer *w, uint64_t min,
uint64_t max);
int reftable_writer_set_limits(struct reftable_writer *w, uint64_t min,
uint64_t max);

/*
Add a reftable_ref_record. The record should have names that come after
Expand Down
6 changes: 4 additions & 2 deletions reftable/stack.c
Original file line number Diff line number Diff line change
Expand Up @@ -1058,8 +1058,10 @@ static int stack_write_compact(struct reftable_stack *st,

for (size_t i = first; i <= last; i++)
st->stats.bytes += st->readers[i]->size;
reftable_writer_set_limits(wr, st->readers[first]->min_update_index,
st->readers[last]->max_update_index);
err = reftable_writer_set_limits(wr, st->readers[first]->min_update_index,
st->readers[last]->max_update_index);
if (err < 0)
goto done;

err = reftable_merged_table_new(&mt, st->readers + first, subtabs_len,
st->opts.hash_id);
Expand Down
13 changes: 11 additions & 2 deletions reftable/writer.c
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,20 @@ int reftable_writer_new(struct reftable_writer **out,
return 0;
}

void reftable_writer_set_limits(struct reftable_writer *w, uint64_t min,
uint64_t max)
int reftable_writer_set_limits(struct reftable_writer *w, uint64_t min,
uint64_t max)
{
/*
* The limits should be set before any records are added to the writer.
* Check if any records were added by checking if `last_key` was set.
*/
if (w->last_key.len)
return REFTABLE_API_ERROR;

w->min_update_index = min;
w->max_update_index = max;

return 0;
}

static void writer_release(struct reftable_writer *w)
Expand Down
10 changes: 10 additions & 0 deletions t/t5505-remote.sh
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,16 @@ test_expect_success 'add --mirror setting HEAD' '
)
'

test_expect_success 'non-mirror fetch does not interfere with mirror' '
mkdir headnotmain &&
(
cd headnotmain &&
git init --bare -b notmain &&
git remote add -f other ../two &&
test "$(git symbolic-ref HEAD)" = "refs/heads/notmain"
)
'

test_expect_success 'add --mirror=fetch' '
mkdir mirror-fetch &&
git init -b main mirror-fetch/parent &&
Expand Down
13 changes: 13 additions & 0 deletions t/t5510-fetch.sh
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,19 @@ test_expect_success "fetch test remote HEAD" '
branch=$(git rev-parse refs/remotes/origin/main) &&
test "z$head" = "z$branch"'

test_expect_success "fetch test remote HEAD in bare repository" '
cd "$D" &&
git init --bare barerepo &&
cd barerepo &&
git remote add upstream ../two &&
git fetch upstream &&
git rev-parse --verify refs/remotes/upstream/HEAD &&
git rev-parse --verify refs/remotes/upstream/main &&
head=$(git rev-parse refs/remotes/upstream/HEAD) &&
branch=$(git rev-parse refs/remotes/upstream/main) &&
test "z$head" = "z$branch"'


test_expect_success "fetch test remote HEAD change" '
cd "$D" &&
cd two &&
Expand Down
8 changes: 5 additions & 3 deletions t/unit-tests/t-reftable-stack.c
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ static void t_read_file(void)
static int write_test_ref(struct reftable_writer *wr, void *arg)
{
struct reftable_ref_record *ref = arg;
reftable_writer_set_limits(wr, ref->update_index, ref->update_index);
check(!reftable_writer_set_limits(wr, ref->update_index,
ref->update_index));
return reftable_writer_add_ref(wr, ref);
}

Expand Down Expand Up @@ -143,7 +144,8 @@ static int write_test_log(struct reftable_writer *wr, void *arg)
{
struct write_log_arg *wla = arg;

reftable_writer_set_limits(wr, wla->update_index, wla->update_index);
check(!reftable_writer_set_limits(wr, wla->update_index,
wla->update_index));
return reftable_writer_add_log(wr, wla->log);
}

Expand Down Expand Up @@ -961,7 +963,7 @@ static void t_reflog_expire(void)

static int write_nothing(struct reftable_writer *wr, void *arg UNUSED)
{
reftable_writer_set_limits(wr, 1, 1);
check(!reftable_writer_set_limits(wr, 1, 1));
return 0;
}

Expand Down

0 comments on commit 1603176

Please sign in to comment.