Skip to content

[bazel] Clean up unused exported files #114620

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

Merged
merged 1 commit into from
Nov 4, 2024
Merged

Conversation

thevinster
Copy link
Contributor

Some minor cleanup to the bazel files. These exported files are not being referenced anymore in the tree, afaict, so let's clean them up. Additionally, moved one of the filegroup targets higher to be consistent with other filegroup usages where it's defined before first usage.

@llvmbot llvmbot added the bazel "Peripheral" support tier build system: utils/bazel label Nov 1, 2024
@thevinster thevinster merged commit bb9ff32 into llvm:main Nov 4, 2024
8 checks passed
@thevinster thevinster deleted the cleanup branch November 4, 2024 23:27
@pranavk
Copy link
Contributor

pranavk commented Nov 5, 2024

This is causing problems in bazel build:

ERROR: /var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/3f9470ef82c49ef29515926500bf249a/external/llvm-project/lldb/BUILD.bazel:196:8: in genrule rule @@llvm-project//lldb:lldb-sbapi-dwarf-enums: target '@@llvm-project//llvm:include/llvm/BinaryFormat/Dwarf.def' is not visible from target '@@llvm-project//lldb:lldb-sbapi-dwarf-enums'. Check the visibility declaration of the former target if you think the dependency is legitimate. To set the visibility of that source file target, use the exports_files() function

pranavk added a commit to pranavk/llvm-project that referenced this pull request Nov 5, 2024
pranavk added a commit that referenced this pull request Nov 5, 2024
@thevinster
Copy link
Contributor Author

Sorry about the breakages. I was almost sure that this wouldn't break things, and I failed to see it being used in lldb. Thanks for fixing!

@rupprecht
Copy link
Collaborator

Sorry for not getting to this review earlier. I would have approved it though. Thanks @pranavk for the build fix!

Files often end up in exports_files because some other package needs it, and I'd wager it's often the case due to uses only outside of the LLVM tree. So removing things from exports_files just because there are no in tree uses is often going to break things.

That said, it's trivial to just add them back, preferably w/ a comment explaining that it's needed outside of the tree, so removing entries LGTM.

@pranavk
Copy link
Contributor

pranavk commented Nov 5, 2024

yeah, no worries. this one was easy to fix.

PhilippRados pushed a commit to PhilippRados/llvm-project that referenced this pull request Nov 6, 2024
Some minor cleanup to the bazel files. These exported files are not
being referenced anymore in the tree, afaict, so let's clean them up.
Additionally, moved one of the filegroup targets higher to be consistent
with other filegroup usages where it's defined before first usage.
PhilippRados pushed a commit to PhilippRados/llvm-project that referenced this pull request Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bazel "Peripheral" support tier build system: utils/bazel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants