Prevent .tar file corruption by patching short reads

[joost-j]

Fixes an issue where a .tar output file would contain inconsistencies with regards to expected and actual file size of the included files.

In some cases, a file on disk can report a size of X bytes, but at the time of actually reading X bytes from the file, less than X bytes are actually available in the file (a short read). Acquire would report these issues as OSError in the resulting Acquisition log file, because the Python stdlib tarfile.py handles it that way. Data may however already be written to the destination archive at that point.

Afterwards, Acquire continues to add new files to the archive. When trying to untar the file using tar -xvf <FILE> this would show as a tar: Skipping to next header error and finally, the process exists with a nonzero exit code.

Included a test case which simulates a file that actually returns less bytes than its reported size, to test this case.

[codecov]

Codecov Report

❌ Patch coverage is 86.48649% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.17%. Comparing base (2956b06) to head (4373d8f).

Files with missing lines	Patch %	Lines
acquire/outputs/tar.py	86.48%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #261      +/-   ##
==========================================
+ Coverage   44.75%   45.17%   +0.41%     
==========================================
  Files          26       26              
  Lines        3546     3582      +36     
==========================================
+ Hits         1587     1618      +31     
- Misses       1959     1964       +5     

Flag	Coverage Δ
unittests	45.17% <86.48%> (+0.41%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Schamper]

I think you can generalize this case instead of doing it twice. Keep track of how many bytes you actually wrote (i.e. using .tell() and only pad once.

[joost-j]

Don't know exactly what you mean by this, since it looks like we need to calculate the remainder for two different sizes (bufsize vs tarfile.BLOCKSIZE). But please do change / refactor the code if you can write this more concise.

[Schamper]

You could make this even safer by truncating to the previous offset/tar member end if any exception occurs while writing.

[joost-j]

Done, added a try/except around it to restore back to the original offset, plus a test case.

[qmadev]

Any idea when this is getting fixed? It's affecting me as well. Let me know if there's anything I can do to help!