From 2e29d7b7a5e65c2faf9e13812f5f80007c4db06f Mon Sep 17 00:00:00 2001 From: Jochen Sprickerhof Date: Mon, 25 Oct 2021 19:04:50 +0200 Subject: [PATCH] Don't check VCS for completed builds This speeds up the build a lot. --- fdroidserver/build.py | 60 +++++++-------------- fdroidserver/common.py | 10 ---- tests/build.TestCase | 118 +++++++++++++++++++---------------------- tests/scanner.TestCase | 50 ++++++++--------- 4 files changed, 99 insertions(+), 139 deletions(-) diff --git a/fdroidserver/build.py b/fdroidserver/build.py index a1fc2ff3..906e165d 100644 --- a/fdroidserver/build.py +++ b/fdroidserver/build.py @@ -54,7 +54,7 @@ ssh_channel = None # Note that 'force' here also implies test mode. -def build_server(app, build, vcs, build_dir, output_dir, log_dir, force): +def build_server(app, build, output_dir, log_dir, force, refresh): """Do a build on the builder vm. Parameters @@ -63,10 +63,6 @@ def build_server(app, build, vcs, build_dir, output_dir, log_dir, force): The metadata of the app to build. build The build of the app to build. - vcs - The version control system controller object of the app. - build_dir - The local source-code checkout directory of the app. output_dir The target folder for the build result. log_dir @@ -210,10 +206,6 @@ def build_server(app, build, vcs, build_dir, output_dir, log_dir, force): srclibpaths.append( common.getsrclib(lib, 'build/srclib', basepath=True, prepare=False)) - # If one was used for the main source, add that too. - basesrclib = vcs.getsrclib() - if basesrclib: - srclibpaths.append(basesrclib) for name, number, lib in srclibpaths: logging.info("Sending srclib '%s'" % lib) ftp.chdir(posixpath.join(homedir, 'build', 'srclib')) @@ -230,9 +222,19 @@ def build_server(app, build, vcs, build_dir, output_dir, log_dir, force): else: raise BuildException(_('cannot find required srclibs: "{path}"') .format(path=srclibsfile)) + + vcs, build_dir = common.setup_vcs(app) + # When using server mode, still keep a local cache of the repo, by + # grabbing the source now. + vcs.gotorevision(build.commit, refresh) + + # Initialise submodules if required + if build.submodules: + vcs.initsubmodules() + # Copy the main app source code # (no need if it's a srclib) - if (not basesrclib) and os.path.exists(build_dir): + if os.path.exists(build_dir): ftp.chdir(posixpath.join(homedir, 'build')) fv = '.fdroidvcs-' + app.id ftp.put(os.path.join('build', fv), fv) @@ -411,7 +413,7 @@ def get_metadata_from_apk(app, build, apkfile): return versionCode, versionName -def build_local(app, build, vcs, build_dir, output_dir, log_dir, srclib_dir, extlib_dir, tmp_dir, force, onserver, refresh): +def build_local(app, build, output_dir, log_dir, srclib_dir, extlib_dir, tmp_dir, force, onserver, refresh): """Do a build locally. Parameters @@ -420,10 +422,6 @@ def build_local(app, build, vcs, build_dir, output_dir, log_dir, srclib_dir, ext The metadata of the app to build. build The build of the app to build. - vcs - The version control system controller object of the app. - build_dir - The local source-code checkout directory of the app. output_dir The target folder for the build result. log_dir @@ -513,6 +511,7 @@ def build_local(app, build, vcs, build_dir, output_dir, log_dir, srclib_dir, ext % (app.id, build.versionName, build.sudo)) # Prepare the source code... + vcs, build_dir = common.setup_vcs(app) root_dir, srclibpaths = common.prepare_source(vcs, app, build, build_dir, srclib_dir, extlib_dir, onserver, refresh) @@ -858,8 +857,8 @@ def build_local(app, build, vcs, build_dir, output_dir, log_dir, srclib_dir, ext os.path.join(output_dir, tarname)) -def trybuild(app, build, build_dir, output_dir, log_dir, also_check_dir, - srclib_dir, extlib_dir, tmp_dir, repo_dir, vcs, test, +def trybuild(app, build, output_dir, log_dir, also_check_dir, + srclib_dir, extlib_dir, tmp_dir, repo_dir, test, server, force, onserver, refresh): """Build a particular version of an application, if it needs building. @@ -929,17 +928,9 @@ def trybuild(app, build, build_dir, output_dir, log_dir, also_check_dir, build.versionName, build.versionCode, app.id)) if server: - # When using server mode, still keep a local cache of the repo, by - # grabbing the source now. - vcs.gotorevision(build.commit, refresh) - - # Initialise submodules if required - if build.submodules: - vcs.initsubmodules() - - build_server(app, build, vcs, build_dir, output_dir, log_dir, force) + build_server(app, build, output_dir, log_dir, force, refresh) else: - build_local(app, build, vcs, build_dir, output_dir, log_dir, srclib_dir, extlib_dir, tmp_dir, force, onserver, refresh) + build_local(app, build, output_dir, log_dir, srclib_dir, extlib_dir, tmp_dir, force, onserver, refresh) return True @@ -1171,9 +1162,6 @@ def main(): endtime = time.time() + 72 * 60 * 60 max_build_time_reached = False for appid, app in apps.items(): - - first = True - for build in app.get('Builds', []): if time.time() > endtime: max_build_time_reached = True @@ -1194,18 +1182,10 @@ def main(): tools_version_log = '' try: - - # For the first build of a particular app, we need to set up - # the source repo. We can reuse it on subsequent builds, if - # there are any. - if first: - vcs, build_dir = common.setup_vcs(app) - first = False - logging.debug("Checking %s:%s" % (appid, build.versionCode)) - if trybuild(app, build, build_dir, output_dir, log_dir, + if trybuild(app, build, output_dir, log_dir, also_check_dir, srclib_dir, extlib_dir, - tmp_dir, repo_dir, vcs, options.test, + tmp_dir, repo_dir, options.test, options.server, options.force, options.onserver, options.refresh): toolslog = os.path.join(log_dir, diff --git a/fdroidserver/common.py b/fdroidserver/common.py index 5a546386..c5aa6104 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -1190,7 +1190,6 @@ class vcs: self.local = local self.clone_failed = False self.refreshed = False - self.srclib = None def _gettags(self): raise NotImplementedError @@ -1299,10 +1298,6 @@ class vcs: """Get current commit reference (hash, revision, etc).""" raise VCSException('getref not supported for this vcs type') - def getsrclib(self): - """Return the srclib (name, path) used in setting up the current revision, or None.""" - return self.srclib - class vcs_git(vcs): @@ -2337,11 +2332,6 @@ def prepare_source(vcs, app, build, build_dir, srclib_dir, extlib_dir, onserver= for name, number, libpath in srclibpaths: place_srclib(root_dir, int(number) if number else None, libpath) - basesrclib = vcs.getsrclib() - # If one was used for the main source, add that too. - if basesrclib: - srclibpaths.append(basesrclib) - # Update the local.properties file localprops = [os.path.join(build_dir, 'local.properties')] if build.subdir: diff --git a/tests/build.TestCase b/tests/build.TestCase index f6607634..cbdd235a 100755 --- a/tests/build.TestCase +++ b/tests/build.TestCase @@ -220,6 +220,7 @@ class BuildTest(unittest.TestCase): @mock.patch('fdroidserver.build.FDroidPopen') @mock.patch('fdroidserver.common.is_debuggable_or_testOnly', lambda f: False) @mock.patch('fdroidserver.common.get_native_code', lambda f: 'x86') + @mock.patch('fdroidserver.common.setup_vcs', lambda f: (mock.Mock(), ".")) def test_build_local_maven(self, fake_FDroidPopen, fake_get_apk_id): """Test build_local() with a maven project""" @@ -255,14 +256,11 @@ class BuildTest(unittest.TestCase): build.versionCode, build.versionName, ) - vcs = mock.Mock() build.maven = 'yes@..' fdroidserver.build.build_local( app, build, - vcs, - build_dir=self.testdir, output_dir=self.testdir, log_dir=os.getcwd(), srclib_dir=None, @@ -277,8 +275,6 @@ class BuildTest(unittest.TestCase): fdroidserver.build.build_local( app, build, - vcs, - build_dir=self.testdir, output_dir=self.testdir, log_dir=os.getcwd(), srclib_dir=None, @@ -313,7 +309,6 @@ class BuildTest(unittest.TestCase): build.ndk = 'r21e' # aka 21.4.7075529 ndk_version = '21.4.7075529' ndk_dir = Path(config['sdk_path']) / 'ndk' / ndk_version - vcs = mock.Mock() def make_fake_apk(output, build): with open(build.output, 'w') as fp: @@ -345,6 +340,8 @@ class BuildTest(unittest.TestCase): 'fdroidserver.build.FDroidPopen', FakeProcess ) as _ignored, mock.patch( 'sdkmanager.install', wraps=fake_sdkmanager_install + ) as _ignored, mock.patch( + 'fdroidserver.common.setup_vcs', return_value=(mock.Mock(), testdir) ) as _ignored: _ignored # silence the linters with self.assertRaises( @@ -354,8 +351,6 @@ class BuildTest(unittest.TestCase): fdroidserver.build.build_local( app, build, - vcs, - build_dir=testdir, output_dir=testdir, log_dir=None, srclib_dir=None, @@ -373,8 +368,6 @@ class BuildTest(unittest.TestCase): fdroidserver.build.build_local( app, build, - vcs, - build_dir=testdir, output_dir=testdir, log_dir=os.getcwd(), srclib_dir=None, @@ -397,6 +390,7 @@ class BuildTest(unittest.TestCase): 'fdroidserver.common.sha256sum', lambda f: 'ad7ce5467e18d40050dc51b8e7affc3e635c85bd8c59be62de32352328ed467e', ) + @mock.patch('fdroidserver.common.setup_vcs', lambda f: (mock.Mock(), ".")) def test_build_local_ndk_some_installed(self): """Test if `fdroid build` detects installed NDKs and auto-installs when missing""" with tempfile.TemporaryDirectory() as testdir, TmpCwd( @@ -424,7 +418,6 @@ class BuildTest(unittest.TestCase): build.ndk = 'r21e' # aka 21.4.7075529 ndk_version = '21.4.7075529' ndk_dir = Path(config['sdk_path']) / 'ndk' / ndk_version - vcs = mock.Mock() def make_fake_apk(output, build): with open(build.output, 'w') as fp: @@ -454,8 +447,6 @@ class BuildTest(unittest.TestCase): fdroidserver.build.build_local( app, build, - vcs, - build_dir=testdir, output_dir=testdir, log_dir=os.getcwd(), srclib_dir=None, @@ -489,7 +480,6 @@ class BuildTest(unittest.TestCase): build.scanignore = ['foo.aar'] build.versionCode = 1 build.versionName = '1.0' - vcs = mock.Mock() os.mkdir('reports') os.mkdir('target') @@ -522,30 +512,31 @@ class BuildTest(unittest.TestCase): fp.write('APK PLACEHOLDER') return output - with mock.patch('fdroidserver.common.replace_build_vars', wraps=make_fake_apk): - with mock.patch('fdroidserver.common.get_native_code', return_value='x86'): - with mock.patch( - 'fdroidserver.common.get_apk_id', - return_value=(app.id, build.versionCode, build.versionName), - ): - with mock.patch( - 'fdroidserver.common.is_debuggable_or_testOnly', - return_value=False, - ): - fdroidserver.build.build_local( - app, - build, - vcs, - build_dir=self.testdir, - output_dir=self.testdir, - log_dir=None, - srclib_dir=None, - extlib_dir=None, - tmp_dir=None, - force=False, - onserver=False, - refresh=False, - ) + with mock.patch( + 'fdroidserver.common.replace_build_vars', wraps=make_fake_apk + ) as _ignored, mock.patch( + 'fdroidserver.common.get_native_code', return_value='x86' + ) as _ignored, mock.patch( + 'fdroidserver.common.get_apk_id', + return_value=(app.id, build.versionCode, build.versionName), + ) as _ignored, mock.patch( + 'fdroidserver.common.is_debuggable_or_testOnly', return_value=False + ) as _ignored, mock.patch( + 'fdroidserver.common.setup_vcs', return_value=(mock.Mock(), self.testdir) + ) as _ignored: + _ignored # silence the linters + fdroidserver.build.build_local( + app, + build, + output_dir=self.testdir, + log_dir=None, + srclib_dir=None, + extlib_dir=None, + tmp_dir=None, + force=False, + onserver=False, + refresh=False, + ) self.assertTrue(os.path.exists('foo.aar')) self.assertTrue(os.path.isdir('build')) @@ -880,8 +871,6 @@ class BuildTest(unittest.TestCase): } fdroidserver.common.config = {'sdk_path': '/fake/android/sdk/path'} fdroidserver.build.options = mock.MagicMock() - vcs = mock.Mock() - vcs.getsrclib = mock.Mock(return_value=None) app = fdroidserver.metadata.App() app['metadatapath'] = 'metadata/fake.id.yml' app['id'] = 'fake.id' @@ -896,29 +885,33 @@ class BuildTest(unittest.TestCase): ) app['Builds'] = [build] - test_flag = ('--on-server', True) - fdroidserver.build.build_server(app, build, vcs, '', '', '', False) - self.assertTrue(fdroidserver_vmtools_get_build_vm.called) + with mock.patch( + 'fdroidserver.common.setup_vcs', return_value=(mock.Mock(), "") + ) as _ignored: + _ignored # silence the linters + test_flag = ('--on-server', True) + fdroidserver.build.build_server(app, build, '', '', False, False) + self.assertTrue(fdroidserver_vmtools_get_build_vm.called) - for force in (True, False): - test_flag = ('--force', force) - fdroidserver.build.build_server(app, build, vcs, '', '', '', force) + for force in (True, False): + test_flag = ('--force', force) + fdroidserver.build.build_server(app, build, '', '', force, False) - fdroidserver.build.options.notarball = True - test_flag = ('--no-tarball', True) - fdroidserver.build.build_server(app, build, vcs, '', '', '', False) - fdroidserver.build.options.notarball = False - test_flag = ('--no-tarball', False) - fdroidserver.build.build_server(app, build, vcs, '', '', '', False) + fdroidserver.build.options.notarball = True + test_flag = ('--no-tarball', True) + fdroidserver.build.build_server(app, build, '', '', False, False) + fdroidserver.build.options.notarball = False + test_flag = ('--no-tarball', False) + fdroidserver.build.build_server(app, build, '', '', False, False) - fdroidserver.build.options.skipscan = False - test_flag = ('--scan-binary', True) - fdroidserver.build.build_server(app, build, vcs, '', '', '', False) - fdroidserver.build.options.skipscan = True - test_flag = ('--scan-binary', False) - fdroidserver.build.build_server(app, build, vcs, '', '', '', False) - test_flag = ('--skip-scan', True) - fdroidserver.build.build_server(app, build, vcs, '', '', '', False) + fdroidserver.build.options.skipscan = False + test_flag = ('--scan-binary', True) + fdroidserver.build.build_server(app, build, '', '', False, False) + fdroidserver.build.options.skipscan = True + test_flag = ('--scan-binary', False) + fdroidserver.build.build_server(app, build, '', '', False, False) + test_flag = ('--skip-scan', True) + fdroidserver.build.build_server(app, build, '', '', False, False) @mock.patch('fdroidserver.vmtools.get_build_vm') @mock.patch('fdroidserver.vmtools.get_clean_builder') @@ -929,6 +922,7 @@ class BuildTest(unittest.TestCase): @mock.patch('fdroidserver.build.build_local') @mock.patch('fdroidserver.common.get_android_tools_version_log', lambda: 'versions') @mock.patch('fdroidserver.common.deploy_build_log_with_rsync', lambda a, b, c: None) + @mock.patch('fdroidserver.common.setup_vcs', lambda f: (mock.Mock(), ".")) def test_build_server_no_local_prepare( self, build_build_local, @@ -1004,8 +998,6 @@ class BuildTest(unittest.TestCase): fdroidserver.build.options = options fdroidserver.build.config = {'sdk_path': '/fake/android/sdk/path'} - vcs = mock.Mock() - vcs.getsrclib = mock.Mock(return_value=None) app = fdroidserver.metadata.App() app['metadatapath'] = 'metadata/fake.id.yml' app['id'] = 'fake.id' @@ -1032,7 +1024,6 @@ class BuildTest(unittest.TestCase): fdroidserver.build.trybuild( app, build, - build_dir, 'unsigned', 'logs', None, @@ -1040,7 +1031,6 @@ class BuildTest(unittest.TestCase): extlib_dir, 'tmp', 'repo', - vcs, options.test, options.server, options.force, diff --git a/tests/scanner.TestCase b/tests/scanner.TestCase index 6a85b7f3..dcd009c7 100755 --- a/tests/scanner.TestCase +++ b/tests/scanner.TestCase @@ -245,7 +245,6 @@ class ScannerTest(unittest.TestCase): build.scanignore = ['baz.so', 'foo.aar'] build.versionCode = 1 build.versionName = '1.0' - vcs = mock.Mock() for f in ('baz.so', 'foo.aar', 'gradle-wrapper.jar'): with open(f, 'w') as fp: @@ -268,30 +267,31 @@ class ScannerTest(unittest.TestCase): fp.write('APK PLACEHOLDER') return output - with mock.patch('fdroidserver.common.replace_build_vars', wraps=make_fake_apk): - with mock.patch('fdroidserver.common.get_native_code', return_value='x86'): - with mock.patch( - 'fdroidserver.common.get_apk_id', - return_value=(app.id, build.versionCode, build.versionName), - ): - with mock.patch( - 'fdroidserver.common.is_debuggable_or_testOnly', - return_value=False, - ): - fdroidserver.build.build_local( - app, - build, - vcs, - build_dir=self.testdir, - output_dir=self.testdir, - log_dir=None, - srclib_dir=None, - extlib_dir=None, - tmp_dir=None, - force=False, - onserver=False, - refresh=False, - ) + with mock.patch( + 'fdroidserver.common.replace_build_vars', wraps=make_fake_apk + ) as _ignored, mock.patch( + 'fdroidserver.common.get_native_code', return_value='x86' + ) as _ignored, mock.patch( + 'fdroidserver.common.get_apk_id', + return_value=(app.id, build.versionCode, build.versionName), + ) as _ignored, mock.patch( + 'fdroidserver.common.is_debuggable_or_testOnly', return_value=False + ) as _ignored, mock.patch( + 'fdroidserver.common.setup_vcs', return_value=(mock.Mock(), self.testdir) + ) as _ignored: + _ignored # silence the linters + fdroidserver.build.build_local( + app, + build, + output_dir=self.testdir, + log_dir=None, + srclib_dir=None, + extlib_dir=None, + tmp_dir=None, + force=False, + onserver=False, + refresh=False, + ) self.assertTrue(os.path.exists('baz.so')) self.assertTrue(os.path.exists('foo.aar')) self.assertFalse(os.path.exists('gradle-wrapper.jar'))