Migrate over to using pathlib and subprocess.run() (#198) r=truber

* Refactor code involving resource import to not rely on os.name

* Replace various os/os.path usages with pathlib2/pathlib.

* Ignore pylint "no-member" false positive error for now.

* Remove getAbsPathForAdjacentFile and normExpUserPath.

* Fix #23 - Remove captureStdout and use subprocess32/subprocess from PyPI/stdlib throughout.

* Remove unused imports.

* subprocess-related follow-up fix.

* pathlib-related follow-up fix.

* Ignore more pylint "no-member" false positives.

* Add comment related to is_win_dumping_to_default raising WindowsError in some circumstances.

* In Python 3, winreg functions usually throw OSError instead of WindowsError.

* Oops, WindowsError is a subclass of OSError.

* Revert "Ignore more pylint "no-member" false positives."

This reverts commit 7bf932f.

* Revert "Ignore pylint "no-member" false positive error for now."

This reverts commit 42865b6.

* Remove last no-member ignore line false-positive.

* Continue to ignore no-member pylint errors until newer linter libraries get released.

* Inline scanLine function, then move last detect_malloc_errors function "amiss" into util/file_manipulation.py

* Replace logPrefix file-renaming and suffix-changing with the proper pathlib way of doing so.

* Deal with .fuzzmanagerconf files using pathlib.

* Convert Paths to make them Lithium-friendly.

* Check file sizes in truncateFile using the pathlib way.

* More pathlib fixes.

* We should assert the existence of the concatenated jsfunfuzz file a little later.

* Rename the autoBisectLogFilename and autoBisectLog variables.

* Make handle_rm_readonly Windows-only for simplification since we only use it on Windows.

* Remove superfluous __name__/__main__ line in bot.py

* Python 3 only allows binary mode when using open() with buffering=0 set.

* Use the parent/stem/suffix model of pathlib in more places.

* Make runthis retain its Path components and only convert to string when calling Lithium functions.

* Make quote usage first convert items to strings.

* Replace the filenameToReduce variable name with reduced_log.

* Force shutil usages to operate on strings.

* Rename _ to branch.

* Follow-up fix.

* Whitespace fixes.

* Fix/remove invalid comments as indicated during review.

* Remove more comments.

* Stop using range and use enumerate instead as suggested in review.

* More pathlib fixes.

* Remove bad line.

* Fix pathlib issue related to get_shell_cache_js_bin_path.

* Yet more pathlib fixes.

* Comment and whitespace changes.

* Refactor getCfgCmdExclEnv to get_cfg_cmd_excl_env and setCfgCmdExclEnv to set_cfg_cmd_excl_env.

* self.cfg should be instantiated as a list in compile_shell.

* Add a docstring for CompiledShell class.

* Inline updateRepo and remove the function.

* fork_join tests should become actual tests for pytest, commenting them out for now.

* Make a tempfile import top-level and use the backports.tempfile version from PyPI for Python 2.

* Remove unneeded 'if __name__ == "__main__":' lines.

* Remove unneeded imports from loop.py

* Consolidate subprocess32 import with others in compare_jit.py

* Rename printMachineInfo to print_machine_info and refactor it a little.

* Defend against logPrefix not being a Path, e.g. when Lithium uses it, it is likely a string.

* Rename tempPrefix to cwd_prefix.

* Make cwd_prefix explicit in using Path.cwd() when it is used.

* Convert remaining subprocess calls to use subprocess.run() instead.

* Refactor getEnvAdded to get_env_added and setEnvAdded to set_env_added.

* Fix potential issue in get_shell_compiled_runlibs_path.

* We must call Path on a path, not potentially a symlink.

* Add tests for autoconf_run and ensure_cache_dir.

* More documentation for functions in the CompiledShell class.

* Add more documentation for compare_jit function in compare_jit.

* Fix missing imports.

* Remove print_function from files that do not have any print functions.

* Add unicode_literals to the future import line of all files.

* Refactor ignoreSomeOfStderr to ignore_some_stderr and fix pylint issues.

* Re-add some pylint ignores for the compare_jit function.

* Refactor getRepoName to get_repo_name.

* Refactor getS3TarballWithExt to get_s3_tar_name_with_ext.

* Remove obsolete pylint ignore line.

* Refactor getEnvFull to get_env_full and setEnvFull to set_env_full.

* getMajorVersion and setMajorVersion seem unnecessary since they are just used/fixated off set/getVersion and both are set at the same time.

* Refactor getVersion to get_version and setVersion to set_version.

* Fix str/unicode confusion in repos_update.

* WIP: Add some FIXME comments, they really should be fixed.

* Fix pylint errors in test_autoconf_run.

* Fix the test_ensure_cache_dir test.

* Remove extra whitespace.

* Split off some helper functions from compile_shell.py into sm_compile_helpers.py

* Inline code involving version in compile_shell.

* Mostly stop catching generic Exceptions in compile_shell.

* Refactor compileJs to sm_compile and fix previous WIP/FIXME items.

* Use io.open instead of open throughout.

* Fix more issues involving the move of functions to sm_compile_helpers.

* Fix encoding issues with io.open

* Remove unneeded parenthesis.

* Fix more encoding issues with io.open in tests.

* Binary mode of io.open does not take an encoding argument.

* Fix another encoding issue.

* Lithium only takes in strings, not Paths nor bytes.

* SyntaxError fix.

* Revert "Remove unneeded 'if __name__ == "__main__":' lines."

This reverts commit 1eddcfc.

* Cast more paths to strings before decoding them.

* Most repo_dirs are now Paths.

* Fix missing imports.

* Improve dual Python 2/3 code.

* knownPath is still present so account for it for now.

* Yet another pathlib issue.

* Ensure that cwd_prefix is a Path in js_interesting.

* Collector takes in strings, not Paths.

* Expand the repo_dir path first, in build_options.

* Add an unrelated pylint ignore line.

* Run Paths in the parsing function of compare_jit through .expanduser() first.

* Raise an OSError if a file does not exist in the parsing function of compare_jit instead of a generic Exception.

* Ensure cwd_prefix is a Path in compare_jit.

* When running compare_jit standalone, ensure the temp folder that is created returns its result in the form of a Path.

* Path does not care that it is already working on a Path instance.

* Make sure also to run pathToBinary through .expanduser()

* Add more pylint ignore lines to suit PyPy3.

Author: nth10sd

src/funfuzz/autobisectjs/autobisectjs.py

@@ -7,7 +7,7 @@
 """Known broken changeset ranges of SpiderMonkey are specified in this file.
 """
 
-from __future__ import absolute_import, print_function  # isort:skip
+from __future__ import absolute_import, unicode_literals  # isort:skip
 
 import os
 import platform
@@ -129,7 +129,8 @@ def earliest_known_working_rev(options, flags, skip_revs):  # pylint: disable=mi
     # Note that the sed version check only works with GNU sed, not BSD sed found in macOS.
     if (platform.system() == "Linux" and
             parse_version(subprocess.run(["sed", "--version"],
-                                         stdout=subprocess.PIPE).stdout.split()[3]) >= parse_version("4.3")):
+                                         stdout=subprocess.PIPE).stdout.decode("utf-8", errors="replace").split()[3])
+            >= parse_version("4.3")):
         required.append("ebcbf47a83e7")  # m-c 328765 Fx53, 1st w/ working builds using sed 4.3+ found on Ubuntu 17.04+
     if options.disableProfiling:
         required.append("800a887c705e")  # m-c 324836 Fx53, 1st w/ --disable-profiling, see bug 1321065

src/funfuzz/autobisectjs/known_broken_earliest_working.py

@@ -8,9 +8,10 @@
 
 """
 
-from __future__ import absolute_import, division, print_function  # isort:skip
+from __future__ import absolute_import, division, print_function, unicode_literals  # isort:skip
 
 from builtins import object  # pylint: disable=redefined-builtin
+import io
 import multiprocessing
 from optparse import OptionParser  # pylint: disable=deprecated-module
 import os
@@ -20,19 +21,26 @@
 import tempfile
 import time
 
+from whichcraft import which
+
 from .js import build_options
 from .js import compile_shell
 from .js import loop
 from .util import create_collector
 from .util import fork_join
 from .util import hg_helpers
-from .util import subprocesses as sps
+from .util import sm_compile_helpers
 from .util.lock_dir import LockDir
 
 if sys.version_info.major == 2:
+    from pathlib2 import Path
     import psutil
+    if os.name == "posix":
+        import subprocess32 as subprocess  # pylint: disable=import-error
+else:
+    import subprocess
+    from pathlib import Path  # pylint: disable=import-error
 
-path0 = os.path.dirname(os.path.abspath(__file__))  # pylint: disable=invalid-name
 JS_SHELL_DEFAULT_TIMEOUT = 24  # see comments in loop for tradeoffs
 
 
@@ -84,7 +92,8 @@ def parseOpts():  # pylint: disable=invalid-name,missing-docstring,missing-retur
     if args:
         print("Warning: bot does not use positional arguments")
 
-    if not options.useTreeherderBuilds and not os.path.isdir(build_options.DEFAULT_TREES_LOCATION):
+    # pylint: disable=no-member
+    if not options.useTreeherderBuilds and not build_options.DEFAULT_TREES_LOCATION.is_dir():
         # We don't have trees, so we must use treeherder builds.
         options.useTreeherderBuilds = True
         print()
@@ -102,11 +111,11 @@ def parseOpts():  # pylint: disable=invalid-name,missing-docstring,missing-retur
 
 
 def main():  # pylint: disable=missing-docstring
-    printMachineInfo()
+    print_machine_info()
 
     options = parseOpts()
 
-    collector = create_collector.createCollector("jsfunfuzz")
+    collector = create_collector.make_collector()
     try:
         collector.refresh()
     except RuntimeError as ex:
@@ -118,10 +127,10 @@ def main():  # pylint: disable=missing-docstring
     print(options.tempDir)
 
     build_info = ensureBuild(options)
-    assert os.path.isdir(build_info.buildDir)
+    assert build_info.buildDir.is_dir()
 
     number_of_processes = multiprocessing.cpu_count()
-    if "-asan" in build_info.buildDir:
+    if "-asan" in str(build_info.buildDir):
         # This should really be based on the amount of RAM available, but I don't know how to compute that in Python.
         # I could guess 1 GB RAM per core, but that wanders into sketchyville.
         number_of_processes = max(number_of_processes // 2, 1)
@@ -131,41 +140,41 @@ def main():  # pylint: disable=missing-docstring
     shutil.rmtree(options.tempDir)
 
 
-def printMachineInfo():  # pylint: disable=invalid-name
+def print_machine_info():
     """Log information about the machine."""
     print("Platform details: %s" % " ".join(platform.uname()))
-    print("hg version: %s" % sps.captureStdout(["hg", "-q", "version"])[0])
 
-    # In here temporarily to see if mock Linux slaves on TBPL have gdb installed
-    try:
-        print("gdb version: %s" % sps.captureStdout(["gdb", "--version"], combineStderr=True,
-                                                    ignoreStderr=True, ignoreExitCode=True)[0])
-    except (KeyboardInterrupt, Exception) as ex:  # pylint: disable=broad-except
-        print("Error involving gdb is: %r" % (ex,))
-
-    # FIXME: Should have if os.path.exists(path to git) or something  # pylint: disable=fixme
-    # print("git version: %s" % sps.captureStdout(["git", "--version"], combineStderr=True,
-    #                                             ignoreStderr=True, ignoreExitCode=True)[0])
+    print("hg info: %s" % subprocess.run(["hg", "-q", "version"], check=True, stdout=subprocess.PIPE).stdout.rstrip())
+    if which("gdb"):
+        gdb_version = subprocess.run(["gdb", "--version"],
+                                     stdout=subprocess.PIPE).stdout.decode("utf-8", errors="replace")
+        print("gdb info: %s" % gdb_version.split("\n")[0])
+    if which("git"):
+        print("git info: %s" % subprocess.run(["git", "version"], check=True, stdout=subprocess.PIPE).stdout.rstrip())
     print("Python version: %s" % sys.version.split()[0])
+
     print("Number of cores visible to OS: %d" % multiprocessing.cpu_count())
     if sys.version_info.major == 2:
         rootdir_free_space = psutil.disk_usage("/").free / (1024 ** 3)
     else:
         rootdir_free_space = shutil.disk_usage("/").free / (1024 ** 3)  # pylint: disable=no-member
     print("Free space (GB): %.2f" % rootdir_free_space)
 
-    hgrc_path = os.path.join(path0, ".hg", "hgrc")
-    if os.path.isfile(hgrc_path):
+    hgrc_path = Path("~/.hg/hgrc").expanduser()
+    if hgrc_path.is_file():
         print("The hgrc of this repository is:")
-        with open(hgrc_path, "r") as f:
+        with io.open(str(hgrc_path), "r", encoding="utf-8", errors="replace") as f:
             hgrc_contents = f.readlines()
         for line in hgrc_contents:
             print(line.rstrip())
 
-    if os.name == "posix":
+    try:
         # resource library is only applicable to Linux or Mac platforms.
         import resource  # pylint: disable=import-error
+        # pylint: disable=no-member
         print("Corefile size (soft limit, hard limit) is: %r" % (resource.getrlimit(resource.RLIMIT_CORE),))
+    except ImportError:
+        print("Not checking corefile size as resource module is unavailable")
 
 
 def ensureBuild(options):  # pylint: disable=invalid-name,missing-docstring,missing-return-doc,missing-return-type-doc
@@ -177,16 +186,16 @@ def ensureBuild(options):  # pylint: disable=invalid-name,missing-docstring,miss
         bRev = ""  # pylint: disable=invalid-name
         manyTimedRunArgs = []  # pylint: disable=invalid-name
     elif not options.useTreeherderBuilds:
-        options.build_options = build_options.parseShellOptions(options.build_options)
+        options.build_options = build_options.parse_shell_opts(options.build_options)
         options.timeout = options.timeout or (300 if options.build_options.runWithVg else JS_SHELL_DEFAULT_TIMEOUT)
 
-        with LockDir(compile_shell.getLockDirPath(options.build_options.repoDir)):
-            bRev = hg_helpers.getRepoHashAndId(options.build_options.repoDir)[0]  # pylint: disable=invalid-name
+        with LockDir(sm_compile_helpers.get_lock_dir_path(Path.home(), options.build_options.repo_dir)):
+            bRev = hg_helpers.get_repo_hash_and_id(options.build_options.repo_dir)[0]  # pylint: disable=invalid-name
             cshell = compile_shell.CompiledShell(options.build_options, bRev)
-            updateLatestTxt = (options.build_options.repoDir == "mozilla-central")  # pylint: disable=invalid-name
+            updateLatestTxt = (options.build_options.repo_dir == "mozilla-central")  # pylint: disable=invalid-name
             compile_shell.obtainShell(cshell, updateLatestTxt=updateLatestTxt)
 
-            bDir = cshell.getShellCacheDir()  # pylint: disable=invalid-name
+            bDir = cshell.get_shell_cache_dir()  # pylint: disable=invalid-name
             # Strip out first 3 chars or else the dir name in fuzzing jobs becomes:
             #   js-js-dbg-opt-64-dm-linux
             bType = build_options.computeShellType(options.build_options)[3:]  # pylint: disable=invalid-name
@@ -199,9 +208,9 @@ def ensureBuild(options):  # pylint: disable=invalid-name,missing-docstring,miss
                 "==============================================\n\n" % (
                     "funfuzz.js.compile_shell",
                     options.build_options.build_options_str,
-                    options.build_options.repoDir,
+                    options.build_options.repo_dir,
                     bRev,
-                    cshell.getRepoName(),
+                    cshell.get_repo_name(),
                     time.asctime()
                 ))
 
@@ -216,15 +225,15 @@ def ensureBuild(options):  # pylint: disable=invalid-name,missing-docstring,miss
 
 
 def loopFuzzingAndReduction(options, buildInfo, collector, i):  # pylint: disable=invalid-name,missing-docstring
-    tempDir = tempfile.mkdtemp("loop" + str(i))  # pylint: disable=invalid-name
+    tempDir = Path(tempfile.mkdtemp("loop" + str(i)))  # pylint: disable=invalid-name
     loop.many_timed_runs(options.targetTime, tempDir, buildInfo.mtrArgs, collector)
 
 
 def mtrArgsCreation(options, cshell):  # pylint: disable=invalid-name,missing-param-doc,missing-return-doc
     # pylint: disable=missing-return-type-doc,missing-type-doc
     """Create many_timed_run arguments for compiled builds."""
     manyTimedRunArgs = []  # pylint: disable=invalid-name
-    manyTimedRunArgs.append("--repo=" + sps.normExpUserPath(options.build_options.repoDir))
+    manyTimedRunArgs.append("--repo=%s" % options.build_options.repo_dir)
     manyTimedRunArgs.append("--build=" + options.build_options.build_options_str)
     if options.build_options.runWithVg:
         manyTimedRunArgs.append("--valgrind")
@@ -236,10 +245,6 @@ def mtrArgsCreation(options, cshell):  # pylint: disable=invalid-name,missing-pa
 
     # Ordering of elements in manyTimedRunArgs is important.
     manyTimedRunArgs.append(str(options.timeout))
-    manyTimedRunArgs.append(cshell.getRepoName())  # known bugs' directory
-    manyTimedRunArgs.append(cshell.getShellCacheFullPath())
+    manyTimedRunArgs.append(cshell.get_repo_name())  # known bugs' directory
+    manyTimedRunArgs.append(cshell.get_shell_cache_js_bin_path())
     return manyTimedRunArgs
-
-
-if __name__ == "__main__":
-    main()

src/funfuzz/bot.py

@@ -11,12 +11,17 @@
 
 import io
 import logging
-import os
+import sys
 import tarfile
 import zipfile
 
 import requests
 
+if sys.version_info.major == 2:
+    from pathlib2 import Path
+else:
+    from pathlib import Path  # pylint: disable=import-error
+
 RUN_COV_LOG = logging.getLogger("funfuzz")
 
 
@@ -41,7 +46,8 @@ def get_coverage_build(dirpath, args):
 
     js_cov_bin_name = "js"
     js_cov_bin = extract_folder / "dist" / "bin" / js_cov_bin_name
-    os.chmod(str(js_cov_bin), os.stat(str(js_cov_bin)).st_mode | 0o111)  # Ensure the js binary is executable
+
+    Path.chmod(js_cov_bin, Path.stat(js_cov_bin).st_mode | 0o111)  # Ensure the js binary is executable
     assert js_cov_bin.is_file()
 
     # Check that the binary is non-debug.

src/funfuzz/ccoverage/get_build.py

@@ -13,5 +13,6 @@
 from . import compile_shell
 from . import inspect_shell
 from . import js_interesting
+from . import link_fuzzer
 from . import loop
 from . import shell_flags