diff --git a/code/+openminds/+internal/+utility/+git/downloadRepository.m b/code/+openminds/+internal/+utility/+git/downloadRepository.m index eb8cb71ec..f41ccb634 100644 --- a/code/+openminds/+internal/+utility/+git/downloadRepository.m +++ b/code/+openminds/+internal/+utility/+git/downloadRepository.m @@ -7,6 +7,11 @@ function downloadRepository(repositoryName, options) % Optional parameters: % BranchName - Name of branch % Owner - Name of repository owner +% TargetDirectory - Folder the repository folder is placed in +% +% The commit is recorded only once the repository is in place, so a +% download that failed is retried on the next call rather than taken +% for the current commit. arguments repositoryName = "openMINDS" @@ -17,8 +22,9 @@ function downloadRepository(repositoryName, options) % Todo: Should be a preference. targetDirectory = options.TargetDirectory; - + import openminds.internal.extern.fex.filedownload.downloadFile + import openminds.internal.utility.git.extractRepositoryArchive import openminds.internal.utility.git.getCurrentCommitID import openminds.internal.utility.git.writeRecordedCommitID import openminds.internal.utility.git.isRecordedCommitCurrent @@ -37,40 +43,16 @@ function downloadRepository(repositoryName, options) % - Create path for saving and download types zipFileName = webURI.Path(end); tempZipFilepath = tempname + "-" + zipFileName; - C1 = onCleanup(@(pathStr) delete(tempZipFilepath) ); - + zipCleanup = onCleanup(@() delete(tempZipFilepath)); + fprintf('Downloading repository "%s" from "%s"... ', ... repositoryName, options.Owner) downloadFile(tempZipFilepath, webURI.EncodedURI, 'ShowFilename', true); fprintf('Done.\n') - directoryForUnzip = tempname; - if ~isfolder(directoryForUnzip) - mkdir(directoryForUnzip) - end - - C2 = onCleanup(@(pathStr, mode) rmdir(directoryForUnzip, "s") ); - - fprintf('Unzipping repository file (%s)... ', repositoryName) - unzip(tempZipFilepath, directoryForUnzip) - fprintf('Done.\n') - - sourceDirectory = directoryForUnzip; - - if ~isfolder(targetDirectory) - mkdir(targetDirectory) - end - - % Get repository folder name - L = dir(sourceDirectory); L(startsWith({L.name}, '.')) = []; - assert(isscalar(L), "Expected temporary folder to contain one downloaded item") - folderName = strtrim( L.name ); - if isfolder( fullfile(targetDirectory, folderName) ) - rmdir(fullfile(targetDirectory, folderName), "s") - end - - fprintf('Copying repository "%s" to local directory:\n%s... ', repositoryName, targetDirectory) - copyfile(sourceDirectory, targetDirectory) + fprintf('Extracting repository "%s" into local directory:\n%s... ', ... + repositoryName, targetDirectory) + extractRepositoryArchive(tempZipFilepath, targetDirectory); fprintf('Done.\n') % Save current commit ID and repository details diff --git a/code/+openminds/+internal/+utility/+git/extractRepositoryArchive.m b/code/+openminds/+internal/+utility/+git/extractRepositoryArchive.m new file mode 100644 index 000000000..a0fd64afd --- /dev/null +++ b/code/+openminds/+internal/+utility/+git/extractRepositoryArchive.m @@ -0,0 +1,73 @@ +function repositoryFolder = extractRepositoryArchive(zipFilePath, targetDirectory) +% extractRepositoryArchive - Put the repository in an archive in place of the previous copy +% +% Syntax: +% repositoryFolder = extractRepositoryArchive(zipFilePath, targetDirectory) +% +% Input Arguments: +% zipFilePath - A GitHub archive: a zip file holding one folder, +% named after the repository and branch +% targetDirectory - Folder the repository folder is placed in +% +% Output Arguments: +% repositoryFolder - Folder the repository was placed in +% +% The archive is extracted into a staging folder beside the repository +% folder and moved into place last, so an extraction that fails part +% way, from a full disk or an interrupted job, leaves the previous +% copy where it was instead of a folder that exists but is incomplete. +% Nothing of the staging folder remains afterwards, whether or not the +% extraction succeeded. +% +% See also downloadRepository + + arguments + zipFilePath (1,1) string {mustBeFile} + targetDirectory (1,1) string + end + + if ~isfolder(targetDirectory) + mkdir(targetDirectory) + end + + % Beside the repository folder, not under tempdir, so that moving the + % result into place is a rename on one volume rather than a second + % copy of every file. + [~, stagingName] = fileparts(tempname); + stagingDirectory = fullfile(targetDirectory, stagingName); + stagingCleanup = onCleanup(@() removeFolder(stagingDirectory)); + + unzip(zipFilePath, stagingDirectory) + + L = dir(stagingDirectory); + L(startsWith({L.name}, '.')) = []; + if ~isscalar(L) || ~L.isdir + error('OPENMINDS:Git:UnexpectedArchiveLayout', ... + ['The archive "%s" was expected to hold one folder, as a ', ... + 'GitHub archive does, but holds %d items.'], zipFilePath, numel(L)) + end + folderName = string(L.name); + stagedFolder = fullfile(stagingDirectory, folderName); + repositoryFolder = fullfile(targetDirectory, folderName); + + % The previous copy goes only now that the new one is complete, and it + % must be gone before the move: movefile moves a folder into one that + % exists rather than over it. + if isfolder(repositoryFolder) + rmdir(repositoryFolder, "s") + end + + [isMoved, message] = movefile(stagedFolder, repositoryFolder); + if ~isMoved + error('OPENMINDS:Git:MoveFailed', ... + 'The repository folder "%s" could not be moved into "%s": %s', ... + folderName, targetDirectory, message) + end +end + +function removeFolder(folderPath) +% removeFolder - Remove a folder and its contents if it exists + if isfolder(folderPath) + rmdir(folderPath, "s") + end +end diff --git a/tools/tests/+ommtest/+internal/GitDownloadTest.m b/tools/tests/+ommtest/+internal/GitDownloadTest.m index 14f499438..6f5f27f58 100644 --- a/tools/tests/+ommtest/+internal/GitDownloadTest.m +++ b/tools/tests/+ommtest/+internal/GitDownloadTest.m @@ -18,5 +18,89 @@ function testDownloadRepo(testCase) testCase.verifyTrue(openminds.internal.utility.git.isRecordedCommitCurrent()) end + + function testArchiveReplacesThePreviousCopy(testCase) + % The previous copy is replaced, not merged into, and nothing of + % the staging folder remains. + import openminds.internal.utility.git.extractRepositoryArchive + + targetDirectory = fullfile(pwd, "Repositories"); + writeRepositoryFolder(fullfile(targetDirectory, "repo-main"), "old.txt") + zipFilePath = writeRepositoryArchive(pwd, "repo-main", "new.txt"); + + repositoryFolder = extractRepositoryArchive(zipFilePath, targetDirectory); + + testCase.verifyEqual(repositoryFolder, fullfile(targetDirectory, "repo-main")) + testCase.verifyTrue(isfile(fullfile(repositoryFolder, "new.txt"))) + testCase.verifyFalse(isfile(fullfile(repositoryFolder, "old.txt")), ... + 'The previous copy must be replaced, not merged into.') + testCase.verifyEqual(folderNames(targetDirectory), "repo-main", ... + 'Nothing of the staging folder may remain.') + end + + function testFailedExtractionLeavesThePreviousCopy(testCase) + % An archive that cannot be read is reported, the previous copy is + % untouched, and nothing of the staging folder remains. + import openminds.internal.utility.git.extractRepositoryArchive + + targetDirectory = fullfile(pwd, "Repositories"); + writeRepositoryFolder(fullfile(targetDirectory, "repo-main"), "old.txt") + zipFilePath = fullfile(pwd, "damaged.zip"); + writeTextFile(zipFilePath, "not an archive") + + testCase.verifyError( ... + @() extractRepositoryArchive(zipFilePath, targetDirectory), ?MException) + + testCase.verifyTrue(isfile(fullfile(targetDirectory, "repo-main", "old.txt")), ... + 'The previous copy must survive a failed extraction.') + testCase.verifyEqual(folderNames(targetDirectory), "repo-main", ... + 'Nothing of the staging folder may remain.') + end + + function testAnArchiveNotHoldingOneFolderIsRejected(testCase) + % A GitHub archive holds one folder. Anything else is reported by + % identifier before the previous copy is touched. + import openminds.internal.utility.git.extractRepositoryArchive + + targetDirectory = fullfile(pwd, "Repositories"); + writeRepositoryFolder(fullfile(targetDirectory, "repo-main"), "old.txt") + sourceFolder = fullfile(pwd, "archive-source"); + writeRepositoryFolder(fullfile(sourceFolder, "repo-main"), "new.txt") + writeTextFile(fullfile(sourceFolder, "stray.txt"), "x") + zipFilePath = fullfile(pwd, "two-items.zip"); + zip(zipFilePath, ["repo-main", "stray.txt"], sourceFolder) + + testCase.verifyError( ... + @() extractRepositoryArchive(zipFilePath, targetDirectory), ... + 'OPENMINDS:Git:UnexpectedArchiveLayout') + + testCase.verifyTrue(isfile(fullfile(targetDirectory, "repo-main", "old.txt"))) + testCase.verifyEqual(folderNames(targetDirectory), "repo-main") + end end end + +function writeRepositoryFolder(folderPath, fileName) + mkdir(folderPath) + writeTextFile(fullfile(folderPath, fileName), "x") +end + +function writeTextFile(filePath, text) + fileId = fopen(filePath, "w"); + fwrite(fileId, text); + fclose(fileId); +end + +function zipFilePath = writeRepositoryArchive(rootFolder, folderName, fileName) +% An archive laid out like a GitHub archive: one folder at its top. + sourceFolder = fullfile(rootFolder, "archive-source"); + writeRepositoryFolder(fullfile(sourceFolder, folderName), fileName) + zipFilePath = fullfile(rootFolder, folderName + ".zip"); + zip(zipFilePath, folderName, sourceFolder) +end + +function names = folderNames(folderPath) + L = dir(folderPath); + L(startsWith({L.name}, ".")) = []; + names = string({L.name}); +end