[js-sdk-release-tools] remove existing node_modules before restoring from backup

[jeremymeng]

otherwise the follow error could be thrown when the directory exists

[ERROR]: ENOTEMPTY: directory not empty, rename '/mnt/vss/_work/1/s/azure-sdk-for-js/node_modules_backup' -> '/mnt/vss/_work/1/s/azure-sdk-for-js/node_modules'

[Copilot]

Pull request overview

This PR addresses a file system error that occurs during the node_modules restoration process when the target directory already exists. The solution removes any existing node_modules directory before attempting to rename the backup.

Key changes:

• Renamed variable from nodeModulesPath to nodeModulesBackupPath for clarity
• Added logic to remove existing node_modules directory before restoration
• Split the restoration logic into separate existence checks for current and backup directories

Comments suppressed due to low confidence (1)

tools/js-sdk-release-tools/src/utils/backupNodeModules.ts:28

• The restoreNodeModules function, which includes critical file system operations (recursive deletion and renaming), lacks test coverage. The repository has comprehensive test coverage in the src/test directory for other utilities. Consider adding tests to verify the restore behavior, especially edge cases like when backup exists, when it doesn't exist, and when both backup and current node_modules exist.

export async function restoreNodeModules(folder: string) {
    const nodeModulesBackupPath = path.join(folder, "node_modules_backup");
    const nodeModulesPath = nodeModulesBackupPath.replace('_backup', '');
    if (fs.existsSync(nodeModulesPath)) {
        logger.info(`Remove existing '${nodeModulesPath}' first.`);
        fs.rmSync(nodeModulesPath, { recursive: true, force: true });
    }
    if (fs.existsSync(nodeModulesBackupPath)) {
        logger.info(`Start to rename '${nodeModulesBackupPath}' to '${nodeModulesPath}'.`);
        fs.renameSync(nodeModulesBackupPath, `${nodeModulesPath}`);
    }
    if ('/' === path.dirname(folder)) return;
    await restoreNodeModules(path.dirname(folder));
}

[jeremymeng]

cc @skywing918 I am not sure whether it is expected to assume node_modules does not exists. I hit this while trying to add a validation for dev-tool changes https://dev.azure.com/azure-sdk/public/public%20Team/_build/results?buildId=5657331&view=logs&j=b319f412-9f05-5128-7d78-96d289badc5e&t=359da817-671a-5e0e-eca5-0ca71486f7a0&l=1087

[MaryGao]

I remember we met some issues but I can't remember details and that's why we did the restore things.

@skywing918 Could you verify if we would meet issues with this change?

[qiaozha]

I don't think we can remove this part as it will cause some automation issue. Please double confirm.

[skywing918]

base on #5276, seems backupNodeModules, restoreNodeModules, is a temp solution. I'm not sure that we still need those?

also I have submit some jobs with this change. all jobs passed.
https://dev.azure.com/azure-sdk/internal/_build/results?buildId=5702099&view=results
https://dev.azure.com/azure-sdk/internal/_build/results?buildId=5702100&view=results
https://dev.azure.com/azure-sdk/internal/_build/results?buildId=5702101&view=results

[jeremymeng]

The original issue is probably related to rush or some other tool removing node_modules directory so we add a workaround to restore the old backup.