[bazel] Remove terminfo dependency

[aaronmondal]

Summary

The only enabling configuration of this is irreproducible. Since
terminfo doesn't provide essential functionality, remove it so that all
external dependencies now follow the same config_setting pattern.

This should be an NFC for most setups and all CI setups.

[matts1]

You say that it doesn't provide essential functionality. Doesn't that mean it should be an optional dep, perhaps defaulting to disabled, rather than deleting terminfo entirely?

[MaskRay]

terminfo is used by a few tools such as lldb and clang-repl. The most significant user is lldb, which Bazel doesn't have a port yet. For the other tools, I agree that they "doesn't provide essential functionality,"

When we add lldb, the dependency will be significant...

[aaronmondal]

You say that it doesn't provide essential functionality. Doesn't that mean it should be an optional dep, perhaps defaulting to disabled, rather than deleting terminfo entirely?

For llvm/Support/Unix/Process.inc I question whether terminfo is necessary at all. That code looks hacky in the first place and we already have fallbacks to TERM that I'd imagine to basically accomplish the same thing, but without locks and extern C-to-avoid-searching-for-headers.

terminfo is used by a few tools such as lldb and clang-repl. The most significant user is lldb, which Bazel doesn't have a port yet. For the other tools, I agree that they "doesn't provide essential functionality,"

Hmm we'd definitely want lldb in the overlay at some point. The current implementation of the terminfo dep in Bazel isn't reproducible though. We'd need an "external" terminfo/curses for lldb. The current build files only support "disable" and "system" configurations.

AFAICS one would usually build ncurses to get terminfo, right? terminfo in lldb is also only relevant if curses is present. Would it make sense to create an ncurses build file instead of the terminfo checks?

[MaskRay]

Removing terminfo seems fine. When we need it, we can do llvm_zlib and llvm_zstd style dependencies...

[phosek]

LGTM

[goncharov]

Hi! Just FYI "bazel build" tag was created so that interested people will run bazel build as part of their pre-merge check, not in reviewing changes to bazel. It's also quite large.

[gchatelet]

In D151006#4370963, @goncharov wrote:

Hi! Just FYI "bazel build" tag was created so that interested people will run bazel build as part of their pre-merge check, not in reviewing changes to bazel. It's also quite large.

Given the interest in keeping up with bazel related changes I think it would be nice to add a "bazel change" tag. WDYT?

[aaronmondal]

In D151006#4427484, @gchatelet wrote:

In D151006#4370963, @goncharov wrote:

Hi! Just FYI "bazel build" tag was created so that interested people will run bazel build as part of their pre-merge check, not in reviewing changes to bazel. It's also quite large.

Given the interest in keeping up with bazel related changes I think it would be nice to add a "bazel change" tag. WDYT?

I'd also appreciate that. The "upkeep/fix" changes are probably not too interesting, but it would be very nice to have a tag for actual build logic changes such as this commit.

utils/bazel/WORKSPACE

new_local_repository(
     build_file_content = "# empty",
     path = "../../",
 )
 
 load("@llvm-raw//utils/bazel:configure.bzl", "llvm_configure")
 
 llvm_configure(name = "llvm-project")
 
-load("@llvm-raw//utils/bazel:terminfo.bzl", "llvm_terminfo_from_env")
-
-maybe(
-    llvm_terminfo_from_env,
-    name = "llvm_terminfo",
-)
-
 maybe(
     http_archive,
     name = "llvm_zlib",
     build_file = "@llvm-raw//utils/bazel/third_party_build:zlib-ng.BUILD",
     sha256 = "e36bb346c00472a1f9ff2a0a4643e590a254be6379da7cddd9daeb9a7f296731",
     strip_prefix = "zlib-ng-2.0.7",
     urls = [
         "https://github.com/zlib-ng/zlib-ng/archive/refs/tags/2.0.7.zip",

utils/bazel/configure.bzl

 # This file is licensed under the Apache License v2.0 with LLVM Exceptions.
 # See https://llvm.org/LICENSE.txt for license information.
 # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
 """Helper macros to configure the LLVM overlay project."""
 
 load("@bazel_tools//tools/build_defs/repo:utils.bzl", "maybe")
-load(":terminfo.bzl", "llvm_terminfo_disable", "llvm_terminfo_system")
 
 # Directory of overlay files relative to WORKSPACE
 DEFAULT_OVERLAY_PATH = "llvm-project-overlay"
 
 DEFAULT_TARGETS = [
     "AArch64",
     "AMDGPU",
     "ARM",

 llvm_configure = repository_rule(
     implementation = _llvm_configure_impl,
     local = True,
     configure = True,
     attrs = {
         "targets": attr.string_list(default = DEFAULT_TARGETS),
     },
 )
-
-def llvm_disable_optional_support_deps():
-    maybe(
-        llvm_terminfo_disable,
-        name = "llvm_terminfo",
-    )
-
-def llvm_use_system_support_deps():
-    maybe(
-        llvm_terminfo_system,
-        name = "llvm_terminfo",
-    )

utils/bazel/deps_impl/BUILD.bazel

-# This file is licensed under the Apache License v2.0 with LLVM Exceptions.
-# See https://llvm.org/LICENSE.txt for license information.
-# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-
-# Required to reference files in this package

utils/bazel/deps_impl/terminfo_disable.BUILD

-# This file is licensed under the Apache License v2.0 with LLVM Exceptions.
-# See https://llvm.org/LICENSE.txt for license information.
-# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-
-# Empty stub library. This doesn't include any terminfo library and doesn't set
-# the LLVM `#define`s to enable usage of terminfo.
-cc_library(
-    name = "terminfo",
-    visibility = ["//visibility:public"],
-)

utils/bazel/deps_impl/terminfo_system.BUILD

-# This file is licensed under the Apache License v2.0 with LLVM Exceptions.
-# See https://llvm.org/LICENSE.txt for license information.
-# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-
-# Wrapper library for some system terminfo. Using this only works if the
-# toolchain already has the relevant library search paths configured. It also
-# sets the relevant LLVM `#define`s to enoble using terminfo.
-cc_library(
-    name = "terminfo",
-    defines = ["LLVM_ENABLE_TERMINFO=1"],
-    # Note that we will replace these link options with ones needed to
-    # effectively link against a terminfo providing library on the system.
-    linkopts = {TERMINFO_LINKOPTS},
-    visibility = ["//visibility:public"],
-)

utils/bazel/deps_impl/terminfo_test.c

-/*
-This file is licensed under the Apache License v2.0 with LLVM Exceptions.
-See https://llvm.org/LICENSE.txt for license information.
-SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-*/
-
-extern int setupterm(char *term, int filedes, int *errret);
-extern struct term *set_curterm(struct term *termp);
-extern int del_curterm(struct term *termp);
-extern int tigetnum(char *capname);
-
-int main() {
-  setupterm(0, 0, 0);
-  set_curterm(0);
-  del_curterm(0);
-  tigetnum(0);
-}

utils/bazel/examples/http_archive/WORKSPACE

http_archive(
     sha256 = LLVM_SHA256,
     strip_prefix = "llvm-project-" + LLVM_COMMIT,
     urls = ["https://github.com/llvm/llvm-project/archive/{commit}.tar.gz".format(commit = LLVM_COMMIT)],
 )
 
 load("@llvm-raw//utils/bazel:configure.bzl", "llvm_configure", "llvm_disable_optional_support_deps")
 
 llvm_configure(name = "llvm-project")
-
-# Disables optional dependencies for Support like zlib and terminfo. You may
-# instead want to configure them using the macros in the corresponding bzl
-# files.
-llvm_disable_optional_support_deps()

utils/bazel/examples/submodule/WORKSPACE

new_local_repository(
     build_file_content = "# empty",
     # Or wherever your submodule is located.
     path = "third_party/llvm-project",
 )
 
 load("@llvm-raw//utils/bazel:configure.bzl", "llvm_configure", "llvm_disable_optional_support_deps")
 
 llvm_configure(name = "llvm-project")
-
-# Disables optional dependencies for Support like zlib and terminfo. You may
-# instead want to configure them using the macros in the corresponding bzl
-# files.
-llvm_disable_optional_support_deps()

utils/bazel/llvm-project-overlay/llvm/BUILD.bazel

linkopts = select({
         ],
     }),
     textual_hdrs = glob([
         "include/llvm/Support/*.def",
     ]),
     deps = [
         ":config",
         ":Demangle",
-        # We unconditionally depend on the custom LLVM terminfo wrapper. This
-        # will be an empty library unless terminfo is enabled, in which case it
-        # will both provide the necessary dependencies and configuration
-        # defines.
-        "@llvm_terminfo//:terminfo",
         # We unconditionally depend on the custom LLVM zlib wrapper. This will
         # be an empty library unless zlib is enabled, in which case it will
         # both provide the necessary dependencies and configuration defines.
         "@llvm_zlib//:zlib",
         # We unconditionally depend on the custom LLVM zstd wrapper. This will
         # be an empty library unless zstd is enabled, in which case it will
         # both provide the necessary dependencies and configuration defines.
         "@llvm_zstd//:zstd",

utils/bazel/terminfo.bzl

-# This file is licensed under the Apache License v2.0 with LLVM Exceptions.
-# See https://llvm.org/LICENSE.txt for license information.
-# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-
-"""Repository rules to configure the terminfo used by LLVM.
-
-Most users should pick one of the explicit rules to configure their use of terminfo
-with LLVM:
-- `llvm_terminfo_system` will detect and link against a terminfo-implementing
-  system library (non-hermetically).
-- 'llvm_terminfo_disable` will disable terminfo completely.
-
-If you would like to make your build configurable, you can use
-`llvm_terminfo_from_env`. By default, this will disable terminfo, but will
-inspect the environment variable (most easily set with a `--repo_env` flag to
-the Bazel invocation) `BAZEL_LLVM_TERMINFO_STRATEGY`. If it is set to
-`system` then it will behave the same as `llvm_terminfo_system`. Any other
-setting will disable terminfo the same as not setting it at all.
-"""
-
-def _llvm_terminfo_disable_impl(repository_ctx):
-    repository_ctx.template(
-        "BUILD",
-        repository_ctx.attr._disable_build_template,
-        executable = False,
-    )
-
-_terminfo_disable_attrs = {
-    "_disable_build_template": attr.label(
-        default = "@llvm-raw//utils/bazel/deps_impl:terminfo_disable.BUILD",
-        allow_single_file = True,
-    ),
-}
-
-llvm_terminfo_disable = repository_rule(
-    implementation = _llvm_terminfo_disable_impl,
-    attrs = _terminfo_disable_attrs,
-)
-
-def _find_c_compiler(repository_ctx):
-    """Returns the path to a plausible C compiler.
-
-    This routine will only reliably work on roughly POSIX-y systems as it
-    ultimately falls back on the `cc` binary. Fortunately, the thing we are
-    trying to use it for (detecting if a trivial source file can compile and
-    link against a particular library) requires very little.
-    """
-    cc_env = repository_ctx.os.environ.get("CC")
-    cc = None
-    if cc_env:
-        if "/" in cc_env:
-            return repository_ctx.path(cc_env)
-        else:
-            return repository_ctx.which(cc_env)
-
-    # Look for Clang, GCC, and the POSIX / UNIX specified C compiler
-    # binaries.
-    for compiler in ["clang", "gcc", "c99", "c89", "cc"]:
-        cc = repository_ctx.which(compiler)
-        if cc:
-            return cc
-
-    return None
-
-def _try_link(repository_ctx, cc, source, linker_flags):
-    """Returns `True` if able to link the source with the linker flag.
-
-    Given a source file that contains references to library routines, this
-    will check that when linked with the provided linker flag, those
-    references are successfully resolved. This routine assumes a generally
-    POSIX-y and GCC-ish compiler and environment and shouldn't be expected to
-    work outside of that.
-    """
-    cmd = [
-        cc,
-        # Force discard the linked executable.
-        "-o",
-        "/dev/null",
-        # Leave language detection to the compiler.
-        source,
-    ]
-
-    # The linker flag must be valid for a compiler invocation of the link step,
-    # so just append them to the command.
-    cmd += linker_flags
-    exec_result = repository_ctx.execute(cmd, timeout = 20)
-    return exec_result.return_code == 0
-
-def _llvm_terminfo_system_impl(repository_ctx):
-    # LLVM doesn't need terminfo support on Windows, so just disable it.
-    if repository_ctx.os.name.lower().find("windows") != -1:
-        _llvm_terminfo_disable_impl(repository_ctx)
-        return
-
-    if len(repository_ctx.attr.system_linkopts) > 0:
-        linkopts = repository_ctx.attr.system_linkopts
-    else:
-        required = repository_ctx.attr.system_required
-
-        # Find a C compiler we can use to detect viable linkopts on this system.
-        cc = _find_c_compiler(repository_ctx)
-        if not cc:
-            if required:
-                fail("Failed  to find a C compiler executable")
-            else:
-                _llvm_terminfo_disable_impl(repository_ctx)
-                return
-
-        # Get the source file we use to detect successful linking of terminfo.
-        source = repository_ctx.path(repository_ctx.attr._terminfo_test_source)
-
-        # Collect the candidate linkopts and wrap them into a list. Ideally,
-        # these would be provided as lists, but Bazel doesn't currently
-        # support that. See: https://github.com/bazelbuild/bazel/issues/12178
-        linkopts_candidates = [[x] for x in repository_ctx.attr.candidate_system_linkopts]
-        linkopts = None
-
-        # For each candidate, try to use it to link our test source file.
-        for linkopts_candidate in linkopts_candidates:
-            if _try_link(repository_ctx, cc, source, linkopts_candidate):
-                linkopts = linkopts_candidate
-                break
-
-        # If we never found a viable linkopts candidate, either error or disable
-        # terminfo for LLVM.
-        if not linkopts:
-            if required:
-                fail("Failed to detect which linkopt would successfully provide the " +
-                     "necessary terminfo functionality")
-            else:
-                _llvm_terminfo_disable_impl(repository_ctx)
-                return
-
-    repository_ctx.template(
-        "BUILD",
-        repository_ctx.attr._system_build_template,
-        substitutions = {
-            "{TERMINFO_LINKOPTS}": str(linkopts),
-        },
-        executable = False,
-    )
-
-def _merge_attrs(attrs_list):
-    attrs = {}
-    for input_attrs in attrs_list:
-        attrs.update(input_attrs)
-    return attrs
-
-_terminfo_system_attrs = _merge_attrs([_terminfo_disable_attrs, {
-    "_system_build_template": attr.label(
-        default = "@llvm-raw//utils/bazel/deps_impl:terminfo_system.BUILD",
-        allow_single_file = True,
-    ),
-    "_terminfo_test_source": attr.label(
-        default = "@llvm-raw//utils/bazel/deps_impl:terminfo_test.c",
-        allow_single_file = True,
-    ),
-    "candidate_system_linkopts": attr.string_list(
-        default = [
-            "-lterminfo",
-            "-ltinfo",
-            "-lcurses",
-            "-lncurses",
-            "-lncursesw",
-        ],
-        doc = "Candidate linkopts to test and see if they can link " +
-              "successfully.",
-    ),
-    "system_required": attr.bool(
-        default = False,
-        doc = "Require that one of the candidates is detected successfully on POSIX platforms where it is needed.",
-    ),
-    "system_linkopts": attr.string_list(
-        default = [],
-        doc = "If non-empty, a specific array of linkopts to use to " +
-              "successfully link against the terminfo library. No " +
-              "detection is performed if this option is provided, it " +
-              "directly forces the use of these link options. No test is " +
-              "run to determine if they are valid or work correctly either.",
-    ),
-}])
-
-llvm_terminfo_system = repository_rule(
-    implementation = _llvm_terminfo_system_impl,
-    configure = True,
-    local = True,
-    attrs = _terminfo_system_attrs,
-)
-
-def _llvm_terminfo_from_env_impl(repository_ctx):
-    terminfo_strategy = repository_ctx.os.environ.get("BAZEL_LLVM_TERMINFO_STRATEGY")
-    if terminfo_strategy == "system":
-        _llvm_terminfo_system_impl(repository_ctx)
-    else:
-        _llvm_terminfo_disable_impl(repository_ctx)
-
-llvm_terminfo_from_env = repository_rule(
-    implementation = _llvm_terminfo_from_env_impl,
-    configure = True,
-    local = True,
-    attrs = _merge_attrs([_terminfo_disable_attrs, _terminfo_system_attrs]),
-    environ = ["BAZEL_LLVM_TERMINFO_STRATEGY", "CC"],
-)