From fa72087fe0ee12e452c865270c20e0a49aae7004 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Tue, 4 Oct 2022 08:01:17 -0400 Subject: [PATCH 1/4] abspath: make strip_last_path_component() global The strip_last_component() method is helpful for finding the parent directory of a path stored in a strbuf. Extract it to a global method advertised in path.h. With that additional visibility, it is helpful to rename it to be more specific to paths. Signed-off-by: Derrick Stolee --- abspath.c | 6 +++--- path.h | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/abspath.c b/abspath.c index 7a5f114382..058e638e98 100644 --- a/abspath.c +++ b/abspath.c @@ -12,7 +12,7 @@ int is_directory(const char *path) } /* removes the last path component from 'path' except if 'path' is root */ -static void strip_last_component(struct strbuf *path) +void strip_last_path_component(struct strbuf *path) { size_t offset = offset_1st_component(path->buf); size_t len = path->len; @@ -117,7 +117,7 @@ static char *strbuf_realpath_1(struct strbuf *resolved, const char *path, continue; /* '.' component */ } else if (next.len == 2 && !strcmp(next.buf, "..")) { /* '..' component; strip the last path component */ - strip_last_component(resolved); + strip_last_path_component(resolved); continue; } @@ -169,7 +169,7 @@ static char *strbuf_realpath_1(struct strbuf *resolved, const char *path, * strip off the last component since it will * be replaced with the contents of the symlink */ - strip_last_component(resolved); + strip_last_path_component(resolved); } /* diff --git a/path.h b/path.h index 0a59c85a62..db417a43bd 100644 --- a/path.h +++ b/path.h @@ -179,6 +179,10 @@ const char *git_path_auto_merge(struct repository *r); const char *git_path_fetch_head(struct repository *r); const char *git_path_shallow(struct repository *r); +/** + * Remove the last path component from 'path' except if 'path' is root. + */ +void strip_last_path_component(struct strbuf *path); int ends_with_path_components(const char *path, const char *components); From cea863e346c77a77083e76e1b8dcdbdd306a6fd6 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Fri, 30 Sep 2022 12:41:27 -0400 Subject: [PATCH 2/4] scalar: .scalarCache should live above enlistment We should not be putting the .scalarCache inside the enlistment as a sibling to the 'src' directory. This only happens in "unattended" mode, but it also negates any benefit of a shared object cache because each enlistment absolutely does not share any objects with others. Move the shared object cache in this case to a level above the enlistment, so at least there is some hope that it can be reused. This is also critical to the upcoming --no-src option, since the shared object cache cannot be located within the Git repository. Signed-off-by: Derrick Stolee --- scalar.c | 10 ++++++++-- t/t9210-scalar.sh | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/scalar.c b/scalar.c index b2e8c39107..ae35b7ef49 100644 --- a/scalar.c +++ b/scalar.c @@ -16,6 +16,7 @@ #include "help.h" #include "json-parser.h" #include "remote.h" +#include "path.h" static int is_unattended(void) { return git_env_bool("Scalar_UNATTENDED", 0); @@ -461,8 +462,13 @@ static char *default_cache_root(const char *root) { const char *env; - if (is_unattended()) - return xstrfmt("%s/.scalarCache", root); + if (is_unattended()) { + struct strbuf path = STRBUF_INIT; + strbuf_addstr(&path, root); + strip_last_path_component(&path); + strbuf_addstr(&path, "/.scalarCache"); + return strbuf_detach(&path, NULL); + } #ifdef WIN32 (void)env; diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh index 8414cfc024..c65550de59 100755 --- a/t/t9210-scalar.sh +++ b/t/t9210-scalar.sh @@ -282,7 +282,7 @@ test_expect_success '`scalar clone` with GVFS-enabled server' ' cache_key="url_$(printf "%s" http://$HOST_PORT/ | tr A-Z a-z | test-tool sha1)" && - echo "$(pwd)/using-gvfs/.scalarCache/$cache_key" >expect && + echo "$(pwd)/.scalarCache/$cache_key" >expect && git -C using-gvfs/src config gvfs.sharedCache >actual && test_cmp expect actual && From 8d46c8e0f6bef552b30179376ce15272c00c7259 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Fri, 30 Sep 2022 12:44:27 -0400 Subject: [PATCH 3/4] scalar: add --no-src option Some users have strong aversions to Scalar's opinion that the repository should be in a 'src' directory, even though it creates a clean slate for placing build outputs in adjacent directories. The --no-src option allows users to opt-out of the default behavior. The parse-opt logic automatically figures out that '--src' is a possible flag that negates '--no-src'. Signed-off-by: Derrick Stolee --- Documentation/scalar.txt | 5 ++++- scalar.c | 14 ++++++++++++-- t/t9210-scalar.sh | 8 ++++++++ 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/Documentation/scalar.txt b/Documentation/scalar.txt index 0424d020d9..35f67801e9 100644 --- a/Documentation/scalar.txt +++ b/Documentation/scalar.txt @@ -9,7 +9,7 @@ SYNOPSIS -------- [verse] scalar clone [--single-branch] [--branch ] [--full-clone] - [--local-cache-path ] [--cache-server-url ] + [--local-cache-path ] [--cache-server-url ] [--[no-]src] [] scalar list scalar register [] @@ -83,6 +83,9 @@ remote-tracking branch for the branch this option was used for the initial cloning. If the HEAD at the remote did not point at any branch when `--single-branch` clone was made, no remote-tracking branch is created. +--no-src:: + Skip adding a `src` directory within the target enlistment. + --[no-]full-clone:: A sparse-checkout is initialized by default. This behavior can be turned off via `--full-clone`. diff --git a/scalar.c b/scalar.c index ae35b7ef49..5a576392d2 100644 --- a/scalar.c +++ b/scalar.c @@ -691,6 +691,8 @@ static int cmd_clone(int argc, const char **argv) int full_clone = 0, single_branch = 0, dummy = 0; const char *cache_server_url = NULL, *local_cache_root = NULL; char *default_cache_server_url = NULL, *local_cache_root_abs = NULL; + const char *enlistment_parent; + int no_src = 0; struct option clone_options[] = { OPT_STRING('b', "branch", &branch, N_(""), N_("branch to checkout after clone")), @@ -699,6 +701,8 @@ static int cmd_clone(int argc, const char **argv) OPT_BOOL(0, "single-branch", &single_branch, N_("only download metadata for the branch that will " "be checked out")), + OPT_BOOL(0, "no-src", &no_src, + N_("skip creating a 'src' directory")), OPT_STRING(0, "cache-server-url", &cache_server_url, N_(""), N_("the url or friendly name of the cache server")), @@ -749,7 +753,13 @@ static int cmd_clone(int argc, const char **argv) ensure_absolute_path(enlistment, &enlistment); - dir = xstrfmt("%s/src", enlistment); + if (!no_src) { + dir = xstrfmt("%s/src", enlistment); + enlistment_parent = "../.."; + } else { + dir = xstrdup(enlistment); + enlistment_parent = ".."; + } if (!local_cache_root) local_cache_root = local_cache_root_abs = @@ -790,7 +800,7 @@ static int cmd_clone(int argc, const char **argv) struct strbuf path = STRBUF_INIT; strbuf_addstr(&path, enlistment); - if (chdir("../..") < 0 || + if (chdir(enlistment_parent) < 0 || remove_dir_recursively(&path, 0) < 0) die(_("'--local-cache-path' cannot be inside the src " "folder;\nCould not remove '%s'"), enlistment); diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh index c65550de59..f2fdf32478 100755 --- a/t/t9210-scalar.sh +++ b/t/t9210-scalar.sh @@ -367,4 +367,12 @@ test_expect_success '`scalar delete` with existing repo' ' test_path_is_missing existing ' +test_expect_success '`scalar clone --no-src`' ' + scalar clone --src "file://$(pwd)" with-src && + scalar clone --no-src "file://$(pwd)" without-src && + + test_path_is_dir with-src/src && + test_path_is_missing without-src/src +' + test_done From 6508076132dbacf8f06e9c0d3a7facfd9bc2984a Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Fri, 30 Sep 2022 12:59:40 -0400 Subject: [PATCH 4/4] worktree: allow in Scalar repositories The 'git worktree' command was marked as BLOCK_ON_GVFS_REPO because it does not interact well with the virtual filesystem of VFS for Git. When a Scalar clone uses the GVFS protocol, it enables the GVFS_BLOCK_COMMANDS flag, since commands like 'git gc' do not work well with the GVFS protocol. However, 'git worktree' works just fine with the GVFS protocol since it isn't doing anything special. It copies the sparse-checkout from the current worktree, so it does not have performance issues. This is a highly requested option. The solution is to stop using the BLOCK_ON_GVFS_REPO option and instead add a special-case check in cmd_worktree() specifically for a particular bit of the 'core_gvfs' global variable (loaded by very early config reading) that corresponds to the virtual filesystem. The bit that most closely resembled this behavior was non-obviously named, but does provide a signal that we are in a Scalar clone and not a VFS for Git clone. The error message is copied from git.c, so it will have the same output as before if a user runs this in a VFS for Git clone. Signed-off-by: Derrick Stolee --- builtin/worktree.c | 8 ++++++++ git.c | 2 +- gvfs.h | 11 +++++++++++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index c6710b2552..5a4904efdb 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -1,4 +1,5 @@ #include "cache.h" +#include "gvfs.h" #include "checkout.h" #include "config.h" #include "builtin.h" @@ -1127,6 +1128,13 @@ int cmd_worktree(int ac, const char **av, const char *prefix) git_config(git_worktree_config, NULL); + /* + * git-worktree is special-cased to work in Scalar repositories + * even when they use the GVFS Protocol. + */ + if (core_gvfs & GVFS_USE_VIRTUAL_FILESYSTEM) + die("'git %s' is not supported on a GVFS repo", "worktree"); + if (!prefix) prefix = ""; diff --git a/git.c b/git.c index f591a6ef8b..e7aa35a207 100644 --- a/git.c +++ b/git.c @@ -703,7 +703,7 @@ static struct cmd_struct commands[] = { { "verify-tag", cmd_verify_tag, RUN_SETUP }, { "version", cmd_version }, { "whatchanged", cmd_whatchanged, RUN_SETUP }, - { "worktree", cmd_worktree, RUN_SETUP | BLOCK_ON_GVFS_REPO }, + { "worktree", cmd_worktree, RUN_SETUP }, { "write-tree", cmd_write_tree, RUN_SETUP }, }; diff --git a/gvfs.h b/gvfs.h index 7d999f3e8d..99c5205aa0 100644 --- a/gvfs.h +++ b/gvfs.h @@ -14,7 +14,18 @@ #define GVFS_SKIP_SHA_ON_INDEX (1 << 0) #define GVFS_BLOCK_COMMANDS (1 << 1) #define GVFS_MISSING_OK (1 << 2) + +/* + * This behavior of not deleting outside of the sparse-checkout + * is specific to the virtual filesystem support. It is only + * enabled by VFS for Git, and so can be used as an indicator + * that we are in a virtualized filesystem environment and not + * in a Scalar environment. This bit has two names to reflect + * that. + */ #define GVFS_NO_DELETE_OUTSIDE_SPARSECHECKOUT (1 << 3) +#define GVFS_USE_VIRTUAL_FILESYSTEM (1 << 3) + #define GVFS_FETCH_SKIP_REACHABILITY_AND_UPLOADPACK (1 << 4) #define GVFS_BLOCK_FILTERS_AND_EOL_CONVERSIONS (1 << 6) #define GVFS_PREFETCH_DURING_FETCH (1 << 7)