From 43cabf89ae72fae2527f50c882096b12e6d8653c Mon Sep 17 00:00:00 2001 From: Cihan Demirci <1352288+femnad@users.noreply.github.com> Date: Sat, 9 Dec 2023 03:51:47 +0300 Subject: [PATCH] fix archive root determination logic (#2) Also add integration tests --- .github/workflows/test.yml | 13 ++- base/settings/settings.go | 11 +++ provision/archive.go | 139 +++++++++++++++++++---------- provision/archive_test.go | 178 +++++++++++++++++++++++++++++++++++++ provision/binary.go | 2 +- provision/link.go | 2 +- provision/provision.go | 7 +- tests/test_archives.py | 140 +++++++++++++++++++++++++++++ 8 files changed, 439 insertions(+), 53 deletions(-) create mode 100644 provision/archive_test.go create mode 100644 tests/test_archives.py diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4104c0b..c67e4d9 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1,7 +1,12 @@ on: + pull_request: + types: + - opened push: branches: - main + workflow_dispatch: + inputs: jobs: test: @@ -10,4 +15,10 @@ jobs: steps: - uses: actions/checkout@v4 - - run: go test ./... + - name: Run Go tests + run: go test ./... + + - name: Run integration tests + run: | + go install + python3 tests/test_archives.py diff --git a/base/settings/settings.go b/base/settings/settings.go index 1b7e599..525a5fc 100644 --- a/base/settings/settings.go +++ b/base/settings/settings.go @@ -10,9 +10,12 @@ import ( "github.com/femnad/fup/internal" ) +const defaultBinPath = "~/bin" + type FactMap map[string]map[string]string type Settings struct { + BinDir string `yaml:"bin_dir"` CloneDir string `yaml:"clone_dir"` EnsureEnv map[string]string `yaml:"ensure_env"` EnsurePaths []string `yaml:"ensure_paths"` @@ -24,6 +27,14 @@ type Settings struct { VirtualEnvDir string `yaml:"virtualenv_dir"` } +func (s Settings) GetBinPath() string { + if s.BinDir != "" { + return s.BinDir + } + + return defaultBinPath +} + func expand(s string, lookup map[string]string) string { var cur bytes.Buffer var out bytes.Buffer diff --git a/provision/archive.go b/provision/archive.go index e1f7a96..2a4adf0 100644 --- a/provision/archive.go +++ b/provision/archive.go @@ -14,35 +14,37 @@ import ( "regexp" "strings" + mapset "github.com/deckarep/golang-set/v2" "github.com/xi2/xz" - marecmd "github.com/femnad/mare/cmd" - "github.com/femnad/fup/base" "github.com/femnad/fup/base/settings" "github.com/femnad/fup/internal" "github.com/femnad/fup/precheck/unless" "github.com/femnad/fup/precheck/when" "github.com/femnad/fup/remote" + "github.com/femnad/mare" + marecmd "github.com/femnad/mare/cmd" ) const ( bufferSize = 8192 dirMode = 0755 xzDictMax = 1 << 27 - tarFileRegex = `\.tar(\.(gz|bz2|xz))$` + tarFileRegex = `\.tar(\.(gz|bz2|xz))?$` ) -type archiveInfo struct { - hasRootDir bool - target string -} - type archiveEntry struct { info os.FileInfo name string } +// archiveRoot stores an archive's root dir and species if the root dir is part of the archive files. +type archiveRoot struct { + hasRootDir bool + target string +} + func processDownload(archive base.Archive, s settings.Settings) (string, error) { url := archive.ExpandURL(s) if url == "" { @@ -66,7 +68,7 @@ func processDownload(archive base.Archive, s settings.Settings) (string, error) return "", err } - return extractFn(response, dirName) + return extractFn(archive, response, dirName) } func getReader(response remote.Response, tempFile *os.File) (io.Reader, error) { @@ -191,8 +193,8 @@ func downloadTempFile(response remote.Response) (string, error) { return tempFilePath, nil } -func isExec(info os.FileInfo) bool { - return info.Mode().Perm()&0100 != 0 +func isExecutableFile(info os.FileInfo) bool { + return !info.IsDir() && info.Mode().Perm()&0100 != 0 } func getFilename(response remote.Response) string { @@ -203,43 +205,75 @@ func getFilename(response remote.Response) string { return filename } -func getArchiveInfo(target string) archiveInfo { - if strings.Index(target, "/") >= 0 { - firstSlash := strings.Index(target, "/") - target = target[:firstSlash] +func commonPrefix(names []string) string { + if len(names) == 0 { + return "" + } + + first := names[0] + minLength := len(first) + for _, name := range names { + if len(name) < minLength { + minLength = len(name) + } + } + + for i := 0; i < minLength; i++ { + currentChar := first[i] + for _, name := range names { + if name[i] != currentChar { + return first[:i] + } + } } - return archiveInfo{target: target, hasRootDir: true} + + return first[:minLength] } -func inspectEntries(entries []archiveEntry, url string) (archiveInfo, error) { - var execs []string +func determineArchiveRoot(archive base.Archive, entries []archiveEntry) (archiveRoot, error) { + names := mare.Map(entries, func(entry archiveEntry) string { + return entry.name + }) + prefix := commonPrefix(names) + roots := mapset.NewSet[string]() + var execs []archiveEntry for _, entry := range entries { - info := entry.info - name := entry.name - if info.IsDir() { - return getArchiveInfo(entry.name), nil - } else if isExec(info) { - execs = append(execs, name) + rootDir := strings.Split(entry.name, "/") + roots.Add(rootDir[0]) + if isExecutableFile(entry.info) { + execs = append(execs, entry) } } - if len(execs) == 1 { - return getArchiveInfo(execs[0]), nil + var hasRootDir bool + var target string + if roots.Cardinality() == 1 { + root, ok := roots.Pop() + if !ok { + return archiveRoot{}, fmt.Errorf("error determining root dir for %s", archive.Url) + } + + hasRootDir = strings.Index(prefix, "/") > -1 + target = root + } else if archive.Name() != "" { + target = archive.Name() + } else { + target = execs[0].name } - return archiveInfo{}, fmt.Errorf("unable to determine root for archive: %s", url) + return archiveRoot{hasRootDir: hasRootDir, target: target}, nil } -func getTarInfo(tempfile string, response remote.Response) (archiveInfo, error) { +func getTarInfo(archive base.Archive, response remote.Response, tempfile string) (archiveRoot, error) { f, err := os.Open(tempfile) if err != nil { - return archiveInfo{}, err + return archiveRoot{}, err } defer f.Close() reader, err := getReader(response, f) if err != nil { - return archiveInfo{}, err + return archiveRoot{}, err } var entries []archiveEntry @@ -258,25 +292,34 @@ func getTarInfo(tempfile string, response remote.Response) (archiveInfo, error) }) } - return inspectEntries(entries, response.URL) + return determineArchiveRoot(archive, entries) } -func getOutputPath(info archiveInfo, fileName, dirName string) string { - if info.hasRootDir { +func getOutputPath(archiveRoot archiveRoot, fileName, dirName string) string { + if archiveRoot.hasRootDir { return filepath.Join(dirName, fileName) } - return filepath.Join(dirName, info.target, fileName) + return filepath.Join(dirName, archiveRoot.target, fileName) +} + +func getAbsTarget(dirName string, root archiveRoot) (string, error) { + wd, err := os.Getwd() + if err != nil { + return "", err + } + + return path.Join(wd, dirName, root.target), nil } // Shamelessly lifted from https://golangdocs.com/tar-gzip-in-golang -func untar(response remote.Response, dirName string) (string, error) { +func untar(archive base.Archive, response remote.Response, dirName string) (string, error) { tempfile, err := downloadTempFile(response) if err != nil { return "", err } - aInfo, err := getTarInfo(tempfile, response) + root, err := getTarInfo(archive, response, tempfile) f, err := os.Open(tempfile) if err != nil { @@ -298,7 +341,7 @@ func untar(response remote.Response, dirName string) (string, error) { panic(err) } - outputPath := getOutputPath(aInfo, header.Name, dirName) + outputPath := getOutputPath(root, header.Name, dirName) err = extractCompressedFile(header.FileInfo(), outputPath, tarReader) if err != nil { return "", err @@ -310,13 +353,13 @@ func untar(response remote.Response, dirName string) (string, error) { return "", err } - return aInfo.target, nil + return getAbsTarget(dirName, root) } -func getZipInfo(tempFile string, response remote.Response) (archiveInfo, error) { +func getZipInfo(archive base.Archive, tempFile string) (archiveRoot, error) { zipArchive, err := zip.OpenReader(tempFile) if err != nil { - return archiveInfo{}, err + return archiveRoot{}, err } var entries []archiveEntry @@ -329,16 +372,16 @@ func getZipInfo(tempFile string, response remote.Response) (archiveInfo, error) err = zipArchive.Close() if err != nil { - return archiveInfo{}, err + return archiveRoot{}, err } - return inspectEntries(entries, response.URL) + return determineArchiveRoot(archive, entries) } -func unzip(response remote.Response, dirName string) (string, error) { +func unzip(archive base.Archive, response remote.Response, dirName string) (string, error) { tempFile, err := downloadTempFile(response) - aInfo, err := getZipInfo(tempFile, response) + root, err := getZipInfo(archive, tempFile) if err != nil { return "", err } @@ -349,7 +392,7 @@ func unzip(response remote.Response, dirName string) (string, error) { } for _, f := range zipArchive.File { - output := getOutputPath(aInfo, f.Name, dirName) + output := getOutputPath(root, f.Name, dirName) err = unzipFile(f, output) if err != nil { return "", err @@ -361,10 +404,10 @@ func unzip(response remote.Response, dirName string) (string, error) { return "", err } - return aInfo.target, nil + return getAbsTarget(dirName, root) } -func getExtractionFn(archive base.Archive, s settings.Settings, contentDisposition string) (func(remote.Response, string) (string, error), error) { +func getExtractionFn(archive base.Archive, s settings.Settings, contentDisposition string) (func(base.Archive, remote.Response, string) (string, error), error) { fileName := archive.ExpandURL(s) if contentDisposition != "" { fileName = contentDisposition @@ -443,7 +486,7 @@ func extractArchive(archive base.Archive, s settings.Settings) { } for _, symlink := range archive.ExpandSymlinks(s, target) { - err = createSymlink(symlink, path.Join(s.ExtractDir, target)) + err = createSymlink(symlink, target, s.BinDir) if err != nil { internal.Log.Errorf("error creating symlink for archive %s: %v", url, err) return diff --git a/provision/archive_test.go b/provision/archive_test.go new file mode 100644 index 0000000..84657ff --- /dev/null +++ b/provision/archive_test.go @@ -0,0 +1,178 @@ +package provision + +import ( + "io/fs" + "reflect" + "testing" + "time" + + "github.com/femnad/fup/base" +) + +type mockFileInfo struct { + mode uint32 + name string + isDir bool +} + +func (m mockFileInfo) Name() string { + //TODO implement me + panic("implement me") +} + +func (m mockFileInfo) Size() int64 { + //TODO implement me + panic("implement me") +} + +func (m mockFileInfo) Mode() fs.FileMode { + return fs.FileMode(m.mode) +} + +func (m mockFileInfo) ModTime() time.Time { + //TODO implement me + panic("implement me") +} + +func (m mockFileInfo) IsDir() bool { + return m.isDir +} + +func (m mockFileInfo) Sys() any { + //TODO implement me + panic("implement me") +} + +func mockExec(name string) mockFileInfo { + return mockFileInfo{mode: 493, name: name, isDir: false} +} + +func mockFile(name string) mockFileInfo { + return mockFileInfo{mode: 420, name: name, isDir: false} +} + +func mockDir(name string) mockFileInfo { + return mockFileInfo{mode: 493, name: name, isDir: true} +} + +func Test_determineArchiveRoot(t *testing.T) { + type args struct { + archive base.Archive + entries []archiveEntry + } + tests := []struct { + name string + args args + want archiveRoot + wantErr bool + }{ + { + name: "Single file in a dir", + args: args{ + entries: []archiveEntry{{ + info: mockExec("foo"), + name: "dir/foo", + }}, + }, + want: archiveRoot{ + hasRootDir: true, + target: "dir", + }, + }, + { + name: "Single file not in a dir", + args: args{ + entries: []archiveEntry{{ + info: mockExec("foo"), + name: "foo", + }}, + }, + want: archiveRoot{ + hasRootDir: false, + target: "foo", + }, + }, + { + name: "Multiple files without root dir with archive name", + args: args{ + archive: base.Archive{Ref: "qux"}, + entries: []archiveEntry{ + { + info: mockExec("foo"), + name: "foo", + }, + { + info: mockDir("dir"), + name: "dir", + }, + { + info: mockFile("baz"), + name: "dir/baz", + }, + }, + }, + want: archiveRoot{ + hasRootDir: false, + target: "qux", + }, + }, + { + name: "Multiple files without root dir and archive name", + args: args{ + entries: []archiveEntry{ + { + info: mockExec("foo"), + name: "foo", + }, + { + info: mockDir("dir"), + name: "dir", + }, + { + info: mockFile("baz"), + name: "dir/baz", + }, + }, + }, + want: archiveRoot{ + hasRootDir: false, + target: "foo", + }, + }, + { + name: "Multiple files with root dir", + args: args{ + entries: []archiveEntry{ + { + info: mockExec("foo"), + name: "qux/baz/foo", + }, + { + info: mockDir("fred"), + name: "qux/fred", + }, + { + info: mockFile("baz"), + name: "qux/bar/baz", + }, + }, + }, + want: archiveRoot{ + hasRootDir: true, + target: "qux", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := determineArchiveRoot(tt.args.archive, tt.args.entries) + if (err != nil) != tt.wantErr { + t.Errorf("determineArchiveRoot() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("determineArchiveRoot() got = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/provision/binary.go b/provision/binary.go index 5b282e1..1582d71 100644 --- a/provision/binary.go +++ b/provision/binary.go @@ -54,7 +54,7 @@ func downloadBinary(binary entity.Binary, config base.Config) error { return err } - return createSymlink(base.NamedLink{Target: name}, binaryDir) + return createSymlink(base.NamedLink{Target: name}, binaryDir, s.BinDir) } func (p Provisioner) downloadBinaries() { diff --git a/provision/link.go b/provision/link.go index fec8ebf..8cd9564 100644 --- a/provision/link.go +++ b/provision/link.go @@ -8,7 +8,7 @@ import ( "github.com/femnad/fup/internal" ) -func createSymlink(symlink base.NamedLink, linkDir string) error { +func createSymlink(symlink base.NamedLink, linkDir, binPath string) error { symlinkTarget := path.Join(linkDir, symlink.Target) symlinkTarget = internal.ExpandUser(symlinkTarget) diff --git a/provision/provision.go b/provision/provision.go index 0a72cf5..5806929 100644 --- a/provision/provision.go +++ b/provision/provision.go @@ -7,8 +7,6 @@ import ( "github.com/femnad/fup/internal" ) -const binPath = "~/bin" - type Provisioner struct { Config base.Config Packager packager @@ -109,6 +107,11 @@ func (p Provisioner) Apply() { func (p Provisioner) extractArchives() { internal.Log.Notice("Extracting archives") + if p.Config.Settings.ExtractDir == "" { + internal.Log.Errorf("Empty archive extraction directory") + return + } + for _, archive := range p.Config.Archives { extractArchive(archive, p.Config.Settings) } diff --git a/tests/test_archives.py b/tests/test_archives.py new file mode 100644 index 0000000..53933d6 --- /dev/null +++ b/tests/test_archives.py @@ -0,0 +1,140 @@ +import dataclasses +import os +import shlex +import shutil +import subprocess +import unittest + +import yaml + +ARTIFACTS_DIR = 'out' +BASE_CONFIG = { + 'archives': [], + 'settings': { + 'bin_dir': f'{ARTIFACTS_DIR}/bin', + 'extract_dir': f'{ARTIFACTS_DIR}/ext', + } +} + + +@dataclasses.dataclass +class Archive: + name: str + url: str + + +@dataclasses.dataclass +class Symlink: + link_name: str + target: str + + +@dataclasses.dataclass +class ArchiveTest: + archive: Archive + name: str + symlink: Symlink + + +TESTS_CASES = [ + ArchiveTest( + name='tar_archive_no_root_dir', + archive=Archive( + name='foo', + url='https://github.com/femnad/fup/releases/download/test-payload/release-no-root-dir.tar', + ), + symlink=Symlink( + link_name='bin/foo', + target='ext/foo/foo' + ), + ), + ArchiveTest( + name='tar_archive_root_dir_different_than_exec', + archive=Archive( + name='foo', + url='https://github.com/femnad/fup/releases/download/test-payload/release-root-dir-different-than-exec.tar', + ), + symlink=Symlink( + link_name='bin/foo', + target='ext/foo-1.2.3-amd64/foo' + ), + ), + ArchiveTest( + name='tar_archive_root_dir_same_as_exec', + archive=Archive( + name='foo', + url='https://github.com/femnad/fup/releases/download/test-payload/release-root-dir-same-as-exec.tar', + ), + symlink=Symlink( + link_name='bin/foo', + target='ext/foo/foo' + ), + ), + ArchiveTest( + name='zip_archive_no_root_dir', + archive=Archive( + name='foo', + url='https://github.com/femnad/fup/releases/download/test-payload/release-no-root-dir.zip', + ), + symlink=Symlink( + link_name='bin/foo', + target='ext/foo/foo' + ), + ), + ArchiveTest( + name='zip_archive_root_dir_different_than_exec', + archive=Archive( + name='foo', + url='https://github.com/femnad/fup/releases/download/test-payload/release-root-dir-different-than-exec.zip', + ), + symlink=Symlink( + link_name='bin/foo', + target='ext/foo-1.2.3-amd64/foo' + ), + ), + ArchiveTest( + name='zip_archive_root_dir_same_as_exec', + archive=Archive( + name='foo', + url='https://github.com/femnad/fup/releases/download/test-payload/release-root-dir-same-as-exec.zip', + ), + symlink=Symlink( + link_name='bin/foo', + target='ext/foo/foo' + ), + ) +] + + +class BaseTestCase(unittest.TestCase): + def tearDown(self): + if os.path.exists(ARTIFACTS_DIR): + shutil.rmtree(ARTIFACTS_DIR) + + +def gen_test(test_case: ArchiveTest): + def test(self): + archives = [test_case.archive.__dict__] + config = BASE_CONFIG | {'archives': archives} + + if not os.path.exists(ARTIFACTS_DIR): + os.mkdir(ARTIFACTS_DIR) + with open(f'{ARTIFACTS_DIR}/fup.yml', 'wt') as fd: + yaml.dump(config, Dumper=yaml.SafeDumper, stream=fd) + + cmd = shlex.split(f'fup -p archive -f {ARTIFACTS_DIR}/fup.yml -b') + proc = subprocess.run(cmd) + self.assertTrue(proc.returncode == 0) + + link_name = os.path.join(os.path.realpath(ARTIFACTS_DIR), test_case.symlink.link_name) + target = os.path.join(os.path.realpath(ARTIFACTS_DIR), test_case.symlink.target) + self.assertEqual(os.path.realpath(link_name), target) + + return test + + +if __name__ == '__main__': + for case in TESTS_CASES: + test_method = gen_test(case) + setattr(BaseTestCase, f'test_{case.name}', test_method) + unittest.TextTestRunner().run(unittest.TestLoader().loadTestsFromTestCase(BaseTestCase))