Bazel: Might be worth removing dependency on rules_cc

[cpsauer]

Hey guys! First and foremost, thanks for a wonderful library.

I'm making use of it from bazel, and updating to 0.9.4, things broke with errors like ERROR: no such package '@rules_cc//cc/compiler': BUILD file not found in directory 'cc/compiler' of external repository @rules_cc. Add a BUILD file to a directory to mark it as a package.
(Copying that in so it's searchable by others.)

The issue is that some other (bazel first-party) repos (like skylib) bring in super old versions of rules_cc that don't have the compiler subdirectory, preempting this one's use of it. Most users, will, I think have some trouble tracking this down, since it's a bit tricky to figure out which transitive dependency calls pull it in. Plus, like rules_cc is explicitly not really intended for primetime use (per its readme). Those usages of it are, to the best of my knowledge, out-of-date holdovers from a time when bazel thought it was going to federate out more of its core.

I know that at some level that's not your problem. (And don't worry, I'll try to work to get those other repos fixed.) But I wonder if the most elegant solution might be to switch bazel/copts.bzl back to referencing the equivalent @bazel_tools//tools/cpp:(compiler) values rather than @rules_cc//cc/compiler, eliminating the unnecessary (and problematic) dependency on rules_cc. (Alternatively, we could add a transitive dependencies macro to bring in the rules_cc dependency for WORKSPACE users, but I think it's very likely to have been already shadowed by the time people use this.) What do you think? Tagging @zaucy, since I know he's been working on improvements here. Tagging also @fmeum, since I saw he'd given the feedback, and I've had lots of great interactions with him in the past :)

Thanks for your consideration,
Chris

[zaucy]

Plus, like rules_cc is explicitly not really intended for primetime use (per its readme)

I don't think this is the case anymore. That part of the README hasn't been updated since 2019 and the rules are mentioned/used throughout the bazel.build documentation. Although I will admit there are open/closed issues in rules_cc that make this less clear.

The issue is that some other (bazel first-party) repos (like skylib) bring in super old versions of rules_cc that don't have the compiler subdirectory, preempting this one's use of it.

I think the best option is to start using bzlmod to use magic_enum as a bazel dependency. As this is a problem that bzlmod was designed to fix. I was going to edit the README after I got magic_enum published on the BCR and I could mention the rules_cc problem for WORKSPACE users.

[fmeum]

As far as I can tell rules_cc is safe to use for rules, just its toolchain definitions are outdated (but usually not registered). Hopefully that can be fixed soon as it has been for rules_java.

[cpsauer]

Original take:
I hear ya on bzlmod, but don't we think it's going to be a while before all users are switched over from the WORKSPACE? I feel like the bazel dependencies aren't getting us too much here; they're easy to eliminate (and it's work to keep them up to date; they're already outdated).

After a little more noodling:
I'm also realizing that these copts don't actually do anything in the released library (since there are no compilation actions on a header-only library and they're not propagated). So I think we should at least move them to be test-only to avoid propagating dependencies we don't need, and sidestep this issue. I'm also noticing that (on macOS, where I'm testing atm) we don't hit any of these conditions anyway, so it's good we have that default condition.

To make it concrete: #312 is a change that I think preserves the behavior, while eliminating those dependencies and cleaning the COPTs out from where they're not used. It also includes #311 (since bazel tests aren't otherwise building right now) and does some minor merging/reordering for clarity. @fmeum, I'd love to check against your better understanding that these are the equivalent compiler conditions (since again, none were triggering on Mac, before or after the change). And if you all think we should really re-add the bzlmod dependencies inside the test workspace/module, go for it. I'll give you guys write access to the PR's fork.

[zaucy]

I'm also realizing that these copts don't actually do anything in the released library

That's true. If someone is using magic_enum they should have configured c++17 or higher flags anyways. Good point.

[Neargye]

@cpsauer Could you look at the problem with the bazel build? I can’t figure out what the problem is, it seems something has changed in bazel

[zaucy]

its because bazel updated and bazelisk isn't reading the .bazelversion for some reason in the CI. I had to deal with it it in the EnTT repository here as well: skypjack/entt#1098

if someone else doesn't get to it then I can late tomorrow!

[cpsauer]

Sorry--just seeing. A bit backlogged over here.
Looks like we can probably just strip the unrecognized flag since it's a completed migration? I'll toss up a quick PR doing that. #325

[cpsauer]

@zaucy: Any idea why bazelisk isn't reading the .bazelversion? Very weird. Could you take point on that since it sounds like you've been working with already and would have the advantage? Would love to know what you learn, though. (If we get this fixed, maybe we should enable Renovate so there are auto-PRs keeping bazel up to date?)

[cpsauer]

Oh, um, I bet its because it's not finding a .bazelversion file in the test directory even though, per their README it should find the parent...

IMO we should probably add Renovate and add a .bazelversion to /test, so we're both up to date and fully reproducible by default (including rules_cc in tests) and never have a Bazel based breakage. @Neargye would have to add the Renovate app to the repo. Otherwise I think the move would be to strip the .bazelversion file and at least be up to date by default, figuring that's simpler and that bazel breakages will be rare.

[cpsauer]

Looks like that diagnosis was right: bzlmod's parent directory inheritance isn't cross workspace boundaries, despite the documented behavior being otherwise. I've reported in bazelbuild/bazelisk#523. y'all might want to follow that issue

@Neargye, if you're down to enable Renovate on this repo, would you? (happy to help configure, but I think only you can add Github Apps.) And if not, would you go ahead and delete .bazelversion? (reasoning above)