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

Auto download of clojure-tools.zip fails with mismatching checksum #113

Closed
DerGuteMoritz opened this issue Sep 26, 2023 · 12 comments
Closed

Comments

@DerGuteMoritz
Copy link
Contributor

Some time during the past two hours, the automatic download of clojure-tools.zip started failing with this error:

Type:     java.lang.Exception
Message:  Error: sha256 of zip and expected sha256 do not match: a7c79ff7e9e1947d8ab6a1d01b695abc017b51252d814687f8b708b43fcb02 vs. 00a7c79ff7e9e1947d8ab6a1d01b695abc017b51252d814687f8b708b43fcb02
 Please download manually from https://github.com/clojure/brew-install/releases/download/1.11.1.1413/clojure-tools.zip to .deps/deps.clj/clojure-tools.zip

Note that I ran into this by invoking babashka.tasks/clojure using babashka v1.3.184 but figured since the relevant code lives in deps.clj, it'd make more sense to report it here.

I suspect the root cause is that GitHub changed something on their end: Both https://github.com/clojure/brew-install/releases/download/1.11.1.1413/clojure-tools.zip and https://github.com/clojure/brew-install/releases/download/1.11.1.1413/clojure-tools.zip.sha256 now respond with a 302 redirect, so when curling them one has to pass -L to make it follow to the new location. ISTR this wasn't the case in the past and perhaps the auto-downloader doesn't follow redirects either?

@DerGuteMoritz
Copy link
Contributor Author

Hm then again, it does appear to follow the redirect for https://github.com/clojure/brew-install/releases/download/1.11.1.1413/clojure-tools.zip.sha256 - the checksum it reports in the error is indeed the one from that file...

@borkdude
Copy link
Owner

The error message is not complete:

Please download manually from https://github.com/clojure/brew-install/releases/download/1.11.1.1413/clojure-tools.zip to .deps/deps.clj/clojure-tools.zip

I believe there should be a version number in between.

@DerGuteMoritz
Copy link
Contributor Author

Aaah forget about all that redirect business: The problem are the leading zeroes of the checksum from the file! Formatting it like this makes it clear:

  a7c79ff7e9e1947d8ab6a1d01b695abc017b51252d814687f8b708b43fcb02
00a7c79ff7e9e1947d8ab6a1d01b695abc017b51252d814687f8b708b43fcb02

@DerGuteMoritz
Copy link
Contributor Author

The error message is not complete:
[...]
I believe there should be a version number in between.

Ah, sorry, this is because I ran it with DEPS_CLJ_TOOLS_DIR=".deps/deps.clj".

@borkdude
Copy link
Owner

oh I see. why don't I see this when I run it from source? Are we on different OSes?

$ clj -M -m borkdude.deps
Clojure tools not yet in expected location: /Users/borkdude/.deps.clj/1.11.1.1403/ClojureTools/clojure-tools-1.11.1.1403.jar
Downloading https://github.com/clojure/brew-install/releases/download/1.11.1.1403/clojure-tools.zip to /Users/borkdude/.deps.clj/1.11.1.1403/ClojureTools/clojure-tools.zip
Unzipping /Users/borkdude/.deps.clj/1.11.1.1403/ClojureTools/clojure-tools.zip ...
Successfully installed clojure tools!

@borkdude
Copy link
Owner

Ah I see, deps.clj is on 1403 but you have probably set DEPS_CLJ_TOOLS_VERSION to something newer?

@DerGuteMoritz
Copy link
Contributor Author

Oh yes, indeed! I'm using the version provided by nixpkgs which apparently always sets it to the current version of clojure. Sorry for the confusion! But it's a legit upstream bug anyway then, right?

@borkdude
Copy link
Owner

I'll make a new release with a fix.

@borkdude
Copy link
Owner

Note that this fix won't work until the next babashka release. So perhaps it's recommended to temporarily downgrade the tools jar version to 1403 again. Also I wouldn't automatically pick the newest version as the code might not be up to date with that.

@borkdude
Copy link
Owner

borkdude commented Sep 26, 2023

So tl;dr: it's better to not mess with the environment variable at all in nix CI. You can override it in user space.

@borkdude
Copy link
Owner

Fixed with f52034c.

I'll revert releasing since I need to fix another thing which I forgot in another issue.

@DerGuteMoritz
Copy link
Contributor Author

DerGuteMoritz commented Sep 26, 2023

Thanks! Note that the nixpkgs version of bb doesn't depend on the auto downloading feature by default, only when passing a custom DEPS_CLJ_TOOLS_DIR which doesn't yet have the files (like I did) would that code path be triggered. So the downgrade isn't strictly necessary but your point about sticking to the supported version makes sense. Will file a downstream issue for that!

DerGuteMoritz added a commit to DerGuteMoritz/nixpkgs that referenced this issue Sep 26, 2023
As recommended by @borkdude[1], we should use the Clojure tools version included in a particular
upstream release since the code might not always work with more recent versions.

[1]  borkdude/deps.clj#113 (comment)
DerGuteMoritz added a commit to DerGuteMoritz/nixpkgs that referenced this issue Sep 26, 2023
As recommended by @borkdude[1], we should use the Clojure tools version included in a particular
upstream release since the code might not always work with more recent versions.

[1]  borkdude/deps.clj#113 (comment)
DerGuteMoritz added a commit to DerGuteMoritz/nixpkgs that referenced this issue Oct 1, 2023
As recommended by @borkdude[1], we should use the Clojure tools version included in a particular
upstream release since the code might not always work with more recent versions.

[1]  borkdude/deps.clj#113 (comment)
DerGuteMoritz added a commit to DerGuteMoritz/nixpkgs that referenced this issue Oct 12, 2023
As recommended by @borkdude[1], we should use the Clojure tools version included in a particular
upstream release since the code might not always work with more recent versions.

[1]  borkdude/deps.clj#113 (comment)
bcdarwin pushed a commit to bcdarwin/nixpkgs that referenced this issue Oct 12, 2023
As recommended by @borkdude[1], we should use the Clojure tools version included in a particular
upstream release since the code might not always work with more recent versions.

[1]  borkdude/deps.clj#113 (comment)
ninjapanzer pushed a commit to ninjapanzer/nixpkgs that referenced this issue Oct 14, 2023
As recommended by @borkdude[1], we should use the Clojure tools version included in a particular
upstream release since the code might not always work with more recent versions.

[1]  borkdude/deps.clj#113 (comment)
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

No branches or pull requests

2 participants