Unexpected behavior of upload_file() when uploading directories

[desolat]

The following behavior of upload_file() is rather suprising to most users: when the target directory does not exist, it copies the contents of the source directory (which is what I was expecting) to the automatically created target directory. If the target folder already exists, an additional folder named after the target folder gets created in the target directory and the source directory contents get copied there!

Looking at the code this originates from the usage of the corelibs mv command. But this implementation detail should be hidden from the user.

I'd suggest the following: It should be defined and documented if the command silently overwrites (files and also directories) or does not and then fails. Currently, for files it already does silent overwriting (mv -f) but does not for directories. I'd prefer fail-if-exists as default and an option for overwriting. The -t and -T options may help to accomplish this but maybe the coreutils are not the right tool for that aim.

[adreyer]

-T is only in gnu mv not bsd so we can't rely on it.

Failing if the file already exists would be a pretty big breaking change and while its safer I don't think it's the most common use case.

It makes sense to change the directory case to overwrite by default to match the file behavior. I think we could expose this change with a feature flag for the rest on the 1.x bolt series.

[MikaelSmith]

How is overwriting instead of copying-into less of a breaking change than failing if copying a directory and the target exists?

[adreyer]

That specifically was about files not directories. The goal seems to be to have the behavior the same. Having directories error if they already exist and files just overwrite seems barely better than the current behavior.

[adreyer]

I suppose that would be different behavior anyway.

upload_file('../file.txt', '~/foo') will copy into foo if it's a directory and replace foo if it's a file. upload_file('../file/', '~/foo') would always replace in that case.

[lucywyman]

To do:

• Check upload_file behavior on other transports
• Possibly have rsync be an option for the SSh transport (similar to smb option for winrm)

@desolat Just fyi, we're likely not going to prioritize this soon, but we'll leave it open for now.

[gucki]

I tried to upload a directory and the current functionality seems totally broken if the target directory already exists. The name of the source directory is always used for a subfolder in the target directory.

Having a sync functionality, which also deletes files not on the source side, would also be great.

[gucki]

I implemented a quick' dirty file sync function based on file_upload (but it works!), see https://gist.github.com/gucki/6b81030910fb5fca034559991b840a9f. Please feel free to use it in any way you like.

[vivostar]

I implemented a quick' dirty file sync function based on file_upload (but it works!), see https://gist.github.com/gucki/6b81030910fb5fca034559991b840a9f. Please feel free to use it in any way you like.

@gucki hi, may I ask how to use the ruby file in the Bolt? Please give some tips.

[gucki]

@vivostar Save it to a file in the modules folder, ex. modules/xyz/lib/puppet/functions/sync_file.rb. Then you should be able to call it just like any other function from your bolt plan.

[vivostar]

@vivostar Save it to a file in the modules folder, ex. modules/xyz/lib/puppet/functions/sync_file.rb. Then you should be able to call it just like any other function from your bolt plan.

Oh, thank you😝

[github-actions[bot]]

This issue has not had activity for 60 days and will be marked as stale. If this issue continues to have no activity for 7 days, it will be closed.

[github-actions[bot]]

This issue is stale and has been closed. If you believe this is in error, or would like the Bolt team to reconsider it, please reopen the issue.