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

Support windows container. #1645

Merged
merged 4 commits into from
Apr 24, 2018
Merged

Support windows container. #1645

merged 4 commits into from
Apr 24, 2018

Conversation

ZCube
Copy link

@ZCube ZCube commented Apr 22, 2018

Ignore symbolic link checks for Windows Container Docker

  • prevent '/ContainerMappedDirectories/': No such file or directory
    Copy and remove file. MoveFile has problem
  • unable to write symref for HEAD: Permission denied
  • etc....

related issue : #1007

Signed-off-by: ZCube [email protected]

@dscho
Copy link
Member

dscho commented Apr 23, 2018

This looks really good! Thank you!

I would like to ask for a few minor changes before merging:

  • could you add a "Signed-off-by:" line using your real name to the commit message? Git for Windows is a friendly fork of https://github.com/git/git and we want to contribute back all our commits eventually, and they require that sign-off.

  • I would suggest to refactor the code to detect whether we are running in a Windows container into a function is_inside_windows_container() in compat/mingw.c that stores the result in a static int inside_container that gets initialized only once, upon demand. We will need to call this function at least in two locations, I think.

  • it would be better to not even report the "ContainerMappedDirectories" as symbolic link. To do this, the signature of file_attr_to_st_mode() needs to be extended to receive the full path, too, and the function needs to be moved from compat/win32.h to compat/mingw.c. Then we can test is_running_in_windows_container() when we detect a reparse point, and look whether it points at that ContainerMappedDirectories, and if that is the case, simply switch to S_IFDIR instead.

  • it seems that the work-around for the failed rename() needs to be guarded at least behind a if (gle == ERROR_ACCESS_DENIED && is_running_in_windows_container())? (The exact error code might be a different one, of course, could you test?)

With these changes, I will be super happy to merge this and finally fix #1007. Thank you very much!

@ZCube ZCube force-pushed the master branch 2 times, most recently from b4b112f to cc9582e Compare April 23, 2018 13:30
@ZCube
Copy link
Author

ZCube commented Apr 23, 2018

I modified the source code.
This looks better.
Need I change my git account to real name?
BusyBox version needed.

Windows 10 Pro

Microsoft Windows [Version 10.0.14393]
(c) 2016 Microsoft Corporation. All rights reserved.

V:\>ver

Microsoft Windows [Version 10.0.14393]

V:\>

Before this PR

C:\code>dir \
 Volume in drive C has no label.
 Volume Serial Number is 047C-45B9

 Directory of C:\

04/22/2018  05:20 PM    <SYMLINKD>     code [\\?\ContainerMappedDirectories\647A5395-BFFF-418E-956E-1059CB896EFA]
04/23/2018  10:07 PM    <DIR>          Git
11/23/2016  07:45 AM             1,894 License.txt
03/05/2018  04:42 AM    <DIR>          PerfLogs
03/05/2018  04:53 AM    <DIR>          Program Files
07/16/2016  10:18 PM    <DIR>          Program Files (x86)
03/05/2018  04:54 AM    <DIR>          Users
03/05/2018  04:52 AM    <DIR>          Windows
               1 File(s)          1,894 bytes
               7 Dir(s)  128,406,323,200 bytes free

C:\code>git init
fatal: Invalid path '/ContainerMappedDirectories': No such file or directory

C:\code>v:

V:\>dir
 Volume in drive V is 1TB
 Volume Serial Number is 8A15-5E5F

 Directory of V:\

04/23/2018  10:05 PM    <DIR>          .
04/23/2018  10:05 PM    <DIR>          ..
               0 File(s)              0 bytes
               2 Dir(s)  92,483,219,456 bytes free

V:\>git init
Rename from 'V:/.git/HEAD.lock' to 'V:/.git/HEAD' failed. Should I try again? (y/n) n
error: unable to write symref for HEAD: Permission denied

After this PR

C:\code>cd test

C:\code\test>git clone https://github.com/githubtraining/example-dependency --recursive
Cloning into 'example-dependency'...
remote: Counting objects: 39, done.
remote: Total 39 (delta 0), reused 0 (delta 0), pack-reused 39
Unpacking objects: 100% (39/39), done.
Submodule 'js' (https://github.com/githubtraining/example-submodule.git) registered for path 'js'
Cloning into 'C:/code/test/example-dependency/js'...
remote: Counting objects: 18, done.
remote: Total 18 (delta 0), reused 0 (delta 0), pack-reused 18
Submodule path 'js': checked out 'c3c588713233609f5bbbb2d9e7f3fb4a660f3f72'

C:\code\test>dir
 Volume in drive C has no label.
 Volume Serial Number is 047C-45B9

 Directory of C:\code\test

04/23/2018  10:07 PM    <DIR>          .
04/23/2018  10:07 PM    <DIR>          ..
04/23/2018  10:07 PM    <DIR>          example-dependency
               0 File(s)              0 bytes
               3 Dir(s)  92,314,771,456 bytes free

C:\code\test>v:

V:\test>git clone https://github.com/githubtraining/example-dependency --recursive example-dependency2
Cloning into 'example-dependency2'...
remote: Counting objects: 39, done.
remote: Total 39 (delta 0), reused 0 (delta 0), pack-reused 39
Unpacking objects: 100% (39/39), done.
Submodule 'js' (https://github.com/githubtraining/example-submodule.git) registered for path 'js'
Cloning into 'V:/test/example-dependency2/js'...
remote: Counting objects: 18, done.
remote: Total 18 (delta 0), reused 0 (delta 0), pack-reused 18
Submodule path 'js': checked out 'c3c588713233609f5bbbb2d9e7f3fb4a660f3f72'

V:\test>dir
 Volume in drive V is 1TB
 Volume Serial Number is 8A15-5E5F

 Directory of V:\test

04/23/2018  10:09 PM    <DIR>          .
04/23/2018  10:09 PM    <DIR>          ..
04/23/2018  10:07 PM    <DIR>          example-dependency
04/23/2018  10:09 PM    <DIR>          example-dependency2
               0 File(s)              0 bytes
               4 Dir(s)  92,281,122,816 bytes free

V:\test>

This will come in handy in the next commit.

Signed-off-by: JiSeop Moon <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
@dscho
Copy link
Member

dscho commented Apr 23, 2018

I fear that the code in this patch disables all symbolic links, not just bogus ones that correspond to Docker volumes.

To fix that, I added that parameter to the signature of file_attr_to_st_mode() as I hinted earlier, and I also used the opportunity to split the patch into nice, modular commits, then pushed the result out as branch support-windows-container to https://github.com/dscho/git.

Would you mind testing that branch?

ZCube and others added 3 commits April 24, 2018 14:56
It is a known issue that a rename() can fail with an "Access denied"
error at times, when copying followed by deleting the original file
works. Let's just fall back to that behavior.

Signed-off-by: JiSeop Moon <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
In preparation for making this function a bit more complicated (to allow
for special-casing the `ContainerMappedDirectories` in Windows
containers, which look like a symbolic link, but are not), let's move it
out of the header.

Signed-off-by: JiSeop Moon <[email protected]>
Signed-off-by: Johannes Schindelin <[email protected]>
... even if they may look like them.

As looking up the target of the "symbolic link" (just to see whether it
starts with `/ContainerMappedDirectories/`) is pretty expensive, we
do it when we can be *really* sure that there is a possibility that this
might be the case.

Signed-off-by: Johannes Schindelin <[email protected]>
Signed-off-by: JiSeop Moon <[email protected]>
@ZCube
Copy link
Author

ZCube commented Apr 24, 2018

It works with some modifications.

https://github.com/dscho/git/blob/46b62580eae7812041ddd93249c566a46b2acc47/compat/mingw.c#L2485

  • mingw_unlink->!mingw_unlink

https://github.com/dscho/git/blob/46b62580eae7812041ddd93249c566a46b2acc47/compat/mingw.c#L3602

  • \\?\ContainerMappedDirectories\-> /ContainerMappedDirectories/

Are there any additional considerations when copying files?

@dscho
Copy link
Member

dscho commented Apr 24, 2018

@ZCube nice! So: can you make those modifications, squash them into the appropriate commits (using git commit --fixup <commit> and eventually git rebase -i --autosquash <git-for-windows/master>), then force-push the result to your master (which updates this PR automatically)?

Are there any additional considerations when copying files?

I do not think so. In any case, I think we can run with this for now, and if more problems raise their heads, we can try to fix them as they arise.

@ZCube
Copy link
Author

ZCube commented Apr 24, 2018 via email

@dscho dscho added this to the v2.17.0(2) milestone Apr 24, 2018
@dscho
Copy link
Member

dscho commented Apr 24, 2018

I already did it. :)

Excellent!

git rebase is too difficult for me...

From what I can see, you did it brilliantly.

Thank you so much for your contribution!

@dscho dscho merged commit e6ee2f7 into git-for-windows:master Apr 24, 2018
dscho added a commit to git-for-windows/build-extra that referenced this pull request Apr 25, 2018
Git was [fixed](git-for-windows/git#1645)
to work correctly in Docker volumes inside Windows containers.

Signed-off-by: Johannes Schindelin <[email protected]>
dscho added a commit that referenced this pull request May 29, 2018
Support windows container.
dscho added a commit that referenced this pull request May 29, 2018
Support windows container.
PKRoma pushed a commit to PKRoma/git-for-windows that referenced this pull request Jun 22, 2018
PKRoma pushed a commit to PKRoma/git-for-windows that referenced this pull request Jun 22, 2018
PKRoma pushed a commit to PKRoma/git-for-windows that referenced this pull request Jun 22, 2018
dscho added a commit that referenced this pull request Aug 22, 2018
Support windows container.
dscho added a commit to dscho/git that referenced this pull request Aug 22, 2018
dscho added a commit that referenced this pull request Aug 23, 2018
Support windows container.
dscho added a commit to dscho/git that referenced this pull request Dec 30, 2024
This was pull request git-for-windows#1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
dscho added a commit to dscho/git that referenced this pull request Dec 30, 2024
This was pull request git-for-windows#1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
dscho added a commit to dscho/git that referenced this pull request Dec 30, 2024
This was pull request git-for-windows#1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
dscho added a commit to dscho/git that referenced this pull request Dec 30, 2024
This was pull request git-for-windows#1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci pushed a commit that referenced this pull request Dec 31, 2024
This was pull request #1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci pushed a commit that referenced this pull request Dec 31, 2024
This was pull request #1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci pushed a commit that referenced this pull request Jan 1, 2025
This was pull request #1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci pushed a commit that referenced this pull request Jan 1, 2025
This was pull request #1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci pushed a commit that referenced this pull request Jan 1, 2025
This was pull request #1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci pushed a commit that referenced this pull request Jan 1, 2025
This was pull request #1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci pushed a commit that referenced this pull request Jan 1, 2025
This was pull request #1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci pushed a commit that referenced this pull request Jan 2, 2025
This was pull request #1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci pushed a commit that referenced this pull request Jan 2, 2025
This was pull request #1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci pushed a commit that referenced this pull request Jan 2, 2025
This was pull request #1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci pushed a commit that referenced this pull request Jan 3, 2025
This was pull request #1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci pushed a commit that referenced this pull request Jan 6, 2025
This was pull request #1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci pushed a commit that referenced this pull request Jan 6, 2025
This was pull request #1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci pushed a commit that referenced this pull request Jan 7, 2025
This was pull request #1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci pushed a commit that referenced this pull request Jan 7, 2025
This was pull request #1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
dscho added a commit to dscho/git that referenced this pull request Jan 7, 2025
This was pull request git-for-windows#1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
dscho added a commit to dscho/git that referenced this pull request Jan 7, 2025
This was pull request git-for-windows#1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci pushed a commit that referenced this pull request Jan 7, 2025
This was pull request #1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci pushed a commit that referenced this pull request Jan 7, 2025
This was pull request #1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci pushed a commit that referenced this pull request Jan 8, 2025
This was pull request #1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci pushed a commit that referenced this pull request Jan 8, 2025
This was pull request #1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci pushed a commit that referenced this pull request Jan 8, 2025
This was pull request #1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci pushed a commit that referenced this pull request Jan 8, 2025
This was pull request #1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci pushed a commit that referenced this pull request Jan 9, 2025
This was pull request #1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci pushed a commit that referenced this pull request Jan 9, 2025
This was pull request #1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
dscho added a commit that referenced this pull request Jan 17, 2025
This was pull request #1645 from ZCube/master

Support windows container.

Signed-off-by: Johannes Schindelin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants