Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BISON_PKGDATADIR breaks when using cmake() from rules_foreign_cc #17

Closed
mbland opened this issue Sep 10, 2024 · 11 comments
Closed

BISON_PKGDATADIR breaks when using cmake() from rules_foreign_cc #17

mbland opened this issue Sep 10, 2024 · 11 comments

Comments

@mbland
Copy link

mbland commented Sep 10, 2024

In Migrating to Bazel Modules (a.k.a. Bzlmod) - Repo Names, Macros, and Variables, I described a problem when using rules_bison with the cmake() rule from rules_foreign_cc.

Without defining a custom Make variable as described in that post, and using that to define BiSON_PKGDATADIR, the build will fail with:

[ ...snip... ]
[ 33%] [BISON][parser] Building parser with bison 3.3.2
bison: external/rules_bison~~bison_repository_ext~bison_v3.3.2__cfg00000B62/data/m4sugar/m4sugar.m4: cannot open: No such file or directory
make[2]: *** [CMakeFiles/repro.dir/build.make:74: rpcalc.c] Error 1
make[1]: *** [CMakeFiles/Makefile2:82: CMakeFiles/repro.dir/all] Error 2
make: *** [Makefile:91: all] Error 2
_____ END BUILD LOGS _____
[ ...snip... ]

The above output comes from a minimal repo I've created to reproduce the issue: mbland/rules-bison-pkgdatadir-repro


Filing at @jmillikin's request from a thread in the #bzlmod channel of the Bazel slack.

@jmillikin
Copy link
Owner

Some investigation notes:

The reason $(M4) doesn't work in the cmake(env={ ... }) dict but $(execpath @m4//bin:m4) does is because rules_foreign_cc special-cases environment variables starting with $(execpath and rewrites them to include the prefix $$EXT_BUILD_ROOT$$/. If I change the env= attribute to this:

    env = {
        "BISON_EXECUTABLE": "$$EXT_BUILD_ROOT$$/$(BISON)",
        "M4": "$$EXT_BUILD_ROOT$$/$(M4)",

Then the paths work, although obviously the ergonomics aren't great. Using $(execpath ___) is probably clearer.


The Bison data directory can't be found in the runfiles because the .runfiles directory doesn't exist:

$ bazel build //repro --sandbox_debug
BISON_EXECUTABLE=/private/var/tmp/_bazel_john/d1cfe8269f7676760279a7c91fc8010c/sandbox/darwin-sandbox/320/execroot/_main/bazel-out/darwin_arm64-opt-exec-ST-d57f47055a04/bin/external/rules_bison~~default_toolchain_ext~bison_v3.3.2/bin/bison
BISON_PKGDATADIR=/private/var/tmp/_bazel_john/d1cfe8269f7676760279a7c91fc8010c/sandbox/darwin-sandbox/320/execroot/_main/bazel-out/darwin_arm64-opt-exec-ST-d57f47055a04/bin/external/rules_bison~~default_toolchain_ext~bison_v3.3.2/bin/bison.runfiles/rules_bison~~default_toolchain_ext~bison_v3.3.2/data
[...]
$ ls /private/var/tmp/_bazel_john/d1cfe8269f7676760279a7c91fc8010c/sandbox/darwin-sandbox/320/execroot/_main/bazel-out/darwin_arm64-opt-exec-ST-d57f47055a04/bin/external/rules_bison~~default_toolchain_ext~bison_v3.3.2/bin/bison.runfiles
ls: /private/[...]/bison.runfiles: No such file or directory

$ ls /private/var/tmp/_bazel_john/d1cfe8269f7676760279a7c91fc8010c/sandbox/darwin-sandbox/320/execroot/_main/bazel-out/darwin_arm64-opt-exec-ST-d57f47055a04/bin/external/rules_bison~~default_toolchain_ext~bison_v3.3.2/bin/
bison

Looking into the cmake rule further, the bison executable is being passed as build_data=, but somewhere in cc_external_rule_impl the bison executable gets separated from its runfiles. The ctx.files.build_data attr does get passed to run_shell(tools=...), it just isn't having the expected effect.

The reason your override of BISON_PKGDATADIR works in combination with $(execpath) is because the latter is pulling from a different ~~*_ext~~ entirely, which does seem to have its runfiles set up as expected:

BISON_PKGDATADIR_2=/private/var/tmp/_bazel_john/d1cfe8269f7676760279a7c91fc8010c/sandbox/darwin-sandbox/321/execroot/_main/bazel-out/darwin_arm64-opt-exec-ST-d57f47055a04/bin/external/rules_bison~~bison_repository_ext~bison_v3.3.2__cfg00000B62/bin/bison.runfiles/rules_bison~~bison_repository_ext~bison_v3.3.2__cfg00000B62/data

I'm not sure how to begin fixing this, or even figure out which repository the fix(es) should be applied to. So for now, I'm going to investigate patching Bison's data lookup code so it incorporates some of the Bazel runfiles lookup logic.

@jmillikin
Copy link
Owner

@mbland With the change to patch Bison to use the Bazel runfiles support library, https://github.com/mbland/rules-bison-pkgdatadir-repro successfully builds with this env:

    env = {
        "BISON_EXECUTABLE": "$(execpath @bison//bin:bison)",
        "M4": "$(execpath @m4//bin:m4)",
    },

Could you try pointing your original repository at the current rules_bison HEAD and see if it works for you?

@mbland
Copy link
Author

mbland commented Oct 3, 2024

Just updated my MODULE.bazel with:

bazel_dep(name = "rules_bison", version = "0.2.2")
git_override(
    module_name = "rules_bison",
    remote = "https://github.com/jmillikin/rules_bison",
    commit="e8d151d3efdac2b1ff4208de81e3312729242f49",
)

Works like a champ, with both Bazel 7.3.2 and 8.0.0-pre.20240911.1! (I know the fix was in commit ce9ddfd, but HEAD is currently e8d151d.)

I'm happy to close this once there's a new release with the fix. (But it's your repo; close it now if you want.)

@jmillikin
Copy link
Owner

@mbland
Copy link
Author

mbland commented Oct 7, 2024

Thanks so much! When do you expect to post it to https://registry.bazel.build/modules/rules_bison?

@jmillikin
Copy link
Owner

The BCR PR is bazelbuild/bazel-central-registry#2904, which is blocked by issues with the BCR CI configuration for Java (bazelbuild/bazel-central-registry#2905).

According to bzlmod slack, the place to fix their CI would be somewhere in https://github.com/bazelbuild/continuous-integration/blob/master/buildkite/bazel-central-registry/bcr_presubmit.py, but I don't know how long that would take to review or what the release cadence is like for BCR container images.

I'll probably poke at bcr_presubmit.py some time this week, and if I can't get a fix in place soon-ish then I'll just remove @rules_bison//tests:HelloJavaMain.jar from the BCR presubmit targets list.

@mbland
Copy link
Author

mbland commented Oct 7, 2024

Thanks for the heads-up. Subscribed to both issues, as well as the corresponding #bzlmod Bazel Slack thread.

@jjmaestro
Copy link

jjmaestro commented Oct 8, 2024

@jmillikin just as a heads-up, I found this issue while trying to compile a project with rules_foreign_cc using meson, I've been getting the same error, even when using the latest version with git_override :-/ I've also tried @mbland's BISON_PKGDATADIR workaround but it doesn't work, I guess there's different logic in rules_foreign_cc for meson :-/

I've forked @mbland's repro and added a meson build that fails: jjmaestro/rules-bison-pkgdatadir-repro/tree/repro-meson:

$ bazel build //repro:repro_meson
(...)
Program bison found: YES (/private/var/tmp/_bazel_jjmaestro/56ba7e876133b2873efa8185951c7ba6/sandbox/darwin-sandbox/3/execroot/_main/bazel-out/darwin_arm64-opt-exec-ST-d57f47055a04/bin/external/rules_bison~~bison_repository_ext~bison_v3.3.2__cfg00000B62/bin/bison)
Build targets in project: 2

rpcalc undefined

  User defined options
    prefix: /private/var/tmp/_bazel_jjmaestro/56ba7e876133b2873efa8185951c7ba6/sandbox/darwin-sandbox/3/execroot/_main/bazel-out/darwin_arm64-fastbuild/bin/repro/repro_meson

Found ninja-1.12.1 at '/private/var/tmp/_bazel_jjmaestro/56ba7e876133b2873efa8185951c7ba6/sandbox/darwin-sandbox/3/execroot/_main/external/rules_foreign_cc~~tools~ninja_1.12.1_mac_aarch64/ninja'
[1/3] Generating rpcalc_parser with a custom command
FAILED: rpcalc.tab.c rpcalc.tab.h 
'/private/var/tmp/_bazel_jjmaestro/56ba7e876133b2873efa8185951c7ba6/sandbox/darwin-sandbox/3/execroot/_main/bazel-out/darwin_arm64-opt-exec-ST-d57f47055a04/bin/external/rules_bison~~bison_repository_ext~bison_v3.3.2__cfg00000B62/bin/bison' -d -o rpcalc.tab.c ../../../../../repro/rpcalc.y
bison: /private/var/tmp/_bazel_jjmaestro/56ba7e876133b2873efa8185951c7ba6/sandbox/darwin-sandbox/3/execroot/_main/bazel-out/darwin_arm64-opt-exec-ST-d57f47055a04/bin/external/rules_foreign_cc~/toolchains/private/meson_tool.runfiles/rules_bison~~bison_repository_ext~bison_v3.3.2__cfg00000B62/data/m4sugar/m4sugar.m4: cannot open: No such file or directory
ninja: build stopped: subcommand failed.

Meson.log

I'll try to see if I can find something in the rules_foreign_cc code that can help but I'd love if one of you could have a look, maybe you can come up with something... 🙏

Thanks!!

P.S. the failure was also present when using rules_foreign_cc's configure_make but the fix in the latest commit also fixed it, so it's only meson that's left! 😄

@jmillikin
Copy link
Owner

In that repro, the bison binary is being launched with RUNFILES_DIR set to the wrong directory. There's nothing that can be done about that from the rules_bison side -- you'll need to file an issue against rules_foreign_cc to adjust and/or clear RUNFILES_DIR if they're going to be running subprocesses from their custom-built tools.

The relevant code is https://github.com/bazelbuild/bazel/blob/7.3.2/tools/cpp/runfiles/runfiles_src.h#L54-L78

Another option is to add another layer of wrapping, such that meson invokes a bison shim that does this:

RUNFILES_DIR= exec path/to/actual/bison "$@"

argv[0]='/private/var/tmp/_bazel_john/bec6465c885719c412cb1f71a43706e5/sandbox/darwin-sandbox/264/execroot/_main/bazel-out/darwin_arm64-opt-exec-ST-d57f47055a04/bin/external/rules_bison~~bison_repository_ext~bison_v3.3.2__cfg00000B62/bin/bison'
RUNFILES_MANIFEST_FILE='(null)'
RUNFILES_DIR='/private/var/tmp/_bazel_john/bec6465c885719c412cb1f71a43706e5/sandbox/darwin-sandbox/264/execroot/_main/bazel-out/darwin_arm64-opt-exec-ST-d57f47055a04/bin/external/rules_foreign_cc~/toolchains/private/meson_tool.runfiles'
[...]
$ ls /private/var/tmp/_bazel_john/bec6465c885719c412cb1f71a43706e5/sandbox/darwin-sandbox/264/execroot/_main/bazel-out/darwin_arm64-opt-exec-ST-d57f47055a04/bin/external/rules_foreign_cc~/toolchains/private/meson_tool.runfiles
__init__.py						rules_foreign_cc~
_main							rules_foreign_cc~~tools~meson_src
_repo_mapping						rules_python~~python~python_3_12_aarch64-apple-darwin

@jjmaestro
Copy link

@jmillikin I see! Then, I'll go look at rules_foreign_cc and see if I can (1) create an issue and (2) create some temporary fix or something. Thanks a lot for your help!! 🙌

@jjmaestro
Copy link

jjmaestro commented Oct 8, 2024

@jmillikin looks like setting RUNFILES_DIR in the environment to the Bison runfiles dir works!

(...)
    env = select({
        "//conditions:default": {
            "PATH": ":".join([
                "$$(dirname $$EXT_BUILD_ROOT/$(execpath @python_3_12//:python3))",
                "$$(dirname $$EXT_BUILD_ROOT/$(execpath @bison//bin:bison))",
                "$$PATH",
            ]),
            "RUNFILES_DIR": "$$EXT_BUILD_ROOT/$(execpath @bison//bin:bison).runfiles/",
        },
    }),
(...)

I'm now getting other errors that I think I can fix to get the repro working! Plus I've tried this in the project I was working and it did compile! Thanks a ton!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants