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 includePatterns and excludePatterns support for copy #239

Merged
merged 4 commits into from
Jun 2, 2021

Conversation

aaronlehmann
Copy link
Contributor

Tested with:

fs busybox() {
        image "busybox"
}

fs copyWithPatterns() {
        copy busybox "/bin" "/" with option {
                includePatterns "x*"
                excludePatterns "xargs"
        }
        download "."
}

Depends on upstream PRs tonistiigi/fsutil#101 and moby/buildkit#2082

@coryb
Copy link
Contributor

coryb commented Apr 28, 2021

LGTM, think we just need to update generated content with ./hlb run -t gen

}

func (ip CopyIncludePatterns) SetCopyOption(ci *llb.CopyInfo) {
ci.IncludePatterns = ([]string)(ip)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we want to append? What do users expect when they do something like:

copy foo "/" "/" with option {
    includePatterns "a"
    includePatterns "b"
 }

This example may be in practical scenarios when trying to refactor an copy::option function with common patterns but also want to extend with additional patterns.

@hinshun
Copy link
Contributor

hinshun commented Apr 29, 2021

Overall looks good. Pending upstream PRs to be merged.

@aaronlehmann aaronlehmann force-pushed the copy-include-exclude branch from 39a579e to dc3615d Compare June 1, 2021 19:53
@aaronlehmann
Copy link
Contributor Author

@hinshun: Updated.

Tested with:

```
fs busybox() {
        image "busybox"
}

fs copyWithPatterns() {
        copy busybox "/bin" "/" with option {
                includePatterns "x*"
                excludePatterns "xargs"
        }
        download "."
}
```
@aaronlehmann aaronlehmann force-pushed the copy-include-exclude branch from dc3615d to e60d1d9 Compare June 1, 2021 20:01
@aaronlehmann aaronlehmann marked this pull request as ready for review June 1, 2021 20:01
Copy link
Contributor

@hinshun hinshun left a comment

Choose a reason for hiding this comment

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

Tested myself, LGTM on test green

@aaronlehmann aaronlehmann merged commit 0bca4d2 into openllb:master Jun 2, 2021
@aaronlehmann aaronlehmann deleted the copy-include-exclude branch June 2, 2021 00:19
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.

3 participants