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

Add path mapping support for C++ compile action templates #22890

Closed
wants to merge 25 commits into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Jun 25, 2024

C++ action templates are now properly path mapped.

Since the default Unix toolchain doesn't support object file groups on macOS, apple_support is needed in the test. This requires wiring up LocalEnvProvider in the remote worker to get DEVELOPER_DIR to be set, which in turn requires improving the runfiles handling for the the worker.

Work towards #6526

@fmeum
Copy link
Collaborator Author

fmeum commented Jun 28, 2024

@keith Any idea why this test may be failing on macOS?

ld: unknown options: --start-lib --end-lib 

Does Apple lld not support --start-lib?

@keith
Copy link
Member

keith commented Jun 28, 2024

Apple's linker does not support start-lib/end-lib. ld64.lld from llvm does since looks like version 14+

@fmeum
Copy link
Collaborator Author

fmeum commented Jun 29, 2024

@keith I added apple_support for its object_file_group support, but I'm getting an error about DEVELOPER_DIR missing. Any idea about that one?

@keith
Copy link
Member

keith commented Jun 30, 2024

looks like it's the remote only test (at least on the last failure), isn't this where the remote executor has to duplicate the logic around setting DEVELOPER_DIR?

@fmeum fmeum marked this pull request as ready for review July 1, 2024 10:55
@fmeum fmeum requested review from a team and lberki as code owners July 1, 2024 10:55
@github-actions github-actions bot added team-Rules-CPP Issues for C++ rules team-Remote-Exec Issues and PRs for the Execution (Remote) team awaiting-review PR is awaiting review from an assigned reviewer labels Jul 1, 2024
@fmeum fmeum requested review from comius and removed request for a team and lberki July 1, 2024 10:56
@comius comius added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Jul 5, 2024
@fmeum
Copy link
Collaborator Author

fmeum commented Jul 5, 2024

@bazel-io fork 7.3.0

@iancha1992
Copy link
Member

@fmeum @comius @keith Just a friendly reminder, the 7.3.0RC1 is scheduled for next Monday, July 29th. Thanks!

@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Jul 30, 2024
@fmeum fmeum deleted the 6526-cpp-tree-artifact branch July 31, 2024 12:03
@fmeum
Copy link
Collaborator Author

fmeum commented Jul 31, 2024

@bazel-io fork 7.4.0

iancha1992 pushed a commit to iancha1992/bazel that referenced this pull request Aug 19, 2024
C++ action templates are now properly path mapped.

Since the default Unix toolchain doesn't support object file groups on macOS, `apple_support` is needed in the test. This requires wiring up `LocalEnvProvider` in the remote worker to get `DEVELOPER_DIR` to be set, which in turn requires improving the runfiles handling for the the worker.

Work towards bazelbuild#6526

Closes bazelbuild#22890.

PiperOrigin-RevId: 657733074
Change-Id: I132e338d7a44964a8a1aad3062a5c9a4c8e79e57
github-merge-queue bot pushed a commit that referenced this pull request Aug 26, 2024
…3349)

C++ action templates are now properly path mapped.

Since the default Unix toolchain doesn't support object file groups on
macOS, `apple_support` is needed in the test. This requires wiring up
`LocalEnvProvider` in the remote worker to get `DEVELOPER_DIR` to be
set, which in turn requires improving the runfiles handling for the the
worker.

Work towards #6526

Closes #22890.

PiperOrigin-RevId: 657733074
Change-Id: I132e338d7a44964a8a1aad3062a5c9a4c8e79e57

Commit
23f3be0

Co-authored-by: Googler <ilist@google.com>
@iancha1992
Copy link
Member

The changes in this PR have been included in Bazel 7.4.0 RC1. Please test out the release candidate and report any issues as soon as possible.
If you're using Bazelisk, you can point to the latest RC by setting USE_BAZEL_VERSION=7.4.0rc1. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Remote-Exec Issues and PRs for the Execution (Remote) team team-Rules-CPP Issues for C++ rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants