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

feat: add an option to not include copy_to_directory output in runfiles #886

Merged
merged 3 commits into from
Sep 19, 2024

Conversation

kormide
Copy link
Collaborator

@kormide kormide commented Jul 25, 2024

Fixes #748.


Changes are visible to end-users: yes

  • Searched for relevant documentation and updated as needed: yes
  • Breaking change (forces users to change their own code or config: no
  • Suggested release notes appear below: yes

Added an attr add_directory_to_runfiles to copy_to_directory to not include the output directory in the target's runfiles.

Test plan

  • Covered by existing test cases

@kormide kormide requested a review from alexeagle July 25, 2024 00:34
@kormide
Copy link
Collaborator Author

kormide commented Jul 25, 2024

I'm having trouble testing this one due to the issue with sh_test I pointed out for the existing test that checks that files are added to runfiles. Even after excluding them they still appear because sh_test includes default outputs of targets passed to data in its own runfiles. Can you think of a way to test this? Should I write a custom sh_test just for these tests?

Also, I don't write rules often so my knowledge around runfiles is murky. Best take a careful look at this.

@kormide
Copy link
Collaborator Author

kormide commented Jul 25, 2024

@dieortin Does this solve your use case?

@dieortin
Copy link

@dieortin Does this solve your use case?

I haven't tested it, but it should solve it, yes!

@alexeagle alexeagle enabled auto-merge (squash) September 19, 2024 01:10
@alexeagle alexeagle merged commit 716af22 into main Sep 19, 2024
22 checks passed
@alexeagle alexeagle deleted the issue-4519 branch September 19, 2024 01:16
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.

copy_to_directory includes all files as runfiles
3 participants