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

Honor copy=true in directory mode for local file copies #327

Open
wants to merge 29 commits into
base: main
Choose a base branch
from

Conversation

ebudan
Copy link

@ebudan ebudan commented Jun 24, 2021

This looks like an oversight: when getting local files, directory copies always result in a symlink. This may not be desirable in all use cases.

@hashicorp-cla
Copy link

hashicorp-cla commented Jun 24, 2021

CLA assistant check
All committers have signed the CLA.

Copy link

@sylviamoss sylviamoss left a comment

Choose a reason for hiding this comment

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

This is looking good 👍🏼
I miss a test though. A test similar to: https://github.com/hashicorp/go-getter/blob/main/get_file_test.go#L129

go.mod Outdated Show resolved Hide resolved
get_file_unix.go Outdated Show resolved Hide resolved
danieleva and others added 25 commits July 22, 2021 10:14
…request (hashicorp#321)

* Add test to verify canceled context.Context aborts http request

* http: Initialize http.Request with ctx so cancelation interrupts the request
9e42df5 introduced an `Insecure` flag for the http getter and in its error
reporting relied on the `url.Redacted()` method introduced in golang 1.15
(See https://golang.org/doc/go1.15). This breaks CI, but golang 1.14 is out of
support so our minimum supported version of go should now be on the 1.15.x
series.
@ebudan
Copy link
Author

ebudan commented Jul 22, 2021

Added test as requested, and reverted my dir creation change - the reason for that was obscure and not useful here. Module name was changed as I had to use my fork - reverted, will need an additional fork unless this request works for you.

Two realizations during this commit: this leaves the Windows version uncovered (no opportunity here), and there is no support in the CLI tool for hard copy. What would be preferrable for the latter: a command line option (that only applies to copy... ouch), or a url parameter (go-getter file:///tmp/foo?copy=true /tmp/dst ...ouch)?

@Smithx10
Copy link

Smithx10 commented Feb 2, 2024

Merge this?

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.