drsybren/Flamenco
1
0
Fork 0
Code Issues 1 Pull Requests Actions Packages Projects Releases Wiki Activity

Shaman: fix bunch of path issues on Windows

Browse Source 
Shaman was made on Linux, using the `path` module, which only supports
forward slashes. This mostly replaces `path` with `path/filepath` to use
native paths and increase Windows compatibility.

The work isn't done yet, though.
This commit is contained in:
Sybren A. Stüvel 2022-03-28 11:02:18 +02:00
parent a6834137b7
commit fe9849b134
9 changed files with 62 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
pkg/shaman/checkout/checkout_test.go
View File

@ -3,7 +3,7 @@ package checkout
import ( import (
"context" "context"
"os" "os"
"path" "path/filepath"
"testing" "testing"
"git.blender.org/flamenco/pkg/api" "git.blender.org/flamenco/pkg/api"
@ -34,21 +34,21 @@ func TestCheckout(t *testing.T) {
} }
// Check the symlinks of the checkout // Check the symlinks of the checkout
coPath := path.Join(manager.checkoutBasePath, actualCheckoutPath) coPath := filepath.Join(manager.checkoutBasePath, actualCheckoutPath)
assert.FileExists(t, path.Join(coPath, "subdir", "replacer.py")) assert.FileExists(t, filepath.Join(coPath, "subdir", "replacer.py"))
assert.FileExists(t, path.Join(coPath, "feed.py")) assert.FileExists(t, filepath.Join(coPath, "feed.py"))
assert.FileExists(t, path.Join(coPath, "httpstuff.py")) assert.FileExists(t, filepath.Join(coPath, "httpstuff.py"))
assert.FileExists(t, path.Join(coPath, "filesystemstuff.py")) assert.FileExists(t, filepath.Join(coPath, "filesystemstuff.py"))
storePath := manager.fileStore.StoragePath() storePath := manager.fileStore.StoragePath()
assertLinksTo(t, path.Join(coPath, "subdir", "replacer.py"), assertLinksTo(t, filepath.Join(coPath, "subdir", "replacer.py"),
path.Join(storePath, "59", "0c148428d5c35fab3ebad2f3365bb469ab9c531b60831f3e826c472027a0b9", "3367.blob")) filepath.Join(storePath, "59", "0c148428d5c35fab3ebad2f3365bb469ab9c531b60831f3e826c472027a0b9", "3367.blob"))
assertLinksTo(t, path.Join(coPath, "feed.py"), assertLinksTo(t, filepath.Join(coPath, "feed.py"),
path.Join(storePath, "80", "b749c27b2fef7255e7e7b3c2029b03b31299c75ff1f1c72732081c70a713a3", "7488.blob")) filepath.Join(storePath, "80", "b749c27b2fef7255e7e7b3c2029b03b31299c75ff1f1c72732081c70a713a3", "7488.blob"))
assertLinksTo(t, path.Join(coPath, "httpstuff.py"), assertLinksTo(t, filepath.Join(coPath, "httpstuff.py"),
path.Join(storePath, "91", "4853599dd2c351ab7b82b219aae6e527e51518a667f0ff32244b0c94c75688", "486.blob")) filepath.Join(storePath, "91", "4853599dd2c351ab7b82b219aae6e527e51518a667f0ff32244b0c94c75688", "486.blob"))
assertLinksTo(t, path.Join(coPath, "filesystemstuff.py"), assertLinksTo(t, filepath.Join(coPath, "filesystemstuff.py"),
path.Join(storePath, "d6", "fc7289b5196cc96748ea72f882a22c39b8833b457fe854ef4c03a01f5db0d3", "7217.blob")) filepath.Join(storePath, "d6", "fc7289b5196cc96748ea72f882a22c39b8833b457fe854ef4c03a01f5db0d3", "7217.blob"))
} }
func assertLinksTo(t *testing.T, linkPath, expectedTarget string) { func assertLinksTo(t *testing.T, linkPath, expectedTarget string) {

2
pkg/shaman/checkout/manager.go
View File

@ -183,7 +183,7 @@ func (m *Manager) EraseCheckout(checkoutID string) error {
// SymlinkToCheckout creates a symlink at symlinkPath to blobPath. // SymlinkToCheckout creates a symlink at symlinkPath to blobPath.
// It does *not* do any validation of the validity of the paths! // It does *not* do any validation of the validity of the paths!
func (m *Manager) SymlinkToCheckout(blobPath, checkoutPath, symlinkRelativePath string) error { func (m *Manager) SymlinkToCheckout(blobPath, checkoutPath, symlinkRelativePath string) error {
symlinkPath := path.Join(checkoutPath, symlinkRelativePath) symlinkPath := filepath.Join(checkoutPath, symlinkRelativePath)
logger := log.With(). logger := log.With().
Str("blobPath", blobPath). Str("blobPath", blobPath).
Str("symlinkPath", symlinkPath). Str("symlinkPath", symlinkPath).

6
pkg/shaman/checkout/manager_test.go
View File

@ -25,7 +25,7 @@ package checkout
import ( import (
"io/ioutil" "io/ioutil"
"os" "os"
"path" "path/filepath"
"testing" "testing"
"time" "time"
@ -46,7 +46,7 @@ func TestSymlinkToCheckout(t *testing.T) {
defer cleanup() defer cleanup()
// Fake an older file. // Fake an older file.
blobPath := path.Join(manager.checkoutBasePath, "jemoeder.blob") blobPath := filepath.Join(manager.checkoutBasePath, "jemoeder.blob")
err := ioutil.WriteFile(blobPath, []byte("op je hoofd"), 0600) err := ioutil.WriteFile(blobPath, []byte("op je hoofd"), 0600)
assert.NoError(t, err) assert.NoError(t, err)
@ -68,7 +68,7 @@ func TestSymlinkToCheckout(t *testing.T) {
stat.ModTime().After(wayBackWhen), stat.ModTime().After(wayBackWhen),
"File must be touched (%v must be later than %v)", stat.ModTime(), wayBackWhen) "File must be touched (%v must be later than %v)", stat.ModTime(), wayBackWhen)
symlinkPath := path.Join(manager.checkoutBasePath, symlinkRelativePath) symlinkPath := filepath.Join(manager.checkoutBasePath, symlinkRelativePath)
stat, err = os.Lstat(symlinkPath) stat, err = os.Lstat(symlinkPath)
assert.NoError(t, err) assert.NoError(t, err)
assert.True(t, stat.Mode()&os.ModeType == os.ModeSymlink, assert.True(t, stat.Mode()&os.ModeType == os.ModeSymlink,

15
pkg/shaman/cleanup_test.go
View File

@ -25,6 +25,7 @@ package shaman
import ( import (
"os" "os"
"path" "path"
"path/filepath"
"testing" "testing"
"time" "time"
@ -43,7 +44,7 @@ func createTestShaman() (*Server, func()) {
func makeOld(shaman *Server, expectOld mtimeMap, relPath string) { func makeOld(shaman *Server, expectOld mtimeMap, relPath string) {
oldTime := time.Now().Add(-2 * shaman.config.GarbageCollect.MaxAge) oldTime := time.Now().Add(-2 * shaman.config.GarbageCollect.MaxAge)
absPath := path.Join(shaman.config.FileStorePath(), relPath) absPath := filepath.Join(shaman.config.FileStorePath(), relPath)
err := os.Chtimes(absPath, oldTime, oldTime) err := os.Chtimes(absPath, oldTime, oldTime)
if err != nil { if err != nil {
@ -95,7 +96,7 @@ func TestGCComponents(t *testing.T) {
server, cleanup := createTestShaman() server, cleanup := createTestShaman()
defer cleanup() defer cleanup()
extraCheckoutDir := path.Join(server.config.TestTempDir, "extra-checkout") extraCheckoutDir := filepath.Join(server.config.TestTempDir, "extra-checkout")
server.config.GarbageCollect.ExtraCheckoutDirs = []string{extraCheckoutDir} server.config.GarbageCollect.ExtraCheckoutDirs = []string{extraCheckoutDir}
filestore.LinkTestFileStore(server.config.FileStorePath()) filestore.LinkTestFileStore(server.config.FileStorePath())
@ -131,10 +132,10 @@ func TestGCComponents(t *testing.T) {
checkoutInfo, err := server.checkoutMan.PrepareCheckout("checkoutID") checkoutInfo, err := server.checkoutMan.PrepareCheckout("checkoutID")
assert.NoError(t, err) assert.NoError(t, err)
err = server.checkoutMan.SymlinkToCheckout(absPaths["3367.blob"], server.config.CheckoutPath(), err = server.checkoutMan.SymlinkToCheckout(absPaths["3367.blob"], server.config.CheckoutPath(),
path.Join(checkoutInfo.RelativePath, "use-of-3367.blob")) filepath.Join(checkoutInfo.RelativePath, "use-of-3367.blob"))
assert.NoError(t, err) assert.NoError(t, err)
err = server.checkoutMan.SymlinkToCheckout(absPaths["781.blob"], extraCheckoutDir, err = server.checkoutMan.SymlinkToCheckout(absPaths["781.blob"], extraCheckoutDir,
path.Join(checkoutInfo.RelativePath, "use-of-781.blob")) filepath.Join(checkoutInfo.RelativePath, "use-of-781.blob"))
assert.NoError(t, err) assert.NoError(t, err)
// There should only be two old file reported now. // There should only be two old file reported now.
@ -179,7 +180,7 @@ func TestGarbageCollect(t *testing.T) {
server, cleanup := createTestShaman() server, cleanup := createTestShaman()
defer cleanup() defer cleanup()
extraCheckoutDir := path.Join(server.config.TestTempDir, "extra-checkout") extraCheckoutDir := filepath.Join(server.config.TestTempDir, "extra-checkout")
server.config.GarbageCollect.ExtraCheckoutDirs = []string{extraCheckoutDir} server.config.GarbageCollect.ExtraCheckoutDirs = []string{extraCheckoutDir}
filestore.LinkTestFileStore(server.config.FileStorePath()) filestore.LinkTestFileStore(server.config.FileStorePath())
@ -201,10 +202,10 @@ func TestGarbageCollect(t *testing.T) {
checkoutInfo, err := server.checkoutMan.PrepareCheckout("checkoutID") checkoutInfo, err := server.checkoutMan.PrepareCheckout("checkoutID")
assert.NoError(t, err) assert.NoError(t, err)
err = server.checkoutMan.SymlinkToCheckout(absPaths["3367.blob"], server.config.CheckoutPath(), err = server.checkoutMan.SymlinkToCheckout(absPaths["3367.blob"], server.config.CheckoutPath(),
path.Join(checkoutInfo.RelativePath, "use-of-3367.blob")) filepath.Join(checkoutInfo.RelativePath, "use-of-3367.blob"))
assert.NoError(t, err) assert.NoError(t, err)
err = server.checkoutMan.SymlinkToCheckout(absPaths["781.blob"], extraCheckoutDir, err = server.checkoutMan.SymlinkToCheckout(absPaths["781.blob"], extraCheckoutDir,
path.Join(checkoutInfo.RelativePath, "use-of-781.blob")) filepath.Join(checkoutInfo.RelativePath, "use-of-781.blob"))
assert.NoError(t, err) assert.NoError(t, err)
// Running the garbage collector should only remove those two unused files. // Running the garbage collector should only remove those two unused files.

18
pkg/shaman/filestore/filestore.go
View File

@ -24,7 +24,7 @@ package filestore
import ( import (
"os" "os"
"path" "path/filepath"
"strconv" "strconv"
"git.blender.org/flamenco/pkg/shaman/config" "git.blender.org/flamenco/pkg/shaman/config"
@ -55,7 +55,7 @@ func New(conf config.Config) *Store {
// Create the base directory structure for this store. // Create the base directory structure for this store.
func (s *Store) createDirectoryStructure() { func (s *Store) createDirectoryStructure() {
mkdir := func(subdir string) { mkdir := func(subdir string) {
path := path.Join(s.baseDir, subdir) path := filepath.Join(s.baseDir, subdir)
logger := log.With().Str("path", path).Logger() logger := log.With().Str("path", path).Logger()
logger.Debug().Msg("shaman: creating directory") logger.Debug().Msg("shaman: creating directory")
@ -75,7 +75,7 @@ func (s *Store) createDirectoryStructure() {
// StoragePath returns the directory path of the 'stored' storage bin. // StoragePath returns the directory path of the 'stored' storage bin.
func (s *Store) StoragePath() string { func (s *Store) StoragePath() string {
return path.Join(s.stored.basePath, s.stored.dirName) return filepath.Join(s.stored.basePath, s.stored.dirName)
} }
// BasePath returns the directory path of the storage. // BasePath returns the directory path of the storage.
@ -86,7 +86,7 @@ func (s *Store) BasePath() string {
// Returns the checksum/filesize dependent parts of the file's path. // Returns the checksum/filesize dependent parts of the file's path.
// To be combined with a base directory, status directory, and status-dependent suffix. // To be combined with a base directory, status directory, and status-dependent suffix.
func (s *Store) partialFilePath(checksum string, filesize int64) string { func (s *Store) partialFilePath(checksum string, filesize int64) string {
return path.Join(checksum[0:2], checksum[2:], strconv.FormatInt(filesize, 10)) return filepath.Join(checksum[0:2], checksum[2:], strconv.FormatInt(filesize, 10))
} }
// ResolveFile checks the status of the file in the store. // ResolveFile checks the status of the file in the store.
@ -136,7 +136,7 @@ func (s *Store) MoveToStored(checksum string, filesize int64, uploadedFilePath s
// Move to the other storage bin. // Move to the other storage bin.
targetPath := s.stored.pathFor(partial) targetPath := s.stored.pathFor(partial)
targetDir, _ := path.Split(targetPath) targetDir, _ := filepath.Split(targetPath)
if err := os.MkdirAll(targetDir, 0777); err != nil { if err := os.MkdirAll(targetDir, 0777); err != nil {
return err return err
} }
@ -161,9 +161,9 @@ func (s *Store) removeFile(filePath string) error {
} }
// Clean up directory structure, but ignore any errors (dirs may not be empty) // Clean up directory structure, but ignore any errors (dirs may not be empty)
directory := path.Dir(filePath) directory := filepath.Dir(filePath)
os.Remove(directory) os.Remove(directory)
os.Remove(path.Dir(directory)) os.Remove(filepath.Dir(directory))
return err return err
} }
@ -177,7 +177,9 @@ func (s *Store) RemoveUploadedFile(filePath string) {
Msg("shaman: RemoveUploadedFile called with file not in 'uploading' storage bin") Msg("shaman: RemoveUploadedFile called with file not in 'uploading' storage bin")
return return
} }
s.removeFile(filePath) // Ignore the error here. It could very well be that the uploaded file has
// been moved somewhere else already.
_ = s.removeFile(filePath)
} }
// RemoveStoredFile removes a file from the 'stored' storage bin. // RemoveStoredFile removes a file from the 'stored' storage bin.

33
pkg/shaman/filestore/filestore_test.go
View File

@ -25,7 +25,7 @@ package filestore
import ( import (
"io/ioutil" "io/ioutil"
"os" "os"
"path" "path/filepath"
"testing" "testing"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
@ -33,13 +33,13 @@ import (
// mustCreateFile creates an empty file. // mustCreateFile creates an empty file.
// The containing directory structure is created as well, if necessary. // The containing directory structure is created as well, if necessary.
func mustCreateFile(filepath string) { func mustCreateFile(file_path string) {
err := os.MkdirAll(path.Dir(filepath), 0777) err := os.MkdirAll(filepath.Dir(file_path), 0777)
if err != nil { if err != nil {
panic(err) panic(err)
} }
file, err := os.Create(filepath) file, err := os.Create(file_path)
if err != nil { if err != nil {
panic(err) panic(err)
} }
@ -50,11 +50,11 @@ func TestCreateDirectories(t *testing.T) {
store := CreateTestStore() store := CreateTestStore()
defer CleanupTestStore(store) defer CleanupTestStore(store)
assert.Equal(t, path.Join(store.baseDir, "uploading", "x"), store.uploading.storagePrefix("x")) assert.Equal(t, filepath.Join(store.baseDir, "uploading", "x"), store.uploading.storagePrefix("x"))
assert.Equal(t, path.Join(store.baseDir, "stored", "x"), store.stored.storagePrefix("x")) assert.Equal(t, filepath.Join(store.baseDir, "stored", "x"), store.stored.storagePrefix("x"))
assert.DirExists(t, path.Join(store.baseDir, "uploading")) assert.DirExists(t, filepath.Join(store.baseDir, "uploading"))
assert.DirExists(t, path.Join(store.baseDir, "stored")) assert.DirExists(t, filepath.Join(store.baseDir, "stored"))
} }
func TestResolveStoredFile(t *testing.T) { func TestResolveStoredFile(t *testing.T) {
@ -65,7 +65,7 @@ func TestResolveStoredFile(t *testing.T) {
assert.Equal(t, "", foundPath) assert.Equal(t, "", foundPath)
assert.Equal(t, StatusDoesNotExist, status) assert.Equal(t, StatusDoesNotExist, status)
fname := path.Join(store.baseDir, "stored", "ab", "cdefxxx", "123.blob") fname := filepath.Join(store.baseDir, "stored", "ab", "cdefxxx", "123.blob")
mustCreateFile(fname) mustCreateFile(fname)
foundPath, status = store.ResolveFile("abcdefxxx", 123, ResolveStoredOnly) foundPath, status = store.ResolveFile("abcdefxxx", 123, ResolveStoredOnly)
@ -85,7 +85,7 @@ func TestResolveUploadingFile(t *testing.T) {
assert.Equal(t, "", foundPath) assert.Equal(t, "", foundPath)
assert.Equal(t, StatusDoesNotExist, status) assert.Equal(t, StatusDoesNotExist, status)
fname := path.Join(store.baseDir, "uploading", "ab", "cdefxxx", "123-unique-code.tmp") fname := filepath.Join(store.baseDir, "uploading", "ab", "cdefxxx", "123-unique-code.tmp")
mustCreateFile(fname) mustCreateFile(fname)
foundPath, status = store.ResolveFile("abcdefxxx", 123, ResolveStoredOnly) foundPath, status = store.ResolveFile("abcdefxxx", 123, ResolveStoredOnly)
@ -137,7 +137,7 @@ func TestMoveToStored(t *testing.T) {
tempLocation := file.Name() tempLocation := file.Name()
err = store.MoveToStored("abcdefxxx", fileSize, file.Name()) err = store.MoveToStored("abcdefxxx", fileSize, file.Name())
assert.NoError(t, err) assert.NoError(t, err, "moving file %s", file.Name())
foundPath, status := store.ResolveFile("abcdefxxx", fileSize, ResolveEverything) foundPath, status := store.ResolveFile("abcdefxxx", fileSize, ResolveEverything)
assert.NotEqual(t, file.Name(), foundPath) assert.NotEqual(t, file.Name(), foundPath)
@ -146,12 +146,7 @@ func TestMoveToStored(t *testing.T) {
assert.FileExists(t, foundPath) assert.FileExists(t, foundPath)
// The entire directory structure should be kept clean. // The entire directory structure should be kept clean.
assertDoesNotExist(t, tempLocation) assert.NoFileExists(t, tempLocation)
assertDoesNotExist(t, path.Dir(tempLocation)) assert.NoDirExists(t, filepath.Dir(tempLocation))
assertDoesNotExist(t, path.Dir(path.Dir(tempLocation))) assert.NoDirExists(t, filepath.Dir(filepath.Dir(tempLocation)))
}
func assertDoesNotExist(t *testing.T, path string) {
_, err := os.Stat(path)
assert.True(t, os.IsNotExist(err), "%s should not exist, err=%v", path, err)
} }

5
pkg/shaman/filestore/substore.go
View File

@ -25,7 +25,6 @@ package filestore
import ( import (
"errors" "errors"
"os" "os"
"path"
"path/filepath" "path/filepath"
) )
@ -41,7 +40,7 @@ var (
) )
func (s *storageBin) storagePrefix(partialPath string) string { func (s *storageBin) storagePrefix(partialPath string) string {
return path.Join(s.basePath, s.dirName, partialPath) return filepath.Join(s.basePath, s.dirName, partialPath)
} }
// Returns whether 'someFullPath' is pointing to a path inside our storage for the given partial path. // Returns whether 'someFullPath' is pointing to a path inside our storage for the given partial path.
@ -95,7 +94,7 @@ func (s *storageBin) openForWriting(partialPath string) (*os.File, error) {
} }
pathOrGlob := s.pathOrGlob(partialPath) pathOrGlob := s.pathOrGlob(partialPath)
dirname, filename := path.Split(pathOrGlob) dirname, filename := filepath.Split(pathOrGlob)
if err := os.MkdirAll(dirname, 0777); err != nil { if err := os.MkdirAll(dirname, 0777); err != nil {
return nil, err return nil, err

15
pkg/shaman/filestore/substore_test.go
View File

@ -24,6 +24,7 @@ package filestore
import ( import (
"os" "os"
"path/filepath"
"testing" "testing"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
@ -34,10 +35,10 @@ func TestStoragePrefix(t *testing.T) {
basePath: "/base", basePath: "/base",
dirName: "testunit", dirName: "testunit",
} }
assert.Equal(t, "/base/testunit", bin.storagePrefix("")) assert.Equal(t, filepath.FromSlash("/base/testunit"), bin.storagePrefix(""))
assert.Equal(t, "/base/testunit", bin.storagePrefix("/")) assert.Equal(t, filepath.FromSlash("/base/testunit"), bin.storagePrefix("/"))
assert.Equal(t, "/base/testunit/xxx", bin.storagePrefix("xxx")) assert.Equal(t, filepath.FromSlash("/base/testunit/xxx"), bin.storagePrefix("xxx"))
assert.Equal(t, "/base/testunit/xxx", bin.storagePrefix("/xxx")) assert.Equal(t, filepath.FromSlash("/base/testunit/xxx"), bin.storagePrefix("/xxx"))
} }
func TestContains(t *testing.T) { func TestContains(t *testing.T) {
@ -45,9 +46,9 @@ func TestContains(t *testing.T) {
basePath: "/base", basePath: "/base",
dirName: "testunit", dirName: "testunit",
} }
assert.True(t, bin.contains("", "/base/testunit/jemoeder.txt")) assert.True(t, bin.contains("", filepath.FromSlash("/base/testunit/jemoeder.txt")))
assert.True(t, bin.contains("jemoeder", "/base/testunit/jemoeder.txt")) assert.True(t, bin.contains("jemoeder", filepath.FromSlash("/base/testunit/jemoeder.txt")))
assert.False(t, bin.contains("jemoeder", "/base/testunit/opjehoofd/jemoeder.txt")) assert.False(t, bin.contains("jemoeder", filepath.FromSlash("/base/testunit/opjehoofd/jemoeder.txt")))
assert.False(t, bin.contains("", "/etc/passwd")) assert.False(t, bin.contains("", "/etc/passwd"))
assert.False(t, bin.contains("/", "/etc/passwd")) assert.False(t, bin.contains("/", "/etc/passwd"))
assert.False(t, bin.contains("/etc", "/etc/passwd")) assert.False(t, bin.contains("/etc", "/etc/passwd"))

4
pkg/shaman/filestore/testing.go
View File

@ -80,7 +80,7 @@ func (s *Store) MustStoreFileForTest(checksum string, filesize int64, contents [
// Panics if there are any errors. // Panics if there are any errors.
func LinkTestFileStore(cloneTo string) { func LinkTestFileStore(cloneTo string) {
_, myFilename, _, _ := runtime.Caller(0) _, myFilename, _, _ := runtime.Caller(0)
fileStorePath := path.Join(path.Dir(path.Dir(myFilename)), "_test_file_store") fileStorePath := filepath.Join(path.Dir(path.Dir(myFilename)), "_test_file_store")
now := time.Now() now := time.Now()
visit := func(visitPath string, info os.FileInfo, err error) error { visit := func(visitPath string, info os.FileInfo, err error) error {
@ -93,7 +93,7 @@ func LinkTestFileStore(cloneTo string) {
return err return err
} }
targetPath := path.Join(cloneTo, relpath) targetPath := filepath.Join(cloneTo, relpath)
if info.IsDir() { if info.IsDir() {
return os.MkdirAll(targetPath, 0755) return os.MkdirAll(targetPath, 0755)
} }