[PATCH] D143344: [bazel] Port zstd support

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

GMNGeoffrey accepted this revision.
GMNGeoffrey added a comment.

Couple nits, but generally LGTM



================
Comment at: utils/bazel/WORKSPACE:126
+maybe(
+    http_archive,
+    name = "llvm_zstd",
----------------
Can we use `new_git_repository` here instead?


================
Comment at: utils/bazel/third_party_build/zstd.BUILD:17-20
+config_setting(
+    name = "llvm_zstd_disabled",
+    flag_values = {":llvm_enable_zstd": "false"},
+)
----------------
I think I commented on this in another change, but can we make this a positive setting rather than a negative? I think it's easier to grok if you don't have to apply a negation.


================
Comment at: utils/bazel/third_party_build/zstd.BUILD:53
+    }),
+    includes = ["lib"],
+)
----------------
We could try `strip_include_prefix` here. I think it avoids some of the problems with "includes" that make them pollute the whole dependency graph.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143344