[Tar] Fill in the file size even if the file is empty.#106409
[Tar] Fill in the file size even if the file is empty.#106409carlossanlop merged 4 commits intodotnet:mainfrom
Conversation
This matches standard behavior.
ghost
added
the
dotnet-policy-service
bot
added
the
community-contribution
|
Tagging subscribers to this area: @dotnet/area-system-formats-tar |
carlossanlop
left a comment
carlossanlop
left a comment
There was a problem hiding this comment.
Thank you for the contribution. Can you please add a test based on the repro code in your issue description? #95354 (comment)
|
@sobczyk I see your comment in the issue:
Let me know if you have any questions or I can be of assistance. I'll be happy to help. |
|
@carlossanlop Ok, I added the repro code as an unit test. |
|
@dotnet-policy-service agree |
carlossanlop
left a comment
carlossanlop
left a comment
There was a problem hiding this comment.
Left a minor suggestion (disposing the test types). Otherwise looks good.
I see your test only verifies Pax, which is fine for this case. I would be surprised if the issue happens only in certain formats instead of in all of them, as the affected code lines are shared among all types.
src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.Tests.cs
Outdated
Show resolved
Hide resolved
| MemoryStream emptyData = new(0); | ||
| MemoryStream output = new(); | ||
| TarWriter archive = new(output, TarEntryFormat.Pax); |
There was a problem hiding this comment.
Did you only see the problem in Pax, or is this the only format that you tested?
There was a problem hiding this comment.
The one I tested, but the problem would apply equally to all, since small sizes are encoded the same way.
It was not nullable on purpose. I don't recall the specific reasons at the moment but this was intended. System.Formats.Tar was introduced in .NET 7. Nullables already existed in that version, so that was not the reason. |
Co-authored-by: Carlos Sánchez López <1175054+carlossanlop@users.noreply.github.com>
|
Is the code OK? |
| } | ||
|
|
||
| [Fact] | ||
| void Verify_Size_RegularFile_Empty() |
There was a problem hiding this comment.
nit: the test methods should be public
There was a problem hiding this comment.
Yup, for consistency and repo conventions. However, it is known to work with xunit: xunit/xunit#2438 (reply in thread) :)
|
Will it be included in .NET 9? |
|
It's only in main right now. But we could consider requesting approval for backporting to RC2. |
|
Did you encounter a case where you need it, @am11? |
|
If it is going in GA release, that would be nice. I haven't stumbled upon it, but I was thinking about implementing an msbuild task based on System.Formats.Tar (to replace dotnet/arcade#15038 (comment)), so I thought of checking up on the status of (this relatively new lib) support. :) |
|
/backport to release/9.0 |
|
Started backporting to release/9.0: https://github.com/dotnet/runtime/actions/runs/10797621721 |
5 participants
This matches standard behavior.
See #95354