Upstreaming the Scalar command
Subsystems (2)
PATCH v8 12/17 scalar: teach 'clone' to support the --single-branch option
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
Just like git clone, the scalar clone command now also offers to restrict the clone to a single branch.
--- contrib/scalar/scalar.c | 9 +++++++-- contrib/scalar/scalar.txt | 12 +++++++++++- contrib/scalar/t/t9099-scalar.sh | 6 +++++- 3 files changed, 23 insertions(+), 4 deletions(-)
contrib/scalar/scalar.c +7 -2
@@ -327,12 +327,15 @@ static char *remote_default_branch(const char *url)static int cmd_clone (int argc,const char **argv) {const char *branch =NULL ; -int full_clone =0 ; +int full_clone =0 , single_branch =0 ;struct option clone_options[] = {OPT_STRING ('b' ,"branch" , &branch,N_ ("<branch>" ),N_ ("branch to checkout after clone" )),OPT_BOOL (0 ,"full-clone" , &full_clone,N_ ("when cloning, create full working directory" )), +OPT_BOOL (0 ,"single-branch" , &single_branch, +N_ ("only download metadata for the branch that will " +"be checked out" )),OPT_END (), };const char *const clone_usage[] = {
@@ -403,7 +406,9 @@ static int cmd_clone(int argc, const char **argv)if (set_config ("remote.origin.url=%s" , url) ||set_config ("remote.origin.fetch=" -"+refs/heads/*:refs/remotes/origin/*" ) || +"+refs/heads/%s:refs/remotes/origin/%s" , + single_branch ? branch :"*" , + single_branch ? branch :"*" ) ||set_config ("remote.origin.promisor=true" ) ||set_config ("remote.origin.partialCloneFilter=blob:none" )) { res =error (_ ("could not configure remote in '%s'" ), dir);
contrib/scalar/scalar.txt +11 -1
@@ -8,7 +8,7 @@ scalar - an opinionated repository management tool SYNOPSIS -------- [verse] -scalar clone [--branch <main-branch>] [--full-clone] <url> [<enlistment>] +scalar clone [--single-branch] [--branch <main-branch>] [--full-clone] <url> [<enlistment>] scalar list scalar register [<enlistment>] scalar unregister [<enlistment>]
@@ -57,6 +57,16 @@ HEAD[:<directory>]`. Instead of checking out the branch pointed to by the cloned repository's HEAD, check out the `<name>` branch instead. +--[no-]single-branch:: + Clone only the history leading to the tip of a single branch, either + specified by the `--branch` option or the primary branch remote's + `HEAD` points at. ++ +Further fetches into the resulting repository will only update the +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-]full-clone:: A sparse-checkout is initialized by default. This behavior can be turned off via `--full-clone`.
contrib/scalar/t/t9099-scalar.sh +5 -1
@@ -45,13 +45,17 @@ test_expect_success 'set up repository to clone' ' test_expect_success 'scalar clone' ' second=$(git rev-parse --verify second:second.t) && - scalar clone "file://$(pwd)" cloned && + scalar clone "file://$(pwd)" cloned --single-branch && ( cd cloned/src && git config --get --global --fixed-value maintenance.repo \ "$(pwd)" && + git for-each-ref --format="%(refname)" refs/remotes/origin/ >actual && + echo "refs/remotes/origin/parallel" >expect && + test_cmp expect actual && + test_path_is_missing 1/2 && test_must_fail git rev-list --missing=print $second && git rev-list $second &&
Signature · 1 line
gitgitgadget
PATCH v8 13/17 scalar: implement the `run` command
In reply to Johannes Schindelin via GitGitGadgetFrom: Derrick Stolee <[email protected]>
Note: this subcommand is provided primarily for backwards-compatibility, for existing Scalar uses. It is mostly just a shim for git maintenance, mapping task names from the way Scalar called them to the way Git calls them.
The reason why those names differ? The background maintenance was first implemented in Scalar, and when it was contributed as a patch series implementing the git maintenance command, reviewers suggested better names, those suggestions were accepted before the patches were integrated into core Git.
--- contrib/scalar/scalar.c | 64 +++++++++++++++++++++++++++++++++++++++ contrib/scalar/scalar.txt | 19 ++++++++++++ 2 files changed, 83 insertions(+)
contrib/scalar/scalar.c +64 -0
@@ -484,6 +484,69 @@ static int cmd_register(int argc, const char **argv)return register_dir (); } +static int cmd_run (int argc,const char **argv) +{ +struct option options[] = { +OPT_END (), + }; +struct { +const char *arg, *task; + } tasks[] = { + {"config" ,NULL }, + {"commit-graph" ,"commit-graph" }, + {"fetch" ,"prefetch" }, + {"loose-objects" ,"loose-objects" }, + {"pack-files" ,"incremental-repack" }, + {NULL ,NULL } + }; +struct strbuf buf =STRBUF_INIT ; +const char *usagestr[] = {NULL ,NULL }; +int i; + +strbuf_addstr (&buf,N_ ("scalar run <task> [<enlistment>]\nTasks:\n" )); +for (i =0 ; tasks[i].arg; i++) +strbuf_addf (&buf,"\t%s\n" , tasks[i].arg); + usagestr[0 ] = buf.buf; + + argc =parse_options (argc, argv,NULL , options, + usagestr,0 ); + +if (!argc) +usage_with_options (usagestr, options); + +if (!strcmp ("all" , argv[0 ])) { + i =-1 ; + }else { +for (i =0 ; tasks[i].arg &&strcmp (tasks[i].arg, argv[0 ]); i++) + ;/* keep looking for the task */ + +if (i >0 && !tasks[i].arg) { +error (_ ("no such task: '%s'" ), argv[0 ]); +usage_with_options (usagestr, options); + } + } + + argc--; + argv++; +setup_enlistment_directory (argc, argv, usagestr, options,NULL ); +strbuf_release (&buf); + +if (i ==0 ) +return register_dir (); + +if (i >0 ) +return run_git ("maintenance" ,"run" , +"--task" , tasks[i].task,NULL ); + +if (register_dir ()) +return -1 ; +for (i =1 ; tasks[i].arg; i++) +if (run_git ("maintenance" ,"run" , +"--task" , tasks[i].task,NULL )) +return -1 ; +return 0 ; +} +static int remove_deleted_enlistment (struct strbuf *path) {int res =0 ;
@@ -556,6 +619,7 @@ static struct { { "list", cmd_list }, { "register", cmd_register }, { "unregister", cmd_unregister }, + { "run", cmd_run }, { NULL, NULL}, };
contrib/scalar/scalar.txt +19 -0
@@ -12,6 +12,7 @@ scalar clone [--single-branch] [--branch <main-branch>] [--full-clone] <url> [<e scalar list scalar register [<enlistment>] scalar unregister [<enlistment>] +scalar run ( all | config | commit-graph | fetch | loose-objects | pack-files ) [<enlistment>] DESCRIPTION -----------
@@ -98,6 +99,24 @@ unregister [<enlistment>]:: Remove the specified repository from the list of repositories registered with Scalar and stop the scheduled background maintenance. +Run +~~~ + +scalar run ( all | config | commit-graph | fetch | loose-objects | pack-files ) [<enlistment>]:: + Run the given maintenance task (or all tasks, if `all` was specified). + Except for `all` and `config`, this subcommand simply hands off to + linkgit:git-maintenance[1] (mapping `fetch` to `prefetch` and + `pack-files` to `incremental-repack`). ++ +These tasks are run automatically as part of the scheduled maintenance, +as soon as the repository is registered with Scalar. It should therefore +not be necessary to run this subcommand manually. ++ +The `config` task is specific to Scalar and configures all those +opinionated default settings that make Git work more efficiently with +large repositories. As this task is run as part of `scalar clone` +automatically, explicit invocations of this task are rarely needed. + SEE ALSO -------- linkgit:git-clone[1], linkgit:git-maintenance[1].
Signature · 1 line
gitgitgadget
PATCH v8 14/17 scalar: allow reconfiguring an existing enlistment
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
This comes in handy during Scalar upgrades, or when config settings were messed up by mistake.
--- contrib/scalar/scalar.c | 79 +++++++++++++++++++++----------- contrib/scalar/scalar.txt | 8 ++++ contrib/scalar/t/t9099-scalar.sh | 8 ++++ 3 files changed, 67 insertions(+), 28 deletions(-)
contrib/scalar/scalar.c +51 -28
@@ -108,18 +108,20 @@ static int run_git(const char *arg, ...)return res; } -static int set_recommended_config (void ) +static int set_recommended_config (int reconfigure) {struct {const char *key;const char *value; +int overwrite_on_reconfigure; } config[] = { - {"am.keepCR" ,"true" }, - {"core.FSCache" ,"true" }, - {"core.multiPackIndex" ,"true" }, - {"core.preloadIndex" ,"true" }, +/* Required */ + {"am.keepCR" ,"true" ,1 }, + {"core.FSCache" ,"true" ,1 }, + {"core.multiPackIndex" ,"true" ,1 }, + {"core.preloadIndex" ,"true" ,1 }, #ifndef WIN32 - {"core.untrackedCache" ,"true" }, + {"core.untrackedCache" ,"true" ,1 }, #else /* * Unfortunately, Scalar's Functional Tests demonstrated
@@ -133,28 +135,29 @@ static int set_recommended_config(void) * Therefore, with a sad heart, we disable this very useful * feature on Windows. */ - { "core.untrackedCache", "false" }, + { "core.untrackedCache", "false", 1 }, #endif - { "core.logAllRefUpdates", "true" }, - { "credential.https://dev.azure.com.useHttpPath", "true" }, - { "credential.validate", "false" }, /* GCM4W-only */ - { "gc.auto", "0" }, - { "gui.GCWarning", "false" }, - { "index.threads", "true" }, - { "index.version", "4" }, - { "merge.stat", "false" }, - { "merge.renames", "false" }, - { "pack.useBitmaps", "false" }, - { "pack.useSparse", "true" }, - { "receive.autoGC", "false" }, - { "reset.quiet", "true" }, - { "feature.manyFiles", "false" }, - { "feature.experimental", "false" }, - { "fetch.unpackLimit", "1" }, - { "fetch.writeCommitGraph", "false" }, + { "core.logAllRefUpdates", "true", 1 }, + { "credential.https://dev.azure.com.useHttpPath", "true", 1 }, + { "credential.validate", "false", 1 }, /* GCM4W-only */ + { "gc.auto", "0", 1 }, + { "gui.GCWarning", "false", 1 }, + { "index.threads", "true", 1 }, + { "index.version", "4", 1 }, + { "merge.stat", "false", 1 }, + { "merge.renames", "false", 1 }, + { "pack.useBitmaps", "false", 1 }, + { "pack.useSparse", "true", 1 }, + { "receive.autoGC", "false", 1 }, + { "reset.quiet", "true", 1 }, + { "feature.manyFiles", "false", 1 }, + { "feature.experimental", "false", 1 }, + { "fetch.unpackLimit", "1", 1 }, + { "fetch.writeCommitGraph", "false", 1 }, #ifdef WIN32 - { "http.sslBackend", "schannel" }, + { "http.sslBackend", "schannel", 1 }, #endif + /* Optional */ { "status.aheadBehind", "false" }, { "commitGraph.generationVersion", "1" }, { "core.autoCRLF", "false" },
@@ -166,7 +169,8 @@ static int set_recommended_config(void)char *value;for (i =0 ; config[i].key; i++) { -if (git_config_get_string (config[i].key, &value)) { +if ((reconfigure && config[i].overwrite_on_reconfigure) || +git_config_get_string (config[i].key, &value)) {trace2_data_string ("scalar" , the_repository, config[i].key,"created" );if (git_config_set_gently (config[i].key, config[i].value) <0 )
@@ -231,7 +235,7 @@ static int register_dir(void)int res =add_or_remove_enlistment (1 );if (!res) - res =set_recommended_config (); + res =set_recommended_config (0 );if (!res) res =toggle_maintenance (1 );
@@ -419,7 +423,7 @@ static int cmd_clone(int argc, const char **argv) (res = run_git("sparse-checkout", "init", "--cone", NULL))) goto cleanup; -if (set_recommended_config ()) +if (set_recommended_config (0 ))return error (_ ("could not configure '%s'" ), dir);if ((res =run_git ("fetch" ,"--quiet" ,"origin" ,NULL ))) {
@@ -484,6 +488,24 @@ static int cmd_register(int argc, const char **argv)return register_dir (); } +static int cmd_reconfigure (int argc,const char **argv) +{ +struct option options[] = { +OPT_END (), + }; +const char *const usage[] = { +N_ ("scalar reconfigure [<enlistment>]" ), +NULL + }; + + argc =parse_options (argc, argv,NULL , options, + usage,0 ); + +setup_enlistment_directory (argc, argv, usage, options,NULL ); + +return set_recommended_config (1 ); +} +static int cmd_run (int argc,const char **argv) {struct option options[] = {
@@ -620,6 +642,7 @@ static struct { { "register", cmd_register }, { "unregister", cmd_unregister }, { "run", cmd_run }, + { "reconfigure", cmd_reconfigure }, { NULL, NULL}, };
contrib/scalar/scalar.txt +8 -0
@@ -13,6 +13,7 @@ scalar list scalar register [<enlistment>] scalar unregister [<enlistment>] scalar run ( all | config | commit-graph | fetch | loose-objects | pack-files ) [<enlistment>] +scalar reconfigure <enlistment> DESCRIPTION -----------
@@ -117,6 +118,13 @@ opinionated default settings that make Git work more efficiently with large repositories. As this task is run as part of `scalar clone` automatically, explicit invocations of this task are rarely needed. +Reconfigure +~~~~~~~~~~~ + +After a Scalar upgrade, or when the configuration of a Scalar enlistment +was somehow corrupted or changed by mistake, this subcommand allows to +reconfigure the enlistment. + SEE ALSO -------- linkgit:git-clone[1], linkgit:git-maintenance[1].
contrib/scalar/t/t9099-scalar.sh +8 -0
@@ -65,4 +65,12 @@ test_expect_success 'scalar clone' ' ) ' +test_expect_success 'scalar reconfigure' ' + git init one/src && + scalar register one && + git -C one/src config core.preloadIndex false && + scalar reconfigure one && + test true = "$(git -C one/src config core.preloadIndex)" +' + test_done
Signature · 1 line
gitgitgadget
PATCH v8 15/17 scalar: teach 'reconfigure' to optionally handle all registered enlistments
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
After a Scalar upgrade, it can come in really handy if there is an easy way to reconfigure all Scalar enlistments. This new option offers this functionality.
--- contrib/scalar/scalar.c | 61 ++++++++++++++++++++++++++++++-- contrib/scalar/scalar.txt | 9 +++-- contrib/scalar/t/t9099-scalar.sh | 3 ++ 3 files changed, 67 insertions(+), 6 deletions(-)
contrib/scalar/scalar.c +58 -3
@@ -488,22 +488,77 @@ static int cmd_register(int argc, const char **argv)return register_dir (); } +static int get_scalar_repos (const char *key,const char *value,void *data) +{ +struct string_list *list = data; + +if (!strcmp (key,"scalar.repo" )) +string_list_append (list, value); + +return 0 ; +} +static int cmd_reconfigure (int argc,const char **argv) { +int all =0 ;struct option options[] = { +OPT_BOOL ('a' ,"all" , &all, +N_ ("reconfigure all registered enlistments" )),OPT_END (), };const char *const usage[] = { -N_ ("scalar reconfigure [<enlistment>]" ), +N_ ("scalar reconfigure [--all | <enlistment>]" ),NULL }; +struct string_list scalar_repos =STRING_LIST_INIT_DUP ; +int i, res =0 ; +struct repository r = {NULL }; +struct strbuf commondir =STRBUF_INIT , gitdir =STRBUF_INIT ; argc =parse_options (argc, argv,NULL , options, usage,0 ); -setup_enlistment_directory (argc, argv, usage, options,NULL ); +if (!all) { +setup_enlistment_directory (argc, argv, usage, options,NULL ); + +return set_recommended_config (1 ); + } + +if (argc >0 ) +usage_msg_opt (_ ("--all or <enlistment>, but not both" ), + usage, options); + +git_config (get_scalar_repos, &scalar_repos); -return set_recommended_config (1 ); +for (i =0 ; i < scalar_repos.nr; i++) { +const char *dir = scalar_repos.items[i].string; + +strbuf_reset (&commondir); +strbuf_reset (&gitdir); + +if (chdir (dir) <0 ) { +warning_errno (_ ("could not switch to '%s'" ), dir); + res =-1 ; + }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 ; + } + } + +string_list_clear (&scalar_repos,1 ); +strbuf_release (&commondir); +strbuf_release (&gitdir); + +return res; }static int cmd_run (int argc,const char **argv)
contrib/scalar/scalar.txt +6 -3
@@ -13,7 +13,7 @@ scalar list scalar register [<enlistment>] scalar unregister [<enlistment>] scalar run ( all | config | commit-graph | fetch | loose-objects | pack-files ) [<enlistment>] -scalar reconfigure <enlistment> +scalar reconfigure [ --all | <enlistment> ] DESCRIPTION -----------
@@ -32,8 +32,8 @@ an existing Git worktree with Scalar whose name is not `src`, the enlistment will be identical to the worktree. The `scalar` command implements various subcommands, and different options -depending on the subcommand. With the exception of `clone` and `list`, all -subcommands expect to be run in an enlistment. +depending on the subcommand. With the exception of `clone`, `list` and +`reconfigure --all`, all subcommands expect to be run in an enlistment. COMMANDS --------
@@ -125,6 +125,9 @@ After a Scalar upgrade, or when the configuration of a Scalar enlistment was somehow corrupted or changed by mistake, this subcommand allows to reconfigure the enlistment. +With the `--all` option, all enlistments currently registered with Scalar +will be reconfigured. Use this option after each Scalar upgrade. + SEE ALSO -------- linkgit:git-clone[1], linkgit:git-maintenance[1].
contrib/scalar/t/t9099-scalar.sh +3 -0
@@ -70,6 +70,9 @@ test_expect_success 'scalar reconfigure' ' scalar register one && git -C one/src config core.preloadIndex false && scalar reconfigure one && + test true = "$(git -C one/src config core.preloadIndex)" && + git -C one/src config core.preloadIndex false && + scalar reconfigure -a && test true = "$(git -C one/src config core.preloadIndex)" '
Signature · 1 line
gitgitgadget
PATCH v8 16/17 scalar: implement the `delete` command
In reply to Johannes Schindelin via GitGitGadgetFrom: Matthew John Cheetham <[email protected]>
Delete an enlistment by first unregistering the repository and then deleting the enlistment directory (usually the directory containing the worktree src/ directory).
On Windows, if the current directory is inside the enlistment's directory, change to the parent of the enlistment directory, to allow us to delete the enlistment (directories used by processes e.g. as current working directories cannot be deleted on Windows).
--- contrib/scalar/scalar.c | 63 ++++++++++++++++++++++++++++++++ contrib/scalar/scalar.txt | 8 ++++ contrib/scalar/t/t9099-scalar.sh | 9 +++++ 3 files changed, 80 insertions(+)
contrib/scalar/scalar.c +63 -0
@@ -8,6 +8,8 @@#include "config.h" #include "run-command.h" #include "refs.h" +#include "dir.h" +#include "packfile.h" /* * Remove the deepest subdirectory in the provided path string. Path must not
@@ -328,6 +330,33 @@ static char *remote_default_branch(const char *url)return NULL ; } +static int delete_enlistment (struct strbuf *enlistment) +{ +#ifdef WIN32 +struct strbuf parent =STRBUF_INIT ; +#endif + +if (unregister_dir ()) +die (_ ("failed to unregister repository" )); + +#ifdef WIN32 +/* +* Change the current directory to one outside of the enlistment so +* that we may delete everything underneath it. +*/ +strbuf_addbuf (&parent, enlistment); +strbuf_parent_directory (&parent); +if (chdir (parent.buf) <0 ) +die_errno (_ ("could not switch to '%s'" ), parent.buf); +strbuf_release (&parent); +#endif + +if (remove_dir_recursively (enlistment,0 )) +die (_ ("failed to delete enlistment directory" )); + +return 0 ; +} +static int cmd_clone (int argc,const char **argv) {const char *branch =NULL ;
@@ -688,6 +717,39 @@ static int cmd_unregister(int argc, const char **argv)return unregister_dir (); } +static int cmd_delete (int argc,const char **argv) +{ +char *cwd =xgetcwd (); +struct option options[] = { +OPT_END (), + }; +const char *const usage[] = { +N_ ("scalar delete <enlistment>" ), +NULL + }; +struct strbuf enlistment =STRBUF_INIT ; +int res =0 ; + + argc =parse_options (argc, argv,NULL , options, + usage,0 ); + +if (argc !=1 ) +usage_with_options (usage, options); + +setup_enlistment_directory (argc, argv, usage, options, &enlistment); + +if (dir_inside_of (cwd, enlistment.buf) >=0 ) + res =error (_ ("refusing to delete current working directory" )); +else { +close_object_store (the_repository->objects); + res =delete_enlistment (&enlistment); + } +strbuf_release (&enlistment); +free (cwd); + +return res; +} + static struct { const char *name; int (*fn)(int, const char **);
@@ -698,6 +760,7 @@ static struct { { "unregister", cmd_unregister }, { "run", cmd_run }, { "reconfigure", cmd_reconfigure }, + { "delete", cmd_delete }, { NULL, NULL}, };
contrib/scalar/scalar.txt +8 -0
@@ -14,6 +14,7 @@ scalar register [<enlistment>] scalar unregister [<enlistment>] scalar run ( all | config | commit-graph | fetch | loose-objects | pack-files ) [<enlistment>] scalar reconfigure [ --all | <enlistment> ] +scalar delete <enlistment> DESCRIPTION -----------
@@ -128,6 +129,13 @@ reconfigure the enlistment. With the `--all` option, all enlistments currently registered with Scalar will be reconfigured. Use this option after each Scalar upgrade. +Delete +~~~~~~ + +delete <enlistment>:: + This subcommand lets you delete an existing Scalar enlistment from your + local file system, unregistering the repository. + SEE ALSO -------- linkgit:git-clone[1], linkgit:git-maintenance[1].
contrib/scalar/t/t9099-scalar.sh +9 -0
@@ -76,4 +76,13 @@ test_expect_success 'scalar reconfigure' ' test true = "$(git -C one/src config core.preloadIndex)" ' +test_expect_success 'scalar delete without enlistment shows a usage' ' + test_expect_code 129 scalar delete +' + +test_expect_success 'scalar delete with enlistment' ' + scalar delete cloned && + test_path_is_missing cloned +' + test_done
Signature · 1 line
gitgitgadget
PATCH v8 17/17 scalar: implement the `version` command
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
The .NET version of Scalar has a version command. This was necessary because it was versioned independently of Git.
Since Scalar is now tightly coupled with Git, it does not make sense for them to show different versions. Therefore, it shows the same output as git version. For backwards-compatibility with the .NET version, scalar version prints to stderr, though (git version prints to stdout instead).
--- contrib/scalar/scalar.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
contrib/scalar/scalar.c +39 -0
@@ -10,6 +10,7 @@#include "refs.h" #include "dir.h" #include "packfile.h" +#include "help.h" /* * Remove the deepest subdirectory in the provided path string. Path must not
@@ -357,6 +358,15 @@ static int delete_enlistment(struct strbuf *enlistment)return 0 ; } +/* +* Dummy implementation; Using `get_version_info()` would cause a link error +* without this. +*/ +void load_builtin_commands (const char *prefix,struct cmdnames *cmds) +{ +die ("not implemented" ); +} +static int cmd_clone (int argc,const char **argv) {const char *branch =NULL ;
@@ -750,6 +760,34 @@ static int cmd_delete(int argc, const char **argv)return res; } +static int cmd_version (int argc,const char **argv) +{ +int verbose =0 , build_options =0 ; +struct option options[] = { +OPT__VERBOSE (&verbose,N_ ("include Git version" )), +OPT_BOOL (0 ,"build-options" , &build_options, +N_ ("include Git's build options" )), +OPT_END (), + }; +const char *const usage[] = { +N_ ("scalar verbose [-v | --verbose] [--build-options]" ), +NULL + }; +struct strbuf buf =STRBUF_INIT ; + + argc =parse_options (argc, argv,NULL , options, + usage,0 ); + +if (argc !=0 ) +usage_with_options (usage, options); + +get_version_info (&buf, build_options); +fprintf (stderr,"%s\n" , buf.buf); +strbuf_release (&buf); + +return 0 ; +} + static struct { const char *name; int (*fn)(int, const char **);
@@ -761,6 +799,7 @@ static struct { { "run", cmd_run }, { "reconfigure", cmd_reconfigure }, { "delete", cmd_delete }, + { "version", cmd_version }, { NULL, NULL}, };
Signature · 1 line
gitgitgadget
Quoted 4 lines · Johannes Schindelin via GitGitGadget
tl;dr: This series contributes the core part of the Scalar command to the Git project. This command provides an opinionated way to create and configure Git repositories with a focus on very large repositories.
I thought after
https://lore.kernel.org/git/[email protected]/
that you'd update merge.renames to true on what is now patch 7. Did you end up changing your mind, or was this overlooked?
Other than that, this round looks good to me. (I have no opinion on the build system integration, other than that I like it being optional and not installed by default.)
Hi Elijah,
Quoted 8 lines · Elijah Newren
Quoted 5 lines · Johannes Schindelin via GitGitGadget
tl;dr: This series contributes the core part of the Scalar command to the Git project. This command provides an opinionated way to create and configure Git repositories with a focus on very large repositories.
Quoted 4 lines
I thought after
https://lore.kernel.org/git/[email protected]/
that you'd update merge.renames to true on what is now patch 7. Did you end up changing your mind, or was this overlooked?
Oops! Thank you so much for the reminder.
Will fix. I do not plan on sending out a new iteration for a few more days because I do not want to send lots of patches to the list right now, reviewer bandwidth seems to be stretched quite a bit already.
Quoted 3 lines
Other than that, this round looks good to me. (I have no opinion on the build system integration, other than that I like it being optional and not installed by default.)
Yes, I very much wanted to keep this optional and as well-encapsulated as possible for the moment. (Hence the way it integrates with Git's build process.)
Thank you for chiming in!
Ciao,
Dscho
Quoted 17 lines · Johannes Schindelin
Hi Elijah,
Quoted 8 lines · Elijah Newren
Quoted 5 lines · Johannes Schindelin via GitGitGadget
tl;dr: This series contributes the core part of the Scalar command to the Git project. This command provides an opinionated way to create and configure Git repositories with a focus on very large repositories.
Quoted 4 lines
I thought after
https://lore.kernel.org/git/[email protected]/
that you'd update merge.renames to true on what is now patch 7. Did you end up changing your mind, or was this overlooked?
Quoted 5 lines
Oops! Thank you so much for the reminder.
Will fix. I do not plan on sending out a new iteration for a few more days because I do not want to send lots of patches to the list right now, reviewer bandwidth seems to be stretched quite a bit already.
Bandwidth which is further stretched by continuing to send updates to this topic while ignoring outstanding feedback.
I.e. "seen" being broken now due to a merger of this topic and another topic of mine, which as noted in [1] is really just revealing an existing breakage in this topic, which I sent you an unresponded-to patch to fix almost a month ago.
Quoted 9 lines
Quoted 3 lines
Other than that, this round looks good to me. (I have no opinion on the build system integration, other than that I like it being optional and not installed by default.)
Yes, I very much wanted to keep this optional and as well-encapsulated as possible for the moment. (Hence the way it integrates with Git's build process.)
Thank you for chiming in!
Whatever disagreement we have about the particulars of how scalar lands in-tree is one thing, and I'd be the first to admit that some of the stuff I've been suggesting is just my opinion.
But I've also been pointing out in reviews of this series (all/most of which you've ignored) that there's specific things that are categorically broken in this series, and clearly not working as you intend them to work. And you're simply ignoring those reports.
One of those is that your topic here changes CI in in a way that you clearly didn't intend, and which is an emergent unintended effect of how you're approaching this scalar integration.
I.e. the compile-only CI targets now doing tests as a result, which is broken, and the combination of that and my CI topic revealed that breakage.
Anyway, as noted in [2] I was hoping to leave this whole scalar thing behind, since I got tired of those reports/suggestions being ignored. I'm only replying here again because it's clear that you don't understand some of the things this scalar topic breaks/changes, and the only reason you wouldn't understand that is because you've been ignoring specific reports/patches-on top that address those breakages.
1. https://lore.kernel.org/git/[email protected]/
2. https://lore.kernel.org/git/[email protected]/
Hi Ævar,
Quoted 17 lines · Ævar Arnfjörð Bjarmason
Quoted 15 lines · Johannes Schindelin
Quoted 12 lines · Elijah Newren
Quoted 5 lines · Johannes Schindelin via GitGitGadget
tl;dr: This series contributes the core part of the Scalar command to the Git project. This command provides an opinionated way to create and configure Git repositories with a focus on very large repositories.
I thought after
https://lore.kernel.org/git/[email protected]/
that you'd update merge.renames to true on what is now patch 7. Did you end up changing your mind, or was this overlooked?
Quoted 8 lines
Quoted 5 lines
Oops! Thank you so much for the reminder.
Will fix. I do not plan on sending out a new iteration for a few more days because I do not want to send lots of patches to the list right now, reviewer bandwidth seems to be stretched quite a bit already.
Bandwidth which is further stretched by continuing to send updates to this topic while ignoring outstanding feedback.
The feedback you are referring to is probably the repeated demand to integrate Scalar deeply into Git's build process.
As I have tired of replying, it is not the time for that yet.
Repeating that demand does not make it more sensible, nor does it magically make it the right time.
Nor is it credible to call the build "broken" when it does what it is supposed to do, thank you very much.
Ciao,
Johannes
Quoted 28 lines · Johannes Schindelin
Quoted 17 lines · Ævar Arnfjörð Bjarmason
Quoted 15 lines · Johannes Schindelin
Quoted 12 lines · Elijah Newren
Quoted 5 lines · Johannes Schindelin via GitGitGadget
tl;dr: This series contributes the core part of the Scalar command to the Git project. This command provides an opinionated way to create and configure Git repositories with a focus on very large repositories.
I thought after
https://lore.kernel.org/git/[email protected]/
that you'd update merge.renames to true on what is now patch 7. Did you end up changing your mind, or was this overlooked?Quoted 8 lines
Quoted 5 lines
Oops! Thank you so much for the reminder.
Will fix. I do not plan on sending out a new iteration for a few more days because I do not want to send lots of patches to the list right now, reviewer bandwidth seems to be stretched quite a bit already.
Bandwidth which is further stretched by continuing to send updates to this topic while ignoring outstanding feedback.
Quoted 7 lines
The feedback you are referring to is probably the repeated demand to integrate Scalar deeply into Git's build process.
As I have tired of replying, it is not the time for that yet.
Repeating that demand does not make it more sensible, nor does it magically make it the right time.
I'm not repeating that demand. I clearly also think the approach you're insisting picking isn't a good one, but let's leave that aside.
What I'm referring to with "I've also been pointing out" in the context you elided is that if you:
1. Check out your topic
2. Apply my proposed patch on top (a newer version than on-list is in avar/scalar-move-build-from-contrib-2 in my GH fork)
3. Push both to CI
4. Diff the two logs of the runs (or just manually click through and inspect them)
You'd have seen before sending your version of the CI integration that the difference in behavior that started with your version of the topic was particular to the contrib/scalar/ integration, but not the top-level Makefile integration. I.e. adding the scalar tests to the previously build-only jobs.
I've been noting that as clearly as I'm able to in numerous past exchanges. You've either ignored those reports, or like here, selectivtely replied only to parts of what I've told you.
I.e. something like "I'm not going 100% for the approach you suggest". Sure, I'm not saying you have to. But I also noted that the patch with that suggested approach can be considered a bug report against your series.
The reason that patch isn't split into two things, one fixing all the issues I noticed, and another implementing some "alternate build" approach is that I found that to be impossible to do.
Those issues are all particular to emergent effects of the build integration you're choosing to go with.
E.g. in the case of "seen" being broken the CI simply runs "make test" as it did before, and scalar integrates into that, and you can run that target without having built anything already.
Quoted 2 lines
Nor is it credible to call the build "broken" when it does what it is supposed to do, thank you very much.
Here's specific commands showing that it's broken.
On your version (I've got v7 locally, but the same is true of v8):
$ make clean; make -C contrib/scalar test
[...]
CC hook.o
CC version.o
CC help.o
AR libgit.a
make[1]: Leaving directory '/home/avar/g/git'
SUBDIR ../..
make[1]: Entering directory '/home/avar/g/git'
* new link flags
CC contrib/scalar/scalar.o
LINK contrib/scalar/scalar
make[1]: Leaving directory '/home/avar/g/git'
make -C t
make[1]: Entering directory '/home/avar/g/git/contrib/scalar/t'
*** prove ***
error: GIT-BUILD-OPTIONS missing (has Git been built?).
t9099-scalar.sh .. Dubious, test returned 1 (wstat 256, 0x100)
No subtests run Test Summary Report
-------------------
t9099-scalar.sh (Wstat: 256 Tests: 0 Failed: 0)
Non-zero exit status: 1
Parse errors: No plan found in TAP output
On the patch I proposed to apply on top:
$ make clean; make test T=t9099-scalar.sh
[...]
CC t/helper/test-xml-encode.o
GEN bin-wrappers/git
GEN bin-wrappers/git-receive-pack
GEN bin-wrappers/git-shell
GEN bin-wrappers/git-upload-archive
GEN bin-wrappers/git-upload-pack
GEN bin-wrappers/scalar
GEN bin-wrappers/git-cvsserver
GEN bin-wrappers/test-fake-ssh
GEN bin-wrappers/test-tool
LINK t/helper/test-fake-ssh
LINK t/helper/test-tool
make -C t/ all
make[1]: Entering directory '/home/avar/g/git/t'
rm -f -r 'test-results'
*** prove ***
t9099-scalar.sh .. ok
All tests successful.You might correctly note that this doesn't work either on that version (or for any other existing test in t/):
$ make clean >/dev/null; make -C t T=t9099-scalar.sh
GIT_VERSION = 2.34.GIT-dev
make: Entering directory '/home/avar/g/git/t'
rm -f -r 'test-results'
*** prove ***
error: GIT-BUILD-OPTIONS missing (has Git been built?).
t9099-scalar.sh .. Dubious, test returned 1 (wstat 256, 0x100)
No subtests run Test Summary Report
-------------------
t9099-scalar.sh (Wstat: 256 Tests: 0 Failed: 0)
Non-zero exit status: 1
Parse errors: No plan found in TAP output
Which is true, but that's not broken because it's not attempting to build the top-level via some incomplete integration, but your version is.
Hi Ævar,
Quoted 12 lines · Ævar Arnfjörð Bjarmason
[...]
$ make clean; make -C contrib/scalar test [...] CC hook.o CC version.o CC help.o AR libgit.a make[1]: Leaving directory '/home/avar/g/git' SUBDIR ../.. make[1]: Entering directory '/home/avar/g/git' * new link flags CC contrib/scalar/scalar.o
Quoted 8 lines
LINK contrib/scalar/scalar
make[1]: Leaving directory '/home/avar/g/git'
make -C t
make[1]: Entering directory '/home/avar/g/git/contrib/scalar/t'
*** prove ***
error: GIT-BUILD-OPTIONS missing (has Git been built?).
t9099-scalar.sh .. Dubious, test returned 1 (wstat 256, 0x100)
No subtests runThat's cute. You seem to have missed that this is contrib/? The assumption of pretty much _everything_ in there is that Git was already built.
Try this at home: make clean && make -C contrib/subtree/ test
Yep. It "fails" in the same way. "has Git been built?".
So if that was all the evidence in favor of that misinformation "Scalar's build is broken! Broken, broken, BROKEN!", I think we can now let it rest. At last.
Ciao,
Johannes
Quoted 29 lines · Johannes Schindelin
Hi Ævar,
Quoted 12 lines · Ævar Arnfjörð Bjarmason
[...]
$ make clean; make -C contrib/scalar test [...] CC hook.o CC version.o CC help.o AR libgit.a make[1]: Leaving directory '/home/avar/g/git' SUBDIR ../.. make[1]: Entering directory '/home/avar/g/git' * new link flags CC contrib/scalar/scalar.oQuoted 8 lines
LINK contrib/scalar/scalar make[1]: Leaving directory '/home/avar/g/git' make -C t make[1]: Entering directory '/home/avar/g/git/contrib/scalar/t' *** prove *** error: GIT-BUILD-OPTIONS missing (has Git been built?). t9099-scalar.sh .. Dubious, test returned 1 (wstat 256, 0x100) No subtests runThat's cute. You seem to have missed that this is
contrib/? The assumption of pretty much _everything_ in there is that Git was already built.
Quoted 7 lines
Try this at home:
make clean && make -C contrib/subtree/ testYep. It "fails" in the same way. "has Git been built?".
So if that was all the evidence in favor of that misinformation "Scalar's build is broken! Broken, broken, BROKEN!", I think we can now let it rest. At last.
No, it doesn't fail in the same way. Really, it seems like you're either not fully reading through E-Mails before replying, or entirely misunderstanding what I'm saying. I highlighted the difference in "[...]that's not broken because[...]" below the context you're quoting.
I'm specifically pointing out the difference between how these act:
make clean; make -C contrib/subtree/ test
make clean; make -C t
Which fail right away without trying to build anything, and how your:
make clean; make -C contrib/scalar test
Won't fail right away, but get most of the way towards building what it needs at the top-level.
So yes, I fully agree with your contrib/subtree example, but it's making my argument for me.
As I pointed out in the just-sent [1] the main issue is that your latest iteration added "make -C contrib/subtree/ test" in the wrong place, and we thus ended up running those tests in places we didn't intend. The main conflict is that semantic conflict.
But that failure also highlighted that contrib/scalar/Makefile will run a build of the top-level C code, only to error out with "GIT-BUILD-OPTIONS".
That specifically is what I consider broken. It should either actually work and properly build its dependency, or not even try.
Either one would be fine in this context, it's the in-between that's clearly (to me at least) broken. Working software should either attempt a task and succeed, or not attempt the task at all.
Hi Ævar,
Quoted 35 lines · Ævar Arnfjörð Bjarmason
Quoted 33 lines · Johannes Schindelin
Hi Ævar,
Quoted 12 lines · Ævar Arnfjörð Bjarmason
[...]
$ make clean; make -C contrib/scalar test [...] CC hook.o CC version.o CC help.o AR libgit.a make[1]: Leaving directory '/home/avar/g/git' SUBDIR ../.. make[1]: Entering directory '/home/avar/g/git' * new link flags CC contrib/scalar/scalar.oQuoted 8 lines
LINK contrib/scalar/scalar make[1]: Leaving directory '/home/avar/g/git' make -C t make[1]: Entering directory '/home/avar/g/git/contrib/scalar/t' *** prove *** error: GIT-BUILD-OPTIONS missing (has Git been built?). t9099-scalar.sh .. Dubious, test returned 1 (wstat 256, 0x100) No subtests runThat's cute. You seem to have missed that this is
contrib/? The assumption of pretty much _everything_ in there is that Git was already built.Try this at home:
make clean && make -C contrib/subtree/ testYep. It "fails" in the same way. "has Git been built?".
Quoted 7 lines
Quoted 3 lines
So if that was all the evidence in favor of that misinformation "Scalar's build is broken! Broken, broken, BROKEN!", I think we can now let it rest. At last.
No, it doesn't fail in the same way. Really, it seems like you're either not fully reading through E-Mails before replying, or entirely misunderstanding what I'm saying.
That's really... fresh.
I told you half a dozen times that at this point, the build works well enough, and that what you keep insisting is a problem simply isn't.
Yes, you have to build Git before building Scalar.
Have you actually looked at the design of Scalar? What it does to allow working with large repositories?
_That_ is what counts.
That you are told to please build Git before running Scalar's tests is maybe a minor annoyance, but not worth the dozens of mails and the weeks of delay that you caused over this.
Ciao,
Johannes
v9 cover Upstreaming the Scalar command
In reply to Johannes Schindelin via GitGitGadgettl;dr: This series contributes the core part of the Scalar command to the Git project. This command provides a convenient way to clone/initialize very large repositories (think: monorepos).
Note: This patch series' focus is entirely on Scalar, on choosing sensible defaults and offering a delightful user experience around working with monorepos, and not about changing any existing paradigms for contrib/ (even if catching up on the mail thread is likely to give interested readers that false impression).
Changes since v8:
* The rebase on top of v2.34.0, which changed the default merge strategy to ORT, should have changed the default for merge.renames to true. This is now the case.
* Accommodate preemptively for ab/ci-updates which invalidates assumptions made by this patch series that would still hold true with v2.34.0 but are no longer valid in seen and would trigger CI build breakages.
Changes since v7:
* Clarified in the commit message why we cannot easily encapsulate the Scalar part of the CMake configuration in contrib/scalar/.
* Improved the README.md.
Changes since v6:
* Rebased on top of v2.34.0.
* Inserted a commit that adds contrib/scalar/README.md, containing the roadmap of what I have planned for Scalar.
* The Scalar test's definition of GIT_TEST_MAINT_SCHEDULER has been adjusted to accommodate for a change in v2.32.0..v2.34.0.
* The config setting defaults now include fetch.showForcedUpdates=false, which has been identified as helping with a performance issue in large repositories.
* To avoid mistaking the current patch series for being feature-complete enough to unleash onto end users, I moved the Makefile rules to build HTML/manual pages to a later patch series.
* The patch that adds support for -c <key>=<value> and -C <directory> was moved to its own add-on patch series: While it is obvious that those options are valuable to have, an open question is whether there are other "pre-command" options in git that would be useful, too, and I would like to postpone that discussion to that date.
* I added two patches that I had planned on keeping in an add-on patch series for later, to build and test Scalar as part of the CI. I am still not 100% certain that it is a good idea to do so already now, but let's see what the reviewers have to say.
Changes since v5:
* Fixed the commit message talking about make -C contrib/scalar/Makefile.
* Fixed the git ls-tree invocation suggested in the manual for scalar clone.
* Invoking make -C contrib/scalar, then changing a source file of libgit.a and then immediately invoking make -C contrib/scalar again will now implicitly rebuild libgit.a.
Changes since v4:
* scalar delete now refuses to delete anything if it was started from within the enlistment.
* scalar delete releases any handles to the object store before deleting the enlistment.
* The OBJECTS list in the Makefile will now include Scalar.
* scalar register now supports secondary worktrees, in addition to the primary worktree.
Changes since v3:
* Moved the "Changes since" section to the top, to make it easier to see what changed.
* Reworded the commit message of the first patch.
* Removed the [RFC] prefix because I did not hear any objections against putting this into contrib/.
Changes since v2:
* Adjusted the description of the list command in the manual page , as suggested by Bagas.
* Addressed two style nits in cmd_run().
* The documentation of git reconfigure -a was improved.
Changes since v1:
* A couple typos were fixed
* The code parsing the output of ls-remote was made more readable
* The indentation used in scalar.txt now consistently uses tabs
* We no longer hard-code core.bare = false when registering with Scalar
Background
==========
Microsoft invested a lot of effort into scaling Git to the needs of the Windows operating system source code. Based on the experience of the first approach, VFS for Git, the Scalar project was started. Scalar specifically has as its core goal to funnel all improvements into core Git.
The present
===========
The Scalar project provides a completely functional non-virtual experience for monorepos. But why stop there. The Scalar project was designed to be a self-destructing vehicle to allow those key concepts to be moved into core Git itself for the benefit of all. For example, partial clone, sparse-checkout, and scheduled background maintenance have already been upstreamed and removed from Scalar proper. This patch series provides a C-based implementation of the final remaining portions of the Scalar command. This will make it easier for users to experiment with the Scalar command. It will also make it substantially easier to experiment with moving functionality from Scalar into core Git, while maintaining backwards-compatibility for existing Scalar users.
The C-based Scalar has been shipped to Scalar users, and can be tested by any interested reader: https://github.com/microsoft/git/releases/ (it offers a Git for Windows installer, a macOS package and an Ubuntu package, Scalar has been included since v2.33.0.vfs.0.0).
Next steps
==========
Since there are existing Scalar users, I want to ensure backwards-compatibility with its existing command-line interface. Keeping that in mind, everything in this series is up for discussion.
I obviously believe that Scalar brings a huge benefit, and think that it would be ideal for all of Scalar's learnings to end up in git clone/git init/git maintenance eventually. It is also conceivable, however, that the scalar command could graduate to be a core part of Git at some stage in the future (such a decision would probably depend highly on users' feedback). See also the discussion about the architecture of Scalar
[https://lore.kernel.org/git/[email protected]/],
kicked off by Stolee.
On top of this patch series, I have lined up a few more:
1. Implement a scalar diagnose command.
2. Use the built-in FSMonitor (that patch series obviously needs to wait for FSMonitor to be integrated).
3. Modify the config machinery to be more generous about concurrent writes, say, to the user-wide config.
4. A few patches to optionally build and install scalar as part of a regular Git install (also teaching git help scalar to find the Scalar documentation
These are included in my vfs-with-scalar branch thicket
[https://github.com/dscho/git/commits/vfs-with-scalar]. On top of that, this branch thicket also includes patches I do not plan on upstreaming, mainly because they are too specific either to VFS for Git, or they support Azure Repos (which does not offer partial clones but speaks the GVFS protocol, which can be used to emulate partial clones).
One other thing is very interesting about that vfs-with-scalar branch
thicket: it contains a GitHub workflow which will run Scalar's quite extensive Functional Tests suite. This test suite is quite comprehensive and caught us a lot of bugs in the past, not only in the Scalar code, but also core Git.
Epilogue
========
Now, to address some questions that I imagine every reader has who made it this far:
* Why not put the Scalar functionality directly into core Git, even a built-in? I wanted to provide an easy way for Git contributors to "play with" Scalar, without forcing a new top-level command into Git.
* Why implement the Scalar command in the Git code base? Apart from simplifying Scalar maintenance in the Microsoft port of Git, the tight version coupling between Git and Scalar reduces the maintenance burden even further. Besides, I believe that it will make it much easier to shift functionality from Scalar into core Git, once we took the hurdle of accepting the Scalar code into the code base.
* Why contribute Scalar to the Git project? We are biased, of course, yet our data-driven approach provides evidence that Scalar helps handling huge repositories with ease. By contributing it to the core Git project, we are able to share it with more users, especially some users who do not want to install Microsoft's fork of Git. We also hope that a lot of Scalar (maybe all of it) will end up in core Git, to benefit even more users.
Derrick Stolee (4):
scalar: 'register' sets recommended config and starts maintenance
scalar: 'unregister' stops background maintenance
scalar: implement 'scalar list'
scalar: implement the run command
Johannes Schindelin (12):
scalar: add a README with a roadmap
scalar: create a rudimentary executable
scalar: start documenting the command
scalar: create test infrastructure
cmake: optionally build scalar, too
ci: also run the scalar tests
scalar: let 'unregister' handle a deleted enlistment directory
gracefully
scalar: implement the clone subcommand
scalar: teach 'clone' to support the --single-branch option
scalar: allow reconfiguring an existing enlistment
scalar: teach 'reconfigure' to optionally handle all registered
enlistments
scalar: implement the version command
Matthew John Cheetham (1):
scalar: implement the delete command
.github/workflows/main.yml | 15 + Makefile | 9 + ci/run-build-and-tests.sh | 2 + ci/run-test-slice.sh | 5 + contrib/buildsystems/CMakeLists.txt | 14 + contrib/scalar/.gitignore | 2 + contrib/scalar/Makefile | 45 ++ contrib/scalar/README.md | 82 +++ contrib/scalar/scalar.c | 826 ++++++++++++++++++++++++++++ contrib/scalar/scalar.txt | 145 +++++ contrib/scalar/t/Makefile | 78 +++ contrib/scalar/t/t9099-scalar.sh | 88 +++ 12 files changed, 1311 insertions(+) create mode 100644 contrib/scalar/.gitignore create mode 100644 contrib/scalar/Makefile create mode 100644 contrib/scalar/README.md create mode 100644 contrib/scalar/scalar.c create mode 100644 contrib/scalar/scalar.txt create mode 100644 contrib/scalar/t/Makefile create mode 100755 contrib/scalar/t/t9099-scalar.sh
base-commit: cd3e606211bb1cf8bc57f7d76bab98cc17a150bc
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1005%2Fdscho%2Fscalar-the-beginning-v9
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1005/dscho/scalar-the-beginning-v9
Pull-Request: https://github.com/gitgitgadget/git/pull/1005
Range-diff vs v8:
1: 3a2e28275f1 = 1: 3a2e28275f1 scalar: add a README with a roadmap 2: 50160d61a41 = 2: 50160d61a41 scalar: create a rudimentary executable 3: 74cd6410931 = 3: 74cd6410931 scalar: start documenting the command 4: 37231a4dd07 = 4: 37231a4dd07 scalar: create test infrastructure 5: a39b9c81214 = 5: a39b9c81214 cmake: optionally build `scalar`, too 6: 8e3542e43f7 ! 6: 8c6762def30 ci: also run the `scalar` tests
@@ .github/workflows/main.yml: jobs:
- name: upload tracked files and build artifacts
## ci/run-build-and-tests.sh ##
-@@ ci/run-build-and-tests.sh: linux-gcc-4.8|pedantic)
- make test
- ;;
- esac
-+make -C contrib/scalar test
+@@ ci/run-build-and-tests.sh: esac
check_unignored_build_artifacts
++make && make -C contrib/scalar test
++
+ save_good_tree
## ci/run-test-slice.sh ##
@@ ci/run-test-slice.sh: make --quiet -C t T="$(cd t &&
7: 385abdb8d8e ! 7: 936ee0475ad scalar: 'register' sets recommended config and starts maintenance
@@ contrib/scalar/scalar.c
+ { "index.threads", "true" },
+ { "index.version", "4" },
+ { "merge.stat", "false" },
-+ { "merge.renames", "false" },
++ { "merge.renames", "true" },
+ { "pack.useBitmaps", "false" },
+ { "pack.useSparse", "true" },
+ { "receive.autoGC", "false" },
8: 64c6a75353e = 8: 09a15f86c3d scalar: 'unregister' stops background maintenance
9: f7fc1958b9e = 9: 42121a5764d scalar: let 'unregister' handle a deleted enlistment directory gracefully
10: fd2680bc945 = 10: 6afb2eb4163 scalar: implement 'scalar list'
11: 4966a43aad9 = 11: dd4e3a4b761 scalar: implement the `clone` subcommand
12: b00d68b37b0 = 12: abd9c8827cd scalar: teach 'clone' to support the --single-branch option
13: 771d826bbb1 = 13: 5601f82dbe1 scalar: implement the `run` command
14: a8b2d26a830 ! 14: 08e4f548aa8 scalar: allow reconfiguring an existing enlistment
@@ contrib/scalar/scalar.c: static int set_recommended_config(void)
- { "index.threads", "true" },
- { "index.version", "4" },
- { "merge.stat", "false" },
-- { "merge.renames", "false" },
+- { "merge.renames", "true" },
- { "pack.useBitmaps", "false" },
- { "pack.useSparse", "true" },
- { "receive.autoGC", "false" },
@@ contrib/scalar/scalar.c: static int set_recommended_config(void)
+ { "index.threads", "true", 1 },
+ { "index.version", "4", 1 },
+ { "merge.stat", "false", 1 },
-+ { "merge.renames", "false", 1 },
++ { "merge.renames", "true", 1 },
+ { "pack.useBitmaps", "false", 1 },
+ { "pack.useSparse", "true", 1 },
+ { "receive.autoGC", "false", 1 },
15: ca284ff34a2 = 15: 0cec6dbd2cb scalar: teach 'reconfigure' to optionally handle all registered enlistments
16: 9983eb8912c = 16: 835f1c79792 scalar: implement the `delete` command
17: 889f613ab18 = 17: 4ee1b701c7b scalar: implement the `version` commandSignature · 1 line
gitgitgadget
PATCH v9 1/17 scalar: add a README with a roadmap
From: Johannes Schindelin <[email protected]>
The Scalar command will be contributed incrementally, over a bunch of patch series. Let's document what Scalar is about, and then describe the patch series that are planned.
--- contrib/scalar/README.md | 82 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 contrib/scalar/README.md
contrib/scalar/README.md new file +82 -0
@@ -0,0 +1,82 @@ +# Scalar - an opinionated repository management tool + +Scalar is an add-on to Git that helps users take advantage of advanced +performance features in Git. Originally implemented in C# using .NET Core, +based on the learnings from the VFS for Git project, most of the techniques +developed by the Scalar project have been integrated into core Git already: + +* partial clone, +* commit graphs, +* multi-pack index, +* sparse checkout (cone mode), +* scheduled background maintenance, +* etc + +This directory contains the remaining parts of Scalar that are not (yet) in +core Git. + +## Roadmap + +The idea is to populate this directory via incremental patch series and +eventually move to a top-level directory next to `gitk-git/` and to `git-gui/`. The +current plan involves the following patch series: + +- `scalar-the-beginning`: The initial patch series which sets up + `contrib/scalar/` and populates it with a minimal `scalar` command that + demonstrates the fundamental ideas. + +- `scalar-c-and-C`: The `scalar` command learns about two options that can be + specified before the command, `-c <key>=<value>` and `-C <directory>`. + +- `scalar-diagnose`: The `scalar` command is taught the `diagnose` subcommand. + +- `scalar-and-builtin-fsmonitor`: The built-in FSMonitor is enabled in `scalar + register` and in `scalar clone`, for an enormous performance boost when + working in large worktrees. This patch series necessarily depends on Jeff + Hostetler's FSMonitor patch series to be integrated into Git. + +- `scalar-gentler-config-locking`: Scalar enlistments are registered in the + user's Git config. This usually does not represent any problem because it is + rare for a user to register an enlistment. However, in Scalar's functional + tests, Scalar enlistments are created galore, and in parallel, which can lead + to lock contention. This patch series works around that problem by re-trying + to lock the config file in a gentle fashion. + +- `scalar-extra-docs`: Add some extensive documentation that has been written + in the original Scalar project (all subject to discussion, of course). + +- `optionally-install-scalar`: Now that Scalar is feature (and documentation) + complete and is verified in CI builds, let's offer to install it. + +- `move-scalar-to-toplevel`: Now that Scalar is complete, let's move it next to + `gitk-git/` and to `git-gui/`, making it a top-level command. + +The following two patch series exist in Microsoft's fork of Git and are +publicly available. There is no current plan to upstream them, not because I +want to withhold these patches, but because I don't think the Git community is +interested in these patches. + +There are some interesting ideas there, but the implementation is too specific +to Azure Repos and/or VFS for Git to be of much help in general (and also: my +colleagues tried to upstream some patches already and the enthusiasm for +integrating things related to Azure Repos and VFS for Git can be summarized in +very, very few words). + +These still exist mainly because the GVFS protocol is what Azure Repos has +instead of partial clone, while Git is focused on improving partial clone: + +- `scalar-with-gvfs`: The primary purpose of this patch series is to support + existing Scalar users whose repositories are hosted in Azure Repos (which + does not support Git's partial clones, but supports its predecessor, the GVFS + protocol, which is used by Scalar to emulate the partial clone). + + Since the GVFS protocol will never be supported by core Git, this patch + series will remain in Microsoft's fork of Git. + +- `run-scalar-functional-tests`: The Scalar project developed a quite + comprehensive set of integration tests (or, "Functional Tests"). They are the + sole remaining part of the original C#-based Scalar project, and this patch + adds a GitHub workflow that runs them all. + + Since the tests partially depend on features that are only provided in the + `scalar-with-gvfs` patch series, this patch cannot be upstreamed.
Signature · 1 line
gitgitgadget
PATCH v9 2/17 scalar: create a rudimentary executable
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
The idea of Scalar (https://github.com/microsoft/scalar), and before that, of VFS for Git, has always been to prove that Git _can_ scale, and to upstream whatever strategies have been demonstrated to help.
With this patch, we start the journey from that C# project to move what is left to Git's own contrib/ directory, reimplementing it in pure C, with the intention to facilitate integrating the functionality into core Git all while maintaining backwards-compatibility for existing Scalar users (which will be much easier when both live in the same worktree). It has always been the plan to contribute all of the proven strategies back to core Git.
For example, while the virtual filesystem provided by VFS for Git helped the team developing the Windows operating system to move onto Git, while trying to upstream it we realized that it cannot be done: getting the virtual filesystem to work (which we only managed to implement fully on Windows, but not on, say, macOS or Linux), and the required server-side support for the GVFS protocol, made this not quite feasible.
The Scalar project learned from that and tackled the problem with different tactics: instead of pretending to Git that the working directory is fully populated, it _specifically_ teaches Git about partial clone (which is based on VFS for Git's cache server), about sparse checkout (which VFS for Git tried to do transparently, in the file system layer), and regularly runs maintenance tasks to keep the repository in a healthy state.
With partial clone, sparse checkout and git maintenance having been upstreamed, there is little left that scalar.exe does which git.exe cannot do. One such thing is that scalar clone <url> will automatically set up a partial, sparse clone, and configure known-helpful settings from the start.
So let's bring this convenience into Git's tree.
The idea here is that you can (optionally) build Scalar via
make -C contrib/scalar/
This will build the scalar executable and put it into the contrib/scalar/ subdirectory.
The slightly awkward addition of the contrib/scalar/* bits to the top-level Makefile are actually really required: we want to link to libgit.a, which means that we will need to use the very same CFLAGS and LDFLAGS as the rest of Git.
An early development version of this patch tried to replicate all the conditional code in contrib/scalar/Makefile (e.g. NO_POLL) just like contrib/svn-fe/Makefile used to do before it was retired. It turned out to be quite the whack-a-mole game: the SHA-1-related flags, the flags enabling/disabling compat/poll/, compat/regex/, compat/win32mmap.c & friends depending on the current platform... To put it mildly: it was a major mess.
Instead, this patch makes minimal changes to the top-level Makefile so that the bits in contrib/scalar/ can be compiled and linked, and adds a contrib/scalar/Makefile that uses the top-level Makefile in a most minimal way to do the actual compiling.
Note: With this commit, we only establish the infrastructure, no Scalar functionality is implemented yet; We will do that incrementally over the next few commits.
--- Makefile | 9 +++++++++ contrib/scalar/.gitignore | 2 ++ contrib/scalar/Makefile | 34 ++++++++++++++++++++++++++++++++++ contrib/scalar/scalar.c | 36 ++++++++++++++++++++++++++++++++++++ 4 files changed, 81 insertions(+) create mode 100644 contrib/scalar/.gitignore create mode 100644 contrib/scalar/Makefile create mode 100644 contrib/scalar/scalar.c
Makefile +9 -0
@@ -2456,6 +2456,11 @@ OBJECTS += $(FUZZ_OBJS) ifndef NO_CURL OBJECTS += http.o http-walker.o remote-curl.o endif + +SCALAR_SOURCES := contrib/scalar/scalar.c +SCALAR_OBJECTS := $(SCALAR_SOURCES:c=o) +OBJECTS += $(SCALAR_OBJECTS) + .PHONY: objects objects: $(OBJECTS)
@@ -2589,6 +2594,10 @@ $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS $(GITLIBS $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \ $(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS) +contrib/scalar/scalar$X: $(SCALAR_OBJECTS) GIT-LDFLAGS $(GITLIBS) + $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \ + $(filter %.o,$^) $(LIBS) + $(LIB_FILE): $(LIB_OBJS) $(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^
contrib/scalar/.gitignore new file +2 -0
@@ -0,0 +1,2 @@ +/*.exe +/scalar
contrib/scalar/Makefile new file +34 -0
@@ -0,0 +1,34 @@ +QUIET_SUBDIR0 = +$(MAKE) -C # space to separate -C and subdir +QUIET_SUBDIR1 = + +ifneq ($(findstring s,$(MAKEFLAGS)),s) +ifndef V + QUIET_SUBDIR0 = +@subdir= + QUIET_SUBDIR1 = ;$(NO_SUBDIR) echo ' ' SUBDIR $$subdir; \ + $(MAKE) $(PRINT_DIR) -C $$subdir +else + export V +endif +endif + +all: + +include ../../config.mak.uname +-include ../../config.mak.autogen +-include ../../config.mak + +TARGETS = scalar$(X) scalar.o +GITLIBS = ../../common-main.o ../../libgit.a ../../xdiff/lib.a + +all: scalar$(X) + +$(GITLIBS): + $(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) $(subst ../../,,$@) + +$(TARGETS): $(GITLIBS) scalar.c + $(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) $(patsubst %,contrib/scalar/%,$@) + +clean: + $(RM) $(TARGETS) + +.PHONY: $(GITLIBS) all clean FORCE
contrib/scalar/scalar.c new file +36 -0
@@ -0,0 +1,36 @@ +/* +* The Scalar command-line interface. +*/ + +#include "cache.h" +#include "gettext.h" +#include "parse-options.h" + +static struct { +const char *name; +int (*fn)(int ,const char **); +} builtins[] = { + {NULL ,NULL }, +}; + +int cmd_main (int argc,const char **argv) +{ +struct strbuf scalar_usage =STRBUF_INIT ; +int i; + +if (argc >1 ) { + argv++; + argc--; + +for (i =0 ; builtins[i].name; i++) +if (!strcmp (builtins[i].name, argv[0 ])) +return !!builtins[i].fn (argc, argv); + } + +strbuf_addstr (&scalar_usage, +N_ ("scalar <command> [<options>]\n\nCommands:\n" )); +for (i =0 ; builtins[i].name; i++) +strbuf_addf (&scalar_usage,"\t%s\n" , builtins[i].name); + +usage (scalar_usage.buf); +}
Signature · 1 line
gitgitgadget
PATCH v9 3/17 scalar: start documenting the command
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
Let's build up the documentation for the Scalar command along with the patches that implement its functionality.
Note: To discourage the feature-incomplete documentation from being mistaken for the complete thing, we do not yet provide any way to build HTML or manual pages from the text file.
--- contrib/scalar/scalar.txt | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 contrib/scalar/scalar.txt
contrib/scalar/scalar.txt new file +38 -0
@@ -0,0 +1,38 @@ +scalar(1) +========= + +NAME +---- +scalar - an opinionated repository management tool + +SYNOPSIS +-------- +[verse] +scalar <command> [<options>] + +DESCRIPTION +----------- + +Scalar is an opinionated repository management tool. By creating new +repositories or registering existing repositories with Scalar, your Git +experience will speed up. Scalar sets advanced Git config settings, +maintains your repositories in the background, and helps reduce data sent +across the network. + +An important Scalar concept is the enlistment: this is the top-level directory +of the project. It usually contains the subdirectory `src/` which is a Git +worktree. This encourages the separation between tracked files (inside `src/`) +and untracked files, such as build artifacts (outside `src/`). When registering +an existing Git worktree with Scalar whose name is not `src`, the enlistment +will be identical to the worktree. + +The `scalar` command implements various subcommands, and different options +depending on the subcommand. + +SEE ALSO +-------- +linkgit:git-maintenance[1]. + +Scalar +--- +Associated with the linkgit:git[1] suite
Signature · 1 line
gitgitgadget
PATCH v9 4/17 scalar: create test infrastructure
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
To test the Scalar command, create a test script in contrib/scalar/t that is executed as make -C contrib/scalar test. Since Scalar has no meaningful capabilities yet, the only test is rather simple. We will add more tests in subsequent commits that introduce corresponding, new functionality.
Note: This test script is intended to test scalar only lightly, even after all of the functionality is implemented.
A more comprehensive functional (or: integration) test suite can be found at https://github.com/microsoft/scalar; It is used in the workflow
https://github.com/microsoft/git/blob/HEAD/.github/workflows/scalar-functional-tests.yml
in Microsoft's Git fork. This test suite performs end-to-end tests with a real remote repository, and is run as part of the regular CI and PR builds in that fork.
Since those tests require some functionality supported only by Microsoft's Git fork ("GVFS protocol"), there is no intention to port that fuller test suite to contrib/scalar/.
--- contrib/scalar/Makefile | 17 +++++-- contrib/scalar/t/Makefile | 78 ++++++++++++++++++++++++++++++++ contrib/scalar/t/t9099-scalar.sh | 17 +++++++ 3 files changed, 109 insertions(+), 3 deletions(-) create mode 100644 contrib/scalar/t/Makefile create mode 100755 contrib/scalar/t/t9099-scalar.sh
contrib/scalar/Makefile +14 -3
@@ -3,6 +3,7 @@ QUIET_SUBDIR1 = ifneq ($(findstring s,$(MAKEFLAGS)),s) ifndef V + QUIET_GEN = @echo ' ' GEN $@; QUIET_SUBDIR0 = +@subdir= QUIET_SUBDIR1 = ;$(NO_SUBDIR) echo ' ' SUBDIR $$subdir; \ $(MAKE) $(PRINT_DIR) -C $$subdir
@@ -20,7 +21,7 @@ include ../../config.mak.uname TARGETS = scalar$(X) scalar.o GITLIBS = ../../common-main.o ../../libgit.a ../../xdiff/lib.a -all: scalar$(X) +all: scalar$(X) ../../bin-wrappers/scalar $(GITLIBS): $(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) $(subst ../../,,$@)
@@ -29,6 +30,16 @@ $(TARGETS): $(GITLIBS) scalar.c $(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) $(patsubst %,contrib/scalar/%,$@) clean: - $(RM) $(TARGETS) + $(RM) $(TARGETS) ../../bin-wrappers/scalar -.PHONY: $(GITLIBS) all clean FORCE +../../bin-wrappers/scalar: ../../wrap-for-bin.sh Makefile + @mkdir -p ../../bin-wrappers + $(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \ + -e 's|@@BUILD_DIR@@|$(shell cd ../.. && pwd)|' \ + -e 's|@@PROG@@|contrib/scalar/scalar$(X)|' < $< > $@ && \ + chmod +x $@ + +test: all + $(MAKE) -C t + +.PHONY: $(GITLIBS) all clean test FORCE
contrib/scalar/t/Makefile new file +78 -0
78 lines not on this page · raw message
contrib/scalar/t/t9099-scalar.sh new file +17 -0
@@ -0,0 +1,17 @@ +#!/bin/sh + +test_description='test the `scalar` command' + +TEST_DIRECTORY=$PWD/../../../t +export TEST_DIRECTORY + +# Make it work with --no-bin-wrappers +PATH=$PWD/..:$PATH + +. ../../../t/test-lib.sh + +test_expect_success 'scalar shows a usage' ' + test_expect_code 129 scalar -h +' + +test_done
Signature · 1 line
gitgitgadget
PATCH v9 5/17 cmake: optionally build `scalar`, too
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
The CMake configuration unfortunately does not let us easily encapsulate Scalar's build definition in the contrib/scalar/ subdirectory: The scalar executable needs to link in libgit.a and common-main.o, for example.
Also, scalar.c includes Git's header files, which means that scalar.c needs to be compiled with the very same flags as libgit.a lest scalar.o and libgit.a have different ideas of, say, platform_SHA_CTX, which would naturally lead to memory corruption.
To alleviate that somewhat, we guard the inclusion of Scalar via the INCLUDE_SCALAR environment variable.
--- contrib/buildsystems/CMakeLists.txt | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
contrib/buildsystems/CMakeLists.txt +14 -0
@@ -729,6 +729,13 @@ if(CURL_FOUND) endif() endif() +if(DEFINED ENV{INCLUDE_SCALAR} AND NOT ENV{INCLUDE_SCALAR} STREQUAL "") + add_executable(scalar ${CMAKE_SOURCE_DIR}/contrib/scalar/scalar.c) + target_link_libraries(scalar common-main) + set_target_properties(scalar PROPERTIES RUNTIME_OUTPUT_DIRECTORY_DEBUG ${CMAKE_BINARY_DIR}/contrib/scalar) + set_target_properties(scalar PROPERTIES RUNTIME_OUTPUT_DIRECTORY_RELEASE ${CMAKE_BINARY_DIR}/contrib/scalar) +endif() + parse_makefile_for_executables(git_builtin_extra "BUILT_INS") option(SKIP_DASHED_BUILT_INS "Skip hardlinking the dashed versions of the built-ins")
@@ -953,6 +960,13 @@ string(REPLACE "@@BUILD_DIR@@" "${CMAKE_BINARY_DIR}" content "${content}") string(REPLACE "@@PROG@@" "git-cvsserver" content "${content}") file(WRITE ${CMAKE_BINARY_DIR}/bin-wrappers/git-cvsserver ${content}) +if(DEFINED ENV{INCLUDE_SCALAR} AND NOT ENV{INCLUDE_SCALAR} STREQUAL "") + file(STRINGS ${CMAKE_SOURCE_DIR}/wrap-for-bin.sh content NEWLINE_CONSUME) + string(REPLACE "@@BUILD_DIR@@" "${CMAKE_BINARY_DIR}" content "${content}") + string(REPLACE "@@PROG@@" "contrib/scalar/scalar${EXE_EXTENSION}" content "${content}") + file(WRITE ${CMAKE_BINARY_DIR}/bin-wrappers/scalar ${content}) +endif() + #options for configuring test options option(PERL_TESTS "Perform tests that use perl" ON) option(PYTHON_TESTS "Perform tests that use python" ON)
Signature · 1 line
gitgitgadget
PATCH v9 6/17 ci: also run the `scalar` tests
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
Since Scalar depends on libgit.a, it makes sense to ensure in the CI and the PR builds that it does not get broken in case of industrious refactorings of the core Git code.
--- .github/workflows/main.yml | 15 +++++++++++++++ ci/run-build-and-tests.sh | 2 ++ ci/run-test-slice.sh | 5 +++++ 3 files changed, 22 insertions(+)
.github/workflows/main.yml +15 -0
@@ -89,6 +89,13 @@ jobs: HOME: ${{runner.workspace}} NO_PERL: 1 run: . /etc/profile && ci/make-test-artifacts.sh artifacts + - name: build Scalar + shell: bash + run: | + make -C contrib/scalar && + mkdir -p artifacts/bin-wrappers artifacts/contrib/scalar && + cp contrib/scalar/scalar.exe artifacts/contrib/scalar/ && + cp bin-wrappers/scalar artifacts/bin-wrappers/ - name: zip up tracked files run: git archive -o artifacts/tracked.tar.gz HEAD - name: upload tracked files and build artifacts
@@ -157,6 +164,8 @@ jobs: run: compat\vcbuild\vcpkg_copy_dlls.bat release - name: generate Visual Studio solution shell: bash + env: + INCLUDE_SCALAR: YesPlease run: | cmake `pwd`/contrib/buildsystems/ -DCMAKE_PREFIX_PATH=`pwd`/compat/vcbuild/vcpkg/installed/x64-windows \ -DNO_GETTEXT=YesPlease -DPERL_TESTS=OFF -DPYTHON_TESTS=OFF -DCURL_NO_CURL_CMAKE=ON
@@ -170,6 +179,12 @@ jobs: run: | mkdir -p artifacts && eval "$(make -n artifacts-tar INCLUDE_DLLS_IN_ARTIFACTS=YesPlease ARTIFACTS_DIRECTORY=artifacts NO_GETTEXT=YesPlease 2>&1 | grep ^tar)" + - name: copy Scalar + shell: bash + run: | + mkdir -p artifacts/bin-wrappers artifacts/contrib/scalar && + cp contrib/scalar/scalar.exe artifacts/contrib/scalar/ && + cp bin-wrappers/scalar artifacts/bin-wrappers/ - name: zip up tracked files run: git archive -o artifacts/tracked.tar.gz HEAD - name: upload tracked files and build artifacts
ci/run-build-and-tests.sh +2 -0
@@ -52,4 +52,6 @@ esac check_unignored_build_artifacts +make && make -C contrib/scalar test + save_good_tree
ci/run-test-slice.sh +5 -0
@@ -14,4 +14,9 @@ make --quiet -C t T="$(cd t && ./helper/test-tool path-utils slice-tests "$1" "$2" t[0-9]*.sh | tr '\n' ' ')" +if test 0 = "$1" +then + make -C contrib/scalar test +fi + check_unignored_build_artifacts
Signature · 1 line
gitgitgadget
PATCH v9 7/17 scalar: 'register' sets recommended config and starts maintenance
In reply to Johannes Schindelin via GitGitGadgetFrom: Derrick Stolee <[email protected]>
Let's start implementing the register command. With this commit, recommended settings are configured upon scalar register, and Git's background maintenance is started.
The recommended config settings may very well change in the future. For example, once the built-in FSMonitor is available, we will want to enable it upon scalar register. For that reason, we explicitly support running scalar register in an already-registered enlistment.
--- contrib/scalar/scalar.c | 249 ++++++++++++++++++++++++++++++++++++++ contrib/scalar/scalar.txt | 18 ++- 2 files changed, 266 insertions(+), 1 deletion(-)
contrib/scalar/scalar.c +249 -0
@@ -5,11 +5,260 @@#include "cache.h" #include "gettext.h" #include "parse-options.h" +#include "config.h" +#include "run-command.h" + +/* +* Remove the deepest subdirectory in the provided path string. Path must not +* include a trailing path separator. Returns 1 if parent directory found, +* otherwise 0. +*/ +static int strbuf_parent_directory (struct strbuf *buf) +{ +size_t len = buf->len; +size_t offset =offset_1st_component (buf->buf); +char *path_sep =find_last_dir_sep (buf->buf + offset); +strbuf_setlen (buf, path_sep ? path_sep - buf->buf : offset); + +return buf->len < len; +} + +static void setup_enlistment_directory (int argc,const char **argv, +const char *const *usagestr, +const struct option *options, +struct strbuf *enlistment_root) +{ +struct strbuf path =STRBUF_INIT ; +char *root; +int enlistment_found =0 ; + +if (startup_info->have_repository) +BUG ("gitdir already set up?!?" ); + +if (argc >1 ) +usage_with_options (usagestr, options); + +/* find the worktree, determine its corresponding root */ +if (argc ==1 ) +strbuf_add_absolute_path (&path, argv[0 ]); +else if (strbuf_getcwd (&path) <0 ) +die (_ ("need a working directory" )); + +strbuf_trim_trailing_dir_sep (&path); +do { +const size_t len = path.len; + +/* check if currently in enlistment root with src/ workdir */ +strbuf_addstr (&path,"/src" ); +if (is_nonbare_repository_dir (&path)) { +if (enlistment_root) +strbuf_add (enlistment_root, path.buf, len); + + enlistment_found =1 ; +break ; + } + +/* reset to original path */ +strbuf_setlen (&path, len); + +/* check if currently in workdir */ +if (is_nonbare_repository_dir (&path)) { +if (enlistment_root) { +/* +* If the worktree's directory's name is `src`, the enlistment is the +* parent directory, otherwise it is identical to the worktree. +*/ + root =strip_path_suffix (path.buf,"src" ); +strbuf_addstr (enlistment_root, root ? root : path.buf); +free (root); + } + + enlistment_found =1 ; +break ; + } + }while (strbuf_parent_directory (&path)); + +if (!enlistment_found) +die (_ ("could not find enlistment root" )); + +if (chdir (path.buf) <0 ) +die_errno (_ ("could not switch to '%s'" ), path.buf); + +strbuf_release (&path); +setup_git_directory (); +} + +static int run_git (const char *arg, ...) +{ +struct strvec argv =STRVEC_INIT ; +va_list args; +const char *p; +int res; + +va_start (args, arg); +strvec_push (&argv, arg); + while ((p = va_arg(args, const char *))) +strvec_push (&argv, p); +va_end (args); + + res =run_command_v_opt (argv.v,RUN_GIT_CMD ); + +strvec_clear (&argv); +return res; +} + +static int set_recommended_config (void ) +{ +struct { +const char *key; +const char *value; + } config[] = { + {"am.keepCR" ,"true" }, + {"core.FSCache" ,"true" }, + {"core.multiPackIndex" ,"true" }, + {"core.preloadIndex" ,"true" }, +#ifndef WIN32 + {"core.untrackedCache" ,"true" }, +#else +/* +* Unfortunately, Scalar's Functional Tests demonstrated +* that the untracked cache feature is unreliable on Windows +* (which is a bummer because that platform would benefit the +* most from it). For some reason, freshly created files seem +* not to update the directory's `lastModified` time +* immediately, but the untracked cache would need to rely on +* that. +* +* Therefore, with a sad heart, we disable this very useful +* feature on Windows. +*/ + {"core.untrackedCache" ,"false" }, +#endif + {"core.logAllRefUpdates" ,"true" }, + {"credential.https://dev.azure.com.useHttpPath" ,"true" }, + {"credential.validate" ,"false" },/* GCM4W-only */ + {"gc.auto" ,"0" }, + {"gui.GCWarning" ,"false" }, + {"index.threads" ,"true" }, + {"index.version" ,"4" }, + {"merge.stat" ,"false" }, + {"merge.renames" ,"true" }, + {"pack.useBitmaps" ,"false" }, + {"pack.useSparse" ,"true" }, + {"receive.autoGC" ,"false" }, + {"reset.quiet" ,"true" }, + {"feature.manyFiles" ,"false" }, + {"feature.experimental" ,"false" }, + {"fetch.unpackLimit" ,"1" }, + {"fetch.writeCommitGraph" ,"false" }, +#ifdef WIN32 + {"http.sslBackend" ,"schannel" }, +#endif + {"status.aheadBehind" ,"false" }, + {"commitGraph.generationVersion" ,"1" }, + {"core.autoCRLF" ,"false" }, + {"core.safeCRLF" ,"false" }, + {"fetch.showForcedUpdates" ,"false" }, + {NULL ,NULL }, + }; +int i; +char *value; + +for (i =0 ; config[i].key; i++) { +if (git_config_get_string (config[i].key, &value)) { +trace2_data_string ("scalar" , the_repository, config[i].key,"created" ); +if (git_config_set_gently (config[i].key, + config[i].value) <0 ) +return error (_ ("could not configure %s=%s" ), + config[i].key, config[i].value); + }else { +trace2_data_string ("scalar" , the_repository, config[i].key,"exists" ); +free (value); + } + } + +/* +* The `log.excludeDecoration` setting is special because it allows +* for multiple values. +*/ +if (git_config_get_string ("log.excludeDecoration" , &value)) { +trace2_data_string ("scalar" , the_repository, +"log.excludeDecoration" ,"created" ); +if (git_config_set_multivar_gently ("log.excludeDecoration" , +"refs/prefetch/*" , +CONFIG_REGEX_NONE ,0 )) +return error (_ ("could not configure " +"log.excludeDecoration" )); + }else { +trace2_data_string ("scalar" , the_repository, +"log.excludeDecoration" ,"exists" ); +free (value); + } + +return 0 ; +} + +static int start_maintenance (void ) +{ +return run_git ("maintenance" ,"start" ,NULL ); +} + +static int add_enlistment (void ) +{ +int res; + +if (!the_repository->worktree) +die (_ ("Scalar enlistments require a worktree" )); + + res =run_git ("config" ,"--global" ,"--get" ,"--fixed-value" , +"scalar.repo" , the_repository->worktree,NULL ); + +/* +* If the setting is already there, then do nothing. +*/ +if (!res) +return 0 ; + +return run_git ("config" ,"--global" ,"--add" , +"scalar.repo" , the_repository->worktree,NULL ); +} + +static int register_dir (void ) +{ +int res =add_enlistment (); + +if (!res) + res =set_recommended_config (); + +if (!res) + res =start_maintenance (); + +return res; +} + +static int cmd_register (int argc,const char **argv) +{ +struct option options[] = { +OPT_END (), + }; +const char *const usage[] = { +N_ ("scalar register [<enlistment>]" ), +NULL + }; + + argc =parse_options (argc, argv,NULL , options, + usage,0 ); + +setup_enlistment_directory (argc, argv, usage, options,NULL ); + +return register_dir (); +}static struct {const char *name;int (*fn)(int ,const char **); } builtins[] = { + {"register" , cmd_register }, {NULL ,NULL }, };
contrib/scalar/scalar.txt +17 -1
@@ -8,7 +8,7 @@ scalar - an opinionated repository management tool SYNOPSIS -------- [verse] -scalar <command> [<options>] +scalar register [<enlistment>] DESCRIPTION -----------
@@ -29,6 +29,22 @@ will be identical to the worktree. The `scalar` command implements various subcommands, and different options depending on the subcommand. +COMMANDS +-------- + +Register +~~~~~~~~ + +register [<enlistment>]:: + Adds the enlistment's repository to the list of registered repositories + and starts background maintenance. If `<enlistment>` is not provided, + then the enlistment associated with the current working directory is + registered. ++ +Note: when this subcommand is called in a worktree that is called `src/`, its +parent directory is considered to be the Scalar enlistment. If the worktree is +_not_ called `src/`, it itself will be considered to be the Scalar enlistment. + SEE ALSO -------- linkgit:git-maintenance[1].
Signature · 1 line
gitgitgadget
PATCH v9 8/17 scalar: 'unregister' stops background maintenance
In reply to Johannes Schindelin via GitGitGadgetFrom: Derrick Stolee <[email protected]>
Just like scalar register starts the scheduled background maintenance, scalar unregister stops it. Note that we use git maintenance start in scalar register, but we do not use git maintenance stop in scalar unregister: this would stop maintenance for _all_ repositories, not just for the one we want to unregister.
The unregister command also removes the corresponding entry from the [scalar] section in the global Git config.
--- contrib/scalar/scalar.c | 50 ++++++++++++++++++++++++++++++++------- contrib/scalar/scalar.txt | 8 +++++++ 2 files changed, 50 insertions(+), 8 deletions(-)
contrib/scalar/scalar.c +42 -8
@@ -198,12 +198,12 @@ static int set_recommended_config(void)return 0 ; } -static int start_maintenance (void ) +static int toggle_maintenance (int enable) { -return run_git ("maintenance" ,"start" ,NULL ); +return run_git ("maintenance" , enable ?"start" :"unregister" ,NULL ); } -static int add_enlistment (void ) +static int add_or_remove_enlistment (int add) {int res;
@@ -214,24 +214,39 @@ static int add_enlistment(void) "scalar.repo", the_repository->worktree, NULL);/* -* If the setting is already there, then do nothing. +* If we want to add and the setting is already there, then do nothing. +* If we want to remove and the setting is not there, then do nothing. */ -if (!res) +if ((add && !res) || (!add && res))return 0 ; -return run_git ("config" ,"--global" ,"--add" , +return run_git ("config" ,"--global" , add ?"--add" :"--unset" , + add ?"--no-fixed-value" :"--fixed-value" ,"scalar.repo" , the_repository->worktree,NULL ); }static int register_dir (void ) { -int res =add_enlistment (); +int res =add_or_remove_enlistment (1 );if (!res) res =set_recommended_config ();if (!res) - res =start_maintenance (); + res =toggle_maintenance (1 ); + +return res; +} + +static int unregister_dir (void ) +{ +int res =0 ; + +if (toggle_maintenance (0 ) <0 ) + res =-1 ; + +if (add_or_remove_enlistment (0 ) <0 ) + res =-1 ;return res; }
@@ -254,11 +269,30 @@ static int cmd_register(int argc, const char **argv)return register_dir (); } +static int cmd_unregister (int argc,const char **argv) +{ +struct option options[] = { +OPT_END (), + }; +const char *const usage[] = { +N_ ("scalar unregister [<enlistment>]" ), +NULL + }; + + argc =parse_options (argc, argv,NULL , options, + usage,0 ); + +setup_enlistment_directory (argc, argv, usage, options,NULL ); + +return unregister_dir (); +} +static struct {const char *name;int (*fn)(int ,const char **); } builtins[] = { {"register" , cmd_register }, + {"unregister" , cmd_unregister }, {NULL ,NULL }, };
contrib/scalar/scalar.txt +8 -0
@@ -9,6 +9,7 @@ SYNOPSIS -------- [verse] scalar register [<enlistment>] +scalar unregister [<enlistment>] DESCRIPTION -----------
@@ -45,6 +46,13 @@ Note: when this subcommand is called in a worktree that is called `src/`, its parent directory is considered to be the Scalar enlistment. If the worktree is _not_ called `src/`, it itself will be considered to be the Scalar enlistment. +Unregister +~~~~~~~~~~ + +unregister [<enlistment>]:: + Remove the specified repository from the list of repositories + registered with Scalar and stop the scheduled background maintenance. + SEE ALSO -------- linkgit:git-maintenance[1].
Signature · 1 line
gitgitgadget
PATCH v9 9/17 scalar: let 'unregister' handle a deleted enlistment directory gracefully
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
When a user deleted an enlistment manually, let's be generous and _still_ unregister it.
--- contrib/scalar/scalar.c | 46 ++++++++++++++++++++++++++++++++ contrib/scalar/t/t9099-scalar.sh | 15 +++++++++++ 2 files changed, 61 insertions(+)
contrib/scalar/scalar.c +46 -0
@@ -269,6 +269,24 @@ static int cmd_register(int argc, const char **argv)return register_dir (); } +static int remove_deleted_enlistment (struct strbuf *path) +{ +int res =0 ; +strbuf_realpath_forgiving (path, path->buf,1 ); + +if (run_git ("config" ,"--global" , +"--unset" ,"--fixed-value" , +"scalar.repo" , path->buf,NULL ) <0 ) + res =-1 ; + +if (run_git ("config" ,"--global" , +"--unset" ,"--fixed-value" , +"maintenance.repo" , path->buf,NULL ) <0 ) + res =-1 ; + +return res; +} +static int cmd_unregister (int argc,const char **argv) {struct option options[] = {
@@ -282,6 +300,34 @@ static int cmd_unregister(int argc, const char **argv) argc =parse_options (argc, argv,NULL , options, usage,0 ); +/* +* Be forgiving when the enlistment or worktree does not even exist any +* longer; This can be the case if a user deleted the worktree by +* mistake and _still_ wants to unregister the thing. +*/ +if (argc ==1 ) { +struct strbuf src_path =STRBUF_INIT , workdir_path =STRBUF_INIT ; + +strbuf_addf (&src_path,"%s/src/.git" , argv[0 ]); +strbuf_addf (&workdir_path,"%s/.git" , argv[0 ]); +if (!is_directory (src_path.buf) && !is_directory (workdir_path.buf)) { +/* remove possible matching registrations */ +int res =-1 ; + +strbuf_strip_suffix (&src_path,"/.git" ); + res =remove_deleted_enlistment (&src_path) && res; + +strbuf_strip_suffix (&workdir_path,"/.git" ); + res =remove_deleted_enlistment (&workdir_path) && res; + +strbuf_release (&src_path); +strbuf_release (&workdir_path); +return res; + } +strbuf_release (&src_path); +strbuf_release (&workdir_path); + } +setup_enlistment_directory (argc, argv, usage, options,NULL );return unregister_dir ();
contrib/scalar/t/t9099-scalar.sh +15 -0
@@ -14,4 +14,19 @@ test_expect_success 'scalar shows a usage' ' test_expect_code 129 scalar -h ' +test_expect_success 'scalar unregister' ' + git init vanish/src && + scalar register vanish/src && + git config --get --global --fixed-value \ + maintenance.repo "$(pwd)/vanish/src" && + scalar list >scalar.repos && + grep -F "$(pwd)/vanish/src" scalar.repos && + rm -rf vanish/src/.git && + scalar unregister vanish && + test_must_fail git config --get --global --fixed-value \ + maintenance.repo "$(pwd)/vanish/src" && + scalar list >scalar.repos && + ! grep -F "$(pwd)/vanish/src" scalar.repos +' + test_done
Signature · 1 line
gitgitgadget
PATCH v9 10/17 scalar: implement 'scalar list'
In reply to Johannes Schindelin via GitGitGadgetFrom: Derrick Stolee <[email protected]>
The produced list simply consists of those repositories registered under the multi-valued scalar.repo config setting in the user's Git config.
--- contrib/scalar/scalar.c | 11 +++++++++++ contrib/scalar/scalar.txt | 11 ++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-)
contrib/scalar/scalar.c +11 -0
@@ -251,6 +251,16 @@ static int unregister_dir(void)return res; } +static int cmd_list (int argc,const char **argv) +{ +if (argc !=1 ) +die (_ ("`scalar list` does not take arguments" )); + +if (run_git ("config" ,"--global" ,"--get-all" ,"scalar.repo" ,NULL ) <0 ) +return -1 ; +return 0 ; +} +static int cmd_register (int argc,const char **argv) {struct option options[] = {
@@ -337,6 +347,7 @@ static struct {const char *name;int (*fn)(int ,const char **); } builtins[] = { + { "list", cmd_list }, { "register", cmd_register }, { "unregister", cmd_unregister }, { NULL, NULL},
contrib/scalar/scalar.txt +10 -1
@@ -8,6 +8,7 @@ scalar - an opinionated repository management tool SYNOPSIS -------- [verse] +scalar list scalar register [<enlistment>] scalar unregister [<enlistment>]
@@ -28,11 +29,19 @@ an existing Git worktree with Scalar whose name is not `src`, the enlistment will be identical to the worktree. The `scalar` command implements various subcommands, and different options -depending on the subcommand. +depending on the subcommand. With the exception of `list`, all subcommands +expect to be run in an enlistment. COMMANDS -------- +List +~~~~ + +list:: + List enlistments that are currently registered by Scalar. This + subcommand does not need to be run inside an enlistment. + Register ~~~~~~~~
Signature · 1 line
gitgitgadget
PATCH v9 11/17 scalar: implement the `clone` subcommand
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
This implements Scalar's opinionated clone command: it tries to use a partial clone and sets up a sparse checkout by default. In contrast to git clone, scalar clone sets up the worktree in the src/ subdirectory, to encourage a separation between the source files and the build output (which helps Git tremendously because it avoids untracked files that have to be specifically ignored when refreshing the index).
Also, it registers the repository for regular, scheduled maintenance, and configures a flurry of configuration settings based on the experience and experiments of the Microsoft Windows and the Microsoft Office development teams.
Note: since the scalar clone command is by far the most commonly called scalar subcommand, we document it at the top of the manual page.
--- contrib/scalar/scalar.c | 201 +++++++++++++++++++++++++++++++ contrib/scalar/scalar.txt | 32 ++++- contrib/scalar/t/t9099-scalar.sh | 32 +++++ 3 files changed, 262 insertions(+), 3 deletions(-)
contrib/scalar/scalar.c +201 -0
@@ -7,6 +7,7 @@#include "parse-options.h" #include "config.h" #include "run-command.h" +#include "refs.h" /* * Remove the deepest subdirectory in the provided path string. Path must not
@@ -251,6 +252,205 @@ static int unregister_dir(void)return res; } +/* printf-style interface, expects `<key>=<value>` argument */ +static int set_config (const char *fmt, ...) +{ +struct strbuf buf =STRBUF_INIT ; +char *value; +int res; +va_list args; + +va_start (args, fmt); +strbuf_vaddf (&buf, fmt, args); +va_end (args); + + value =strchr (buf.buf,'=' ); +if (value) + *(value++) ='\0' ; + res =git_config_set_gently (buf.buf, value); +strbuf_release (&buf); + +return res; +} + +static char *remote_default_branch (const char *url) +{ +struct child_process cp =CHILD_PROCESS_INIT ; +struct strbuf out =STRBUF_INIT ; + + cp.git_cmd =1 ; +strvec_pushl (&cp.args,"ls-remote" ,"--symref" , url,"HEAD" ,NULL ); +if (!pipe_command (&cp,NULL ,0 , &out,0 ,NULL ,0 )) { +const char *line = out.buf; + +while (*line) { +const char *eol =strchrnul (line,'\n' ), *p; +size_t len = eol - line; +char *branch; + +if (!skip_prefix (line,"ref: " , &p) || + !strip_suffix_mem (line, &len,"\tHEAD" )) { + line = eol + (*eol =='\n' ); +continue ; + } + + eol = line + len; +if (skip_prefix (p,"refs/heads/" , &p)) { + branch =xstrndup (p, eol - p); +strbuf_release (&out); +return branch; + } + +error (_ ("remote HEAD is not a branch: '%.*s'" ), + (int )(eol - p), p); +strbuf_release (&out); +return NULL ; + } + } +warning (_ ("failed to get default branch name from remote; " +"using local default" )); +strbuf_reset (&out); + +child_process_init (&cp); + cp.git_cmd =1 ; +strvec_pushl (&cp.args,"symbolic-ref" ,"--short" ,"HEAD" ,NULL ); +if (!pipe_command (&cp,NULL ,0 , &out,0 ,NULL ,0 )) { +strbuf_trim (&out); +return strbuf_detach (&out,NULL ); + } + +strbuf_release (&out); +error (_ ("failed to get default branch name" )); +return NULL ; +} + +static int cmd_clone (int argc,const char **argv) +{ +const char *branch =NULL ; +int full_clone =0 ; +struct option clone_options[] = { +OPT_STRING ('b' ,"branch" , &branch,N_ ("<branch>" ), +N_ ("branch to checkout after clone" )), +OPT_BOOL (0 ,"full-clone" , &full_clone, +N_ ("when cloning, create full working directory" )), +OPT_END (), + }; +const char *const clone_usage[] = { +N_ ("scalar clone [<options>] [--] <repo> [<dir>]" ), +NULL + }; +const char *url; +char *enlistment =NULL , *dir =NULL ; +struct strbuf buf =STRBUF_INIT ; +int res; + + argc =parse_options (argc, argv,NULL , clone_options, clone_usage,0 ); + +if (argc ==2 ) { + url = argv[0 ]; + enlistment =xstrdup (argv[1 ]); + }else if (argc ==1 ) { + url = argv[0 ]; + +strbuf_addstr (&buf, url); +/* Strip trailing slashes, if any */ +while (buf.len >0 &&is_dir_sep (buf.buf[buf.len -1 ])) +strbuf_setlen (&buf, buf.len -1 ); +/* Strip suffix `.git`, if any */ +strbuf_strip_suffix (&buf,".git" ); + + enlistment =find_last_dir_sep (buf.buf); +if (!enlistment) { +die (_ ("cannot deduce worktree name from '%s'" ), url); + } + enlistment =xstrdup (enlistment +1 ); + }else { +usage_msg_opt (_ ("You must specify a repository to clone." ), + clone_usage, clone_options); + } + +if (is_directory (enlistment)) +die (_ ("directory '%s' exists already" ), enlistment); + + dir =xstrfmt ("%s/src" , enlistment); + +strbuf_reset (&buf); +if (branch) +strbuf_addf (&buf,"init.defaultBranch=%s" , branch); +else { +char *b =repo_default_branch_name (the_repository,1 ); +strbuf_addf (&buf,"init.defaultBranch=%s" , b); +free (b); + } + +if ((res =run_git ("-c" , buf.buf,"init" ,"--" , dir,NULL ))) + goto cleanup; + +if (chdir (dir) <0 ) { + res =error_errno (_ ("could not switch to '%s'" ), dir); + goto cleanup; + } + +setup_git_directory (); + +/* common-main already logs `argv` */ +trace2_def_repo (the_repository); + +if (!branch && !(branch =remote_default_branch (url))) { + res =error (_ ("failed to get default branch for '%s'" ), url); + goto cleanup; + } + +if (set_config ("remote.origin.url=%s" , url) || +set_config ("remote.origin.fetch=" +"+refs/heads/*:refs/remotes/origin/*" ) || +set_config ("remote.origin.promisor=true" ) || +set_config ("remote.origin.partialCloneFilter=blob:none" )) { + res =error (_ ("could not configure remote in '%s'" ), dir); + goto cleanup; + } + +if (!full_clone && + (res =run_git ("sparse-checkout" ,"init" ,"--cone" ,NULL ))) + goto cleanup; + +if (set_recommended_config ()) +return error (_ ("could not configure '%s'" ), dir); + +if ((res =run_git ("fetch" ,"--quiet" ,"origin" ,NULL ))) { +warning (_ ("partial clone failed; attempting full clone" )); + +if (set_config ("remote.origin.promisor" ) || +set_config ("remote.origin.partialCloneFilter" )) { + res =error (_ ("could not configure for full clone" )); + goto cleanup; + } + +if ((res =run_git ("fetch" ,"--quiet" ,"origin" ,NULL ))) + goto cleanup; + } + +if ((res =set_config ("branch.%s.remote=origin" , branch))) + goto cleanup; +if ((res =set_config ("branch.%s.merge=refs/heads/%s" , + branch, branch))) + goto cleanup; + +strbuf_reset (&buf); +strbuf_addf (&buf,"origin/%s" , branch); + res =run_git ("checkout" ,"-f" ,"-t" , buf.buf,NULL ); +if (res) + goto cleanup; + + res =register_dir (); + +cleanup: +free (enlistment); +free (dir); +strbuf_release (&buf); +return res; +} +static int cmd_list (int argc,const char **argv) {if (argc !=1 )
@@ -347,6 +547,7 @@ static struct {const char *name;int (*fn)(int ,const char **); } builtins[] = { + { "clone", cmd_clone }, { "list", cmd_list }, { "register", cmd_register }, { "unregister", cmd_unregister },
contrib/scalar/scalar.txt +29 -3
@@ -8,6 +8,7 @@ scalar - an opinionated repository management tool SYNOPSIS -------- [verse] +scalar clone [--branch <main-branch>] [--full-clone] <url> [<enlistment>] scalar list scalar register [<enlistment>] scalar unregister [<enlistment>]
@@ -29,12 +30,37 @@ an existing Git worktree with Scalar whose name is not `src`, the enlistment will be identical to the worktree. The `scalar` command implements various subcommands, and different options -depending on the subcommand. With the exception of `list`, all subcommands -expect to be run in an enlistment. +depending on the subcommand. With the exception of `clone` and `list`, all +subcommands expect to be run in an enlistment. COMMANDS -------- +Clone +~~~~~ + +clone [<options>] <url> [<enlistment>]:: + Clones the specified repository, similar to linkgit:git-clone[1]. By + default, only commit and tree objects are cloned. Once finished, the + worktree is located at `<enlistment>/src`. ++ +The sparse-checkout feature is enabled (except when run with `--full-clone`) +and the only files present are those in the top-level directory. Use +`git sparse-checkout set` to expand the set of directories you want to see, +or `git sparse-checkout disable` to expand to all files (see +linkgit:git-sparse-checkout[1] for more details). You can explore the +subdirectories outside your sparse-checkout by using `git ls-tree +HEAD[:<directory>]`. + +-b <name>:: +--branch <name>:: + Instead of checking out the branch pointed to by the cloned + repository's HEAD, check out the `<name>` branch instead. + +--[no-]full-clone:: + A sparse-checkout is initialized by default. This behavior can be + turned off via `--full-clone`. + List ~~~~
@@ -64,7 +90,7 @@ unregister [<enlistment>]:: SEE ALSO -------- -linkgit:git-maintenance[1]. +linkgit:git-clone[1], linkgit:git-maintenance[1]. Scalar ---
contrib/scalar/t/t9099-scalar.sh +32 -0
@@ -10,6 +10,9 @@ PATH=$PWD/..:$PATH . ../../../t/test-lib.sh +GIT_TEST_MAINT_SCHEDULER="crontab:test-tool crontab ../cron.txt,launchctl:true,schtasks:true" +export GIT_TEST_MAINT_SCHEDULER + test_expect_success 'scalar shows a usage' ' test_expect_code 129 scalar -h '
@@ -29,4 +32,33 @@ test_expect_success 'scalar unregister' ' ! grep -F "$(pwd)/vanish/src" scalar.repos ' +test_expect_success 'set up repository to clone' ' + test_commit first && + test_commit second && + test_commit third && + git switch -c parallel first && + mkdir -p 1/2 && + test_commit 1/2/3 && + git config uploadPack.allowFilter true && + git config uploadPack.allowAnySHA1InWant true +' + +test_expect_success 'scalar clone' ' + second=$(git rev-parse --verify second:second.t) && + scalar clone "file://$(pwd)" cloned && + ( + cd cloned/src && + + git config --get --global --fixed-value maintenance.repo \ + "$(pwd)" && + + test_path_is_missing 1/2 && + test_must_fail git rev-list --missing=print $second && + git rev-list $second && + git cat-file blob $second >actual && + echo "second" >expect && + test_cmp expect actual + ) +' + test_done
Signature · 1 line
gitgitgadget
PATCH v9 12/17 scalar: teach 'clone' to support the --single-branch option
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
Just like git clone, the scalar clone command now also offers to restrict the clone to a single branch.
--- contrib/scalar/scalar.c | 9 +++++++-- contrib/scalar/scalar.txt | 12 +++++++++++- contrib/scalar/t/t9099-scalar.sh | 6 +++++- 3 files changed, 23 insertions(+), 4 deletions(-)
contrib/scalar/scalar.c +7 -2
@@ -327,12 +327,15 @@ static char *remote_default_branch(const char *url)static int cmd_clone (int argc,const char **argv) {const char *branch =NULL ; -int full_clone =0 ; +int full_clone =0 , single_branch =0 ;struct option clone_options[] = {OPT_STRING ('b' ,"branch" , &branch,N_ ("<branch>" ),N_ ("branch to checkout after clone" )),OPT_BOOL (0 ,"full-clone" , &full_clone,N_ ("when cloning, create full working directory" )), +OPT_BOOL (0 ,"single-branch" , &single_branch, +N_ ("only download metadata for the branch that will " +"be checked out" )),OPT_END (), };const char *const clone_usage[] = {
@@ -403,7 +406,9 @@ static int cmd_clone(int argc, const char **argv)if (set_config ("remote.origin.url=%s" , url) ||set_config ("remote.origin.fetch=" -"+refs/heads/*:refs/remotes/origin/*" ) || +"+refs/heads/%s:refs/remotes/origin/%s" , + single_branch ? branch :"*" , + single_branch ? branch :"*" ) ||set_config ("remote.origin.promisor=true" ) ||set_config ("remote.origin.partialCloneFilter=blob:none" )) { res =error (_ ("could not configure remote in '%s'" ), dir);
contrib/scalar/scalar.txt +11 -1
@@ -8,7 +8,7 @@ scalar - an opinionated repository management tool SYNOPSIS -------- [verse] -scalar clone [--branch <main-branch>] [--full-clone] <url> [<enlistment>] +scalar clone [--single-branch] [--branch <main-branch>] [--full-clone] <url> [<enlistment>] scalar list scalar register [<enlistment>] scalar unregister [<enlistment>]
@@ -57,6 +57,16 @@ HEAD[:<directory>]`. Instead of checking out the branch pointed to by the cloned repository's HEAD, check out the `<name>` branch instead. +--[no-]single-branch:: + Clone only the history leading to the tip of a single branch, either + specified by the `--branch` option or the primary branch remote's + `HEAD` points at. ++ +Further fetches into the resulting repository will only update the +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-]full-clone:: A sparse-checkout is initialized by default. This behavior can be turned off via `--full-clone`.
contrib/scalar/t/t9099-scalar.sh +5 -1
@@ -45,13 +45,17 @@ test_expect_success 'set up repository to clone' ' test_expect_success 'scalar clone' ' second=$(git rev-parse --verify second:second.t) && - scalar clone "file://$(pwd)" cloned && + scalar clone "file://$(pwd)" cloned --single-branch && ( cd cloned/src && git config --get --global --fixed-value maintenance.repo \ "$(pwd)" && + git for-each-ref --format="%(refname)" refs/remotes/origin/ >actual && + echo "refs/remotes/origin/parallel" >expect && + test_cmp expect actual && + test_path_is_missing 1/2 && test_must_fail git rev-list --missing=print $second && git rev-list $second &&
Signature · 1 line
gitgitgadget
PATCH v9 13/17 scalar: implement the `run` command
In reply to Johannes Schindelin via GitGitGadgetFrom: Derrick Stolee <[email protected]>
Note: this subcommand is provided primarily for backwards-compatibility, for existing Scalar uses. It is mostly just a shim for git maintenance, mapping task names from the way Scalar called them to the way Git calls them.
The reason why those names differ? The background maintenance was first implemented in Scalar, and when it was contributed as a patch series implementing the git maintenance command, reviewers suggested better names, those suggestions were accepted before the patches were integrated into core Git.
--- contrib/scalar/scalar.c | 64 +++++++++++++++++++++++++++++++++++++++ contrib/scalar/scalar.txt | 19 ++++++++++++ 2 files changed, 83 insertions(+)
contrib/scalar/scalar.c +64 -0
@@ -484,6 +484,69 @@ static int cmd_register(int argc, const char **argv)return register_dir (); } +static int cmd_run (int argc,const char **argv) +{ +struct option options[] = { +OPT_END (), + }; +struct { +const char *arg, *task; + } tasks[] = { + {"config" ,NULL }, + {"commit-graph" ,"commit-graph" }, + {"fetch" ,"prefetch" }, + {"loose-objects" ,"loose-objects" }, + {"pack-files" ,"incremental-repack" }, + {NULL ,NULL } + }; +struct strbuf buf =STRBUF_INIT ; +const char *usagestr[] = {NULL ,NULL }; +int i; + +strbuf_addstr (&buf,N_ ("scalar run <task> [<enlistment>]\nTasks:\n" )); +for (i =0 ; tasks[i].arg; i++) +strbuf_addf (&buf,"\t%s\n" , tasks[i].arg); + usagestr[0 ] = buf.buf; + + argc =parse_options (argc, argv,NULL , options, + usagestr,0 ); + +if (!argc) +usage_with_options (usagestr, options); + +if (!strcmp ("all" , argv[0 ])) { + i =-1 ; + }else { +for (i =0 ; tasks[i].arg &&strcmp (tasks[i].arg, argv[0 ]); i++) + ;/* keep looking for the task */ + +if (i >0 && !tasks[i].arg) { +error (_ ("no such task: '%s'" ), argv[0 ]); +usage_with_options (usagestr, options); + } + } + + argc--; + argv++; +setup_enlistment_directory (argc, argv, usagestr, options,NULL ); +strbuf_release (&buf); + +if (i ==0 ) +return register_dir (); + +if (i >0 ) +return run_git ("maintenance" ,"run" , +"--task" , tasks[i].task,NULL ); + +if (register_dir ()) +return -1 ; +for (i =1 ; tasks[i].arg; i++) +if (run_git ("maintenance" ,"run" , +"--task" , tasks[i].task,NULL )) +return -1 ; +return 0 ; +} +static int remove_deleted_enlistment (struct strbuf *path) {int res =0 ;
@@ -556,6 +619,7 @@ static struct { { "list", cmd_list }, { "register", cmd_register }, { "unregister", cmd_unregister }, + { "run", cmd_run }, { NULL, NULL}, };
contrib/scalar/scalar.txt +19 -0
@@ -12,6 +12,7 @@ scalar clone [--single-branch] [--branch <main-branch>] [--full-clone] <url> [<e scalar list scalar register [<enlistment>] scalar unregister [<enlistment>] +scalar run ( all | config | commit-graph | fetch | loose-objects | pack-files ) [<enlistment>] DESCRIPTION -----------
@@ -98,6 +99,24 @@ unregister [<enlistment>]:: Remove the specified repository from the list of repositories registered with Scalar and stop the scheduled background maintenance. +Run +~~~ + +scalar run ( all | config | commit-graph | fetch | loose-objects | pack-files ) [<enlistment>]:: + Run the given maintenance task (or all tasks, if `all` was specified). + Except for `all` and `config`, this subcommand simply hands off to + linkgit:git-maintenance[1] (mapping `fetch` to `prefetch` and + `pack-files` to `incremental-repack`). ++ +These tasks are run automatically as part of the scheduled maintenance, +as soon as the repository is registered with Scalar. It should therefore +not be necessary to run this subcommand manually. ++ +The `config` task is specific to Scalar and configures all those +opinionated default settings that make Git work more efficiently with +large repositories. As this task is run as part of `scalar clone` +automatically, explicit invocations of this task are rarely needed. + SEE ALSO -------- linkgit:git-clone[1], linkgit:git-maintenance[1].
Signature · 1 line
gitgitgadget
PATCH v9 14/17 scalar: allow reconfiguring an existing enlistment
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
This comes in handy during Scalar upgrades, or when config settings were messed up by mistake.
--- contrib/scalar/scalar.c | 79 +++++++++++++++++++++----------- contrib/scalar/scalar.txt | 8 ++++ contrib/scalar/t/t9099-scalar.sh | 8 ++++ 3 files changed, 67 insertions(+), 28 deletions(-)
contrib/scalar/scalar.c +51 -28
@@ -108,18 +108,20 @@ static int run_git(const char *arg, ...)return res; } -static int set_recommended_config (void ) +static int set_recommended_config (int reconfigure) {struct {const char *key;const char *value; +int overwrite_on_reconfigure; } config[] = { - {"am.keepCR" ,"true" }, - {"core.FSCache" ,"true" }, - {"core.multiPackIndex" ,"true" }, - {"core.preloadIndex" ,"true" }, +/* Required */ + {"am.keepCR" ,"true" ,1 }, + {"core.FSCache" ,"true" ,1 }, + {"core.multiPackIndex" ,"true" ,1 }, + {"core.preloadIndex" ,"true" ,1 }, #ifndef WIN32 - {"core.untrackedCache" ,"true" }, + {"core.untrackedCache" ,"true" ,1 }, #else /* * Unfortunately, Scalar's Functional Tests demonstrated
@@ -133,28 +135,29 @@ static int set_recommended_config(void) * Therefore, with a sad heart, we disable this very useful * feature on Windows. */ - { "core.untrackedCache", "false" }, + { "core.untrackedCache", "false", 1 }, #endif - { "core.logAllRefUpdates", "true" }, - { "credential.https://dev.azure.com.useHttpPath", "true" }, - { "credential.validate", "false" }, /* GCM4W-only */ - { "gc.auto", "0" }, - { "gui.GCWarning", "false" }, - { "index.threads", "true" }, - { "index.version", "4" }, - { "merge.stat", "false" }, - { "merge.renames", "true" }, - { "pack.useBitmaps", "false" }, - { "pack.useSparse", "true" }, - { "receive.autoGC", "false" }, - { "reset.quiet", "true" }, - { "feature.manyFiles", "false" }, - { "feature.experimental", "false" }, - { "fetch.unpackLimit", "1" }, - { "fetch.writeCommitGraph", "false" }, + { "core.logAllRefUpdates", "true", 1 }, + { "credential.https://dev.azure.com.useHttpPath", "true", 1 }, + { "credential.validate", "false", 1 }, /* GCM4W-only */ + { "gc.auto", "0", 1 }, + { "gui.GCWarning", "false", 1 }, + { "index.threads", "true", 1 }, + { "index.version", "4", 1 }, + { "merge.stat", "false", 1 }, + { "merge.renames", "true", 1 }, + { "pack.useBitmaps", "false", 1 }, + { "pack.useSparse", "true", 1 }, + { "receive.autoGC", "false", 1 }, + { "reset.quiet", "true", 1 }, + { "feature.manyFiles", "false", 1 }, + { "feature.experimental", "false", 1 }, + { "fetch.unpackLimit", "1", 1 }, + { "fetch.writeCommitGraph", "false", 1 }, #ifdef WIN32 - { "http.sslBackend", "schannel" }, + { "http.sslBackend", "schannel", 1 }, #endif + /* Optional */ { "status.aheadBehind", "false" }, { "commitGraph.generationVersion", "1" }, { "core.autoCRLF", "false" },
@@ -166,7 +169,8 @@ static int set_recommended_config(void)char *value;for (i =0 ; config[i].key; i++) { -if (git_config_get_string (config[i].key, &value)) { +if ((reconfigure && config[i].overwrite_on_reconfigure) || +git_config_get_string (config[i].key, &value)) {trace2_data_string ("scalar" , the_repository, config[i].key,"created" );if (git_config_set_gently (config[i].key, config[i].value) <0 )
@@ -231,7 +235,7 @@ static int register_dir(void)int res =add_or_remove_enlistment (1 );if (!res) - res =set_recommended_config (); + res =set_recommended_config (0 );if (!res) res =toggle_maintenance (1 );
@@ -419,7 +423,7 @@ static int cmd_clone(int argc, const char **argv) (res = run_git("sparse-checkout", "init", "--cone", NULL))) goto cleanup; -if (set_recommended_config ()) +if (set_recommended_config (0 ))return error (_ ("could not configure '%s'" ), dir);if ((res =run_git ("fetch" ,"--quiet" ,"origin" ,NULL ))) {
@@ -484,6 +488,24 @@ static int cmd_register(int argc, const char **argv)return register_dir (); } +static int cmd_reconfigure (int argc,const char **argv) +{ +struct option options[] = { +OPT_END (), + }; +const char *const usage[] = { +N_ ("scalar reconfigure [<enlistment>]" ), +NULL + }; + + argc =parse_options (argc, argv,NULL , options, + usage,0 ); + +setup_enlistment_directory (argc, argv, usage, options,NULL ); + +return set_recommended_config (1 ); +} +static int cmd_run (int argc,const char **argv) {struct option options[] = {
@@ -620,6 +642,7 @@ static struct { { "register", cmd_register }, { "unregister", cmd_unregister }, { "run", cmd_run }, + { "reconfigure", cmd_reconfigure }, { NULL, NULL}, };
contrib/scalar/scalar.txt +8 -0
@@ -13,6 +13,7 @@ scalar list scalar register [<enlistment>] scalar unregister [<enlistment>] scalar run ( all | config | commit-graph | fetch | loose-objects | pack-files ) [<enlistment>] +scalar reconfigure <enlistment> DESCRIPTION -----------
@@ -117,6 +118,13 @@ opinionated default settings that make Git work more efficiently with large repositories. As this task is run as part of `scalar clone` automatically, explicit invocations of this task are rarely needed. +Reconfigure +~~~~~~~~~~~ + +After a Scalar upgrade, or when the configuration of a Scalar enlistment +was somehow corrupted or changed by mistake, this subcommand allows to +reconfigure the enlistment. + SEE ALSO -------- linkgit:git-clone[1], linkgit:git-maintenance[1].
contrib/scalar/t/t9099-scalar.sh +8 -0
@@ -65,4 +65,12 @@ test_expect_success 'scalar clone' ' ) ' +test_expect_success 'scalar reconfigure' ' + git init one/src && + scalar register one && + git -C one/src config core.preloadIndex false && + scalar reconfigure one && + test true = "$(git -C one/src config core.preloadIndex)" +' + test_done
Signature · 1 line
gitgitgadget
PATCH v9 15/17 scalar: teach 'reconfigure' to optionally handle all registered enlistments
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
After a Scalar upgrade, it can come in really handy if there is an easy way to reconfigure all Scalar enlistments. This new option offers this functionality.
--- contrib/scalar/scalar.c | 61 ++++++++++++++++++++++++++++++-- contrib/scalar/scalar.txt | 9 +++-- contrib/scalar/t/t9099-scalar.sh | 3 ++ 3 files changed, 67 insertions(+), 6 deletions(-)
contrib/scalar/scalar.c +58 -3
@@ -488,22 +488,77 @@ static int cmd_register(int argc, const char **argv)return register_dir (); } +static int get_scalar_repos (const char *key,const char *value,void *data) +{ +struct string_list *list = data; + +if (!strcmp (key,"scalar.repo" )) +string_list_append (list, value); + +return 0 ; +} +static int cmd_reconfigure (int argc,const char **argv) { +int all =0 ;struct option options[] = { +OPT_BOOL ('a' ,"all" , &all, +N_ ("reconfigure all registered enlistments" )),OPT_END (), };const char *const usage[] = { -N_ ("scalar reconfigure [<enlistment>]" ), +N_ ("scalar reconfigure [--all | <enlistment>]" ),NULL }; +struct string_list scalar_repos =STRING_LIST_INIT_DUP ; +int i, res =0 ; +struct repository r = {NULL }; +struct strbuf commondir =STRBUF_INIT , gitdir =STRBUF_INIT ; argc =parse_options (argc, argv,NULL , options, usage,0 ); -setup_enlistment_directory (argc, argv, usage, options,NULL ); +if (!all) { +setup_enlistment_directory (argc, argv, usage, options,NULL ); + +return set_recommended_config (1 ); + } + +if (argc >0 ) +usage_msg_opt (_ ("--all or <enlistment>, but not both" ), + usage, options); + +git_config (get_scalar_repos, &scalar_repos); -return set_recommended_config (1 ); +for (i =0 ; i < scalar_repos.nr; i++) { +const char *dir = scalar_repos.items[i].string; + +strbuf_reset (&commondir); +strbuf_reset (&gitdir); + +if (chdir (dir) <0 ) { +warning_errno (_ ("could not switch to '%s'" ), dir); + res =-1 ; + }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 ; + } + } + +string_list_clear (&scalar_repos,1 ); +strbuf_release (&commondir); +strbuf_release (&gitdir); + +return res; }static int cmd_run (int argc,const char **argv)
contrib/scalar/scalar.txt +6 -3
@@ -13,7 +13,7 @@ scalar list scalar register [<enlistment>] scalar unregister [<enlistment>] scalar run ( all | config | commit-graph | fetch | loose-objects | pack-files ) [<enlistment>] -scalar reconfigure <enlistment> +scalar reconfigure [ --all | <enlistment> ] DESCRIPTION -----------
@@ -32,8 +32,8 @@ an existing Git worktree with Scalar whose name is not `src`, the enlistment will be identical to the worktree. The `scalar` command implements various subcommands, and different options -depending on the subcommand. With the exception of `clone` and `list`, all -subcommands expect to be run in an enlistment. +depending on the subcommand. With the exception of `clone`, `list` and +`reconfigure --all`, all subcommands expect to be run in an enlistment. COMMANDS --------
@@ -125,6 +125,9 @@ After a Scalar upgrade, or when the configuration of a Scalar enlistment was somehow corrupted or changed by mistake, this subcommand allows to reconfigure the enlistment. +With the `--all` option, all enlistments currently registered with Scalar +will be reconfigured. Use this option after each Scalar upgrade. + SEE ALSO -------- linkgit:git-clone[1], linkgit:git-maintenance[1].
contrib/scalar/t/t9099-scalar.sh +3 -0
@@ -70,6 +70,9 @@ test_expect_success 'scalar reconfigure' ' scalar register one && git -C one/src config core.preloadIndex false && scalar reconfigure one && + test true = "$(git -C one/src config core.preloadIndex)" && + git -C one/src config core.preloadIndex false && + scalar reconfigure -a && test true = "$(git -C one/src config core.preloadIndex)" '
Signature · 1 line
gitgitgadget
PATCH v9 16/17 scalar: implement the `delete` command
In reply to Johannes Schindelin via GitGitGadgetFrom: Matthew John Cheetham <[email protected]>
Delete an enlistment by first unregistering the repository and then deleting the enlistment directory (usually the directory containing the worktree src/ directory).
On Windows, if the current directory is inside the enlistment's directory, change to the parent of the enlistment directory, to allow us to delete the enlistment (directories used by processes e.g. as current working directories cannot be deleted on Windows).
--- contrib/scalar/scalar.c | 63 ++++++++++++++++++++++++++++++++ contrib/scalar/scalar.txt | 8 ++++ contrib/scalar/t/t9099-scalar.sh | 9 +++++ 3 files changed, 80 insertions(+)
contrib/scalar/scalar.c +63 -0
@@ -8,6 +8,8 @@#include "config.h" #include "run-command.h" #include "refs.h" +#include "dir.h" +#include "packfile.h" /* * Remove the deepest subdirectory in the provided path string. Path must not
@@ -328,6 +330,33 @@ static char *remote_default_branch(const char *url)return NULL ; } +static int delete_enlistment (struct strbuf *enlistment) +{ +#ifdef WIN32 +struct strbuf parent =STRBUF_INIT ; +#endif + +if (unregister_dir ()) +die (_ ("failed to unregister repository" )); + +#ifdef WIN32 +/* +* Change the current directory to one outside of the enlistment so +* that we may delete everything underneath it. +*/ +strbuf_addbuf (&parent, enlistment); +strbuf_parent_directory (&parent); +if (chdir (parent.buf) <0 ) +die_errno (_ ("could not switch to '%s'" ), parent.buf); +strbuf_release (&parent); +#endif + +if (remove_dir_recursively (enlistment,0 )) +die (_ ("failed to delete enlistment directory" )); + +return 0 ; +} +static int cmd_clone (int argc,const char **argv) {const char *branch =NULL ;
@@ -688,6 +717,39 @@ static int cmd_unregister(int argc, const char **argv)return unregister_dir (); } +static int cmd_delete (int argc,const char **argv) +{ +char *cwd =xgetcwd (); +struct option options[] = { +OPT_END (), + }; +const char *const usage[] = { +N_ ("scalar delete <enlistment>" ), +NULL + }; +struct strbuf enlistment =STRBUF_INIT ; +int res =0 ; + + argc =parse_options (argc, argv,NULL , options, + usage,0 ); + +if (argc !=1 ) +usage_with_options (usage, options); + +setup_enlistment_directory (argc, argv, usage, options, &enlistment); + +if (dir_inside_of (cwd, enlistment.buf) >=0 ) + res =error (_ ("refusing to delete current working directory" )); +else { +close_object_store (the_repository->objects); + res =delete_enlistment (&enlistment); + } +strbuf_release (&enlistment); +free (cwd); + +return res; +} + static struct { const char *name; int (*fn)(int, const char **);
@@ -698,6 +760,7 @@ static struct { { "unregister", cmd_unregister }, { "run", cmd_run }, { "reconfigure", cmd_reconfigure }, + { "delete", cmd_delete }, { NULL, NULL}, };
contrib/scalar/scalar.txt +8 -0
@@ -14,6 +14,7 @@ scalar register [<enlistment>] scalar unregister [<enlistment>] scalar run ( all | config | commit-graph | fetch | loose-objects | pack-files ) [<enlistment>] scalar reconfigure [ --all | <enlistment> ] +scalar delete <enlistment> DESCRIPTION -----------
@@ -128,6 +129,13 @@ reconfigure the enlistment. With the `--all` option, all enlistments currently registered with Scalar will be reconfigured. Use this option after each Scalar upgrade. +Delete +~~~~~~ + +delete <enlistment>:: + This subcommand lets you delete an existing Scalar enlistment from your + local file system, unregistering the repository. + SEE ALSO -------- linkgit:git-clone[1], linkgit:git-maintenance[1].
contrib/scalar/t/t9099-scalar.sh +9 -0
@@ -76,4 +76,13 @@ test_expect_success 'scalar reconfigure' ' test true = "$(git -C one/src config core.preloadIndex)" ' +test_expect_success 'scalar delete without enlistment shows a usage' ' + test_expect_code 129 scalar delete +' + +test_expect_success 'scalar delete with enlistment' ' + scalar delete cloned && + test_path_is_missing cloned +' + test_done
Signature · 1 line
gitgitgadget
PATCH v9 17/17 scalar: implement the `version` command
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
The .NET version of Scalar has a version command. This was necessary because it was versioned independently of Git.
Since Scalar is now tightly coupled with Git, it does not make sense for them to show different versions. Therefore, it shows the same output as git version. For backwards-compatibility with the .NET version, scalar version prints to stderr, though (git version prints to stdout instead).
--- contrib/scalar/scalar.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
contrib/scalar/scalar.c +39 -0
@@ -10,6 +10,7 @@#include "refs.h" #include "dir.h" #include "packfile.h" +#include "help.h" /* * Remove the deepest subdirectory in the provided path string. Path must not
@@ -357,6 +358,15 @@ static int delete_enlistment(struct strbuf *enlistment)return 0 ; } +/* +* Dummy implementation; Using `get_version_info()` would cause a link error +* without this. +*/ +void load_builtin_commands (const char *prefix,struct cmdnames *cmds) +{ +die ("not implemented" ); +} +static int cmd_clone (int argc,const char **argv) {const char *branch =NULL ;
@@ -750,6 +760,34 @@ static int cmd_delete(int argc, const char **argv)return res; } +static int cmd_version (int argc,const char **argv) +{ +int verbose =0 , build_options =0 ; +struct option options[] = { +OPT__VERBOSE (&verbose,N_ ("include Git version" )), +OPT_BOOL (0 ,"build-options" , &build_options, +N_ ("include Git's build options" )), +OPT_END (), + }; +const char *const usage[] = { +N_ ("scalar verbose [-v | --verbose] [--build-options]" ), +NULL + }; +struct strbuf buf =STRBUF_INIT ; + + argc =parse_options (argc, argv,NULL , options, + usage,0 ); + +if (argc !=0 ) +usage_with_options (usage, options); + +get_version_info (&buf, build_options); +fprintf (stderr,"%s\n" , buf.buf); +strbuf_release (&buf); + +return 0 ; +} + static struct { const char *name; int (*fn)(int, const char **);
@@ -761,6 +799,7 @@ static struct { { "run", cmd_run }, { "reconfigure", cmd_reconfigure }, { "delete", cmd_delete }, + { "version", cmd_version }, { NULL, NULL}, };
Signature · 1 line
gitgitgadget
Quoted 25 lines · Johannes Schindelin via GitGitGadget
[...]
* The rebase on top of v2.34.0, which changed the default merge strategy to ORT, should have changed the default for merge.renames to true. This is now the case.
* Accommodate preemptively for ab/ci-updates which invalidates assumptions made by this patch series that would still hold true with v2.34.0 but are no longer valid in seen and would trigger CI build breakages.
[...]1: 3a2e28275f1 = 1: 3a2e28275f1 scalar: add a README with a roadmap 2: 50160d61a41 = 2: 50160d61a41 scalar: create a rudimentary executable 3: 74cd6410931 = 3: 74cd6410931 scalar: start documenting the command 4: 37231a4dd07 = 4: 37231a4dd07 scalar: create test infrastructure 5: a39b9c81214 = 5: a39b9c81214 cmake: optionally build `scalar`, too 6: 8e3542e43f7 ! 6: 8c6762def30 ci: also run the `scalar` tests@@
.github/workflows/main.yml: jobs:
- name: upload tracked files and build artifacts##
ci/run-build-and-tests.sh##
-@@ci/run-build-and-tests.sh: linux-gcc-4.8|pedantic)
- make test
- ;;
- esac
-+make -C contrib/scalar test
+@@ci/run-build-and-tests.sh: esac
Quoted 5 lines
check_unignored_build_artifacts++make && make -C contrib/scalar test
++
+save_good_tree
This gets rid of the hard CI failure we saw in "seen" but still carries forward the logic error noted in [1] and of the "pedantic" compilation-only job now running tests, which isn't what that job is supposed to be doing.
See the "Don't run the tests" comment in cebead1ebfb (ci: run a pedantic build as part of the GitHub workflow, 2021-08-08). As noted before you can see that at the tail-end of your own CI output[2] on top of "master".
There's other seemingly unintended interactions with the ci/ code on "master" here (which I also also noted in previous threads). FWIW the naïve merge with ab/ci-updates happens to fix at least one of those. I.e. the issue here of "linux-gcc" and "linux-clang" only running the scalar tests in one half of their test modes, but running everything else in both.
I think it's clear from past exchanges that you vehemently disagree with my proposed direction for solving these and other issues in one fell swoop[3], which is fair enough.
But I really don't think it's OK to continue to ignore reports from me of specific issues in this series just because you don't like that larger set of fixes [3].
I honestly don't care if you'd pick that up as-is, just as long as outstanding issues it addressed are either fixed, or commit messages/docs are updated to note that the bugs/behavior changes are intentional.
In this case I think it would be fine to keep the patch as-is and have the commit message argue for why the scalar tests should be a special snowflake in being the only tests that are run in "pedantic".
Or to just fix the seemingly unintentional behavior change in some smaller way. I think the "added thusly" comment I hade in [4] should be the easiest way to do that (well, [3] is easier, but let's leave that aside...).
1. https://lore.kernel.org/git/[email protected]/
2. https://github.com/gitgitgadget/git/runs/4292915519?check_suite_focus=true
3. https://lore.kernel.org/git/[email protected]/
4. https://lore.kernel.org/git/[email protected]/
Hi Ævar,
Quoted 3 lines · Ævar Arnfjörð Bjarmason
Quoted 1 line · Johannes Schindelin via GitGitGadget
[...]
Unfortunately, you clipped the most important part, the part that I put in there mostly for your benefit. So let me repeat it once again:
This patch series' focus is entirely on Scalar, on choosing sensible defaults and offering a delightful user experience around working with monorepos, and not about changing any existing paradigms for contrib/.
I do see that you want to drag the conversation back to discussing the build process, and the CI integration. And on changing the way things are done in contrib/. You've made that point abundantly clear. I just don't see how that could possibly improve Scalar. I mean, if it failed during the past 4 months, why expect any different outcome in the future.
Ciao,
Johannes
Quoted 15 lines · Johannes Schindelin
Hi Ævar,
Quoted 3 lines · Ævar Arnfjörð Bjarmason
Quoted 1 line · Johannes Schindelin via GitGitGadget
[...]
Unfortunately, you clipped the most important part, the part that I put in there mostly for your benefit. So let me repeat it once again:
This patch series' focus is entirely on Scalar, on choosing sensible defaults and offering a delightful user experience around working with monorepos, and not about changing any existing paradigms for contrib/.
Quoted 5 lines
I do see that you want to drag the conversation back to discussing the build process, and the CI integration. And on changing the way things are done in
contrib/. You've made that point abundantly clear. I just don't see how that could possibly improve Scalar. I mean, if it failed during the past 4 months, why expect any different outcome in the future.
The seemingly unintentional behavior change in CI jobs that aren't scalar jobs you're introducing started in v7 of this series, submitted on November 17th:
https://lore.kernel.org/git/1b0328fa236a35c2427b82f53c32944e513580d3.1637158762.git.gitgitgadget@gmail.com/
So as far as any CI testing is concerned we're talking about just under 2 weeks.
I really don't see how that and other unintentional behavior changes in the CI on top of "master" have anything to do with "the build process" in the sense that we've discussed as part of the greater scalar integration topic in the past.
I only linked to those thread(s) because some of that behavior changing (i.e. now running tests in a previously compile-only job) is apparent when either running with my patch-on-top of this series, or it was discussed in some threads related to that & the merger with ab/ci-updates.
Is it intentional that the previously compile-only "pedantic" job is now running the scalar tests?
Quoted 3 lines · Johannes Schindelin
This patch series' focus is entirely on Scalar, on choosing sensible defaults and offering a delightful user experience around working with monorepos, and not about changing any existing paradigms for contrib/.
Sorry, but I am confused.
The change to add "make &&" before testing scalar is a good change that allows CI to work with "existing paradigm for contrib/" that is "you need to build the top before doing anything in contrib/".
But none of the contrib/ stuff is tested in the pedantic job, but if I understand correctly, we start some (namely, scalar) stuff in it tested there, deviating from existing practice. Is that intended?
Hi Junio,
Quoted 8 lines · Junio C Hamano
Quoted 3 lines · Johannes Schindelin
This patch series' focus is entirely on Scalar, on choosing sensible defaults and offering a delightful user experience around working with monorepos, and not about changing any existing paradigms for contrib/.
Sorry, but I am confused.
Quoted 7 lines
The change to add "make &&" before testing scalar is a good change that allows CI to work with "existing paradigm for contrib/" that is "you need to build the top before doing anything in contrib/".
But none of the contrib/ stuff is tested in the pedantic job, but if I understand correctly, we start some (namely, scalar) stuff in it tested there, deviating from existing practice. Is that intended?
No, it was not intended. It was not even intended to integrate Scalar this tightly with Git's CI, but since you did not move along js/scalar into next for the past weeks, when no reviewer had anything to add to the actual code in contrib/scalar/ nor were there any objections to integrate it, I made the mistake of assuming that you agreed with Ævar that such a tight integration into Git's CI was desired.
However, I do not want to make the mistake again of assuming what your thoughts are, so let me ask you directly: from your perspective, what is stopping the inclusion of js/scalar into next?
I am patiently and eagerly awaiting your answer,
Dscho
Quoted 6 lines · Johannes Schindelin
No, it was not intended. It was not even intended to integrate Scalar this tightly with Git's CI, but since you did not move along
js/scalarintonextfor the past weeks, when no reviewer had anything to add to the actual code incontrib/scalar/nor were there any objections to integrate it, I made the mistake of assuming that you agreed with Ævar that such a tight integration into Git's CI was desired.
OK, sorry to hear that we had miscommunication.
I took the lack of comments an indication that people are not either interested in it, or viewing it as not-quite-ready-yet and waiting for a "more or less done" version.
I think the CI updates from Ævar would be one of the things we'd have early in 'next' in this cycle, so if the topic does not play nice with it, the perception that it is not yet part of the regular CI testing would continue, I am afraid.
Thanks.
Quoted 10 lines · Junio C Hamano
Quoted 6 lines · Johannes Schindelin
No, it was not intended. It was not even intended to integrate Scalar this tightly with Git's CI, but since you did not move along
js/scalarintonextfor the past weeks, when no reviewer had anything to add to the actual code incontrib/scalar/nor were there any objections to integrate it, I made the mistake of assuming that you agreed with Ævar that such a tight integration into Git's CI was desired.
Quoted 5 lines
OK, sorry to hear that we had miscommunication.
I took the lack of comments an indication that people are not either interested in it, or viewing it as not-quite-ready-yet and waiting for a "more or less done" version.
Sorry that my work project rendered me unable to respond for over a month.
Quoted 4 lines
I think the CI updates from Ævar would be one of the things we'd have early in 'next' in this cycle, so if the topic does not play nice with it, the perception that it is not yet part of the regular CI testing would continue, I am afraid.
I think I missed the answer. I believe Johannes was curious if he reverted the recent CI testing of scalar he added in v7 (which would as a side effect make it play nicely with Ævar's CI updates), if the resulting version of js/scalar would be acceptable for next.
(If my opinion matters: I'd be in favor. In more detail: Personally, while I think CI testing would be nice once we have a functionally useful scalar, the CI tests of this early version aren't really netting us anything. And they're blocking future scalar series unnecessarily. Johannes already said he had planned CI testing for a future series, so I'd rather just take this version of js/scalar minus the CI integration for next.)
Quoted 6 lines · Elijah Newren
while I think CI testing would be nice once we have a functionally useful scalar, the CI tests of this early version aren't really netting us anything. And they're blocking future scalar series unnecessarily. Johannes already said he had planned CI testing for a future series, so I'd rather just take this version of js/scalar minus the CI integration for next.)
Yeah, with less stomping on each others' toes, things may flow smoother. As long as people are happy with the core part, that sounds like the best approach forward.
Hi Junio,
Quoted 11 lines · Junio C Hamano
Quoted 6 lines · Elijah Newren
while I think CI testing would be nice once we have a functionally useful scalar, the CI tests of this early version aren't really netting us anything. And they're blocking future scalar series unnecessarily. Johannes already said he had planned CI testing for a future series, so I'd rather just take this version of js/scalar minus the CI integration for next.)
Yeah, with less stomping on each others' toes, things may flow smoother.
I also would find it nice if that was the case.
If I remember correctly, you mentioned quite a couple of times that you expect, particularly oldtimers on this list, to be mindful when contributing patch series, and to delay patches that would interfere with other patch series that are already in flight.
I saw with sorrow that this rule was ignored a couple of times recently, even with new contributors, and I sincerely hope that we can unignore that rule again.
Quoted 2 lines
As long as people are happy with the core part, that sounds like the best approach forward.
Thank you. I have to admit that it was quite frustrating to see so many obstacles being put in my way, only to end up at pretty much the same place as I had been over a month ago. So it is extra gratifying to hear that you move forward with the Scalar patch series. I truly believe that our users will be better off for it.
Thanks,
Dscho
Quoted 8 lines · Johannes Schindelin
If I remember correctly, you mentioned quite a couple of times that you expect, particularly oldtimers on this list, to be mindful when contributing patch series, and to delay patches that would interfere with other patch series that are already in flight.
I saw with sorrow that this rule was ignored a couple of times recently, even with new contributors, and I sincerely hope that we can unignore that rule again.
Sorry, but I am not sure what you are complaining about.
In general, I do try to ask more experienced and competent folks to bear more burden when playing the role of a traffic coordinator, as they are more capable of doing so to help the process.
Relative importance and complexity of the topics also play a role, so it is also possible that a more junior contributor may be asked to yield for a more urgent or a less complex topic.
I would give strong preference to things that are already in 'next', of course. There has to be an extraordinary reason before we kick something out of 'next', only to yield the way to allow another topic to graduate first.
So, I am not sure if there is a need to unignore any rule here.
Thanks.
v10 cover Upstreaming the Scalar command
In reply to Johannes Schindelin via GitGitGadgettl;dr: This series contributes the core part of the Scalar command to the Git project. This command provides a convenient way to clone/initialize very large repositories (think: monorepos).
Note: This patch series' focus is entirely on Scalar, on choosing sensible defaults and offering a delightful user experience around working with monorepos, and not about changing any existing paradigms for contrib/ (even if catching up on the mail thread is likely to give interested readers that false impression).
Changes since v9:
* The patches to build Scalar and run its tests as part of Git's CI/PR, have been dropped because a recent unrelated patch series does not interact well with them.
Changes since v8:
* The rebase on top of v2.34.0, which changed the default merge strategy to ORT, should have changed the default for merge.renames to true. This is now the case.
* Accommodate preemptively for ab/ci-updates which invalidates assumptions made by this patch series that would still hold true with v2.34.0 but are no longer valid in seen and would trigger CI build breakages.
Changes since v7:
* Clarified in the commit message why we cannot easily encapsulate the Scalar part of the CMake configuration in contrib/scalar/.
* Improved the README.md.
Changes since v6:
* Rebased on top of v2.34.0.
* Inserted a commit that adds contrib/scalar/README.md, containing the roadmap of what I have planned for Scalar.
* The Scalar test's definition of GIT_TEST_MAINT_SCHEDULER has been adjusted to accommodate for a change in v2.32.0..v2.34.0.
* The config setting defaults now include fetch.showForcedUpdates=false, which has been identified as helping with a performance issue in large repositories.
* To avoid mistaking the current patch series for being feature-complete enough to unleash onto end users, I moved the Makefile rules to build HTML/manual pages to a later patch series.
* The patch that adds support for -c <key>=<value> and -C <directory> was moved to its own add-on patch series: While it is obvious that those options are valuable to have, an open question is whether there are other "pre-command" options in git that would be useful, too, and I would like to postpone that discussion to that date.
* I added two patches that I had planned on keeping in an add-on patch series for later, to build and test Scalar as part of the CI. I am still not 100% certain that it is a good idea to do so already now, but let's see what the reviewers have to say.
Changes since v5:
* Fixed the commit message talking about make -C contrib/scalar/Makefile.
* Fixed the git ls-tree invocation suggested in the manual for scalar clone.
* Invoking make -C contrib/scalar, then changing a source file of libgit.a and then immediately invoking make -C contrib/scalar again will now implicitly rebuild libgit.a.
Changes since v4:
* scalar delete now refuses to delete anything if it was started from within the enlistment.
* scalar delete releases any handles to the object store before deleting the enlistment.
* The OBJECTS list in the Makefile will now include Scalar.
* scalar register now supports secondary worktrees, in addition to the primary worktree.
Changes since v3:
* Moved the "Changes since" section to the top, to make it easier to see what changed.
* Reworded the commit message of the first patch.
* Removed the [RFC] prefix because I did not hear any objections against putting this into contrib/.
Changes since v2:
* Adjusted the description of the list command in the manual page , as suggested by Bagas.
* Addressed two style nits in cmd_run().
* The documentation of git reconfigure -a was improved.
Changes since v1:
* A couple typos were fixed
* The code parsing the output of ls-remote was made more readable
* The indentation used in scalar.txt now consistently uses tabs
* We no longer hard-code core.bare = false when registering with Scalar
Background
==========
Microsoft invested a lot of effort into scaling Git to the needs of the Windows operating system source code. Based on the experience of the first approach, VFS for Git, the Scalar project was started. Scalar specifically has as its core goal to funnel all improvements into core Git.
The present
===========
The Scalar project provides a completely functional non-virtual experience for monorepos. But why stop there. The Scalar project was designed to be a self-destructing vehicle to allow those key concepts to be moved into core Git itself for the benefit of all. For example, partial clone, sparse-checkout, and scheduled background maintenance have already been upstreamed and removed from Scalar proper. This patch series provides a C-based implementation of the final remaining portions of the Scalar command. This will make it easier for users to experiment with the Scalar command. It will also make it substantially easier to experiment with moving functionality from Scalar into core Git, while maintaining backwards-compatibility for existing Scalar users.
The C-based Scalar has been shipped to Scalar users, and can be tested by any interested reader: https://github.com/microsoft/git/releases/ (it offers a Git for Windows installer, a macOS package and an Ubuntu package, Scalar has been included since v2.33.0.vfs.0.0).
Next steps
==========
Since there are existing Scalar users, I want to ensure backwards-compatibility with its existing command-line interface. Keeping that in mind, everything in this series is up for discussion.
I obviously believe that Scalar brings a huge benefit, and think that it would be ideal for all of Scalar's learnings to end up in git clone/git init/git maintenance eventually. It is also conceivable, however, that the scalar command could graduate to be a core part of Git at some stage in the future (such a decision would probably depend highly on users' feedback). See also the discussion about the architecture of Scalar
[https://lore.kernel.org/git/[email protected]/],
kicked off by Stolee.
On top of this patch series, I have lined up a few more:
1. Implement a scalar diagnose command.
2. Use the built-in FSMonitor (that patch series obviously needs to wait for FSMonitor to be integrated).
3. Modify the config machinery to be more generous about concurrent writes, say, to the user-wide config.
4. A few patches to optionally build and install scalar as part of a regular Git install (also teaching git help scalar to find the Scalar documentation
These are included in my vfs-with-scalar branch thicket
[https://github.com/dscho/git/commits/vfs-with-scalar]. On top of that, this branch thicket also includes patches I do not plan on upstreaming, mainly because they are too specific either to VFS for Git, or they support Azure Repos (which does not offer partial clones but speaks the GVFS protocol, which can be used to emulate partial clones).
One other thing is very interesting about that vfs-with-scalar branch
thicket: it contains a GitHub workflow which will run Scalar's quite extensive Functional Tests suite. This test suite is quite comprehensive and caught us a lot of bugs in the past, not only in the Scalar code, but also core Git.
Epilogue
========
Now, to address some questions that I imagine every reader has who made it this far:
* Why not put the Scalar functionality directly into core Git, even a built-in? I wanted to provide an easy way for Git contributors to "play with" Scalar, without forcing a new top-level command into Git.
* Why implement the Scalar command in the Git code base? Apart from simplifying Scalar maintenance in the Microsoft port of Git, the tight version coupling between Git and Scalar reduces the maintenance burden even further. Besides, I believe that it will make it much easier to shift functionality from Scalar into core Git, once we took the hurdle of accepting the Scalar code into the code base.
* Why contribute Scalar to the Git project? We are biased, of course, yet our data-driven approach provides evidence that Scalar helps handling huge repositories with ease. By contributing it to the core Git project, we are able to share it with more users, especially some users who do not want to install Microsoft's fork of Git. We also hope that a lot of Scalar (maybe all of it) will end up in core Git, to benefit even more users.
Derrick Stolee (4):
scalar: 'register' sets recommended config and starts maintenance
scalar: 'unregister' stops background maintenance
scalar: implement 'scalar list'
scalar: implement the run command
Johannes Schindelin (10):
scalar: add a README with a roadmap
scalar: create a rudimentary executable
scalar: start documenting the command
scalar: create test infrastructure
scalar: let 'unregister' handle a deleted enlistment directory
gracefully
scalar: implement the clone subcommand
scalar: teach 'clone' to support the --single-branch option
scalar: allow reconfiguring an existing enlistment
scalar: teach 'reconfigure' to optionally handle all registered
enlistments
scalar: implement the version command
Matthew John Cheetham (1):
scalar: implement the delete command
Makefile | 9 + contrib/scalar/.gitignore | 2 + contrib/scalar/Makefile | 45 ++ contrib/scalar/README.md | 82 +++ contrib/scalar/scalar.c | 826 +++++++++++++++++++++++++++++++ contrib/scalar/scalar.txt | 145 ++++++ contrib/scalar/t/Makefile | 78 +++ contrib/scalar/t/t9099-scalar.sh | 88 ++++ 8 files changed, 1275 insertions(+) create mode 100644 contrib/scalar/.gitignore create mode 100644 contrib/scalar/Makefile create mode 100644 contrib/scalar/README.md create mode 100644 contrib/scalar/scalar.c create mode 100644 contrib/scalar/scalar.txt create mode 100644 contrib/scalar/t/Makefile create mode 100755 contrib/scalar/t/t9099-scalar.sh
base-commit: cd3e606211bb1cf8bc57f7d76bab98cc17a150bc
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1005%2Fdscho%2Fscalar-the-beginning-v10
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1005/dscho/scalar-the-beginning-v10
Pull-Request: https://github.com/gitgitgadget/git/pull/1005
Range-diff vs v9:
1: 3a2e28275f1 = 1: 3a2e28275f1 scalar: add a README with a roadmap 2: 50160d61a41 = 2: 50160d61a41 scalar: create a rudimentary executable 3: 74cd6410931 = 3: 74cd6410931 scalar: start documenting the command 4: 37231a4dd07 = 4: 37231a4dd07 scalar: create test infrastructure 5: a39b9c81214 < -: ----------- cmake: optionally build `scalar`, too 6: 8c6762def30 < -: ----------- ci: also run the `scalar` tests 7: 936ee0475ad = 5: 4439ab4de0b scalar: 'register' sets recommended config and starts maintenance 8: 09a15f86c3d = 6: 376056066a0 scalar: 'unregister' stops background maintenance 9: 42121a5764d = 7: c865e89beb3 scalar: let 'unregister' handle a deleted enlistment directory gracefully 10: 6afb2eb4163 = 8: 3f8b0abd7d6 scalar: implement 'scalar list' 11: dd4e3a4b761 = 9: 60659c47196 scalar: implement the `clone` subcommand 12: abd9c8827cd = 10: 45aca840764 scalar: teach 'clone' to support the --single-branch option 13: 5601f82dbe1 = 11: 15e649a1734 scalar: implement the `run` command 14: 08e4f548aa8 = 12: 2a3fb40bd9a scalar: allow reconfiguring an existing enlistment 15: 0cec6dbd2cb = 13: efd808a0c4a scalar: teach 'reconfigure' to optionally handle all registered enlistments 16: 835f1c79792 = 14: 8b69462b906 scalar: implement the `delete` command 17: 4ee1b701c7b = 15: b5f416d79b4 scalar: implement the `version` command
Signature · 1 line
gitgitgadget
PATCH v10 1/15 scalar: add a README with a roadmap
From: Johannes Schindelin <[email protected]>
The Scalar command will be contributed incrementally, over a bunch of patch series. Let's document what Scalar is about, and then describe the patch series that are planned.
--- contrib/scalar/README.md | 82 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 contrib/scalar/README.md
contrib/scalar/README.md new file +82 -0
@@ -0,0 +1,82 @@ +# Scalar - an opinionated repository management tool + +Scalar is an add-on to Git that helps users take advantage of advanced +performance features in Git. Originally implemented in C# using .NET Core, +based on the learnings from the VFS for Git project, most of the techniques +developed by the Scalar project have been integrated into core Git already: + +* partial clone, +* commit graphs, +* multi-pack index, +* sparse checkout (cone mode), +* scheduled background maintenance, +* etc + +This directory contains the remaining parts of Scalar that are not (yet) in +core Git. + +## Roadmap + +The idea is to populate this directory via incremental patch series and +eventually move to a top-level directory next to `gitk-git/` and to `git-gui/`. The +current plan involves the following patch series: + +- `scalar-the-beginning`: The initial patch series which sets up + `contrib/scalar/` and populates it with a minimal `scalar` command that + demonstrates the fundamental ideas. + +- `scalar-c-and-C`: The `scalar` command learns about two options that can be + specified before the command, `-c <key>=<value>` and `-C <directory>`. + +- `scalar-diagnose`: The `scalar` command is taught the `diagnose` subcommand. + +- `scalar-and-builtin-fsmonitor`: The built-in FSMonitor is enabled in `scalar + register` and in `scalar clone`, for an enormous performance boost when + working in large worktrees. This patch series necessarily depends on Jeff + Hostetler's FSMonitor patch series to be integrated into Git. + +- `scalar-gentler-config-locking`: Scalar enlistments are registered in the + user's Git config. This usually does not represent any problem because it is + rare for a user to register an enlistment. However, in Scalar's functional + tests, Scalar enlistments are created galore, and in parallel, which can lead + to lock contention. This patch series works around that problem by re-trying + to lock the config file in a gentle fashion. + +- `scalar-extra-docs`: Add some extensive documentation that has been written + in the original Scalar project (all subject to discussion, of course). + +- `optionally-install-scalar`: Now that Scalar is feature (and documentation) + complete and is verified in CI builds, let's offer to install it. + +- `move-scalar-to-toplevel`: Now that Scalar is complete, let's move it next to + `gitk-git/` and to `git-gui/`, making it a top-level command. + +The following two patch series exist in Microsoft's fork of Git and are +publicly available. There is no current plan to upstream them, not because I +want to withhold these patches, but because I don't think the Git community is +interested in these patches. + +There are some interesting ideas there, but the implementation is too specific +to Azure Repos and/or VFS for Git to be of much help in general (and also: my +colleagues tried to upstream some patches already and the enthusiasm for +integrating things related to Azure Repos and VFS for Git can be summarized in +very, very few words). + +These still exist mainly because the GVFS protocol is what Azure Repos has +instead of partial clone, while Git is focused on improving partial clone: + +- `scalar-with-gvfs`: The primary purpose of this patch series is to support + existing Scalar users whose repositories are hosted in Azure Repos (which + does not support Git's partial clones, but supports its predecessor, the GVFS + protocol, which is used by Scalar to emulate the partial clone). + + Since the GVFS protocol will never be supported by core Git, this patch + series will remain in Microsoft's fork of Git. + +- `run-scalar-functional-tests`: The Scalar project developed a quite + comprehensive set of integration tests (or, "Functional Tests"). They are the + sole remaining part of the original C#-based Scalar project, and this patch + adds a GitHub workflow that runs them all. + + Since the tests partially depend on features that are only provided in the + `scalar-with-gvfs` patch series, this patch cannot be upstreamed.
Signature · 1 line
gitgitgadget
PATCH v10 2/15 scalar: create a rudimentary executable
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
The idea of Scalar (https://github.com/microsoft/scalar), and before that, of VFS for Git, has always been to prove that Git _can_ scale, and to upstream whatever strategies have been demonstrated to help.
With this patch, we start the journey from that C# project to move what is left to Git's own contrib/ directory, reimplementing it in pure C, with the intention to facilitate integrating the functionality into core Git all while maintaining backwards-compatibility for existing Scalar users (which will be much easier when both live in the same worktree). It has always been the plan to contribute all of the proven strategies back to core Git.
For example, while the virtual filesystem provided by VFS for Git helped the team developing the Windows operating system to move onto Git, while trying to upstream it we realized that it cannot be done: getting the virtual filesystem to work (which we only managed to implement fully on Windows, but not on, say, macOS or Linux), and the required server-side support for the GVFS protocol, made this not quite feasible.
The Scalar project learned from that and tackled the problem with different tactics: instead of pretending to Git that the working directory is fully populated, it _specifically_ teaches Git about partial clone (which is based on VFS for Git's cache server), about sparse checkout (which VFS for Git tried to do transparently, in the file system layer), and regularly runs maintenance tasks to keep the repository in a healthy state.
With partial clone, sparse checkout and git maintenance having been upstreamed, there is little left that scalar.exe does which git.exe cannot do. One such thing is that scalar clone <url> will automatically set up a partial, sparse clone, and configure known-helpful settings from the start.
So let's bring this convenience into Git's tree.
The idea here is that you can (optionally) build Scalar via
make -C contrib/scalar/
This will build the scalar executable and put it into the contrib/scalar/ subdirectory.
The slightly awkward addition of the contrib/scalar/* bits to the top-level Makefile are actually really required: we want to link to libgit.a, which means that we will need to use the very same CFLAGS and LDFLAGS as the rest of Git.
An early development version of this patch tried to replicate all the conditional code in contrib/scalar/Makefile (e.g. NO_POLL) just like contrib/svn-fe/Makefile used to do before it was retired. It turned out to be quite the whack-a-mole game: the SHA-1-related flags, the flags enabling/disabling compat/poll/, compat/regex/, compat/win32mmap.c & friends depending on the current platform... To put it mildly: it was a major mess.
Instead, this patch makes minimal changes to the top-level Makefile so that the bits in contrib/scalar/ can be compiled and linked, and adds a contrib/scalar/Makefile that uses the top-level Makefile in a most minimal way to do the actual compiling.
Note: With this commit, we only establish the infrastructure, no Scalar functionality is implemented yet; We will do that incrementally over the next few commits.
--- Makefile | 9 +++++++++ contrib/scalar/.gitignore | 2 ++ contrib/scalar/Makefile | 34 ++++++++++++++++++++++++++++++++++ contrib/scalar/scalar.c | 36 ++++++++++++++++++++++++++++++++++++ 4 files changed, 81 insertions(+) create mode 100644 contrib/scalar/.gitignore create mode 100644 contrib/scalar/Makefile create mode 100644 contrib/scalar/scalar.c
Makefile +9 -0
@@ -2456,6 +2456,11 @@ OBJECTS += $(FUZZ_OBJS) ifndef NO_CURL OBJECTS += http.o http-walker.o remote-curl.o endif + +SCALAR_SOURCES := contrib/scalar/scalar.c +SCALAR_OBJECTS := $(SCALAR_SOURCES:c=o) +OBJECTS += $(SCALAR_OBJECTS) + .PHONY: objects objects: $(OBJECTS)
@@ -2589,6 +2594,10 @@ $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS $(GITLIBS $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \ $(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS) +contrib/scalar/scalar$X: $(SCALAR_OBJECTS) GIT-LDFLAGS $(GITLIBS) + $(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \ + $(filter %.o,$^) $(LIBS) + $(LIB_FILE): $(LIB_OBJS) $(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^
contrib/scalar/.gitignore new file +2 -0
@@ -0,0 +1,2 @@ +/*.exe +/scalar
contrib/scalar/Makefile new file +34 -0
@@ -0,0 +1,34 @@ +QUIET_SUBDIR0 = +$(MAKE) -C # space to separate -C and subdir +QUIET_SUBDIR1 = + +ifneq ($(findstring s,$(MAKEFLAGS)),s) +ifndef V + QUIET_SUBDIR0 = +@subdir= + QUIET_SUBDIR1 = ;$(NO_SUBDIR) echo ' ' SUBDIR $$subdir; \ + $(MAKE) $(PRINT_DIR) -C $$subdir +else + export V +endif +endif + +all: + +include ../../config.mak.uname +-include ../../config.mak.autogen +-include ../../config.mak + +TARGETS = scalar$(X) scalar.o +GITLIBS = ../../common-main.o ../../libgit.a ../../xdiff/lib.a + +all: scalar$(X) + +$(GITLIBS): + $(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) $(subst ../../,,$@) + +$(TARGETS): $(GITLIBS) scalar.c + $(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) $(patsubst %,contrib/scalar/%,$@) + +clean: + $(RM) $(TARGETS) + +.PHONY: $(GITLIBS) all clean FORCE
contrib/scalar/scalar.c new file +36 -0
@@ -0,0 +1,36 @@ +/* +* The Scalar command-line interface. +*/ + +#include "cache.h" +#include "gettext.h" +#include "parse-options.h" + +static struct { +const char *name; +int (*fn)(int ,const char **); +} builtins[] = { + {NULL ,NULL }, +}; + +int cmd_main (int argc,const char **argv) +{ +struct strbuf scalar_usage =STRBUF_INIT ; +int i; + +if (argc >1 ) { + argv++; + argc--; + +for (i =0 ; builtins[i].name; i++) +if (!strcmp (builtins[i].name, argv[0 ])) +return !!builtins[i].fn (argc, argv); + } + +strbuf_addstr (&scalar_usage, +N_ ("scalar <command> [<options>]\n\nCommands:\n" )); +for (i =0 ; builtins[i].name; i++) +strbuf_addf (&scalar_usage,"\t%s\n" , builtins[i].name); + +usage (scalar_usage.buf); +}
Signature · 1 line
gitgitgadget
PATCH v10 3/15 scalar: start documenting the command
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
Let's build up the documentation for the Scalar command along with the patches that implement its functionality.
Note: To discourage the feature-incomplete documentation from being mistaken for the complete thing, we do not yet provide any way to build HTML or manual pages from the text file.
--- contrib/scalar/scalar.txt | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 contrib/scalar/scalar.txt
contrib/scalar/scalar.txt new file +38 -0
@@ -0,0 +1,38 @@ +scalar(1) +========= + +NAME +---- +scalar - an opinionated repository management tool + +SYNOPSIS +-------- +[verse] +scalar <command> [<options>] + +DESCRIPTION +----------- + +Scalar is an opinionated repository management tool. By creating new +repositories or registering existing repositories with Scalar, your Git +experience will speed up. Scalar sets advanced Git config settings, +maintains your repositories in the background, and helps reduce data sent +across the network. + +An important Scalar concept is the enlistment: this is the top-level directory +of the project. It usually contains the subdirectory `src/` which is a Git +worktree. This encourages the separation between tracked files (inside `src/`) +and untracked files, such as build artifacts (outside `src/`). When registering +an existing Git worktree with Scalar whose name is not `src`, the enlistment +will be identical to the worktree. + +The `scalar` command implements various subcommands, and different options +depending on the subcommand. + +SEE ALSO +-------- +linkgit:git-maintenance[1]. + +Scalar +--- +Associated with the linkgit:git[1] suite
Signature · 1 line
gitgitgadget
PATCH v10 4/15 scalar: create test infrastructure
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
To test the Scalar command, create a test script in contrib/scalar/t that is executed as make -C contrib/scalar test. Since Scalar has no meaningful capabilities yet, the only test is rather simple. We will add more tests in subsequent commits that introduce corresponding, new functionality.
Note: This test script is intended to test scalar only lightly, even after all of the functionality is implemented.
A more comprehensive functional (or: integration) test suite can be found at https://github.com/microsoft/scalar; It is used in the workflow
https://github.com/microsoft/git/blob/HEAD/.github/workflows/scalar-functional-tests.yml
in Microsoft's Git fork. This test suite performs end-to-end tests with a real remote repository, and is run as part of the regular CI and PR builds in that fork.
Since those tests require some functionality supported only by Microsoft's Git fork ("GVFS protocol"), there is no intention to port that fuller test suite to contrib/scalar/.
--- contrib/scalar/Makefile | 17 +++++-- contrib/scalar/t/Makefile | 78 ++++++++++++++++++++++++++++++++ contrib/scalar/t/t9099-scalar.sh | 17 +++++++ 3 files changed, 109 insertions(+), 3 deletions(-) create mode 100644 contrib/scalar/t/Makefile create mode 100755 contrib/scalar/t/t9099-scalar.sh
contrib/scalar/Makefile +14 -3
@@ -3,6 +3,7 @@ QUIET_SUBDIR1 = ifneq ($(findstring s,$(MAKEFLAGS)),s) ifndef V + QUIET_GEN = @echo ' ' GEN $@; QUIET_SUBDIR0 = +@subdir= QUIET_SUBDIR1 = ;$(NO_SUBDIR) echo ' ' SUBDIR $$subdir; \ $(MAKE) $(PRINT_DIR) -C $$subdir
@@ -20,7 +21,7 @@ include ../../config.mak.uname TARGETS = scalar$(X) scalar.o GITLIBS = ../../common-main.o ../../libgit.a ../../xdiff/lib.a -all: scalar$(X) +all: scalar$(X) ../../bin-wrappers/scalar $(GITLIBS): $(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) $(subst ../../,,$@)
@@ -29,6 +30,16 @@ $(TARGETS): $(GITLIBS) scalar.c $(QUIET_SUBDIR0)../.. $(QUIET_SUBDIR1) $(patsubst %,contrib/scalar/%,$@) clean: - $(RM) $(TARGETS) + $(RM) $(TARGETS) ../../bin-wrappers/scalar -.PHONY: $(GITLIBS) all clean FORCE +../../bin-wrappers/scalar: ../../wrap-for-bin.sh Makefile + @mkdir -p ../../bin-wrappers + $(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \ + -e 's|@@BUILD_DIR@@|$(shell cd ../.. && pwd)|' \ + -e 's|@@PROG@@|contrib/scalar/scalar$(X)|' < $< > $@ && \ + chmod +x $@ + +test: all + $(MAKE) -C t + +.PHONY: $(GITLIBS) all clean test FORCE
contrib/scalar/t/Makefile new file +78 -0
78 lines not on this page · raw message
contrib/scalar/t/t9099-scalar.sh new file +17 -0
@@ -0,0 +1,17 @@ +#!/bin/sh + +test_description='test the `scalar` command' + +TEST_DIRECTORY=$PWD/../../../t +export TEST_DIRECTORY + +# Make it work with --no-bin-wrappers +PATH=$PWD/..:$PATH + +. ../../../t/test-lib.sh + +test_expect_success 'scalar shows a usage' ' + test_expect_code 129 scalar -h +' + +test_done
Signature · 1 line
gitgitgadget
PATCH v10 5/15 scalar: 'register' sets recommended config and starts maintenance
In reply to Johannes Schindelin via GitGitGadgetFrom: Derrick Stolee <[email protected]>
Let's start implementing the register command. With this commit, recommended settings are configured upon scalar register, and Git's background maintenance is started.
The recommended config settings may very well change in the future. For example, once the built-in FSMonitor is available, we will want to enable it upon scalar register. For that reason, we explicitly support running scalar register in an already-registered enlistment.
--- contrib/scalar/scalar.c | 249 ++++++++++++++++++++++++++++++++++++++ contrib/scalar/scalar.txt | 18 ++- 2 files changed, 266 insertions(+), 1 deletion(-)
contrib/scalar/scalar.c +249 -0
@@ -5,11 +5,260 @@#include "cache.h" #include "gettext.h" #include "parse-options.h" +#include "config.h" +#include "run-command.h" + +/* +* Remove the deepest subdirectory in the provided path string. Path must not +* include a trailing path separator. Returns 1 if parent directory found, +* otherwise 0. +*/ +static int strbuf_parent_directory (struct strbuf *buf) +{ +size_t len = buf->len; +size_t offset =offset_1st_component (buf->buf); +char *path_sep =find_last_dir_sep (buf->buf + offset); +strbuf_setlen (buf, path_sep ? path_sep - buf->buf : offset); + +return buf->len < len; +} + +static void setup_enlistment_directory (int argc,const char **argv, +const char *const *usagestr, +const struct option *options, +struct strbuf *enlistment_root) +{ +struct strbuf path =STRBUF_INIT ; +char *root; +int enlistment_found =0 ; + +if (startup_info->have_repository) +BUG ("gitdir already set up?!?" ); + +if (argc >1 ) +usage_with_options (usagestr, options); + +/* find the worktree, determine its corresponding root */ +if (argc ==1 ) +strbuf_add_absolute_path (&path, argv[0 ]); +else if (strbuf_getcwd (&path) <0 ) +die (_ ("need a working directory" )); + +strbuf_trim_trailing_dir_sep (&path); +do { +const size_t len = path.len; + +/* check if currently in enlistment root with src/ workdir */ +strbuf_addstr (&path,"/src" ); +if (is_nonbare_repository_dir (&path)) { +if (enlistment_root) +strbuf_add (enlistment_root, path.buf, len); + + enlistment_found =1 ; +break ; + } + +/* reset to original path */ +strbuf_setlen (&path, len); + +/* check if currently in workdir */ +if (is_nonbare_repository_dir (&path)) { +if (enlistment_root) { +/* +* If the worktree's directory's name is `src`, the enlistment is the +* parent directory, otherwise it is identical to the worktree. +*/ + root =strip_path_suffix (path.buf,"src" ); +strbuf_addstr (enlistment_root, root ? root : path.buf); +free (root); + } + + enlistment_found =1 ; +break ; + } + }while (strbuf_parent_directory (&path)); + +if (!enlistment_found) +die (_ ("could not find enlistment root" )); + +if (chdir (path.buf) <0 ) +die_errno (_ ("could not switch to '%s'" ), path.buf); + +strbuf_release (&path); +setup_git_directory (); +} + +static int run_git (const char *arg, ...) +{ +struct strvec argv =STRVEC_INIT ; +va_list args; +const char *p; +int res; + +va_start (args, arg); +strvec_push (&argv, arg); + while ((p = va_arg(args, const char *))) +strvec_push (&argv, p); +va_end (args); + + res =run_command_v_opt (argv.v,RUN_GIT_CMD ); + +strvec_clear (&argv); +return res; +} + +static int set_recommended_config (void ) +{ +struct { +const char *key; +const char *value; + } config[] = { + {"am.keepCR" ,"true" }, + {"core.FSCache" ,"true" }, + {"core.multiPackIndex" ,"true" }, + {"core.preloadIndex" ,"true" }, +#ifndef WIN32 + {"core.untrackedCache" ,"true" }, +#else +/* +* Unfortunately, Scalar's Functional Tests demonstrated +* that the untracked cache feature is unreliable on Windows +* (which is a bummer because that platform would benefit the +* most from it). For some reason, freshly created files seem +* not to update the directory's `lastModified` time +* immediately, but the untracked cache would need to rely on +* that. +* +* Therefore, with a sad heart, we disable this very useful +* feature on Windows. +*/ + {"core.untrackedCache" ,"false" }, +#endif + {"core.logAllRefUpdates" ,"true" }, + {"credential.https://dev.azure.com.useHttpPath" ,"true" }, + {"credential.validate" ,"false" },/* GCM4W-only */ + {"gc.auto" ,"0" }, + {"gui.GCWarning" ,"false" }, + {"index.threads" ,"true" }, + {"index.version" ,"4" }, + {"merge.stat" ,"false" }, + {"merge.renames" ,"true" }, + {"pack.useBitmaps" ,"false" }, + {"pack.useSparse" ,"true" }, + {"receive.autoGC" ,"false" }, + {"reset.quiet" ,"true" }, + {"feature.manyFiles" ,"false" }, + {"feature.experimental" ,"false" }, + {"fetch.unpackLimit" ,"1" }, + {"fetch.writeCommitGraph" ,"false" }, +#ifdef WIN32 + {"http.sslBackend" ,"schannel" }, +#endif + {"status.aheadBehind" ,"false" }, + {"commitGraph.generationVersion" ,"1" }, + {"core.autoCRLF" ,"false" }, + {"core.safeCRLF" ,"false" }, + {"fetch.showForcedUpdates" ,"false" }, + {NULL ,NULL }, + }; +int i; +char *value; + +for (i =0 ; config[i].key; i++) { +if (git_config_get_string (config[i].key, &value)) { +trace2_data_string ("scalar" , the_repository, config[i].key,"created" ); +if (git_config_set_gently (config[i].key, + config[i].value) <0 ) +return error (_ ("could not configure %s=%s" ), + config[i].key, config[i].value); + }else { +trace2_data_string ("scalar" , the_repository, config[i].key,"exists" ); +free (value); + } + } + +/* +* The `log.excludeDecoration` setting is special because it allows +* for multiple values. +*/ +if (git_config_get_string ("log.excludeDecoration" , &value)) { +trace2_data_string ("scalar" , the_repository, +"log.excludeDecoration" ,"created" ); +if (git_config_set_multivar_gently ("log.excludeDecoration" , +"refs/prefetch/*" , +CONFIG_REGEX_NONE ,0 )) +return error (_ ("could not configure " +"log.excludeDecoration" )); + }else { +trace2_data_string ("scalar" , the_repository, +"log.excludeDecoration" ,"exists" ); +free (value); + } + +return 0 ; +} + +static int start_maintenance (void ) +{ +return run_git ("maintenance" ,"start" ,NULL ); +} + +static int add_enlistment (void ) +{ +int res; + +if (!the_repository->worktree) +die (_ ("Scalar enlistments require a worktree" )); + + res =run_git ("config" ,"--global" ,"--get" ,"--fixed-value" , +"scalar.repo" , the_repository->worktree,NULL ); + +/* +* If the setting is already there, then do nothing. +*/ +if (!res) +return 0 ; + +return run_git ("config" ,"--global" ,"--add" , +"scalar.repo" , the_repository->worktree,NULL ); +} + +static int register_dir (void ) +{ +int res =add_enlistment (); + +if (!res) + res =set_recommended_config (); + +if (!res) + res =start_maintenance (); + +return res; +} + +static int cmd_register (int argc,const char **argv) +{ +struct option options[] = { +OPT_END (), + }; +const char *const usage[] = { +N_ ("scalar register [<enlistment>]" ), +NULL + }; + + argc =parse_options (argc, argv,NULL , options, + usage,0 ); + +setup_enlistment_directory (argc, argv, usage, options,NULL ); + +return register_dir (); +}static struct {const char *name;int (*fn)(int ,const char **); } builtins[] = { + {"register" , cmd_register }, {NULL ,NULL }, };
contrib/scalar/scalar.txt +17 -1
@@ -8,7 +8,7 @@ scalar - an opinionated repository management tool SYNOPSIS -------- [verse] -scalar <command> [<options>] +scalar register [<enlistment>] DESCRIPTION -----------
@@ -29,6 +29,22 @@ will be identical to the worktree. The `scalar` command implements various subcommands, and different options depending on the subcommand. +COMMANDS +-------- + +Register +~~~~~~~~ + +register [<enlistment>]:: + Adds the enlistment's repository to the list of registered repositories + and starts background maintenance. If `<enlistment>` is not provided, + then the enlistment associated with the current working directory is + registered. ++ +Note: when this subcommand is called in a worktree that is called `src/`, its +parent directory is considered to be the Scalar enlistment. If the worktree is +_not_ called `src/`, it itself will be considered to be the Scalar enlistment. + SEE ALSO -------- linkgit:git-maintenance[1].
Signature · 1 line
gitgitgadget
PATCH v10 6/15 scalar: 'unregister' stops background maintenance
In reply to Johannes Schindelin via GitGitGadgetFrom: Derrick Stolee <[email protected]>
Just like scalar register starts the scheduled background maintenance, scalar unregister stops it. Note that we use git maintenance start in scalar register, but we do not use git maintenance stop in scalar unregister: this would stop maintenance for _all_ repositories, not just for the one we want to unregister.
The unregister command also removes the corresponding entry from the [scalar] section in the global Git config.
--- contrib/scalar/scalar.c | 50 ++++++++++++++++++++++++++++++++------- contrib/scalar/scalar.txt | 8 +++++++ 2 files changed, 50 insertions(+), 8 deletions(-)
contrib/scalar/scalar.c +42 -8
@@ -198,12 +198,12 @@ static int set_recommended_config(void)return 0 ; } -static int start_maintenance (void ) +static int toggle_maintenance (int enable) { -return run_git ("maintenance" ,"start" ,NULL ); +return run_git ("maintenance" , enable ?"start" :"unregister" ,NULL ); } -static int add_enlistment (void ) +static int add_or_remove_enlistment (int add) {int res;
@@ -214,24 +214,39 @@ static int add_enlistment(void) "scalar.repo", the_repository->worktree, NULL);/* -* If the setting is already there, then do nothing. +* If we want to add and the setting is already there, then do nothing. +* If we want to remove and the setting is not there, then do nothing. */ -if (!res) +if ((add && !res) || (!add && res))return 0 ; -return run_git ("config" ,"--global" ,"--add" , +return run_git ("config" ,"--global" , add ?"--add" :"--unset" , + add ?"--no-fixed-value" :"--fixed-value" ,"scalar.repo" , the_repository->worktree,NULL ); }static int register_dir (void ) { -int res =add_enlistment (); +int res =add_or_remove_enlistment (1 );if (!res) res =set_recommended_config ();if (!res) - res =start_maintenance (); + res =toggle_maintenance (1 ); + +return res; +} + +static int unregister_dir (void ) +{ +int res =0 ; + +if (toggle_maintenance (0 ) <0 ) + res =-1 ; + +if (add_or_remove_enlistment (0 ) <0 ) + res =-1 ;return res; }
@@ -254,11 +269,30 @@ static int cmd_register(int argc, const char **argv)return register_dir (); } +static int cmd_unregister (int argc,const char **argv) +{ +struct option options[] = { +OPT_END (), + }; +const char *const usage[] = { +N_ ("scalar unregister [<enlistment>]" ), +NULL + }; + + argc =parse_options (argc, argv,NULL , options, + usage,0 ); + +setup_enlistment_directory (argc, argv, usage, options,NULL ); + +return unregister_dir (); +} +static struct {const char *name;int (*fn)(int ,const char **); } builtins[] = { {"register" , cmd_register }, + {"unregister" , cmd_unregister }, {NULL ,NULL }, };
contrib/scalar/scalar.txt +8 -0
@@ -9,6 +9,7 @@ SYNOPSIS -------- [verse] scalar register [<enlistment>] +scalar unregister [<enlistment>] DESCRIPTION -----------
@@ -45,6 +46,13 @@ Note: when this subcommand is called in a worktree that is called `src/`, its parent directory is considered to be the Scalar enlistment. If the worktree is _not_ called `src/`, it itself will be considered to be the Scalar enlistment. +Unregister +~~~~~~~~~~ + +unregister [<enlistment>]:: + Remove the specified repository from the list of repositories + registered with Scalar and stop the scheduled background maintenance. + SEE ALSO -------- linkgit:git-maintenance[1].
Signature · 1 line
gitgitgadget
PATCH v10 7/15 scalar: let 'unregister' handle a deleted enlistment directory gracefully
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
When a user deleted an enlistment manually, let's be generous and _still_ unregister it.
--- contrib/scalar/scalar.c | 46 ++++++++++++++++++++++++++++++++ contrib/scalar/t/t9099-scalar.sh | 15 +++++++++++ 2 files changed, 61 insertions(+)
contrib/scalar/scalar.c +46 -0
@@ -269,6 +269,24 @@ static int cmd_register(int argc, const char **argv)return register_dir (); } +static int remove_deleted_enlistment (struct strbuf *path) +{ +int res =0 ; +strbuf_realpath_forgiving (path, path->buf,1 ); + +if (run_git ("config" ,"--global" , +"--unset" ,"--fixed-value" , +"scalar.repo" , path->buf,NULL ) <0 ) + res =-1 ; + +if (run_git ("config" ,"--global" , +"--unset" ,"--fixed-value" , +"maintenance.repo" , path->buf,NULL ) <0 ) + res =-1 ; + +return res; +} +static int cmd_unregister (int argc,const char **argv) {struct option options[] = {
@@ -282,6 +300,34 @@ static int cmd_unregister(int argc, const char **argv) argc =parse_options (argc, argv,NULL , options, usage,0 ); +/* +* Be forgiving when the enlistment or worktree does not even exist any +* longer; This can be the case if a user deleted the worktree by +* mistake and _still_ wants to unregister the thing. +*/ +if (argc ==1 ) { +struct strbuf src_path =STRBUF_INIT , workdir_path =STRBUF_INIT ; + +strbuf_addf (&src_path,"%s/src/.git" , argv[0 ]); +strbuf_addf (&workdir_path,"%s/.git" , argv[0 ]); +if (!is_directory (src_path.buf) && !is_directory (workdir_path.buf)) { +/* remove possible matching registrations */ +int res =-1 ; + +strbuf_strip_suffix (&src_path,"/.git" ); + res =remove_deleted_enlistment (&src_path) && res; + +strbuf_strip_suffix (&workdir_path,"/.git" ); + res =remove_deleted_enlistment (&workdir_path) && res; + +strbuf_release (&src_path); +strbuf_release (&workdir_path); +return res; + } +strbuf_release (&src_path); +strbuf_release (&workdir_path); + } +setup_enlistment_directory (argc, argv, usage, options,NULL );return unregister_dir ();
contrib/scalar/t/t9099-scalar.sh +15 -0
@@ -14,4 +14,19 @@ test_expect_success 'scalar shows a usage' ' test_expect_code 129 scalar -h ' +test_expect_success 'scalar unregister' ' + git init vanish/src && + scalar register vanish/src && + git config --get --global --fixed-value \ + maintenance.repo "$(pwd)/vanish/src" && + scalar list >scalar.repos && + grep -F "$(pwd)/vanish/src" scalar.repos && + rm -rf vanish/src/.git && + scalar unregister vanish && + test_must_fail git config --get --global --fixed-value \ + maintenance.repo "$(pwd)/vanish/src" && + scalar list >scalar.repos && + ! grep -F "$(pwd)/vanish/src" scalar.repos +' + test_done
Signature · 1 line
gitgitgadget
PATCH v10 8/15 scalar: implement 'scalar list'
In reply to Johannes Schindelin via GitGitGadgetFrom: Derrick Stolee <[email protected]>
The produced list simply consists of those repositories registered under the multi-valued scalar.repo config setting in the user's Git config.
--- contrib/scalar/scalar.c | 11 +++++++++++ contrib/scalar/scalar.txt | 11 ++++++++++- 2 files changed, 21 insertions(+), 1 deletion(-)
contrib/scalar/scalar.c +11 -0
@@ -251,6 +251,16 @@ static int unregister_dir(void)return res; } +static int cmd_list (int argc,const char **argv) +{ +if (argc !=1 ) +die (_ ("`scalar list` does not take arguments" )); + +if (run_git ("config" ,"--global" ,"--get-all" ,"scalar.repo" ,NULL ) <0 ) +return -1 ; +return 0 ; +} +static int cmd_register (int argc,const char **argv) {struct option options[] = {
@@ -337,6 +347,7 @@ static struct {const char *name;int (*fn)(int ,const char **); } builtins[] = { + { "list", cmd_list }, { "register", cmd_register }, { "unregister", cmd_unregister }, { NULL, NULL},
contrib/scalar/scalar.txt +10 -1
@@ -8,6 +8,7 @@ scalar - an opinionated repository management tool SYNOPSIS -------- [verse] +scalar list scalar register [<enlistment>] scalar unregister [<enlistment>]
@@ -28,11 +29,19 @@ an existing Git worktree with Scalar whose name is not `src`, the enlistment will be identical to the worktree. The `scalar` command implements various subcommands, and different options -depending on the subcommand. +depending on the subcommand. With the exception of `list`, all subcommands +expect to be run in an enlistment. COMMANDS -------- +List +~~~~ + +list:: + List enlistments that are currently registered by Scalar. This + subcommand does not need to be run inside an enlistment. + Register ~~~~~~~~
Signature · 1 line
gitgitgadget
PATCH v10 9/15 scalar: implement the `clone` subcommand
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
This implements Scalar's opinionated clone command: it tries to use a partial clone and sets up a sparse checkout by default. In contrast to git clone, scalar clone sets up the worktree in the src/ subdirectory, to encourage a separation between the source files and the build output (which helps Git tremendously because it avoids untracked files that have to be specifically ignored when refreshing the index).
Also, it registers the repository for regular, scheduled maintenance, and configures a flurry of configuration settings based on the experience and experiments of the Microsoft Windows and the Microsoft Office development teams.
Note: since the scalar clone command is by far the most commonly called scalar subcommand, we document it at the top of the manual page.
--- contrib/scalar/scalar.c | 201 +++++++++++++++++++++++++++++++ contrib/scalar/scalar.txt | 32 ++++- contrib/scalar/t/t9099-scalar.sh | 32 +++++ 3 files changed, 262 insertions(+), 3 deletions(-)
contrib/scalar/scalar.c +201 -0
@@ -7,6 +7,7 @@#include "parse-options.h" #include "config.h" #include "run-command.h" +#include "refs.h" /* * Remove the deepest subdirectory in the provided path string. Path must not
@@ -251,6 +252,205 @@ static int unregister_dir(void)return res; } +/* printf-style interface, expects `<key>=<value>` argument */ +static int set_config (const char *fmt, ...) +{ +struct strbuf buf =STRBUF_INIT ; +char *value; +int res; +va_list args; + +va_start (args, fmt); +strbuf_vaddf (&buf, fmt, args); +va_end (args); + + value =strchr (buf.buf,'=' ); +if (value) + *(value++) ='\0' ; + res =git_config_set_gently (buf.buf, value); +strbuf_release (&buf); + +return res; +} + +static char *remote_default_branch (const char *url) +{ +struct child_process cp =CHILD_PROCESS_INIT ; +struct strbuf out =STRBUF_INIT ; + + cp.git_cmd =1 ; +strvec_pushl (&cp.args,"ls-remote" ,"--symref" , url,"HEAD" ,NULL ); +if (!pipe_command (&cp,NULL ,0 , &out,0 ,NULL ,0 )) { +const char *line = out.buf; + +while (*line) { +const char *eol =strchrnul (line,'\n' ), *p; +size_t len = eol - line; +char *branch; + +if (!skip_prefix (line,"ref: " , &p) || + !strip_suffix_mem (line, &len,"\tHEAD" )) { + line = eol + (*eol =='\n' ); +continue ; + } + + eol = line + len; +if (skip_prefix (p,"refs/heads/" , &p)) { + branch =xstrndup (p, eol - p); +strbuf_release (&out); +return branch; + } + +error (_ ("remote HEAD is not a branch: '%.*s'" ), + (int )(eol - p), p); +strbuf_release (&out); +return NULL ; + } + } +warning (_ ("failed to get default branch name from remote; " +"using local default" )); +strbuf_reset (&out); + +child_process_init (&cp); + cp.git_cmd =1 ; +strvec_pushl (&cp.args,"symbolic-ref" ,"--short" ,"HEAD" ,NULL ); +if (!pipe_command (&cp,NULL ,0 , &out,0 ,NULL ,0 )) { +strbuf_trim (&out); +return strbuf_detach (&out,NULL ); + } + +strbuf_release (&out); +error (_ ("failed to get default branch name" )); +return NULL ; +} + +static int cmd_clone (int argc,const char **argv) +{ +const char *branch =NULL ; +int full_clone =0 ; +struct option clone_options[] = { +OPT_STRING ('b' ,"branch" , &branch,N_ ("<branch>" ), +N_ ("branch to checkout after clone" )), +OPT_BOOL (0 ,"full-clone" , &full_clone, +N_ ("when cloning, create full working directory" )), +OPT_END (), + }; +const char *const clone_usage[] = { +N_ ("scalar clone [<options>] [--] <repo> [<dir>]" ), +NULL + }; +const char *url; +char *enlistment =NULL , *dir =NULL ; +struct strbuf buf =STRBUF_INIT ; +int res; + + argc =parse_options (argc, argv,NULL , clone_options, clone_usage,0 ); + +if (argc ==2 ) { + url = argv[0 ]; + enlistment =xstrdup (argv[1 ]); + }else if (argc ==1 ) { + url = argv[0 ]; + +strbuf_addstr (&buf, url); +/* Strip trailing slashes, if any */ +while (buf.len >0 &&is_dir_sep (buf.buf[buf.len -1 ])) +strbuf_setlen (&buf, buf.len -1 ); +/* Strip suffix `.git`, if any */ +strbuf_strip_suffix (&buf,".git" ); + + enlistment =find_last_dir_sep (buf.buf); +if (!enlistment) { +die (_ ("cannot deduce worktree name from '%s'" ), url); + } + enlistment =xstrdup (enlistment +1 ); + }else { +usage_msg_opt (_ ("You must specify a repository to clone." ), + clone_usage, clone_options); + } + +if (is_directory (enlistment)) +die (_ ("directory '%s' exists already" ), enlistment); + + dir =xstrfmt ("%s/src" , enlistment); + +strbuf_reset (&buf); +if (branch) +strbuf_addf (&buf,"init.defaultBranch=%s" , branch); +else { +char *b =repo_default_branch_name (the_repository,1 ); +strbuf_addf (&buf,"init.defaultBranch=%s" , b); +free (b); + } + +if ((res =run_git ("-c" , buf.buf,"init" ,"--" , dir,NULL ))) + goto cleanup; + +if (chdir (dir) <0 ) { + res =error_errno (_ ("could not switch to '%s'" ), dir); + goto cleanup; + } + +setup_git_directory (); + +/* common-main already logs `argv` */ +trace2_def_repo (the_repository); + +if (!branch && !(branch =remote_default_branch (url))) { + res =error (_ ("failed to get default branch for '%s'" ), url); + goto cleanup; + } + +if (set_config ("remote.origin.url=%s" , url) || +set_config ("remote.origin.fetch=" +"+refs/heads/*:refs/remotes/origin/*" ) || +set_config ("remote.origin.promisor=true" ) || +set_config ("remote.origin.partialCloneFilter=blob:none" )) { + res =error (_ ("could not configure remote in '%s'" ), dir); + goto cleanup; + } + +if (!full_clone && + (res =run_git ("sparse-checkout" ,"init" ,"--cone" ,NULL ))) + goto cleanup; + +if (set_recommended_config ()) +return error (_ ("could not configure '%s'" ), dir); + +if ((res =run_git ("fetch" ,"--quiet" ,"origin" ,NULL ))) { +warning (_ ("partial clone failed; attempting full clone" )); + +if (set_config ("remote.origin.promisor" ) || +set_config ("remote.origin.partialCloneFilter" )) { + res =error (_ ("could not configure for full clone" )); + goto cleanup; + } + +if ((res =run_git ("fetch" ,"--quiet" ,"origin" ,NULL ))) + goto cleanup; + } + +if ((res =set_config ("branch.%s.remote=origin" , branch))) + goto cleanup; +if ((res =set_config ("branch.%s.merge=refs/heads/%s" , + branch, branch))) + goto cleanup; + +strbuf_reset (&buf); +strbuf_addf (&buf,"origin/%s" , branch); + res =run_git ("checkout" ,"-f" ,"-t" , buf.buf,NULL ); +if (res) + goto cleanup; + + res =register_dir (); + +cleanup: +free (enlistment); +free (dir); +strbuf_release (&buf); +return res; +} +static int cmd_list (int argc,const char **argv) {if (argc !=1 )
@@ -347,6 +547,7 @@ static struct {const char *name;int (*fn)(int ,const char **); } builtins[] = { + { "clone", cmd_clone }, { "list", cmd_list }, { "register", cmd_register }, { "unregister", cmd_unregister },
contrib/scalar/scalar.txt +29 -3
@@ -8,6 +8,7 @@ scalar - an opinionated repository management tool SYNOPSIS -------- [verse] +scalar clone [--branch <main-branch>] [--full-clone] <url> [<enlistment>] scalar list scalar register [<enlistment>] scalar unregister [<enlistment>]
@@ -29,12 +30,37 @@ an existing Git worktree with Scalar whose name is not `src`, the enlistment will be identical to the worktree. The `scalar` command implements various subcommands, and different options -depending on the subcommand. With the exception of `list`, all subcommands -expect to be run in an enlistment. +depending on the subcommand. With the exception of `clone` and `list`, all +subcommands expect to be run in an enlistment. COMMANDS -------- +Clone +~~~~~ + +clone [<options>] <url> [<enlistment>]:: + Clones the specified repository, similar to linkgit:git-clone[1]. By + default, only commit and tree objects are cloned. Once finished, the + worktree is located at `<enlistment>/src`. ++ +The sparse-checkout feature is enabled (except when run with `--full-clone`) +and the only files present are those in the top-level directory. Use +`git sparse-checkout set` to expand the set of directories you want to see, +or `git sparse-checkout disable` to expand to all files (see +linkgit:git-sparse-checkout[1] for more details). You can explore the +subdirectories outside your sparse-checkout by using `git ls-tree +HEAD[:<directory>]`. + +-b <name>:: +--branch <name>:: + Instead of checking out the branch pointed to by the cloned + repository's HEAD, check out the `<name>` branch instead. + +--[no-]full-clone:: + A sparse-checkout is initialized by default. This behavior can be + turned off via `--full-clone`. + List ~~~~
@@ -64,7 +90,7 @@ unregister [<enlistment>]:: SEE ALSO -------- -linkgit:git-maintenance[1]. +linkgit:git-clone[1], linkgit:git-maintenance[1]. Scalar ---
contrib/scalar/t/t9099-scalar.sh +32 -0
@@ -10,6 +10,9 @@ PATH=$PWD/..:$PATH . ../../../t/test-lib.sh +GIT_TEST_MAINT_SCHEDULER="crontab:test-tool crontab ../cron.txt,launchctl:true,schtasks:true" +export GIT_TEST_MAINT_SCHEDULER + test_expect_success 'scalar shows a usage' ' test_expect_code 129 scalar -h '
@@ -29,4 +32,33 @@ test_expect_success 'scalar unregister' ' ! grep -F "$(pwd)/vanish/src" scalar.repos ' +test_expect_success 'set up repository to clone' ' + test_commit first && + test_commit second && + test_commit third && + git switch -c parallel first && + mkdir -p 1/2 && + test_commit 1/2/3 && + git config uploadPack.allowFilter true && + git config uploadPack.allowAnySHA1InWant true +' + +test_expect_success 'scalar clone' ' + second=$(git rev-parse --verify second:second.t) && + scalar clone "file://$(pwd)" cloned && + ( + cd cloned/src && + + git config --get --global --fixed-value maintenance.repo \ + "$(pwd)" && + + test_path_is_missing 1/2 && + test_must_fail git rev-list --missing=print $second && + git rev-list $second && + git cat-file blob $second >actual && + echo "second" >expect && + test_cmp expect actual + ) +' + test_done
Signature · 1 line
gitgitgadget
PATCH v10 10/15 scalar: teach 'clone' to support the --single-branch option
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
Just like git clone, the scalar clone command now also offers to restrict the clone to a single branch.
--- contrib/scalar/scalar.c | 9 +++++++-- contrib/scalar/scalar.txt | 12 +++++++++++- contrib/scalar/t/t9099-scalar.sh | 6 +++++- 3 files changed, 23 insertions(+), 4 deletions(-)
contrib/scalar/scalar.c +7 -2
@@ -327,12 +327,15 @@ static char *remote_default_branch(const char *url)static int cmd_clone (int argc,const char **argv) {const char *branch =NULL ; -int full_clone =0 ; +int full_clone =0 , single_branch =0 ;struct option clone_options[] = {OPT_STRING ('b' ,"branch" , &branch,N_ ("<branch>" ),N_ ("branch to checkout after clone" )),OPT_BOOL (0 ,"full-clone" , &full_clone,N_ ("when cloning, create full working directory" )), +OPT_BOOL (0 ,"single-branch" , &single_branch, +N_ ("only download metadata for the branch that will " +"be checked out" )),OPT_END (), };const char *const clone_usage[] = {
@@ -403,7 +406,9 @@ static int cmd_clone(int argc, const char **argv)if (set_config ("remote.origin.url=%s" , url) ||set_config ("remote.origin.fetch=" -"+refs/heads/*:refs/remotes/origin/*" ) || +"+refs/heads/%s:refs/remotes/origin/%s" , + single_branch ? branch :"*" , + single_branch ? branch :"*" ) ||set_config ("remote.origin.promisor=true" ) ||set_config ("remote.origin.partialCloneFilter=blob:none" )) { res =error (_ ("could not configure remote in '%s'" ), dir);
contrib/scalar/scalar.txt +11 -1
@@ -8,7 +8,7 @@ scalar - an opinionated repository management tool SYNOPSIS -------- [verse] -scalar clone [--branch <main-branch>] [--full-clone] <url> [<enlistment>] +scalar clone [--single-branch] [--branch <main-branch>] [--full-clone] <url> [<enlistment>] scalar list scalar register [<enlistment>] scalar unregister [<enlistment>]
@@ -57,6 +57,16 @@ HEAD[:<directory>]`. Instead of checking out the branch pointed to by the cloned repository's HEAD, check out the `<name>` branch instead. +--[no-]single-branch:: + Clone only the history leading to the tip of a single branch, either + specified by the `--branch` option or the primary branch remote's + `HEAD` points at. ++ +Further fetches into the resulting repository will only update the +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-]full-clone:: A sparse-checkout is initialized by default. This behavior can be turned off via `--full-clone`.
contrib/scalar/t/t9099-scalar.sh +5 -1
@@ -45,13 +45,17 @@ test_expect_success 'set up repository to clone' ' test_expect_success 'scalar clone' ' second=$(git rev-parse --verify second:second.t) && - scalar clone "file://$(pwd)" cloned && + scalar clone "file://$(pwd)" cloned --single-branch && ( cd cloned/src && git config --get --global --fixed-value maintenance.repo \ "$(pwd)" && + git for-each-ref --format="%(refname)" refs/remotes/origin/ >actual && + echo "refs/remotes/origin/parallel" >expect && + test_cmp expect actual && + test_path_is_missing 1/2 && test_must_fail git rev-list --missing=print $second && git rev-list $second &&
Signature · 1 line
gitgitgadget
PATCH v10 11/15 scalar: implement the `run` command
In reply to Johannes Schindelin via GitGitGadgetFrom: Derrick Stolee <[email protected]>
Note: this subcommand is provided primarily for backwards-compatibility, for existing Scalar uses. It is mostly just a shim for git maintenance, mapping task names from the way Scalar called them to the way Git calls them.
The reason why those names differ? The background maintenance was first implemented in Scalar, and when it was contributed as a patch series implementing the git maintenance command, reviewers suggested better names, those suggestions were accepted before the patches were integrated into core Git.
--- contrib/scalar/scalar.c | 64 +++++++++++++++++++++++++++++++++++++++ contrib/scalar/scalar.txt | 19 ++++++++++++ 2 files changed, 83 insertions(+)
contrib/scalar/scalar.c +64 -0
@@ -484,6 +484,69 @@ static int cmd_register(int argc, const char **argv)return register_dir (); } +static int cmd_run (int argc,const char **argv) +{ +struct option options[] = { +OPT_END (), + }; +struct { +const char *arg, *task; + } tasks[] = { + {"config" ,NULL }, + {"commit-graph" ,"commit-graph" }, + {"fetch" ,"prefetch" }, + {"loose-objects" ,"loose-objects" }, + {"pack-files" ,"incremental-repack" }, + {NULL ,NULL } + }; +struct strbuf buf =STRBUF_INIT ; +const char *usagestr[] = {NULL ,NULL }; +int i; + +strbuf_addstr (&buf,N_ ("scalar run <task> [<enlistment>]\nTasks:\n" )); +for (i =0 ; tasks[i].arg; i++) +strbuf_addf (&buf,"\t%s\n" , tasks[i].arg); + usagestr[0 ] = buf.buf; + + argc =parse_options (argc, argv,NULL , options, + usagestr,0 ); + +if (!argc) +usage_with_options (usagestr, options); + +if (!strcmp ("all" , argv[0 ])) { + i =-1 ; + }else { +for (i =0 ; tasks[i].arg &&strcmp (tasks[i].arg, argv[0 ]); i++) + ;/* keep looking for the task */ + +if (i >0 && !tasks[i].arg) { +error (_ ("no such task: '%s'" ), argv[0 ]); +usage_with_options (usagestr, options); + } + } + + argc--; + argv++; +setup_enlistment_directory (argc, argv, usagestr, options,NULL ); +strbuf_release (&buf); + +if (i ==0 ) +return register_dir (); + +if (i >0 ) +return run_git ("maintenance" ,"run" , +"--task" , tasks[i].task,NULL ); + +if (register_dir ()) +return -1 ; +for (i =1 ; tasks[i].arg; i++) +if (run_git ("maintenance" ,"run" , +"--task" , tasks[i].task,NULL )) +return -1 ; +return 0 ; +} +static int remove_deleted_enlistment (struct strbuf *path) {int res =0 ;
@@ -556,6 +619,7 @@ static struct { { "list", cmd_list }, { "register", cmd_register }, { "unregister", cmd_unregister }, + { "run", cmd_run }, { NULL, NULL}, };
contrib/scalar/scalar.txt +19 -0
@@ -12,6 +12,7 @@ scalar clone [--single-branch] [--branch <main-branch>] [--full-clone] <url> [<e scalar list scalar register [<enlistment>] scalar unregister [<enlistment>] +scalar run ( all | config | commit-graph | fetch | loose-objects | pack-files ) [<enlistment>] DESCRIPTION -----------
@@ -98,6 +99,24 @@ unregister [<enlistment>]:: Remove the specified repository from the list of repositories registered with Scalar and stop the scheduled background maintenance. +Run +~~~ + +scalar run ( all | config | commit-graph | fetch | loose-objects | pack-files ) [<enlistment>]:: + Run the given maintenance task (or all tasks, if `all` was specified). + Except for `all` and `config`, this subcommand simply hands off to + linkgit:git-maintenance[1] (mapping `fetch` to `prefetch` and + `pack-files` to `incremental-repack`). ++ +These tasks are run automatically as part of the scheduled maintenance, +as soon as the repository is registered with Scalar. It should therefore +not be necessary to run this subcommand manually. ++ +The `config` task is specific to Scalar and configures all those +opinionated default settings that make Git work more efficiently with +large repositories. As this task is run as part of `scalar clone` +automatically, explicit invocations of this task are rarely needed. + SEE ALSO -------- linkgit:git-clone[1], linkgit:git-maintenance[1].
Signature · 1 line
gitgitgadget
PATCH v10 12/15 scalar: allow reconfiguring an existing enlistment
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
This comes in handy during Scalar upgrades, or when config settings were messed up by mistake.
--- contrib/scalar/scalar.c | 79 +++++++++++++++++++++----------- contrib/scalar/scalar.txt | 8 ++++ contrib/scalar/t/t9099-scalar.sh | 8 ++++ 3 files changed, 67 insertions(+), 28 deletions(-)
contrib/scalar/scalar.c +51 -28
@@ -108,18 +108,20 @@ static int run_git(const char *arg, ...)return res; } -static int set_recommended_config (void ) +static int set_recommended_config (int reconfigure) {struct {const char *key;const char *value; +int overwrite_on_reconfigure; } config[] = { - {"am.keepCR" ,"true" }, - {"core.FSCache" ,"true" }, - {"core.multiPackIndex" ,"true" }, - {"core.preloadIndex" ,"true" }, +/* Required */ + {"am.keepCR" ,"true" ,1 }, + {"core.FSCache" ,"true" ,1 }, + {"core.multiPackIndex" ,"true" ,1 }, + {"core.preloadIndex" ,"true" ,1 }, #ifndef WIN32 - {"core.untrackedCache" ,"true" }, + {"core.untrackedCache" ,"true" ,1 }, #else /* * Unfortunately, Scalar's Functional Tests demonstrated
@@ -133,28 +135,29 @@ static int set_recommended_config(void) * Therefore, with a sad heart, we disable this very useful * feature on Windows. */ - { "core.untrackedCache", "false" }, + { "core.untrackedCache", "false", 1 }, #endif - { "core.logAllRefUpdates", "true" }, - { "credential.https://dev.azure.com.useHttpPath", "true" }, - { "credential.validate", "false" }, /* GCM4W-only */ - { "gc.auto", "0" }, - { "gui.GCWarning", "false" }, - { "index.threads", "true" }, - { "index.version", "4" }, - { "merge.stat", "false" }, - { "merge.renames", "true" }, - { "pack.useBitmaps", "false" }, - { "pack.useSparse", "true" }, - { "receive.autoGC", "false" }, - { "reset.quiet", "true" }, - { "feature.manyFiles", "false" }, - { "feature.experimental", "false" }, - { "fetch.unpackLimit", "1" }, - { "fetch.writeCommitGraph", "false" }, + { "core.logAllRefUpdates", "true", 1 }, + { "credential.https://dev.azure.com.useHttpPath", "true", 1 }, + { "credential.validate", "false", 1 }, /* GCM4W-only */ + { "gc.auto", "0", 1 }, + { "gui.GCWarning", "false", 1 }, + { "index.threads", "true", 1 }, + { "index.version", "4", 1 }, + { "merge.stat", "false", 1 }, + { "merge.renames", "true", 1 }, + { "pack.useBitmaps", "false", 1 }, + { "pack.useSparse", "true", 1 }, + { "receive.autoGC", "false", 1 }, + { "reset.quiet", "true", 1 }, + { "feature.manyFiles", "false", 1 }, + { "feature.experimental", "false", 1 }, + { "fetch.unpackLimit", "1", 1 }, + { "fetch.writeCommitGraph", "false", 1 }, #ifdef WIN32 - { "http.sslBackend", "schannel" }, + { "http.sslBackend", "schannel", 1 }, #endif + /* Optional */ { "status.aheadBehind", "false" }, { "commitGraph.generationVersion", "1" }, { "core.autoCRLF", "false" },
@@ -166,7 +169,8 @@ static int set_recommended_config(void)char *value;for (i =0 ; config[i].key; i++) { -if (git_config_get_string (config[i].key, &value)) { +if ((reconfigure && config[i].overwrite_on_reconfigure) || +git_config_get_string (config[i].key, &value)) {trace2_data_string ("scalar" , the_repository, config[i].key,"created" );if (git_config_set_gently (config[i].key, config[i].value) <0 )
@@ -231,7 +235,7 @@ static int register_dir(void)int res =add_or_remove_enlistment (1 );if (!res) - res =set_recommended_config (); + res =set_recommended_config (0 );if (!res) res =toggle_maintenance (1 );
@@ -419,7 +423,7 @@ static int cmd_clone(int argc, const char **argv) (res = run_git("sparse-checkout", "init", "--cone", NULL))) goto cleanup; -if (set_recommended_config ()) +if (set_recommended_config (0 ))return error (_ ("could not configure '%s'" ), dir);if ((res =run_git ("fetch" ,"--quiet" ,"origin" ,NULL ))) {
@@ -484,6 +488,24 @@ static int cmd_register(int argc, const char **argv)return register_dir (); } +static int cmd_reconfigure (int argc,const char **argv) +{ +struct option options[] = { +OPT_END (), + }; +const char *const usage[] = { +N_ ("scalar reconfigure [<enlistment>]" ), +NULL + }; + + argc =parse_options (argc, argv,NULL , options, + usage,0 ); + +setup_enlistment_directory (argc, argv, usage, options,NULL ); + +return set_recommended_config (1 ); +} +static int cmd_run (int argc,const char **argv) {struct option options[] = {
@@ -620,6 +642,7 @@ static struct { { "register", cmd_register }, { "unregister", cmd_unregister }, { "run", cmd_run }, + { "reconfigure", cmd_reconfigure }, { NULL, NULL}, };
contrib/scalar/scalar.txt +8 -0
@@ -13,6 +13,7 @@ scalar list scalar register [<enlistment>] scalar unregister [<enlistment>] scalar run ( all | config | commit-graph | fetch | loose-objects | pack-files ) [<enlistment>] +scalar reconfigure <enlistment> DESCRIPTION -----------
@@ -117,6 +118,13 @@ opinionated default settings that make Git work more efficiently with large repositories. As this task is run as part of `scalar clone` automatically, explicit invocations of this task are rarely needed. +Reconfigure +~~~~~~~~~~~ + +After a Scalar upgrade, or when the configuration of a Scalar enlistment +was somehow corrupted or changed by mistake, this subcommand allows to +reconfigure the enlistment. + SEE ALSO -------- linkgit:git-clone[1], linkgit:git-maintenance[1].
contrib/scalar/t/t9099-scalar.sh +8 -0
@@ -65,4 +65,12 @@ test_expect_success 'scalar clone' ' ) ' +test_expect_success 'scalar reconfigure' ' + git init one/src && + scalar register one && + git -C one/src config core.preloadIndex false && + scalar reconfigure one && + test true = "$(git -C one/src config core.preloadIndex)" +' + test_done
Signature · 1 line
gitgitgadget
PATCH v10 13/15 scalar: teach 'reconfigure' to optionally handle all registered enlistments
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
After a Scalar upgrade, it can come in really handy if there is an easy way to reconfigure all Scalar enlistments. This new option offers this functionality.
--- contrib/scalar/scalar.c | 61 ++++++++++++++++++++++++++++++-- contrib/scalar/scalar.txt | 9 +++-- contrib/scalar/t/t9099-scalar.sh | 3 ++ 3 files changed, 67 insertions(+), 6 deletions(-)
contrib/scalar/scalar.c +58 -3
@@ -488,22 +488,77 @@ static int cmd_register(int argc, const char **argv)return register_dir (); } +static int get_scalar_repos (const char *key,const char *value,void *data) +{ +struct string_list *list = data; + +if (!strcmp (key,"scalar.repo" )) +string_list_append (list, value); + +return 0 ; +} +static int cmd_reconfigure (int argc,const char **argv) { +int all =0 ;struct option options[] = { +OPT_BOOL ('a' ,"all" , &all, +N_ ("reconfigure all registered enlistments" )),OPT_END (), };const char *const usage[] = { -N_ ("scalar reconfigure [<enlistment>]" ), +N_ ("scalar reconfigure [--all | <enlistment>]" ),NULL }; +struct string_list scalar_repos =STRING_LIST_INIT_DUP ; +int i, res =0 ; +struct repository r = {NULL }; +struct strbuf commondir =STRBUF_INIT , gitdir =STRBUF_INIT ; argc =parse_options (argc, argv,NULL , options, usage,0 ); -setup_enlistment_directory (argc, argv, usage, options,NULL ); +if (!all) { +setup_enlistment_directory (argc, argv, usage, options,NULL ); + +return set_recommended_config (1 ); + } + +if (argc >0 ) +usage_msg_opt (_ ("--all or <enlistment>, but not both" ), + usage, options); + +git_config (get_scalar_repos, &scalar_repos); -return set_recommended_config (1 ); +for (i =0 ; i < scalar_repos.nr; i++) { +const char *dir = scalar_repos.items[i].string; + +strbuf_reset (&commondir); +strbuf_reset (&gitdir); + +if (chdir (dir) <0 ) { +warning_errno (_ ("could not switch to '%s'" ), dir); + res =-1 ; + }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 ; + } + } + +string_list_clear (&scalar_repos,1 ); +strbuf_release (&commondir); +strbuf_release (&gitdir); + +return res; }static int cmd_run (int argc,const char **argv)
contrib/scalar/scalar.txt +6 -3
@@ -13,7 +13,7 @@ scalar list scalar register [<enlistment>] scalar unregister [<enlistment>] scalar run ( all | config | commit-graph | fetch | loose-objects | pack-files ) [<enlistment>] -scalar reconfigure <enlistment> +scalar reconfigure [ --all | <enlistment> ] DESCRIPTION -----------
@@ -32,8 +32,8 @@ an existing Git worktree with Scalar whose name is not `src`, the enlistment will be identical to the worktree. The `scalar` command implements various subcommands, and different options -depending on the subcommand. With the exception of `clone` and `list`, all -subcommands expect to be run in an enlistment. +depending on the subcommand. With the exception of `clone`, `list` and +`reconfigure --all`, all subcommands expect to be run in an enlistment. COMMANDS --------
@@ -125,6 +125,9 @@ After a Scalar upgrade, or when the configuration of a Scalar enlistment was somehow corrupted or changed by mistake, this subcommand allows to reconfigure the enlistment. +With the `--all` option, all enlistments currently registered with Scalar +will be reconfigured. Use this option after each Scalar upgrade. + SEE ALSO -------- linkgit:git-clone[1], linkgit:git-maintenance[1].
contrib/scalar/t/t9099-scalar.sh +3 -0
@@ -70,6 +70,9 @@ test_expect_success 'scalar reconfigure' ' scalar register one && git -C one/src config core.preloadIndex false && scalar reconfigure one && + test true = "$(git -C one/src config core.preloadIndex)" && + git -C one/src config core.preloadIndex false && + scalar reconfigure -a && test true = "$(git -C one/src config core.preloadIndex)" '
Signature · 1 line
gitgitgadget
PATCH v10 14/15 scalar: implement the `delete` command
In reply to Johannes Schindelin via GitGitGadgetFrom: Matthew John Cheetham <[email protected]>
Delete an enlistment by first unregistering the repository and then deleting the enlistment directory (usually the directory containing the worktree src/ directory).
On Windows, if the current directory is inside the enlistment's directory, change to the parent of the enlistment directory, to allow us to delete the enlistment (directories used by processes e.g. as current working directories cannot be deleted on Windows).
--- contrib/scalar/scalar.c | 63 ++++++++++++++++++++++++++++++++ contrib/scalar/scalar.txt | 8 ++++ contrib/scalar/t/t9099-scalar.sh | 9 +++++ 3 files changed, 80 insertions(+)
contrib/scalar/scalar.c +63 -0
@@ -8,6 +8,8 @@#include "config.h" #include "run-command.h" #include "refs.h" +#include "dir.h" +#include "packfile.h" /* * Remove the deepest subdirectory in the provided path string. Path must not
@@ -328,6 +330,33 @@ static char *remote_default_branch(const char *url)return NULL ; } +static int delete_enlistment (struct strbuf *enlistment) +{ +#ifdef WIN32 +struct strbuf parent =STRBUF_INIT ; +#endif + +if (unregister_dir ()) +die (_ ("failed to unregister repository" )); + +#ifdef WIN32 +/* +* Change the current directory to one outside of the enlistment so +* that we may delete everything underneath it. +*/ +strbuf_addbuf (&parent, enlistment); +strbuf_parent_directory (&parent); +if (chdir (parent.buf) <0 ) +die_errno (_ ("could not switch to '%s'" ), parent.buf); +strbuf_release (&parent); +#endif + +if (remove_dir_recursively (enlistment,0 )) +die (_ ("failed to delete enlistment directory" )); + +return 0 ; +} +static int cmd_clone (int argc,const char **argv) {const char *branch =NULL ;
@@ -688,6 +717,39 @@ static int cmd_unregister(int argc, const char **argv)return unregister_dir (); } +static int cmd_delete (int argc,const char **argv) +{ +char *cwd =xgetcwd (); +struct option options[] = { +OPT_END (), + }; +const char *const usage[] = { +N_ ("scalar delete <enlistment>" ), +NULL + }; +struct strbuf enlistment =STRBUF_INIT ; +int res =0 ; + + argc =parse_options (argc, argv,NULL , options, + usage,0 ); + +if (argc !=1 ) +usage_with_options (usage, options); + +setup_enlistment_directory (argc, argv, usage, options, &enlistment); + +if (dir_inside_of (cwd, enlistment.buf) >=0 ) + res =error (_ ("refusing to delete current working directory" )); +else { +close_object_store (the_repository->objects); + res =delete_enlistment (&enlistment); + } +strbuf_release (&enlistment); +free (cwd); + +return res; +} + static struct { const char *name; int (*fn)(int, const char **);
@@ -698,6 +760,7 @@ static struct { { "unregister", cmd_unregister }, { "run", cmd_run }, { "reconfigure", cmd_reconfigure }, + { "delete", cmd_delete }, { NULL, NULL}, };
contrib/scalar/scalar.txt +8 -0
@@ -14,6 +14,7 @@ scalar register [<enlistment>] scalar unregister [<enlistment>] scalar run ( all | config | commit-graph | fetch | loose-objects | pack-files ) [<enlistment>] scalar reconfigure [ --all | <enlistment> ] +scalar delete <enlistment> DESCRIPTION -----------
@@ -128,6 +129,13 @@ reconfigure the enlistment. With the `--all` option, all enlistments currently registered with Scalar will be reconfigured. Use this option after each Scalar upgrade. +Delete +~~~~~~ + +delete <enlistment>:: + This subcommand lets you delete an existing Scalar enlistment from your + local file system, unregistering the repository. + SEE ALSO -------- linkgit:git-clone[1], linkgit:git-maintenance[1].
contrib/scalar/t/t9099-scalar.sh +9 -0
@@ -76,4 +76,13 @@ test_expect_success 'scalar reconfigure' ' test true = "$(git -C one/src config core.preloadIndex)" ' +test_expect_success 'scalar delete without enlistment shows a usage' ' + test_expect_code 129 scalar delete +' + +test_expect_success 'scalar delete with enlistment' ' + scalar delete cloned && + test_path_is_missing cloned +' + test_done
Signature · 1 line
gitgitgadget
PATCH v10 15/15 scalar: implement the `version` command
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
The .NET version of Scalar has a version command. This was necessary because it was versioned independently of Git.
Since Scalar is now tightly coupled with Git, it does not make sense for them to show different versions. Therefore, it shows the same output as git version. For backwards-compatibility with the .NET version, scalar version prints to stderr, though (git version prints to stdout instead).
--- contrib/scalar/scalar.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
contrib/scalar/scalar.c +39 -0
@@ -10,6 +10,7 @@#include "refs.h" #include "dir.h" #include "packfile.h" +#include "help.h" /* * Remove the deepest subdirectory in the provided path string. Path must not
@@ -357,6 +358,15 @@ static int delete_enlistment(struct strbuf *enlistment)return 0 ; } +/* +* Dummy implementation; Using `get_version_info()` would cause a link error +* without this. +*/ +void load_builtin_commands (const char *prefix,struct cmdnames *cmds) +{ +die ("not implemented" ); +} +static int cmd_clone (int argc,const char **argv) {const char *branch =NULL ;
@@ -750,6 +760,34 @@ static int cmd_delete(int argc, const char **argv)return res; } +static int cmd_version (int argc,const char **argv) +{ +int verbose =0 , build_options =0 ; +struct option options[] = { +OPT__VERBOSE (&verbose,N_ ("include Git version" )), +OPT_BOOL (0 ,"build-options" , &build_options, +N_ ("include Git's build options" )), +OPT_END (), + }; +const char *const usage[] = { +N_ ("scalar verbose [-v | --verbose] [--build-options]" ), +NULL + }; +struct strbuf buf =STRBUF_INIT ; + + argc =parse_options (argc, argv,NULL , options, + usage,0 ); + +if (argc !=0 ) +usage_with_options (usage, options); + +get_version_info (&buf, build_options); +fprintf (stderr,"%s\n" , buf.buf); +strbuf_release (&buf); + +return 0 ; +} + static struct { const char *name; int (*fn)(int, const char **);
@@ -761,6 +799,7 @@ static struct { { "run", cmd_run }, { "reconfigure", cmd_reconfigure }, { "delete", cmd_delete }, + { "version", cmd_version }, { NULL, NULL}, };
Signature · 1 line
gitgitgadget
Quoted 11 lines · Johannes Schindelin via GitGitGadget
tl;dr: This series contributes the core part of the Scalar command to the Git project. This command provides a convenient way to clone/initialize very large repositories (think: monorepos).
Note: This patch series' focus is entirely on Scalar, on choosing sensible defaults and offering a delightful user experience around working with monorepos, and not about changing any existing paradigms for contrib/ (even if catching up on the mail thread is likely to give interested readers that false impression).
Quoted 5 lines
Changes since v9:
* The patches to build Scalar and run its tests as part of Git's CI/PR, have been dropped because a recent unrelated patch series does not interact well with them.
i.e. basically undoing this:
...
Quoted 1 line
Changes since v6:
...
Quoted 4 lines
* I added two patches that I had planned on keeping in an add-on patch series for later, to build and test Scalar as part of the CI. I am still not 100% certain that it is a good idea to do so already now, but let's see what the reviewers have to say.
...and returning to the original plan:
...
Quoted 1 line
On top of this patch series, I have lined up a few more:
...
Quoted 3 lines
4. A few patches to optionally build and install scalar as part of a regular Git install (also teaching git help scalar to find the Scalar documentation
Avoiding the issues and adding the CI later seems reasonable to me. You addressed the last of my points in v9; I think this version is good to go. But one quick comment...
Quoted 7 lines
These are included in my vfs-with-scalar branch thicket
[https://github.com/dscho/git/commits/vfs-with-scalar]. On top of that, this branch thicket also includes patches I do not plan on upstreaming, mainly because they are too specific either to VFS for Git, or they support Azure Repos (which does not offer partial clones but speaks the GVFS protocol, which can be used to emulate partial clones).
Quoted 5 lines
One other thing is very interesting about that vfs-with-scalar branch
thicket: it contains a GitHub workflow which will run Scalar's quite extensive Functional Tests suite. This test suite is quite comprehensive and caught us a lot of bugs in the past, not only in the Scalar code, but also core Git.
From your wording it sounds like the plan might not include moving these tests over. Perhaps it doesn't make sense to move them all over, but since they've caught problems in both Scalar and core Git, it would be nice to see many of those tests come to Git as well as part of a future follow on series.
Quoted 5 lines · Elijah Newren
From your wording it sounds like the plan might not include moving these tests over. Perhaps it doesn't make sense to move them all over, but since they've caught problems in both Scalar and core Git, it would be nice to see many of those tests come to Git as well as part of a future follow on series.
Yeah, we may be initially queuing this without tests for expediency, but a production code cannot go forever without CI tests to ensure continued code health. People make changes in other parts of the system Scalar may depend on and unknowingly break some assumption Scalar makes on it.
Quoted 8 lines · Junio C Hamano
Quoted 5 lines · Elijah Newren
From your wording it sounds like the plan might not include moving these tests over. Perhaps it doesn't make sense to move them all over, but since they've caught problems in both Scalar and core Git, it would be nice to see many of those tests come to Git as well as part of a future follow on series.
Quoted 5 lines
Yeah, we may be initially queuing this without tests for expediency, but a production code cannot go forever without CI tests to ensure continued code health. People make changes in other parts of the system Scalar may depend on and unknowingly break some assumption Scalar makes on it.
In this case there really isn't any reason not to have the tests go in at the same time. The explanation in the v10 CL is:
Changes since v9:
* The patches to build Scalar and run its tests as part of Git's CI/PR, have been dropped because a recent unrelated patch series does not interact well with them.
That assessment isn't correct.
The change in v8->v9 of adding a "make &&" before the "test" was only necessary because of a logic error in the v8 version. Yes it broke because the "scalar test" target didn't know how to build its prerequisites, but the real underlying issue is that it was even trying at that point. It had no business running in the static-analysis target where we hadn't built git already.
Now v9->v10 has
dropped the tests entirely, allegedly due to an interaction with my ab/ci-updates, but there's nothing new there that isn't also the case on "master".
But we can have our cake and eat it too.
The below patch on top of v9 would make the scalar tests do the right thing. I.e. whenever we do a "make test" we'll run the scalar tests too.
The code changes somewhat with ab/ci-updates, but the conflict with js/scalar is mostly textual, not semantic (and as I've pointed out, to the extent that ab/ci-updates changed anything it made things a bit better for js/scalar).
I'd really like to see this scalar series land, but I really don't see why it's necessary to entirety eject the CI test coverage due to what's a rather trivilly solved issue.
As I've noted ad-nauseum at this point I think the necessity for the below patch is rather silly, this should just nicely integrate with "make test", but <brokenrecord.gif>. But even without that IMO better approach it's clearly rather trivial to make this series have test coverage.
It was just broken because it added a test run to the "pedantic" run, and didn't properly integrate with the multi-"make test" runs on "master" , both of which are addressed by the patch below.
ci/run-build-and-tests.sh +20 -2
@@ -15,6 +15,26 @@ then export DEVOPTS=pedantic fi +make() { + scalar_tests= + for target + do + if test $target = "test" + then + scalar_tests=t + fi + done + + # Do whatever we would have done with "make" + command make "$@" + + # Running tests? Run scalar tests too + if test -n "$scalar_tests" + then + command make -C contrib/scalar test + fi +} + make case "$jobname" in linux-gcc)
@@ -52,6 +72,4 @@ esac check_unignored_build_artifacts -make && make -C contrib/scalar test - save_good_tree
Quoted 36 lines · Ævar Arnfjörð Bjarmason
It was just broken because it added a test run to the "pedantic" run, and didn't properly integrate with the multi-"make test" runs on "master" , both of which are addressed by the patch below.
ci/run-build-and-tests.sh +20
-0@@ -15,6 +15,26 @@ then export DEVOPTS=pedantic fi +make() { + scalar_tests= + for target + do + if test $target = "test" + then + scalar_tests=t + fi + done + + # Do whatever we would have done with "make" + command make "$@" + + # Running tests? Run scalar tests too + if test -n "$scalar_tests" + then + command make -C contrib/scalar test + fi +} + make case "$jobname" in linux-gcc)
ci/run-build-and-tests.sh esac · line 75
Quoted 6 lines
ci/run-build-and-tests.sh +0
-2@@ -52,6 +72,4 @@ esac check_unignored_build_artifacts -make && make -C contrib/scalar test - save_good_tree
That is an interesting way to demonstrate how orthogonal the issues are, which in turn means that it is not such a big deal to add back the coverage to the part that goes to contrib/scalar/. As the actual implementation, it is a bit too icky, though.
[RFC/PATCH] Makefile: add test-all target
We ship contrib/ stuff within our primary source tree but except for the completion scripts that are tested from our primary test suite, their test suites are not run in the CI.
Teach the main Makefile a "test-extra" target, which goes into each package in contrib/ whose Makefile has its own "test" target and runs "make test" there. Add a "test-all" target to make it easy to drive both the primary tests and these contrib tests from CI and use it.
---
Quoted 4 lines · Junio C Hamano
That is an interesting way to demonstrate how orthogonal the issues are, which in turn means that it is not such a big deal to add back the coverage to the part that goes to contrib/scalar/. As the actual implementation, it is a bit too icky, though.
So, how about doing it this way? This is based on 'master' and does not cover contrib/scalar, but if we want to go this route, it should be trivial to do it on top of a merge of ab/ci-updates and js/scalar into 'master'. Good idea? Terrible idea? Not good enough?
Makefile | 12 +++++++++++- ci/run-build-and-tests.sh | 10 +++++----- 2 files changed, 16 insertions(+), 6 deletions(-)
Makefile +11 -1
@@ -2878,10 +2878,20 @@ export TEST_NO_MALLOC_CHECK test: all $(MAKE) -C t/ all +# Additional tests from places in contrib/ that are prepared to take +# "make -C $there test", but expects that the primary build is done +# already. +test-extra: all + $(MAKE) -C contrib/diff-highlight test + $(MAKE) -C contrib/mw-to-git test + $(MAKE) -C contrib/subtree test + +test-all:: test test-extra + perf: all $(MAKE) -C t/perf/ all -.PHONY: test perf +.PHONY: test test-extra test-all perf .PRECIOUS: $(TEST_OBJS)
ci/run-build-and-tests.sh +5 -5
@@ -19,7 +19,7 @@ make case "$jobname" in linux-gcc) export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main - make test + make test-all export GIT_TEST_SPLIT_INDEX=yes export GIT_TEST_MERGE_ALGORITHM=recursive export GIT_TEST_FULL_IN_PACK_ARRAY=true
@@ -33,20 +33,20 @@ linux-gcc) export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master export GIT_TEST_WRITE_REV_INDEX=1 export GIT_TEST_CHECKOUT_WORKERS=2 - make test + make test-all ;; linux-clang) export GIT_TEST_DEFAULT_HASH=sha1 - make test + make test-all export GIT_TEST_DEFAULT_HASH=sha256 - make test + make test-all ;; linux-gcc-4.8|pedantic) # Don't run the tests; we only care about whether Git can be # built with GCC 4.8 or with pedantic ;; *) - make test + make test-all ;; esac
Re: [RFC/PATCH] Makefile: add test-all target
Quoted 15 lines · Junio C Hamano
We ship contrib/ stuff within our primary source tree but except for the completion scripts that are tested from our primary test suite, their test suites are not run in the CI.
Teach the main
Makefilea "test-extra" target, which goes into each package in contrib/ whoseMakefilehas its own "test" target and runs "make test" there. Add a "test-all" target to make it easy to drive both the primary tests and these contrib tests from CI and use it.So, how about doing it this way? This is based on 'master' and does not cover contrib/scalar, but if we want to go this route, it should be trivial to do it on top of a merge of ab/ci-updates and js/scalar into 'master'. Good idea? Terrible idea? Not good enough?
Makefile export TEST_NO_MALLOC_CHECK · line 2887
Quoted 7 lines
Makefile +7
-0+# Additional tests from places in contrib/ that are prepared to take +# "make -C $there test", but expects that the primary build is done +# already. +test-extra: all + $(MAKE) -C contrib/diff-highlight test + $(MAKE) -C contrib/mw-to-git test + $(MAKE) -C contrib/subtree test
I like how this is obviously extendible to include contrib/scalar in a later change, then remove it when Scalar moves.
Quoted 1 line
+test-all:: test test-extra
And this test-all implies that test runs before test-extra, so libgit.a is compiled appropriately.
I think this approach looks good to me.
ci/run-build-and-tests.sh make · line 22
Quoted 10 lines
ci/run-build-and-tests.sh +1
-1@@ -19,7 +19,7 @@ make case "$jobname" in linux-gcc) export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main - make test + make test-all
Since we are now building and testing things that we have not been testing recently, it is worth checking that we don't have any work to do to make this pass. I assume that you've run 'make test-all' on your own machine. It will be good to see what the full action reports (probably all good).
Thanks,
-Stolee
Re: [RFC/PATCH] Makefile: add test-all target
Quoted 8 lines · Derrick Stolee
Makefile export TEST_NO_MALLOC_CHECK · line 2887
Quoted 4 lines
Makefile +4
-0+test-extra: all + $(MAKE) -C contrib/diff-highlight test + $(MAKE) -C contrib/mw-to-git test + $(MAKE) -C contrib/subtree testI like how this is obviously extendible to include contrib/scalar in a later change, then remove it when Scalar moves.
Quoted 4 lines
Quoted 1 line
+test-all:: test test-extra
And this test-all implies that test runs before test-extra, so libgit.a is compiled appropriately.
I do not think this implies the ordering between the main test and the extra test. "make test-all" actually makes a confusing mess on the terminal by conflating outputs from the main test and tests run in contrib.
But because test-extra depends on all, we are keeping the assumption that Makefiles in contrib/ may assume that the primary build has already been done.
Quoted 8 lines
Quoted 8 lines
ci/run-build-and-tests.sh +0
-0@@ -19,7 +19,7 @@ make case "$jobname" in linux-gcc) export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
Quoted 8 lines
ci/run-build-and-tests.sh make · line 22
Quoted 2 lines
ci/run-build-and-tests.sh +1
-1- make test + make test-allSince we are now building and testing things that we have not been testing recently, it is worth checking that we don't have any work to do to make this pass. I assume that you've run 'make test-all' on your own machine. It will be good to see what the full action reports (probably all good).
Yes, I am tempted to queue this at the tip of 'seen'.
Re: [RFC/PATCH] Makefile: add test-all target
In reply to Junio C HamanoQuoted 9 lines · Junio C Hamano
Quoted 4 lines
That is an interesting way to demonstrate how orthogonal the issues are, which in turn means that it is not such a big deal to add back the coverage to the part that goes to contrib/scalar/. As the actual implementation, it is a bit too icky, though.
So, how about doing it this way? This is based on 'master' and does not cover contrib/scalar, but if we want to go this route, it should be trivial to do it on top of a merge of ab/ci-updates and js/scalar into 'master'. Good idea? Terrible idea? Not good enough?
I don't mind the general direction, but...
Makefile export TEST_NO_MALLOC_CHECK · line 2887
Quoted 7 lines
Makefile +7
-0+# Additional tests from places in contrib/ that are prepared to take +# "make -C $there test", but expects that the primary build is done +# already. +test-extra: all + $(MAKE) -C contrib/diff-highlight test + $(MAKE) -C contrib/mw-to-git test + $(MAKE) -C contrib/subtree test
I'm not sure of the quality of tests in some of the contrib stuff. The tests in diff-highlight worked for me when I added them, but it's not like I ever run them regularly, or that they've been tested on a wide variety of platforms.
So I think this is as likely to cause somebody a headache due to a dumb portability problem or random bitrot as it is to actually find a bug. I guess test-extra wouldn't be run by default, but only via CI, so maybe that limits the blast radius sufficiently.
For diff-highlight in particular, you need to have a working perl, so you'd probably want to at least wrap it with a NO_PERL ifndef. For mw-to-git, you need to have MediaWiki::API installed, though I think the tests at least notice this and skip everything if you don't.
-Peff
Re: [RFC/PATCH] Makefile: add test-all target
Quoted 4 lines · Jeff King
For diff-highlight in particular, you need to have a working perl, so you'd probably want to at least wrap it with a
NO_PERLifndef. For mw-to-git, you need to haveMediaWiki::APIinstalled, though I think the tests at least notice this and skip everything if you don't.
I know I locally lack MediaWiki::API and saw the test skipped everything. I do not know that would be true on a box without any perl, though.
Re: [RFC/PATCH] Makefile: add test-all target
In reply to Jeff KingQuoted 15 lines · Jeff King
I don't mind the general direction, but...
Makefile export TEST_NO_MALLOC_CHECK · line 2887
Quoted 7 lines
Makefile +7
-0+# Additional tests from places in contrib/ that are prepared to take +# "make -C $there test", but expects that the primary build is done +# already. +test-extra: all + $(MAKE) -C contrib/diff-highlight test + $(MAKE) -C contrib/mw-to-git test + $(MAKE) -C contrib/subtree testI'm not sure of the quality of tests in some of the contrib stuff. The tests in diff-highlight worked for me when I added them, but it's not like I ever run them regularly, or that they've been tested on a wide variety of platforms.
Quoted 4 lines
So I think this is as likely to cause somebody a headache due to a dumb portability problem or random bitrot as it is to actually find a bug. I guess test-extra wouldn't be run by default, but only via CI, so maybe that limits the blast radius sufficiently.
Yeah, that is the exact thought I had when I did it. Anybody who is not aware of test target other than 'test' will not be hurt, and we explicitly make the CI aware of 'test-all' to trigger it. But as long as somebody bothered to write the tests, exercising them to reveal bitrot-bugs either in the tested contrib stuff or the tests themselves to be fixed or removed would be a good thing to do.
An updated version of the posted patch is in 'seen' that also covers credential/netrc; https://github.com/git/git/runs/4465323829 shows the logs from its jobs.
It is not particularly interesting that most of the jobs are marked as failed, as t1092 was broken the same way in my local test. What I found interesting from my quick scan of randomly chosen jobs are that (1) nobody seemed to have failed test-extra, and (2) nobody had mediawiki installed to test mw-to-git.
So I am tempted to do
test-extra: all
$(MAKE) -C contrib/credential/netrc test $(MAKE) -C contrib/diff-highlight test : $(MAKE) -C contrib/mw-to-git test $(MAKE) -C contrib/subtree test
in the topic itself, while adding
$(MAKE) -C contrib/scalar test
before the subtree test (alphabetically) when it is merged to 'seen' with the js/scalar topic.
Re: [RFC/PATCH] Makefile: add test-all target
Quoted 11 lines · Junio C Hamano
Quoted 4 lines
So I think this is as likely to cause somebody a headache due to a dumb portability problem or random bitrot as it is to actually find a bug. I guess test-extra wouldn't be run by default, but only via CI, so maybe that limits the blast radius sufficiently.
Yeah, that is the exact thought I had when I did it. Anybody who is not aware of test target other than 'test' will not be hurt, and we explicitly make the CI aware of 'test-all' to trigger it. But as long as somebody bothered to write the tests, exercising them to reveal bitrot-bugs either in the tested contrib stuff or the tests themselves to be fixed or removed would be a good thing to do.
I'm don't have strong feelings on it either way. But if we think those tests are worth running in CI, then...
Quoted 7 lines
So I am tempted to do
test-extra: all
$(MAKE) -C contrib/credential/netrc test $(MAKE) -C contrib/diff-highlight test : $(MAKE) -C contrib/mw-to-git test $(MAKE) -C contrib/subtree test
...we'd probably want to keep running mw-to-git tests, and teach one of the CI environments to install the appropriate perl modules to avoid skipping them.
-Peff
Re: [RFC/PATCH] Makefile: add test-all target
Quoted 11 lines · Jeff King
I'm don't have strong feelings on it either way. But if we think those tests are worth running in CI, then...
Quoted 7 lines
So I am tempted to do
test-extra: all
$(MAKE) -C contrib/credential/netrc test $(MAKE) -C contrib/diff-highlight test : $(MAKE) -C contrib/mw-to-git test $(MAKE) -C contrib/subtree test
Quoted 3 lines
...we'd probably want to keep running mw-to-git tests, and teach one of the CI environments to install the appropriate perl modules to avoid skipping them.
I saw netrc credential helper break on one of the jobs that lack Perl, so the test there needs to be fixed before we can include it in test-extra.
Re: [RFC/PATCH] Makefile: add test-all target
In reply to Junio C HamanoQuoted 19 lines · Junio C Hamano
We ship contrib/ stuff within our primary source tree but except for the completion scripts that are tested from our primary test suite, their test suites are not run in the CI.
Teach the main
Makefilea "test-extra" target, which goes into each package in contrib/ whoseMakefilehas its own "test" target and runs "make test" there. Add a "test-all" target to make it easy to drive both the primary tests and these contrib tests from CI and use it.Signed-off-by Junio C Hamano <[email protected]>---
Quoted 4 lines · Junio C Hamano
That is an interesting way to demonstrate how orthogonal the issues are, which in turn means that it is not such a big deal to add back the coverage to the part that goes to contrib/scalar/. As the actual implementation, it is a bit too icky, though.
Quoted 4 lines
So, how about doing it this way? This is based on 'master' and does not cover contrib/scalar, but if we want to go this route, it should be trivial to do it on top of a merge of ab/ci-updates and js/scalar into 'master'. Good idea? Terrible idea? Not good enough?
With the caveat that I think the greater direction here makes no sense, i.e. scalar didn't need its own build system etc. in the first place, so having hack-upon-hack to fix various integration issues is clearly worse than just having it behave like everything else....
... then yes, adding this to the top-level Makefile makes more sense....
Quoted 19 lines
Makefile | 12 +++++++++++- ci/run-build-and-tests.sh | 10 +++++----- 2 files changed, 16 insertions(+), 6 deletions(-)Makefile +7
-0@@ -2878,10 +2878,20 @@ export TEST_NO_MALLOC_CHECK test: all $(MAKE) -C t/ all +# Additional tests from places in contrib/ that are prepared to take +# "make -C $there test", but expects that the primary build is done +# already. +test-extra: all + $(MAKE) -C contrib/diff-highlight test + $(MAKE) -C contrib/mw-to-git test + $(MAKE) -C contrib/subtree test
Makefile export TEST_NO_MALLOC_CHECK · line 2896
Quoted 10 lines
Makefile +4
-1+ +test-all:: test test-extra + perf: all $(MAKE) -C t/perf/ all -.PHONY: test perf +.PHONY: test test-extra test-all perf .PRECIOUS: $(TEST_OBJS)
Which, if we're nitpicking this would be better, i.e. it allows them to run in parallel, as they won't be defined by only one rule, and will be listede individuall in the test-all and test-extra prereqs:
Makefile +15 -5
@@ -2878,15 +2878,25 @@ export TEST_NO_MALLOC_CHECK test: all $(MAKE) -C t/ all +define TMPL_test-extra +TEST_EXTRA_TARGETS += test-$(1) +.PHONY: test-$(1) +test-$(1): all + $$(MAKE) -C $(1) test +endef + # Additional tests from places in contrib/ that are prepared to take # "make -C $there test", but expects that the primary build is done # already. -test-extra: all - $(MAKE) -C contrib/diff-highlight test - $(MAKE) -C contrib/mw-to-git test - $(MAKE) -C contrib/subtree test +$(eval $(call TMPL_test-extra,contrib/diff-highlight)) +$(eval $(call TMPL_test-extra,contrib/mw-to-git)) +$(eval $(call TMPL_test-extra,contrib/subtree)) + +.PHONY: test-extra +test-extra:: all $(TEST_EXTRA_TARGETS) -test-all:: test test-extra +.PHONY: test-all +test-all: test $(TEST_EXTRA_TARGETS) perf: all $(MAKE) -C t/perf/ all
ci/run-build-and-tests.sh make · line 22
Quoted 11 lines
ci/run-build-and-tests.sh +1
-1@@ -19,7 +19,7 @@ make case "$jobname" in linux-gcc) export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main - make test + make test-all
But I think we're expanding the scope quite a bit here. The reason we were talking about testing scalar by default is because it uses libgit.a, so it's not decoupled at all, whereas the "contrib" programs are only using the built "git" command.
I think it would probably be good to test these anyway, but it's an argument beyond that which applies to scalar.
I also share Jeff's general concerns that the other stuff in contrib may not be all that stable.
But I don't see why we should be pursuing this direction of running certain tests in CI only, as opposed to just under "make test", that distinction is something new in js/scalar (before that we run libgit.a test *modes* in CI, but not a different set of tests).
Re: [RFC/PATCH] Makefile: add test-all target
Quoted 9 lines · Ævar Arnfjörð Bjarmason
Quoted 4 lines
So, how about doing it this way? This is based on 'master' and does not cover contrib/scalar, but if we want to go this route, it should be trivial to do it on top of a merge of ab/ci-updates and js/scalar into 'master'. Good idea? Terrible idea? Not good enough?
With the caveat that I think the greater direction here makes no sense, i.e. scalar didn't need its own build system etc. in the first place, so having hack-upon-hack to fix various integration issues is clearly worse than just having it behave like everything else....
We decided to start Scalar in contrib/, as it hasn't been proven that Scalar is in a good enough shape to deserve to be in this tree, and we are giving it a chance by adding it to contrib/ first, hoping that it may graduate to the more official status someday [*].
And 'test-extra' is a way to give test coverage to things already in contrib/ that has 'test' target in their Makefile. When js/scalar gets merged to a tree with 'test-extra' target, it may be tested in that target, too, because we want to have it behave like everything else.
[Footnote]
*1* You may not like the "try unproven things in contrib/ first and
then we may graduate it later" approach, but that particular ship has sailed and this is not a time to complain and waste project's time.
Re: [RFC/PATCH] Makefile: add test-all target
Quoted 17 lines · Junio C Hamano
Quoted 9 lines · Ævar Arnfjörð Bjarmason
Quoted 4 lines
So, how about doing it this way? This is based on 'master' and does not cover contrib/scalar, but if we want to go this route, it should be trivial to do it on top of a merge of ab/ci-updates and js/scalar into 'master'. Good idea? Terrible idea? Not good enough?
With the caveat that I think the greater direction here makes no sense, i.e. scalar didn't need its own build system etc. in the first place, so having hack-upon-hack to fix various integration issues is clearly worse than just having it behave like everything else....
We decided to start Scalar in contrib/, as it hasn't been proven that Scalar is in a good enough shape to deserve to be in this tree, and we are giving it a chance by adding it to contrib/ first, hoping that it may graduate to the more official status someday [*].
Quoted 8 lines
And 'test-extra' is a way to give test coverage to things already in contrib/ that has 'test' target in their
Makefile. When js/scalar gets merged to a tree with 'test-extra' target, it may be tested in that target, too, because we want to have it behave like everything else.[Footnote]
I'm referring to the divide between testing things in CI v.s. "make test" making no sense. Not the path at which scalar is stored in-tree, which I don't care about, except insofar as it's used as a proxy for behavior that doesn't make sense.
Scalar uses libgit.a, and is built by default in the js/scalar topic. So we don't have a choice to really ignore it. E.g. there's part of the refs.h API used only by it. If you're changing that API you need to test against scalar.
Which makes sense, and I'd like to have scalar in-tree.
I just don't think it makes any sense that I edit say refs.[ch], run "make test" locally, but only see that something broke in scalar's specific use of libgit.a later when I look at GitHub CI.
If you're preparing a series for submission you'll need to get the CI passing. Except for portability issues etc. it should be trivial to run the same set of tests locally as we run in CI, that's the case now with any change you make to libgit and its consumers.
With the scalar topic we lose that 1=1 mapping. I don't think doing that in the name of it living in contrib makes sense.
If I'm preparing patches for submission I'll need to get CI passing, so I'll need to fix those tests & behavior either way as it's in-tree. Knowing about the failures later-not-sooner wastes more time, not less.
Quoted 4 lines
*1* You may not like the "try unproven things in contrib/ first and
then we may graduate it later" approach, but that particular ship has sailed and this is not a time to complain and waste project's time.
We have t/t9902-completion.sh testing contrib/completion/, and we run that under "make test" and in CI alike, the same goes for t/t1021-rerere-in-workdir.sh and contrib/workdir/git-new-workdir, we've got similar (but optional) tests for contrib/credential in t/ too.
The reason we do that with the completion is because some changes to e.g. tweak getopts will need to have a corresponding change to the completion.
The reason we've not done that with contrib/{subtree,mw-to-git}/ is because those are thoroughly in the category of only incidentally being in-tree.
I.e. we don't have any reason to think that testing those would stress core features of git itself any more than testing say out-of-tree software like git-lfs or git-annex.
Testing those is still interesting, but that's for the benefit of that software itself not bitrotting, not that we're likely to introduce in-tree breakages by changing an API and missing one of its users.
Scalar is thoroughly on the "completion" side of that divide, not "subtree".
Re: [RFC/PATCH] Makefile: add test-all target
Quoted 3 lines · Ævar Arnfjörð Bjarmason
I just don't think it makes any sense that I edit say refs.[ch], run "make test" locally, but only see that something broke in scalar's specific use of libgit.a later when I look at GitHub CI.
I'm definitely sympathetic to this. Having been surprised by CI failure on something that worked locally is annoying at best, and downright frustrating when you can't easily reproduce the problem.
But isn't that already true for most of the value that CI provides? While part of its purpose may be a back-stop for folks who don't run "make test" locally, I think the biggest value is that it covers a much wider variety of platforms and scenarios that you don't get out of "make test" already.
In some of those cases you can reproduce the problem locally by tweaking build or test knobs. But in others it can be quite a bit more challenging (e.g., something that segfaults only on Windows). At least in the proposed change here you'd only be a "make test-all" away from reproducing the problem locally.
I dunno. I don't feel that strongly either way about whether scalar tests should be part of "make test". Mostly just observing that this is not exactly a new case.
Quoted 4 lines
If I'm preparing patches for submission I'll need to get CI passing, so I'll need to fix those tests & behavior either way as it's in-tree. Knowing about the failures later-not-sooner wastes more time, not less.
I think there's probably a tradeoff here. How often you get a "late" notification of a bug (and how much of your time that wastes) versus how much time you spend locally running tests that you don't care about.
I do agree that CI presents a bit of a conundrum for stuff at the edge of the project. It's become a de facto requirement for it to pass. In general that's good. But it means that features which were introduced under the notion of "the people who care about this area will tend to its maintenance" slowly become _everybody's_ problem as soon as they have any CI coverage. Another example here is the cmake stuff. Or the recent discussion about "-x" and bash.
I wonder if there's a good way to make some CI results informational, rather than "failing". I.e., run scalar tests via CI, but if you're not working on scalar, you don't have to care. Folks who are interested in the area would keep tabs on those results and make sure that Junio's tree stays passing.
That view disagrees with the final paragraph here, though:
Quoted 10 lines
The reason we do that with the completion is because some changes to e.g. tweak getopts will need to have a corresponding change to the completion.
The reason we've not done that with contrib/{subtree,mw-to-git}/ is because those are thoroughly in the category of only incidentally being in-tree.
[...]
Scalar is thoroughly on the "completion" side of that divide, not "subtree".
I haven't followed the discussion closely, but in my mind "scalar" was still in the "it may live in-tree for convenience, but people who aren't working on it don't necessarily need to care about it" camp. Maybe that's not the plan, though.
-Peff
Re: [RFC/PATCH] Makefile: add test-all target
Quoted 22 lines · Jeff King
Quoted 3 lines · Ævar Arnfjörð Bjarmason
I just don't think it makes any sense that I edit say refs.[ch], run "make test" locally, but only see that something broke in scalar's specific use of libgit.a later when I look at GitHub CI.
I'm definitely sympathetic to this. Having been surprised by CI failure on something that worked locally is annoying at best, and downright frustrating when you can't easily reproduce the problem.
But isn't that already true for most of the value that CI provides? While part of its purpose may be a back-stop for folks who don't run "make test" locally, I think the biggest value is that it covers a much wider variety of platforms and scenarios that you don't get out of "make test" already.
In some of those cases you can reproduce the problem locally by tweaking build or test knobs. But in others it can be quite a bit more challenging (e.g., something that segfaults only on Windows). At least in the proposed change here you'd only be a "make test-all" away from reproducing the problem locally.
Quoted 3 lines
I dunno. I don't feel that strongly either way about whether scalar tests should be part of "make test". Mostly just observing that this is not exactly a new case.
Yes. I'm not saying that "make test" should always run what a full CI run covers.
Just that a proposed change that's really only adding one-more-test-file testing a thing in contrib in the sense that we test t/t9902-completion.sh should similarly be part of "make test".
Quoted 17 lines
Quoted 4 lines
If I'm preparing patches for submission I'll need to get CI passing, so I'll need to fix those tests & behavior either way as it's in-tree. Knowing about the failures later-not-sooner wastes more time, not less.
I think there's probably a tradeoff here. How often you get a "late" notification of a bug (and how much of your time that wastes) versus how much time you spend locally running tests that you don't care about.
I do agree that CI presents a bit of a conundrum for stuff at the edge of the project. It's become a de facto requirement for it to pass. In general that's good. But it means that features which were introduced under the notion of "the people who care about this area will tend to its maintenance" slowly become _everybody's_ problem as soon as they have any CI coverage. Another example here is the cmake stuff. Or the recent discussion about "-x" and bash.
Quoted 5 lines
I wonder if there's a good way to make some CI results informational, rather than "failing". I.e., run scalar tests via CI, but if you're not working on scalar, you don't have to care. Folks who are interested in the area would keep tabs on those results and make sure that Junio's tree stays passing.
I think if we're not caring about its failures in combination with git.git changes there wouldn't be much point in having it in-tree at all. That would just be like what we've got with git-cinnabar.git.
I would like it in tree. I just don' think the test/CI setup needs to be a special snowflake.
Quoted 13 lines
That view disagrees with the final paragraph here, though:
Quoted 10 lines
The reason we do that with the completion is because some changes to e.g. tweak getopts will need to have a corresponding change to the completion.
The reason we've not done that with contrib/{subtree,mw-to-git}/ is because those are thoroughly in the category of only incidentally being in-tree.
[...]
Scalar is thoroughly on the "completion" side of that divide, not "subtree".
Quoted 4 lines
I haven't followed the discussion closely, but in my mind "scalar" was still in the "it may live in-tree for convenience, but people who aren't working on it don't necessarily need to care about it" camp. Maybe that's not the plan, though.
Since v1 of the series[1] it's been compiled unconditionally, and there have been tests. We just didn't run the tests.
In v6 the tests started being run as part of CI, which was ejected in v10 due to "[an] unrelated patch series does not interact well with them", which as I noted upthread in [2] isn't accurate, so I think the stated reason for ejecting the CI from the proposed topic doesn't reflect reality.
Since then 1d855a6b335 (Merge branch 'ab/ci-updates' into next, 2021-12-07) landed, so I'd think that any narrow tweaks to get the CI working could be based on top of that topic.
1. https://lore.kernel.org/git/[email protected]/
2. https://lore.kernel.org/git/[email protected]/
Re: [RFC/PATCH] Makefile: add test-all target
In reply to Jeff KingHi Peff,
Quoted 9 lines · Jeff King
Quoted 3 lines · Ævar Arnfjörð Bjarmason
I just don't think it makes any sense that I edit say refs.[ch], run "make test" locally, but only see that something broke in scalar's specific use of libgit.a later when I look at GitHub CI.
I'm definitely sympathetic to this. Having been surprised by CI failure on something that worked locally is annoying at best, and downright frustrating when you can't easily reproduce the problem.
I feel your frustration. Same here.
Quoted 12 lines
But isn't that already true for most of the value that CI provides? While part of its purpose may be a back-stop for folks who don't run "make test" locally, I think the biggest value is that it covers a much wider variety of platforms and scenarios that you don't get out of "make test" already.
In some of those cases you can reproduce the problem locally by tweaking build or test knobs. But in others it can be quite a bit more challenging (e.g., something that segfaults only on Windows). At least in the proposed change here you'd only be a "make test-all" away from reproducing the problem locally.
Quoted 3 lines
I dunno. I don't feel that strongly either way about whether scalar tests should be part of "make test". Mostly just observing that this is not exactly a new case.
It isn't a new case.
What is new is that we are talking about CI for patches targeting contrib/ specifically to introduce something cautiously that still has a chance of not ending up in Git proper (for whatever reasons), as Junio seems to be anxious to not give any premature "go" to integrate Scalar fully.
In that light, I am somewhat surprised that we are still discussing putting a burden on contributors having to adapt contrib/scalar/ to their changes, when Junio still endeavors the option of not accepting that to-be-adapted code into core Git, after all.
I fully expected everybody to be on board with leaving the responsibility to keep contrib/scalar/ building and passing the tests to _me_, until the day Scalar is accepted as a full Git command (which might not happen).
Quoted 36 lines
Quoted 4 lines
If I'm preparing patches for submission I'll need to get CI passing, so I'll need to fix those tests & behavior either way as it's in-tree. Knowing about the failures later-not-sooner wastes more time, not less.
I think there's probably a tradeoff here. How often you get a "late" notification of a bug (and how much of your time that wastes) versus how much time you spend locally running tests that you don't care about.
I do agree that CI presents a bit of a conundrum for stuff at the edge of the project. It's become a de facto requirement for it to pass. In general that's good. But it means that features which were introduced under the notion of "the people who care about this area will tend to its maintenance" slowly become _everybody's_ problem as soon as they have any CI coverage. Another example here is the cmake stuff. Or the recent discussion about "-x" and bash.
I wonder if there's a good way to make some CI results informational, rather than "failing". I.e., run scalar tests via CI, but if you're not working on scalar, you don't have to care. Folks who are interested in the area would keep tabs on those results and make sure that Junio's tree stays passing.
That view disagrees with the final paragraph here, though:
Quoted 10 lines
The reason we do that with the completion is because some changes to e.g. tweak getopts will need to have a corresponding change to the completion.
The reason we've not done that with contrib/{subtree,mw-to-git}/ is because those are thoroughly in the category of only incidentally being in-tree.
[...]
Scalar is thoroughly on the "completion" side of that divide, not "subtree".
Quoted 4 lines
I haven't followed the discussion closely, but in my mind "scalar" was still in the "it may live in-tree for convenience, but people who aren't working on it don't necessarily need to care about it" camp. Maybe that's not the plan, though.
I had hoped for a clearer answer from Junio where he sees Scalar in the long term, for now he seems to be undecided.
As a consequence, I kept targeting contrib/scalar/ with this first patch series, to leave the door open for keeping it in contrib/ as a "not maintained by Junio!" part of the tree.
That is independent, of course, of my intention to keep maintaining Scalar's code (once we get a few steps further, that is, because we're still quite stuck here, the Scalar patch series has not seen any concerns in the last half dozen iterations about its design nor about its actual code). I intend to keep maintainig the Scalar code no matter whether it lives in contrib/ or whether it will be turned into a first-class command whose source code lives in the top-level directory.
So yes, from my side I do not understand at all where this notion comes from that contrib/scalar/ should be treated any different than contrib/subtree/ for now. At least until contrib/scalar/ is feature-complete, that won't change.
But of course, we can keep discussing back and forth the build process of Scalar, whether it should be tested in CI or not, whether it should be in contrib/ or in the top-level directory or not in Git at all, without getting the Scalar patches anywhere, for the next few years, in which case the outcome of that discussion will be completely moot because the Scalar patches would still be as stuck as they are right now. In which case it would be super annoying for any contributor who had to adapt the code in contrib/scalar/ to code changes in libgit.a, for no value in return whatsoever. So far, that contributor has been me.
I sincerely hope that it won't come to that, and that we can move forward with this here patch series, with the next ones I have lined up to make Scalar feature-complete, and _then_ discuss the merits of making Scalar a first-class Git command or not. At that point we will automatically have the answer whether to build Scalar and run its tests as part of Git's CI.
Ciao,
Dscho
Re: [RFC/PATCH] Makefile: add test-all target
In reply to Junio C HamanoQuoted 13 lines · Junio C Hamano
Quoted 9 lines · Ævar Arnfjörð Bjarmason
Quoted 4 lines
So, how about doing it this way? This is based on 'master' and does not cover contrib/scalar, but if we want to go this route, it should be trivial to do it on top of a merge of ab/ci-updates and js/scalar into 'master'. Good idea? Terrible idea? Not good enough?
With the caveat that I think the greater direction here makes no sense, i.e. scalar didn't need its own build system etc. in the first place, so having hack-upon-hack to fix various integration issues is clearly worse than just having it behave like everything else....
Quoted 4 lines
We decided to start Scalar in contrib/, as it hasn't been proven that Scalar is in a good enough shape to deserve to be in this tree, and we are giving it a chance by adding it to contrib/ first, hoping that it may graduate to the more official status someday [*].
Is that the hope? I thought the wish was for it to eventually "disappear" rather than "graduate", as per the following bits of Dscho's cover letter:
"""
The Scalar project was designed to be a self-destructing vehicle...For example, partial clone, sparse-checkout, and scheduled background maintenance have already been upstreamed and removed from Scalar proper...[Adding Scalar to contrib will] make it substantially easier to experiment with moving functionality from Scalar into core Git. """
Re: [RFC/PATCH] Makefile: add test-all target
Quoted 23 lines · Elijah Newren
Quoted 13 lines · Junio C Hamano
Quoted 9 lines · Ævar Arnfjörð Bjarmason
Quoted 4 lines
So, how about doing it this way? This is based on 'master' and does not cover contrib/scalar, but if we want to go this route, it should be trivial to do it on top of a merge of ab/ci-updates and js/scalar into 'master'. Good idea? Terrible idea? Not good enough?
With the caveat that I think the greater direction here makes no sense, i.e. scalar didn't need its own build system etc. in the first place, so having hack-upon-hack to fix various integration issues is clearly worse than just having it behave like everything else....
Quoted 4 lines
We decided to start Scalar in contrib/, as it hasn't been proven that Scalar is in a good enough shape to deserve to be in this tree, and we are giving it a chance by adding it to contrib/ first, hoping that it may graduate to the more official status someday [*].
Is that the hope? I thought the wish was for it to eventually "disappear" rather than "graduate", as per the following bits of Dscho's cover letter:
Quoted 7 lines
"""
The Scalar project was designed to be a self-destructing vehicle...For example, partial clone, sparse-checkout, and scheduled background maintenance have already been upstreamed and removed from Scalar proper...[Adding Scalar to contrib will] make it substantially easier to experiment with moving functionality from Scalar into core Git. """
I can go either way, but my impression from Dscho's messages has always been that there is no strong reason to switch existing scalar users to say "git clone <options that give behaviour like scalar>" when their fingers and scripts are used to say "scalar <this>", and a very thin shell may remain in some form in the ideal world.
Re: [RFC/PATCH] Makefile: add test-all target
In reply to Junio C HamanoHi Junio,
Quoted 9 lines · Junio C Hamano
We ship contrib/ stuff within our primary source tree but except for the completion scripts that are tested from our primary test suite, their test suites are not run in the CI.
Teach the main
Makefilea "test-extra" target, which goes into each package in contrib/ whoseMakefilehas its own "test" target and runs "make test" there. Add a "test-all" target to make it easy to drive both the primary tests and these contrib tests from CI and use it.
That sends a strong message that the stuff in contrib/ is now fully under your maintenance, i.e. first-class supported.
If I were you, I wouldn't.
Quoted 5 lines
Quoted 3 lines · Junio C Hamano
That is an interesting way to demonstrate how orthogonal the issues are, which in turn means that it is not such a big deal to add back the coverage to the part that goes to contrib/scalar/.
I'd rather focus, _some_ focus, on the actual Scalar idea and code.
Quoted 6 lines
Quoted 1 line
As the actual implementation, it is a bit too icky, though.
So, how about doing it this way? This is based on 'master' and does not cover contrib/scalar, but if we want to go this route, it should be trivial to do it on top of a merge of ab/ci-updates and js/scalar into 'master'. Good idea? Terrible idea? Not good enough?
Peff mentioned a couple of times how tedious it is to address CI failures e.g. in the Windows part of Git's CI runs.
So it makes only sense to avoid the same problem with contrib/scalar/ altogether, especially as long as you keep saying that you are still uncertain whether it will make it into Git as a top-level command.
Which is a strong argument in favor of just leaving the CI part of contrib/scalar/ out for now, and let it remain _my_ responsibility to react to any build/test problems arising from unrelated patch series entering seen.
Doing it that way would also have the benefit of allowing more focus on the actual code in contrib/scalar/scalar.c.
Not that it needs more review, I don't think, as both Stolee and Elijah gave their thumbs-up already, and I've not received any feedback that would require further changes to scalar.c, at least as of _this_ patch series.
Ciao,
Dscho
Re: [RFC/PATCH] Makefile: add test-all target
Quoted 8 lines · Johannes Schindelin
Quoted 5 lines
Teach the main
Makefilea "test-extra" target, which goes into each package in contrib/ whoseMakefilehas its own "test" target and runs "make test" there. Add a "test-all" target to make it easy to drive both the primary tests and these contrib tests from CI and use it.That sends a strong message that the stuff in contrib/ is now fully under your maintenance, i.e. first-class supported.
I do not think running tests on stuff in contrib/ sends any such message. It primarily helps _us_ to catch more regressions than we may otherwise miss. By the way, this is not limited to contrib/; if we had tests for gitk, we would have caught the recent regression in "diff -m" before it got inflicted on the general public, but that would not have been just to help "gitk", but to help keep "diff -m" sane and stable [*].
By running tests on in-tree contrib/ like scalar, at least we would notice when we are making breaking changes. At least, the need for scalar (either for the API broken by such a change to be kept unchanged or done in a different way, or the code that uses the API on the scalar side to be updated) would be noticed earlier than stuff totally outside and not even in contrib/.
Of course, you have to bear the burden of (A) changing the way scalar uses the API, or (B) participating in the design of the change to the API that may break scalar's use so that everybody including scalar would be happy, or both. It's not like I am responsible for everything that happens in the tree, and it is our shared responsibility to maintain the health of the codebase. It is not limited to stuff inside or outside contrib/.
There are projects that want to use libgit.a by binding us as a submodule and without interacting with us very much. And they are on their own when we change the internals. Do you mean that you want to make scalar into the same status as they are?
Quoted 4 lines
Not that it needs more review, I don't think, as both Stolee and Elijah gave their thumbs-up already, and I've not received any feedback that would require further changes to
scalar.c, at least as of _this_ patch series.
So that argues even more to have a way to make sure we catch unintended breakages by any future mindless tree-wide "clean-ups" and interface changes, no?
[Footnote]
* I just double checked the candidates for "test-extra" to see if they are meant to run with a random Git they happen to see on the $PATH, or they are designed to test with the version of Git we just built, and it seems it is the latter for the ones nominated in the test-extra patch. Otherwise it would indeed reduce the benefit in half---we are not helping to catch regressions in the core stuff in such a case.
Re: [RFC/PATCH] Makefile: add test-all target
Quoted 11 lines · Junio C Hamano
Quoted 8 lines · Johannes Schindelin
Quoted 5 lines
Teach the main
Makefilea "test-extra" target, which goes into each package in contrib/ whoseMakefilehas its own "test" target and runs "make test" there. Add a "test-all" target to make it easy to drive both the primary tests and these contrib tests from CI and use it.That sends a strong message that the stuff in contrib/ is now fully under your maintenance, i.e. first-class supported.
Quoted 7 lines
I do not think running tests on stuff in contrib/ sends any such message. It primarily helps _us_ to catch more regressions than we may otherwise miss. By the way, this is not limited to contrib/; if we had tests for gitk, we would have caught the recent regression in "diff -m" before it got inflicted on the general public, but that would not have been just to help "gitk", but to help keep "diff -m" sane and stable [*].
I'd actually be a lot more sympathetic to automatically running gitk tests, because it's just consuming the public API of git (i.e., the scriptable plumbing interface). If we accidentally break that, it is the problem of the person who made the breaking change, and we would want them to know it as soon as possible.
With something like scalar, though, it is adding new callers of the private API. It might be useful for somebody doing tree-wide refactoring to know they've broken something there. But it might also be a hassle, because now they have to care about fixing it, if they are interested in un-breaking their build (or un-breaking CI). The scalar code is now their problem, even though it's "just" in contrib/.
In other words, it comes down to a question of where the burden for fixing things lies. Of course it is nice if somebody doing tree-wide refactoring fixes up scalar, too. But by making it optional to build and/or test stuff in contrib/ (rather than tying it to "make all" or to CI), it lets people decide how nice they want to be.
For other stuff in contrib/, I'm not sure to what degree it applies. diff-highlight is pretty standalone for instance. I guess it _could_ be broken by a public-API change in Git, but I find it pretty unlikely.
Quoted 8 lines
Of course, you have to bear the burden of (A) changing the way scalar uses the API, or (B) participating in the design of the change to the API that may break scalar's use so that everybody including scalar would be happy, or both. It's not like I am responsible for everything that happens in the tree, and it is our shared responsibility to maintain the health of the codebase. It is not limited to stuff inside or outside contrib/.
Quoted 4 lines
There are projects that want to use libgit.a by binding us as a submodule and without interacting with us very much. And they are on their own when we change the internals. Do you mean that you want to make scalar into the same status as they are?
I kind of thought that final paragraph was the plan, at least to start with.
-Peff
Re: [RFC/PATCH] Makefile: add test-all target
Quoted 7 lines · Jeff King
Quoted 4 lines
There are projects that want to use libgit.a by binding us as a submodule and without interacting with us very much. And they are on their own when we change the internals. Do you mean that you want to make scalar into the same status as they are?
I kind of thought that final paragraph was the plan, at least to start with.
Oh, and just to be clear: I am really OK with either direction. I'm only claiming that I think both approaches are self-consistent and are making a tradeoff (finding bugs earlier, versus shifting burden of bug-fixing around).
-Peff
Bagas Sanjaya
Tested-byRe: [RFC/PATCH] Makefile: add test-all target
In reply to Junio C HamanoQuoted 11 lines · Junio C Hamano
We ship contrib/ stuff within our primary source tree but except for the completion scripts that are tested from our primary test suite, their test suites are not run in the CI.
Teach the main
Makefilea "test-extra" target, which goes into each package in contrib/ whoseMakefilehas its own "test" target and runs "make test" there. Add a "test-all" target to make it easy to drive both the primary tests and these contrib tests from CI and use it.Signed-off-by Junio C Hamano <[email protected]>
No test failures found with test-all on my system.
Signature · 1 line
An old man doll... just what I always wanted! - Clara
Hi Junio,
Quoted 8 lines · Junio C Hamano
Quoted 5 lines · Elijah Newren
From your wording it sounds like the plan might not include moving these tests over. Perhaps it doesn't make sense to move them all over, but since they've caught problems in both Scalar and core Git, it would be nice to see many of those tests come to Git as well as part of a future follow on series.
Quoted 5 lines
Yeah, we may be initially queuing this without tests for expediency, but a production code cannot go forever without CI tests to ensure continued code health. People make changes in other parts of the system Scalar may depend on and unknowingly break some assumption Scalar makes on it.
The Scalar Functional Tests were designed with Azure Repos in mind, i.e. they specifically verify that the gvfs-helper (emulating Partial Clone using the predecessor of Partial Clone, the GVFS protocol) manages to access the repositories in the intended way.
I do not know off-hand how entangled the GVFS part is in the test suite, but from what I recall, every single test starts with cloning a test repository. From Azure Repos. Using the gvfs-helper.
Which means that the gvfs-helper would need to be upstreamed and be maintained in the git.git repository proper.
Previously I was under the impression that that might be met with grumpy rejection.
I do realize, though, that clarity of intention has been missing from this mail thread all around, so let me ask point blank: Junio, do you want me to include upstreaming gvfs-helper in the overall Scalar plan?
Ciao,
Dscho
Quoted 30 lines · Johannes Schindelin
Hi Junio,
Quoted 8 lines · Junio C Hamano
Quoted 5 lines · Elijah Newren
From your wording it sounds like the plan might not include moving these tests over. Perhaps it doesn't make sense to move them all over, but since they've caught problems in both Scalar and core Git, it would be nice to see many of those tests come to Git as well as part of a future follow on series.
Quoted 5 lines
Yeah, we may be initially queuing this without tests for expediency, but a production code cannot go forever without CI tests to ensure continued code health. People make changes in other parts of the system Scalar may depend on and unknowingly break some assumption Scalar makes on it.
The Scalar Functional Tests were designed with Azure Repos in mind, i.e. they specifically verify that the
gvfs-helper(emulating Partial Clone using the predecessor of Partial Clone, the GVFS protocol) manages to access the repositories in the intended way.I do not know off-hand how entangled the GVFS part is in the test suite, but from what I recall, every single test starts with cloning a test repository. From Azure Repos. Using the
gvfs-helper.Which means that the
gvfs-helperwould need to be upstreamed and be maintained in the git.git repository proper.
Quoted 6 lines
Previously I was under the impression that that might be met with grumpy rejection.
I do realize, though, that clarity of intention has been missing from this mail thread all around, so let me ask point blank: Junio, do you want me to include upstreaming
gvfs-helperin the overall Scalar plan?
An alternate way would be be to have our own tests build git, and then clone and build those third party repos and test them.
I had a patch to do that for git-annex. I think it would be a good idea to pursue it in general for prominent downstream projects as part of some extended integration testing:
https://lore.kernel.org/git/[email protected]/
Quoted 11 lines · Johannes Schindelin
Hi Junio,
Quoted 7 lines · Junio C Hamano
Quoted 5 lines · Elijah Newren
From your wording it sounds like the plan might not include moving these tests over. Perhaps it doesn't make sense to move them all over, but since they've caught problems in both Scalar and core Git, it would be nice to see many of those tests come to Git as well as part of a future follow on series.
Moving the C# test suite over doesn't make a lot of sense. We also are re-using the test suite from VFS for Git, which is probably overkill here. Those tests were created due to issues that arose with the virtual filesystem (paired with the GVFS protocol for finding missing objects) and most of them probably don't test anything interesting in Scalar.
When we _do_ find something interesting in that suite, we port over the test as a normal Git test so the regression is avoided in the future.
We work to test the -rc0 version of every release with our custom patches in microsoft/git and then run them through the Scalar and VFS for Git functional tests as a necessary step before releasing to our internal users. Since we are doing that already, it is a better use of time to port tests that actually matter when they come up rather than port the entire test suite.
Quoted 5 lines
Quoted 5 lines
Yeah, we may be initially queuing this without tests for expediency, but a production code cannot go forever without CI tests to ensure continued code health. People make changes in other parts of the system Scalar may depend on and unknowingly break some assumption Scalar makes on it.
I think it is important to keep in mind that the Scalar features that are being submitted here are getting Git-style tests included. The only thing that is missing right now is a firm link with Git's CI system, which can be added quickly once things have calmed down in the build system.
If we are interested in doing something more substantial that is closer to the Scalar functional tests, then it is important to know that those tests are running against a production server to clone data and fetch it dynamically throughout. That is not exactly something we have done in the Git test suite before.
In fact, I don't think Scalar introduces anything novel here: if we want to add more coverage of running Git commands while in a sparse-checkout _and_ partial clone _and_ have a lot of optional config set, then we can do that independently of Scalar. 'scalar clone' just sets up a repository in a state that an expert user could do themselves, so should we spend a lot of effort creating that environment in our test suite?
We have this already in some form:
1. t1091 and t1092 try to cover important sparse-checkout behavior.
2. t0410, t5616, and others try to cover important partial clone behavior.
3. Our GIT_TEST_* variables that are enabled in one of our CI runs test many of the advanced config options enabled by Scalar.
The thing that is missing is "all of these things at once" which would be difficult to do across the test suite with our current test design. I'm happy to provide the service of checking the Scalar functional tests before each release as an expensive way to check that combination of configuration without adding that cost to every CI run and developer inner loop.
Quoted 8 lines
The Scalar Functional Tests were designed with Azure Repos in mind, i.e. they specifically verify that the
gvfs-helper(emulating Partial Clone using the predecessor of Partial Clone, the GVFS protocol) manages to access the repositories in the intended way.I do not know off-hand how entangled the GVFS part is in the test suite, but from what I recall, every single test starts with cloning a test repository. From Azure Repos. Using the
gvfs-helper.
There is a single test that checks that 'scalar clone' against github.com works appropriately [1]. We don't duplicate all of the other tests in this environment.
Quoted 9 lines
Which means that the
gvfs-helperwould need to be upstreamed and be maintained in the git.git repository proper.Previously I was under the impression that that might be met with grumpy rejection.
I do realize, though, that clarity of intention has been missing from this mail thread all around, so let me ask point blank: Junio, do you want me to include upstreaming
gvfs-helperin the overall Scalar plan?
I, for one, don't think that has much value for the core Git project.
Thanks,
-Stolee
I know this was directed to Junio, but I feel like it was my earlier comment that accidentally opened this can of worms, so if my opinion helps resolve it at all...
Quoted 24 lines · Johannes Schindelin
Hi Junio,
Quoted 8 lines · Junio C Hamano
Quoted 5 lines · Elijah Newren
From your wording it sounds like the plan might not include moving these tests over. Perhaps it doesn't make sense to move them all over, but since they've caught problems in both Scalar and core Git, it would be nice to see many of those tests come to Git as well as part of a future follow on series.
Quoted 5 lines
Yeah, we may be initially queuing this without tests for expediency, but a production code cannot go forever without CI tests to ensure continued code health. People make changes in other parts of the system Scalar may depend on and unknowingly break some assumption Scalar makes on it.
The Scalar Functional Tests were designed with Azure Repos in mind, i.e. they specifically verify that the
gvfs-helper(emulating Partial Clone using the predecessor of Partial Clone, the GVFS protocol) manages to access the repositories in the intended way.
Quoted 6 lines
I do not know off-hand how entangled the GVFS part is in the test suite, but from what I recall, every single test starts with cloning a test repository. From Azure Repos. Using the
gvfs-helper.Which means that the
gvfs-helperwould need to be upstreamed and be maintained in the git.git repository proper.
Ah, sorry, I was remembering this from an earlier cover letter of yours:
"""
But it was realized that many of these key concepts were independent of the actual VFS and its projection of the working directory. The Scalar project was created to make that separation, refine the key concepts, and then extract those features into the new Scalar command.
"""
when I read
"""
One other thing is very interesting about that vfs-with-scalar branch
thicket: it contains a GitHub workflow which will run Scalar's quite extensive Functional Tests suite. This test suite is quite comprehensive and caught us a lot of bugs in the past, not only in the Scalar code, but also core Git.
"""
and I was thinking (despite the branch name) that you had some scalar
+ git (w/o gvfs) tests that were interesting but not planning to upstream. I agree that if they're gvfs + scalar + git then they make sense to keep internal to your work, though I hope that for any bugs your internal testcases find in git, that you find an upstreamable testcase to submit. I believe Stolee has done exactly that in the past, so just more of that would be good.
Quoted 6 lines
Previously I was under the impression that that might be met with grumpy rejection.
I do realize, though, that clarity of intention has been missing from this mail thread all around, so let me ask point blank: Junio, do you want me to include upstreaming
gvfs-helperin the overall Scalar plan?
I know I'm not Junio, but if my opinion matters, I don't think that needs to be part of the plan.
Quoted 8 lines · Johannes Schindelin
The Scalar Functional Tests were designed with Azure Repos in mind, i.e. they specifically verify that the
gvfs-helper(emulating Partial Clone using the predecessor of Partial Clone, the GVFS protocol) manages to access the repositories in the intended way.
...
I do realize, though, that clarity of intention has been missing from this mail thread all around, so let me ask point blank: Junio, do you want me to include upstreaminggvfs-helperin the overall Scalar plan?
Sorry, I do not follow.
What I was lamenting about was the lack of CI test coverage of stuff that is already being considered to go 'next'. Specifically, since contrib/scalar/Makefile in 'seen' has a 'test' target, it would be a shame not to exercise it, when we should be able to do so in the CI fairly easily.
I fail to see what gvfs-helper has to do with anything in the context of advancing the js/scalar topic as we have today. If "The Scalar Functional Tests" that were designed with Azure Repos in mind is not a good fit to come into contrib/scalar/, it is fine not to have it here---lack of it would not make the test target you have in contrib/scalar/Makefile any less valuable, I would think.
Unless you are saying that "make -C contrib/scalar test" is useless, that is. But I do not think that is the case.
Hi Junio,
Quoted 11 lines · Junio C Hamano
Quoted 8 lines · Johannes Schindelin
The Scalar Functional Tests were designed with Azure Repos in mind, i.e. they specifically verify that the
gvfs-helper(emulating Partial Clone using the predecessor of Partial Clone, the GVFS protocol) manages to access the repositories in the intended way.
...
I do realize, though, that clarity of intention has been missing from this mail thread all around, so let me ask point blank: Junio, do you want me to include upstreaminggvfs-helperin the overall Scalar plan?
Quoted 1 line
Sorry, I do not follow.
In
https://lore.kernel.org/git/CABPp-BGpe9Q5k22Yu8a=1xwu=pZYSeNQoqEgf+DN07cU4EB1ew@mail.gmail.com/
(i.e. in the great great grand parent of this mail), you specifically replied to my mentioning Scalar's Functional Test suite:
> > One other thing is very interesting about that vfs-with-scalar > > branch thicket: it contains a GitHub workflow which will run > > Scalar's quite extensive Functional Tests suite. This test > > suite is quite comprehensive and caught us a lot of bugs in > > the past, not only in the Scalar code, but also core Git. >
> From your wording it sounds like the plan might not include > moving these tests over. Perhaps it doesn't make sense to move > them all over, but since they've caught problems in both Scalar > and core Git, it would be nice to see many of those tests come > to Git as well as part of a future follow on series.
I had mentioned a couple of times that I had no intention to move Scalar's Function Tests into contrib/scalar/, and your wording "it would be nice to see many of those tests come to Git as well" made it sound as if you disagreed with that intention.
But it was not a clear "please do port them over" nor a "nah, we don't want that test suite implemented in C# and requiring, for the most part, access to a dedicacted Azure Repo".
Hence I was asking for a clear answer to the question whether you want me to spend time on preparing a patch series to contribute Scalar's Functional Tests to contrib/scalar/ as well.
I _suspect_ your clear answer, if you are willing to give it as clearly, to be "no, we do not do integration tests here, and besides, C# is not a language we want to add to Git's tree".
Quoted 5 lines
What I was lamenting about was the lack of CI test coverage of stuff that is already being considered to go 'next'. Specifically, since contrib/scalar/Makefile in 'seen' has a 'test' target, it would be a shame not to exercise it, when we should be able to do so in the CI fairly easily.
We do have a very different understanding of "fairly easily" in that case. Three iterations, and three weeks time spent on implementing what you suggest, only to see broken by the merge of the ab/ci-updates patch series, suggesting a fixup for the incorrect merge, seeing that fixup rejected, and then more discussing, all of that does not strike me as "fairly easily". It strikes me as "a lot of time and effort was spent, mostly stepping on toes".
Granted, if ab/ci-updates would not have happened, it would have been much easier. Or if ab/ci-updates had waited until js/scalar advanced to next. But the way it happened was (unnecessarily?) un-easy.
Quoted 2 lines
I fail to see what gvfs-helper has to do with anything in the context of advancing the js/scalar topic as we have today.
Okay, okay! I was just asking about gvfs-helper because that would be required to port over Scalar's Functional Tests. The same Functional Tests that I heard you mentioning would be "nice to see" to "come to Git as well".
Quoted 4 lines
If "The Scalar Functional Tests" that were designed with Azure Repos in mind is not a good fit to come into contrib/scalar/, it is fine not to have it here---lack of it would not make the test target you have in contrib/scalar/Makefile any less valuable, I would think.
The test target won't go anywhere, no worries. Just like the test target in contrib/subtree/ does not go anywhere.
And just like contrib/subtree/, it does not have to be run as part of Git's CI build.
Quoted 2 lines
Unless you are saying that "make -C contrib/scalar test" is useless, that is. But I do not think that is the case.
It is as useful as make -C contrib/subtree test. Which, as Ævar will readily offer, is broken, because it does not ensure that top-level make all is executed and therefore in a fresh checkout will fail.
Of course, I disagree that it is "broken". It works as designed. It is in the contrib/ part of the tree, i.e. safely in the realm of "you have to build Git first, and then the thing in contrib/". In other words, the idea to "fix" this kind of "broken"ness is a solution in search of a problem.
And as I have said multiple times, I still think that having Scalar's code in contrib/ is a good spot to experiment with it. It sends the right signal of "this is not really something we promise to maintain just yet". It is a logical place for code that developers can build themselves, but that is not built and installed with Git by default.
Having it in the Git tree will give interested developers a chance who want to clone a large repository on Linux, without having to touch anything with "Microsoft" in its repository name.
Having it in the Git tree will give interested developers a chance to experiment with things like "let's try to let scalar clone _not_ clone into <enlistment>/src/, but instead create a bare clone in <enlistment>/.git and make <enlistment>/src/ a worktree". Things like that.
I would find those things quite a bit more useful than to force regular Git contributors who want to change libgit.a (even if it is just pointless refactoring) to pay attention to contrib/scalar/ in CI, when there is still no clear answer whether Scalar will even become a first-class Git command eventually (which I hope it will, of course).
Ciao,
Dscho
Quoted 9 lines · Johannes Schindelin
Hi Junio,
[...]
We do have a very different understanding of "fairly easily" in that case. Three iterations, and three weeks time spent on implementing what you suggest, only to see broken by the merge of theab/ci-updatespatch series, suggesting a fixup for the incorrect merge, seeing that fixup rejected, and then more discussing, all of that does not strike me as "fairly easily". It strikes me as "a lot of time and effort was spent, mostly stepping on toes".
I sent you a working path to a fixup in [1] on the 23rd of November where we won't go from running zero tests in compile-only to running just the scalar test.
Junio replied[2] ("the above" referring to [1]):
I think the above shows that it is a bug in the topic itself,
You didn't reply further in that fixup thread, and then your v9 re-roll a week later still had the same issue[3] discussed therein. I again pointed that out[4]:
Is it intentional that the previously compile-only "pedantic" job is now running the scalar tests?
You didn't reply, but in your v10 decided to make the current iteration of this series have no CI testing at all, and cited the interaction with ab/ci-updates[4]:
because a recent unrelated patch series does not interact well with them.
Which I think is clearly inaccurate, because...
Quoted 3 lines
Granted, if
ab/ci-updateswould not have happened, it would have been much easier. Or ifab/ci-updateshad waited untiljs/scalaradvanced tonext. But the way it happened was (unnecessarily?) un-easy.
...your initial patch to run the scalar tests in CI[5] was part of v7, and had the issue described above. It pre-dates the v1 of ab/ci-updates being on-list by a couple of days[6].
So yes, I do think it was "easy", as in that was an easy fix-up. You just didn't follow up on it and submitted re-rolls with the already noted breakage.
I don't blame you for that, maybe you were busy, it slipped through etc.
But I don't accept that delays in this topic are my fault, or something to the effect that that this whole saga represents some failure of the review process.
Our topics textually/semantically conflicted, it happens. I offered a fixup & way forward. Fixing it was trivial, and still is. You just didn't follow-up.
Quoted 11 lines
[...]
Quoted 4 lines
If "The Scalar Functional Tests" that were designed with Azure Repos in mind is not a good fit to come into contrib/scalar/, it is fine not to have it here---lack of it would not make the test target you have in contrib/scalar/Makefile any less valuable, I would think.
The test target won't go anywhere, no worries. Just like the test target in contrib/subtree/ does not go anywhere.
And just like
contrib/subtree/, it does not have to be run as part of Git's CI build.
But unlike contrib/completion, which we do run as part of Git's CI build[7]?
Quoted 6 lines
Quoted 2 lines
Unless you are saying that "make -C contrib/scalar test" is useless, that is. But I do not think that is the case.
It is as useful as
make -C contrib/subtree test. Which, as Ævar will readily offer, is broken, because it does not ensure that top-levelmake allis executed and therefore in a fresh checkout will fail.
Before the scalar topic there was only one "make" entry point to build libgit.a, contrib/scalar/Makefile makes that two. That was the immediate prompt for the fixup discussion in [1].
So no, I won't offer that "make -C contrib/subtree test" is broken, it doesn't try to build libgit.a and errors out right away if git isn't built.
Your scalar patches do try, get most of the way there, and fail.
Your bicycle isn't broken if it doesn't make coffee, but if your fridge has a built-in coffee maker and it doesn't work it's broken, at least as it pertains to its coffee making function.
I think I made that distinction clear in [8], but apparently not clear enough, as you seem to be under the impression that I was conveying the opposite of the idea I was trying to get across.
Quoted 4 lines
Of course, I disagree that it is "broken". It works as designed. It is in the contrib/ part of the tree, i.e. safely in the realm of "you have to build Git first, and then the thing in contrib/". In other words, the idea to "fix" this kind of "broken"ness is a solution in search of a problem.
I agree with that, but it's your proposed patches that contain the build integration you're describing as unnecessary for "contrib/subtree/". In v8->v8 of the series you changed the CI integration from:
make -C contrib/scalar test
To:
make && make -C contrib/scalar test
While keeping the bits in contrib/scalar/Makefile that made it go most of the way towards a working "libgit.a" useful for testing, but it breaks before we get everything we need to run the "test" target.
Which I find to be odd given the above comparison to contib/subtree/. If you have to build git first at the top level why is it trying and failing to build git? "contrib/subtree" doesn't.
Quoted 6 lines
[...]
I would find those things quite a bit more useful than to force regular Git contributors who want to change libgit.a (even if it is just pointless refactoring) to pay attention to contrib/scalar/ in CI, when there is still no clear answer whether Scalar will even become a first-class Git command eventually (which I hope it will, of course).
It's in-tree, scalar.c is compiled by default, so they'll have to choice but to pay attention to it.
The question is whether we should have test and CI coverage for code in that state.
1. https://lore.kernel.org/git/[email protected]/
2. https://lore.kernel.org/git/[email protected]/
3. https://lore.kernel.org/git/[email protected]/
4. https://lore.kernel.org/git/[email protected]/
5. https://lore.kernel.org/git/1b0328fa236a35c2427b82f53c32944e513580d3.1637158762.git.gitgitgadget@gmail.com/
6. https://lore.kernel.org/git/[email protected]/
7. https://lore.kernel.org/git/[email protected]/
8. https://lore.kernel.org/git/[email protected]/
9. https://lore.kernel.org/git/[email protected]/
Hi Dscho,
Quoted 23 lines · Johannes Schindelin
Hi Junio,
Quoted 11 lines · Junio C Hamano
Quoted 8 lines · Johannes Schindelin
The Scalar Functional Tests were designed with Azure Repos in mind, i.e. they specifically verify that the
gvfs-helper(emulating Partial Clone using the predecessor of Partial Clone, the GVFS protocol) manages to access the repositories in the intended way.
...
I do realize, though, that clarity of intention has been missing from this mail thread all around, so let me ask point blank: Junio, do you want me to include upstreaminggvfs-helperin the overall Scalar plan?Quoted 1 line
Sorry, I do not follow.
In
https://lore.kernel.org/git/CABPp-BGpe9Q5k22Yu8a=1xwu=pZYSeNQoqEgf+DN07cU4EB1ew@mail.gmail.com/
(i.e. in the great great grand parent of this mail), you specifically replied to my mentioning Scalar's Functional Test suite:
Quoted 11 lines
> > One other thing is very interesting about that vfs-with-scalar > > branch thicket: it contains a GitHub workflow which will run > > Scalar's quite extensive Functional Tests suite. This test > > suite is quite comprehensive and caught us a lot of bugs in > > the past, not only in the Scalar code, but also core Git. >
> From your wording it sounds like the plan might not include > moving these tests over. Perhaps it doesn't make sense to move > them all over, but since they've caught problems in both Scalar > and core Git, it would be nice to see many of those tests come > to Git as well as part of a future follow on series.
This is me and my email you are quoting; these aren't Junio's words. I'm afraid my confusion may have snowballed for others here. Sorry about that.
I simply misunderstood at the time -- I thought there were scalar-only tests (rather than scalar+gvfs tests) that were not being considered for upstreaming. As I mentioned before[1], I'm sorry for the confusion and seemingly opening an unrelated can of worms. I agree that we don't need gvfs tests, or tests that combine gvfs with other things like scalar, or c# tests.
[1] https://lore.kernel.org/git/CABPp-BFmNiqY=NfN7Ys3XE8wYBn1EQ_War+0QLq96Tk7FO6zfg@mail.gmail.com/
Hi Elijah,
Quoted 39 lines · Elijah Newren
Quoted 21 lines · Johannes Schindelin
Quoted 11 lines · Junio C Hamano
Quoted 8 lines · Johannes Schindelin
The Scalar Functional Tests were designed with Azure Repos in mind, i.e. they specifically verify that the
gvfs-helper(emulating Partial Clone using the predecessor of Partial Clone, the GVFS protocol) manages to access the repositories in the intended way.
...
I do realize, though, that clarity of intention has been missing from this mail thread all around, so let me ask point blank: Junio, do you want me to include upstreaminggvfs-helperin the overall Scalar plan?Quoted 1 line
Sorry, I do not follow.
In
https://lore.kernel.org/git/CABPp-BGpe9Q5k22Yu8a=1xwu=pZYSeNQoqEgf+DN07cU4EB1ew@mail.gmail.com/
(i.e. in the great great grand parent of this mail), you specifically replied to my mentioning Scalar's Functional Test suite:Quoted 11 lines
> > One other thing is very interesting about that vfs-with-scalar > > branch thicket: it contains a GitHub workflow which will run > > Scalar's quite extensive Functional Tests suite. This test > > suite is quite comprehensive and caught us a lot of bugs in > > the past, not only in the Scalar code, but also core Git. >
> From your wording it sounds like the plan might not include > moving these tests over. Perhaps it doesn't make sense to move > them all over, but since they've caught problems in both Scalar > and core Git, it would be nice to see many of those tests come > to Git as well as part of a future follow on series.This is me and my email you are quoting; these aren't Junio's words. I'm afraid my confusion may have snowballed for others here. Sorry about that.
Quoted 8 lines
I simply misunderstood at the time -- I thought there were scalar-only tests (rather than scalar+gvfs tests) that were not being considered for upstreaming. As I mentioned before[1], I'm sorry for the confusion and seemingly opening an unrelated can of worms. I agree that we don't need gvfs tests, or tests that combine gvfs with other things like scalar, or c# tests.
[1] https://lore.kernel.org/git/CABPp-BFmNiqY=NfN7Ys3XE8wYBn1EQ_War+0QLq96Tk7FO6zfg@mail.gmail.com/
No worries, I am glad it is sorted out now.
Ciao,
Dscho