Skip to content
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.

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

Atomically overwrite a file #6

Merged
merged 1 commit into from Jan 31, 2017

Conversation

@kohsuke
Copy link
Member

commented Jan 30, 2017

If a copy operation fails, for example because the source file doesn't
exist, the current code leaves a 0-length file.

The presence of such 0-length file can trip up tools like make. It's
better to copy into another temporary file, then only after making sure
that the copy is successful, place the file in its intended name.

@reviewbybees

If a copy operation fails, for example because the source file doesn't
exist, the current code leaves a 0-length file.

The presence of such 0-length file can trip up tools like make. It's
better to copy into another temporary file, then only after making sure
that the copy is successful, place the file in its intended name.
@kohsuke

This comment has been minimized.

Copy link
Member Author

commented Jan 30, 2017

Also see JENKINS-41413

kohsuke added a commit to jenkinsci/packaging that referenced this pull request Jan 30, 2017
Otherwise a failed Windows installer build will not be retried correctly
FilePath actual = new FilePath(channel, e.getKey());
workDir.child(e.getValue()).copyToWithPermission(tmp);
if (actual.exists())
actual.delete();

This comment has been minimized.

Copy link
@oleg-nenashev

oleg-nenashev Jan 30, 2017

Member

There is a risk of leaving the temporary file of this operation fails due to whatever reason. Likely it's not important

@oleg-nenashev

This comment has been minimized.

Copy link
Member

commented Jan 30, 2017

🐝

@jtnord

This comment has been minimized.

Copy link
Member

commented Jan 30, 2017

🐝 but seems like an underlying bug in Jenkins FilePath.copyToWithPermission(...)

@kohsuke kohsuke merged commit 4f005b4 into master Jan 31, 2017
1 check passed
1 check passed
Jenkins This pull request looks good
Details
@kohsuke kohsuke deleted the atomic-rename branch Jan 31, 2017
kohsuke added a commit to jenkinsci/packaging that referenced this pull request Jan 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.