[PATCH] D143344: [bazel] Port zstd support

[Geoffrey Martin-Noble via Phabricator via llvm-commits]

GMNGeoffrey accepted this revision.
GMNGeoffrey added inline comments.


================
Comment at: utils/bazel/WORKSPACE:126
+maybe(
+    http_archive,
+    name = "llvm_zstd",
----------------
aaronmondal wrote:
> GMNGeoffrey wrote:
> > Can we use `new_git_repository` here instead?
> This was because of the hash failure for archives with github right? I'm not sure whether `new_git_repository` is a good idea here since we are using a stable archive (which AFAIK is guaranteed to be hash-stable). At least according to the bazel docs 
> 
> > Prefer http_archive to git_repository. The reasons are:
> 
>   - Git repository rules depend on system git(1) whereas the HTTP downloader is built into Bazel and has no system dependencies.
>   - http_archive supports a list of urls as mirrors, and git_repository supports only a single remote.
>   - http_archive works with the repository cache, but not git_repository. See #5116 for more information.
> 
> I'm only really worried about the third point here. You know the best practices better though, so I'm happy to change this if you prefer `new_git_repository`.
Oh I actually didn't know about that third point. Yeah that sucks. You make good points that this is pretty stable. I guess I'm used to frequently-updating dependencies like LLVM :-) Let's keep it as `http_archive`.

An aside on hash stability:

Those archives are only sort-of guaranteed to be hash stable because everyone depends on that. gzip doesn't actually guarantee hash-stability by the spec and neither does git guarantee `git archive` stability. It recently changed the algorithm in a way that changed the hash. That broke the world when GitHub updated its git version: https://github.blog/2023-02-21-update-on-the-future-stability-of-source-code-archives-and-hashes. It seems that GitHub had a guarantee that "release assets" would be hash-stable and many people interpreted that to include the "source archive" link present on the release page (right next to the things manually uploaded), but GitHub didn't. Separately, someone from Bazel had received a private commitment from support that those would remain stable, but obviously that commitment didn't involve anyone who actually worked on the relevant code :-D Anyway it was a fun dumpster fire, but I still think that it would be better if Bazel provided a way that didn't rely on these unstable hashes. Probably using the GitHub API with the ref arg, as suggested in those GitHub docs, would provide a way to at least have a `new_github_repository` rule. Anyway, this is all fine for now :-)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143344/new/

https://reviews.llvm.org/D143344