Add tarfs implementation #265
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We want to have the FS variable available through all the tests, so we we use a common "setup" function to initialise it.
As tarfs is a read-only filesystem backend, we return EROFS (Read-only file system) from any method that makes modifications.
Most of the operations that we want to implement for tarfs.File are already defined in bytes.Reader. We could use a plain slice and implement all the seeking manually, but I think using this is more convenient.
If the call fails, we don't have to close the file
As we modify the struct fields when closing, we don't want to lose the internal representation of the file, in case we want to reopen it. Return a copy of the File, although we keep using the same pointers to tar.Header and buffer.Reader. Maybe we will need to change that in the future.
That test depends too much on the internal imlementation, and it is easier to break if we change it.
To be able to handle directories (File.Readdir, File.Readdirnames), the naive single-map implementation makes it a bit harder to implement. Inspired by the zipfs backend, switch to an internal implementation of a map of directories that contains a map of files, so the directory methods are easier to implement. Also, treat the "virtual" filesystem as absolute, just like zipfs does.
For directory-related operations we will need to access the internal structure in the Fs. As Readdir and Readdirnames are File methods, we need to access such structure from the File.
We added the fs field in the File struct to reference the underlying Fs object, but in the Open cal we were not passing it, making all the opened files to have a nil pointer in that field. Change to make a copy of the original file, and returning that
|
|
|
For some reason this is failing in AppVeyor, but I can't reproduce the panic in my development environment. Is there something I can do to fix it? |
0xmichalis
reviewed
Sep 12, 2020
| var afs *afero.Afero | ||
|
|
||
| func TestMain(m *testing.M) { | ||
| tf, err := os.Open("testdata/t.tar") |
There was a problem hiding this comment.
You probably have to fix this path to work for Windows in order to make appveyor pass.
There was a problem hiding this comment.
Hi, thanks! I think that this is OK, I've checked the logs and the panic caused by I handle the "pseudo-root" entry internally, as I use a literal slash, and to be honest I didn't think on Windows systems. I set up an AppVeyor account and will debug the issue.
|
Hi, I'm closing this PR until I have the tests passing on AppVeyor, will reopen it when it's fixed |
0xmichalis
pushed a commit
that referenced
this pull request
Sep 14, 2020
* Initial commit for tarfs * tarfs: reword "open" status field * tarfs: use TestMain for FS setup We want to have the FS variable available through all the tests, so we we use a common "setup" function to initialise it. * tarfs: test: early exit for nonexisting files * tarfs: create test for filesystem Open * tarfs: implement File.Stat * tarfs: implement Fs.Open * tarfs: return error on non-supported methods As tarfs is a read-only filesystem backend, we return EROFS (Read-only file system) from any method that makes modifications. * tarfs: implement File.data as bytes.Reader Most of the operations that we want to implement for tarfs.File are already defined in bytes.Reader. We could use a plain slice and implement all the seeking manually, but I think using this is more convenient. * tarfs: short format for simple methods * tarfs: add missing closing brace in tests * tarfs: add test for File.ReadAt * tarfs: test File.ReadAt * tarfs: add tests for File.Read * tarfs: implement File.Read * tarfs: add tests for File.Seek * tarfs: implement File.Seek * tarfs: add tests for File.Name * tarfs: implement File.Name * tarfs: add tests for File.Close * tarfs: implement File.Close * tarfs: add tests for OpenFile * tarfs: fix test for Fs.OpenFile If the call fails, we don't have to close the file * tarfs: remove code not needed after using filepath.Clean * tarfs: Open: return a copy of the internal structure As we modify the struct fields when closing, we don't want to lose the internal representation of the file, in case we want to reopen it. Return a copy of the File, although we keep using the same pointers to tar.Header and buffer.Reader. Maybe we will need to change that in the future. * tarfs: implement Fs.OpenFile * tarfs: use Fatalf for unexpected error in TestFsOpen * tarfs: add tests for Fs.Stat * tarfs: implement Fs.Stat * tarfs: remove TestNewFs That test depends too much on the internal imlementation, and it is easier to break if we change it. * tarfs: remove unused code * tarfs: change internal implementation To be able to handle directories (File.Readdir, File.Readdirnames), the naive single-map implementation makes it a bit harder to implement. Inspired by the zipfs backend, switch to an internal implementation of a map of directories that contains a map of files, so the directory methods are easier to implement. Also, treat the "virtual" filesystem as absolute, just like zipfs does. * tarfs: use Fatal errors to avoid panics * tarfs: add pseudoroot * tarfs: add tests for File.Readdir * tarfs: add pointer Fs in the File structure For directory-related operations we will need to access the internal structure in the Fs. As Readdir and Readdirnames are File methods, we need to access such structure from the File. * tarfs: fix error * tarfs: use just the names for TestReaddir, easier than using fill os.FileInfo entries * tarfs: create a copy of the original entry when opening a file We added the fs field in the File struct to reference the underlying Fs object, but in the Open cal we were not passing it, making all the opened files to have a nil pointer in that field. Change to make a copy of the original file, and returning that * tarfs: implement File.Readdir * tarfs: add tests for File.Readdirnames * tarfs: implement Readdirnames * tarfs: add test for File.Name * tarfs: change tests to use the Afero interface instead * tarfs: add tests for Glob from zipfs * tarfs: update main repo references to tarfs * tarfs: use OS-specific file separator for pseudoroot * tarfs: fix path handling in Windows systems
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi,
Heavily inspired by the already existing zipfs implementation, I have implemented the "tar" version. It is a read-only, in-memory representation of the contents of a tarball.
Thanks!