[Tar] Fill in the file size even if the file is empty.

[sobczyk]

This matches standard behavior.
See #95354

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-formats-tar
See info in area-owners.md if you want to be subscribed.

[carlossanlop]

Thank you for the contribution. Can you please add a test based on the repro code in your issue description? #95354 (comment)

[carlossanlop]

@sobczyk I see your comment in the issue:

the change is basically a one liner, the problem is that the unit tests are a bit messy I'll provide the fix today or tomorrow, and will try to do some UT, but that looked more difficult than the fix

Let me know if you have any questions or I can be of assistance. I'll be happy to help.

[sobczyk]

@carlossanlop Ok, I added the repro code as an unit test.
I was wondering whether _size field should be nullable?
I'm guessing it was 0 since older .NET did not have nullables.
If _size would be nullable the check for _dataStream would not be necessary,
though the fix would change more lines.

[sobczyk]

@dotnet-policy-service agree

[carlossanlop]

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.

[carlossanlop]

Did you only see the problem in Pax, or is this the only format that you tested?

[sobczyk]

The one I tested, but the problem would apply equally to all, since small sizes are encoded the same way.

[carlossanlop]

@carlossanlop Ok, I added the repro code as an unit test. I was wondering whether _size field should be nullable? I'm guessing it was 0 since older .NET did not have nullables. If _size would be nullable the check for _dataStream would not be necessary, though the fix would change more lines.

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.

[sobczyk]

Is the code OK?

[carlossanlop]

Is the code OK?

Yes. I was just waiting for the CI to finish.

The failure is known and pre-existing: #104905

Ready to merge. Thank you for your contribution, @sobczyk!

[adamsitnik]

nit: the test methods should be public

[am11]

Yup, for consistency and repo conventions. However, it is known to work with xunit: xunit/xunit#2438 (reply in thread) :)

[am11]

Will it be included in .NET 9?

[carlossanlop]

It's only in main right now. But we could consider requesting approval for backporting to RC2.

[carlossanlop]

Did you encounter a case where you need it, @am11?

[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. :)

[carlossanlop]

/backport to release/9.0

[github-actions]

Started backporting to release/9.0: https://github.com/dotnet/runtime/actions/runs/10797621721