On Fri, Feb 09, 2018 at 03:13:30PM -0800, Elijah Newren wrote:
> This takes advantage of the fact that "worktrees/$WORKTREE/HEAD" will
> currently resolve as a ref, which may not be true in the future with
> different ref backends. I don't think it locks us in to supporting
> resolving other worktree HEADs with this syntax, as I view the
> user-visible error message as more of a pointer to a pathname that the
> user will need to fix. If the underlying ref storage changes, naturally
> both this code and the hint may need to change to match.
I'm leaning more about something like this patch below (which does not
even build, just to demonstrate). A couple points:
- Instead of doing the hacky refs worktrees/foo/HEAD, we get the
correct ref store for each worktree
- We have an API for getting all (non-broken) worktrees. I realize for
fsck, we may even want to examine semi-broken worktrees as well, but
get_worktrees() can take a flag to accomplish that if needed.
- As you can see, I print %p from the ref store instead of something
human friendly. This is something I was stuck at last time. I need
better ref store description (or even the worktree name)
- This ref_name() function makes me think if we should have an
extended sha1 syntax for accessing per-worktree refs from a
different worktree, something like HEAD@{worktree:foo} to resolve to
worktrees/foo/HEAD. Then we have a better description here that can
actually be used from command line, as a regular ref, if needed.
-- 8< --
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 4132034170..73cfcbc07a 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -16,6 +16,7 @@
#include "streaming.h"
#include "decorate.h"
#include "packfile.h"
+#include "worktree.h"
#define REACHABLE 0x0001
#define SEEN 0x0002
@@ -451,17 +452,39 @@ static int fsck_handle_ref(const char *refname, const struct object_id *oid,
return 0;
}
-static int fsck_head_link(const char **head_points_at,
+static const char *ref_name(struct ref_store *refs, const char *name)
+{
+ static struct strbuf sb = STRBUF_INIT;
+
+ if (!refs)
+ return name;
+ strbuf_reset(&sb);
+ strbuf_addf(&sb, "%s (from %p)", name, refs);
+ return sb.buf;
+}
+
+static int fsck_head_link(struct ref_store *refs,
+ const char **head_points_at,
struct object_id *head_oid);
static void get_default_heads(void)
{
const char *head_points_at;
struct object_id head_oid;
+ struct worktree **worktrees, **wt;
- fsck_head_link(&head_points_at, &head_oid);
+ fsck_head_link(NULL, &head_points_at, &head_oid);
if (head_points_at && !is_null_oid(&head_oid))
fsck_handle_ref("HEAD", &head_oid, 0, NULL);
+
+ worktrees = get_worktrees(0);
+ for (wt = worktrees; *wt; wt++) {
+ fsck_head_link(get_worktree_ref_store(*wt), &head_points_at, &head_oid);
+ if (head_points_at && !is_null_oid(&head_oid))
+ fsck_handle_ref(*wt, "HEAD", &head_oid, 0, NULL);
+ }
+ free_worktrees(wt);
+
for_each_rawref(fsck_handle_ref, NULL);
if (include_reflogs)
for_each_reflog(fsck_handle_reflog, NULL);
@@ -553,34 +576,36 @@ static void fsck_object_dir(const char *path)
stop_progress(&progress);
}
-static int fsck_head_link(const char **head_points_at,
+static int fsck_head_link(struct ref_store *refs,
+ const char **head_points_at,
struct object_id *head_oid)
{
int null_is_error = 0;
if (verbose)
- fprintf(stderr, "Checking HEAD link\n");
+ fprintf(stderr, "Checking %s link\n", ref_name(refs, "HEAD"));
- *head_points_at = resolve_ref_unsafe("HEAD", 0, head_oid, NULL);
+ *head_points_at = refs_resolve_ref_unsafe(refs, "HEAD", 0, head_oid, NULL);
if (!*head_points_at) {
errors_found |= ERROR_REFS;
- return error("Invalid HEAD");
+ return error("Invalid HEAD from ref-store %p", refs);
}
if (!strcmp(*head_points_at, "HEAD"))
/* detached HEAD */
null_is_error = 1;
else if (!starts_with(*head_points_at, "refs/heads/")) {
errors_found |= ERROR_REFS;
- return error("HEAD points to something strange (%s)",
- *head_points_at);
+ return error("%s points to something strange (%s)",
+ ref_name(refs, "HEAD"), *head_points_at);
}
if (is_null_oid(head_oid)) {
if (null_is_error) {
errors_found |= ERROR_REFS;
- return error("HEAD: detached HEAD points at nothing");
+ return error("%s: detached HEAD points at nothing",
+ ref_name(refs, "HEAD"));
}
- fprintf(stderr, "notice: HEAD points to an unborn branch (%s)\n",
- *head_points_at + 11);
+ fprintf(stderr, "notice: %s points to an unborn branch (%s)\n",
+ ref_name(refs, "HEAD"), *head_points_at + 11);
}
return 0;
}
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index fa94c59458..3da651be4c 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -102,7 +102,7 @@ test_expect_success 'HEAD link pointing at a funny place' '
grep "HEAD points to something strange" out
'
-test_expect_failure 'other worktree HEAD link pointing at a funny object' '
+test_expect_success 'other worktree HEAD link pointing at a funny object' '
test_when_finished "rm -rf .git/worktrees" &&
mkdir -p .git/worktrees/other &&
echo 0000000000000000000000000000000000000000 >.git/worktrees/other/HEAD &&
@@ -111,7 +111,7 @@ test_expect_failure 'other worktree HEAD link pointing at a funny object' '
grep "worktrees/other/HEAD: detached HEAD points" out
'
-test_expect_failure 'other worktree HEAD link pointing at missing object' '
+test_expect_success 'other worktree HEAD link pointing at missing object' '
test_when_finished "rm -rf .git/worktrees" &&
mkdir -p .git/worktrees/other &&
echo "Contents missing from repo" | git hash-object --stdin >.git/worktrees/other/HEAD &&
@@ -120,7 +120,7 @@ test_expect_failure 'other worktree HEAD link pointing at missing object' '
grep "worktrees/other/HEAD: invalid sha1 pointer" out
'
-test_expect_failure 'other worktree HEAD link pointing at a funny place' '
+test_expect_success 'other worktree HEAD link pointing at a funny place' '
test_when_finished "rm -rf .git/worktrees" &&
mkdir -p .git/worktrees/other &&
echo "ref: refs/funny/place" >.git/worktrees/other/HEAD &&
-- 8< --
