From: Junio C Hamano <gitster@pobox•com>
To: Caleb White <cdwhite3@pm•me>
Cc: git@vger•kernel.org, Taylor Blau <me@ttaylorr•com>,
Phillip Wood <phillip.wood123@gmail•com>,
Eric Sunshine <sunshine@sunshineco•com>
Subject: Re: [PATCH v5 0/8] Allow relative worktree linking to be configured by the user
Date: Tue, 26 Nov 2024 15:18:30 +0900 [thread overview]
Message-ID: <xmqqv7wazfex.fsf@gitster.g> (raw)
In-Reply-To: <20241125-wt_relative_options-v5-0-356d122ff3db@pm.me> (Caleb White's message of "Tue, 26 Nov 2024 01:51:31 +0000")
Caleb White <cdwhite3@pm•me> writes:
> Changes in v5:
> - Added docs to `--relative-paths` option.
You already had doc on this, but the default was not described at
all.
--[no-]relative-paths::
+ Link worktrees using relative paths or absolute paths (default).
> - Added test coverage for `repair_worktrees()` and relative paths.
> - Move `strbuf_reset` call in `infer_backlink()`.
This was more like "revert the change in v4 that moved it
unnecessarily", no?
> - Cleaned up tests.
Yup, there truely a lot of test changes between v4 and v5. Many
tests now use existing test helpers, which is good.
> - Slight stylistic changes.
I saw many changes like these (the diff is between v4 and v5)
static void repair_gitfile(struct worktree *wt,
- worktree_repair_fn fn,
- void *cb_data,
+ worktree_repair_fn fn, void *cb_data,
int use_relative_paths)
which looked good (the original had fn and cb_data defined on the
same line).
> - Tweaked commit messages.
Updates to the proposed log message for `repair` step [7/8] did not
really "clarify", other than helping readers to see how messy things
are. It said:
+ To simplify things, both linking files are written when one of the files
+ needs to be repaired. In some cases, this fixes the other file before it
+ is checked, in other cases this results in a correct file being written
+ with the same contents.
which may describe what the code happens to do correctly, but does
not quite help building the confidence in what it does is correct.
Suppose that the directory X has a repository, and the repository
thinks that the directory W is its worktree. But the worktree at
the directory W thinks that its repository is not X but Y, and there
indeed is a repository at the directory Y. That repository thinks W
belongs to it.
If we examine X first, would we end up updating W to point at X
(because X thinks W is its worktree)?
Or do we make W to point at Y (because Y thinks W is its, and W
thinks it is Y's)"?
Either way, I think the comment is trying to say that, if we decide
to make X and W belong to each other, we'd overwrite links from X to
W and also W to X, even though the link from X was already pointing
at W and the minimum fix we needed to make was to update the link
from W to point at X. Overwriting a link from X to W with a new
link from X to W is a no-op, so it does not seem to help greatly,
since `repair` is not at all performance critical. The correctness
is a lot more important.
> - Updated base to 090d24e9af.
This made it harder than necessary to compare the two iterations, by
the way.
Thanks.
next prev parent reply other threads:[~2024-11-26 6:18 UTC|newest]
Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-01 4:38 [PATCH v4 0/8] Allow relative worktree linking to be configured by the user Caleb White
2024-11-01 4:38 ` [PATCH v4 1/8] setup: correctly reinitialize repository version Caleb White
2024-11-01 4:38 ` [PATCH v4 2/8] worktree: add `relativeWorktrees` extension Caleb White
2024-11-19 15:07 ` Phillip Wood
2024-11-20 5:13 ` Caleb White
2024-11-22 16:44 ` Phillip Wood
2024-11-22 19:27 ` Caleb White
2024-11-01 4:38 ` [PATCH v4 3/8] worktree: refactor infer_backlink return Caleb White
2024-11-19 15:08 ` Phillip Wood
2024-11-20 5:20 ` Caleb White
2024-11-22 16:44 ` Phillip Wood
2024-11-22 19:26 ` Caleb White
2024-11-01 4:38 ` [PATCH v4 4/8] worktree: add `write_worktree_linking_files()` function Caleb White
2024-11-01 4:38 ` [PATCH v4 5/8] worktree: add relative cli/config options to `add` command Caleb White
2024-11-19 15:07 ` Phillip Wood
2024-11-20 5:01 ` Caleb White
2024-11-22 16:44 ` phillip.wood123
2024-11-23 4:40 ` Caleb White
2024-11-01 4:38 ` [PATCH v4 6/8] worktree: add relative cli/config options to `move` command Caleb White
2024-11-22 15:55 ` Phillip Wood
2024-11-23 4:11 ` Caleb White
2024-11-01 4:38 ` [PATCH v4 7/8] worktree: add relative cli/config options to `repair` command Caleb White
2024-11-22 15:55 ` Phillip Wood
2024-11-23 5:41 ` Caleb White
2024-11-24 19:27 ` phillip.wood123
2024-11-26 0:00 ` Caleb White
2024-11-01 4:39 ` [PATCH v4 8/8] worktree: refactor `repair_worktree_after_gitdir_move()` Caleb White
2024-11-22 15:58 ` Phillip Wood
2024-11-23 4:36 ` Caleb White
2024-11-01 7:14 ` [PATCH v4 0/8] Allow relative worktree linking to be configured by the user Junio C Hamano
2024-11-01 13:18 ` Caleb White
2024-11-02 10:09 ` Junio C Hamano
2024-11-02 14:36 ` Kristoffer Haugsbakk
2024-11-22 15:57 ` Phillip Wood
2024-11-23 5:45 ` Caleb White
2024-11-26 1:51 ` [PATCH v5 " Caleb White
2024-11-26 1:51 ` [PATCH v5 1/8] setup: correctly reinitialize repository version Caleb White
2024-11-26 1:51 ` [PATCH v5 2/8] worktree: add `relativeWorktrees` extension Caleb White
2024-11-26 1:51 ` [PATCH v5 3/8] worktree: refactor infer_backlink return Caleb White
2024-11-26 1:51 ` [PATCH v5 4/8] worktree: add `write_worktree_linking_files()` function Caleb White
2024-11-26 1:52 ` [PATCH v5 5/8] worktree: add relative cli/config options to `add` command Caleb White
2024-11-26 1:52 ` [PATCH v5 6/8] worktree: add relative cli/config options to `move` command Caleb White
2024-11-26 1:52 ` [PATCH v5 7/8] worktree: add relative cli/config options to `repair` command Caleb White
2024-11-26 1:52 ` [PATCH v5 8/8] worktree: refactor `repair_worktree_after_gitdir_move()` Caleb White
2024-11-26 6:18 ` Junio C Hamano [this message]
2024-11-26 17:02 ` [PATCH v5 0/8] Allow relative worktree linking to be configured by the user Caleb White
2024-11-28 14:44 ` Phillip Wood
2024-11-28 17:58 ` Caleb White
2024-11-29 22:22 ` [PATCH v6 " Caleb White
2024-11-29 22:22 ` [PATCH v6 1/8] setup: correctly reinitialize repository version Caleb White
2024-11-29 22:22 ` [PATCH v6 2/8] worktree: add `relativeWorktrees` extension Caleb White
2024-11-29 22:22 ` [PATCH v6 3/8] worktree: refactor infer_backlink return Caleb White
2024-11-29 22:22 ` [PATCH v6 4/8] worktree: add `write_worktree_linking_files()` function Caleb White
2024-11-29 22:22 ` [PATCH v6 5/8] worktree: add relative cli/config options to `add` command Caleb White
2024-11-29 22:23 ` [PATCH v6 6/8] worktree: add relative cli/config options to `move` command Caleb White
2024-11-29 22:23 ` [PATCH v6 7/8] worktree: add relative cli/config options to `repair` command Caleb White
2024-11-29 22:23 ` [PATCH v6 8/8] worktree: refactor `repair_worktree_after_gitdir_move()` Caleb White
2024-12-02 14:57 ` [PATCH v6 0/8] Allow relative worktree linking to be configured by the user Phillip Wood
2024-12-03 4:54 ` Junio C Hamano
2024-12-03 5:21 ` Caleb White
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=xmqqv7wazfex.fsf@gitster.g \
--to=gitster@pobox$(echo .)com \
--cc=cdwhite3@pm$(echo .)me \
--cc=git@vger$(echo .)kernel.org \
--cc=me@ttaylorr$(echo .)com \
--cc=phillip.wood123@gmail$(echo .)com \
--cc=sunshine@sunshineco$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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