git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Olga Pilipenco via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: "Patrick Steinhardt" <ps@pks.im>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"René Scharfe" <l.s.r@web.de>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Olga Pilipenco" <olga.pilipenco@shopify.com>,
	"Olga Pilipenco" <olga.pilipenco@shopify.com>
Subject: [PATCH v2] worktree: detect from secondary worktree if main worktree is bare
Date: Thu, 16 Jan 2025 21:35:35 +0000	[thread overview]
Message-ID: <pull.1829.v2.git.1737063335673.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1829.git.1731653548549.gitgitgadget@gmail.com>

From: Olga Pilipenco <olga.pilipenco@shopify.com>

Setup:
1. Have a bare repo with core.bare = true in config.worktree
2. Create a new worktree

Behavior:
From the secondary worktree the main worktree appears as non-bare.

Expected:
From the secondary worktree the main worktree should appear as bare.

Why current behavior is not good?
If the main worktree is detected as not bare it doesn't allow
checking out the branch of the main worktree. There are possibly
other problems associated with that behavior.

Why is it happening?
While we're inside the secondary worktree we don't initialize the main
worktree's repository with its configuration.

How is it fixed?
Load actual configs of the main worktree. Also, skip the config loading
step if we're already inside the current worktree because in that case we
rely on is_bare_repository() to return the correct result.

Other solutions considered:
Alternatively, instead of incorrectly always using
`the_repository` as the main worktree's repository, we can detect
and load the actual repository of the main worktree and then use
that repository's `is_bare` value extracted from correct configs.
However, this approach is a bit riskier and could also affect
performance. Since we had the assignment `worktree->repo =
the_repository` for a long time already, I decided it's safe to
keep it as it is for now; it can be still fixed separately from
this change.

Real life use case:
1. Have a bare repo
2. Create a worktree from the bare repo
3. In the secondary worktree enable sparse-checkout - this enables
extensions.worktreeConfig and keeps core.bare=true setting in
config.worktree of the bare worktree
4. The secondary worktree or any other non-bare worktree created
won't be able to use branch main (not even once), but it should be
able to.

Signed-off-by: Olga Pilipenco <olga.pilipenco@shopify.com>
---
    worktree: detect from secondary worktree if main worktree is bare
    
    Changes since v1:
    
     * no code changes
     * rebased with maint
     * CC added
    
    Existing broken functionality forces our project to use hacks on bare
    repo that we'd like to avoid. I would really appreciate reviews of this
    patch to move closer towards fixing the issue. This is my first
    contribution to git/git, I apologize if I got lost in the instructions,
    but I tried my best to follow the rules.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1829%2Folga-mcbfe%2Ffix-bare-repo-detection-with-worktree-config-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1829/olga-mcbfe/fix-bare-repo-detection-with-worktree-config-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1829

Range-diff vs v1:

 1:  9e7170f07fc = 1:  17f4b24d1da worktree: detect from secondary worktree if main worktree is bare


 t/t3200-branch.sh | 14 ++++++++++++++
 worktree.c        | 38 +++++++++++++++++++++++++++++---------
 2 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index a3a21c54cf6..7ca50c9a78d 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -410,6 +410,20 @@ test_expect_success 'bare main worktree has HEAD at branch deleted by secondary
 	git -C secondary branch -D main
 '
 
+test_expect_success 'secondary worktree can switch to main if common dir is bare worktree' '
+	test_when_finished "rm -rf bare_repo non_bare_repo secondary_worktree" &&
+	git init -b main non_bare_repo &&
+	test_commit -C non_bare_repo x &&
+
+	git clone --bare non_bare_repo bare_repo &&
+	git -C bare_repo config extensions.worktreeConfig true &&
+	git -C bare_repo config unset core.bare &&
+	git -C bare_repo config --worktree core.bare true &&
+
+	git -C bare_repo worktree add ../secondary_worktree &&
+	git -C secondary_worktree checkout main
+'
+
 test_expect_success 'git branch --list -v with --abbrev' '
 	test_when_finished "git branch -D t" &&
 	git branch t &&
diff --git a/worktree.c b/worktree.c
index 248bbb39d43..a7726c67747 100644
--- a/worktree.c
+++ b/worktree.c
@@ -65,6 +65,28 @@ static int is_current_worktree(struct worktree *wt)
 	return is_current;
 }
 
+static int is_bare_git_dir(const char *git_dir)
+{
+	int bare = 0;
+	struct config_set cs = { { 0 } };
+	char *config_file;
+	char *worktree_config_file;
+
+	config_file = xstrfmt("%s/config", git_dir);
+	worktree_config_file = xstrfmt("%s/config.worktree",  git_dir);
+
+	git_configset_init(&cs);
+	git_configset_add_file(&cs, config_file);
+	git_configset_add_file(&cs, worktree_config_file);
+
+	git_configset_get_bool(&cs, "core.bare", &bare);
+
+	git_configset_clear(&cs);
+	free(config_file);
+	free(worktree_config_file);
+	return bare;
+}
+
 /**
  * get the main worktree
  */
@@ -77,18 +99,16 @@ static struct worktree *get_main_worktree(int skip_reading_head)
 	strbuf_strip_suffix(&worktree_path, "/.git");
 
 	CALLOC_ARRAY(worktree, 1);
+	/*
+	 * NEEDSWORK: the_repository is not always main worktree's repository
+	*/
 	worktree->repo = the_repository;
 	worktree->path = strbuf_detach(&worktree_path, NULL);
-	/*
-	 * NEEDSWORK: If this function is called from a secondary worktree and
-	 * config.worktree is present, is_bare_repository_cfg will reflect the
-	 * contents of config.worktree, not the contents of the main worktree.
-	 * This means that worktree->is_bare may be set to 0 even if the main
-	 * worktree is configured to be bare.
-	 */
-	worktree->is_bare = (is_bare_repository_cfg == 1) ||
-		is_bare_repository();
 	worktree->is_current = is_current_worktree(worktree);
+	worktree->is_bare = (is_bare_repository_cfg == 1) ||
+		is_bare_repository() ||
+		(!worktree->is_current && is_bare_git_dir(repo_get_common_dir(the_repository)));
+
 	if (!skip_reading_head)
 		add_head_info(worktree);
 	return worktree;

base-commit: f93ff170b93a1782659637824b25923245ac9dd1
-- 
gitgitgadget

  reply	other threads:[~2025-01-16 21:35 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-15  6:52 [PATCH] worktree: detect from secondary worktree if main worktree is bare Olga Pilipenco via GitGitGadget
2025-01-16 21:35 ` Olga Pilipenco via GitGitGadget [this message]
2025-01-19 22:30   ` [PATCH v2] " Eric Sunshine
2025-01-28 21:44     ` Olga Pilipenco
2025-01-29 13:41       ` Eric Sunshine
2025-01-29 17:08         ` Junio C Hamano
     [not found]         ` <F15C12AB-2238-4553-AFA5-18277B18CE5A@shopify.com>
2025-01-30 14:32           ` Eric Sunshine
2025-01-30 14:44             ` Eric Sunshine
2025-01-31  7:05               ` Olga Pilipenco
2025-01-31 13:28                 ` Eric Sunshine
2025-01-31 18:08   ` [PATCH v3] " Olga Pilipenco via GitGitGadget
2025-01-31 19:19     ` Junio C Hamano
2025-01-31 19:26       ` Junio C Hamano
2025-01-31 20:11         ` Olga Pilipenco
2025-01-31 20:20           ` Junio C Hamano
2025-02-04 19:03             ` Olga Pilipenco
2025-02-04 19:43               ` Junio C Hamano
2025-02-04 20:33                 ` Olga Pilipenco
2025-02-05  6:30     ` [PATCH v4] " Olga Pilipenco via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.1829.v2.git.1737063335673.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=l.s.r@web.de \
    --cc=olga.pilipenco@shopify.com \
    --cc=ps@pks.im \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).