-
Notifications
You must be signed in to change notification settings - Fork 301
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
introducing sha-256 checksum support #625
introducing sha-256 checksum support #625
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, outstanding PR :-), you thought to all the very little details!
The only thing I'm not sure about is whether SHA-256 checking should be enabled by default right now. This would trigger a lot of failed attempts to download sha 256 checksum, as these aren't too common for now...
I'm thinking about allowing to override the default checkums via the env (e.g. COURSIER_CHECKSUMS=SHA-256,SHA-1,none
) and / or Java properties, I'll push that soon. You could enable SHA-256 checking this way. Would that work for you?
@@ -728,6 +728,7 @@ abstract class CentralTests extends TestSuite { | |||
val ver = "1.17" | |||
|
|||
def hasSha1(a: Artifact) = a.checksumUrls.contains("SHA-1") | |||
def hasSha256(a: Artifact) = a.checksumUrls.contains("SHA-256") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That line isn't necessary any more (since your last commit).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, I will remove it soon.
Actually, since I use the library programatically and not in our build system I could just write simply I don't think using SHA-256 checksums will be standard anytime soon either, so that way I would not need to change the defaults at all. |
@alexarchambault would it be possible to merge this master and create a new release? |
|
introducing sha-256 checksum support
thanks! |
Implementing #621. This change introduces support for
SHA-256
hashes. The default checksums to use for validation has been changed fromSHA1 -> None
toSHA-256 -> SHA1 -> None