[KT] Feature: Disable worktrees - #84
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an opt-in/opt-out mechanism for using git worktrees during kt checkout, primarily to support environments (e.g., CentOS 7 VMs) where git is too old for worktrees, and expands test coverage around clone/worktree behavior.
Changes:
- Introduces global (
use_worktrees) + per-kernel (use_worktree) + CLI (--worktree/--no-worktree) precedence for selecting worktree vs full clone. - Extends
RepoWorktreeto support clone mode in addition to worktree mode, including mode detection and mismatch handling. - Adds substantial new tests for workspace behavior and configuration parsing; updates docs/help text.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/kt/ktlib/test_kernels.py | Adds tests for per-kernel use_worktree parsing and precedence logic. |
| tests/kt/ktlib/test_kernel_workspace.py | New test suite covering clone/worktree setup, updates, cleanup, and mismatch scenarios. |
| tests/kt/ktlib/test_config.py | Adds tests for new global use_worktrees config parsing/defaulting. |
| kt/ktlib/kernels.py | Adds use_worktree and should_use_worktree() decision logic with CentOS 7 warning. |
| kt/ktlib/kernel_workspace.py | Adds clone mode, mode detection, mismatch behavior, and propagates use_worktree into workspace repos. |
| kt/ktlib/config.py | Adds use_worktrees config field + validation. |
| kt/KT.md | Documents how to disable/override worktrees via config, overrides, and CLI. |
| kt/data/kernels.yaml | Sets CentOS 7 kernel entry to default use_worktree: false. |
| kt/commands/checkout/impl.py | Resolves worktree mode via precedence and passes it into workspace load. |
| kt/commands/checkout/command.py | Adds --worktree/--no-worktree CLI option and help text. |
Suppressed comments (2)
kt/ktlib/kernel_workspace.py:72
- The RuntimeError raised on mode mismatch uses a placeholder message ("ERROR ... Re-Run kt checkout --override ...") which is hard to act on and doesn't explain what was detected vs. requested. Including the folder and modes makes this actionable while still hinting at
--override.
if disk_mode is None or disk_mode != self.use_worktree:
raise RuntimeError("ERROR ... Re-Run kt checkout <kernel> --override ...")
kt/KT.md:97
- The example config JSON is invalid as written (missing a comma after the "user" field), so copying it will fail to parse.
"user": "USER"
"use_worktrees": false
}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
adeb92f to
df2daaf
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (4)
kt/ktlib/kernel_workspace.py:78
- The mismatch RuntimeError message is confusing (mentions only worktrees, uses placeholder syntax like
< --[no-]-worktree >, and includes an "ERROR ..." prefix). Since this code now supports both worktree and full-clone modes, the message should clearly describe the mode mismatch and provide concrete example commands.
raise RuntimeError(
"ERROR ... Command and git worktree states differ\n"
f"On Disk use_worktree {disk_mode}.\n"
f"Config/Command state use_worktree {self.use_worktree}\n"
"Either override to checkout (will be destructive)\n"
kt/commands/checkout/command.py:41
- This help text says checkout "will not create worktrees" for CentOS 7, but in practice the feature creates full clones instead. Wording should reflect the actual behavior (clone vs worktree) and keep "CentOS 7" formatting consistent.
$ kt checkout cbr-7.9 --no-worktree
Will not create worktrees for CentOS7 bridge.
Note: This is recommended because the git version in the CentOS7 VM is too old to support worktrees.
kt/KT.md:96
- The JSON example is missing a comma after the "user" key, which makes it invalid JSON as written.
"user": "USER"
"use_worktrees": false
kt/KT.md:88
- This code block is JSON, not bash; using the correct fence language improves readability and avoids implying the snippet is directly executable.
```bash
df2daaf to
4a25476
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (3)
kt/KT.md:95
- The JSON example is invalid because the
userfield is missing a trailing comma beforeuse_worktrees, so copying this snippet will fail to parse.
"user": "USER"
kt/ktlib/kernel_workspace.py:72
- The nested conditional expression used to derive
disk_labelis hard to read and easy to misinterpret; a simpler expression improves maintainability.
disk_label = "worktree" if disk_mode else "clone" if disk_mode is False else "unknown"
kt/ktlib/kernel_workspace.py:51
- Docstring grammar/clarity: "its" should be "it's", and the description can be clearer about file-vs-directory
.gitand the return meaning.
Static method to work out if a .git exists in directory.
If the .git is a file its a part of a worktree representation
else a directory is a pure clone.
Return
bmastbergen
left a comment
bmastbergen
left a comment
There was a problem hiding this comment.
A few minor things. Otherwise looks good!
|
|
||
| def cleanup(self): | ||
| disk_mode = self.detect_repo_disk_mode(self.folder) | ||
| if disk_mode is True: |
There was a problem hiding this comment.
If the checkout was originally a worktree, and someone manually deleted the worktree, and then did kt checkout lts-X.Y --cleanup _cleanup_worktree() wouldn't get called. Which is fine wrt the worktree itself, but the local branch in the source_root wouldn't get cleaned up. Prior to this change, even if the worktree got manually deleted, the local branch in the source_root would get cleaned up. Sort of a corner case so maybe not a big deal. I'm not sure how you'd fix it actually. You don't want to unconditionally delete a branch with the same name from the source_root if the original checkout was a clone. You might be deleting an unrelated branch that just happened to have the same name. We may just have to live with the orphaned branch if the worktree is deleted.
There was a problem hiding this comment.
yeah but the counter point could also be true previously if you did that then made a same named branch in the source_root you'd end up unconditionally deleting a branch in the source_root.
worktrees are weird and obfuscation tools run into weird multip workflow issues.
I think we'll leave it and if it becomes a wide problem then we can address it then as there are a couple extreme edge and optimizations are not implemented in here to begin with.
CentOS 7 does VM's git is too old to support worktrees so our kernel_build.sh script doesn't work within the VM. This is possibly easily worked around however not all developers are fluent with WorkTrees. This feature makes them optional. They can be configured at the kernels.yaml level like CentOS 7 is defaulted to off and will generate a warning to the use if they use worktrees while checking out the cbr-7.9. It can also be set per kernel in the .private_repos as well. NOTE this can explode the on disk usage of the kernel, user descression advised. Additionally Claude Opus 4.6 Updated the testing and wrote new tests based on user desired tests. Claude Output: test_kernel_workspace.py (20 tests) — new file: - detect_repo_disk_mode for worktree, clone, and nonexistent - Clone: .git is directory, local branch tracks remote, origin URL is upstream - Clone idempotency: second setup() updates without error - Clone cleanup: folder removed - Clone failure: partial folder cleaned up - Worktree: .git is file, branch created in source root, cleanup removes both - Mode mismatch: both directions raise RuntimeError mentioning --override - load_from_filepath detection: use_worktree correct for both modes - cbr-7.9 default (use_worktree=False): both sub-repos are clones, cleanup works - cbr-7.9 --worktree: both sub-repos are worktrees, cleanup works - lts-9.2 default then re-checkout: worktree mode, second setup updates cleanly - lts-9.2 --no-worktree then --worktree: mismatch error, then override+rebuild works test_config.py (4 tests) — appended: - use_worktrees defaults to True when absent - use_worktrees: false parsed correctly - Non-bool string "false" raises ValueError - Key not in REQUIRED_KEYS test_kernels.py (8 tests) — appended: - use_worktree field defaults to None - Parsed from kernel dict - Settable via kernel_overrides - should_use_worktree precedence: CLI > per-kernel > global > default True - CentOS 7 warning via caplog
4a25476 to
f3beedf
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Suppressed comments (7)
kt/commands/checkout/command.py:72
- Option help text is a bit unclear/grammatically awkward and doesn't mention what happens when disabling worktrees (full clone).
"--worktree/--no-worktree",
"use_worktree",
default=None,
help="Manually override config worktree configuration for kernel.",
kt/KT.md:85
- The CentOS 7 note uses inconsistent naming ("CentOS7") and singular "worktree". Adjusting this makes the note clearer and consistent with the rest of the docs.
NOTE: By default CentOS7 worktree is turned off due to git inside the CentOS7
VM is too old to support worktrees. You can enable it if you want, but note
that this is a limitation of the VM.
kt/KT.md:111
- This sentence is ungrammatical ("use this option for every command that uses every time"). Rewording clarifies that the CLI flag must be supplied each time you run checkout for that kernel.
3. CLI command line override. This will override the global and per kernel
config. However you will need to use this option for every command that uses
every time for that kernel version.
kt/ktlib/kernel_workspace.py:50
- Docstring grammar: "its" should be "it's", and the file-vs-dir distinction reads more clearly if stated explicitly.
Static method to work out if a .git exists in directory.
If the .git is a file its a part of a worktree representation
else a directory is a pure clone.
kt/ktlib/config.py:65
- The inline comment says only "except for user", but this block also treats
use_worktreesas a non-path key. Updating the comment avoids misleading future changes.
# Transform the str values to Path except for user
non_path_keys = {"user", "use_worktrees"}
kt/commands/checkout/command.py:41
- The new epilog example implies
--no-worktreeis specific to CentOS 7 and that it "will not create worktrees" (but it actually uses full clones and applies to any kernel). Updating the text makes the CLI help accurate.
This issue also appears on line 69 of the same file.
$ kt checkout cbr-7.9 --no-worktree
Will not create worktrees for CentOS7 bridge.
Note: This is recommended because the git version in the CentOS7 VM is too old to support worktrees.
kt/KT.md:81
- This paragraph has a few grammar issues ("checkout" vs "check out", double spaces) and the precedence explanation is a bit hard to parse. Tightening the wording will make the docs clearer.
This issue also appears in the following locations of the same file:
- line 83
- line 109
By default, kt will use git worktrees to checkout the kernel source and
appropriate dist-git repo. If you don't want to use worktrees you have several
options. Each option below will override the previous one, so if you turn off
worktrees at the kt config level you can turn them on for specific kernels later
by setting in the .private_repos.yaml or on the command line.
CentOS 7 does VM's git is too old to support worktrees so our kernel_build.sh script doesn't work within the VM. This is possibly easily worked around however not all developers are fluent with WorkTrees. This feature makes them optional. They can be configured at the kernels.yaml level like CentOS 7 is defaulted to off and will generate a warning to the use if they use worktrees while checking out the cbr-7.9.
It can also be set per kernel in the .private_repos as well. NOTE this can explode the on disk usage of the kernel, user descression advised.
Additionally Claude Opus 4.6 Updated the testing and wrote new tests based on user desired tests.
Claude Output:
test_kernel_workspace.py (20 tests) — new file:
test_config.py (4 tests) — appended:
test_kernels.py (8 tests) — appended:
Testing
This is the default config for
use_worktreesbeing true but as shown in this PR cbr-7.9Set the Default for kt config
Running with default, changing .private_repos, and using CLI
This is setting lts-9.6 as use_worktree: true
Note below
.gitis a folder for a clone and a ASCII file for a worktreeCoverage Report