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

[PATCH] worktree: refactor lock_reason_valid and lock_reason to be more sensible

From
nbelakovski@gmail.com <nbelakovski@gmail.com>
Date
Oct 25, 2018, 05:51 UTC
Message-ID
<20181025055142.38077-1-nbelakovski@gmail.com>
In-Reply-To
<CAC05386F1X7TsPr6kgkuLWEwsmdiQ4VKTF5RxaHvzpkwbmXPBw@mail.gmail.com>
From: Nickolai Belakovski <nbelakovski@gmail.com>

lock_reason_valid is renamed to is_locked and lock_reason is removed as a field of the worktree struct. Lock reason can be obtained instead by a standalone function.

This is done in order to make the worktree struct more intuitive when it is used elsewhere in the codebase.

Some unused variables are cleaned up as well.
Signed-off-by: Nickolai Belakovski <nbelakovski@gmail.com>
---
 builtin/worktree.c | 16 ++++++++--------
 worktree.c         | 55 ++++++++++++++++++++++++++++--------------------------
 worktree.h         |  8 +++-----
 3 files changed, 40 insertions(+), 39 deletions(-)
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 41e771439..844789a21 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -634,8 +634,8 @@ 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 (wt->is_locked) {
+		old_reason = worktree_locked_reason(wt);
 		if (*old_reason)
 			die(_("'%s' is already locked, reason: %s"),
 			    av[0], old_reason);
@@ -666,7 +666,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->is_locked)
 		die(_("'%s' is not locked"), av[0]);
 	ret = unlink_or_warn(git_common_path("worktrees/%s/locked", wt->id));
 	free_worktrees(worktrees);
@@ -734,8 +734,8 @@ static int move_worktree(int ac, const char **av, const char *prefix)
 
 	validate_no_submodules(wt);
 
-	reason = is_worktree_locked(wt);
-	if (reason) {
+	if (wt->is_locked) {
+		reason = worktree_locked_reason(wt);
 		if (*reason)
 			die(_("cannot move a locked working tree, lock reason: %s"),
 			    reason);
@@ -860,11 +860,11 @@ 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 (wt->is_locked) {
+		reason = worktree_locked_reason(wt);
 		if (*reason)
 			die(_("cannot remove a locked working tree, lock reason: %s"),
-			    reason);
+				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..a3082d19d 100644
--- a/worktree.c
+++ b/worktree.c
@@ -14,7 +14,6 @@ void free_worktrees(struct worktree **worktrees)
 		free(worktrees[i]->path);
 		free(worktrees[i]->id);
 		free(worktrees[i]->head_ref);
-		free(worktrees[i]->lock_reason);
 		free(worktrees[i]);
 	}
 	free (worktrees);
@@ -41,13 +40,29 @@ static void add_head_info(struct worktree *wt)
 		wt->is_detached = 1;
 }
 
+
+/**
+ * Return 1 if the worktree is locked, 0 otherwise
+ */
+static int is_worktree_locked(const struct worktree *wt)
+{
+	struct strbuf path = STRBUF_INIT;
+	int locked_file_exists;
+
+	assert(!is_main_worktree(wt));
+
+	strbuf_addstr(&path, worktree_git_path(wt, "locked"));
+	locked_file_exists = file_exists(path.buf);
+	strbuf_release(&path);
+	return locked_file_exists;
+}
+
 /**
  * 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 +71,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 +101,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->is_locked = is_worktree_locked(worktree);
 	add_head_info(worktree);
 
 done:
@@ -231,27 +241,20 @@ int is_main_worktree(const struct worktree *wt)
 	return !wt->id;
 }
 
-const char *is_worktree_locked(struct worktree *wt)
+const char *worktree_locked_reason(const struct worktree *wt)
 {
-	assert(!is_main_worktree(wt));
+	struct strbuf path = STRBUF_INIT;
+	struct strbuf lock_reason = STRBUF_INIT;
 
-	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);
-	}
+	assert(!is_main_worktree(wt));
+	assert(wt->is_locked);
 
-	return wt->lock_reason;
+	strbuf_addstr(&path, worktree_git_path(wt, "locked"));
+	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);
 }
 
 /* convenient wrapper to deal with NULL strbuf */
diff --git a/worktree.h b/worktree.h
index df3fc30f7..6717287e8 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 */
 	struct object_id head_oid;
 	int is_detached;
 	int is_bare;
 	int is_current;
-	int lock_reason_valid;
+	int is_locked;
 };
 
 /* Functions for acting on the information about worktrees. */
@@ -57,10 +56,9 @@ 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.
+ * Return the reason string if the given worktree is locked or die
  */
-extern const char *is_worktree_locked(struct worktree *wt);
+extern const char *worktree_locked_reason(const struct worktree *wt);
 
 #define WT_VALIDATE_WORKTREE_MISSING_OK (1 << 0)
 

-- 
2.14.2
Previous: Nickolai BelakovskiNext: Junio C Hamano
Message 4 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.