[Bazel] Change external_zlib attribute to string

[mmcloughlin]

Summary

When using llvm_zlib_external rule with external_zlib attribute set to a
label referring to the main repository, like @//third_party/zlib, it will be
replaced with //third_party/zlib after template substitution. This will then
attempt to find //third_party/zlib within the local repository
@llvm_zlib//third_party/zlib, which does not exist, rather than the intended
reference back to the main repository. The issue appears to be that the
conversion of Label type to string with
str(repository_ctx.attr.external_zlib), which is causing the main repository
qualifier to be lost.

This diff fixes the issue by changing the external_zlib attribute to
attr.string type rather than attr.label.

In future a more elegant solution may be possible that preserves use of the
Label type, depending on resolution of the issue
https://github.com/bazelbuild/bazel/issues/13731.

Ported from Github PR https://github.com/google/llvm-bazel/pull/236.

[mmcloughlin]

@GMNGeoffrey The pre-merge checks failed:

https://buildkite.com/llvm-project/premerge-checks/builds/49068#1c2315ad-dc7b-41a5-bfda-57316dece5cd

This seems unrelated to this diff. Is there something I should do to fix it?

[GMNGeoffrey]

In D106606#2898356, @mmcloughlin wrote:

@GMNGeoffrey The pre-merge checks failed:

https://buildkite.com/llvm-project/premerge-checks/builds/49068#1c2315ad-dc7b-41a5-bfda-57316dece5cd

This seems unrelated to this diff. Is there something I should do to fix it?

WARNING There were changes that could not be mapped to a project. Checking everything
INFO    modified projects: ['clang', 'clang-tools-extra', 'compiler-rt', 'debuginfo-tests', 'flang', 'libc', 'libclc', 'libcxx', 'libcxxabi', 'libunwind', 'lld', 'lldb', 'mlir', 'openmp', 'parallel-libs', 'polly', 'pstl', 'llvm']
Traceback (most recent call last):
  File "/var/lib/buildkite-agent/builds/llvm-premerge-checks/scripts/pipeline_premerge.py", line 56, in <module>
    affected_projects = cp.get_affected_projects(modified_projects)
  File "/var/lib/buildkite-agent/builds/llvm-premerge-checks/scripts/choose_projects.py", line 163, in get_affected_projects
    logging.info(f'added {affected_projects - changed_projects} projects as they are affected')
TypeError: unsupported operand type(s) for -: 'set' and 'list'

So it looks to me like the logic fails in the case where no projects are affected and it somehow ends up passing in a list instead of a set for changed_projects. I traced the call hierarchy a bit and it looks like it gets set to a list here: https://github.com/google/llvm-premerge-checks/blob/75ca275b109e53cc268c8ada60970e780a320fcd/scripts/pipeline_premerge.py#L53

I'll file an issue/send a fix, but I think you should be fine to merge this. No premerge checks check anything about Bazel (yet) and merging this won't make the situation worse.

[mmcloughlin]

Thanks, that's what it looked like but I wanted to check. This is my first LLVM diff :)

[mmcloughlin]

Ah, it appears I cannot commit this myself. Are you able to do this for me?

https://llvm.org/docs/Phabricator.html#committing-a-change

[GMNGeoffrey]

Filed https://github.com/google/llvm-premerge-checks/issues/325 and sent a fix with https://github.com/google/llvm-premerge-checks/pull/326

I'll also talk to Mikhail about better project detection for utils. To start with we should probably just map it to a project that runs no tests to avoid wasteful testing of everything for these patches.

[GMNGeoffrey]

In D106606#2898396, @mmcloughlin wrote:

Ah, it appears I cannot commit this myself. Are you able to do this for me?

https://llvm.org/docs/Phabricator.html#committing-a-change

Ah yeah you have to have commit access (https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access if you're interested). I'll commit this for you :-)

[mmcloughlin]

Thank you!

utils/bazel/zlib.bzl

 setting will disable zlib the same as not setting it at all.
 """
 
 def _llvm_zlib_external_impl(repository_ctx):
     repository_ctx.template(
         "BUILD",
         repository_ctx.attr._external_build_template,
         substitutions = {
-            "@external_zlib_repo//:zlib_rule": str(repository_ctx.attr.external_zlib),
+            "@external_zlib_repo//:zlib_rule": repository_ctx.attr.external_zlib,
         },
         executable = False,
     )
 
 llvm_zlib_external = repository_rule(
     implementation = _llvm_zlib_external_impl,
     attrs = {
         "_external_build_template": attr.label(
             default = Label("//deps_impl:zlib_external.BUILD"),
             allow_single_file = True,
         ),
-        "external_zlib": attr.label(
+        "external_zlib": attr.string(
             doc = "The dependency that should be used for the external zlib library.",
             mandatory = True,
         ),
     },
 )
 
 def _llvm_zlib_system_impl(repository_ctx):
     repository_ctx.template(