While updating git-for-windows/git and microsoft/git for the 2.42.0 release window, a few patches that we've been running in those forks for a while came to light as something that would be beneficial to the core Git project. Here are some that are focused on the 'scalar' command. * Patch 1 adds a --no-src option to scalar clone to appease users who want to use scalar but object to the creation of the src directory. * Patches 2 and 3 help when scalar reconfigure -a fails. Patch 2 is a possibly helpful change on its own for other uses in the future. Updates in V2 ============= Thanks, Junio, for the helpful review! * In Patch 1, the '--[no-]src' documentation is tightened and the tests check the contents of the repository worktree. * In Patch 2, the commit message is reworded to be more clear about positive values of the enum. * In Patch 2, the GIT_DIR_NONE option of the enum is never returned, so it does not need to exist. A case in scalar.c referenced it, so it is removed as part of the patch (though that case was removed later by patch 3 anyway). * In Patch 2, the discover_git_directory() wrapper is updated to return -1 instead of 1, as it did before this patch. * In Patch 3, the 'failed' variable is renamed to 'succeeded' and the cases that update the value are swapped. The return code is set to -1 for any error instead of having a custom value based on the return from error() or error_errno(). Thanks, -Stolee Derrick Stolee (3): scalar: add --[no-]src option setup: add discover_git_directory_reason() scalar reconfigure: help users remove buggy repos Documentation/scalar.txt | 8 ++++- scalar.c | 65 +++++++++++++++++++++++++++++----------- setup.c | 34 ++++++++------------- setup.h | 35 ++++++++++++++++++++-- t/t9211-scalar-clone.sh | 12 ++++++++ 5 files changed, 110 insertions(+), 44 deletions(-) base-commit: a82fb66fed250e16d3010c75404503bea3f0ab61 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1569%2Fderrickstolee%2Fscalar-no-src-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1569/derrickstolee/scalar-no-src-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/1569 Range-diff vs v1: 1: c1c7e2f049e ! 1: 0b6957beccb scalar: add --[no-]src option @@ Documentation/scalar.txt: remote-tracking branch for the branch this option was `--single-branch` clone was made, no remote-tracking branch is created. +--[no-]src:: -+ Specify if the repository should be created within a `src` directory -+ within `<enlistment>`. This is the default behavior, so use -+ `--no-src` to opt-out of the creation of the `src` directory. ++ By default, `scalar clone` places the cloned repository within a ++ `<entlistment>/src` directory. Use `--no-src` to place the cloned ++ repository directly in the `<enlistment>` directory. + --[no-]full-clone:: A sparse-checkout is initialized by default. This behavior can be @@ t/t9211-scalar-clone.sh: test_expect_success 'scalar clone warns when background + scalar clone --no-src "file://$(pwd)/to-clone" without-src && + + test_path_is_dir with-src/src && -+ test_path_is_missing without-src/src ++ test_path_is_missing without-src/src && ++ ++ (cd with-src/src && ls ?*) >with && ++ (cd without-src && ls ?*) >without && ++ test_cmp with without +' + test_done 2: fbba6252aea ! 2: 787af0f9744 setup: add discover_git_directory_reason() @@ Commit message 1. The zero value of the enum is actually GIT_DIR_NONE, so nonpositive results are errors. - 2. There are multiple successful states, so some positive results are + 2. There are multiple successful states; positive results are successful. + It is worth noting that GIT_DIR_NONE is not returned, so we remove this + option from the enum. We must be careful to keep the successful reasons + as positive values, so they are given explicit positive values. + Further, a use in scalar.c was previously impossible, so it is removed. + Instead of updating all callers immediately, add a new method, discover_git_directory_reason(), and convert discover_git_directory() to be a thin shim on top of it. + One thing that is important to note is that discover_git_directory() + previously returned -1 on error, so let's continue that into the future. + There is only one caller (in scalar.c) that depends on that signedness + instead of a non-zero check, so clean that up, too. + Because there are extra checks that discover_git_directory_reason() does after setup_git_directory_gently_1(), there are other modes that can be returned for failure states. Add these modes to the enum, but be sure to @@ Commit message Signed-off-by: Derrick Stolee <derrickstolee@xxxxxxxxxx> + ## scalar.c ## +@@ scalar.c: static int cmd_reconfigure(int argc, const char **argv) + warning(_("removing stale scalar.repo '%s'"), + dir); + strbuf_release(&buf); +- } else if (discover_git_directory(&commondir, &gitdir) < 0) { +- warning_errno(_("git repository gone in '%s'"), dir); +- res = -1; + } else { + git_config_clear(); + + ## setup.c ## @@ setup.c: static const char *allowed_bare_repo_to_string( return NULL; @@ setup.c: static enum discovery_result setup_git_directory_gently_1(struct strbuf cwd_len = dir.len; - if (setup_git_directory_gently_1(&dir, gitdir, NULL, 0) <= 0) { -+ if ((result = setup_git_directory_gently_1(&dir, gitdir, NULL, 0)) <= 0) { ++ result = setup_git_directory_gently_1(&dir, gitdir, NULL, 0); ++ if (result <= 0) { strbuf_release(&dir); - return -1; + return result; @@ setup.c: int discover_git_directory(struct strbuf *commondir, const char *setup_git_directory_gently(int *nongit_ok) @@ setup.c: const char *setup_git_directory_gently(int *nongit_ok) + } *nongit_ok = 1; break; - case GIT_DIR_NONE: +- case GIT_DIR_NONE: + case GIT_DIR_CWD_FAILURE: + case GIT_DIR_INVALID_FORMAT: /* @@ setup.h: const char *resolve_gitdir_gently(const char *suspect, int *return_erro + * from discover_git_directory. + */ +enum discovery_result { -+ GIT_DIR_NONE = 0, -+ GIT_DIR_EXPLICIT, -+ GIT_DIR_DISCOVERED, -+ GIT_DIR_BARE, ++ GIT_DIR_EXPLICIT = 1, ++ GIT_DIR_DISCOVERED = 2, ++ GIT_DIR_BARE = 3, + /* these are errors */ + GIT_DIR_HIT_CEILING = -1, + GIT_DIR_HIT_MOUNT_POINT = -2, @@ setup.h: const char *resolve_gitdir_gently(const char *suspect, int *return_erro /* * Find the commondir and gitdir of the repository that contains the current * working directory, without changing the working directory or other global -@@ setup.h: void setup_work_tree(void); + * state. The result is appended to commondir and gitdir. If the discovered + * gitdir does not correspond to a worktree, then 'commondir' and 'gitdir' will * both have the same result appended to the buffer. The return value is - * either 0 upon success and non-zero if no repository was found. +- * either 0 upon success and non-zero if no repository was found. ++ * either 0 upon success and -1 if no repository was found. */ -int discover_git_directory(struct strbuf *commondir, - struct strbuf *gitdir); +static inline int discover_git_directory(struct strbuf *commondir, + struct strbuf *gitdir) +{ -+ return discover_git_directory_reason(commondir, gitdir) <= 0; ++ if (discover_git_directory_reason(commondir, gitdir) <= 0) ++ return -1; ++ return 0; +} + const char *setup_git_directory_gently(int *); 3: 907410f76c4 ! 3: 7ac7311863d scalar reconfigure: help users remove buggy repos @@ scalar.c: static int cmd_reconfigure(int argc, const char **argv) git_config(get_scalar_repos, &scalar_repos); for (i = 0; i < scalar_repos.nr; i++) { -+ int failed = 0; ++ int succeeded = 0; const char *dir = scalar_repos.items[i].string; strbuf_reset(&commondir); @@ scalar.c: static int cmd_reconfigure(int argc, const char **argv) warning_errno(_("could not switch to '%s'"), dir); - res = -1; - continue; -+ failed = -1; + goto loop_end; } @@ scalar.c: static int cmd_reconfigure(int argc, const char **argv) if (remove_deleted_enlistment(&buf)) - res = error(_("could not remove stale " - "scalar.repo '%s'"), dir); -+ failed = error(_("could not remove stale " -+ "scalar.repo '%s'"), dir); - else +- else - warning(_("removing stale scalar.repo '%s'"), ++ error(_("could not remove stale " ++ "scalar.repo '%s'"), dir); ++ else { + warning(_("removed stale scalar.repo '%s'"), dir); ++ succeeded = 1; ++ } strbuf_release(&buf); -- } else if (discover_git_directory(&commondir, &gitdir) < 0) { -- warning_errno(_("git repository gone in '%s'"), dir); -- res = -1; - } else { - git_config_clear(); -- -- the_repository = &r; -- r.commondir = commondir.buf; -- r.gitdir = gitdir.buf; -- -- if (set_recommended_config(1) < 0) -- res = -1; + goto loop_end; + } + + switch (discover_git_directory_reason(&commondir, &gitdir)) { + case GIT_DIR_INVALID_OWNERSHIP: + warning(_("repository at '%s' has different owner"), dir); -+ failed = -1; + goto loop_end; + + case GIT_DIR_DISCOVERED: ++ succeeded = 1; + break; + + default: + warning(_("repository not found in '%s'"), dir); -+ failed = -1; + break; + } + @@ scalar.c: static int cmd_reconfigure(int argc, const char **argv) + the_repository = &r; + r.commondir = commondir.buf; + r.gitdir = gitdir.buf; -+ -+ if (set_recommended_config(1) < 0) -+ failed = -1; -+ + +- the_repository = &r; +- r.commondir = commondir.buf; +- r.gitdir = gitdir.buf; ++ if (set_recommended_config(1) >= 0) ++ succeeded = 1; + +- if (set_recommended_config(1) < 0) +- res = -1; +loop_end: -+ if (failed) { -+ res = failed; ++ if (!succeeded) { ++ res = -1; + warning(_("to unregister this repository from Scalar, run\n" + "\tgit config --global --unset --fixed-value scalar.repo \"%s\""), + dir); -- gitgitgadget
