git-for-windows / git

A fork of Git containing Windows-specific patches.
http://gitforwindows.org/
Other
8.39k stars 2.55k forks source link

git clean -fdx deletes tracked files #1270

Closed kimbirkelund closed 5 years ago

kimbirkelund commented 7 years ago

Hi

I hope this is gonna sound as weird to you as it does to me.

The link below is a zip of a small git repository that I can reproduce the bug in on 4 machines. The machines were a mix of Win10, WinServer2012R2 and WinServer2016 (all fully updated).

Repo: https://www.dropbox.com/s/fz4d0i5ko7s7ktr/test.zip?dl=0

It contains 2 folders: helpers and b, each of which is an empty npm module. b\package.json refers to the helpers module.

The following reproduces the bug:

1) in terminal cd to the b folder 2) run npm install 3) run git reset HEAD --hard 4) run git clean -fdx

At this point both files in the helpers folder has been deleted and running git status confirms this.

Tool version:

git --version => git version 2.10.2.windows.1 and git version 2.14.1.windows.1 node -v => v6.11.2 npm -v => 5.3.0

I have no idea what is going. Very much hope you can explain :-)

dscho commented 7 years ago

Somebody on the mailing list suggested this may be a bug related to junction points. Can you confirm?

kimbirkelund commented 7 years ago

I don't see how. Unless npm messes with junction points internally.

On Tue, Aug 22, 2017, 14:13 Johannes Schindelin notifications@github.com wrote:

Somebody on the mailing list suggested this may be a bug related to junction points. Can you confirm?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/git-for-windows/git/issues/1270#issuecomment-324007641, or mute the thread https://github.com/notifications/unsubscribe-auth/ABy_cVCfuoN293yMCK1ZGsNInxqdJsBVks5sasYCgaJpZM4O50Yw .

--

/kim

dscho commented 7 years ago

I don't see how. Unless npm messes with junction points internally.

Since you have a setup where you can easily see the issue, it would be a pretty helpful thing to find out on your side if npm messes with junction points internally.

kimbirkelund commented 7 years ago

It apparently does mess with either junctions or symlinks.

When npm install is run in the b module, a link is made at b\node_modules\@sekoia\helpers to helpers.

When git clean is run it presumable deletes the inner most untracked files instead of the outer most untracked folder.

vidartf commented 7 years ago

Node's fs package (filesystem API used by npm) creates junctions on Windows. Note that this issue is potentially more severe than just deleting files in the repo:


REM Setup some directories for testing:
mkdir test-junctions-del
echo "content" > test-junctions-del\file.txt
dir test-junctions-del
mkdir my-git-repo
cd my-git-repo
git init

REM Create junction link:
mklink /J mylink ..\test-junctions-del

REM Blow it up:
git clean -xdf
dir ..\test-junctions-del

Or, if you are brave, point the junction to 'C:\'... (please don't).

vidartf commented 7 years ago

Seems to be a duplicate of #607.

vidartf commented 7 years ago

Here is the line in npm that creates a link, specifying the junction type for Windows.

vidartf commented 7 years ago

I have also opened a corresponding issue on the npm repo: https://github.com/npm/npm/issues/19091

dscho commented 6 years ago

I think this ticket may actually be a duplicate of #607, and we may soon have a fix via #1414.

dscho commented 5 years ago

I think this ticket may actually be a duplicate of #607, and we may soon have a fix via #1414.

Even if I have heard nothing to confirm this (it would have been nice, or at least a validation of my work, if I saw people at least test the stuff I produce, even better would be to help actively, of course), I will just assume that this was indeed fixed, even without confirmation, and close this ticket.

vidartf commented 5 years ago

@dscho I did install it since the pre-release and it has been working as intended. I meant to report back, but since it worked smoothly, it faded from memory. Thanks again!

vidartf commented 5 years ago

(For reference for others, the PR containing the fixes ended up being https://github.com/git-for-windows/git/pull/2268)