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

Feature/additional link outputs #25091

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

yuvalmo
Copy link

@yuvalmo yuvalmo commented Jan 27, 2025

Solves #24990.

Implement additional_link_outputs feature.

The feature allows for retrieving additional files created by the link
command.

The additional filenames are determined by the cc_toolchain_config
attribute additional_link_outputs which lists the suffixes that will
be added to the binary name.

For binary out where additional_link_outputs = [".a", ".b"], we'll
expect the following files to be created by the link action:

  • out
  • out.a
  • out.b

rules_cc pull request with tests for the feature here.

@yuvalmo yuvalmo requested a review from lberki as a code owner January 27, 2025 18:31
@github-actions github-actions bot added team-Rules-CPP Issues for C++ rules awaiting-review PR is awaiting review from an assigned reviewer labels Jan 27, 2025
@sgowroji sgowroji added awaiting-user-response Awaiting a response from the author and removed awaiting-review PR is awaiting review from an assigned reviewer labels Jan 28, 2025
@sgowroji
Copy link
Member

Hi @yuvalmo, Could you please take a look at the failing checks?

@yuvalmo
Copy link
Author

yuvalmo commented Jan 28, 2025

@sgowroji it seems the failures are in the windows tests due to the first commit where I changed _remove_extension to .basename.

I will revert it later today.

The feature allows for retrieving additional files created by the link
command.

The additional filenames are determined by the `cc_toolchain_config`
attribute `additional_link_outputs` which lists the suffixes that will
be added to the binary name.

For binary `out` where `additional_link_outputs = [".a", ".b"]`, we'll
expect the following files to be created by the link action:
- `out`
- `out.a`
- `out.b`

Solves bazelbuild#24990.
This only applies to windows, where the `.exe` suffix will be removed.
This seems to me like a reasonable behaviour for windows devs.
@yuvalmo yuvalmo force-pushed the feature/additional-link-outputs branch from 56ac9e5 to 6da302d Compare January 28, 2025 21:44
@yuvalmo
Copy link
Author

yuvalmo commented Jan 28, 2025

@sgowroji Fixed now, all tests passed.

@sgowroji sgowroji added awaiting-review PR is awaiting review from an assigned reviewer and removed awaiting-user-response Awaiting a response from the author labels Jan 29, 2025
@lior10r
Copy link
Contributor

lior10r commented Jan 30, 2025

Can someone go over this PR? It is really critical to our project

@fmeum
Copy link
Collaborator

fmeum commented Feb 6, 2025

@pzembrod Would you be available to review this? It has the potential to squash many individual "I just need this one weird file my toolchain happens to produce" requests. :⁠-⁠)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review PR is awaiting review from an assigned reviewer team-Rules-CPP Issues for C++ rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants