From d52ca421d52c75837d1614ec54549569f354b7ec Mon Sep 17 00:00:00 2001 From: Daniel Andersson Date: Fri, 1 Apr 2022 12:55:38 +0200 Subject: sync: respect `sync-c` manifest option MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The documentation states that a `sync-c` attribute in the manifest file can set a default for whether only the current branch should be fetched or all branches. This seems to have been broken for some time. Commit 7356114 introduced the `--no-current-branch` CLI option and relied on getting `None` via `optparse` if neither `--current-branch` nor `--no-current-branch` was set to distinguish it from a boolean value. If `None` was received, it would read the value from the manifest option `sync-c`. The parsing went through the utility function `_GetCurrentBranchOnly` which returned `True` if `--current-branch` had been given on the command-line, or fell back on the "superproject" setting, which would either return `True` or `None`. This would incorrectly make `repo` fall back to the manifest setting even if the user had given `--no-current-branch` if no superproject was requested -- the manifest became "too powerful": Command-line Using superproject → `current_branch_only` ------------ ------------------ ----------------------- No From manifest Yes True --current-branch No True --current-branch Yes True --no-current-branch No From manifest ← wrong --no-current-branch Yes True In commit 0cb6e92 the superproject configuration value reading changed from something that could return `None` to something that always returned a boolean. If it returned `False`, this would then incorrectly make `repo` ignore the manifest option even if neither `--current-branch` nor `--no-current-branch` had been given. The manifest default became useless: Command-line Using superproject → `current_branch_only` ------------ ------------------ ----------------------- No False ← wrong Yes True --current-branch No True --current-branch Yes True --no-current-branch No False --no-current-branch Yes True By swapping the order in which the command-line option target and the superproject setting is evaluated, things should work as documented: Command-line Using superproject → `current_branch_only` ------------ ------------------ ----------------------- No From manifest Yes True --current-branch No True --current-branch Yes True --no-current-branch No False --no-current-branch Yes True Change-Id: I933c232d2fbecc6b9bdc364ebac181798bce9175 Tested-by: Daniel Andersson Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/334270 Reviewed-by: Mike Frysinger --- subcmds/sync.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'subcmds/sync.py') diff --git a/subcmds/sync.py b/subcmds/sync.py index baee6b23..9e783205 100644 --- a/subcmds/sync.py +++ b/subcmds/sync.py @@ -280,8 +280,13 @@ later is required to fix a server side protocol bug. return branch def _GetCurrentBranchOnly(self, opt): - """Returns True if current-branch or use-superproject options are enabled.""" - return opt.current_branch_only or git_superproject.UseSuperproject(opt, self.manifest) + """Returns whether current-branch or use-superproject options are enabled. + + Returns: + True if a superproject is requested, otherwise the value of the + current_branch option (True, False or None). + """ + return git_superproject.UseSuperproject(opt, self.manifest) or opt.current_branch_only def _UpdateProjectsRevisionId(self, opt, args, load_local_manifests, superproject_logging_data): """Update revisionId of every project with the SHA from superproject. -- cgit v1.2.3-54-g00ecf