Merge pull request #28 from cmaglie/prepare-v4

Add Remove to FS interface. Prepare for release v4.

Author: cmaglie

README.md

@@ -4,7 +4,7 @@
 [![GitHub license](https://img.shields.io/badge/license-MIT-blue.svg)](https://raw.githubusercontent.com/codeclysm/extract/master/LICENSE)
 [![Godoc Reference](https://img.shields.io/badge/Godoc-Reference-blue.svg)](https://godoc.org/github.com/codeclysm/extract)
 
-    import "github.com/codeclysm/extract/v3"
+    import "github.com/codeclysm/extract/v4"
 
 Package extract allows to extract archives in zip, tar.gz or tar.bz2 formats
 easily.
@@ -54,6 +54,7 @@ type FS interface {
     MkdirAll(string, os.FileMode) error
     OpenFile(name string, flag int, perm os.FileMode) (*os.File, error)
     Symlink(string, string) error
+    Remove(path string) error
 }
 ```
 

extract.go

@@ -92,7 +92,6 @@ func Zip(ctx context.Context, body io.Reader, location string, rename Renamer) e
 type fs struct{}
 
 func (f fs) Link(oldname, newname string) error {
-	_ = os.Remove(newname) // Ignore error. We don't care if the file doesn't exist.
 	return os.Link(oldname, newname)
 }
 
@@ -101,10 +100,13 @@ func (f fs) MkdirAll(path string, perm os.FileMode) error {
 }
 
 func (f fs) Symlink(oldname, newname string) error {
-	_ = os.Remove(newname) // Ignore error. We don't care if the file doesn't exist.
 	return os.Symlink(oldname, newname)
 }
 
 func (f fs) OpenFile(name string, flag int, perm os.FileMode) (*os.File, error) {
 	return os.OpenFile(name, flag, perm)
 }
+
+func (f fs) Remove(path string) error {
+	return os.Remove(path)
+}

extract_test.go

@@ -14,7 +14,7 @@ import (
 	"testing"
 
 	"github.com/arduino/go-paths-helper"
-	"github.com/codeclysm/extract/v3"
+	"github.com/codeclysm/extract/v4"
 	"github.com/stretchr/testify/require"
 )
 
@@ -222,43 +222,45 @@ func TestExtract(t *testing.T) {
 
 func TestExtractIdempotency(t *testing.T) {
 	for _, test := range ExtractCases {
-		dir, _ := os.MkdirTemp("", "")
-		dir = filepath.Join(dir, "test")
-		data, err := os.ReadFile(test.Archive)
-		if err != nil {
-			t.Fatal(err)
-		}
-
-		var extractFn func(context.Context, io.Reader, string, extract.Renamer) error
-		switch filepath.Ext(test.Archive) {
-		case ".bz2":
-			extractFn = extract.Bz2
-		case ".gz":
-			extractFn = extract.Gz
-		case ".zip":
-			extractFn = extract.Zip
-		case ".mistery":
-			extractFn = extract.Archive
-		default:
-			t.Fatal("unknown error")
-		}
-
-		buffer := bytes.NewBuffer(data)
-		if err = extractFn(context.Background(), buffer, dir, test.Renamer); err != nil {
-			t.Fatal(test.Name, ": Should not fail first extraction: "+err.Error())
-		}
-
-		buffer = bytes.NewBuffer(data)
-		if err = extractFn(context.Background(), buffer, dir, test.Renamer); err != nil {
-			t.Fatal(test.Name, ": Should not fail second extraction: "+err.Error())
-		}
-
-		testWalk(t, dir, test.Files)
-
-		err = os.RemoveAll(dir)
-		if err != nil {
-			t.Fatal(err)
-		}
+		t.Run(test.Name, func(t *testing.T) {
+			dir, _ := os.MkdirTemp("", "")
+			dir = filepath.Join(dir, "test")
+			data, err := os.ReadFile(test.Archive)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			var extractFn func(context.Context, io.Reader, string, extract.Renamer) error
+			switch filepath.Ext(test.Archive) {
+			case ".bz2":
+				extractFn = extract.Bz2
+			case ".gz":
+				extractFn = extract.Gz
+			case ".zip":
+				extractFn = extract.Zip
+			case ".mistery":
+				extractFn = extract.Archive
+			default:
+				t.Fatal("unknown error")
+			}
+
+			buffer := bytes.NewBuffer(data)
+			if err = extractFn(context.Background(), buffer, dir, test.Renamer); err != nil {
+				t.Fatal(test.Name, ": Should not fail first extraction: "+err.Error())
+			}
+
+			buffer = bytes.NewBuffer(data)
+			if err = extractFn(context.Background(), buffer, dir, test.Renamer); err != nil {
+				t.Fatal(test.Name, ": Should not fail second extraction: "+err.Error())
+			}
+
+			testWalk(t, dir, test.Files)
+
+			err = os.RemoveAll(dir)
+			if err != nil {
+				t.Fatal(err)
+			}
+		})
 	}
 }
 

extractor.go

@@ -25,7 +25,6 @@ import (
 type Extractor struct {
 	FS interface {
 		// Link creates newname as a hard link to the oldname file. If there is an error, it will be of type *LinkError.
-		// Differently from os.Link, if newname already exists it will be overwritten.
 		Link(oldname, newname string) error
 
 		// MkdirAll creates the directory path and all his parents if needed.
@@ -35,8 +34,10 @@ type Extractor struct {
 		OpenFile(name string, flag int, perm os.FileMode) (*os.File, error)
 
 		// Symlink creates newname as a symbolic link to oldname.
-		// Differently from os.Symlink, if newname already exists it will be overwritten.
 		Symlink(oldname, newname string) error
+
+		// Remove removes the named file or (empty) directory.
+		Remove(path string) error
 	}
 }
 
@@ -230,6 +231,7 @@ func (e *Extractor) Tar(ctx context.Context, body io.Reader, location string, re
 			return errors.New("interrupted")
 		default:
 		}
+		_ = e.FS.Remove(links[i].Path)
 		if err := e.FS.Link(links[i].Name, links[i].Path); err != nil {
 			return errors.Annotatef(err, "Create link %s", links[i].Path)
 		}
@@ -241,6 +243,7 @@ func (e *Extractor) Tar(ctx context.Context, body io.Reader, location string, re
 			return errors.New("interrupted")
 		default:
 		}
+		_ = e.FS.Remove(symlink.Path)
 		if err := e.FS.Symlink(symlink.Name, symlink.Path); err != nil {
 			return errors.Annotatef(err, "Create link %s", symlink.Path)
 		}
@@ -344,6 +347,7 @@ func (e *Extractor) Zip(ctx context.Context, body io.Reader, location string, re
 			return errors.New("interrupted")
 		default:
 		}
+		_ = e.FS.Remove(link.Path)
 		if err := e.FS.Symlink(link.Name, link.Path); err != nil {
 			return errors.Annotatef(err, "Create link %s", link.Path)
 		}
@@ -358,6 +362,7 @@ func (e *Extractor) copy(ctx context.Context, path string, mode os.FileMode, src
 	if err != nil {
 		return err
 	}
+	_ = e.FS.Remove(path)
 	file, err := e.FS.OpenFile(path, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, mode)
 	if err != nil {
 		return err

extractor_test.go

@@ -10,7 +10,7 @@ import (
 	"testing"
 
 	"github.com/arduino/go-paths-helper"
-	"github.com/codeclysm/extract/v3"
+	"github.com/codeclysm/extract/v4"
 	"github.com/stretchr/testify/require"
 )
 
@@ -106,7 +106,6 @@ type MockDisk struct {
 func (m MockDisk) Link(oldname, newname string) error {
 	oldname = filepath.Join(m.Base, oldname)
 	newname = filepath.Join(m.Base, newname)
-	_ = os.Remove(newname)
 	return os.Link(oldname, newname)
 }
 
@@ -118,11 +117,14 @@ func (m MockDisk) MkdirAll(path string, perm os.FileMode) error {
 func (m MockDisk) Symlink(oldname, newname string) error {
 	oldname = filepath.Join(m.Base, oldname)
 	newname = filepath.Join(m.Base, newname)
-	_ = os.Remove(newname)
 	return os.Symlink(oldname, newname)
 }
 
 func (m MockDisk) OpenFile(name string, flag int, perm os.FileMode) (*os.File, error) {
 	name = filepath.Join(m.Base, name)
 	return os.OpenFile(name, flag, perm)
 }
+
+func (m MockDisk) Remove(path string) error {
+	return os.Remove(filepath.Join(m.Base, path))
+}

go.mod

@@ -1,4 +1,4 @@
-module github.com/codeclysm/extract/v3
+module github.com/codeclysm/extract/v4
 
 go 1.22
 

loggingfs_test.go

@@ -29,6 +29,8 @@ func (op *LoggedOp) String() string {
 		return fmt.Sprintf("mkdirall %v %s", op.Mode, op.Path)
 	case "open":
 		return fmt.Sprintf("open     %v %s (flags=%04x)", op.Mode, op.Path, op.Flags)
+	case "remove":
+		return fmt.Sprintf("remove   %v", op.Path)
 	}
 	panic("unknown LoggedOP " + op.Op)
 }
@@ -39,8 +41,7 @@ func (m *LoggingFS) Link(oldname, newname string) error {
 		OldPath: oldname,
 		Path:    newname,
 	})
-	_ = os.Remove(newname)
-	return nil
+	return os.Link(oldname, newname)
 }
 
 func (m *LoggingFS) MkdirAll(path string, perm os.FileMode) error {
@@ -49,7 +50,7 @@ func (m *LoggingFS) MkdirAll(path string, perm os.FileMode) error {
 		Path: path,
 		Mode: perm,
 	})
-	return nil
+	return os.MkdirAll(path, perm)
 }
 
 func (m *LoggingFS) Symlink(oldname, newname string) error {
@@ -58,8 +59,7 @@ func (m *LoggingFS) Symlink(oldname, newname string) error {
 		OldPath: oldname,
 		Path:    newname,
 	})
-	_ = os.Remove(newname)
-	return nil
+	return os.Symlink(oldname, newname)
 }
 
 func (m *LoggingFS) OpenFile(name string, flags int, perm os.FileMode) (*os.File, error) {
@@ -72,6 +72,17 @@ func (m *LoggingFS) OpenFile(name string, flags int, perm os.FileMode) (*os.File
 	return os.OpenFile(os.DevNull, flags, perm)
 }
 
+func (m *LoggingFS) Remove(path string) error {
+	err := os.Remove(path)
+	op := &LoggedOp{
+		Op:   "remove",
+		Path: path,
+	}
+	m.Journal = append(m.Journal, op)
+	fmt.Println("FS>", op)
+	return err
+}
+
 func (m *LoggingFS) String() string {
 	res := ""
 	for _, op := range m.Journal {