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

Added external_include paths to system includes switch #6284

Closed
wants to merge 2 commits into from

Conversation

LeFrosch
Copy link
Collaborator

Checklist

  • I have filed an issue about this change and discussed potential changes with the maintainers.
  • I have received the approval from the maintainers to make this change.
  • This is not a stylistic, refactoring, or cleanup change.

Please note that the maintainers will not be reviewing this change until all checkboxes are ticked. See
the Contributions section in the README for more
details.

Discussion thread for this change

Issue number: #4980

Description of this change

Support for external_includes. Fixes #4980. When --features=external_include_paths is specified includes are added to compilation_context.external_includes and no longer to compilation_context.includes.

@LeFrosch LeFrosch marked this pull request as ready for review March 18, 2024 13:27
@github-actions github-actions bot added product: CLion CLion plugin product: IntelliJ IntelliJ plugin product: GoLand GoLand plugin awaiting-review Awaiting review from Bazel team on PRs labels Mar 18, 2024
Copy link
Collaborator

@blorente blorente left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The general idea looks good, but could you add tests? Ideally both aspect tests aspect/ and an example project in examples.

@LeFrosch
Copy link
Collaborator Author

I added an example project. But I am not sure how to write a test for this because I need to enable the external_include_paths feature and I need an external repository.
Also I noticed that the external_include_paths feature is a bit unreliably because on Linux I can build and run the example project. However, on mac this project fails to build.

@blorente
Copy link
Collaborator

I think the reason the project fails to build on mac is because bazel passes the include directories using -iquote, which means I had to import "simple.h" instead of <simple.h>. Leaving a diff with some changes, feel free to apply as much of it as you want:

diff --git a/examples/cpp/external_includes/project/BUILD b/examples/cpp/external_includes/project/BUILD
index 048db18ecb..cd2df3aeef 100644
--- a/examples/cpp/external_includes/project/BUILD
+++ b/examples/cpp/external_includes/project/BUILD
@@ -1,5 +1,5 @@
 cc_binary(
-    name = "simple",
-    srcs = ["simple.cc"],
-	deps = ["@lib//:simple"],
-)
\ No newline at end of file
+    name = "main",
+    srcs = ["main.cc"],
+    deps = ["@lib//:simple"],
+)
diff --git a/examples/cpp/external_includes/project/simple.cc b/examples/cpp/external_includes/project/main.cc
similarity index 69%
rename from examples/cpp/external_includes/project/simple.cc
rename to examples/cpp/external_includes/project/main.cc
index 3982b98220..4bc95f7584 100644
--- a/examples/cpp/external_includes/project/simple.cc
+++ b/examples/cpp/external_includes/project/main.cc
@@ -1,4 +1,4 @@
-#include <simple.h>
+#include "simple.h"
 
 int main() {
 	simple_header_fun();

@blorente
Copy link
Collaborator

But I am not sure how to write a test for this because I need to enable the external_include_paths feature and I need an external repository.

The process can get a little complicated for sure, but there is a way. We can:

  • Create a repository_rule that writes the contents of the @lib// example into an external repository.
  • Call that repository rule from the WORKSPACE.bzlmod file. Let's say that we give it the name @test_cpp_external_include_paths_example
  • Create a fixture and an aspect test, much like this one.

@LeFrosch LeFrosch closed this Aug 18, 2024
@LeFrosch LeFrosch deleted the Daniel.Brauner/4980 branch August 18, 2024 18:32
@github-actions github-actions bot removed the awaiting-review Awaiting review from Bazel team on PRs label Aug 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product: CLion CLion plugin product: GoLand GoLand plugin product: IntelliJ IntelliJ plugin
Projects
None yet
4 participants