sync: respect `sync-c` manifest option
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 <daniel.r.andersson@volvocars.com>
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/334270
Reviewed-by: Mike Frysinger <vapier@google.com>
diff --git a/subcmds/sync.py b/subcmds/sync.py
index baee6b2..9e78320 100644
--- a/subcmds/sync.py
+++ b/subcmds/sync.py
@@ -280,8 +280,13 @@
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.
diff --git a/tests/test_subcmds_sync.py b/tests/test_subcmds_sync.py
new file mode 100644
index 0000000..c1d1758
--- /dev/null
+++ b/tests/test_subcmds_sync.py
@@ -0,0 +1,45 @@
+# Copyright (C) 2022 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Unittests for the subcmds/sync.py module."""
+
+from unittest import mock
+
+import pytest
+
+from subcmds import sync
+
+
+@pytest.mark.parametrize(
+ 'use_superproject, cli_args, result',
+ [
+ (True, ['--current-branch'], True),
+ (True, ['--no-current-branch'], True),
+ (True, [], True),
+ (False, ['--current-branch'], True),
+ (False, ['--no-current-branch'], False),
+ (False, [], None),
+ ]
+)
+def test_get_current_branch_only(use_superproject, cli_args, result):
+ """Test Sync._GetCurrentBranchOnly logic.
+
+ Sync._GetCurrentBranchOnly should return True if a superproject is requested,
+ and otherwise the value of the current_branch_only option.
+ """
+ cmd = sync.Sync()
+ opts, _ = cmd.OptionParser.parse_args(cli_args)
+
+ with mock.patch('git_superproject.UseSuperproject', return_value=use_superproject):
+ assert cmd._GetCurrentBranchOnly(opts) == result
