Skip to content

branch --recurse-submodules: Bug fixes and clean ups

Glen Choo via GitGitGadget git 2 versions – In review

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v2 Latest4 patches 3 repliesIn review
  2. v14 patches 5 repliesSuperseded

Changes in v2 from the cover letter

* Patch 1: reword the --track comments to be prescriptive
* Patch 3: remove a now-unnecessary die(). I didn't include a suggestion to inline the advice string to save reviewers the trouble of proofreading (and the format string has no placeholders anyway, so I don't think we'd get much benefit out of typechecking). We can inline it in another series.

Patches in v2 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/4branch: support more tracking modes when recursingbranch.c builtin/submodule--helper.c t/t3207-branch-submodule.sh+67 -70 replies
2/4branch: give submodule updating advice before exitbranch.c+4 -20 replies
3/4branch --set-upstream-to: be consistent when advisingbranch.c+4 -60 replies
4/4branch: remove negative exit codebranch.c+1 -10 replies

Reviews trailers per patch

None of the 4 patches has a Reviewed-by or Acked-by.

PatchÆvar Arnfjörð Bjarmason
SeriesHelped-by
1/4
2/4
3/4Helped-by
4/4

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.

Read the v2 thread

Cover letter [PATCH v2 0/4] branch --recurse-submodules: Bug fixes and clean ups

Thanks for the feedback, all. This version incorporates most of the suggestions (which were pretty small anyway).

== Patch organization

Patches 1-2 are bugfixes, 3-4 are clean ups.

Patch 1 fixes a bug where "git branch --recurse-submodules" would not propagate the "--track" option if its value was "--no-track" or "--track=inherit".

Patch 2 fixes a bug where "git branch --recurse-submodules" would give advice before telling the user what the problem is (instead of the other way around).

Patch 3 fixes some old inconsistencies when "git branch --set-upstream-to" gives advice and when it doesn't.

Patch 4 replaces exit(-1) with exit(1).

== Changes

Since v1:

* Patch 1: reword the --track comments to be prescriptive
* Patch 3: remove a now-unnecessary die(). I didn't include a suggestion to inline the advice string to save reviewers the trouble of proofreading (and the format string has no placeholders anyway, so I don't think we'd get much benefit out of typechecking). We can inline it in another series.

Glen Choo (4):
branch: support more tracking modes when recursing
branch: give submodule updating advice before exit branch --set-upstream-to: be consistent when advising
branch: remove negative exit code

 branch.c                    | 47 +++++++++++++++++++++++++++----------
 builtin/submodule--helper.c |  7 +++---
 t/t3207-branch-submodule.sh | 38 +++++++++++++++++++++++++++++-
 3 files changed, 76 insertions(+), 16 deletions(-)

base-commit: abf474a5dd901f28013c52155411a48fd4c09922
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1190%2Fchooglen%2Fbranch%2Frecursive-fixes-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1190/chooglen/branch/recursive-fixes-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1190

Range-diff vs v1:

1: 0b682c173c8 ! 1: 5e96a09199f branch: support more tracking modes when recursing
@@ branch.c: static int submodule_create_branch(struct repository *r,
+ strvec_push(&child.args, "--track=inherit");
+ break;
+ case BRANCH_TRACK_UNSPECIFIED:
-+ /* Default for "git checkout". No need to pass --track. */
++ /* Default for "git checkout". Do not pass --track. */
+ case BRANCH_TRACK_REMOTE:
-+ /* Default for "git branch". No need to pass --track. */
++ /* Default for "git branch". Do not pass --track. */
+ break;
+ }

 2:  f21d283e5ad = 2:  74b839bfc4e branch: give submodule updating advice before exit
 3:  8e6ea3478b3 ! 3:  64d9b8e0f44 branch --set-upstream-to: be consistent when advising
     @@ Commit message
      
          [1] https://lore.kernel.org/git/[email protected]
      
     +    Helped-by: Ævar Arnfjörð Bjarmason <[email protected]>
          Signed-off-by: Glen Choo <[email protected]>
      
       ## branch.c ##
      @@ branch.c: static void dwim_branch_start(struct repository *r, const char *start_name,
     + 	real_ref = NULL;
       	if (get_oid_mb(start_name, &oid)) {
       		if (explicit_tracking) {
     - 			if (advice_enabled(ADVICE_SET_UPSTREAM_FAILURE)) {
     +-			if (advice_enabled(ADVICE_SET_UPSTREAM_FAILURE)) {
      -				error(_(upstream_missing), start_name);
     -+				int code = die_message(_(upstream_missing),
     -+						       start_name);
     - 				advise(_(upstream_advice));
     +-				advise(_(upstream_advice));
      -				exit(1);
     -+				exit(code);
     - 			}
     - 			die(_(upstream_missing), start_name);
     +-			}
     +-			die(_(upstream_missing), start_name);
     ++			int code = die_message(_(upstream_missing), start_name);
     ++			advise_if_enabled(ADVICE_SET_UPSTREAM_FAILURE,
     ++					  _(upstream_advice));
     ++			exit(code);
       		}
     + 		die(_("not a valid object name: '%s'"), start_name);
     + 	}

4: fb2b472d9ae = 4: 306858373cc branch: remove negative exit code

Signature · 1 line
gitgitgadget