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

[7.4.0] Fix inconsistent virtual include paths #23361

Merged

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Aug 20, 2024

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:

Assuming that the location of the file is asserted:

    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

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:
- bazelbuild#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 bazelbuild@7e0df68#diff-403c46ec3075b8e9e6d490ce955db88dae2d457b8046608884039b18b10ab6ccR774

Fixes: bazelbuild#23061

Closes bazelbuild#23071.

PiperOrigin-RevId: 660681269
Change-Id: Ia3f8a8a6cf8bf0e093416a247e348e6de6719584
@fmeum fmeum requested a review from a team as a code owner August 20, 2024 07:47
@fmeum fmeum requested a review from lberki August 20, 2024 07:47
@github-actions github-actions bot added awaiting-review PR is awaiting review from an assigned reviewer team-Rules-CPP Issues for C++ rules labels Aug 20, 2024
@fmeum
Copy link
Collaborator Author

fmeum commented Sep 19, 2024

@lberki Friendly ping

@iancha1992 iancha1992 added this pull request to the merge queue Sep 19, 2024
Merged via the queue into bazelbuild:release-7.4.0 with commit ac9b546 Sep 19, 2024
52 checks passed
@github-actions github-actions bot removed the awaiting-review PR is awaiting review from an assigned reviewer label Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Rules-CPP Issues for C++ rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants