Skip to content

add a stopgap experimental_use_whole_archive_for_native_deps attribute #1269

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

Merged
merged 4 commits into from
Apr 20, 2022

Conversation

krasimirgg
Copy link
Collaborator

This adds a new stopgap attribute enabling us to fall back to the old rustc behavior wrt whole-archive handling, see #1268.

@krasimirgg krasimirgg marked this pull request as draft April 19, 2022 10:25
@krasimirgg krasimirgg requested a review from scentini April 19, 2022 10:28
@krasimirgg krasimirgg marked this pull request as ready for review April 19, 2022 10:28
Copy link
Collaborator

@scentini scentini left a comment

Choose a reason for hiding this comment

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

I guess an integration test is not easy to come up with, but could we add a unit test that tests that the commandline is what we expect when the attribute's value is True?

@krasimirgg
Copy link
Collaborator Author

After chatting 1:1, it appears currently there's no good way to test this (the new feature requires a recent nightly, we don't have a nightly build bot setup yet, and we don't have a fully unit-test style test framework that doesn't attempt to always analyse the target_under_test-s). Will work on improvements to the test infra have some form of rustc nightly bot coverage in the future.

@krasimirgg krasimirgg requested a review from scentini April 20, 2022 11:57
@scentini scentini merged commit f6e7e0a into bazelbuild:main Apr 20, 2022
goffrie pushed a commit to goffrie/rules_rust that referenced this pull request Apr 21, 2022
bazelbuild#1269)

This adds a new stopgap attribute enabling us to fall back to the old rustc behavior wrt whole-archive handling, see bazelbuild#1268.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants