git clean deletes ts client

[Tratcher]

git clean -xdf keeps deleting src\SignalR\clients\ts\signalr and src\SignalR\clients\ts\signalr-protocol-msgpack

Repro steps:

• build /t:restore (runs npm install)
• dir /al /s (shows those folders as hard junctions from \src\SignalR\clients\ts\FunctionalTests\node_modules\@aspnet)
• git clean -xdf (those folders get deleted)
• git status (git shows those folders as deleted)

This happens because we've configured a junction between src\SignalR\clients\ts\FunctionalTests\node_modules\@aspnet\signalr and src\SignalR\clients\ts\signalr (how?) and node_modules/ is in the .gitignore file. When git clean clears the node_modules dir it follows the junction and deletes the client as well.

Proposal: can we populate the node_modules\@aspnet\signalr folder via some other mechanism besides using a junction? E.g. manually copy the files?

Open question: Where is this junction configured? Is it this?
https://github.com/aspnet/AspNetCore/blob/89a7f3cf774d46eaaea26861744a224a6487f31c/src/SignalR/clients/ts/tsconfig.base.json#L21-L28

@davidfowl @BrennanConroy @anurse

[BrennanConroy]

Oh nice! Thanks for getting a repro, we have occasionally seen this but never knew how it happened. I don't think the tsconfig has anything to do with creating a junction, it's supposed to tell the typescript compiler where to grab certain imports from.

…

________________________________ From: Chris Ross <notifications@github.com> Sent: Friday, January 18, 2019 9:50:46 AM To: aspnet/AspNetCore Cc: Brennan Conroy; Mention Subject: [aspnet/AspNetCore] git clean deletes ts client (#6845) git clean -xdf keeps deleting src\SignalR\clients\ts\signalr and src\SignalR\clients\ts\signalr-protocol-msgpack Repro steps: * build /t:restore (runs npm install) * dir /al /s (shows those folders as hard junctions from \src\SignalR\clients\ts\FunctionalTests\node_modules\@aspnet) * git clean -xdf (those folders get deleted) * git status (git shows those folders as deleted) This happens because we've configured a junction between src\SignalR\clients\ts\FunctionalTests\node_modules\@aspnet\signalr and src\SignalR\clients\ts\signalr (how?) and node_modules/ is in the .gitignore file. When git clean clears the node_modules dir it follows the junction and deletes the client as well. Proposal: can we populate the node_modules\@aspnet\signalr folder via some other mechanism besides using a junction? E.g. manually copy the files? Open question: Where is this junction configured? Is it this? https://github.com/aspnet/AspNetCore/blob/89a7f3cf774d46eaaea26861744a224a6487f31c/src/SignalR/clients/ts/tsconfig.base.json#L21-L28<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Faspnet%2FAspNetCore%2Fblob%2F89a7f3cf774d46eaaea26861744a224a6487f31c%2Fsrc%2FSignalR%2Fclients%2Fts%2Ftsconfig.base.json%23L21-L28&data=02%7C01%7Cbrecon%40microsoft.com%7Ce38e5fb1f15a4681ebe508d67d6d79ec%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636834306489314964&sdata=fG5nb04K88VCwF7U%2FYVcbw5T4jn13JdDbapFeLQ%2FCMM%3D&reserved=0> @davidfowl<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fdavidfowl&data=02%7C01%7Cbrecon%40microsoft.com%7Ce38e5fb1f15a4681ebe508d67d6d79ec%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636834306489314964&sdata=xvbblyWQtpsw58FZ1Gwxvr1UT%2BAQBkeqty5Bl9b6v5o%3D&reserved=0> @BrennanConroy<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FBrennanConroy&data=02%7C01%7Cbrecon%40microsoft.com%7Ce38e5fb1f15a4681ebe508d67d6d79ec%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636834306489324961&sdata=i1oUwBu%2FvbOb4p83oLSvFoAVVYYmMGZbwlsHRlwc%2FVw%3D&reserved=0> @anurse<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fanurse&data=02%7C01%7Cbrecon%40microsoft.com%7Ce38e5fb1f15a4681ebe508d67d6d79ec%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636834306489324961&sdata=lkOwL7WF5tOU%2F4c%2F4kMh6fY0MDz5buqRXgadLr0MTuE%3D&reserved=0> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Faspnet%2FAspNetCore%2Fissues%2F6845&data=02%7C01%7Cbrecon%40microsoft.com%7Ce38e5fb1f15a4681ebe508d67d6d79ec%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636834306489334949&sdata=crq2EJhjyDbrbZB241dSYa2aSvGoG7OaQAdq2CPLGtU%3D&reserved=0>, or mute the thread<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAHOVEcpipizuH6EkY4ZAZsMKkEIWVQJNks5vEgl2gaJpZM4aIXMx&data=02%7C01%7Cbrecon%40microsoft.com%7Ce38e5fb1f15a4681ebe508d67d6d79ec%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636834306489334949&sdata=iXaD8MmxpSUc9EjVm5u0kyDkKjAaHO1n7husM%2FNDYtc%3D&reserved=0>.

[natemcmaster]

https://github.com/aspnet/AspNetCore/blob/c2a0f894482895aa5af6e80dafa1e22724d5bd8f/src/SignalR/clients/ts/FunctionalTests/package.json#L7-L9

[natemcmaster]

Looks like a known issue in git. See git-for-windows/git#437 and https://www.bountysource.com/issues/29901518-git-clean-removes-content-of-symlinks

[muratg]

@BrennanConroy I think we should look into removing symbolic link usage :(

[analogrelay]

As @natemcmaster cited, the path references in package.json cause npm to create the link. This is a very common pattern for path-based references between node modules. If it causes problems with git for windows that's concerning. I'm not sure if there's a simple way to work around it.

[muratg]

@anurse I was thinking that we could do dependencies differently. Perhaps not possible, but I thought there could be a way. Multiple people keep hitting this on Windows unfortunately. It's easy to revert but it's definitely a nuisance.

[natemcmaster]

Does yarn have a solution for this?

[bradygaster]

@natemcmaster do you think a mitigation possibility could be that we move the TS client out of the mondo repo and begin the process of separating the non-.NET SignalR clients into their own repositories somewhere else? If that would mitigate here, we could start talking about doing it soon.

[natemcmaster]

The same problem would apply in a new repo too. The problem is a combination of the way npm on windows does “File:/” dependencies and a bug in git-for-windows. Moving to a new repo doesn’t solve either of those problems.

[Tratcher]

@natemcmaster but it does reduce the number of people affected by it, and if you're planning to separate the clients anyways...

[BrennanConroy]

Moving to a new repo solves the problem of everyone complaining. Only a couple people will hit the issue if we move it, and we can ignore it.

[muratg]

IMO, let's not jump to a "new repo" solution just yet. I'd rather have simplified build infra with less repos. Even if it means we can't solve this fully.