Re: [PATCH] branch: update all per-worktree HEADs when renaming a branch
- From
Kazuki Yamaguchi <k@rhe.jp>
- Date
- Mar 25, 2016, 11:56 UTC
- Message-ID
- <20160325115614.GA21341@chikuwa.rhe.jp>
- In-Reply-To
- <CACsJy8BKBiYTsp5wv9ynr++p2TE-dzS9J+DkOPEKumCL-reLVw@mail.gmail.com>
On Tue, Mar 22, 2016 at 07:49:00AM +0700, Duy Nguyen wrote:
Show 44 quoted lines
> On Tue, Mar 22, 2016 at 12:41 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> >> diff --git a/worktree.c b/worktree.c
> >> @@ -217,3 +217,41 @@ char *find_shared_symref(const char *symref, const char *target)
> >> +int update_worktrees_head_symref(const char *oldref, const char *newref)
> >> +{
> >> + int error = 0;
> >> + struct strbuf path = STRBUF_INIT;
> >> + struct strbuf origref = STRBUF_INIT;
> >> + int i;
> >> + struct worktree **worktrees = get_worktrees();
> >> +
> >> + for (i = 0; worktrees[i]; i++) {
> >> + if (worktrees[i]->is_detached)
> >> + continue;
> >> +
> >> + strbuf_reset(&path);
> >> + strbuf_reset(&origref);
> >> + strbuf_addf(&path, "%s/HEAD", worktrees[i]->git_dir);
> >> +
> >> + if (parse_ref(path.buf, &origref, NULL))
> >> + continue;
> >> +
> >> + if (!strcmp(origref.buf, oldref)) {
> >> + int prefix_len = strlen(absolute_path(get_git_common_dir())) + 1;
> >> + const char *symref = path.buf + prefix_len;
> >> +
> >> + /* no need to pass logmsg here as HEAD didn't really move */
> >> + if (create_symref(symref, newref, NULL)) {
> >> + error = -1;
> >> + break;
> >
> > Is aborting upon the first error desired behavior? (Genuine question.)
> > Would it make more sense to continue attempting the rename for the
> > remaining worktrees (and remember that an error was encountered)?
>
> Since all these HEADs stay at the same (or close) location, if one
> fails, I think the rest will fail too. Which leads to a series of
> warnings if we continue anyway. A more interesting approach is update
> HEADs in a transaction, so we successfully update all or we update
> none. But I do not know if ref transactions can be used for HEAD,
> especially worktree HEADs. I'm ok with either abort here or continue
> anyway, though.
> --
> DuyThanks for suggestion, but it looks like ref_transaction can be used only for updating non-symbolic references. Extending it only for this purpose seems too much...