On Tue, Sep 21 2021, Glen Choo wrote:
> static const char *head;
> static struct object_id head_oid;
> +static int recurse_submodules = 0;
Nit: just s/ = 0// will do here, and is the convention typically...
> + r_start_name = strcmp(start_name, "HEAD") ? start_name :
> + refs_resolve_refdup(get_main_ref_store(r), start_name, 0, NULL, NULL);
IMO clearer just as an if/else.
> + if (strcmp(r_start_name, "HEAD"))
> + skip_prefix(r_start_name, "refs/heads/", &r_start_name);
> + create_branch(r, name, r_start_name, force, clobber_head_ok, reflog,
> + quiet, track);
> +
> + if (!recurse_submodules) {
> + return;
> + }
Can lose the braces here...
> +
> + if (repo_read_index(r) < 0)
> + die(_("index file corrupt"));
...Just as you do here..
> +
> + for (i = 0; i < r->index->cache_nr; i++) {
> + const struct cache_entry *ce = r->index->cache[i];
> + if (!S_ISGITLINK(ce->ce_mode))
> + continue;
> + sub = submodule_from_path(r, null_oid(), ce->name);
> + if (repo_submodule_init(&subrepo, r, sub) < 0) {
> + warning(_("Unable to initialize subrepo %s, skipping."), ce->name);
> + continue;
> + }
> + /*
> + * NEEDSWORK: branch tracking is not supported for
> + * submodules because it is not possible to get the
> + * remotes of a submodule.
> + */
It isn't?
$ git -C sha1collisiondetection/ remote -v
origin https://github.com/cr-marcstevens/sha1collisiondetection.git
> [...]
> +test_expect_success 'setup superproject and submodule' '
> + git init parent &&
> + test_commit -C parent bar &&
> + git init child &&
> + test_commit -C child bar &&
> + git -C parent submodule add ../child sub &&
> + git -C parent commit -m "add submodule"
> +'
> +
> +test_expect_success '--recurse-submodules should create branches' '
> + (
> + cd parent &&
> + git branch --recurse-submodules abc &&
> + test_path_is_file .git/refs/heads/abc &&
> + test_path_is_file .git/modules/sub/refs/heads/abc
> + )
> +'
All this manual file checking should depend on REFFILES, but better yet
is there a reason this can't use rev-parse? I.e. why can't we inpect
this state with 'for-each-ref', 'rev-parse' and the like? Does this test
need to assert that these files end up in these specific locations, or
just the ref store? Ditto for the later ones.
> [...]
> + cd parent &&
> + git -c branch.autoSetupMerge=always branch --recurse-submodules ghi main &&
> + test_path_is_file .git/modules/sub/refs/heads/ghi &&
> + test "$(git config branch.ghi.remote)" = . &&
> + test "$(git config branch.ghi.merge)" = refs/heads/main &&
> + test "z$(git -C ../child config branch.ghi.remote)" = z &&
> + test "z$(git -C ../child config branch.ghi.merge)" = z
Use test_cmp, also for this sort of thing the test "x$y" = "x" idiom
isn't needed unless you've got a computer from the mid-90s or something
:)
