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 copy_directory rule #366

Merged
merged 1 commit into from
Jun 1, 2022
Merged

Conversation

kormide
Copy link
Contributor

@kormide kormide commented Apr 13, 2022

Following up on the discussion in @alexeagle's PR (#323) to add a new copy_directory rule.

@kormide
Copy link
Contributor Author

kormide commented May 17, 2022

@tetromino do you have any feedback on this PR?

@tetromino
Copy link
Collaborator

@kormide,please ask one of the other maintainers for review - I will have very limited availability until July (paternity leave).

@kormide
Copy link
Contributor Author

kormide commented May 17, 2022

@kormide,please ask one of the other maintainers for review - I will have very limited availability until July (paternity leave).

Yes, no problem. And congratulations! 🎉

@kormide
Copy link
Contributor Author

kormide commented May 17, 2022

@brandjon Do you have any feedback on this PR?

@kormide
Copy link
Contributor Author

kormide commented May 18, 2022

@comius, @hvadehra, or @c-mita do you have any feedback on this PR?

@comius
Copy link
Collaborator

comius commented May 20, 2022

@hvadehra can you review this one?

rules/private/copy_directory_private.bzl Outdated Show resolved Hide resolved
rules/private/copy_directory_private.bzl Outdated Show resolved Hide resolved
rules/private/copy_directory_private.bzl Outdated Show resolved Hide resolved
tests/copy_directory/copy_directory_tests.sh Show resolved Hide resolved
@kormide kormide force-pushed the copy_directory branch 4 times, most recently from 70f2941 to 6a0e146 Compare May 24, 2022 00:59
@kormide kormide requested a review from hvadehra May 24, 2022 01:00
Copy link
Member

@hvadehra hvadehra left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks!

@kormide
Copy link
Contributor Author

kormide commented May 31, 2022

Are we ready to merge, or does this require further review?

@kkpattern
Copy link

copy_directory is very useful to us, thanks for implementing it!

Maybe we can add another copy_files_to_directory?

The difference between copy_files_to_directory with copy_directory is that only seletced files will be copied and they don't have to be in the same source directroy.

The difference between copy_files_to_directory with copy_file is that we can use filegroup to copy multiple files at the same time. Something like:

filegroup(
    name = "files_to_copy",
    srcs = glob(["a/*.h", "b/*.h"]),
)

copy_files_to_directory(
    name = "copy_files",
    srcs = [":files_to_copy"],
    output = "path/to/target/directory",
)

I found a related copy_filegroup_to_package implementation here from a StackOverflow question.

@kormide
Copy link
Contributor Author

kormide commented Jun 1, 2022

copy_directory is very useful to us, thanks for implementing it!

Maybe we can add another copy_files_to_directory?

The difference between copy_files_to_directory with copy_directory is that only seletced files will be copied and they don't have to be in the same source directroy.

The difference between copy_files_to_directory with copy_file is that we can use filegroup to copy multiple files at the same time. Something like:

filegroup(
    name = "files_to_copy",
    srcs = glob(["a/*.h", "b/*.h"]),
)

copy_files_to_directory(
    name = "copy_files",
    srcs = [":files_to_copy"],
    output = "path/to/target/directory",
)

I found a related copy_filegroup_to_package implementation here from a StackOverflow question.

@kkpattern Something like this copy_to_directory rule that we wrote?

@kkpattern
Copy link

@kkpattern Something like this copy_to_directory rule that we wrote?

Yes! Looks like exactly what we need.

@hvadehra hvadehra merged commit 207acb3 into bazelbuild:main Jun 1, 2022
@kormide kormide deleted the copy_directory branch June 1, 2022 17:20
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.

5 participants