Skip to content

PosixFileSystem::CopyFile: Truncate file before overwrite#28510

Merged
tensorflow-copybara merged 1 commit into
tensorflow:masterfrom
tsawada:o_trunc
May 13, 2019
Merged

PosixFileSystem::CopyFile: Truncate file before overwrite#28510
tensorflow-copybara merged 1 commit into
tensorflow:masterfrom
tsawada:o_trunc

Conversation

@tsawada
Copy link
Copy Markdown
Contributor

@tsawada tsawada commented May 8, 2019

Without O_TRUNC, the original content of the target file will be left
over if the src file is shorter.

Fixes #28508

@tensorflow-bot tensorflow-bot Bot added the size:XS CL Change Size: Extra Small label May 8, 2019
@gbaned gbaned self-assigned this May 8, 2019
@gbaned gbaned requested a review from allenlavoie May 8, 2019 08:02
allenlavoie
allenlavoie previously approved these changes May 8, 2019
@tensorflow-bot tensorflow-bot Bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels May 8, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label May 8, 2019
@gbaned gbaned added the kokoro:force-run Tests on submitted change label May 9, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label May 9, 2019
@gbaned
Copy link
Copy Markdown
Contributor

gbaned commented May 10, 2019

@tsawada could you please resolve the conflicts? Thanks!

@googlebot
Copy link
Copy Markdown

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

ℹ️ Googlers: Go here for more info.

Without O_TRUNC, the original content of the target file will be left
over if the src file is shorter.
@googlebot
Copy link
Copy Markdown

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@tsawada
Copy link
Copy Markdown
Contributor Author

tsawada commented May 10, 2019

Oops, I force pushed a wrong branch with ill rebasing operation. I just did another force-push that is correctly rebased on the latest master.
I can't remove reviewers as I lack such permission, but I appreciate if anyone can remove alextp, azaks2, chsigg, frankchn, jhseu and saeta from the reviewers. Sorry for the spam.

@gbaned gbaned removed request for alextp, frankchn and jhseu May 10, 2019 08:27
@gbaned gbaned requested review from allenlavoie and removed request for azaks2, chsigg and saeta May 10, 2019 08:27
@tensorflow-bot tensorflow-bot Bot added the kokoro:force-run Tests on submitted change label May 10, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label May 10, 2019
@tsawada
Copy link
Copy Markdown
Contributor Author

tsawada commented May 11, 2019

Let me know if I should make another rebase to fix failing builds. It seems those builds were broken on master when I did last rebase.

@gbaned gbaned added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process and removed ready to pull PR ready for merge process labels May 13, 2019
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label May 13, 2019
@tensorflow-copybara tensorflow-copybara merged commit 83c4b27 into tensorflow:master May 13, 2019
@tsawada tsawada deleted the o_trunc branch May 15, 2019 04:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes ready to pull PR ready for merge process size:XS CL Change Size: Extra Small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

gfile.Copy does not overwrite dest file properly on posix filesystems

6 participants