Repository: peterbe/hashin

Issue:
Order of hashes no longer canonical
Since version 0.14, the order of hashes does not seem to be deterministic.
Before 0.14, the hashes used to be lexicographically ordered.

I am guessing this was changed in https://github.com/peterbe/hashin/pull/95

What is the reason for this?

We use hashin in electrum for deterministic builds.
The [requirements files](https://github.com/spesmilo/electrum/blob/c7f3adb67ea6afd50fec60221ba711ddea110281/contrib/deterministic-build/requirements.txt) are generated by [this script](https://github.com/spesmilo/electrum/blob/c7f3adb67ea6afd50fec60221ba711ddea110281/contrib/freeze_packages.sh).
The `freeze_packages.sh` script is manually rerun to update dependencies (see e.g. https://github.com/spesmilo/electrum/commit/ba4af29bf82f7addb68c3ed996002665313ab223).
If the order of hashes for a given package is not canonical, then rerunning the script can result in changes for packages that have not actually been updated, making the diff considerably less readable.

Discussion:
SomberNight: Looks OK, and seems to work for me too. Thanks!

Patch:
diff --git a/hashin.py b/hashin.py
index cbc998c..a43390e 100755
--- a/hashin.py
+++ b/hashin.py
@@ -246,7 +246,7 @@ def run_packages(
         maybe_restriction = "" if not restriction else "; {0}".format(restriction)
         new_lines = "{0}=={1}{2} \\\n".format(req, data["version"], maybe_restriction)
         padding = " " * 4
-        for i, release in enumerate(data["hashes"]):
+        for i, release in enumerate(sorted(data["hashes"], key=lambda r: r["hash"])):
             new_lines += "{0}--hash={1}:{2}".format(padding, algorithm, release["hash"])
             if i != len(data["hashes"]) - 1:
                 new_lines += " \\"


Test patch:
diff --git a/tests/test_cli.py b/tests/test_cli.py
index 459694f..e494a33 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -461,7 +461,7 @@ autocompeter==1.2.3 \\
     assert result == new_lines[2]
 
 
-def test_amend_requirements_content_replacement_amonst_others():
+def test_amend_requirements_content_replacement_amongst_others():
     previous = (
         """
 otherpackage==1.0.0 --hash=sha256:cHay6ATFKumO3svU3B-8qBMYb-f1_dYlR4OgClWntEI
@@ -489,7 +489,7 @@ autocompeter==1.2.3 \\
     assert result == previous + new_lines[2]
 
 
-def test_amend_requirements_content_replacement_amonst_others_2():
+def test_amend_requirements_content_replacement_amongst_others_2():
     previous = (
         "https://github.com/rhelmer/pyinotify/archive/9ff352f.zip"
         "#egg=pyinotify "
@@ -609,7 +609,6 @@ def test_run(murlopen, tmpfile, capsys):
         # Now check the verbose output
         captured = capsys.readouterr()
         out_lines = captured.out.splitlines()
-        # out_lines = my_stdout.getvalue().splitlines()
         assert "https://pypi.org/pypi/hashin/json" in out_lines[0], out_lines[0]
         # url to download
         assert "hashin-0.10-py2-none-any.whl" in out_lines[1], out_lines[1]
@@ -647,6 +646,60 @@ def test_run(murlopen, tmpfile, capsys):
         ) in lines
 
 
+def test_canonical_list_of_hashes(murlopen, tmpfile, capsys):
+    """When hashes are written down, after the package spec, write down the hashes
+    in a canonical way. Essentially, in lexicographic order. But when comparing
+    existing hashes ignore any order.
+    """
+
+    def mocked_get(url, **options):
+        if url == "https://pypi.org/pypi/hashin/json":
+            return _Response(
+                {
+                    "info": {"version": "0.10", "name": "hashin"},
+                    "releases": {
+                        # NOTE that the order of list is different from
+                        # the order you'd get of `sorted(x.digest for x in releases)`
+                        "0.10": [
+                            {
+                                "url": "https://pypi.org/packages/source/p/hashin/hashin-0.10.tar.gz",
+                                "digests": {"sha256": "ccccc"},
+                            },
+                            {
+                                "url": "https://pypi.org/packages/2.7/p/hashin/hashin-0.10-py2-none-any.whl",
+                                "digests": {"sha256": "aaaaa"},
+                            },
+                            {
+                                "url": "https://pypi.org/packages/3.3/p/hashin/hashin-0.10-py3-none-any.whl",
+                                "digests": {"sha256": "bbbbb"},
+                            },
+                        ]
+                    },
+                }
+            )
+        raise NotImplementedError(url)
+
+    murlopen.side_effect = mocked_get
+
+    with tmpfile() as filename:
+        with open(filename, "w") as f:
+            f.write("")
+
+        retcode = hashin.run("hashin==0.10", filename, "sha256")
+
+        assert retcode == 0
+        with open(filename) as f:
+            output = f.read()
+        assert output
+        assert output.endswith("\n")
+        lines = output.splitlines()
+
+        assert lines[0] == "hashin==0.10 \\"
+        assert lines[1] == "    --hash=sha256:aaaaa \\"
+        assert lines[2] == "    --hash=sha256:bbbbb \\"
+        assert lines[3] == "    --hash=sha256:ccccc"
+
+
 def test_run_atomic_not_write_with_error_on_last_package(murlopen, tmpfile):
     def mocked_get(url, **options):
 
@@ -1623,7 +1676,7 @@ def test_run_comments_with_package_spec_patterns(murlopen, tmpfile, capsys):
 
 
 def test_run_dry(murlopen, tmpfile, capsys):
-    """dry run should edit the requirements.txt file and print
+    """dry run should not edit the requirements file and print
     hashes and package name in the console
     """