Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use cksum instead of sha1sum for checksums #54

Merged
merged 2 commits into from Jan 5, 2017
Merged

Conversation

blueyed
Copy link
Collaborator

@blueyed blueyed commented Jan 4, 2017

Fixes #53.

We might to migrate existing checksums?!
Not sure if it's worth the effort though..

Copy link
Owner

@Tarrasch Tarrasch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Can we just double-check with som Mac-user that they actually have cksum available? I noticed my Ubuntu 14.04.05 has it without me making any effort to install it. :)

if ! [[ -e $env_file ]]; then
echo "Missing file argument for _autoenv_hash_pair!" >&2
return 1
fi
env_shasum=$(sha1sum $env_file | cut -d' ' -f1)
env_cksum=${(j:.:)${:-$(cksum "$env_file")}[1,2]}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding an explicit comment explaining what (j:.:) does would be nice. zsh is so not readable...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point, added.
Should be squash-merged.

@blueyed
Copy link
Collaborator Author

blueyed commented Jan 4, 2017

Can we just double-check with som Mac-user that they actually have cksum available?

Should be there according to @rspeed.
@jasisk
Can you confirm this?

@jasisk
Copy link

jasisk commented Jan 4, 2017

Can you confirm this?

Confirmed. 👍

@blueyed blueyed merged commit 398b6f4 into Tarrasch:master Jan 5, 2017
@blueyed blueyed deleted the cksum branch January 5, 2017 00:16
@blueyed
Copy link
Collaborator Author

blueyed commented Jan 5, 2017

Thanks!

@rspeed
Copy link
Contributor

rspeed commented Jan 5, 2017

Well crap. I made my own implementation earlier today and got sidetracked before I could push it. Gonna open a PR anyway since there's one difference that I think is probably important.

rspeed added a commit to rspeed/contrib-zsh-autoenv that referenced this pull request Jan 5, 2017
Adds a new whitelist file hashing algorithm using `cksum`, which is very fast and identical between BSD and GNU.
A new parameter is added to `_autoenv_hash_pair` to specify the version, defaulting to the latest (2). It outputs a `cksum`-based hash for version 2 and `shasum`-based for version 1.
Moves logic to check for an entry in `$AUTOENV_AUTH_FILE` into its own function (`_autoenv_authorized_pair`), as it may need to be called twice.
Modifies `_autoenv_authorized_env_file` to check for v1 entries when v2 fails.

Fixes Tarrasch#53. Alternative implementation to Tarrasch#54.
rspeed added a commit to rspeed/contrib-zsh-autoenv that referenced this pull request Jan 10, 2017
Adds a new whitelist file hashing algorithm using `cksum`, which is very fast and identical between BSD and GNU.
A new parameter is added to `_autoenv_hash_pair` to specify the version, defaulting to the latest (2). It outputs a `cksum`-based hash for version 2 and `shasum`-based for version 1.
Moves logic to check for an entry in `$AUTOENV_AUTH_FILE` into its own function (`_autoenv_authorized_pair`), as it may need to be called twice.
Modifies `_autoenv_authorized_env_file` to check for v1 entries when v2 fails.

Fixes Tarrasch#53. Alternative implementation to Tarrasch#54.
blueyed pushed a commit that referenced this pull request Jan 10, 2017
A new parameter is added to `_autoenv_hash_pair` to specify the version, defaulting to the latest (2). It outputs a `cksum`-based hash for version 2 and `shasum`-based for version 1.
Moves logic to check for an entry in `$AUTOENV_AUTH_FILE` into its own function (`_autoenv_authorized_pair`), as it may need to be called twice.
Modifies `_autoenv_authorized_env_file` to check for v1 entries when v2 fails.

Fixes #53. Alternative implementation to #54.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants