From d0501619342bbf3d6cb672563cd8a90462733abf Mon Sep 17 00:00:00 2001 From: per1234 Date: Thu, 17 Mar 2022 00:56:56 -0700 Subject: [PATCH] Don't preserve old repository on URL change The library maintainers may request a change to the repository URL in the library registration data. This operation is performed by the backend maintainer via the command: libraries-repository-engine modify --repo-url Previously, this command did three things: - Update the DB entry for the library - Move the release archives to the new location (the storage structure is based on the repository URL for some reason) - Move the cached library repository clone to the new location Problem ------- The last of these is problematic because the remote of that repository is still configured for the original URL, meaning the fetch done during the `sync` command execution were still done against the old URL. This bug is not noticeable under either of the following scenarios: The repository was renamed or transferred ========================================= This produces a redirect from the old URL to the new one, so the fetch is done from the intended repo despite the outdated remote configuration. The original repository was deleted =================================== If a fetch fails, the engine deletes the repository and clones from the URL in the DB. The newly cloned repo will have the correct remote configuration. The bug is noticeable under the following scenario: The original repository still exists ==================================== The sync process continues to fetch from the old URL. Solution -------- Change the `modify --repo-url` command behavior to delete the cached library repository clone. The repository will be cloned from the updated URL on the next run of the `sync` command. --- internal/command/modify/modify.go | 28 +++++++--------------------- tests/test_modify.py | 5 ----- 2 files changed, 7 insertions(+), 26 deletions(-) diff --git a/internal/command/modify/modify.go b/internal/command/modify/modify.go index a21315a7..ab90e0f2 100644 --- a/internal/command/modify/modify.go +++ b/internal/command/modify/modify.go @@ -163,32 +163,18 @@ func modifyRepositoryURL(newRepositoryURL string) error { fmt.Printf("Changing URL of library %s from %s to %s\n", libraryName, oldRepositoryURL, newRepositoryURL) - // Move the library Git clone to the new path. - gitClonePath := func(url string) (*paths.Path, error) { - libraryRegistration := libraries.Repo{URL: url} - gitCloneSubfolder, err := libraryRegistration.AsFolder() - if err != nil { - return nil, err - } - - return paths.New(config.GitClonesFolder, gitCloneSubfolder), nil - } - oldGitClonePath, err := gitClonePath(oldRepositoryURL) + // Remove the library Git clone folder. It will be cloned from the new URL on the next sync. + libraryRegistration := libraries.Repo{URL: libraryData.Repository} + gitCloneSubfolder, err := libraryRegistration.AsFolder() if err != nil { return err } - newGitClonePath, err := gitClonePath(newRepositoryURL) - if err != nil { - return err - } - if err := newGitClonePath.Parent().MkdirAll(); err != nil { - return fmt.Errorf("While creating new library Git clone path: %w", err) - } - if err := backup.Backup(oldGitClonePath); err != nil { + gitClonePath := paths.New(config.GitClonesFolder, gitCloneSubfolder) + if err := backup.Backup(gitClonePath); err != nil { return fmt.Errorf("While backing up library's Git clone: %w", err) } - if err := oldGitClonePath.Rename(newGitClonePath); err != nil { - return fmt.Errorf("While moving library's Git clone: %w", err) + if err := gitClonePath.RemoveAll(); err != nil { + return fmt.Errorf("While removing library's Git clone: %w", err) } // Update the library repository URL in the database. diff --git a/tests/test_modify.py b/tests/test_modify.py index f6ef6b64..e8c247d9 100644 --- a/tests/test_modify.py +++ b/tests/test_modify.py @@ -227,9 +227,6 @@ def test_repo_url( new_library_release_archives_folder = pathlib.Path(configuration.data["LibrariesFolder"]).joinpath( new_host, new_owner ) - new_git_clone_path = pathlib.Path(configuration.data["GitClonesFolder"]).joinpath( - new_host, new_owner, new_repo_name - ) # The "canary" library is not modified and so all its content should remain unchanged after running the command canary_name = "ArduinoIoTCloudBearSSL" sanitized_canary_name = "ArduinoIoTCloudBearSSL" @@ -281,7 +278,6 @@ def get_release_archive_url(name, version): raise assert old_git_clone_path.exists() - assert not new_git_clone_path.exists() assert canary_git_clone_path.exists() assert get_library_repo_url(name=name) != new_repo_url assert get_library_repo_url(name=canary_name) == canary_repo_url @@ -314,7 +310,6 @@ def get_release_archive_url(name, version): # Verify the effect of the command was as expected assert not old_git_clone_path.exists() - assert new_git_clone_path.exists() assert canary_release_archive_path.exists() assert canary_git_clone_path.exists() assert get_library_repo_url(name=name) == new_repo_url