Re: [RFC PATCH 2/2] branch: add --recurse-submodules option for branch creation
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-09-22 11:17:03
On Tue, Sep 21 2021, Glen Choo wrote:
quoted hunk ↗ jump to hunk
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);
quoted hunk ↗ jump to hunk
+ create_branch(r, name, r_start_name, force, clobber_head_ok, reflog, + quiet, track); + + if (!recurse_submodules) { + return; + }
Can lose the braces here...
quoted hunk ↗ jump to hunk
+ + if (repo_read_index(r) < 0) + die(_("index file corrupt"));
...Just as you do here..
quoted hunk ↗ jump to hunk
+ + 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[...]
quoted hunk ↗ jump to hunk
+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 :)