Re: [PATCH] branch: update all per-worktree HEADs when renaming a branch

On Tue, Mar 22, 2016 at 07:49:00AM +0700, Duy Nguyen wrote:

On Tue, Mar 22, 2016 at 12:41 AM, Eric Sunshine [off-list ref] wrote:

quoted

quoted

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.
-- 
Duy

Thanks 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...