Skip to content

Enable filtering of tarball extraction - #15717

Merged
NikolaMilosavljevic merged 1 commit into
dotnet:mainfrom
NikolaMilosavljevic:archive.filtering
Mar 13, 2023
Merged

NikolaMilosavljevic merged 1 commit into
dotnet:mainfrom
NikolaMilosavljevic:archive.filtering

Conversation

@NikolaMilosavljevic

@NikolaMilosavljevic NikolaMilosavljevic commented Mar 7, 2023 •

Copy link
Copy Markdown

Fixes: #15698

This change extends the original work (#15272) to Linux archives.

We need to selectively extract content from archives provided by aspnetcore and windowsdesktop. Without this fix, we end up with multiple NETCore.App runtimes in SDK tarball. Besides incorrect SDK tarball content, this causes failures in source-build's SDK diff tests: dotnet/source-build#3298

Partial tarball extraction uses GZipStream and TarReader whcih are supported in .NET 7.0+

External tar command is still used for NETFramework and full extraction code-paths.

@NikolaMilosavljevic

Copy link
Copy Markdown
Author

Investigating why 2 legs are failing - worked fine locally.

@NikolaMilosavljevic

Copy link
Copy Markdown
Author

Investigating why 2 legs are failing - worked fine locally.

tar in Alpine doesn't recognize --wildcards. Even without this switch, it does not return success even when successfully processing any patterns, which is really odd.

We could likely modify the targets file to enable selective extraction for mainline Linux and Windows.

@NikolaMilosavljevic

Copy link
Copy Markdown
Author

tar command on OSX doesn't support --wildcards switch which is required on Debian-based systems. Sending update soon.

@mmitche

mmitche commented Mar 8, 2023

Copy link
Copy Markdown
Member

@NikolaMilosavljevic I think we should fix this across all the platforms. .NET now has Tar support (.NET 7+). Could we use that instead of the local command line tools?

https://learn.microsoft.com/en-us/dotnet/api/system.formats.tar.tarreader?view=net-7.0

@NikolaMilosavljevic

Copy link
Copy Markdown
Author

@NikolaMilosavljevic I think we should fix this across all the platforms. .NET now has Tar support (.NET 7+). Could we use that instead of the local command line tools?

https://learn.microsoft.com/en-us/dotnet/api/system.formats.tar.tarreader?view=net-7.0

I've updated the PR.

{
#if NETFRAMEWORK
// Run the base tool, which uses external 'tar' command
retVal = base.Execute();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do you happen to know whether this codepath is executed any longer? If it's not, we could just add an NYI exception.

My hunch is it's not.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'm not 100% sure about NETFRAMEWORK code-path, but we also use base implementation (and tar command) for full package extraction, which is still needed.

@mmitche mmitche left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me. Small question about whether the tool-based tar implementation is required.

@NikolaMilosavljevic
NikolaMilosavljevic merged commit cddf8e6 into dotnet:main Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Preview 3 tarball package contains two NETCore.App shared runtimes

2 participants