-
Notifications
You must be signed in to change notification settings - Fork 39
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
rm: cannot remove: Invalid argument in docker volume folder #59
Comments
How does this succeed |
"test" folder created with docker mount: docker run -it --rm -v c:\test msys-test. |
I think this one is the crucial tidbit. In Git for Windows, we figured out that Docker mounts are represented as reparse points that totally look like symbolic links, albeit with a totally bogus target path (at least bogus as far as user space is concerned). The workaround looks like this: git-for-windows/git@91060f3. We most likely need to do something similar in the Cygwin runtime (because the MSYS2 runtime is a close fork of the Cygwin runtime, and this issue most likely also affects Cygwin): before "resolving a symbolic link", we need to whether its target path starts with |
I think option |
@github-cygwin If @dscho's analysis is correct this could probably be band-aided pretty easily by checking for that prefix |
No luck even after setting this environment variable. |
Since the demanding tone of #59 (comment) rubs me in all the wrong ways, I will only give a pointer here and refuse to spend any more of my expert time on this ticket: it should be interesting to instrument |
@ZFail Just to rule out the possibility that it could be related to how MSYS bash interprets the string, can you try out the below command & see if it works in your setup? Assuming \a is wrongly interpreted by bash?? |
This is successful strace log,
failed trace, in mounted volume.
It fails in unlink_nt(), there is a problem that does not related to reparsepoint. I tried old msys-2.0.dll, that works with mounted volume.
vvv additional report |
That looks like
@YO4 you mean to imply that this part of the code fails? If so, it is inside a conditional testing for: if (wincap.has_posix_unlink_semantics ()
&& !pc.isremote () && pc.fs_is_ntfs ()) That condition seems to make sense, but maybe it is incomplete? Or maybe something inside the conditional code block needs to be adjusted. To shed more light into this, it would probably make most sense to instrument the code more. To do that, clone https://github.com/msys2/msys2-runtime/, litter that diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index f6ffb1cf61..97bccf818e 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -721,6 +721,7 @@ _unlink_nt (path_conv &pc, bool shareable)
/* First check if we can use POSIX unlink semantics: W10 1709+, local NTFS.
With POSIX unlink semantics the entire job gets MUCH easier and faster.
Just try to do it and if it fails, it fails. */
+small_printf("%s:%d: has posix unlink semantics: %d, is remote: %d, is ntfs: %d\n", __FILE__, __LINE__, (int)wincap.has_posix_unlink_semantics(), (int)pc.isremote(), (int)pc.fs_is_ntfs());
if (wincap.has_posix_unlink_semantics ()
&& !pc.isremote () && pc.fs_is_ntfs ())
{
@@ -752,6 +753,12 @@ _unlink_nt (path_conv &pc, bool shareable)
fdie.Flags |= FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE;
status = NtSetInformationFile (fh, &io, &fdie, sizeof fdie,
FileDispositionInformationEx);
+small_printf("%s:%d: NtSetInformationFile returned status 0x%lx\n", __FILE__, __LINE__, (DWORD)status);
+if (status == STATUS_INVALID_PARAMETER)
+ {
+ status = STATUS_CANNOT_DELETE;
+small_printf("%s:%d: forced status 0x%lx\n", __FILE__, __LINE__, (DWORD)status);
+ }
/* Restore R/O attribute in case we have multiple hardlinks. */
if (access & FILE_WRITE_ATTRIBUTES)
NtSetAttributesFile (fh, pc.file_attributes ()); Then, commit, push and open a draft Pull Request. This will kick off a build and publish the build artifacts. You can then download these artifacts in a GitHub workflow run using something like this: - name: use msys2-runtime build artifact
shell: bash
run: |
run_id=4182314287 &&
name=install &&
curl -L https://api.github.com/repos/msys2/msys2-runtime/actions/runs/$run_id/artifacts |
jq -r '.artifacts[] | select(.name | test("'$name'")) | [.name, .archive_download_url] | @tsv' |
tr -d '\r' |
while read name url
do
echo "$name"
curl -H "Authorization: token ${{secrets.GITHUB_TOKEN}}" \
-#sLo /tmp/"$name".zip "$url" &&
unzip -q -d / /tmp/"$name".zip usr/bin/msys-2.0.dll
done You will most likely have to play variations on this theme because you will need to get this |
tested on my local environment
and unlink success. |
Another problem appeared.
|
related to FILE_RENAME_POSIX_SEMANTICS ? |
Awesome. @YO4 could you prepare a proper commit (leaving out the
I lost the context, could you describe what type of entities
Quite possibly ;-) We know it's a problem for However, 0xC0000034 corresponds to The best advice I can give is to again litter the code with |
Sorry to keep you waiting. I apologize for the inconvenience. And I believe I have confirmed that posix_semantics in bind mount returns an error in the hyper-v container. The runner provided by Github Actions could not be used for this purpose because it cannot run the container using hyper-v isolation so I used Appveyor in this case. I was working on windows 10 that has default hyper-v isolation, so I was affected by this issue. |
I've done some testing locally, and found forcing The output from strace for failed mv: https://pastebin.com/QtP0w4QH strace for working mv: https://pastebin.com/PASMmigz I can't see any file attributes that would signify why ordinary file: 0x1c706df The differing flags being:
|
Given There are related discussions: microsoft/opengcs#337 and moby/moby#39922 though opengcs has now moved to https://github.com/microsoft/hcsshim |
Maybe I missed it while trying to read through all comments: As I see this error both when running git bash and in cywin I wonder whether a ticket for cygwin exists already in this regard? |
@schwaerz the fixes have been in cygwin/master for a while, but have only gone into version 3.5+. The current release of MSYS2 is from 3.4.10. |
@AJDurant You do not happen to know the cygwin package (version) that includes the fix? |
@schwaerz in the source code, the fixes are in both 3.5.0 and 3.5.1 (the current latest). I'm not a cygwin user, so I haven't tested it. I also moved all out workflows to Win11/Server2022 to allow process isolation for containers, and avoid this issue. |
Thanks. I can confirm that file deletion works when using cygwin 3.5.1 👍 |
Setup
Dockerfile
Details
CMD
rm should delete the file
rm returns an error
The text was updated successfully, but these errors were encountered: