Skip to content

Commit d87379e

Browse files
rishabhmalikMSrishabhmalikMS
authored
CP & RM command Fix | copy symbolic links as target files and not as folders | remove symbolic link (#1096)
* Fix to copy symbolic links as target files and not as folders * updates in rm command to delete symbolic lin which is poiting to a folder and also covering case for FIFO type file * removing comments * removing rm test for linux and macos test check * minor fix * adding check for source file/folder existence before operating on its symbolic link * minor check * fixes * multiple level sybbolic link removal fix * more fixes * minor fix * code cleaning * code cleaning * update to calling unlink operation for missing source target in symbolic link * adding code to recussively copy directory and handle symbolic links in it * Added additional logs | Added unit tests for remove command to remove symlinks pointing to non-existing items * removing repeated unit test
1 parent be0310c commit d87379e

File tree

4 files changed

+193
-97
lines changed

4 files changed

+193
-97
lines changed

‎node/task.ts

Lines changed: 94 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ type OptionCases<T extends string> = `-${Uppercase<T> | Lowercase<T>}`;
1313

1414
type OptionsPermutations<T extends string, U extends string = ''> =
1515
T extends `${infer First}${infer Rest}`
16-
? OptionCases<`${U}${First}`> | OptionCases<`${First}${U}`> | OptionsPermutations<Rest, `${U}${First}`> | OptionCases<First>
17-
: OptionCases<U> | '';
16+
? OptionCases<`${U}${First}`> | OptionCases<`${First}${U}`> | OptionsPermutations<Rest, `${U}${First}`> | OptionCases<First>
17+
: OptionCases<U> | '';
1818

1919
export enum TaskResult {
2020
Succeeded = 0,
@@ -893,7 +893,7 @@ export function popd(index: string = ''): string[] {
893893
const _path = path.resolve(dirStack.shift()!);
894894
cd(_path);
895895
}
896-
896+
897897
return getActualStack();
898898
}
899899

@@ -1065,7 +1065,7 @@ export function ls(optionsOrPaths?: unknown, ...paths: unknown[]): string[] {
10651065
paths.push(...pathsFromOptions);
10661066
}
10671067
}
1068-
1068+
10691069
if (paths.length === 0) {
10701070
paths.push(path.resolve('.'));
10711071
}
@@ -1101,7 +1101,7 @@ export function ls(optionsOrPaths?: unknown, ...paths: unknown[]): string[] {
11011101
continue;
11021102
}
11031103
const baseDir = pathsCopy.find(p => entry.startsWith(path.resolve(p as string))) as string || path.resolve('.');
1104-
1104+
11051105
if (fs.lstatSync(entry).isDirectory() && isRecursive) {
11061106
preparedPaths.push(...fs.readdirSync(entry).map(x => path.join(entry, x)));
11071107
entries.push(path.relative(baseDir, entry));
@@ -1179,13 +1179,17 @@ export function cp(sourceOrOptions: unknown, destinationOrSource: string, option
11791179
throw new Error(loc('LIB_PathNotFound', 'cp', destination));
11801180
}
11811181

1182-
const lstatSource = fs.lstatSync(source);
1182+
var lstatSource = fs.lstatSync(source);
11831183

11841184
if (!force && fs.existsSync(destination)) {
11851185
return;
11861186
}
11871187

11881188
try {
1189+
if (lstatSource.isSymbolicLink()) {
1190+
source = fs.readlinkSync(source);
1191+
lstatSource = fs.lstatSync(source);
1192+
}
11891193
if (lstatSource.isFile()) {
11901194
if (fs.existsSync(destination) && fs.lstatSync(destination).isDirectory()) {
11911195
destination = path.join(destination, path.basename(source));
@@ -1197,14 +1201,49 @@ export function cp(sourceOrOptions: unknown, destinationOrSource: string, option
11971201
fs.copyFileSync(source, destination, fs.constants.COPYFILE_EXCL);
11981202
}
11991203
} else {
1200-
fs.cpSync(source, path.join(destination, path.basename(source)), { recursive, force });
1204+
copyDirectoryWithResolvedSymlinks(source, path.join(destination, path.basename(source)), force);
12011205
}
12021206
} catch (error) {
12031207
throw new Error(loc('LIB_OperationFailed', 'cp', error));
12041208
}
1205-
}, [], { retryCount, continueOnError});
1209+
}, [], { retryCount, continueOnError });
12061210
}
12071211

1212+
const copyDirectoryWithResolvedSymlinks = (src: string, dest: string, force: boolean) => {
1213+
var srcPath: string;
1214+
var destPath: string;
1215+
var entry: fs.Dirent;
1216+
const entries = fs.readdirSync(src, { withFileTypes: true });
1217+
1218+
if (!fs.existsSync(dest)) {
1219+
fs.mkdirSync(dest, { recursive: true });
1220+
}
1221+
1222+
for (entry of entries) {
1223+
srcPath = path.join(src, entry.name);
1224+
destPath = path.join(dest, entry.name);
1225+
1226+
if (entry.isSymbolicLink()) {
1227+
// Resolve the symbolic link and copy the target
1228+
const resolvedPath = fs.readlinkSync(srcPath);
1229+
const stat = fs.lstatSync(resolvedPath);
1230+
1231+
if (stat.isFile()) {
1232+
// Use the actual target file's name instead of the symbolic link's name
1233+
const targetFileName = path.basename(resolvedPath);
1234+
const targetDestPath = path.join(dest, targetFileName);
1235+
fs.copyFileSync(resolvedPath, targetDestPath);
1236+
} else if (stat.isDirectory()) {
1237+
copyDirectoryWithResolvedSymlinks(resolvedPath, destPath, force);
1238+
}
1239+
} else if (entry.isFile()) {
1240+
fs.copyFileSync(srcPath, destPath);
1241+
} else if (entry.isDirectory()) {
1242+
copyDirectoryWithResolvedSymlinks(srcPath, destPath, force);
1243+
}
1244+
}
1245+
};
1246+
12081247
type MoveOptionsVariants = OptionsPermutations<'fn'>;
12091248

12101249
/**
@@ -1705,95 +1744,81 @@ function _legacyFindFiles_getMatchingItems(
17051744
*/
17061745
export function rmRF(inputPath: string): void {
17071746
debug('rm -rf ' + inputPath);
1708-
17091747
if (getPlatform() == Platform.Windows) {
17101748
// Node doesn't provide a delete operation, only an unlink function. This means that if the file is being used by another
17111749
// program (e.g. antivirus), it won't be deleted. To address this, we shell out the work to rd/del.
17121750
try {
1713-
if (fs.statSync(inputPath).isDirectory()) {
1751+
const lstats = fs.lstatSync(inputPath);
1752+
if (lstats.isDirectory() && !lstats.isSymbolicLink()) {
17141753
debug('removing directory ' + inputPath);
17151754
childProcess.execFileSync("cmd.exe", ["/c", "rd", "/s", "/q", inputPath]);
1716-
}
1717-
else {
1755+
1756+
} else if (lstats.isSymbolicLink()) {
1757+
debug('removing symbolic link ' + inputPath);
1758+
const realPath = fs.readlinkSync(inputPath);
1759+
if (fs.existsSync(realPath)) {
1760+
const stats = fs.statSync(realPath);
1761+
if (stats.isDirectory()) {
1762+
childProcess.execFileSync("cmd.exe", ["/c", "rd", "/s", "/q", realPath]);
1763+
fs.unlinkSync(inputPath);
1764+
} else {
1765+
fs.unlinkSync(inputPath);
1766+
}
1767+
} else {
1768+
debug(`Symbolic link '${inputPath}' points to a non-existing target '${realPath}'. Removing the symbolic link.`);
1769+
fs.unlinkSync(inputPath);
1770+
}
1771+
} else {
17181772
debug('removing file ' + inputPath);
17191773
childProcess.execFileSync("cmd.exe", ["/c", "del", "/f", "/a", inputPath]);
17201774
}
17211775
} catch (err) {
1722-
// if you try to delete a file that doesn't exist, desired result is achieved
1723-
// other errors are valid
1724-
if (err.code != 'ENOENT') {
1725-
throw new Error(loc('LIB_OperationFailed', 'rmRF', err.message));
1726-
}
1727-
}
1728-
1729-
// Shelling out fails to remove a symlink folder with missing source, this unlink catches that
1730-
try {
1731-
fs.unlinkSync(inputPath);
1732-
} catch (err) {
1733-
// if you try to delete a file that doesn't exist, desired result is achieved
1734-
// other errors are valid
1776+
debug('Error: ' + err.message);
17351777
if (err.code != 'ENOENT') {
17361778
throw new Error(loc('LIB_OperationFailed', 'rmRF', err.message));
17371779
}
17381780
}
17391781
} else {
1740-
// get the lstats in order to workaround a bug in shelljs@0.3.0 where symlinks
1741-
// with missing targets are not handled correctly by "rm('-rf', path)"
17421782
let lstats: fs.Stats;
17431783

17441784
try {
17451785
if (inputPath.includes('*')) {
17461786
const entries = findMatch(path.dirname(inputPath), [path.basename(inputPath)]);
17471787

17481788
for (const entry of entries) {
1749-
fs.rmSync(entry, { recursive: true, force: true });
1789+
rmRF(entry);
17501790
}
1751-
1752-
return;
17531791
} else {
17541792
lstats = fs.lstatSync(inputPath);
1793+
if (lstats.isDirectory() && !lstats.isSymbolicLink()) {
1794+
debug('removing directory ' + inputPath);
1795+
fs.rmSync(inputPath, { recursive: true, force: true });
1796+
} else if (lstats.isSymbolicLink()) {
1797+
debug('removing symbolic link ' + inputPath);
1798+
const realPath = fs.readlinkSync(inputPath);
1799+
if (fs.existsSync(realPath)) {
1800+
const stats = fs.statSync(realPath);
1801+
if (stats.isDirectory()) {
1802+
fs.rmSync(realPath, { recursive: true, force: true });
1803+
fs.unlinkSync(inputPath);
1804+
} else {
1805+
fs.unlinkSync(inputPath);
1806+
}
1807+
} else {
1808+
debug(`Symbolic link '${inputPath}' points to a non-existing target '${realPath}'. Removing the symbolic link.`);
1809+
fs.unlinkSync(inputPath);
1810+
}
1811+
} else {
1812+
debug('removing file ' + inputPath);
1813+
fs.unlinkSync(inputPath);
1814+
}
17551815
}
17561816
} catch (err) {
1757-
// if you try to delete a file that doesn't exist, desired result is achieved
1758-
// other errors are valid
1759-
if (err.code == 'ENOENT') {
1760-
return;
1761-
}
1762-
1763-
throw new Error(loc('LIB_OperationFailed', 'rmRF', err.message));
1764-
}
1765-
1766-
if (lstats.isDirectory()) {
1767-
debug('removing directory');
1768-
try {
1769-
fs.rmSync(inputPath, { recursive: true, force: true });
1770-
} catch (errMsg) {
1771-
throw new Error(loc('LIB_OperationFailed', 'rmRF', errMsg));
1772-
}
1773-
1774-
return;
1775-
} else if (lstats.isSymbolicLink()) {
1776-
debug('removing symbolic link');
1777-
try {
1778-
fs.unlinkSync(inputPath);
1779-
} catch (errMsg) {
1780-
throw new Error(loc('LIB_OperationFailed', 'rmRF', errMsg));
1781-
}
1782-
1783-
return;
1784-
}
1785-
1786-
debug('removing file');
1787-
try {
1788-
const entries = findMatch(path.dirname(inputPath), [path.basename(inputPath)]);
1789-
1790-
for (const entry of entries) {
1791-
fs.rmSync(entry, { recursive: true, force: true });
1817+
debug('Error: ' + err.message);
1818+
if (err.code != 'ENOENT') {
1819+
throw new Error(loc('LIB_OperationFailed', 'rmRF', err.message));
17921820
}
17931821
}
1794-
catch (err) {
1795-
throw new Error(loc('LIB_OperationFailed', 'rmRF', err.message));
1796-
}
17971822
}
17981823
}
17991824

@@ -2726,4 +2751,4 @@ if (!global['_vsts_task_lib_loaded']) {
27262751
im._loadData();
27272752
im._exposeProxySettings();
27282753
im._exposeCertSettings();
2729-
}
2754+
}

‎node/test/cp.ts

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,19 @@ describe('cp cases', () => {
1414
const TEMP_DIR_2_FILE_1 = path.resolve(TEMP_DIR_2, 'file1');
1515
const TESTCASE_1 = path.resolve(TEMP_DIR_1, 'testcase_1');
1616
const TESTCASE_2 = path.resolve(TEMP_DIR_1, 'testcase_2');
17+
const TEST_SRC_DIR = 'test-src';
18+
const TEST_DEST_DIR = 'test-dest';
19+
const OUTSIDE_FILE = path.resolve(DIRNAME, 'outside-file.txt');
20+
const SYMLINK_NAME = 'symlink-outside.txt';
1721

1822
before((done) => {
1923
tl.mkdirP(TEMP_DIR_1);
2024
tl.mkdirP(TEMP_DIR_2);
21-
tl.cd(TEMP_DIR_1);
22-
25+
fs.mkdirSync(TEST_SRC_DIR, { recursive: true });
26+
const symlinkPath = path.join(TEST_SRC_DIR, SYMLINK_NAME);
27+
fs.writeFileSync(OUTSIDE_FILE, 'This is a file outside the source folder.');
28+
fs.symlinkSync(OUTSIDE_FILE, symlinkPath);
29+
fs.mkdirSync(TEST_DEST_DIR, { recursive: true });
2330
fs.writeFileSync(TEMP_DIR_2_FILE_1, 'file1');
2431

2532
try {
@@ -49,14 +56,16 @@ describe('cp cases', () => {
4956
tl.cd(DIRNAME);
5057
tl.rmRF(TEMP_DIR_1);
5158
tl.rmRF(TEMP_DIR_2);
52-
59+
tl.rmRF(OUTSIDE_FILE);
60+
fs.rmSync(TEST_SRC_DIR, { recursive: true, force: true });
61+
fs.rmSync(TEST_DEST_DIR, { recursive: true, force: true })
5362
done();
5463
});
5564

5665
it('Provide the source that does not exist', (done) => {
5766
assert.throws(() => tl.cp('pathdoesnotexist', TEMP_DIR_1), { message: /^ENOENT: no such file or directory/ });
5867
assert.ok(!fs.existsSync(path.join(TEMP_DIR_1, 'pathdoesnotexist')));
59-
68+
6069
done();
6170
});
6271

@@ -140,4 +149,16 @@ describe('cp cases', () => {
140149

141150
done();
142151
});
152+
153+
it('copy a directory containing symbolic link recursively', (done) => {
154+
155+
tl.cp(TEST_SRC_DIR, TEST_DEST_DIR, '-r', false, 0);
156+
157+
assert(fs.existsSync(path.join(TEST_DEST_DIR, TEST_SRC_DIR)), 'Directory was not copied');
158+
assert(fs.existsSync(path.join(TEST_DEST_DIR, TEST_SRC_DIR, 'outside-file.txt')), 'File was not copied');
159+
assert.equal(fs.readFileSync(path.join(TEST_DEST_DIR, TEST_SRC_DIR, 'outside-file.txt'), 'utf8'), 'This is a file outside the source folder.', 'File content is incorrect');
160+
assert(!fs.existsSync(path.join(TEST_DEST_DIR, TEST_SRC_DIR, SYMLINK_NAME)), 'Symbolic link should not be copied');
161+
162+
done();
163+
});
143164
});

‎node/test/dirtests.ts

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1472,8 +1472,8 @@ describe('Dir Operation Tests', function () {
14721472
assert(shell.test('-f', path.join(symlinkDirectory, 'real_file')), 'symlink directory should be created correctly');
14731473

14741474
tl.rmRF(symlinkDirectory);
1475-
assert(shell.test('-d', realDirectory), 'real directory should still exist');
1476-
assert(shell.test('-f', realFile), 'file should still exist');
1475+
assert(!shell.test('-d', realDirectory), 'real directory should be deleted');
1476+
assert(!shell.test('-f', realFile), 'file should be deleted');
14771477
assert(!shell.test('-e', symlinkDirectory), 'symlink directory should have been deleted');
14781478

14791479
done();
@@ -1501,7 +1501,6 @@ describe('Dir Operation Tests', function () {
15011501

15021502
done();
15031503
});
1504-
15051504
it('removes symlink file with missing source using rmRF', (done) => {
15061505
this.timeout(1000);
15071506

@@ -1529,7 +1528,6 @@ describe('Dir Operation Tests', function () {
15291528
catch (err) {
15301529
errcode = err.code;
15311530
}
1532-
15331531
assert.equal(errcode, 'ENOENT');
15341532

15351533
done();
@@ -1621,7 +1619,7 @@ describe('Dir Operation Tests', function () {
16211619

16221620
it('removes symlink folder with missing source using rmRF', (done) => {
16231621
this.timeout(1000);
1624-
1622+
16251623
// create the following layout:
16261624
// real_directory
16271625
// real_directory/real_file
@@ -1634,12 +1632,12 @@ describe('Dir Operation Tests', function () {
16341632
fs.writeFileSync(realFile, 'test file content');
16351633
testutil.createSymlinkDir(realDirectory, symlinkDirectory);
16361634
assert(shell.test('-f', path.join(symlinkDirectory, 'real_file')), 'symlink directory should be created correctly');
1637-
1635+
16381636
// remove the real directory
16391637
fs.unlinkSync(realFile);
16401638
fs.rmdirSync(realDirectory);
16411639
assert.throws(() => { fs.statSync(symlinkDirectory) }, (err: NodeJS.ErrnoException) => err.code == 'ENOENT', 'stat should throw');
1642-
1640+
16431641
// remove the symlink directory
16441642
tl.rmRF(symlinkDirectory);
16451643
let errcode: string;
@@ -1649,9 +1647,8 @@ describe('Dir Operation Tests', function () {
16491647
catch (err) {
16501648
errcode = err.code;
16511649
}
1652-
16531650
assert.equal(errcode, 'ENOENT');
1654-
1651+
16551652
done();
16561653
});
16571654

@@ -1681,7 +1678,7 @@ describe('Dir Operation Tests', function () {
16811678
}
16821679

16831680
tl.rmRF(symlinkLevel2Directory);
1684-
assert(shell.test('-f', path.join(symlinkDirectory, 'real_file')), 'real file should still exist');
1681+
assert(!shell.test('-f', path.join(symlinkDirectory, 'real_file')), 'real file should not exist');
16851682
assert(!shell.test('-e', symlinkLevel2Directory), 'symlink level 2 file should have been deleted');
16861683

16871684
done();

0 commit comments

Comments
 (0)