Added support for sha2 checksums.
ClosedPublic

Authored by vonreth on Dec 31 2015, 3:29 PM.

Details

Summary

Moved digests to own module.
Refactored related code.

Diff Detail

Repository
R138 Craft
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
vonreth updated this revision to Diff 1683.Dec 31 2015, 3:29 PM
vonreth retitled this revision from to Added support for sha2 checksums..
vonreth updated this object.
vonreth edited the test plan for this revision. (Show Details)
vonreth added reviewers: kfunk, sengels.
vonreth updated this revision to Diff 1684.Dec 31 2015, 3:41 PM
  • Fix typos and remove alwaysTrue()
vonreth updated this revision to Diff 1685.Dec 31 2015, 3:49 PM
  • Revert merge artifact
vonreth updated this revision to Diff 1686.Dec 31 2015, 4:12 PM
  • Fix missing definitions
vonreth updated this revision to Diff 1687.Jan 1 2016, 11:22 AM
  • Cleanup and optimization
vonreth updated this revision to Diff 1688.Jan 1 2016, 11:37 AM
  • Revert "Fix typos and remove alwaysTrue()"
vonreth updated this revision to Diff 1689.Jan 1 2016, 12:02 PM
  • Cleanup
vonreth updated this revision to Diff 1692.Jan 1 2016, 3:36 PM
  • Fix typos and remove alwaysTrue()
  • Revert merge artifact
  • Revert "Fix typos and remove alwaysTrue()"
  • Cleanup
  • Improve digestFile performane
sengels edited edge metadata.Jan 1 2016, 10:35 PM

General: Please try to find a single style for spaces and parentheses: either no spaces, or spaces after/before parentheses. Please don't mix, especially not in a single line...

  1. Is it possible to have unittests when e.g. EmergeDebug.py and EmergeHash.py are run directly? Since you add EmergeHash here, could you write some lines that test e.g. that algorithms are found correctly, that empty files (or some specially generated temporary files) do have a reproduceable hash?
bin/EmergeHash.py
100

I am not sure, in theory it would be cool to get the correct indent of the file as well (or the default indent of 2*4 spaces) so that copying that line and entering it inside the package file is easiest...

vonreth updated this revision to Diff 1694.Jan 2 2016, 12:41 AM
vonreth edited edge metadata.
  • Improve digestFile performane
vonreth added inline comments.Jan 2 2016, 12:42 AM
bin/EmergeHash.py
100

You still have to copy it to the destination.
I dropped the new lines to not get problems with indent and handling first and last case for the loop.

I added a basic unit test for the hashes.
What test would you like too?
From sting "[SHA26]...."?
From file name?

For the whitespace, could we add a astyle script or something like that?

kfunk added inline comments.Jan 2 2016, 3:25 AM
bin/EmergeHash.py
109

Can't you move that to an extra file and use proper testing infrastructure (Python's unittest lib) for it?

We already have bin/test/, would be a good idea to store the new test_EmergeHash.py there. And also make sure it runs with that runtests.py runner.

vonreth updated this revision to Diff 1700.Jan 2 2016, 1:06 PM
  • Fix typos and remove alwaysTrue()
  • Revert merge artifact
  • Revert "Fix typos and remove alwaysTrue()"
  • Cleanup
  • Improve digestFile performane
  • Fix unittest and add unittest for EmergeHash
vonreth updated this revision to Diff 1709.Jan 3 2016, 3:36 PM
  • Simplyfy printFilesDigests
vonreth updated this revision to Diff 1710.Jan 3 2016, 4:21 PM
  • Simplyfy printFilesDigests
kfunk edited edge metadata.Jan 3 2016, 5:52 PM

Diff contains lot of unrelated changes again.

My issue with tests was fixed, so +1 from my side. I'll let Patrick review the digest feature itself :)

vonreth updated this revision to Diff 1719.Jan 4 2016, 9:43 AM
vonreth edited edge metadata.
  • Simplyfy printFilesDigests
vonreth updated this revision to Diff 1720.Jan 4 2016, 9:47 AM
  • Simplyfy printFilesDigests
sengels accepted this revision.Jan 4 2016, 2:50 PM
sengels edited edge metadata.

Hm, it would be cool if you could split this into several requests next time...

bin/EmergeHash.py
38

Shouldn't the default be MD5 or whatever we currently have?

This revision is now accepted and ready to land.Jan 4 2016, 2:50 PM
vonreth marked an inline comment as done.Jan 4 2016, 4:27 PM
vonreth added inline comments.
bin/EmergeHash.py
38

I marked it as deprecated and it shouldn't be used at all.
The current default is md5 and it is used as a special fall back but not as a default value.

This revision was automatically updated to reflect the committed changes.
vonreth marked an inline comment as done.