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

MemMapFS doesn't properly rename directories #141

Closed
kklin opened this issue Oct 20, 2017 · 6 comments
Closed

MemMapFS doesn't properly rename directories #141

kklin opened this issue Oct 20, 2017 · 6 comments
Labels

Comments

@kklin
Copy link
Contributor

kklin commented Oct 20, 2017

When I rename a directory with MemMapFS, the files within it don't get moved. I think the files within should also move to the new directory name (as in the behavior of mv <old_dir> <new_dir>), but maybe I'm misunderstanding how Rename works.

I've written a unit test that shows the error:

// TestRenameDirectory creates a directory with a file, and attempts to rename
// the directory. The child file should also be within the renamed directory.
func TestRenameDirectory(t *testing.T) {
        fs := NewMemMapFs()

        srcDir := "/src-dir"
        dstDir := "/dst-dir"
        filename := "file"
        fileInSrcDir := filepath.Join(srcDir, filename)
        fileInDstDir := filepath.Join(dstDir, filename)

        if err := fs.Mkdir(srcDir, 755); err != nil {
                t.Fatalf("Mkdir failed unexpectedly: %s", err)
        }

        if _, err := fs.Create(fileInSrcDir); err != nil {
                t.Fatalf("Create file failed unexpectedly: %s", err)
        }

        if err := fs.Rename(srcDir, dstDir); err != nil {
                t.Fatalf("Rename failed unexpectedly: %s", err)
        }

        if _, err := fs.Stat(fileInSrcDir); err == nil {
                t.Error("The file should not be in the old directory")
        }

        if _, err := fs.Stat(fileInDstDir); err != nil {
                t.Errorf("The file should be in the new directory: %s", err)
        }
}
--- FAIL: TestRenameDirectory (0.00s)
        memmap_test.go:371: The file should not be in the old directory
        memmap_test.go:375: The file should be in the new directory: open /dst-dir/file: file does not exist

I could help out fixing this, but I'm not sure how to approach it. I took a quick glance at the way the filesystem is represented within MemMapFS, and didn't completely understand it.

Thanks!

@mtaufen
Copy link

mtaufen commented Feb 3, 2018

I also noticed this recently while unit testing something that moves directories with MemMapFS; it would work if I tested with the OS default filesystem, but not with MemMapFS.

@jeanlucmongrain
Copy link

Confirm

func TestAferoRename(t *testing.T) {
	fs := afero.NewMemMapFs()
	require.NoError(t, fs.MkdirAll("/d1/d2/d3", 755))
	f, err := fs.OpenFile("/d1/d2/d3/f1", os.O_CREATE|os.O_WRONLY, 0644)
	require.NoError(t, err)
	_, err = f.WriteString("asaagsd")
	require.NoError(t, err)
	require.NoError(t, f.Close())

	files, err := afero.Glob(fs, "/d1/d2/d3/*")
	require.NoError(t, err)
	require.Len(t, files, 1)

	require.NoError(t, fs.Rename("/d1/d2", "/d1/d4"))

	files, err = afero.Glob(fs, "/d1/d2/d3/*")
	require.NoError(t, err)
	require.Len(t, files, 0)

	files, err = afero.Glob(fs, "/d1/d4/*")
	require.NoError(t, err)
	require.Len(t, files, 1)
}

output:

=== RUN   TestAferoRename
--- FAIL: TestAferoRename (0.00s)
        Error Trace:    server_test.go:43
        Error:          "[/d1/d2/d3/f1]" should have 0 item(s), but has 1

mnussbaum added a commit to mnussbaum/afero that referenced this issue Jul 25, 2018
Prior to this change renaming an in-memory directory would leave the
contents of the directory behind at their old paths. Now directory
entries are updated to use the new directory name.

Fixes spf13#141
mnussbaum added a commit to mnussbaum/afero that referenced this issue Jul 25, 2018
Prior to this change renaming an in-memory directory would leave the
contents of the directory behind at their old paths. Now directory
entries are updated to use the new directory name.

Fixes spf13#141
@Dreznel
Copy link

Dreznel commented Aug 14, 2019

Hello,

Just wanted to chime in and say that I'm having the same trouble with this. I'm unit testing with MemMapFs and finding that fs.Rename doesn't seem to be doing anything. I might be doing something wrong, but the code seems to work fine when we're using an OS filesystem.

(here's my code, if it helps at all)

func (suite *UtilsTestSuite) TestRollbackUpdatePackages_RestoresWhenNoActiveInstallation() {
	_ = suite.FileSystem.MkdirAll("test-library/__OLD__CatsAndOranges", 0755)
	_, _ = suite.FileSystem.Create("test-library/__OLD__CatsAndOranges/DESCRIPTION")
	f, _ := suite.FileSystem.Open("test-library/__OLD__CatsAndOranges/DESCRIPTION")
	f.WriteString("contents")
	defer f.Close()

	updateAttemptFixture := []UpdateAttempt{
		UpdateAttempt{
			Package:                "CatsAndOranges",
			BackupPackageDirectory: "test-library/__OLD__CatsAndOranges",
			ActivePackageDirectory: "test-library/CatsAndOranges",
			NewVersion:             "2",
			OldVersion:             "1",
		},
	}

        // Should call fs.Rename("test-library/__OLD__CatsAndOranges", "test-library/CatsAndOranges")
	RollbackUpdatePackages(suite.FileSystem, updateAttemptFixture)

        //Line fails
	suite.True(afero.DirExists(suite.FileSystem, "/test-library/CatsAndOranges"))
        //Line fails
	suite.True(afero.Exists(suite.FileSystem, "/test-library/CatsAndOranges/DESCRIPTION"))

         //Both of these succeed
	suite.False(afero.DirExists(suite.FileSystem, "/test-library/__OLD__CatsAndOranges"))
	suite.False(afero.Exists(suite.FileSystem, "/test-library/__OLD__CatsAndOranges/DESCRIPTION"))

}

@0xmichalis 0xmichalis added the bug label Mar 27, 2020
JohnStarich added a commit to JohnStarich/afero that referenced this issue Jun 28, 2020
da-ar added a commit to da-ar/afero that referenced this issue Sep 2, 2021
@maxkaneco
Copy link

Please fix this issue. This sorta defeat the whole point of using afero, which allows me to use memfs for testing.

@hanagantig
Copy link
Contributor

hanagantig commented Jun 25, 2022

Please check my PR with fix. I can't use the library with this bug.

bep pushed a commit that referenced this issue Feb 23, 2023
@hanagantig
Copy link
Contributor

hanagantig commented May 1, 2023

Actually this merged PR will not work.

You need to update underlying memdir map as well.
here is a case where you will get a panic:

fs := NewMemMapFs()

err := fs.MkdirAll("src/originDir", 0o777)
if err != nil {
    t.Fatalf("MkDirAll failed: %s", err)
}

f, err := fs.Create("src/originDir/originFile.txt")
if err != nil {
    t.Fatalf("Create failed: %s", err)
}
if err = f.Close(); err != nil {
    t.Fatalf("Close failed: %s", err)
}

err = fs.Rename("src/originDir", "src/updatedDir")
if err != nil {
t.Fatalf("Rename failed: %s", err)
}

err = fs.Rename("src/updatedDir/originFile.txt", "src/updatedDir/updatedFile.txt")
if err != nil {
t.Fatalf("Rename failed: %s", err)
}

@bep please have a look to my PR which I did almost a year ago.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants