Merge branch 'build_no_vcs' into 'master'

Don't check VCS for completed builds

Closes #920

See merge request fdroid/fdroidserver!1036

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,
+def trybuild(app, build, output_dir, log_dir, also_check_dir,
-             srclib_dir, extlib_dir, tmp_dir, repo_dir, vcs, test,
+             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
+        build_server(app, build, output_dir, log_dir, force, refresh)
-        # 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)
     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,

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:

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,21 +512,22 @@ 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.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(
-                    with mock.patch(
+            'fdroidserver.common.is_debuggable_or_testOnly', return_value=False
-                        'fdroidserver.common.is_debuggable_or_testOnly',
+        ) as _ignored, mock.patch(
-                        return_value=False,
+            'fdroidserver.common.setup_vcs', return_value=(mock.Mock(), self.testdir)
-                    ):
+        ) as _ignored:
+            _ignored  # silence the linters
             fdroidserver.build.build_local(
                 app,
                 build,
-                            vcs,
-                            build_dir=self.testdir,
                 output_dir=self.testdir,
                 log_dir=None,
                 srclib_dir=None,
@@ -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]
 
+        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, vcs, '', '', '', False)
+            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)
+                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.build_server(app, build, '', '', False, False)
             fdroidserver.build.options.notarball = False
             test_flag = ('--no-tarball', False)
-        fdroidserver.build.build_server(app, build, vcs, '', '', '', 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.build_server(app, build, '', '', False, False)
             fdroidserver.build.options.skipscan = True
             test_flag = ('--scan-binary', False)
-        fdroidserver.build.build_server(app, build, vcs, '', '', '', False)
+            fdroidserver.build.build_server(app, build, '', '', False, False)
             test_flag = ('--skip-scan', True)
-        fdroidserver.build.build_server(app, build, vcs, '', '', '', False)
+            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,

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,21 +267,22 @@ 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.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(
-                    with mock.patch(
+            'fdroidserver.common.is_debuggable_or_testOnly', return_value=False
-                        'fdroidserver.common.is_debuggable_or_testOnly',
+        ) as _ignored, mock.patch(
-                        return_value=False,
+            'fdroidserver.common.setup_vcs', return_value=(mock.Mock(), self.testdir)
-                    ):
+        ) as _ignored:
+            _ignored  # silence the linters
             fdroidserver.build.build_local(
                 app,
                 build,
-                            vcs,
-                            build_dir=self.testdir,
                 output_dir=self.testdir,
                 log_dir=None,
                 srclib_dir=None,