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.

Sign up for GitHub

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

Jump to bottom

[7.4.0] Fix inconsistent virtual include paths #23221

Closed
bazel-io opened this issue Aug 6, 2024 · 4 comments
Closed

[7.4.0] Fix inconsistent virtual include paths #23221

bazel-io opened this issue Aug 6, 2024 · 4 comments
Milestone
7.4.0 release blo...

Comments

@bazel-io
Copy link
Member

bazel-io commented Aug 6, 2024

Forked from #23071
@bazel-io bazel-io added this to the 7.4.0 release blockers milestone Aug 6, 2024
@bazel-io
Copy link
Member Author

bazel-io commented Aug 8, 2024

The branch, release-7.4.0, may not exist. Please retry the cherry-pick after the branch is created.

@bazel-io
Copy link
Member Author

Cherry-pick was attempted but there were merge conflicts in the following file(s). Please resolve manually.

src/main/java/com/google/devtools/build/lib/rules/cpp/CcStarlarkInternal.java

cc: @bazelbuild/triage

@iancha1992
Copy link
Member

Cherry-pick was attempted but there were merge conflicts in the following file(s). Please resolve manually.

src/main/java/com/google/devtools/build/lib/rules/cpp/CcStarlarkInternal.java

cc: @bazelbuild/triage

cc: @fmeum @lberki

@fmeum fmeum mentioned this issue Aug 20, 2024
Merged
@fmeum
Copy link
Collaborator

fmeum commented Aug 20, 2024

#23361

github-merge-queue bot pushed a commit that referenced this issue Sep 19, 2024
@fmeum @FaBrand
[7.4.0] Fix inconsistent virtual include paths (#23361)
ac9b546 
So far only the generated header in the virtual includes is tested but
the Include path pointing to that directory isnt

This is a testcase to reproduce the suspected error reported here:
- #23061

Assuming that the location of the file is asserted:
```Java
    assertThat(artifactsToStrings(ccInfo.getCcCompilationContext().getDirectPublicHdrs()))
        .contains("bin third_party/bar/_virtual_includes/starlark_lib_suffix/starlark_lib.h");
```
Adding a testcase for the include path seems reasonable.

A local run of the testcase shows the error:
```
1) testStripIncludePrefixIncludePath(com.google.devtools.build.lib.rules.cpp.StarlarkCcCommonTest)
value of: getIncludeDirs().onlyElement()
expected: bazel-out/k8-fastbuild/bin/third_party/bar/_virtual_includes/starlark_lib_suffix
but was : bazel-out/k8-fastbuild/bin/third_party/bar/_virtual_includes/starlark_lib
        at com.google.devtools.build.lib.rules.cpp.StarlarkCcCommonTest.testStripIncludePrefixIncludePath(StarlarkCcCommonTest.java:5902)

FAILURES!!!
Tests run: 73,  Failures: 1
```

The change in behaviour was introduced in
7e0df68#diff-403c46ec3075b8e9e6d490ce955db88dae2d457b8046608884039b18b10ab6ccR774

Fixes: #23061

Closes #23071.

PiperOrigin-RevId: 660681269
Change-Id: Ia3f8a8a6cf8bf0e093416a247e348e6de6719584

Closes #23221

Co-authored-by: FaBrand <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
7.4.0 release blockers
Development

No branches or pull requests
3 participants
@fmeum @bazel-io @iancha1992