diff options
author | Raman Tenneti <rtenneti@google.com> | 2021-05-02 19:47:29 -0700 |
---|---|---|
committer | Raman Tenneti <rtenneti@google.com> | 2021-05-03 05:13:23 +0000 |
commit | feb28914bd17aacc2d251516e504a33f991f86c4 (patch) | |
tree | 66865f9cb25d6134acb02e63502636c366a963cd | |
parent | d1f3e149df4482ee24feee038c88df32fbc77380 (diff) | |
download | git-repo-feb28914bd17aacc2d251516e504a33f991f86c4.tar.gz |
superproject: Don't update the commit ids of projects if remote is different.
1) Skip setting the revision id (commit id) for the projects whose
remote doesn't match superproject's remote.
2) exp-superproject/superproject_override.xml includes local_manfiest's
projects. When we load this XML, don't reload projects from local.xml
(otherwise we will get duplicate projects errors).
Tested the code with the following commands.
$ ./run_tests -v
+ Test with local.xml
$ repo_dev init -u sso://android.git.corp.google.com/platform/manifest -b master --use-superproject --partial-clone --clone-filter=blob:limit=10M && mkdir -p .repo/local_manifests && (gcertstatus -quiet=true || gcert) && ln -s /google/src/head/depot/google3/wireless/android/build_tools/aosp/manifests/mirror-aosp-master-with-vendor/local.xml .repo/local_manifests/local.xml
$ repo_dev sync -c -j8
+ Test without local.xml
$ repo_dev init -u sso://android.git.corp.google.com/platform/manifest -b master --partial-clone --clone-filter=blob:limit=10M --repo-rev=main --use-superproject
$ repo_dev sync -c -j8
Bug: [google internal] b/186395810
Change-Id: I4e9d4ac2d94a9fc0cef0ccd787b6310758009e86
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/304882
Tested-by: Raman Tenneti <rtenneti@google.com>
Reviewed-by: Mike Frysinger <vapier@google.com>
-rw-r--r-- | git_superproject.py | 10 | ||||
-rw-r--r-- | manifest_xml.py | 4 | ||||
-rw-r--r-- | subcmds/sync.py | 22 | ||||
-rw-r--r-- | tests/test_git_superproject.py | 70 |
4 files changed, 87 insertions, 19 deletions
diff --git a/git_superproject.py b/git_superproject.py index 2516545d..031f45c9 100644 --- a/git_superproject.py +++ b/git_superproject.py | |||
@@ -262,10 +262,20 @@ class Superproject(object): | |||
262 | return None | 262 | return None |
263 | 263 | ||
264 | projects_missing_commit_ids = [] | 264 | projects_missing_commit_ids = [] |
265 | superproject_remote_name = self._manifest.superproject['remote'].name | ||
265 | for project in projects: | 266 | for project in projects: |
266 | path = project.relpath | 267 | path = project.relpath |
267 | if not path: | 268 | if not path: |
268 | continue | 269 | continue |
270 | # Some manifests that pull projects from the "chromium" GoB | ||
271 | # (remote="chromium"), and have a private manifest that pulls projects | ||
272 | # from both the chromium GoB and "chrome-internal" GoB (remote="chrome"). | ||
273 | # For such projects, one of the remotes will be different from | ||
274 | # superproject's remote. Until superproject, supports multiple remotes, | ||
275 | # don't update the commit ids of remotes that don't match superproject's | ||
276 | # remote. | ||
277 | if project.remote.name != superproject_remote_name: | ||
278 | continue | ||
269 | commit_id = commit_ids.get(path) | 279 | commit_id = commit_ids.get(path) |
270 | if commit_id: | 280 | if commit_id: |
271 | project.SetRevisionId(commit_id) | 281 | project.SetRevisionId(commit_id) |
diff --git a/manifest_xml.py b/manifest_xml.py index 64b7fb4e..73556a5e 100644 --- a/manifest_xml.py +++ b/manifest_xml.py | |||
@@ -596,6 +596,10 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md | |||
596 | return set(x.strip() for x in exclude.split(',')) | 596 | return set(x.strip() for x in exclude.split(',')) |
597 | 597 | ||
598 | @property | 598 | @property |
599 | def HasLocalManifests(self): | ||
600 | return self._load_local_manifests and self.local_manifests | ||
601 | |||
602 | @property | ||
599 | def IsMirror(self): | 603 | def IsMirror(self): |
600 | return self.manifestProject.config.GetBoolean('repo.mirror') | 604 | return self.manifestProject.config.GetBoolean('repo.mirror') |
601 | 605 | ||
diff --git a/subcmds/sync.py b/subcmds/sync.py index 0c386add..9f8de9e5 100644 --- a/subcmds/sync.py +++ b/subcmds/sync.py | |||
@@ -277,7 +277,7 @@ later is required to fix a server side protocol bug. | |||
277 | """Returns True if current-branch or use-superproject options are enabled.""" | 277 | """Returns True if current-branch or use-superproject options are enabled.""" |
278 | return opt.current_branch_only or self._UseSuperproject(opt) | 278 | return opt.current_branch_only or self._UseSuperproject(opt) |
279 | 279 | ||
280 | def _UpdateProjectsRevisionId(self, opt, args): | 280 | def _UpdateProjectsRevisionId(self, opt, args, load_local_manifests): |
281 | """Update revisionId of every project with the SHA from superproject. | 281 | """Update revisionId of every project with the SHA from superproject. |
282 | 282 | ||
283 | This function updates each project's revisionId with SHA from superproject. | 283 | This function updates each project's revisionId with SHA from superproject. |
@@ -287,6 +287,7 @@ later is required to fix a server side protocol bug. | |||
287 | opt: Program options returned from optparse. See _Options(). | 287 | opt: Program options returned from optparse. See _Options(). |
288 | args: Arguments to pass to GetProjects. See the GetProjects | 288 | args: Arguments to pass to GetProjects. See the GetProjects |
289 | docstring for details. | 289 | docstring for details. |
290 | load_local_manifests: Whether to load local manifests. | ||
290 | 291 | ||
291 | Returns: | 292 | Returns: |
292 | Returns path to the overriding manifest file. | 293 | Returns path to the overriding manifest file. |
@@ -302,7 +303,7 @@ later is required to fix a server side protocol bug. | |||
302 | print('error: Update of revsionId from superproject has failed', | 303 | print('error: Update of revsionId from superproject has failed', |
303 | file=sys.stderr) | 304 | file=sys.stderr) |
304 | sys.exit(1) | 305 | sys.exit(1) |
305 | self._ReloadManifest(manifest_path) | 306 | self._ReloadManifest(manifest_path, load_local_manifests) |
306 | return manifest_path | 307 | return manifest_path |
307 | 308 | ||
308 | def _FetchProjectList(self, opt, projects): | 309 | def _FetchProjectList(self, opt, projects): |
@@ -565,10 +566,18 @@ later is required to fix a server side protocol bug. | |||
565 | t.join() | 566 | t.join() |
566 | pm.end() | 567 | pm.end() |
567 | 568 | ||
568 | def _ReloadManifest(self, manifest_name=None): | 569 | def _ReloadManifest(self, manifest_name=None, load_local_manifests=True): |
570 | """Reload the manfiest from the file specified by the |manifest_name|. | ||
571 | |||
572 | It unloads the manifest if |manifest_name| is None. | ||
573 | |||
574 | Args: | ||
575 | manifest_name: Manifest file to be reloaded. | ||
576 | load_local_manifests: Whether to load local manifests. | ||
577 | """ | ||
569 | if manifest_name: | 578 | if manifest_name: |
570 | # Override calls _Unload already | 579 | # Override calls _Unload already |
571 | self.manifest.Override(manifest_name) | 580 | self.manifest.Override(manifest_name, load_local_manifests=load_local_manifests) |
572 | else: | 581 | else: |
573 | self.manifest._Unload() | 582 | self.manifest._Unload() |
574 | 583 | ||
@@ -857,8 +866,9 @@ later is required to fix a server side protocol bug. | |||
857 | else: | 866 | else: |
858 | self._UpdateManifestProject(opt, mp, manifest_name) | 867 | self._UpdateManifestProject(opt, mp, manifest_name) |
859 | 868 | ||
869 | load_local_manifests = not self.manifest.HasLocalManifests | ||
860 | if self._UseSuperproject(opt): | 870 | if self._UseSuperproject(opt): |
861 | manifest_name = self._UpdateProjectsRevisionId(opt, args) | 871 | manifest_name = self._UpdateProjectsRevisionId(opt, args, load_local_manifests) |
862 | 872 | ||
863 | if self.gitc_manifest: | 873 | if self.gitc_manifest: |
864 | gitc_manifest_projects = self.GetProjects(args, | 874 | gitc_manifest_projects = self.GetProjects(args, |
@@ -926,7 +936,7 @@ later is required to fix a server side protocol bug. | |||
926 | # Iteratively fetch missing and/or nested unregistered submodules | 936 | # Iteratively fetch missing and/or nested unregistered submodules |
927 | previously_missing_set = set() | 937 | previously_missing_set = set() |
928 | while True: | 938 | while True: |
929 | self._ReloadManifest(manifest_name) | 939 | self._ReloadManifest(manifest_name, load_local_manifests) |
930 | all_projects = self.GetProjects(args, | 940 | all_projects = self.GetProjects(args, |
931 | missing_ok=True, | 941 | missing_ok=True, |
932 | submodules_ok=opt.fetch_submodules) | 942 | submodules_ok=opt.fetch_submodules) |
diff --git a/tests/test_git_superproject.py b/tests/test_git_superproject.py index 9550949b..5c1455f5 100644 --- a/tests/test_git_superproject.py +++ b/tests/test_git_superproject.py | |||
@@ -141,12 +141,12 @@ class SuperprojectTestCase(unittest.TestCase): | |||
141 | manifest_xml = fp.read() | 141 | manifest_xml = fp.read() |
142 | self.assertEqual( | 142 | self.assertEqual( |
143 | manifest_xml, | 143 | manifest_xml, |
144 | '<?xml version="1.0" ?><manifest>' + | 144 | '<?xml version="1.0" ?><manifest>' |
145 | '<remote name="default-remote" fetch="http://localhost"/>' + | 145 | '<remote name="default-remote" fetch="http://localhost"/>' |
146 | '<default remote="default-remote" revision="refs/heads/main"/>' + | 146 | '<default remote="default-remote" revision="refs/heads/main"/>' |
147 | '<project name="platform/art" path="art" revision="ABCDEF" ' + | 147 | '<project name="platform/art" path="art" revision="ABCDEF" ' |
148 | 'groups="notdefault,platform-' + self.platform + '"/>' + | 148 | 'groups="notdefault,platform-' + self.platform + '"/>' |
149 | '<superproject name="superproject"/>' + | 149 | '<superproject name="superproject"/>' |
150 | '</manifest>') | 150 | '</manifest>') |
151 | 151 | ||
152 | def test_superproject_update_project_revision_id(self): | 152 | def test_superproject_update_project_revision_id(self): |
@@ -168,13 +168,57 @@ class SuperprojectTestCase(unittest.TestCase): | |||
168 | manifest_xml = fp.read() | 168 | manifest_xml = fp.read() |
169 | self.assertEqual( | 169 | self.assertEqual( |
170 | manifest_xml, | 170 | manifest_xml, |
171 | '<?xml version="1.0" ?><manifest>' + | 171 | '<?xml version="1.0" ?><manifest>' |
172 | '<remote name="default-remote" fetch="http://localhost"/>' + | 172 | '<remote name="default-remote" fetch="http://localhost"/>' |
173 | '<default remote="default-remote" revision="refs/heads/main"/>' + | 173 | '<default remote="default-remote" revision="refs/heads/main"/>' |
174 | '<project name="platform/art" path="art" ' + | 174 | '<project name="platform/art" path="art" ' |
175 | 'revision="2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea" ' + | 175 | 'revision="2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea" ' |
176 | 'groups="notdefault,platform-' + self.platform + '"/>' + | 176 | 'groups="notdefault,platform-' + self.platform + '"/>' |
177 | '<superproject name="superproject"/>' + | 177 | '<superproject name="superproject"/>' |
178 | '</manifest>') | ||
179 | |||
180 | def test_superproject_update_project_revision_id_with_different_remotes(self): | ||
181 | """Test update of commit ids of a manifest with mutiple remotes.""" | ||
182 | manifest = self.getXmlManifest(""" | ||
183 | <manifest> | ||
184 | <remote name="default-remote" fetch="http://localhost" /> | ||
185 | <remote name="goog" fetch="http://localhost2" /> | ||
186 | <default remote="default-remote" revision="refs/heads/main" /> | ||
187 | <superproject name="superproject"/> | ||
188 | <project path="vendor/x" name="platform/vendor/x" remote="goog" groups="vendor" | ||
189 | revision="master-with-vendor" clone-depth="1" /> | ||
190 | <project path="art" name="platform/art" groups="notdefault,platform-""" + self.platform + """ | ||
191 | " /></manifest> | ||
192 | """) | ||
193 | self.maxDiff = None | ||
194 | self._superproject = git_superproject.Superproject(manifest, self.repodir) | ||
195 | self.assertEqual(len(self._superproject._manifest.projects), 2) | ||
196 | projects = self._superproject._manifest.projects | ||
197 | data = ('160000 commit 2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea\tart\x00' | ||
198 | '160000 commit e9d25da64d8d365dbba7c8ee00fe8c4473fe9a06\tbootable/recovery\x00') | ||
199 | with mock.patch.object(self._superproject, '_Init', return_value=True): | ||
200 | with mock.patch.object(self._superproject, '_Fetch', return_value=True): | ||
201 | with mock.patch.object(self._superproject, | ||
202 | '_LsTree', | ||
203 | return_value=data): | ||
204 | # Create temporary directory so that it can write the file. | ||
205 | os.mkdir(self._superproject._superproject_path) | ||
206 | manifest_path = self._superproject.UpdateProjectsRevisionId(projects) | ||
207 | self.assertIsNotNone(manifest_path) | ||
208 | with open(manifest_path, 'r') as fp: | ||
209 | manifest_xml = fp.read() | ||
210 | self.assertEqual( | ||
211 | manifest_xml, | ||
212 | '<?xml version="1.0" ?><manifest>' | ||
213 | '<remote name="default-remote" fetch="http://localhost"/>' | ||
214 | '<remote name="goog" fetch="http://localhost2"/>' | ||
215 | '<default remote="default-remote" revision="refs/heads/main"/>' | ||
216 | '<project name="platform/art" path="art" ' | ||
217 | 'revision="2c2724cb36cd5a9cec6c852c681efc3b7c6b86ea" ' | ||
218 | 'groups="notdefault,platform-' + self.platform + '"/>' | ||
219 | '<project name="platform/vendor/x" path="vendor/x" remote="goog" ' | ||
220 | 'revision="master-with-vendor" groups="vendor" clone-depth="1"/>' | ||
221 | '<superproject name="superproject"/>' | ||
178 | '</manifest>') | 222 | '</manifest>') |
179 | 223 | ||
180 | 224 | ||