Fix EEXIST error when building kibana a second time #34852
Conversation
|
Pinging @elastic/kibana-operations |
💔 Build Failed |
|
Test failures look legit :) |
|
Any steps to reproduce? During the build, we remove the entire target/build directories. |
|
To reproduce: The clean task doesn't remove files in |
|
I'm kinda suspicious of this, https://nodejs.org/api/fs.html#fs_fs_copyfile_src_dest_flags_callback implies that this is not the behavior of
|
|
@spalger I agree, this is not the behaviour that the documentation suggests, I've created nodejs/node#27273 |
💚 Build Succeeded |
💚 Build Succeeded |
src/dev/build/lib/fs.js
Outdated
| // Delete `destination` since `fs.copyFile` with the copy-on-write reflink/COPYFILE_FICLONE | ||
| // flag fails if the destination already exists. https://github.com/nodejs/node/issues/27273 | ||
| try { | ||
| await unlinkAsync(destination); |
There was a problem hiding this comment.
Is this still necicary after cleaning the .node_binaries directory? My concern is the overhead of an additional delete call before any copy when it's not needed.
There was a problem hiding this comment.
cleaning node_binaries solves the original problem, so we don't need to add the unlink call.
I kept it there since it felt like a bug in our fs.copy() implementation, because we abstract away how we do the copy, I would expect this to behave like Node's fs.copy and then it weirdly doesn't without it being obvious from the calling code that we're passing in COPYFILE_FICLONE.
Since the whole point of using COPYFILE_FICLONE is for performance it makes sense that we want to prevent unnecessary calls.
I'm thinking perhaps it's best to rename the function to copyOnWrite and add a comment explaining the behaviour, but removing the unlink call.
💔 Build Failed |
💔 Build Failed |
💔 Build Failed |
|
Looks like the failure is related |
💔 Build Failed |
|
retest |
💚 Build Succeeded |
rudolf
changed the title
* Unlink destination before copy-on-write * Catch unlink exception if destination file doesn't exist * Add link to upstream node issue * Add .node_binaries to kbn cleanup * Remove unlink call * Remove fs.copy() * Fix extract_node_builds_task test * Remove copy from build/lib exports
rudolf
changed the title
💔 Build Failed |
* Unlink destination before copy-on-write * Catch unlink exception if destination file doesn't exist * Add link to upstream node issue * Add .node_binaries to kbn cleanup * Remove unlink call * Remove fs.copy() * Fix extract_node_builds_task test * Remove copy from build/lib exports
Summary
Building Kibana for the second time fails with:
This PR unlinks the destination before attempting to do a copy-on-write to prevent these failures.This PR adds
.node_binariestokbn cleanupand moves the very specialisedfs.copy()function into the calling code.