ninja -t cleandead should consider not failing on failure to remove

[mathstuf]

The cleandead tool is an assistance, and as such should not trigger failures for "unimportant" problems. One of which is failing to delete one of these files. This was reported to CMake's Discourse but belongs here.

[mathstuf]

The purpose (AFAIK) is to help remove files which used to be part of the build, but are no longer in it (e.g., you've removed a source file, so its object file is now no longer needed). It's a utility tool. If you clean the vacuum the deps log and then do cleandead, it will leave files behind since ninja has removed its knowledge of them. It's not a perfect tool, but an assistant. If it gets in the way like this (which is obviously "something weird going on"), just ignoring the issue (or noting it in output with -v) should be sufficient.

[trmagma]

I am the poster of the problem at cmake-discourse.
One source of the problem are version controlled files, which are copied from source to build directory. In some SCMs ( e.g. like clearcase or perforce) all files are read only (in source directory) until explicit checked out. A simple copy will just keep the permissions. I assume, even if the read only state was explicit set during the copy ninja will fail (not tested).

For this simple case it would be sufficient if ninja tries harder to delete the file by trying to remove the read only permission (similar like the -f option of the rm shell command). If this also failed, this might be an error.

CMake seems to call ninja -t cleandead automatically after generating the ninja build scripts (which would do there job perfectly) but fails, cause the ninja call failed. Think this call of cmake is perfectly right, cause it must be done after the new build scripts are created, but before the next build.

[jhasse]

If you clean the vacuum the deps log [...]

What do you mean by that?

For this simple case it would be sufficient if ninja tries harder to delete the file by trying to remove the read only permission (similar like the -f option of the rm shell command). If this also failed, this might be an error.

Hm ... not sure if this should be the default behavior though.

Why not remove the read-only permission when copying the files to the build directory?

[mathstuf]

Sorry, edited the sentence and never went back to proofread. When something like ninja -t recompact or rm .ninja_deps occurs.

Why not remove the read-only permission when copying the files to the build directory?

That's also a possibility. I think new enough CMake has file(COPY … FILE_PERMISSIONS) (and related arguments). 3.19's configure_file has that support as well.

Not sure about other generators, but CMake might be able to avoid this pretty easily at least.

[trmagma]

@jhasse:
I am also not sure if it should be the default behavior to try to get the write permission, but at least there should be an option to do so (as the rm shell tool has the -f option) .
If it is an option, there should be a way to tell cmake to use this option when calling the cleandead tool after generating the build scripts (e.g. a cmake variable).
What we know about the files, which might justify this behavior as default behavior : All files, that will be deleted are output files of a ninja build (at least there was a rule to build them), in prior build a ninja clean or ninja -t clean would have deleted them (or will also fail?, think this should be consistent), and we only succeed if the owner of the files is also calling cmake (otherwise permission change will fail).

@mathstuf
Some reasoning why explicit adding write-permissions is not a practical idea:

1. There a a lot of ways the file is generated in build directory. It might be a copy, it might be by unzipping an archive, or created by a custom command. In the end all this ways must ensure, that the generated files does not have the read only attribute. In the end all build-artifacts must be not be read only. At the end (at least almost) anything inside the build tree must have write permissions.
2. During install steps often the permissions are transferred from build directory to install directory, so decision of permissions is given to build (and as you say, there we have often explicit options to specify them). Otherwise every file permission must be known by install step.

[jhasse]

I've just tried: cleandead has no problem removing files without write permissions on my machine. Can you put together a minimal reproducible example?

[trmagma]

@jhasse: Have you tried on Windows? I remember that the problem occurred on windows platform (in a msys shell). Not sure, if there was a failure on linux. Will generate a small example this week.

[jhasse]

No, it was on Linux.

[bradking]

On Windows, read-only files sometimes need the read-only permission to be removed before the files themselves can be removed. One might also be able to reproduce this on Linux using files in a read-only directory.

[jhasse]

Feel free to re-open, if you think the fix is not enough.

[bradking]

For reference, I tested #1892 and #1913 with this example on Windows (in a MSYS prompt to get touch):

rule make_readonly
  command = cmd /c "touch $out && attrib +r $out"
build example: make_readonly
default example

Running ninja followed by ninja -t clean shows the problem, but is fixed by the PRs.