bazel-contrib / rules_foreign_cc

Build rules for interfacing with "foreign" (non-Bazel) build systems (CMake, configure-make, GNU Make, boost, ninja, Meson)
https://bazel-contrib.github.io/rules_foreign_cc
Apache License 2.0
679 stars 248 forks source link

tools from the `tools_deps` attribute in `configure_make` are not made available anymore in 0.10.0 #1115

Open sitaktif opened 1 year ago

sitaktif commented 1 year ago

On version 0.9.0, tools present in the tools_deps attribute were made available to the configure script, but as of 0.10.0, they are not anymore.

The issue has been pinned down to these two commits:

To reproduce:

# Check out a commit that surfaces the issue
git checkout 0ed27c13b18f412e00e9122fc01327503d52579c

# Modify an example to surface regression
patch -p1 < 'EOF'
diff --git a/examples/WORKSPACE.bazel b/examples/WORKSPACE.bazel
index 481353e..ea151ed 100644
--- a/examples/WORKSPACE.bazel
+++ b/examples/WORKSPACE.bazel
@@ -17,9 +17,9 @@ load("//deps:repositories.bzl", "repositories")

 repositories()

-load("//deps:deps_android.bzl", "deps_android")
-
-deps_android()
+# load("//deps:deps_android.bzl", "deps_android")
+#
+# deps_android()

 load("//deps:deps_jvm_external.bzl", "deps_jvm_external")

diff --git a/examples/configure_with_bazel_transitive/BUILD.bazel b/examples/configure_with_bazel_transitive/BUILD.bazel
index ba364c2..31f787d 100644
--- a/examples/configure_with_bazel_transitive/BUILD.bazel
+++ b/examples/configure_with_bazel_transitive/BUILD.bazel
@@ -8,6 +8,13 @@ cc_library(
     includes = ["."],
 )

+genrule(
+    name = "mytool.rule",
+    outs = ["mytool"],
+    cmd = "echo '#!/bin/sh\n\necho Seargent Tom to Ground Control' > $@",
+    executable = True,
+)
+
 configure_make(
     name = "simple",
     configure_in_place = True,
@@ -16,6 +23,7 @@ configure_make(
         "simple",
         "install",
     ],
+    tools_deps = [":mytool"],
     deps = [":built_with_bazel"],
 )

diff --git a/examples/configure_with_bazel_transitive/simple_lib/configure b/examples/configure_with_bazel_transitive/simple_lib/configure
index 52db02d..50b27fc 100755
--- a/examples/configure_with_bazel_transitive/simple_lib/configure
+++ b/examples/configure_with_bazel_transitive/simple_lib/configure
@@ -1,4 +1,14 @@
 #!/usr/bin/env bash
+
+if ! echo $PATH | grep mytool; then
+    echo "ERROR: mytool is not present in the PATH"
+else
+    echo "OK: mytool is present in the PATH"
+fi
+
+mytool || exit 3
+
+
 echo "# simple" > Makefile

 echo "CC = ${CC}" > Makefile
EOF

# Build the relevant target. Notice that the target fails to build.
bazel build //configure_with_bazel_transitive:simple -s --sandbox_debug

# Check out grand-parent commit
git stash
git checkout HEAD^^
git stash pop

# Build the relevant target. Notice that building the target succeeds.
bazel build //configure_with_bazel_transitive:simple -s --sandbox_debug
sitaktif commented 1 year ago

Digging up to figure out the issue, I saw that there was a deprecation warning buried in the logs:

DEBUG: /private/var/tmp/_bazel_rchossart/94f88902fcd7029b431367a0b8cc4cbb/external/rules_foreign_cc/foreign_cc/private/framework.bzl:232:14: @//configure_with_bazel_transitive:simple Attribute `tools_deps` is deprecated. Please use `build_data`.

It seems to me that configure_make should fail(...) explicitly when the attribute is used instead of just printing. wdyt?

jsharpe commented 1 year ago

I'm not sure that the removal of the deprecated field was actually intentional. Happy to take a PR which either restores it or switches to fail()