[PATCH] D143344: [bazel] Port zstd support

[Aaron Siddhartha Mondal via Phabricator via llvm-commits]

aaronmondal added a comment.

@GMNGeoffrey Unfortunately I think reverting might be better after all. Making this work with WORKSPACES requires wayy too much boilerplate. Same issue for D143320 <https://reviews.llvm.org/D143320>.

Adding the `http_archive` somewhere with a `maybe` will break bzlmod builds in the future as module extensions intentionally don't support `maybe`. The current implementation doesn't work well with WORKSPACEs, so that's also not good. Using `config_setting`s seems like a good way in general, but they probably should be in the `llvm/BUILD.bazel` and not in `third_party/zstd.BUILD`. After playing around with this for way too long I'm now concluding that having the `select`s in the llvm BUILD files might be the best way to go.

I also think we need to have a clear way of how external deps in general should be handled. I think we *either* need to enforce configuration via `.bazelrc` *or* configuration via the `llvm_configure` rule. I'm leaning towards handling it in `llvm_configure`, but then we'll need to change the attributes of that rule.

Maybe we should set a "best effort" goal of replicating the CMake configuration flags in `llvm_configure` and additional bazel specific flags in `.bazelrc`. This way we could stay close to the CMake build, let downstream users to declare a specific configuration via `llvm_configure` and let users that depend on bazel-specific config options (e.g. custom dep overrides) to do this via `.bazelrc`.

Overall there are just too many things problematic with the current state of this patch to roll forward.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143344