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

Expand the scope of remote:CopyFile to support copying directories #23

Closed
Tracked by #449
iwahbe opened this issue Dec 16, 2021 · 0 comments · Fixed by #423
Closed
Tracked by #449

Expand the scope of remote:CopyFile to support copying directories #23

iwahbe opened this issue Dec 16, 2021 · 0 comments · Fixed by #423
Assignees
Labels
kind/enhancement Improvements or new features resolution/fixed This issue was fixed

Comments

@iwahbe
Copy link
Member

iwahbe commented Dec 16, 2021

Hello!

  • Vote on this issue by adding a 👍 reaction
  • If you want to implement this feature, comment to let us know (we'll work with you on design, scheduling, etc.)

Issue details

Affected area/feature

@lukehoban lukehoban added the kind/enhancement Improvements or new features label Jul 15, 2023
@thomas11 thomas11 self-assigned this Apr 23, 2024
@lukehoban lukehoban mentioned this issue May 18, 2024
12 tasks
thomas11 added a commit that referenced this issue Jun 20, 2024
This PR allows the
[CopyFile](https://www.pulumi.com/registry/packages/command/api-docs/remote/copyfile/)
resource to recursively copy directories as well, similar to `scp -r`.

This PR enhances the CopyFile resource in a few ways.
- It can now copy directories as well, recursively with their contents.
- It now takes the standard Pulumi [asserts and
archives](https://www.pulumi.com/docs/concepts/assets-archives/) as
input, allowing for seamless interop with other resources.
- It now checks whether the specified file or directory have changed (in
content, not timestamp) and copies only if they did.

In light of these changes, I've renamed `CopyFile` to just `CopyToRemote` which
is a **breaking change**.

Resolves #23
Resolves #33
Resolves #42

## TODO

- [x] ~~The current implementation fails as soon as a file or directory
already exists on the remote, like the previous `CopyFile`.~~
- [x] Before GA, should we rename `CopyFile` to `Copy` or `RemoteCopy`
or _insert your proposal here_?
- [ ] The integration test copies a bunch of EC2 setup code with
`TestEc2RemoteTs`, which we should share instead. _What would be a good
way to do that, given the code is in the TS tests, not in the Go
driver?_

## Design considerations

The behavior of the copy operation was modeled after `cp` and `scp`.

```
source | dest - exists as dir | dest - does not exist | dest - exists as file
-------|----------------------|-----------------------|-----------------------
dir    | dest/dir             | dest/dir              | error
dir/   | dest/x for x in dir  | dest/dir              | error
file   | dest/file            | dest                  | dest (overwritten)
```

Specifically:
- When copying a directory, we overwrite existing files. (The current
CopyFile resource for single files does that, too.)
- When copying a directory, we _don't_ clear the remote directory first
so that no left-over files from previous copies will be around. We can
always add a flag for that if needed.

## Implementation notes

- I've looked around for open source Go packages implementing scp of
folders, but to my surprise, I couldn't find one with an acceptable
license that 1) wasn't ancient and 2) was cross-platform.
- The current implementation copies files sequentially and is therefore
probably slow for large trees. If we replaced the implementation, I
don't think the shape of the resource would change, so this shouldn't be
a GA blocker.
- The `size.ts` file in both steps of the integration test is useless
because it always has the same value. However, I found that without a TS
file present in the additional step, it would not be run. Haven't
debugged further.

---------

Co-authored-by: Ian Wahbe <ian@wahbe.com>
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Improvements or new features resolution/fixed This issue was fixed
Projects
None yet
4 participants