download! method doesn't close file

[roooneey]

which causes hard-to-debug errors when you download many files in a row and try to read the file contents immediately after downloading. It works most of the time, but at some point a downloaded file will not be flushed to disk completely when you try to read the file.

Reading only the partially written file from disk will, of course, result in corrupted file contents. I was able to avoid this behavior by explicitly closing the downloaded file before reading it again. But this was not very intuitive, so please consider closing the file within the download method.

tmp_file = Tempfile.new(file_name, tmp_dir)
src = File.join(dir, file_name)
          
download! src, tmp_file, log_percent: -1

# it's important to close the tmp file, otherwise data might not be written 
# to disk already, when we try to read the file again
tmp_file.close 

begin
   File.read tmp_file
ensure
   tmp_file.close
   tmp_file.unlink
end

[mattbrictson]

@roooneey thanks for the report!

I agree, this behavior is a bit unintuitive. You would think that download! with a string argument would work similar to Ruby's File.write, which opens a file, writes to it, and closes it.

However in this case SSHKit is just delegating to download! from the net-scp gem, so we inherit its file handling behavior.

I hesitate to a make change here, because it would introduce an inconsistency between SSHKit's download! and the net-scp method that it aliases.

Maybe you could report this issue upstream, to net-scp and see if they would be willing to fix the behavior there?

https://github.com/net-ssh/net-scp