Skip to content
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.

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

Add podman tar for relative symlinks test #280

Merged

Conversation

am11
Copy link
Member

@am11 am11 commented Oct 22, 2022

Test asset for dotnet/runtime#77303.

@akoeplinger
Copy link
Member

Where is that file coming from?

@am11
Copy link
Member Author

am11 commented Oct 24, 2022

@akoeplinger, linked issue has details.

@akoeplinger
Copy link
Member

Ok but I don't think it should be going into the go test assets directory since it's not from their test suite and that might be confusing.

@am11
Copy link
Member Author

am11 commented Oct 24, 2022

We added docker one in the same spirit last week #279. Since they both use golang, I added it in go directory. I'm open to ideas if you prefer another directory for these. 🙂

@akoeplinger
Copy link
Member

I'd just create a separate folder next to golang_tar. The files in that folder are coming from https://github.com/golang/go/tree/master/src/archive/tar/testdata so I think it'd be good to have separation (also license wise)

Btw. looks like there was a new test file added in golang/go@0bf7ee9 that we might want to port too.

@akoeplinger akoeplinger enabled auto-merge (squash) October 25, 2022 10:13
@am11 am11 disabled auto-merge October 25, 2022 10:14
@am11
Copy link
Member Author

am11 commented Oct 25, 2022

It's 1MB, I'll compress it with gz (they had it as bz2, which we don't support OOTB).

@akoeplinger
Copy link
Member

I don't think 1MB is an issue fwiw

@am11
Copy link
Member Author

am11 commented Oct 25, 2022

I don't think 1MB is an issue fwiw

We don't add huge files in this repo. Majority of files in golang dir are < 20 K.

@tmds
Copy link
Member

tmds commented Oct 25, 2022

The tar file from docker added in #279 is an invalid file.
docker (and go <= 1.8) include file type bits which should not be there.
Because it's invalid, that file cannot be generated using the .NET api.

The tar file that gets added here is a valid one.
It is meant to test this branch in the .NET implementation. There is nothing go/podman specific about this.
Rather than adding a file, this archive could be generated by the test using the .NET api.

Personally, I prefer tests generating the archive if possible, as that makes more visible what is special about the archive under test.

@am11
Copy link
Member Author

am11 commented Nov 1, 2022

@akoeplinger, looks like auto-merge got cancelled. can you manually merge?

@akoeplinger
Copy link
Member

I didn't yet because @tmds raised a question about generating the file instead, do we want to do that? or are we fine with the checked in file?

@am11
Copy link
Member Author

am11 commented Nov 1, 2022

I think we can do both. This one is for consistency that the podman and docker archives are exercised (these are small files). It will help projects like https://github.com/WeihanLi/nocker to use System.Formats.Tar with confidence.

@akoeplinger
Copy link
Member

Ok sounds good.

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.

3 participants