fix: should not fail when output directory is the root directory#10545
fix: should not fail when output directory is the root directory#10545haoqunjiang wants to merge 5 commits into
Conversation
Fixes webpack#10544. Though the `EISDIR` error code is not mentioned in the POSIX standard, FreeBSD and memfs both throws this error when trying to run `mkdir` on `/`. So essentially we should treat it the same as `EEXIST`. References: * https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=59739 * streamich/memfs#326
|
For maintainers only:
|
|
|
||
| module.exports = { | ||
| findBundle: function() { | ||
| return [path.relative(outputDirectory, "/tmp/bundle.js")]; |
There was a problem hiding this comment.
If I return an absolute path here directly, the _require function in ConfigTestCases.test.js would require the bundle directly, making it lack of essential context utilities like the it function.
So the only workaround I found is to calculate the relative path of /tmp/bundle.js.
|
/cc @sodatea friendly ping |
|
I think I need to work out a more generic way to deal with the root directory case, instead of relying on the error code because different OSes😅 |
mkdirp| * @returns {void} | ||
| */ | ||
| const mkdirp = (fs, p, callback) => { | ||
| if (path.resolve(p) === path.resolve("/")) { |
There was a problem hiding this comment.
Do you really need so many path.resolve? They have a performance cost. I think p is always a absolute path anyway. What is about windows absolute paths?
| if (path.resolve(p) === path.resolve("/")) { | |
| if (/^(\/|[A-Za-z]:\\?|\\\\[^\\]+\\?)$/.test(p)) { |
There was a problem hiding this comment.
This will fail edge cases like multiple trailing backslashes (such as C:\\\\).
Though, we can avoid the performance overhead by moving the logic into the error handling part.
|
Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon. |
Fixes #10544.
Though the
EISDIRerror code is not mentioned in the POSIX standard,FreeBSD and
memfsboth throw this error when trying to runmkdiron/. So essentially we should treat it the same asEEXIST.References:
What kind of change does this PR introduce?
A bugfix
Did you add tests for your changes?
I'm not sure how I should add tests, because this issue only happens on BSD systems.
Does this PR introduce a breaking change?
No
What needs to be documented once your changes are merged?
Nothing