Better git usage in donate-cpu.py to reduce bandwidth and disk usage (#3584)

* Better git usage in donate-cpu.py to reduce bandwidth and disk usage

Main changes:
* Bump client version
* Move try+retry logic to function try_retry to reduce duplication
* Use exponential backoff for try_retry
* git clone with --depth=1 to reduce bandwidth and disk use
* Use multiple worktree to work with multiple versions, instead of back-and-forth checkouts

* donate-cpu.py fixes for review comments and automated check failures

* Move compile_cppcheck within (if ver == 'main) branch to avoid duplicate compile_cppcheck+compile_version cals
* Use classic format syntax for python 3.5 compatibility
* Fix undefined CalledProcessError detected by pylint

* donate-cpu.py code changes following code review

* Migration existing "cppcheck" directory if available instead of "git clone"
* Logging message tweaks
* Use subprocess' cwd parameter instead of os.chdir() to avoid risk around changing and not restoring the working directory

* Update tools/test-my-pr.py to account for donate_cpu_lib changes

* donate-cpu.py: ensure correct workspace locations with relative --work-path

tools/donate-cpu.py

@@ -47,7 +47,7 @@ for arg in sys.argv[1:]:
         package_url = arg[arg.find('=')+1:]
         print('Package:' + package_url)
     elif arg.startswith('--work-path='):
-        work_path = arg[arg.find('=')+1:]
+        work_path = os.path.abspath(arg[arg.find('=')+1:])
         print('work_path:' + work_path)
         if not os.path.exists(work_path):
             print('work path does not exist!')
@@ -116,8 +116,19 @@ if max_packages:
     print('Maximum number of packages to download and analyze: {}'.format(max_packages))
 if not os.path.exists(work_path):
     os.mkdir(work_path)
-cppcheck_path = os.path.join(work_path, 'cppcheck')
+repo_path = os.path.join(work_path, 'repo')
+# This is a temporary migration step which should be removed in the future
+migrate_repo_path = os.path.join(work_path, 'cppcheck')
+
 packages_processed = 0
+
+print('Get Cppcheck..')
+try:
+    try_retry(clone_cppcheck, fargs=(repo_path, migrate_repo_path))
+except:
+    print('Error: Failed to clone Cppcheck, retry later')
+    sys.exit(1)
+
 while True:
     if max_packages:
         if packages_processed >= max_packages:
@@ -130,9 +141,6 @@ while True:
         if stop_time < time.strftime('%H:%M'):
             print('Stopping. Thank you!')
             sys.exit(0)
-    if not get_cppcheck(cppcheck_path, work_path):
-        print('Failed to clone Cppcheck, retry later')
-        sys.exit(1)
     cppcheck_versions = get_cppcheck_versions(server_address)
     if cppcheck_versions is None:
         print('Failed to communicate with server, retry later')
@@ -142,12 +150,25 @@ while True:
         sys.exit(1)
     for ver in cppcheck_versions:
         if ver == 'head':
-            if not compile_cppcheck(cppcheck_path, jobs):
+            ver = 'main'
-                print('Failed to compile Cppcheck, retry later')
+        current_cppcheck_dir = os.path.join(work_path, 'tree-'+ver)
-                sys.exit(1)
+        try:
-        elif not compile_version(work_path, jobs, ver):
+            print('Fetching Cppcheck-{}..'.format(ver))
-            print('Failed to compile Cppcheck-{}, retry later'.format(ver))
+            try_retry(checkout_cppcheck_version, fargs=(repo_path, ver, current_cppcheck_dir))
+        except KeyboardInterrupt as e:
+            # Passthrough for user abort
+            raise e
+        except:
+            print('Failed to update Cppcheck, retry later')
             sys.exit(1)
+        if ver == 'main':
+            if not compile_cppcheck(current_cppcheck_dir, jobs):
+                print('Failed to compile Cppcheck-{}, retry later'.format(ver))
+                sys.exit(1)
+        else:
+            if not compile_version(current_cppcheck_dir, jobs):
+                print('Failed to compile Cppcheck-{}, retry later'.format(ver))
+                sys.exit(1)
     if package_url:
         package = package_url
     else:
@@ -172,12 +193,11 @@ while True:
     libraries = get_libraries()
 
     for ver in cppcheck_versions:
+        tree_path = os.path.join(work_path, 'tree-'+ver)
         if ver == 'head':
-            current_cppcheck_dir = 'cppcheck'
+            tree_path = os.path.join(work_path, 'tree-main')
-            cppcheck_head_info = get_cppcheck_info(work_path + '/cppcheck')
+            cppcheck_head_info = get_cppcheck_info(tree_path)
-        else:
+        c, errout, info, t, cppcheck_options, timing_info = scan_package(work_path, tree_path, jobs, libraries)
-            current_cppcheck_dir = ver
-        c, errout, info, t, cppcheck_options, timing_info = scan_package(work_path, current_cppcheck_dir, jobs, libraries)
         if c < 0:
             if c == -101 and 'error: could not find or open any of the paths given.' in errout:
                 # No sourcefile found (for example only headers present)

tools/donate_cpu_lib.py

@@ -15,11 +15,13 @@ import shlex
 # Version scheme (MAJOR.MINOR.PATCH) should orientate on "Semantic Versioning" https://semver.org/
 # Every change in this script should result in increasing the version number accordingly (exceptions may be cosmetic
 # changes)
-CLIENT_VERSION = "1.3.16"
+CLIENT_VERSION = "1.3.17"
 
 # Timeout for analysis with Cppcheck in seconds
 CPPCHECK_TIMEOUT = 30 * 60
 
+CPPCHECK_REPO_URL = "https://github.com/danmar/cppcheck.git"
+
 # Return code that is used to mark a timed out analysis
 RETURN_CODE_TIMEOUT = -999
 
@@ -35,42 +37,59 @@ def check_requirements():
     return result
 
 
-def get_cppcheck(cppcheck_path, work_path):
+# Try and retry with exponential backoff if an exception is raised
-    print('Get Cppcheck..')
+def try_retry(fun, fargs=(), max_tries=5):
-    for i in range(5):
+    sleep_duration = 5.0
-        if os.path.exists(cppcheck_path):
+    for i in range(max_tries):
-            try:
-                os.chdir(cppcheck_path)
-                try:
-                    subprocess.check_call(['git', 'checkout', '-f', 'main'])
-                except subprocess.CalledProcessError:
-                    subprocess.check_call(['git', 'checkout', '-f', 'master'])
-                    subprocess.check_call(['git', 'pull'])
-                    subprocess.check_call(['git', 'checkout', 'origin/main', '-b', 'main'])
-                subprocess.check_call(['git', 'pull'])
-            except:
-                print('Failed to update Cppcheck sources! Retrying..')
-                time.sleep(10)
-                continue
-        else:
-            try:
-                subprocess.check_call(['git', 'clone', 'https://github.com/danmar/cppcheck.git', cppcheck_path])
-            except:
-                print('Failed to clone, will try again in 10 minutes..')
-                time.sleep(600)
-                continue
-        time.sleep(2)
-        return True
-    if os.path.exists(cppcheck_path):
-        print('Failed to update Cppcheck sources, trying a fresh clone..')
         try:
-            os.chdir(work_path)
+            return fun(*fargs)
-            shutil.rmtree(cppcheck_path)
+        except KeyboardInterrupt as e:
-            get_cppcheck(cppcheck_path, work_path)
+            # Do not retry in case of user abort
-        except:
+            raise e
-            print('Failed to remove Cppcheck folder, please manually remove ' + work_path)
+        except BaseException as e:
-            return False
+            if i < max_tries - 1:
-    return False
+                print("{} in {}: {}".format(type(e).__name__, fun.__name__, str(e)))
+                print("Trying {} again in {} seconds".format(fun.__name__, sleep_duration))
+                time.sleep(sleep_duration)
+                sleep_duration *= 2.0
+            else:
+                print("Maximum number of tries reached for {}".format(fun.__name__))
+                raise e
+
+
+def clone_cppcheck(repo_path, migrate_from_path):
+    repo_git_dir = os.path.join(repo_path, '.git')
+    if os.path.exists(repo_git_dir):
+        return
+    # Attempt to migrate clone directory used prior to 1.3.17
+    if os.path.exists(migrate_from_path):
+        os.rename(migrate_from_path, repo_path)
+    else:
+        # A shallow git clone (depth = 1) is enough for building and scanning.
+        # Do not checkout until fetch_cppcheck_version.
+        subprocess.check_call(['git', 'clone', '--depth=1', '--no-checkout', CPPCHECK_REPO_URL, repo_path])
+        # Checkout an empty branch to allow "git worktree add" for main later on
+    try:
+        # git >= 2.27
+        subprocess.check_call(['git', 'switch', '--orphan', 'empty'], cwd=repo_path)
+    except subprocess.CalledProcessError:
+        subprocess.check_call(['git', 'checkout','--orphan', 'empty'], cwd=repo_path)
+
+
+def checkout_cppcheck_version(repo_path, version, cppcheck_path):
+    if not os.path.isabs(cppcheck_path):
+        raise ValueError("cppcheck_path is not an absolute path")
+    if os.path.exists(cppcheck_path):
+        subprocess.check_call(['git', 'checkout' , '-f', version], cwd=cppcheck_path)
+        # It is possible to pull branches, not tags
+        if version == 'main':
+            subprocess.check_call(['git', 'pull'], cwd=cppcheck_path)
+    else:
+        if version != 'main':
+            # Since this is a shallow clone, explicitly fetch the remote version tag
+            refspec = 'refs/tags/' + version + ':ref/tags/' + version
+            subprocess.check_call(['git', 'fetch', '--depth=1', 'origin', refspec], cwd=repo_path)
+        subprocess.check_call(['git', 'worktree', 'add', cppcheck_path,  version], cwd=repo_path)
 
 
 def get_cppcheck_info(cppcheck_path):
@@ -81,28 +100,18 @@ def get_cppcheck_info(cppcheck_path):
         return ''
 
 
-def compile_version(work_path, jobs, version):
+def compile_version(cppcheck_path, jobs):
-    if os.path.isfile(work_path + '/' + version + '/cppcheck'):
+    if os.path.isfile(os.path.join(cppcheck_path, 'cppcheck')):
         return True
-    os.chdir(work_path + '/cppcheck')
+    # Build
-    subprocess.call(['git', 'checkout', version])
+    ret = compile_cppcheck(cppcheck_path, jobs)
-    subprocess.call(['make', 'clean'])
+    # Clean intermediate build files
-    subprocess.call(['make', jobs, 'MATCHCOMPILER=yes', 'CXXFLAGS=-O2 -g'])
+    subprocess.call(['git', 'clean', '-f', '-d', '-x', '--exclude', 'cppcheck'], cwd=cppcheck_path)
-    if os.path.isfile(work_path + '/cppcheck/cppcheck'):
+    return ret
-        os.mkdir(work_path + '/' + version)
-        dest_path = work_path + '/' + version + '/'
-        subprocess.call(['cp', '-R', work_path + '/cppcheck/cfg', dest_path])
-        subprocess.call(['cp', 'cppcheck', dest_path])
-    subprocess.call(['git', 'checkout', 'main'])
-    try:
-        subprocess.call([work_path + '/' + version + '/cppcheck', '--version'])
-    except OSError:
-        return False
-    return True
 
 
 def compile_cppcheck(cppcheck_path, jobs):
-    print('Compiling Cppcheck..')
+    print('Compiling {}'.format(os.path.basename(cppcheck_path)))
     try:
         os.chdir(cppcheck_path)
         subprocess.call(['make', jobs, 'MATCHCOMPILER=yes', 'CXXFLAGS=-O2 -g'])

tools/test-my-pr.py

@@ -33,7 +33,9 @@ if __name__ == "__main__":
     work_path = os.path.abspath(args.work_path)
     if not os.path.exists(work_path):
         os.makedirs(work_path)
-    main_dir = os.path.join(work_path, 'cppcheck')
+    repo_dir = os.path.join(work_path, 'repo')
+    old_repo_dir = os.path.join(work_path, 'cppcheck')
+    main_dir = os.path.join(work_path, 'tree-main')
 
     jobs = '-j' + str(args.j)
     result_file = os.path.join(work_path, args.o)
@@ -46,8 +48,18 @@ if __name__ == "__main__":
     if os.path.exists(timing_file):
         os.remove(timing_file)
 
-    if not lib.get_cppcheck(main_dir, work_path):
+    try:
-        print('Failed to clone main of Cppcheck, retry later')
+        lib.clone_cppcheck(repo_dir, old_repo_dir)
+        pass
+    except:
+        print('Failed to clone Cppcheck repository, retry later')
+        sys.exit(1)
+
+    try:
+        lib.checkout_cppcheck_version(repo_dir, 'main', main_dir)
+        pass
+    except:
+        print('Failed to checkout main, retry later')
         sys.exit(1)
 
     try:
@@ -62,8 +74,10 @@ if __name__ == "__main__":
                 'Package', 'main', 'your', 'Factor', package_width=package_width, timing_width=timing_width))
 
         os.chdir(main_dir)
+        subprocess.check_call(['git', 'fetch', '--depth=1', 'origin', commit_id])
         subprocess.check_call(['git', 'checkout', '-f', commit_id])
-    except:
+    except BaseException as e:
+        print('Error: {}'.format(e))
         print('Failed to switch to common ancestor of your branch and main')
         sys.exit(1)