`podman cp` supports .dockerignore #2745

QiWang19 · 2019-03-22T17:39:33Z

podman cp can exclude (or keep) some files by specifying the rules in .dockerignoe file, so that can avoid sending unnecessary files to container.

Signed-off-by: Qi Wang [email protected]

rh-atomic-bot · 2019-03-22T17:49:33Z

Can one of the admins verify this patch?
I understand the following commands:

bot, add author to whitelist
bot, test pull request
bot, test pull request once

mheon · 2019-03-22T17:50:40Z

Alright, this is getting ridiculous, we've had to manually whitelist every PR from Qi...

mheon · 2019-03-22T17:50:51Z

bot, add author to whitelist

TomSweeneyRedHat · 2019-03-22T17:56:45Z

Ditto on Buildah, if you find the magic bits to flip @mheon, please let me know.

QiWang19 · 2019-03-23T00:53:39Z

Because my format vendor PR ruined the libpod repo last week 😨 ? bot, I'm sorry 😭

rhatdan · 2019-03-24T11:55:01Z

I think we have to add her to the OWNERS file.

rhatdan · 2019-03-25T11:03:34Z

/approve

openshift-ci-robot · 2019-03-25T11:03:37Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: QiWang19, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

rhatdan · 2019-03-25T11:07:07Z

cmd/podman/cp.go

-	extract := c.Flag("extract").Changed
-	return copyBetweenHostAndContainer(runtime, args[0], args[1], extract)
+	copyOpts := copyOptions{
+		extract: c.Flag("extract").Changed,


This code is wrong
Should be

copyOpts := copyOptions{ extract: c.Extract }

giuseppe · 2019-03-25T11:14:51Z

cmd/podman/cp.go

+				if exclude.IsExcluded {
+					return nil
+				}
+				break


why break here? Don't we need to check all the exclude rules?

if it's not match, I have continue in line 295

shouldn't we consider the last match though?

Something like:

a/b !a/b/c

Wouldn't we skip a/b/c in this way?

yes, @giuseppe , needs review 🕵️‍♂️

giuseppe · 2019-03-25T11:14:54Z

cmd/podman/cp.go

+		if exclude.IsExcluded {
+			return nil
+		}
+		break


same here, don't we need to check all the excludes?

giuseppe · 2019-03-25T11:35:41Z

cmd/podman/cp.go

+				return err
+			}
+			if info.IsDir() {
+				return nil


what happens if a directory is marked to be excluded?

For example, if I've:

mkdir -p a/b touch a/b/c echo a/b > .dockerignore

Shouldn't we skip also the file c from being copied?

QiWang19 · 2019-03-27T20:54:12Z

/test lint

rh-atomic-bot · 2019-03-28T14:21:50Z

☔ The latest upstream changes (presumably #2789) made this pull request unmergeable. Please resolve the merge conflicts.

podman cp can exclude (or keep) some files by specifying the rules in .dockerignoe file, so that can avoid sending unnecessary files to container. Signed-off-by: Qi Wang <[email protected]>

QiWang19 · 2019-03-28T17:26:16Z

/retest

mheon · 2019-04-01T20:07:21Z

cmd/podman/cp.go

-	extract := c.Flag("extract").Changed
-	return copyBetweenHostAndContainer(runtime, args[0], args[1], extract)
+	copyOpts := copyOptions{
+		extract: c.Extract,


Why are we only filling one field in here?

If we're not using all of copyOptions, just pass in Extract to copyBetweenHostAndContainer and make a copyOptions in there

mheon · 2019-04-01T20:11:52Z

cmd/podman/cp.go

 		glob = append(glob, srcPath)
 	}
+
+	dir, err := os.Getwd()


Is this actually how Docker find .dockerignore? I would assume they would start at the source directory and look for .dockerignore in the source directory and its parents?

docker looks for a file named .dockerignore in the root directory of the context
what does this mean, it it the source directory?

That's for Dockerfiles. We don't have a context directory in podman copy, really. We have a source and a destination we're copying.

I'm looking for documentation here on how docker cp actually handles this and not finding much.

Does Docker even do this? I can't find any documentation for this actually working in Docker. I don't really see why we would want to implement it if they don't?

I didn't find the documentation, either. But just found the cp command was influenced if there is a .dockerignore file in the context directory

hm...maybe we don't need this.

openshift-ci-robot added the size/L label Mar 22, 2019

openshift-ci-robot requested review from giuseppe and rhatdan March 22, 2019 17:39

QiWang19 force-pushed the ignore branch from b5186fb to ace385b Compare March 22, 2019 17:49

openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L labels Mar 22, 2019

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 25, 2019

rhatdan reviewed Mar 25, 2019

View reviewed changes

giuseppe requested changes Mar 25, 2019

View reviewed changes

QiWang19 force-pushed the ignore branch 2 times, most recently from f81658d to 6e2513f Compare March 25, 2019 16:45

mheon added the Release Notes 1.2.0 label Mar 25, 2019

QiWang19 force-pushed the ignore branch from 6e2513f to ff3f307 Compare March 26, 2019 19:33

QiWang19 requested a review from giuseppe March 27, 2019 03:13

QiWang19 force-pushed the ignore branch from ff3f307 to fc25344 Compare March 27, 2019 20:20

QiWang19 force-pushed the ignore branch from fc25344 to 5066d14 Compare March 28, 2019 13:18

QiWang19 force-pushed the ignore branch from 5066d14 to 5268d0c Compare March 28, 2019 15:35

openshift-ci-robot added size/L and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 28, 2019

podman cp supports .dockerignore

ee82e61

podman cp can exclude (or keep) some files by specifying the rules in .dockerignoe file, so that can avoid sending unnecessary files to container. Signed-off-by: Qi Wang <[email protected]>

QiWang19 force-pushed the ignore branch from 5268d0c to ee82e61 Compare March 28, 2019 17:09

mheon added Release Notes 1.3.0 and removed Release Notes 1.2.0 labels Mar 29, 2019

mheon reviewed Apr 1, 2019

View reviewed changes

QiWang19 closed this Apr 3, 2019

QiWang19 deleted the ignore branch June 26, 2020 15:10

github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 24, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2023

podman cp supports .dockerignore #2745

podman cp supports .dockerignore #2745

Uh oh!

Conversation

QiWang19 commented Mar 22, 2019

Uh oh!

rh-atomic-bot commented Mar 22, 2019

Uh oh!

mheon commented Mar 22, 2019

Uh oh!

mheon commented Mar 22, 2019

Uh oh!

TomSweeneyRedHat commented Mar 22, 2019

Uh oh!

QiWang19 commented Mar 23, 2019

Uh oh!

rhatdan commented Mar 24, 2019

Uh oh!

rhatdan commented Mar 25, 2019

Uh oh!

openshift-ci-robot commented Mar 25, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

QiWang19 commented Mar 27, 2019

Uh oh!

rh-atomic-bot commented Mar 28, 2019

Uh oh!

QiWang19 commented Mar 28, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

`podman cp` supports .dockerignore #2745

`podman cp` supports .dockerignore #2745