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

Add checksums for LibCURL #39702

Merged
merged 1 commit into from
Feb 18, 2021
Merged

Conversation

DilumAluthge
Copy link
Member

@DilumAluthge DilumAluthge commented Feb 17, 2021

These checksums were deleted in #38963.

However:

  1. When I run make locally, these checksums are generated.
  2. If you look in the deps/checksums/curl file, these checksums are not located there.

@KristofferC
Copy link
Member

When I run make locally, these checksums are generated.

Not for me. Have you cleaned out old deps stuff from your repo?

@DilumAluthge
Copy link
Member Author

Yep, I even tried in a brand-new clone and it still generates the checksums.

This is on macOS.

@DilumAluthge
Copy link
Member Author

I get this with a brand-new clone on both macOS and Linux.

@KristofferC
Copy link
Member

KristofferC commented Feb 17, 2021

Yeah, I managed to get it now as well after a git clean -fdX.

Perhaps these two lines should rather be added back: https://github.com/JuliaLang/julia/pull/39625/files#diff-c5e29ac5eae5352c7cd703597b517a569d2a4fee6684dc03f54e891e6883854eL2

I don't really understand why one of the tarballs in there is not used though?

Or since this is for jll, maybe it is better to have them in a separate file like in this PR (it seems we do for the other stdlibs).

@DilumAluthge DilumAluthge requested a review from omus February 17, 2021 10:27
@DilumAluthge
Copy link
Member Author

Perhaps these two lines should rather be added back: https://github.com/JuliaLang/julia/pull/39625/files#diff-c5e29ac5eae5352c7cd703597b517a569d2a4fee6684dc03f54e891e6883854eL2

Or since this is for jll, maybe it is better to have them in a separate file like in this PR (it seems we do for the other stdlibs).

Good question. I don't feel strongly either way. @vtjnash since you did the reorganization of the checksums recently, which one would you prefer?

@vtjnash
Copy link
Member

vtjnash commented Feb 18, 2021

Either is fine

@KristofferC KristofferC merged commit 779d693 into master Feb 18, 2021
@KristofferC KristofferC deleted the dpa/libcurl-checksums-83104870 branch February 18, 2021 16:15
ElOceanografo pushed a commit to ElOceanografo/julia that referenced this pull request May 4, 2021
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request May 9, 2021
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.

3 participants