Merge pull request #48 from dfxml-working-group/add_isort_via_pre_commit

.github/workflows/continuous-integration.yml

@@ -37,6 +37,11 @@ jobs:
           sudo apt update
           sudo apt install --yes libxml2-utils
 
+      - name: Pre-commit Checks
+        run: |
+          pip -q install pre-commit
+          pre-commit run --all-files
+
       - name: Make check
         run: make check
 

.gitignore

@@ -11,3 +11,4 @@ python/demo.dfxml
 .pytest_cache
 *.egg-info
 *.log
+.venv-pre-commit

.pre-commit-config.yaml

@@ -0,0 +1,6 @@
+repos:
+  - repo: https://github.com/pycqa/isort
+    rev: 5.13.2
+    hooks:
+      - id: isort
+        name: isort (python)

CONTRIBUTE.md

@@ -1,6 +1,25 @@
 # Contributing to DFXML's Python code base
 
 
+## Pre-commit
+
+This project uses [the `pre-commit` tool](https://pre-commit.com/) for linting.
+
+`pre-commit` hooks into Git's commit machinery to run a set of linters and static analyzers over each change. To install `pre-commit` into Git's hooks, run one (not both) of the following sets of commands:
+
+```bash
+pip install pre-commit
+pre-commit --version
+pre-commit install
+```
+
+Or:
+
+```bash
+make
+```
+
+
 ## Installable tools versus in-place scripts
 
 The [`dfxml/bin/`](dfxml/bin/) directory contains scripts for interacting with DFXML.  Some of the tools are installed in the command-line `$PATH` when the `dfxml` package is installed.

Makefile

@@ -18,11 +18,18 @@ ifeq ($(shell basename $(SHELL)),sh)
 SHELL := $(shell which /bin/bash 2>/dev/null || which /usr/local/bin/bash)
 endif
 
-all:
+PYTHON3 ?= python3
+ifeq ($(PYTHON3),)
+$(error python3 not found)
+endif
+
+all: \
+  .venv-pre-commit/var/.pre-commit-built.log
 
 .PHONY: \
   check-mypy \
-  check-supply-chain
+  check-supply-chain \
+  check-supply-chain-pre-commit
 
 .git_submodule_init.done.log: .gitmodules
 	# Confirm dfxml_schema has been checked out at least once.
@@ -31,28 +38,86 @@ all:
 	test -r dependencies/dfxml_schema/dfxml.xsd
 	touch $@
 
+# This virtual environment is meant to be built once and then persist, even through 'make clean'.
+# If a recipe is written to remove this flag file, it should first run `pre-commit uninstall`.
+.venv-pre-commit/var/.pre-commit-built.log:
+	rm -rf .venv-pre-commit
+	test -r .pre-commit-config.yaml \
+	  || (echo "ERROR:Makefile:pre-commit is expected to install for this repository, but .pre-commit-config.yaml does not seem to exist." >&2 ; exit 1)
+	$(PYTHON3) -m venv \
+	  .venv-pre-commit
+	source .venv-pre-commit/bin/activate \
+	  && pip install \
+	    --upgrade \
+	    pip \
+	    setuptools \
+	    wheel
+	source .venv-pre-commit/bin/activate \
+	  && pip install \
+	    pre-commit
+	source .venv-pre-commit/bin/activate \
+	  && pre-commit install
+	mkdir -p \
+	  .venv-pre-commit/var
+	touch $@
+
 clean:
 	find . -name '*~' -exec rm {} \;
 	$(MAKE) \
 	  --directory tests \
 	  clean
 
 check: \
-  check-mypy 
+  .git_submodule_init.done.log \
+  .venv-pre-commit/var/.pre-commit-built.log
 	$(MAKE) \
+	  PYTHON3=$(PYTHON3) \
 	  SHELL=$(SHELL) \
 	  --directory tests \
 	  check
 
 check-mypy: \
   .git_submodule_init.done.log
 	$(MAKE) \
+	  PYTHON3=$(PYTHON3) \
 	  SHELL=$(SHELL) \
 	  --directory tests \
 	  check-mypy
 
 check-supply-chain: \
+  check-supply-chain-pre-commit \
   check-mypy
 
+# Update pre-commit configuration and use the updated config file to
+# review code.  Only have Make exit if 'pre-commit run' modifies files.
+check-supply-chain-pre-commit: \
+  .venv-pre-commit/var/.pre-commit-built.log
+	source .venv-pre-commit/bin/activate \
+	  && pre-commit autoupdate
+	git diff \
+	  --exit-code \
+	  .pre-commit-config.yaml \
+	  || ( \
+	      source .venv-pre-commit/bin/activate \
+	        && pre-commit run \
+	          --all-files \
+	          --config .pre-commit-config.yaml \
+	    ) \
+	    || git diff \
+	      --stat \
+	      --exit-code \
+	      || ( \
+	          echo \
+	            "WARNING:Makefile:pre-commit configuration can be updated.  It appears the updated would change file formatting." \
+	            >&2 \
+	            ; exit 1 \
+                )
+	@git diff \
+	  --exit-code \
+	  .pre-commit-config.yaml \
+	  || echo \
+	    "INFO:Makefile:pre-commit configuration can be updated.  It appears the update would not change file formatting." \
+	    >&2
+
 check-tools:
 	(cd tests/misc_object_tests;make check)

demos/demo_fiwalk_diskimage.py

@@ -6,10 +6,12 @@
 if the required buffer size exceeds available RAM!
 """
 
-import sys
 import io
+import sys
+
 from dfxml import fiwalk
 
+
 def writeDfxml(imageFile: str, outFile: str) -> None:
     """Generate filesystem metadata for disk image and and write resulting
     dfxml to file"""

demos/demo_mac_timeline.py

@@ -7,7 +7,6 @@
 sys.path.append( os.path.join(os.path.dirname(__file__), ".."))
 import dfxml
 
-
 timeline = []
 
 def process(fi):

demos/demo_mac_timeline_iter.py

@@ -22,6 +22,7 @@
 sys.path.append( os.path.join(os.path.dirname(__file__), ".."))
 import dfxml
 
+
 def main():
     if len(sys.argv) < 2:
         print("Usage: {} <filename.xml>".format(sys.argv[0]))

demos/demo_piecewise.py

@@ -9,10 +9,11 @@
 import sys
 
 sys.path.append( os.path.join(os.path.dirname(__file__), ".."))
-import dfxml
-
+import collections
+import math
+import sys
 
-import math,sys,collections
+import dfxml
 
 
 class SectorCorrelator:

demos/demo_plot_times.py

@@ -1,14 +1,13 @@
 #!/usr/bin/python                                                                                                     
-import fiwalk
-import time
-
 import os
 import sys
+import time
+
+import fiwalk
 
 sys.path.append( os.path.join(os.path.dirname(__file__), ".."))
 import dfxml
 
-
 if __name__=="__main__":
     import sys
     from optparse import OptionParser

demos/demo_readtimes.py

@@ -1,15 +1,15 @@
 #!/usr/bin/python
 """Reads an fiwalk XML file and reports how many of the files are still in the image..."""
 
-import sys,os
-
 import os
 import sys
 
 sys.path.append( os.path.join(os.path.dirname(__file__), ".."))
+import time
+
 import dfxml
 import dfxml.fiwalk as fiwalk
-import time
+
 
 def calc_jumps(fis,title):
     print(title)
@@ -39,7 +39,7 @@ def calc_jumps(fis,title):
 if __name__=="__main__":
     import sys
     from optparse import OptionParser
-    from subprocess import Popen,PIPE
+    from subprocess import PIPE, Popen
     global options
 
     parser = OptionParser()

demos/demo_registry_timeline.py

@@ -5,7 +5,6 @@
 sys.path.append( os.path.join(os.path.dirname(__file__), ".."))
 import dfxml
 
-
 timeline = []
 
 def process(co):

demos/demo_sizes.py

@@ -4,10 +4,10 @@
 # Demo program that shows how to calculate the average size of file objects in a DFXML file
 #
 
+import collections
 import math
-import sys
 import os
-import collections
+import sys
 
 sys.path.append( os.path.join(os.path.dirname(__file__), ".."))
 import dfxml

demos/demo_spark.py

@@ -4,18 +4,19 @@
 # This program runs spark if it is not already running
 
 
-import sys
 import os
+import sys
 
 sys.path.append("../python")
 from dfxml_writer import DFXMLWriter
 
+
 def spark_demo():
     """A small spark program. Must be run under spark"""
-    from pyspark import SparkConf
-    from pyspark import SparkContext
     import operator
 
+    from pyspark import SparkConf, SparkContext
+
     conf = SparkConf()
     sc   = SparkContext(conf=conf)
     m    = 1000000

demos/spark/demo_spark.py

@@ -4,18 +4,19 @@
 # This program runs Spark if it is not already running
 
 
-import sys
 import os
+import sys
 
 sys.path.append("../python")
 from dfxml_writer import DFXMLWriter
 
+
 def spark_demo():
     """A small Spark program. Must be run under Spark"""
-    from pyspark import SparkConf
-    from pyspark import SparkContext
     import operator
 
+    from pyspark import SparkConf, SparkContext
+
     conf = SparkConf()
     sc   = SparkContext(conf=conf)
     m    = 1000000

demos/vmstats/vmstats.py

@@ -6,9 +6,10 @@
 import os
 import os.path
 import sys
+import time
 import xml.etree.ElementTree as ET
+
 import psutil
-import time
 
 sys.path.append( os.path.join(os.path.dirname(__file__), "../../python") )
 
@@ -70,8 +71,8 @@ def write_process_dfxml_to_file(fname,prettyprint=False):
             f.write("\n")
 
 if __name__=="__main__":
-    from argparse import ArgumentParser, ArgumentDefaultsHelpFormatter
     import time
+    from argparse import ArgumentDefaultsHelpFormatter, ArgumentParser
     parser = ArgumentParser(formatter_class=ArgumentDefaultsHelpFormatter)
     parser.add_argument("fname",help="filename")
     parser.add_argument("--repeat",help="Number of times to repeat",type=int,default=1)

demos/vmstats/vmstats_decode.py

@@ -2,20 +2,22 @@
 #
 # plot vmstats output
 
+import datetime
+import json
 import os
 import os.path
+import shutil
+import statistics
 import sys
+import time
 import xml.etree.ElementTree as ET
+
 import psutil
-import time
-import json
-import statistics
-import shutil
-import datetime
 
 sys.path.append( os.path.join(os.path.dirname(__file__), "../python") )
 import dfxml
 
+
 def get_dfxml(fname):
     # Given a file, return dfxml objects
     with open(fname,"r") as f:
@@ -70,9 +72,10 @@ def html_filename(root):
 
 TEMPLATE_FILE = "vmstats_decode.html"
 def html_generate(root, *, prev_fname, next_fname):
-    import jinja2 
     import os.path
 
+    import jinja2
+
     stats   = get_stats(root)
     ps_list = list( get_processes(root) )
 
@@ -91,8 +94,8 @@ def html_generate(root, *, prev_fname, next_fname):
 
 
 if __name__=="__main__":
-    from argparse import ArgumentParser, ArgumentDefaultsHelpFormatter
     import time
+    from argparse import ArgumentDefaultsHelpFormatter, ArgumentParser
     parser = ArgumentParser(formatter_class=ArgumentDefaultsHelpFormatter)
     parser.add_argument("fname",nargs="+", help="filename")
     parser.add_argument("--ps", help="Show the processes", action='store_true')
@@ -170,6 +173,7 @@ def myconverter(o):
 
     if args.plot:
         import datetime
+
         import matplotlib.pyplot as plt
         t0 = stats[0]['start_time']
         when = [ (st['start_time']-t0).total_seconds() for st in stats]