sparse checkout: fix a few bugs and check argument validity for set/add
Get series with b4
b4 am [email protected]Versions newest first · linked by author and title
- v5 Latest5 patches 2 repliesIn review
- v45 patches 4 repliesSuperseded
- v35 patches 9 repliesSuperseded
Earlier versions · 2
| Version | Posted | Patches | Replies | Status |
|---|---|---|---|---|
| v2 | 6 patches | 4 replies | Superseded | |
| v1 | 7 patches | 18 replies | Superseded |
Changes in v5 from the cover letter
* have --skip-checks enable running from a subdirectory in non-cone mode
* make sure new die() messages are marked for translation (and using single quotes instead of double)
Patches in v5 No applies recorded yet
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/5 | sparse-checkout: correct reapply's handling of optionsbuiltin/ t/ | +31 | 0 replies | |
| 2/5 | sparse-checkout: correctly set non-cone mode when expectedbuiltin/ | +1 | 0 replies | |
| 3/5 | sparse-checkout: pay attention to prefix for {set, add}builtin/ t/ | +67 | 0 replies | |
| 4/5 | sparse-checkout: error or warn when given individual filesbuiltin/ t/ | +51 | 0 replies | |
| 5/5 | sparse-checkout: reject arguments in cone-mode that look like patternsbuiltin/ t/ | +12 | 0 replies |
Reviews trailers per patch
All 5 patches have a Reviewed-by or Acked-by.
| Patch | Derrick Stolee | Junio Hamano |
|---|---|---|
| 1/5 | R-b | |
| 2/5 | R-b | |
| 3/5 | R-b | Helped-by |
| 4/5 | R-b | |
| 5/5 | R-b |
Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.
Cover letter [PATCH v5 0/5] sparse checkout: fix a few bugs and check argument validity for set/add
== Maintainer notes ==
Note: There is a small textual and small semantic conflict with ds/sparse-checkout-requires-per-worktree-config in seen. I included the diff with the correct resolution near the end of this cover letter. If you'd prefer I rebased on top of ds/sparse-chckout-requires-per-worktree-config, let me know.
== Overview ==
This series continues attempts to make sparse-checkouts more user friendly. A quick overview:
* Patches 1-2 fix existing bugs from en/sparse-checkout-set (i.e. in v2.35.0)
* Patch 3 fixes sparse-checkout-from-subdirectories-ignores-"prefix" (see
https://lore.kernel.org/git/[email protected]/),
in cone mode. Since we'll get nasty surprises whether we use or don't use "prefix" for non-cone mode, simply throw an error if set/add subcommands of sparse-checkout are run from a subdirectory.
* Patches 4-5 check positional arguments to set/add and provide errors/warnings for very likely mistakes. It also adds a --skip-checks flag for overridding in case you have a very unusual situation.
== Update history ==
Changes since v4:
* have --skip-checks enable running from a subdirectory in non-cone mode
* make sure new die() messages are marked for translation (and using single quotes instead of double)
Changes since v3:
* Use strpbrk() instead of multiple strchr(), fix commit message relative to backslashes.
Changes since v2:
* Dropped patch 5
* Added Stolee's Reviewed-by
Changes since v1:
* Dropped the commit changing cone-mode to default (patch 7, which will be split into multiple patches and submitted as a separate series)
* Removed the RFC label
* Decided to error out when running set/add with paths from a subdirectory in non-cone mode, and added tests
* Changed the warning for non-cone mode with individual files to point out that the user is likely trying to select an individual file, but should likely add a leading slash to ensure that is what happens
* Fixed typos, removed unnecessary condition checks
== Conflict resolution ==
Patch to resolve textual and semantic conflict with ds/sparse-checkout-requires-per-worktree-config:
t/t1091-sparse-checkout-builtin.sh +0 -0
remerge CONFLICT (content): Merge conflict in t/t1091-sparse-checkout-builtin.sh index 3c6adeb885..3a95d2996d 100755
t/t1091-sparse-checkout-builtin.sh +4 -17
@@ -275,24 +275,8 @@ test_expect_success 'sparse-index enabled and disabled' ' diff -u sparse full | tail -n +3 >actual && test_cmp expect actual && -<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< 286c22e5ec (sparse-checkout: reject arguments in cone-mode that look like patterns) git -C repo config --list >config && - ! grep index.sparse config -|||||||||||||||||||||||||||||||| 89bece5c8c - diff -u sparse full | tail -n +3 >actual && - test_cmp expect actual && - - git -C repo config --list >config && - ! grep index.sparse config - ) -================================ - diff -u sparse full | tail -n +3 >actual && - test_cmp expect actual && - - git -C repo config --list >config && - test_cmp_config -C repo false index.sparse - ) ->>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 3ce1138272 (config: make git_configset_get_string_tmp() private) + test_cmp_config -C repo false index.sparse ' test_expect_success 'cone mode: init and set' '
@@ -532,6 +516,7 @@ test_expect_success 'reapply can handle config options' ' cat >expect <<-\EOF && core.sparsecheckout=true core.sparsecheckoutcone=true + index.sparse=false EOF test_cmp expect actual &&
@@ -539,6 +524,8 @@ test_expect_success 'reapply can handle config options' ' git -C repo config --worktree --list >actual && cat >expect <<-\EOF && core.sparsecheckout=true + core.sparsecheckoutcone=false + index.sparse=false EOF test_cmp expect actual &&
== CCs ==
Elijah Newren (5):
sparse-checkout: correct reapply's handling of options
sparse-checkout: correctly set non-cone mode when expected
sparse-checkout: pay attention to prefix for {set, add}
sparse-checkout: error or warn when given individual files
sparse-checkout: reject arguments in cone-mode that look like patterns
builtin/sparse-checkout.c | 78 +++++++++++++++++++++++++-- t/t1091-sparse-checkout-builtin.sh | 87 +++++++++++++++++++++++++++++- 2 files changed, 158 insertions(+), 7 deletions(-)
base-commit: b80121027d1247a0754b3cc46897fee75c050b44
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1118%2Fnewren%2Fsparse-checkout-default-and-arg-validity-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1118/newren/sparse-checkout-default-and-arg-validity-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/1118
Range-diff vs v4:
1: 5215b7f7179 = 1: 5215b7f7179 sparse-checkout: correct reapply's handling of options
2: 0c2ab523e74 = 2: 0c2ab523e74 sparse-checkout: correctly set non-cone mode when expected
3: e68b0a37ff3 ! 3: 06cf71bfca4 sparse-checkout: pay attention to prefix for {set, add}
@@ builtin/sparse-checkout.c: static int modify_pattern_list(int argc, const char *
+ if (!argc)
+ return;
+
-+ if (prefix && *prefix) {
++ if (prefix && *prefix && core_sparse_checkout_cone) {
+ /*
+ * The args are not pathspecs, so unfortunately we
+ * cannot imitate how cmd_add() uses parse_pathspec().
@@ builtin/sparse-checkout.c: static int modify_pattern_list(int argc, const char *
+ int i;
+ int prefix_len = strlen(prefix);
+
-+ if (!core_sparse_checkout_cone)
-+ die("please run from the toplevel directory in non-cone mode");
-+
+ for (i = 0; i < argc; i++)
+ argv[i] = prefix_path(prefix, prefix_len, argv[i]);
+ }
++
++ if (prefix && *prefix && !core_sparse_checkout_cone)
++ die(_("please run from the toplevel directory in non-cone mode"));
++
+}
+
static char const * const builtin_sparse_checkout_add_usage[] = {
4: 1fdebc1953f ! 4: 78bf6016687 sparse-checkout: error or warn when given individual files
@@ Commit message
in any directory. Thus users will likely want to prefix any paths they
specify with a leading '/' character; warn users if the patterns they
specify exactly name a file because it means they are likely missing
- such a missing leading slash.
+ such a leading slash.
Reviewed-by: Derrick Stolee <[email protected]>
Signed-off-by: Elijah Newren <[email protected]>
@@ builtin/sparse-checkout.c: static void sanitize_paths(int argc, const char **arg
- int i;
int prefix_len = strlen(prefix);
- if (!core_sparse_checkout_cone)
-@@ builtin/sparse-checkout.c: static void sanitize_paths(int argc, const char **argv, const char *prefix)
for (i = 0; i < argc; i++)
argv[i] = prefix_path(prefix, prefix_len, argv[i]);
}
-+
+
+ if (skip_checks)
+ return;
+
+ if (prefix && *prefix && !core_sparse_checkout_cone)
+ die(_("please run from the toplevel directory in non-cone mode"));
+
+ for (i = 0; i < argc; i++) {
+ struct cache_entry *ce;
+ struct index_state *index = the_repository->index;
@@ builtin/sparse-checkout.c: static void sanitize_paths(int argc, const char **arg
+ continue;
+
+ if (core_sparse_checkout_cone)
-+ die(_("\"%s\" is not a directory; to treat it as a directory anyway, rerun with --skip-checks"), argv[i]);
++ die(_("'%s' is not a directory; to treat it as a directory anyway, rerun with --skip-checks"), argv[i]);
+ else
-+ warning(_("pass a leading slash before paths such as \"%s\" if you want a single file (see NON-CONE PROBLEMS in the git-sparse-checkout manual)."), argv[i]);
++ warning(_("pass a leading slash before paths such as '%s' if you want a single file (see NON-CONE PROBLEMS in the git-sparse-checkout manual)."), argv[i]);
+ }
} 5: d68682c25c4 ! 5: 53183d07502 sparse-checkout: reject arguments in cone-mode that look like patterns
@@ Commit message
## builtin/sparse-checkout.c ##
@@ builtin/sparse-checkout.c: static void sanitize_paths(int argc, const char **argv,
- if (skip_checks)
- return;
+ if (prefix && *prefix && !core_sparse_checkout_cone)
+ die(_("please run from the toplevel directory in non-cone mode"));
+ if (core_sparse_checkout_cone) {
+ for (i = 0; i < argc; i++) {
Signature · 1 line
gitgitgadget