From: Stefan Sperling Subject: Re: always fetch remote HEAD except when -b is used To: Mark Jamsek Cc: Christian Weisgerber , gameoftrees@openbsd.org Date: Fri, 17 Feb 2023 13:35:44 +0100 On Fri, Feb 17, 2023 at 02:59:44AM +1100, Mark Jamsek wrote: > Actual diff implementing stsp's fetch suggestion: This seems fine to me. Thanks! We will continue fine-tuning in time I suppose ;) > diffstat /home/mark/src/got > M got/got.1 | 4+ 0- > M got/got.c | 53+ 9- > M include/got_fetch.h | 2+ 2- > M lib/fetch.c | 3+ 2- > M lib/got_lib_privsep.h | 3+ 1- > M lib/privsep.c | 15+ 5- > M libexec/got-fetch-pack/got-fetch-pack.c | 35+ 11- > > 7 files changed, 115 insertions(+), 30 deletions(-) > > diff /home/mark/src/got > commit - ac51614e087279834159667c6f71fffe546cffa6 > path + /home/mark/src/got > blob - d9209e2ae1dd1c1aa01e368bc0016cf86830a9e7 > file + got/got.1 > --- got/got.1 > +++ got/got.1 > @@ -358,6 +358,10 @@ This default behaviour can be overridden with the > .Xr got.conf 5 > or via a work tree, or said branches are not found on the server, a branch > resolved via the remote repository's HEAD reference will be fetched. > +Likewise, if a HEAD reference for the > +.Ar remote-repository > +exists but its target no longer matches the remote HEAD, then > +the new target branch will be fetched. > This default behaviour can be overridden with the > .Fl a > and > blob - 1ef76ea95b69bf7e45b91f9053a232d241269e7d > file + got/got.c > --- got/got.c > +++ got/got.c > @@ -1544,7 +1544,7 @@ cmd_clone(int argc, char *argv[]) > struct got_fetch_progress_arg fpa; > char *git_url = NULL; > int verbosity = 0, fetch_all_branches = 0, mirror_references = 0; > - int list_refs_only = 0; > + int bflag = 0, list_refs_only = 0; > int *pack_fds = NULL; > > TAILQ_INIT(&refs); > @@ -1562,6 +1562,7 @@ cmd_clone(int argc, char *argv[]) > optarg, NULL); > if (error) > return error; > + bflag = 1; > break; > case 'l': > list_refs_only = 1; > @@ -1717,7 +1718,7 @@ cmd_clone(int argc, char *argv[]) > error = got_fetch_pack(&pack_hash, &refs, &symrefs, > GOT_FETCH_DEFAULT_REMOTE_NAME, mirror_references, > fetch_all_branches, &wanted_branches, &wanted_refs, > - list_refs_only, verbosity, fetchfd, repo, NULL, 0, > + list_refs_only, verbosity, fetchfd, repo, NULL, NULL, bflag, > fetch_progress, &fpa); > if (error) > goto done; > @@ -2282,6 +2283,8 @@ cmd_fetch(int argc, char *argv[]) > const struct got_gotconfig *repo_conf = NULL, *worktree_conf = NULL; > struct got_pathlist_head refs, symrefs, wanted_branches, wanted_refs; > struct got_pathlist_entry *pe; > + struct got_reflist_head remote_refs; > + struct got_reflist_entry *re; > struct got_object_id *pack_hash = NULL; > int i, ch, fetchfd = -1, fetchstatus; > pid_t fetchpid = -1; > @@ -2289,10 +2292,11 @@ cmd_fetch(int argc, char *argv[]) > int verbosity = 0, fetch_all_branches = 0, list_refs_only = 0; > int delete_refs = 0, replace_tags = 0, delete_remote = 0; > int *pack_fds = NULL, have_bflag = 0; > - const char *worktree_branch = NULL; > + const char *remote_head = NULL, *worktree_branch = NULL; > > TAILQ_INIT(&refs); > TAILQ_INIT(&symrefs); > + TAILQ_INIT(&remote_refs); > TAILQ_INIT(&wanted_branches); > TAILQ_INIT(&wanted_refs); > > @@ -2531,12 +2535,51 @@ cmd_fetch(int argc, char *argv[]) > if (error) > goto done; > > - if (worktree && !have_bflag) { > - const char *refname; > + if (!have_bflag) { > + /* > + * If set, get this remote's HEAD ref target so > + * if it has changed on the server we can fetch it. > + */ > + error = got_ref_list(&remote_refs, repo, "refs/remotes", > + got_ref_cmp_by_name, repo); > + if (error) > + goto done; > > - refname = got_worktree_get_head_ref_name(worktree); > - if (strncmp(refname, "refs/heads/", 11) == 0) > - worktree_branch = refname; > + TAILQ_FOREACH(re, &remote_refs, entry) { > + const char *remote_refname, *remote_target; > + size_t remote_name_len; > + > + if (!got_ref_is_symbolic(re->ref)) > + continue; > + > + remote_name_len = strlen(remote->name); > + remote_refname = got_ref_get_name(re->ref); > + > + /* we only want refs/remotes/$remote->name/HEAD */ > + if (strncmp(remote_refname + 13, remote->name, > + remote_name_len) != 0) > + continue; > + > + if (strcmp(remote_refname + remote_name_len + 14, > + GOT_REF_HEAD) != 0) > + continue; > + > + /* > + * Take the name itself because we already > + * only match with refs/heads/ in fetch_pack(). > + */ > + remote_target = got_ref_get_symref_target(re->ref); > + remote_head = remote_target + remote_name_len + 14; > + break; > + } > + > + if (worktree) { > + const char *refname; > + > + refname = got_worktree_get_head_ref_name(worktree); > + if (strncmp(refname, "refs/heads/", 11) == 0) > + worktree_branch = refname; > + } > } > > fpa.last_scaled_size[0] = '\0'; > @@ -2551,7 +2594,7 @@ cmd_fetch(int argc, char *argv[]) > error = got_fetch_pack(&pack_hash, &refs, &symrefs, remote->name, > remote->mirror_references, fetch_all_branches, &wanted_branches, > &wanted_refs, list_refs_only, verbosity, fetchfd, repo, > - worktree_branch, have_bflag, fetch_progress, &fpa); > + worktree_branch, remote_head, have_bflag, fetch_progress, &fpa); > if (error) > goto done; > > @@ -2730,6 +2773,7 @@ done: > got_pathlist_free(&symrefs, GOT_PATHLIST_FREE_ALL); > got_pathlist_free(&wanted_branches, GOT_PATHLIST_FREE_NONE); > got_pathlist_free(&wanted_refs, GOT_PATHLIST_FREE_NONE); > + got_ref_list_free(&remote_refs); > free(id_str); > free(cwd); > free(repo_path); > blob - 5768c9157c81ca12b00f65491c540dd712c37e6a > file + include/got_fetch.h > --- include/got_fetch.h > +++ include/got_fetch.h > @@ -46,5 +46,5 @@ const struct got_error *got_fetch_pack(struct got_obje > const struct got_error *got_fetch_pack(struct got_object_id **, > struct got_pathlist_head *, struct got_pathlist_head *, const char *, > int, int, struct got_pathlist_head *, struct got_pathlist_head *, > - int, int, int, struct got_repository *, const char *, int, > - got_fetch_progress_cb, void *); > + int, int, int, struct got_repository *, const char *, const char *, > + int, got_fetch_progress_cb, void *); > blob - e37808ed2a0bf37771cbf76e39cfa92b60ad43cc > file + lib/fetch.c > --- lib/fetch.c > +++ lib/fetch.c > @@ -105,7 +105,8 @@ got_fetch_pack(struct got_object_id **pack_hash, struc > struct got_pathlist_head *wanted_branches, > struct got_pathlist_head *wanted_refs, int list_refs_only, int verbosity, > int fetchfd, struct got_repository *repo, const char *worktree_refname, > - int no_head, got_fetch_progress_cb progress_cb, void *progress_arg) > + const char *remote_head, int no_head, got_fetch_progress_cb progress_cb, > + void *progress_arg) > { > size_t i; > int imsg_fetchfds[2], imsg_idxfds[2]; > @@ -261,7 +262,7 @@ got_fetch_pack(struct got_object_id **pack_hash, struc > } > err = got_privsep_send_fetch_req(&fetchibuf, nfetchfd, &have_refs, > fetch_all_branches, wanted_branches, wanted_refs, > - list_refs_only, worktree_refname, no_head, verbosity); > + list_refs_only, worktree_refname, remote_head, no_head, verbosity); > if (err != NULL) > goto done; > nfetchfd = -1; > blob - b8170cb9afee37b2625c705535e43662ca7dd06c > file + lib/got_lib_privsep.h > --- lib/got_lib_privsep.h > +++ lib/got_lib_privsep.h > @@ -409,10 +409,12 @@ struct got_imsg_fetch_request { > int list_refs_only; > int verbosity; > size_t worktree_branch_len; > + size_t remote_head_len; > size_t n_have_refs; > size_t n_wanted_branches; > size_t n_wanted_refs; > /* Followed by worktree_branch_len bytes of reference name. */ > + /* Followed by remote_head_len bytes of reference name. */ > /* Followed by n_have_refs GOT_IMSG_FETCH_HAVE_REF messages. */ > /* Followed by n_wanted_branches times GOT_IMSG_FETCH_WANTED_BRANCH. */ > /* Followed by n_wanted_refs times GOT_IMSG_FETCH_WANTED_REF. */ > @@ -701,7 +703,7 @@ const struct got_error *got_privsep_send_fetch_req(str > int *, int *, struct imsgbuf *ibuf); > const struct got_error *got_privsep_send_fetch_req(struct imsgbuf *, int, > struct got_pathlist_head *, int, struct got_pathlist_head *, > - struct got_pathlist_head *, int, const char *, int, int); > + struct got_pathlist_head *, int, const char *, const char *, int, int); > const struct got_error *got_privsep_send_fetch_outfd(struct imsgbuf *, int); > const struct got_error *got_privsep_recv_fetch_progress(int *, > struct got_object_id **, char **, struct got_pathlist_head *, char **, > blob - ed227bda3d7a4ce423d882c19ae47793a302c675 > file + lib/privsep.c > --- lib/privsep.c > +++ lib/privsep.c > @@ -532,19 +532,23 @@ got_privsep_send_fetch_req(struct imsgbuf *ibuf, int f > struct got_pathlist_head *have_refs, int fetch_all_branches, > struct got_pathlist_head *wanted_branches, > struct got_pathlist_head *wanted_refs, int list_refs_only, > - const char *worktree_branch, int no_head, int verbosity) > + const char *worktree_branch, const char *remote_head, > + int no_head, int verbosity) > { > const struct got_error *err = NULL; > struct ibuf *wbuf; > - size_t len, worktree_branch_len; > struct got_pathlist_entry *pe; > struct got_imsg_fetch_request fetchreq; > + size_t remote_head_len, worktree_branch_len, len = sizeof(fetchreq); > > if (worktree_branch) { > worktree_branch_len = strlen(worktree_branch); > - len = sizeof(fetchreq) + worktree_branch_len; > - } else > - len = sizeof(fetchreq); > + len += worktree_branch_len; > + } > + if (remote_head) { > + remote_head_len = strlen(remote_head); > + len += remote_head_len; > + } > > if (len >= MAX_IMSGSIZE - IMSG_HEADER_SIZE) { > close(fd); > @@ -562,6 +566,8 @@ got_privsep_send_fetch_req(struct imsgbuf *ibuf, int f > fetchreq.verbosity = verbosity; > if (worktree_branch != NULL) > fetchreq.worktree_branch_len = worktree_branch_len; > + if (remote_head != NULL) > + fetchreq.remote_head_len = remote_head_len; > TAILQ_FOREACH(pe, have_refs, entry) > fetchreq.n_have_refs++; > TAILQ_FOREACH(pe, wanted_branches, entry) > @@ -574,6 +580,10 @@ got_privsep_send_fetch_req(struct imsgbuf *ibuf, int f > if (imsg_add(wbuf, worktree_branch, worktree_branch_len) == -1) > return got_error_from_errno("imsg_add FETCH_REQUEST"); > } > + if (remote_head) { > + if (imsg_add(wbuf, remote_head, remote_head_len) == -1) > + return got_error_from_errno("imsg_add FETCH_REQUEST"); > + } > wbuf->fd = fd; > imsg_close(ibuf, wbuf); > > blob - 2a22df95ff37a15b31f41c7c01f263af824ddc15 > file + libexec/got-fetch-pack/got-fetch-pack.c > --- libexec/got-fetch-pack/got-fetch-pack.c > +++ libexec/got-fetch-pack/got-fetch-pack.c > @@ -333,7 +333,8 @@ fetch_pack(int fd, int packfd, uint8_t *pack_sha1, > struct got_pathlist_head *have_refs, int fetch_all_branches, > struct got_pathlist_head *wanted_branches, > struct got_pathlist_head *wanted_refs, int list_refs_only, > - const char *worktree_branch, int no_head, struct imsgbuf *ibuf) > + const char *worktree_branch, const char *remote_head, > + int no_head, struct imsgbuf *ibuf) > { > const struct got_error *err = NULL; > char buf[GOT_PKT_MAX]; > @@ -501,14 +502,27 @@ fetch_pack(int fd, int packfd, uint8_t *pack_sha1, > if (list_refs_only) > goto done; > > - if (!found_branch && !no_head && default_branch && default_id_str && > + /* > + * If -b was not used and either none of the requested branches > + * (got.conf, worktree) were found or the client already has the > + * remote HEAD symref but its target changed, fetch remote's HEAD. > + */ > + if (!no_head && default_branch && default_id_str && > strncmp(default_branch, "refs/heads/", 11) == 0) { > - err = fetch_ref(ibuf, have_refs, &have[nref], > - &want[nref], default_branch, default_id_str); > - if (err) > - goto done; > - nref++; > - found_branch = 1; > + int remote_head_changed = 0; > + > + if (remote_head) { > + if (strcmp(remote_head, default_branch + 11) != 0) > + remote_head_changed = 1; > + } > + > + if (!found_branch || remote_head_changed) { > + err = fetch_ref(ibuf, have_refs, &have[nref], > + &want[nref], default_branch, default_id_str); > + if (err) > + goto done; > + nref++; > + } > } > > /* Abort if we haven't found anything to fetch. */ > @@ -834,7 +848,7 @@ main(int argc, char **argv) > struct got_imsg_fetch_wanted_branch wbranch; > struct got_imsg_fetch_wanted_ref wref; > size_t datalen, i; > - char *worktree_branch = NULL; > + char *remote_head = NULL, *worktree_branch = NULL; > #if 0 > static int attached; > while (!attached) > @@ -875,7 +889,7 @@ main(int argc, char **argv) > fetchfd = imsg.fd; > > if (datalen != sizeof(fetch_req) + > - fetch_req.worktree_branch_len) { > + fetch_req.worktree_branch_len + fetch_req.remote_head_len) { > err = got_error(GOT_ERR_PRIVSEP_LEN); > goto done; > } > @@ -889,6 +903,15 @@ main(int argc, char **argv) > } > } > > + if (fetch_req.remote_head_len != 0) { > + remote_head = strndup(imsg.data + sizeof(fetch_req) + > + fetch_req.worktree_branch_len, fetch_req.remote_head_len); > + if (remote_head == NULL) { > + err = got_error_from_errno("strndup"); > + goto done; > + } > + } > + > imsg_free(&imsg); > > if (fetch_req.verbosity > 0) > @@ -1045,9 +1068,10 @@ main(int argc, char **argv) > err = fetch_pack(fetchfd, packfd, pack_sha1, &have_refs, > fetch_req.fetch_all_branches, &wanted_branches, > &wanted_refs, fetch_req.list_refs_only, > - worktree_branch, fetch_req.no_head, &ibuf); > + worktree_branch, remote_head, fetch_req.no_head, &ibuf); > done: > free(worktree_branch); > + free(remote_head); > got_pathlist_free(&have_refs, GOT_PATHLIST_FREE_ALL); > got_pathlist_free(&wanted_branches, GOT_PATHLIST_FREE_PATH); > if (fetchfd != -1 && close(fetchfd) == -1 && err == NULL) > > -- > Mark Jamsek > GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68 >