Skip to content

Commit

Permalink
[CopyFilesV2] Considered delay between retries for cp action. Switche…
Browse files Browse the repository at this point in the history 
…d to node 6. (#15098) (#15102)

* Added delay for retries - cp action

* Improve retry logic

* Changed error messages and added fuction description

* Revert "[CopyFilesV2] Migrated to Node10 (#14710)"

This reverts commit e8abe8b.

* Tests

* Fixed tests

Co-authored-by: Nikita Ezzhev <[email protected]>

Co-authored-by: Nikita Ezzhev <[email protected]>
  • Loading branch information
@EzzhevNikita
Anatoly Bolshakov and EzzhevNikita authored Jul 29, 2021
1 parent d4b9f8c commit 38ba049
Show file tree
Hide file tree
Showing 18 changed files with 4,143 additions and 455 deletions.
51 changes: 34 additions & 17 deletions Tasks/CopyFilesV2/Tests/L0.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ describe('CopyFiles L0 Suite', function () {

after(() => { });

it('copy files from srcdir to destdir', (done: Mocha.Done) => {
it('copy files from srcdir to destdir', (done: MochaDone) => {
this.timeout(1000);

let testPath = path.join(__dirname, 'L0copyAllFiles.js');
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
Expand Down Expand Up @@ -49,7 +50,8 @@ describe('CopyFiles L0 Suite', function () {
done();
});

it('copy files from srcdir to destdir with brackets in src path', (done: Mocha.Done) => {
it('copy files from srcdir to destdir with brackets in src path', (done: MochaDone) => {
this.timeout(1000);

let testPath = path.join(__dirname, 'L0copyAllFilesWithBracketsInSrcPath.js');
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
Expand Down Expand Up @@ -88,7 +90,8 @@ describe('CopyFiles L0 Suite', function () {
done();
});

it('copy files and subtract based on exclude pattern', (done: Mocha.Done) => {
it('copy files and subtract based on exclude pattern', (done: MochaDone) => {
this.timeout(1000);

let testPath = path.join(__dirname, 'L0copySubtractExclude.js');
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
Expand Down Expand Up @@ -118,7 +121,8 @@ describe('CopyFiles L0 Suite', function () {
done();
});

it('fails if Contents not set', (done: Mocha.Done) => {
it('fails if Contents not set', (done: MochaDone) => {
this.timeout(1000);

let testPath = path.join(__dirname, 'L0failsIfContentsNotSet.js');
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
Expand All @@ -129,7 +133,8 @@ describe('CopyFiles L0 Suite', function () {
done();
});

it('fails if SourceFolder not set', (done: Mocha.Done) => {
it('fails if SourceFolder not set', (done: MochaDone) => {
this.timeout(1000);

let testPath = path.join(__dirname, 'L0failsIfSourceFolderNotSet.js');
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
Expand All @@ -140,7 +145,8 @@ describe('CopyFiles L0 Suite', function () {
done();
});

it('fails if TargetFolder not set', (done: Mocha.Done) => {
it('fails if TargetFolder not set', (done: MochaDone) => {
this.timeout(1000);

let testPath = path.join(__dirname, 'L0failsIfTargetFolderNotSet.js');
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
Expand All @@ -151,7 +157,8 @@ describe('CopyFiles L0 Suite', function () {
done();
});

it('fails if SourceFolder not found', (done: Mocha.Done) => {
it('fails if SourceFolder not found', (done: MochaDone) => {
this.timeout(1000);

let testPath = path.join(__dirname, 'L0failsIfSourceFolderNotFound.js');
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
Expand All @@ -162,7 +169,8 @@ describe('CopyFiles L0 Suite', function () {
done();
});

it('fails if target file is a directory', (done: Mocha.Done) => {
it('fails if target file is a directory', (done: MochaDone) => {
this.timeout(1000);

let testPath = path.join(__dirname, 'L0failsIfTargetFileIsDir.js');
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
Expand All @@ -173,7 +181,8 @@ describe('CopyFiles L0 Suite', function () {
done();
});

it('skips if exists', (done: Mocha.Done) => {
it('skips if exists', (done: MochaDone) => {
this.timeout(1000);

let testPath = path.join(__dirname, 'L0skipsIfExists.js');
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
Expand All @@ -197,7 +206,8 @@ describe('CopyFiles L0 Suite', function () {
done();
});

it('overwrites if specified', (done: Mocha.Done) => {
it('overwrites if specified', (done: MochaDone) => {
this.timeout(1000);

let testPath = path.join(__dirname, 'L0overwritesIfSpecified.js');
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
Expand All @@ -221,7 +231,8 @@ describe('CopyFiles L0 Suite', function () {
done();
});

it('preserves timestamp if specified', (done: Mocha.Done) => {
it('preserves timestamp if specified', (done: MochaDone) => {
this.timeout(1000);

let testPath = path.join(__dirname, 'L0preservesTimestampIfSpecified.js');
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
Expand All @@ -248,7 +259,8 @@ describe('CopyFiles L0 Suite', function () {
done();
});

it('cleans if specified', (done: Mocha.Done) => {
it('cleans if specified', (done: MochaDone) => {
this.timeout(1000);

let testPath = path.join(__dirname, 'L0cleansIfSpecified.js');
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
Expand Down Expand Up @@ -278,7 +290,8 @@ describe('CopyFiles L0 Suite', function () {
done();
});

it('cleans if specified and target is file', (done: Mocha.Done) => {
it('cleans if specified and target is file', (done: MochaDone) => {
this.timeout(1000);

let testPath = path.join(__dirname, 'L0cleansIfSpecifiedAndTargetIsFile.js');
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
Expand All @@ -305,7 +318,9 @@ describe('CopyFiles L0 Suite', function () {
done();
});

it('roots patterns', (done: Mocha.Done) => {
it('roots patterns', (done: MochaDone) => {
this.timeout(1000);

let testPath = path.join(__dirname, 'L0rootsPatterns.js');
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
runner.run();
Expand All @@ -325,7 +340,7 @@ describe('CopyFiles L0 Suite', function () {
done();
});

it('ignores errors during target folder creation if ignoreMakeDirErrors is true', (done: Mocha.Done) => {
it('ignores errors during target folder creation if ignoreMakeDirErrors is true', (done: MochaDone) => {
let testPath = path.join(__dirname, 'L0IgnoresMakeDirError.js');
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
runner.run();
Expand All @@ -343,7 +358,7 @@ describe('CopyFiles L0 Suite', function () {
done();
});

it('fails if there are errors during target folder creation if ignoreMakeDirErrors is false', (done: Mocha.Done) => {
it('fails if there are errors during target folder creation if ignoreMakeDirErrors is false', (done: MochaDone) => {
let testPath = path.join(__dirname, 'L0FailsIfThereIsMkdirError.js');
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
runner.run();
Expand All @@ -355,7 +370,9 @@ describe('CopyFiles L0 Suite', function () {
});

if (process.platform == 'win32') {
it('overwrites readonly', (done: Mocha.Done) => {
it('overwrites readonly', (done: MochaDone) => {
this.timeout(1000);

let testPath = path.join(__dirname, 'L0overwritesReadonly.js');
let runner: mocktest.MockTestRunner = new mocktest.MockTestRunner(testPath);
runner.run();
Expand Down
9 changes: 6 additions & 3 deletions Tasks/CopyFilesV2/Tests/L0cleansIfSpecified.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,17 @@ runner.registerMockExport('stats', (itemPath: string) => {
}
});
let origReaddirSync = fs.readdirSync;
fs.readdirSync = (p) => {
fs.readdirSync = (p: string | Buffer) => {
console.log('HERE path ' + p);
let result: string[];
if (p == path.normalize('/destDir')) {
return [ 'clean-subDir', 'clean-file.txt'] as any;
result = [ 'clean-subDir', 'clean-file.txt' ];
}
else {
return origReaddirSync(p);
result = origReaddirSync(p);
}

return result;
}

// as a precaution, disable fs.chmodSync. it should not be called during this scenario.
Expand Down
5 changes: 2 additions & 3 deletions Tasks/CopyFilesV2/Tests/L0preservesTimestampIfSpecified.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import fs = require('fs');
import mockanswer = require('azure-pipelines-task-lib/mock-answer');
import mockrun = require('azure-pipelines-task-lib/mock-run');
import path = require('path');
const { promisify } = require('util')

let taskPath = path.join(__dirname, '..', 'copyfiles.js');
let runner: mockrun.TaskMockRunner = new mockrun.TaskMockRunner(taskPath);
Expand Down Expand Up @@ -47,9 +46,9 @@ runner.registerMockExport('stats', (itemPath: string) => {
}
});

fs.utimes = promisify(function (targetPath, atime, mtime, err) {
fs.utimes = function (targetPath, atime, mtime, err) {
console.log('Calling fs.utimes on', targetPath);
});
}
runner.registerMock('fs', fs);

runner.run();
98 changes: 60 additions & 38 deletions Tasks/CopyFilesV2/copyfiles.ts
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import fs = require('fs');
import path = require('path');
import tl = require('azure-pipelines-task-lib/task');
import { RetryOptions, RetryHelper } from './retryHelper';
import { RetryOptions, RetryHelper } from './retrylogichelper';

/**
* Shows timestamp change operation results
Expand Down Expand Up @@ -37,6 +37,23 @@ function makeDirP(targetFolder: string, ignoreErrors: boolean): void {
}
}

/**
* Gets stats for the provided path. Will ignore ENOENT error if ignoreEnoent is true.
* If ignoreEnoent is false ENOENT will be thrown from the function.
* @param path path for which methid will try to get tl.FsStats.
* @param ignoreEnoent ignore ENOENT error during check of path stats.
* @returns
*/
function stats(path: string, ignoreEnoent: boolean): tl.FsStats {
try {
return tl.stats(path);
} catch (err) {
if (err.code != 'ENOENT' && ignoreEnoent) {
throw err;
}
}
}

async function main(): Promise<void> {
// we allow broken symlinks - since there could be broken symlinks found in source folder, but filtered by contents pattern
const findOptions: tl.FindOptions = {
Expand Down Expand Up @@ -80,7 +97,7 @@ async function main(): Promise<void> {
let allPaths: string[] = tl.find(sourceFolder, findOptions);
let sourceFolderPattern = sourceFolder.replace('[', '[[]'); // directories can have [] in them, and they have special meanings as a pattern, so escape them
let matchedPaths: string[] = tl.match(allPaths, contents, sourceFolderPattern); // default match options
let matchedFiles: string[] = matchedPaths.filter((itemPath: string) => !tl.stats(itemPath).isDirectory()); // filter-out directories
let matchedFiles: string[] = matchedPaths.filter((itemPath: string) => !stats(itemPath, false).isDirectory()); // filter-out directories

// copy the files to the target folder
console.log(tl.loc('FoundNFiles', matchedFiles.length));
Expand All @@ -92,43 +109,40 @@ async function main(): Promise<void> {

// stat the targetFolder path
let targetFolderStats: tl.FsStats;
try {
targetFolderStats = await retryHelper.RunWithRetrySingleArg<tl.FsStats, string>(
() => tl.stats(targetFolder),
targetFolder);
} catch (err) {
if (err.code != 'ENOENT') {
throw err;
}
}
targetFolderStats = await retryHelper.RunWithRetry<tl.FsStats>(
() => stats(targetFolder, true),
`stats for ${targetFolder}`
);

if (targetFolderStats) {
if (targetFolderStats.isDirectory()) {
// delete the child items
const folderItems: string[] = await retryHelper.RunWithRetrySingleArg<string[], string>(
const folderItems: string[] = await retryHelper.RunWithRetry<string[]>(
() => fs.readdirSync(targetFolder),
targetFolder);
`readdirSync for ${targetFolder}`
);

for (let item of folderItems) {
let itemPath = path.join(targetFolder, item);
await retryHelper.RunWithRetrySingleArg<void, string>(() =>
await retryHelper.RunWithRetry(() =>
tl.rmRF(itemPath),
targetFolder
`delete of ${itemPath}`
);
}
} else {
await retryHelper.RunWithRetrySingleArg<void, string>(() =>
await retryHelper.RunWithRetry(() =>
tl.rmRF(targetFolder),
targetFolder);
`delete of ${targetFolder}`
);
}
}
}

// make sure the target folder exists
await retryHelper.RunWithRetryMultiArgs<void, string, boolean>(() =>
await retryHelper.RunWithRetry(() =>
makeDirP(targetFolder, ignoreMakeDirErrors),
targetFolder,
ignoreMakeDirErrors);
`makeDirP for ${targetFolder}`
);
try {
let createdFolders: { [folder: string]: boolean } = {};
for (let file of matchedFiles) {
Expand All @@ -150,23 +164,20 @@ async function main(): Promise<void> {

if (!createdFolders[targetDir]) {
await retryHelper.RunWithRetry(
() => makeDirP(targetDir, ignoreMakeDirErrors));
() => makeDirP(targetDir, ignoreMakeDirErrors),
`makeDirP for ${targetDir}`
);

createdFolders[targetDir] = true;
}

// stat the target
let targetStats: tl.FsStats;
if (!cleanTargetFolder) { // optimization - no need to check if relative target exists when CleanTargetFolder=true
try {
targetStats = await retryHelper.RunWithRetrySingleArg<tl.FsStats, string>(
() => tl.stats(targetPath),
targetPath);
} catch (err) {
if (err.code != 'ENOENT') {
throw err;
}
}
targetStats = await retryHelper.RunWithRetry<tl.FsStats>(
() => stats(targetPath, true),
`Stats for ${targetPath}`
);
}

// validate the target is not a directory
Expand All @@ -179,13 +190,17 @@ async function main(): Promise<void> {
console.log(tl.loc('FileAlreadyExistAt', file, targetPath));
} else { // copy
console.log(tl.loc('CopyingTo', file, targetPath));
tl.cp(file, targetPath, undefined, undefined, retryCount);
await retryHelper.RunWithRetry(
() => tl.cp(file, targetPath),
`copy ${file} to ${targetPath}`
);
if (preserveTimestamp) {
try {
let fileStats;
fileStats = await retryHelper.RunWithRetrySingleArg<tl.FsStats, string>(
(file) => tl.stats(file),
file);
fileStats = await retryHelper.RunWithRetry<tl.FsStats>(
() => stats(file, false),
`stats for ${file}`
);
fs.utimes(targetPath, fileStats.atime, fileStats.mtime, (err) => {
displayTimestampChangeResults(fileStats, err);
});
Expand Down Expand Up @@ -214,15 +229,22 @@ async function main(): Promise<void> {
// https://github.com/nodejs/node/blob/v5.x/deps/uv/src/win/fs.c#L1064
tl.debug(`removing readonly attribute on '${targetPath}'`);

await retryHelper.RunWithRetrySingleArg<void, string>(
await retryHelper.RunWithRetry(
() => fs.chmodSync(targetPath, targetStats.mode | 146),
targetPath);
`chmodSync for ${targetPath}`
);
}
await retryHelper.RunWithRetry(
() => tl.cp(file, targetPath, "-f"),
`copy ${file} to ${targetPath}`
);

tl.cp(file, targetPath, "-f", undefined, retryCount);
if (preserveTimestamp) {
try {
const fileStats = tl.stats(file);
const fileStats = await retryHelper.RunWithRetry<tl.FsStats>(
() => stats(file, false),
`stats for ${file}`
);
fs.utimes(targetPath, fileStats.atime, fileStats.mtime, (err) => {
displayTimestampChangeResults(fileStats, err);
});
Expand Down
Loading

0 comments on commit 38ba049

Please sign in to comment.