Skip to content
This repository has been archived by the owner on Jun 1, 2024. It is now read-only.

Handle OSX's md5sum -r being slightly different to GNU Coreutils #3

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hawkowl
Copy link
Member

@hawkowl hawkowl commented May 12, 2014

OS X's md5sum uses one space inbetween the sum and filename, not two. Since they both have two items, we can handle both by just fetching the first and last item.

@exarkun
Copy link
Contributor

exarkun commented May 12, 2014

Some unit tests would probably be a good idea at this point.

@thijstriemstra
Copy link
Contributor

There are tests for that macro already in twisted_trac_plugins/test/test_release_macro.py

@mithrandi
Copy link

It seems to me that the md5sums should be generated in -b mode, so there should be an asterisk and a space between the sum and the filename, not two spaces. Of course, this doesn't matter on Linux / OS X, but anyone trying to verify the md5sums on Windows might be unpleasantly confused.

This comment is, I suppose, not directly related to this pull request, let me know if I should file a separate issue.

@hawkowl
Copy link
Member Author

hawkowl commented May 12, 2014

okay now this should make it actually work
i don't have trac and am currently doing this from bed so will have to try the unit tests when i have time to deal with downloading trac

@thijstriemstra
Copy link
Contributor

brokenmacro

it's showing an error on the homepage now.

@glyph
Copy link

glyph commented May 12, 2014

I updated the home page so as not to include the MD5 hash for now, temporarily fixing the error.

That whole "downloads" section should start with instructions to "pip install" now, shouldn't it?

@glyph
Copy link

glyph commented May 12, 2014

Also, given that this is a python script, can't we just use Python's built-in hashing functions?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants