Skip to content

[DEPENDENCY] Switch from "rimraf" to native "fs.rm" #780

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
matz3 merged 1 commit into from
Nov 25, 2024
Merged

[DEPENDENCY] Switch from "rimraf" to native "fs.rm" #780

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/build/ProjectBuilder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {rimraf} from "rimraf";
import {rmrf} from "../utils/fs.js";
import * as resourceFactory from "@ui5/fs/resourceFactory";
import BuildLogger from "@ui5/logger/internal/loggers/Build";
import composeProjectList from "./helpers/composeProjectList.js";
Expand Down Expand Up @@ -233,7 +233,7 @@ class ProjectBuilder {

if (cleanDest) {
this.#log.info(`Cleaning target directory...`);
await rimraf(destPath);
await rmrf(destPath);
}
const startTime = process.hrtime();
try {
Expand Down
8 changes: 4 additions & 4 deletions lib/ui5Framework/maven/Installer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {promisify} from "node:util";
import Registry from "./Registry.js";
import AbstractInstaller from "../AbstractInstaller.js";
import CacheMode from "./CacheMode.js";
import {rimraf} from "rimraf";
import {rmrf} from "../../utils/fs.js";
const stat = promisify(fs.stat);
const readFile = promisify(fs.readFile);
const writeFile = promisify(fs.writeFile);
Expand Down Expand Up @@ -275,7 +275,7 @@ class Installer extends AbstractInstaller {
if (pkgName) {
const packageDir = this._getTargetDirForPackage(pkgName, revision);
log.verbose(`Removing directory ${packageDir}...`);
await rimraf(packageDir);
await rmrf(packageDir);
}
}
}
Expand Down Expand Up @@ -326,7 +326,7 @@ class Installer extends AbstractInstaller {
// Check whether staging dir already exists and remove it
if (await this._pathExists(stagingDir)) {
log.verbose(`Removing stale staging directory at ${stagingDir}...`);
await rimraf(stagingDir);
await rmrf(stagingDir);
}

await mkdirp(stagingDir);
Expand All @@ -345,7 +345,7 @@ class Installer extends AbstractInstaller {
// Check whether target dir already exists and remove it
if (await this._pathExists(targetDir)) {
log.verbose(`Removing existing target directory at ${targetDir}...`);
await rimraf(targetDir);
await rmrf(targetDir);
}

// Do not create target dir itself to prevent EPERM error in following rename operation
Expand Down
6 changes: 3 additions & 3 deletions lib/ui5Framework/npm/Installer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import fs from "graceful-fs";
import {promisify} from "node:util";
import Registry from "./Registry.js";
import AbstractInstaller from "../AbstractInstaller.js";
import {rimraf} from "rimraf";
import {rmrf} from "../../utils/fs.js";
const stat = promisify(fs.stat);
const readFile = promisify(fs.readFile);
const rename = promisify(fs.rename);
Expand Down Expand Up @@ -99,7 +99,7 @@ class Installer extends AbstractInstaller {
// Check whether staging dir already exists and remove it
if (await this._pathExists(stagingDir)) {
log.verbose(`Removing existing staging directory at ${stagingDir}...`);
await rimraf(stagingDir);
await rmrf(stagingDir);
}

// Check whether target dir already exists and remove it.
Expand All @@ -108,7 +108,7 @@ class Installer extends AbstractInstaller {
// directory so that the rename operation won't have any no trouble.
if (await this._pathExists(targetDir)) {
log.verbose(`Removing existing target directory at ${targetDir}...`);
await rimraf(targetDir);
await rmrf(targetDir);
}

log.verbose(`Installing ${pkgName} in version ${version} to ${stagingDir}...`);
Expand Down
5 changes: 5 additions & 0 deletions lib/utils/fs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import fs from "graceful-fs";
import {promisify} from "node:util";
const mkdir = promisify(fs.mkdir);
const rm = promisify(fs.rm);

export async function mkdirp(dirPath) {
return mkdir(dirPath, {recursive: true});
}

export async function rmrf(dirPath) {
return rm(dirPath, {recursive: true, force: true});
}
9 changes: 8 additions & 1 deletion package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
"version": "git-chglog --sort semver --next-tag v$npm_package_version -o CHANGELOG.md v4.0.0.. && git add CHANGELOG.md",
"prepublishOnly": "git push --follow-tags",
"release-note": "git-chglog --sort semver -c .chglog/release-config.yml v$npm_package_version",
"depcheck": "depcheck --ignores @ui5/project,docdash,@istanbuljs/esm-loader-hook"
"depcheck": "depcheck --ignores @ui5/project,docdash,@istanbuljs/esm-loader-hook,rimraf"
},
"files": [
"CHANGELOG.md",
Expand Down Expand Up @@ -138,7 +138,6 @@
"read-package-up": "^11.0.0",
"read-pkg": "^9.0.1",
"resolve": "^1.22.8",
"rimraf": "^6.0.1",
"semver": "^7.6.3",
"xml2js": "^0.6.2",
"yesno": "^0.4.0"
Expand Down Expand Up @@ -173,6 +172,7 @@
"jsdoc": "^4.0.4",
"nyc": "^17.1.0",
"open-cli": "^8.0.0",
"rimraf": "^6.0.1",
"sinon": "^19.0.2",
"tap-xunit": "^2.4.1"
}
Expand Down
7 changes: 3 additions & 4 deletions test/lib/ui5framework/maven/Installer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@ import sinon from "sinon";
import esmock from "esmock";
import path from "node:path";
import fs from "graceful-fs";
import {rimraf} from "rimraf";

test.beforeEach(async (t) => {
t.context.mkdirpStub = sinon.stub().resolves();
t.context.rimrafStub = sinon.stub().resolves();
t.context.rmrfStub = sinon.stub().resolves();
t.context.readFileStub = sinon.stub();
t.context.writeFileStub = sinon.stub();
t.context.renameStub = sinon.stub().returns();
Expand All @@ -20,7 +19,6 @@ test.beforeEach(async (t) => {
t.context.promisifyStub.withArgs(fs.rename).callsFake(() => t.context.renameStub);
t.context.promisifyStub.withArgs(fs.rm).callsFake(() => t.context.rmStub);
t.context.promisifyStub.withArgs(fs.stat).callsFake(() => t.context.statStub);
t.context.promisifyStub.withArgs(rimraf).callsFake(() => t.context.rimrafStub);

t.context.lockStub = sinon.stub();
t.context.unlockStub = sinon.stub();
Expand All @@ -39,7 +37,8 @@ test.beforeEach(async (t) => {

t.context.AbstractInstaller = await esmock.p("../../../../lib/ui5Framework/AbstractInstaller.js", {
"../../../../lib/utils/fs.js": {
mkdirp: t.context.mkdirpStub
mkdirp: t.context.mkdirpStub,
rmrf: t.context.rmrfStub
},
"lockfile": {
lock: t.context.lockStub,
Expand Down
30 changes: 13 additions & 17 deletions test/lib/ui5framework/npm/Installer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const __dirname = import.meta.dirname;

test.beforeEach(async (t) => {
t.context.mkdirpStub = sinon.stub().resolves();
t.context.rimrafStub = sinon.stub().resolves();
t.context.rmrfStub = sinon.stub().resolves();

t.context.lockStub = sinon.stub();
t.context.unlockStub = sinon.stub();
Expand All @@ -16,10 +16,8 @@ test.beforeEach(async (t) => {

t.context.AbstractResolver = await esmock.p("../../../../lib/ui5Framework/AbstractInstaller.js", {
"../../../../lib/utils/fs.js": {
mkdirp: t.context.mkdirpStub
},
"rimraf": {
rimraf: t.context.rimrafStub
mkdirp: t.context.mkdirpStub,
rmrf: t.context.rmrfStub
},
"lockfile": {
lock: t.context.lockStub,
Expand All @@ -29,10 +27,8 @@ test.beforeEach(async (t) => {
t.context.Installer = await esmock.p("../../../../lib/ui5Framework/npm/Installer.js", {
"../../../../lib/ui5Framework/AbstractInstaller.js": t.context.AbstractResolver,
"../../../../lib/utils/fs.js": {
mkdirp: t.context.mkdirpStub
},
"rimraf": {
rimraf: t.context.rimrafStub
mkdirp: t.context.mkdirpStub,
rmrf: t.context.rmrfStub
},
"graceful-fs": {
rename: t.context.renameStub,
Expand Down Expand Up @@ -512,7 +508,7 @@ test.serial("Installer: installPackage with new package", async (t) => {
"_packageJsonExists should be called with the correct arguments");
t.is(pathExistsStub.getCall(1).args[0], targetDir,
"_packageJsonExists should be called with the correct arguments");
t.is(t.context.rimrafStub.callCount, 0, "rimraf should never be called");
t.is(t.context.rmrfStub.callCount, 0, "rmrf should never be called");

t.is(extractPackageStub.callCount, 1, "_extractPackage should be called once");

Expand Down Expand Up @@ -577,7 +573,7 @@ test.serial("Installer: installPackage with already installed package", async (t
t.is(t.context.unlockStub.callCount, 0, "unlock should never be called");
t.is(getStagingDirForPackageStub.callCount, 0, "_getStagingDirForPackage should never be called");
t.is(pathExistsStub.callCount, 0, "_pathExists should never be called");
t.is(t.context.rimrafStub.callCount, 0, "rimraf should never be called");
t.is(t.context.rmrfStub.callCount, 0, "rmrf should never be called");
t.is(extractPackageStub.callCount, 0, "_extractPackage should never be called");
t.is(t.context.mkdirpStub.callCount, 0, "mkdirp should never be called");
t.is(t.context.renameStub.callCount, 0, "fs.rename should never be called");
Expand Down Expand Up @@ -633,7 +629,7 @@ test.serial("Installer: installPackage with install already in progress", async
t.is(t.context.lockStub.callCount, 1, "lock should be called once");
t.is(t.context.unlockStub.callCount, 1, "unlock should be called once");

t.is(t.context.rimrafStub.callCount, 0, "rimraf should never be called");
t.is(t.context.rmrfStub.callCount, 0, "rmrf should never be called");

t.is(t.context.mkdirpStub.callCount, 1, "mkdirp should be called once");
t.is(t.context.mkdirpStub.getCall(0).args[0], path.join("/", "ui5Data", "framework", "locks"),
Expand Down Expand Up @@ -709,11 +705,11 @@ test.serial("Installer: installPackage with new package and existing target and
t.is(pathExistsStub.getCall(1).args[0], targetDir,
"_packageJsonExists should be called with the correct arguments");

t.is(t.context.rimrafStub.callCount, 2, "rimraf should be called twice");
t.is(t.context.rimrafStub.getCall(0).args[0], "staging-dir-path",
"rimraf should be called with the correct arguments");
t.is(t.context.rimrafStub.getCall(1).args[0], targetDir,
"rimraf should be called with the correct arguments");
t.is(t.context.rmrfStub.callCount, 2, "rmrf should be called twice");
t.is(t.context.rmrfStub.getCall(0).args[0], "staging-dir-path",
"rmrf should be called with the correct arguments");
t.is(t.context.rmrfStub.getCall(1).args[0], targetDir,
"rmrf should be called with the correct arguments");

t.is(extractPackageStub.callCount, 1, "_extractPackage should be called once");

Expand Down