Skip to content

Implement Tar Global Extended Attributes API changes #70869

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
carlossanlop merged 8 commits into from
Jun 22, 2022
Merged

Implement Tar Global Extended Attributes API changes #70869

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
37cfb52
ref: Global Extended Attributes API changes
carlossanlop Jun 17, 2022
4e11d92
src: Global Extended Attributes API changes
carlossanlop Jun 17, 2022
5279945
tests: Verify Global Extended Attributes API changes
carlossanlop Jun 17, 2022
aa9c213
Address suggestions
carlossanlop Jun 20, 2022
94315d3
Address debug suggestion.
carlossanlop Jun 22, 2022
5895408
Improve the decision of reducing the size of the GEA path.
carlossanlop Jun 22, 2022
df9e81d
Use Path.GetTempPath for the GEA path.
carlossanlop Jun 22, 2022
273394d
Rename field tracking GEA entry number.
carlossanlop Jun 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Use Path.GetTempPath for the GEA path.
  • Loading branch information
@carlossanlop
carlossanlop committed Jun 22, 2022
commit df9e81d6d0c0bd68adda4f3c4cf55a22fb039883
8 changes: 2 additions & 6 deletions src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarHeader.Write.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -615,12 +615,8 @@ private static string GenerateGlobalExtendedAttributeName(int globalExtendedAttr
{
Debug.Assert(globalExtendedAttributesEntryNumber >= 1);

Copy link
Member

@danmoseley danmoseley Jun 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an aside - the code below reading TMPDIR then falling back to /tmp- why doesn't it just call Path.GetTempPath() that does the same thing?

Copy link
Contributor Author

@carlossanlop carlossanlop Jun 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion. I'll use Path.GetTempPath.

Here's why I wasn't using that:

The GNU tar manual specifies how the global extended attribute header name field obtains its value:

8.3.7.1 Controlling Extended Header Keywords

$TMPDIR/GlobalHead.%p.%n

[...] $TMPDIR stands for the value of the TMPDIR environment variable. If TMPDIR is not set, tar uses /tmp.

This description looks very similar to the remarks section of Path.GetTempPath:

Remarks

This method checks for the existence of environment variables in the following order and uses the first path found:

Linux

  1. The path specified by the TMPDIR environment variable. If the path is not specified in the TMPDIR environment variable, the default path /tmp/ is used.

Windows

  1. The path specified by the TMP environment variable.
  2. The path specified by the TEMP environment variable.
  3. The path specified by the USERPROFILE environment variable.
  4. The Windows directory.

So a couple of things need to be considered if we are to use Path.GetTempPath:

  • I don't see why any tool would be reading the GEA path, or having reading/writing behavior depend on it. The field isn't very interesting, unless the user cares about the process ID or the sequence number.
  • Keep in mind that none of the tar specs (at least the ones I read) specify what the expected behavior for Windows should be. Everything is Unix focused. If we format that string embedding Windows paths, I hope other tools don't break due to finding unexpected drive names or \ separators in that field. But go back to the previous point: I don't think any tool should make its behavior depend on this field.
  • If the Windows path ends up being larger than than 100 bytes, the sequence number and process ID would get truncated. Which is fine, I guess, because a couple of lines below, I re-generate the string by forcing the usage of /tmp.

string? tmpDir = Environment.GetEnvironmentVariable("TMPDIR");
if (string.IsNullOrWhiteSpace(tmpDir))
{
tmpDir = "/tmp";
}
else if (Path.EndsInDirectorySeparator(tmpDir))
string tmpDir = Path.GetTempPath();
if (Path.EndsInDirectorySeparator(tmpDir))
{
tmpDir = Path.TrimEndingDirectorySeparator(tmpDir);
}
Expand Down