diff --git a/.prettierignore b/.prettierignore index df8e1481..fdbfccbf 100644 --- a/.prettierignore +++ b/.prettierignore @@ -1,3 +1,3 @@ # See: https://prettier.io/docs/en/ignore.html#ignoring-files-prettierignore -/test/testdata/golden/logs/ +/test/testdata/test_all/golden/logs/ diff --git a/internal/libraries/git_integration_test.go b/internal/libraries/git_integration_test.go index 77a09ea8..30c3d0f5 100644 --- a/internal/libraries/git_integration_test.go +++ b/internal/libraries/git_integration_test.go @@ -31,7 +31,6 @@ import ( "arduino.cc/repository/internal/libraries/db" "arduino.cc/repository/internal/libraries/gitutils" - "github.com/go-git/go-git/v5" "github.com/stretchr/testify/require" ) @@ -63,15 +62,7 @@ func TestUpdateLibraryJson(t *testing.T) { tag, err := tags.Next() require.NoError(t, err) - repoTree, err := r.Repository.Worktree() - require.NoError(t, err) - // Annotated tags have their own hash, different from the commit hash, so the tag must be resolved before checkout - resolvedTag, err := gitutils.ResolveTag(tag, r.Repository) - require.NoError(t, err) - err = repoTree.Checkout(&git.CheckoutOptions{Hash: *resolvedTag, Force: true}) - require.NoError(t, err) - err = repoTree.Clean(&git.CleanOptions{Dir: true}) - require.NoError(t, err) + err = gitutils.CheckoutTag(r.Repository, tag) library, err := GenerateLibraryFromRepo(r) require.NoError(t, err) diff --git a/internal/libraries/gitutils/gitutils.go b/internal/libraries/gitutils/gitutils.go index 80dbee76..e45bb71b 100644 --- a/internal/libraries/gitutils/gitutils.go +++ b/internal/libraries/gitutils/gitutils.go @@ -24,6 +24,7 @@ package gitutils import ( + "fmt" "sort" "github.com/go-git/go-git/v5" @@ -31,8 +32,8 @@ import ( "github.com/go-git/go-git/v5/plumbing/object" ) -// ResolveTag returns the commit hash associated with a tag. -func ResolveTag(tag *plumbing.Reference, repository *git.Repository) (*plumbing.Hash, error) { +// resolveTag returns the commit hash associated with a tag. +func resolveTag(tag *plumbing.Reference, repository *git.Repository) (*plumbing.Hash, error) { // Annotated tags have their own hash, different from the commit hash, so the tag must be resolved to get the has for // the associated commit. // Tags may point to any Git object. Although not common, this can include tree and blob objects in addition to commits. @@ -117,7 +118,7 @@ func SortedCommitTags(repository *git.Repository) ([]*plumbing.Reference, error) // Annotated tags have their own hash, different from the commit hash, so tags must be resolved before // cross-referencing against the commit hashes. - resolvedTag, err := ResolveTag(tag, repository) + resolvedTag, err := resolveTag(tag, repository) if err != nil { // Non-commit object tags are not included in the sorted list. continue @@ -156,3 +157,66 @@ func SortedCommitTags(repository *git.Repository) ([]*plumbing.Reference, error) return sortedTags, nil } + +// CheckoutTag checks out the repository to the given tag. +func CheckoutTag(repository *git.Repository, tag *plumbing.Reference) error { + repoTree, err := repository.Worktree() + if err != nil { + return err + } + + // Annotated tags have their own hash, different from the commit hash, so the tag must be resolved before checkout + resolvedTag, err := resolveTag(tag, repository) + if err != nil { + return err + } + + if err = repoTree.Checkout(&git.CheckoutOptions{Hash: *resolvedTag, Force: true}); err != nil { + return err + } + + // Ensure the repository is checked out to a clean state. + // Because it might not succeed on the first attempt, a retry is allowed. + for range [2]int{} { + clean, err := cleanRepository(repoTree) + if err != nil { + return err + } + if clean { + return nil + } + } + + return fmt.Errorf("failed to get repository to clean state") +} + +func cleanRepository(repoTree *git.Worktree) (bool, error) { + // Remove now-empty folders which are left behind after checkout. These would not be removed by the reset action. + // Remove untracked files. These would also be removed by the reset action. + if err := repoTree.Clean(&git.CleanOptions{Dir: true}); err != nil { + return false, err + } + + // Remove untracked files and reset tracked files to clean state. + // Even though in theory it shouldn't ever be necessary to do a hard reset in this application, under certain + // circumstances, go-git can fail to complete checkout, while not even returning an error. This results in an + // unexpected dirty repository state, which is corrected via a hard reset. + // See: https://github.com/go-git/go-git/issues/99 + if err := repoTree.Reset(&git.ResetOptions{Mode: git.HardReset}); err != nil { + return false, err + } + + // Get status to detect some forms of failed cleaning. + repoStatus, err := repoTree.Status() + if err != nil { + return false, err + } + + // IsClean() detects: + // - Untracked files + // - Modified tracked files + // This does not detect: + // - Empty directories + // - Ignored files + return repoStatus.IsClean(), nil +} diff --git a/internal/libraries/gitutils/gitutils_test.go b/internal/libraries/gitutils/gitutils_test.go index 1c4423ea..53f8f411 100644 --- a/internal/libraries/gitutils/gitutils_test.go +++ b/internal/libraries/gitutils/gitutils_test.go @@ -84,7 +84,7 @@ func TestResolveTag(t *testing.T) { } { testName := fmt.Sprintf("%s, %s", testTable.objectTypeName, annotationConfig.descriptor) tag := makeTag(t, repository, testName, testTable.objectHash, annotationConfig.annotated) - resolvedTag, err := ResolveTag(tag, repository) + resolvedTag, err := resolveTag(tag, repository) testTable.errorAssertion(t, err, fmt.Sprintf("%s tag resolution error", testName)) if err == nil { assert.Equal(t, testTable.objectHash, *resolvedTag, fmt.Sprintf("%s tag resolution", testName)) @@ -139,9 +139,69 @@ func TestSortedCommitTags(t *testing.T) { assert.Equal(t, tags, sorted) } +func TestCheckoutTag(t *testing.T) { + // Create a folder for the test repository. + repositoryPath, err := paths.TempDir().MkTempDir("gitutils-TestCheckoutTag-repo") + require.NoError(t, err) + + // Create test repository. + repository, err := git.PlainInit(repositoryPath.String(), false) + require.NoError(t, err) + + // Generate meaningless commit history, creating some tags along the way. + var tags []*plumbing.Reference + tags = append(tags, makeTag(t, repository, "1.0.0", makeCommit(t, repository, repositoryPath), true)) + makeCommit(t, repository, repositoryPath) + makeCommit(t, repository, repositoryPath) + tags = append(tags, makeTag(t, repository, "1.0.1", makeCommit(t, repository, repositoryPath), true)) + makeCommit(t, repository, repositoryPath) + makeTag(t, repository, "tree-tag", getTreeHash(t, repository), true) + tags = append(tags, makeTag(t, repository, "1.0.2", makeCommit(t, repository, repositoryPath), true)) + makeTag(t, repository, "blob-tag", getBlobHash(t, repository), true) + trackedFilePath, _ := commitFile(t, repository, repositoryPath) + + for _, tag := range tags { + // Put the repository into a dirty state. + // Add an untracked file. + _, err = paths.WriteToTempFile([]byte{}, repositoryPath, "gitutils-TestCheckoutTag-tempfile") + require.NoError(t, err) + // Modify a tracked file. + err = trackedFilePath.WriteFile([]byte{42}) + require.NoError(t, err) + // Create empty folder. + emptyFolderPath, err := repositoryPath.MkTempDir("gitutils-TestCheckoutTag-emptyFolder") + require.NoError(t, err) + + err = CheckoutTag(repository, tag) + assert.NoError(t, err, fmt.Sprintf("Checking out tag %s", tag)) + + expectedHash, err := resolveTag(tag, repository) + require.NoError(t, err) + headRef, err := repository.Head() + require.NoError(t, err) + assert.Equal(t, *expectedHash, headRef.Hash(), "HEAD is at tag") + + // Check if cleanup was successful. + tree, err := repository.Worktree() + require.NoError(t, err) + status, err := tree.Status() + require.NoError(t, err) + assert.True(t, status.IsClean(), "Repository is clean") + emptyFolderExists, err := emptyFolderPath.ExistCheck() + require.NoError(t, err) + assert.False(t, emptyFolderExists, "Empty folder was removed") + } +} + // makeCommit creates a test commit in the given repository and returns its plumbing.Hash object. func makeCommit(t *testing.T, repository *git.Repository, repositoryPath *paths.Path) plumbing.Hash { - _, err := paths.WriteToTempFile([]byte{}, repositoryPath, "gitutils-makeCommit-tempfile") + _, hash := commitFile(t, repository, repositoryPath) + return hash +} + +// commitFile commits a file in the given repository and returns its path and the commit's plumbing.Hash object. +func commitFile(t *testing.T, repository *git.Repository, repositoryPath *paths.Path) (*paths.Path, plumbing.Hash) { + filePath, err := paths.WriteToTempFile([]byte{}, repositoryPath, "gitutils-makeCommit-tempfile") require.Nil(t, err) worktree, err := repository.Worktree() @@ -163,7 +223,7 @@ func makeCommit(t *testing.T, repository *git.Repository, repositoryPath *paths. ) require.Nil(t, err) - return commit + return filePath, commit } // getTreeHash returns the plumbing.Hash object for an arbitrary Git tree object. diff --git a/sync_libraries.go b/sync_libraries.go index 21fcbd5d..0c38cda1 100644 --- a/sync_libraries.go +++ b/sync_libraries.go @@ -37,7 +37,6 @@ import ( "arduino.cc/repository/internal/libraries/gitutils" "arduino.cc/repository/internal/libraries/hash" cc "github.com/arduino/golang-concurrent-workers" - "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" ) @@ -249,27 +248,10 @@ func syncLibraryTaggedRelease(logger *log.Logger, repo *libraries.Repository, ta // Checkout desired tag logger.Printf("Checking out tag: %s", tag.Name().Short()) - - repoTree, err := repo.Repository.Worktree() - if err != nil { - return err - } - - // Annotated tags have their own hash, different from the commit hash, so the tag must be resolved before checkout - resolvedTag, err := gitutils.ResolveTag(tag, repo.Repository) - if err != nil { - // All unresolvable tags were already removed by gitutils.SortedCommitTags(), so there will never be an error under normal conditions. - panic(err) - } - - if err = repoTree.Checkout(&git.CheckoutOptions{Hash: *resolvedTag, Force: true}); err != nil { + if err := gitutils.CheckoutTag(repo.Repository, tag); err != nil { return fmt.Errorf("Error checking out repo: %s", err) } - if err = repoTree.Clean(&git.CleanOptions{Dir: true}); err != nil { - return fmt.Errorf("Error cleaning repo: %s", err) - } - // Create library metadata from library.properties library, err := libraries.GenerateLibraryFromRepo(repo) if err != nil { diff --git a/test/test_all.py b/test/test_all.py index 9752c4b7..430eee25 100644 --- a/test/test_all.py +++ b/test/test_all.py @@ -58,7 +58,7 @@ def test_all(run_command, working_dir): libraries_repository_engine_command = [ working_dir_path.joinpath("config.json"), - test_data_path.joinpath("repos.txt"), + test_data_path.joinpath("test_all", "repos.txt"), ] # Run the engine @@ -69,12 +69,12 @@ def test_all(run_command, working_dir): check_libraries(configuration=configuration) check_logs( configuration=configuration, - golden_logs_parent_path=test_data_path.joinpath("golden", "logs", "generate"), + golden_logs_parent_path=test_data_path.joinpath("test_all", "golden", "logs", "generate"), logs_subpath=pathlib.Path("github.com", "arduino-libraries", "ArduinoCloudThing", "index.html"), ) check_logs( configuration=configuration, - golden_logs_parent_path=test_data_path.joinpath("golden", "logs", "generate"), + golden_logs_parent_path=test_data_path.joinpath("test_all", "golden", "logs", "generate"), logs_subpath=pathlib.Path("github.com", "arduino-libraries", "SpacebrewYun", "index.html"), ) check_index(configuration=configuration) @@ -87,12 +87,12 @@ def test_all(run_command, working_dir): check_libraries(configuration=configuration) check_logs( configuration=configuration, - golden_logs_parent_path=test_data_path.joinpath("golden", "logs", "update"), + golden_logs_parent_path=test_data_path.joinpath("test_all", "golden", "logs", "update"), logs_subpath=pathlib.Path("github.com", "arduino-libraries", "ArduinoCloudThing", "index.html"), ) check_logs( configuration=configuration, - golden_logs_parent_path=test_data_path.joinpath("golden", "logs", "update"), + golden_logs_parent_path=test_data_path.joinpath("test_all", "golden", "logs", "update"), logs_subpath=pathlib.Path("github.com", "arduino-libraries", "SpacebrewYun", "index.html"), ) check_index(configuration=configuration) @@ -160,7 +160,9 @@ def check_index(configuration): del release["checksum"] # Load golden index - golden_library_index_template = test_data_path.joinpath("golden", "library_index.json").read_text(encoding="utf-8") + golden_library_index_template = test_data_path.joinpath("test_all", "golden", "library_index.json").read_text( + encoding="utf-8" + ) # Fill in mutable content golden_library_index_string = string.Template(template=golden_library_index_template).substitute( base_download_url=configuration["BaseDownloadUrl"] @@ -173,6 +175,43 @@ def check_index(configuration): assert release in golden_library_index["libraries"] +# The engine's Git code struggles to get a clean checkout of releases under some circumstances. +def test_clean_checkout(run_command, working_dir): + working_dir_path = pathlib.Path(working_dir) + configuration = { + "BaseDownloadUrl": "http://www.example.com/libraries/", + "LibrariesFolder": working_dir_path.joinpath("libraries").as_posix(), + "LogsFolder": working_dir_path.joinpath("logs").as_posix(), + "LibrariesDB": working_dir_path.joinpath("libraries_db.json").as_posix(), + "LibrariesIndex": working_dir_path.joinpath("libraries", "library_index.json").as_posix(), + "GitClonesFolder": working_dir_path.joinpath("gitclones").as_posix(), + "DoNotRunClamav": True, + # Arduino Lint should be installed under PATH + "ArduinoLintPath": "", + } + + # Generate configuration file + with working_dir_path.joinpath("config.json").open("w", encoding="utf-8") as configuration_file: + json.dump(obj=configuration, fp=configuration_file, indent=2) + + libraries_repository_engine_command = [ + working_dir_path.joinpath("config.json"), + test_data_path.joinpath("test_clean_checkout", "repos.txt"), + ] + + # Run the engine + result = run_command(cmd=libraries_repository_engine_command) + assert result.ok + + # Load generated index + with pathlib.Path(configuration["LibrariesIndex"]).open(mode="r", encoding="utf-8") as library_index_file: + library_index = json.load(fp=library_index_file) + + for release in library_index["libraries"]: + # ssd1306@1.0.0 contains a .exe and so should fail validation. + assert not (release["name"] == "ssd1306" and release["version"] == "1.0.0") + + @pytest.fixture(scope="function") def run_command(pytestconfig, working_dir) -> typing.Callable[..., invoke.runners.Result]: """Provide a wrapper around invoke's `run` API so that every test will work in the same temporary folder. diff --git a/test/testdata/golden/library_index.json b/test/testdata/test_all/golden/library_index.json similarity index 100% rename from test/testdata/golden/library_index.json rename to test/testdata/test_all/golden/library_index.json diff --git a/test/testdata/golden/logs/generate/github.com/arduino-libraries/ArduinoCloudThing/index.html b/test/testdata/test_all/golden/logs/generate/github.com/arduino-libraries/ArduinoCloudThing/index.html similarity index 100% rename from test/testdata/golden/logs/generate/github.com/arduino-libraries/ArduinoCloudThing/index.html rename to test/testdata/test_all/golden/logs/generate/github.com/arduino-libraries/ArduinoCloudThing/index.html diff --git a/test/testdata/golden/logs/generate/github.com/arduino-libraries/SpacebrewYun/index.html b/test/testdata/test_all/golden/logs/generate/github.com/arduino-libraries/SpacebrewYun/index.html similarity index 100% rename from test/testdata/golden/logs/generate/github.com/arduino-libraries/SpacebrewYun/index.html rename to test/testdata/test_all/golden/logs/generate/github.com/arduino-libraries/SpacebrewYun/index.html diff --git a/test/testdata/golden/logs/update/github.com/arduino-libraries/ArduinoCloudThing/index.html b/test/testdata/test_all/golden/logs/update/github.com/arduino-libraries/ArduinoCloudThing/index.html similarity index 100% rename from test/testdata/golden/logs/update/github.com/arduino-libraries/ArduinoCloudThing/index.html rename to test/testdata/test_all/golden/logs/update/github.com/arduino-libraries/ArduinoCloudThing/index.html diff --git a/test/testdata/golden/logs/update/github.com/arduino-libraries/SpacebrewYun/index.html b/test/testdata/test_all/golden/logs/update/github.com/arduino-libraries/SpacebrewYun/index.html similarity index 100% rename from test/testdata/golden/logs/update/github.com/arduino-libraries/SpacebrewYun/index.html rename to test/testdata/test_all/golden/logs/update/github.com/arduino-libraries/SpacebrewYun/index.html diff --git a/test/testdata/repos.txt b/test/testdata/test_all/repos.txt similarity index 100% rename from test/testdata/repos.txt rename to test/testdata/test_all/repos.txt diff --git a/test/testdata/test_clean_checkout/repos.txt b/test/testdata/test_clean_checkout/repos.txt new file mode 100644 index 00000000..722e1a06 --- /dev/null +++ b/test/testdata/test_clean_checkout/repos.txt @@ -0,0 +1 @@ +https://github.com/lexus2k/ssd1306.git|Contributed|ssd1306