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

Fix --output in some cases (#53) #54

Merged
merged 1 commit into from
Mar 13, 2024
Merged

Fix --output in some cases (#53) #54

merged 1 commit into from
Mar 13, 2024

Conversation

JohnCMoon
Copy link
Contributor

Currently, when --output is used multiple times on the same directory, the underlying command which copies the output can fail with a misleading error message. Even if your regular expression matches a file in the target directory, you're presented with:

"no files found within ./target matching the provided output regexp"

If the "mv" command's stderr is printed, it shows:

"mv: inter-device move failed: '/tmp/earthly/lib/rust/release' to 'target/release'; unable to remove target: Directory not empty"

Since "mv" doesn't handle existing directories the way we may want here, let's use "cp -ruT" to overlay the files into the target directory (regardless of what's already present there) and then remove the temporary directory.

Currently, when --output is used multiple times on the same directory,
the underlying command which copies the output can fail with a
misleading error message. Even if your regular expression matches a file
in the target directory, you're presented with:

"no files found within ./target matching the provided output regexp"

If the "mv" command's stderr is printed, it shows:

"mv: inter-device move failed: '/tmp/earthly/lib/rust/release' to 'target/release'; unable to remove target: Directory not empty"

Since "mv" doesn't handle existing directories the way we may want here,
let's use "cp -ruT" to overlay the files into the target directory
(regardless of what's already present there) and then remove the
temporary directory.
@JohnCMoon JohnCMoon requested a review from a team as a code owner March 13, 2024 02:17
@CLAassistant
Copy link

CLAassistant commented Mar 13, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@idelvall idelvall left a comment

Choose a reason for hiding this comment

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

LGTM,
thanks @JohnCMoon!

@idelvall idelvall enabled auto-merge (squash) March 13, 2024 11:32
@idelvall idelvall disabled auto-merge March 13, 2024 11:33
@idelvall idelvall merged commit 855c98f into earthly:main Mar 13, 2024
1 of 3 checks passed
@idelvall
Copy link
Member

Nice picture BTW!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants