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

bazel clean --expunge often fails on Windows #1586

Closed
rokstrnisa opened this issue Jul 29, 2016 · 19 comments
Closed

bazel clean --expunge often fails on Windows #1586

rokstrnisa opened this issue Jul 29, 2016 · 19 comments
Assignees
Labels
P1 I'll work on this now. (Assignee required) platform: windows type: bug
Milestone

Comments

@rokstrnisa
Copy link

$ bazel clean --expunge
INFO: Starting clean (this may take a while). Consider using --expunge_async if the clean takes more than several minutes.
ERROR: C:/tools/msys64/var/tmp/Bazel/$KNt4ZiP/command.log (Permission denied).

command.log contains no information about the bazel clean --expunge failure, only output of the previous build commands.

@damienmg damienmg added type: bug P2 We'll consider working on this in future. (Assignee optional) platform: windows labels Jul 29, 2016
@damienmg damienmg added this to the 0.4 milestone Jul 29, 2016
@laszlocsomor
Copy link
Contributor

The command.log is what contains the output above, so the clean command is trying to log the clean event and delete the log file at the same time.

@laszlocsomor laszlocsomor self-assigned this Sep 6, 2016
@dslomov dslomov removed this from the 0.4 milestone Sep 21, 2016
@meteorcloudy meteorcloudy added P1 I'll work on this now. (Assignee required) and removed P2 We'll consider working on this in future. (Assignee optional) labels Oct 7, 2016
@laszlocsomor
Copy link
Contributor

I did some debugging. On Linux we can delete the command.log at any time using rm, but on Windows we always get "Device or resource busy" as long as the bazel server is running (i.e. until you run bazel shutdown).

@jakemac53
Copy link

I get a similar issue when using new_local_repository, it seems the tools\msys64 directory is read only, and it keeps reverting back to read only on its own if I change it....

@laszlocsomor
Copy link
Contributor

@jakemac53 : Thanks for reporting that. It doesn't sound like the same issue to me. Is it reproducible? If so, could you please report a separate issue for it?

As for this bug I made sure the command.log is closed before attempting to actually clean the output tree and now the command can proceed, but then finds another open file jvm.out. We may need to find a principled way to close all file handles before cleaning. The alternative would be to open files with delete permission (I recall CreateFile has something similar on Windows) but I haven't found any way to do that with java.io/java.nio.

bazel-io pushed a commit that referenced this issue Oct 27, 2016
Open files cannot be deleted on Windows, thus
`bazel clean --expunge` fails when it attemps to
delete the `command.log` that stdout/stderr is
tee'd into, and so does BlazeCommandDispatcher
when it attemps to delete the `command.log` just
before dispatching to the command implementation
(not just `clean` but any command).

This change:
- closes `command.log` before we attempt to
  delete it
- marks CleanCommand (through the Command
  annotation) as one that should not write to the
  command.log, thus we don't create a new instance
  of the file

This change allows `bazel clean --expunge` to
delete everything in the output base, with the
exception of `jvm.log`. Unfortunately that file is
opened by the C++ bazel client process, so we have
to close it there prior to sending the clean to
the bazel server.

See #1586

--
MOS_MIGRATED_REVID=137278553
@petemounce
Copy link
Contributor

Is 7f3ffbc in 0.4.0-rc3?

λ bazel clean --expunge
INFO: Starting clean (this may take a while). Consider using --expunge_async if the clean takes more than several minutes.
ERROR: C:/tools/msys64/var/tmp/Bazel/pkGed45b/command.log (Permission denied).

@laszlocsomor
Copy link
Contributor

The release candidate was cut prior to this commit. :(
@damienmg , can we cut the new RC at a later commit? This is not the first change I'd like to get into 0.4

@damienmg
Copy link
Contributor

+Kristina Chodorow kchodorow@google.com is the 0.4 release manager. I
guess we can cherry-pick it since a RC today still means we can release on
wednesday. How important it is?

On Mon, Oct 31, 2016 at 4:57 PM László Csomor notifications@github.com
wrote:

The release candidate was cut prior to this commit. :(
@damienmg https://github.com/damienmg , can we cut the new RC at a
later commit? This is not the first change I'd like to get into 0.4


You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#1586 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADjHfyhd_e6pkSeTEEOd4JhWmStC6q7rks5q5g_mgaJpZM4JYSwo
.

@laszlocsomor
Copy link
Contributor

It's a P1, so important. When's the next release expected, if this doesn't make into 0.4?

@damienmg
Copy link
Contributor

Every month so somewhere in November.

On Mon, Oct 31, 2016 at 5:24 PM László Csomor notifications@github.com
wrote:

It's a P1, so important. When's the next release expected, if this doesn't
make into 0.4?


You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#1586 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADjHf7SXUSL5F9MoLcF_O7Izlxa9swPnks5q5hY4gaJpZM4JYSwo
.

@ulfjack
Copy link
Contributor

ulfjack commented Dec 1, 2016

It's certainly in 0.4.2 now.

@laszlocsomor
Copy link
Contributor

We still see "bazel clean" failures sometimes, and it may be due to the persistent java workers holding on to open files, which is also the suspected culprit of #2538.

@laszlocsomor laszlocsomor reopened this Feb 23, 2017
@dslomov
Copy link
Contributor

dslomov commented Feb 28, 2017

@laszlocsomor @philwo what's the right target milestone? Is it still a P1?

@philwo
Copy link
Member

philwo commented Mar 2, 2017

(Sorry, wrong bug. Moving my comment to the correct one.)

@dslomov dslomov added this to the 0.6 milestone Apr 4, 2017
@laszlocsomor
Copy link
Contributor

Sorry, dcaee09 is an irrelevant commit.

@damienmg
Copy link
Contributor

Steren probably have the wrong bug number in he change

@Helcaraxan
Copy link

Helcaraxan commented May 11, 2017

This is still an issue.
From what I have seen @laszlocsomor's comment #1586 (comment) is spot-on as I have to kill all Bazel processes manually before the bazel clean works again.

@philwo
Copy link
Member

philwo commented May 29, 2017

I ran into this just now:

> bazel clean --expunge                                                                                                   
INFO: Reading 'startup' options from C:\Users\philwo/.bazelrc: --output_user_root=c:/src/_bazel_philwo                    
INFO: Starting clean (this may take a while). Consider using --expunge_async if the clean takes more than several minutes.
ERROR: C:/src/_bazel_philwo/o7dye3qe/java.log (Permission denied)                                                         

@laszlocsomor explained that it happens because the Java server tries to delete its log file, which it holds an open handle to (because its stdout is redirected to that file) and thus fails to delete it. We could create the file with delete sharing permissions, which would allow the server to delete it despite the open handle, but then writes to the deleted file will be lost.

@philwo pointed out that we only try to delete the log file with clean --expunge, but when doing so, we also exit the server process after the clean command finished - so the situation where we try to write to the deleted log file should actually never happen (and even if it happens, it's probably rather irrelevant).

@laszlocsomor
Copy link
Contributor

@laszlocsomor explained that it happens because the Java server tries to delete its log file

I realized this is wrong. The file in question is "java.log", which is created and held open by the java process, see startup_options.cc:416. The file I talked about earlier was "jvm.log", which is indeed created in blaze::CreateJvmOutputFile so we could open that with deletion sharing.

@laszlocsomor
Copy link
Contributor

Related bugs: #3043, #2480, #1906

bazel-io pushed a commit that referenced this issue Jun 2, 2017
This allows `bazel clean` to delete this file.

See #1586
See #1906
See #2480
See #3043

Change-Id: I245f368c2f2564511bbe6f06193a3ead49724d7b
PiperOrigin-RevId: 157818284
luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this issue Sep 5, 2022
    Open files cannot be deleted on Windows, thus
    `bazel clean --expunge` fails when it attemps to
    delete the `command.log` that stdout/stderr is
    tee'd into, and so does BlazeCommandDispatcher
    when it attemps to delete the `command.log` just
    before dispatching to the command implementation
    (not just `clean` but any command).

    This change:
    - closes `command.log` before we attempt to
      delete it
    - marks CleanCommand (through the Command
      annotation) as one that should not write to the
      command.log, thus we don't create a new instance
      of the file

    This change allows `bazel clean --expunge` to
    delete everything in the output base, with the
    exception of `jvm.log`. Unfortunately that file is
    opened by the C++ bazel client process, so we have
    to close it there prior to sending the clean to
    the bazel server.

    See bazelbuild/bazel#1586

    --
    MOS_MIGRATED_REVID=137278553
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 I'll work on this now. (Assignee required) platform: windows type: bug
Projects
None yet
Development

No branches or pull requests

10 participants