git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] worktree: populate lock_reason in get_worktrees and light refactor/cleanup in worktree files

From
nbelakovski@gmail.com <nbelakovski@gmail.com>
Date
Oct 24, 2018, 06:39 UTC
Message-ID
<20181024063904.36096-1-nbelakovski@gmail.com>
From: Nickolai Belakovski <nbelakovski@gmail.com>
lock_reason is now populated during the execution of get_worktrees

is_worktree_locked has been simplified, renamed, and changed to internal linkage. It is simplified to only return the lock reason (or NULL in case there is no lock reason) and to not have any side effects on the inputs. As such it made sense to rename it since it only returns the reason.

Since this function was now being used to populate the worktree struct's lock_reason field, it made sense to move the function to internal linkage and have callers refer to the lock_reason field. The lock_reason_valid field was removed since a NULL/non-NULL value of lock_reason accomplishes the same effect.

Some unused variables within worktree source code were removed.
Signed-off-by: Nickolai Belakovski <nbelakovski@gmail.com>
---
Notes:
    Travis CI results: https://travis-ci.org/nbelakovski/git/builds/445500127
 builtin/worktree.c | 27 +++++++++++----------------
 worktree.c         | 53 +++++++++++++++++++++++------------------------------
 worktree.h         |  9 +--------
 3 files changed, 35 insertions(+), 54 deletions(-)
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 41e771439..fb203f61d 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -615,7 +615,7 @@ static int list(int ac, const char **av, const char *prefix)
 
 static int lock_worktree(int ac, const char **av, const char *prefix)
 {
-	const char *reason = "", *old_reason;
+	const char *reason = "";
 	struct option options[] = {
 		OPT_STRING(0, "reason", &reason, N_("string"),
 			   N_("reason for locking")),
@@ -634,11 +634,10 @@ static int lock_worktree(int ac, const char **av, const char *prefix)
 	if (is_main_worktree(wt))
 		die(_("The main working tree cannot be locked or unlocked"));
 
-	old_reason = is_worktree_locked(wt);
-	if (old_reason) {
-		if (*old_reason)
+	if (wt->lock_reason) {
+		if (*wt->lock_reason)
 			die(_("'%s' is already locked, reason: %s"),
-			    av[0], old_reason);
+			    av[0], wt->lock_reason);
 		die(_("'%s' is already locked"), av[0]);
 	}
 
@@ -666,7 +665,7 @@ static int unlock_worktree(int ac, const char **av, const char *prefix)
 		die(_("'%s' is not a working tree"), av[0]);
 	if (is_main_worktree(wt))
 		die(_("The main working tree cannot be locked or unlocked"));
-	if (!is_worktree_locked(wt))
+	if (!wt->lock_reason)
 		die(_("'%s' is not locked"), av[0]);
 	ret = unlink_or_warn(git_common_path("worktrees/%s/locked", wt->id));
 	free_worktrees(worktrees);
@@ -703,7 +702,6 @@ static int move_worktree(int ac, const char **av, const char *prefix)
 	struct worktree **worktrees, *wt;
 	struct strbuf dst = STRBUF_INIT;
 	struct strbuf errmsg = STRBUF_INIT;
-	const char *reason;
 	char *path;
 
 	ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
@@ -734,11 +732,10 @@ static int move_worktree(int ac, const char **av, const char *prefix)
 
 	validate_no_submodules(wt);
 
-	reason = is_worktree_locked(wt);
-	if (reason) {
-		if (*reason)
+	if (wt->lock_reason) {
+		if (*wt->lock_reason)
 			die(_("cannot move a locked working tree, lock reason: %s"),
-			    reason);
+			    wt->lock_reason);
 		die(_("cannot move a locked working tree"));
 	}
 	if (validate_worktree(wt, &errmsg, 0))
@@ -847,7 +844,6 @@ static int remove_worktree(int ac, const char **av, const char *prefix)
 	};
 	struct worktree **worktrees, *wt;
 	struct strbuf errmsg = STRBUF_INIT;
-	const char *reason;
 	int ret = 0;
 
 	ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
@@ -860,11 +856,10 @@ static int remove_worktree(int ac, const char **av, const char *prefix)
 		die(_("'%s' is not a working tree"), av[0]);
 	if (is_main_worktree(wt))
 		die(_("'%s' is a main working tree"), av[0]);
-	reason = is_worktree_locked(wt);
-	if (reason) {
-		if (*reason)
+	if (wt->lock_reason) {
+		if (*wt->lock_reason)
 			die(_("cannot remove a locked working tree, lock reason: %s"),
-			    reason);
+			    wt->lock_reason);
 		die(_("cannot remove a locked working tree"));
 	}
 	if (validate_worktree(wt, &errmsg, WT_VALIDATE_WORKTREE_MISSING_OK))
diff --git a/worktree.c b/worktree.c
index 97cda5f97..3bd25983c 100644
--- a/worktree.c
+++ b/worktree.c
@@ -41,13 +41,34 @@ static void add_head_info(struct worktree *wt)
 		wt->is_detached = 1;
 }
 
+/**
+ * Return the reason the worktree is locked, or NULL if it is not locked
+ */
+static char *worktree_locked_reason(const struct worktree *wt)
+{
+	struct strbuf lock_reason = STRBUF_INIT;
+	struct strbuf path = STRBUF_INIT;
+
+	assert(!is_main_worktree(wt));
+
+	strbuf_addstr(&path, worktree_git_path(wt, "locked"));
+	if (file_exists(path.buf)) {
+		if (strbuf_read_file(&lock_reason, path.buf, 0) < 0)
+			die_errno(_("failed to read '%s'"), path.buf);
+		strbuf_trim(&lock_reason);
+		strbuf_release(&path);
+		return strbuf_detach(&lock_reason, NULL);
+	}
+	strbuf_release(&path);
+	return NULL;
+}
+
 /**
  * get the main worktree
  */
 static struct worktree *get_main_worktree(void)
 {
 	struct worktree *worktree = NULL;
-	struct strbuf path = STRBUF_INIT;
 	struct strbuf worktree_path = STRBUF_INIT;
 	int is_bare = 0;
 
@@ -56,14 +77,11 @@ static struct worktree *get_main_worktree(void)
 	if (is_bare)
 		strbuf_strip_suffix(&worktree_path, "/.");
 
-	strbuf_addf(&path, "%s/HEAD", get_git_common_dir());
-
 	worktree = xcalloc(1, sizeof(*worktree));
 	worktree->path = strbuf_detach(&worktree_path, NULL);
 	worktree->is_bare = is_bare;
 	add_head_info(worktree);
 
-	strbuf_release(&path);
 	strbuf_release(&worktree_path);
 	return worktree;
 }
@@ -89,12 +107,10 @@ static struct worktree *get_linked_worktree(const char *id)
 		strbuf_strip_suffix(&worktree_path, "/.");
 	}
 
-	strbuf_reset(&path);
-	strbuf_addf(&path, "%s/worktrees/%s/HEAD", get_git_common_dir(), id);
-
 	worktree = xcalloc(1, sizeof(*worktree));
 	worktree->path = strbuf_detach(&worktree_path, NULL);
 	worktree->id = xstrdup(id);
+	worktree->lock_reason = worktree_locked_reason(worktree);
 	add_head_info(worktree);
 
 done:
@@ -231,29 +247,6 @@ int is_main_worktree(const struct worktree *wt)
 	return !wt->id;
 }
 
-const char *is_worktree_locked(struct worktree *wt)
-{
-	assert(!is_main_worktree(wt));
-
-	if (!wt->lock_reason_valid) {
-		struct strbuf path = STRBUF_INIT;
-
-		strbuf_addstr(&path, worktree_git_path(wt, "locked"));
-		if (file_exists(path.buf)) {
-			struct strbuf lock_reason = STRBUF_INIT;
-			if (strbuf_read_file(&lock_reason, path.buf, 0) < 0)
-				die_errno(_("failed to read '%s'"), path.buf);
-			strbuf_trim(&lock_reason);
-			wt->lock_reason = strbuf_detach(&lock_reason, NULL);
-		} else
-			wt->lock_reason = NULL;
-		wt->lock_reason_valid = 1;
-		strbuf_release(&path);
-	}
-
-	return wt->lock_reason;
-}
-
 /* convenient wrapper to deal with NULL strbuf */
 static void strbuf_addf_gently(struct strbuf *buf, const char *fmt, ...)
 {
diff --git a/worktree.h b/worktree.h
index df3fc30f7..0214630fd 100644
--- a/worktree.h
+++ b/worktree.h
@@ -10,12 +10,11 @@ struct worktree {
 	char *path;
 	char *id;
 	char *head_ref;		/* NULL if HEAD is broken or detached */
-	char *lock_reason;	/* internal use */
+	char *lock_reason;
 	struct object_id head_oid;
 	int is_detached;
 	int is_bare;
 	int is_current;
-	int lock_reason_valid;
 };
 
 /* Functions for acting on the information about worktrees. */
@@ -56,12 +55,6 @@ extern struct worktree *find_worktree(struct worktree **list,
  */
 extern int is_main_worktree(const struct worktree *wt);
 
-/*
- * Return the reason string if the given worktree is locked or NULL
- * otherwise.
- */
-extern const char *is_worktree_locked(struct worktree *wt);
-
 #define WT_VALIDATE_WORKTREE_MISSING_OK (1 << 0)
 
 /*

-- 
2.14.2
Next: Eric Sunshine
Message 1 of 19 in “worktree: populate lock_reason in get_worktrees and light refactor/cleanup in worktree files”
  1. worktree: populate lock_reason in get_worktrees and light refactor/cleanup in worktree filesnbelakovski@gmail.com, Oct 24, 2018
  2. Eric SunshineOct 24, 2018
  3. Nickolai BelakovskiOct 25, 2018
  4. worktree: refactor lock_reason_valid and lock_reason to be more sensiblenbelakovski@gmail.com, Oct 25, 2018
  5. Junio C HamanoOct 25, 2018
  6. Nickolai BelakovskiOct 28, 2018
  7. Junio C HamanoOct 29, 2018
  8. Nickolai BelakovskiOct 29, 2018
  9. Junio C HamanoOct 29, 2018
  10. 1/2 worktree: update documentation for lock_reason and lock_reason_validnbelakovski@gmail.com, Oct 30, 2018
  11. 2/2 worktree: rename is_worktree_locked to worktree_lock_reasonnbelakovski@gmail.com, Oct 30, 2018
  12. Junio C HamanoOct 31, 2018
  13. Junio C HamanoOct 31, 2018
  14. Eric SunshineOct 25, 2018
  15. Eric SunshineOct 28, 2018
  16. Nickolai BelakovskiOct 29, 2018
  17. Eric SunshineOct 29, 2018
  18. Nickolai BelakovskiOct 29, 2018
  19. Eric SunshineOct 29, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.