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 long paths for NTFS #1944

Merged
merged 1 commit into from
Oct 10, 2016
Merged

Support long paths for NTFS #1944

merged 1 commit into from
Oct 10, 2016

Conversation

johannesegger
Copy link
Contributor

@johannesegger johannesegger commented Oct 6, 2016

According to this blog post there are two ways to enable support for long paths in .NET applications for Windows 10: either target .NET 4.6.2 (1) or add some special app.config and app.manifest settings (2).

This PR implements (2).

app.config got formatted and this line is the important thing I added.

Since paket.res is a binary file and git doesn't provide a nice diff view, I'll try it here:

Before:
devenv_2016-10-06_22-10-33

After:
devenv_2016-10-06_22-07-52
I don't know why I had to put this under "RT_MANIFEST". I got this from powershell.exe.
Content of RT_MANIFEST/1 is copied from the blog post linked above:

<application xmlns="urn:schemas-microsoft-com:asm.v3">
  <windowsSettings>
    <longPathAware xmlns="http://schemas.microsoft.com/SMI/2016/WindowsSettings">true</longPathAware>
  </windowsSettings>
</application>

I couldn't figure out a nice way to test this so I only tried it locally.
Do you think paket.bootstrapper should have this as well?

@0x53A
Copy link
Contributor

0x53A commented Oct 6, 2016

I think you can use a xml manifest and modify the .fsproj

see http://stackoverflow.com/questions/26161113/how-do-i-embed-application-manifest-in-f-application

@johannesegger
Copy link
Contributor Author

You mean an app.manifest? I tried that but it seems you can't have both a .res file and a .manifest file (see here - I know this is for C# but I got the same error from the F# compiler).

@0x53A
Copy link
Contributor

0x53A commented Oct 7, 2016

ah, I think you are correct - nevermind then. I just thought a xml manifest was a bit easier to modify than some binary res file.

@johannesegger
Copy link
Contributor Author

I thought so too... until I got that compiler error :-)

@forki
Copy link
Member

forki commented Oct 10, 2016

Have to revert that. #1955 (comment)

It seems to break people

@johannesegger
Copy link
Contributor Author

Hm, sry about that, I hoped it would just be ignored. I currently can't find a way around it unfortunately.

@0x53A 0x53A mentioned this pull request Jul 16, 2017
@matthid
Copy link
Member

matthid commented Jul 20, 2017

Question is are we able to set the app.config dynamically when we detect windows 10 (for example via bootstrapper)?

@0x53A
Copy link
Contributor

0x53A commented Jul 20, 2017

that wouldn't work when you call paket.exe directly. Or it would need to copy itself to temp, modify the app config, and relaunch it.

@0x53A
Copy link
Contributor

0x53A commented Jul 20, 2017

Our buildservers don't have internet, so we just check in paket.exe directly

@FrankGagnon
Copy link

@forki Can we consider adding this change back into paket? Maybe add it into version 6 so the users on older systems would no be impacted? I would be hugely beneficial with the long .Net Standard packages names.

@forki
Copy link
Member

forki commented Apr 8, 2020

@FrankGagnon if some finds a way to make it non-breaking?!

@FrankGagnon
Copy link

@forki wouldn't it be fair to include a breaking change in a major version? Version 6 is supposed to be aimed at full .Net Core/Standard compatibility, I believe supporting long paths is a very important feature in achieving this goal.

As far as the breaking change itself, sadly the user that reported it didn't mention his environment, it would have been necessary to diagnose the issue and know why it would be breaking. I believe this is well supported on all non-deprecated versions of Windows.

@forki
Copy link
Member

forki commented Apr 9, 2020 via email

@FrankGagnon
Copy link

Do we know there's no work around? Two things could have failed for the user, the app.config setting or the manifest setting. It could me made so that the app.config would be downloaded in an "opt-in" fashion if it's a problem. If the manifest is a problem then it's trickier, I agree.

Don't you think it's worth giving it another shot and at least trying the fix the issues if any arise?

@0x53A
Copy link
Contributor

0x53A commented Apr 9, 2020

The manifest is the issue.

A manifested app fails on systems which don't support this entry.

@FrankGagnon
Copy link

Ok thanks for the insight, what are those systems?

@0x53A
Copy link
Contributor

0x53A commented Apr 9, 2020

Afaik anything pre Windows 10 Anniversary update (1607).

This is one place where it would be neat to have telemetry for paket usage.

@cartermp , do you have usage statistics how many Visual Studio users are still on old versions you can publicly share?

@FrankGagnon
Copy link

Understood, I see it in the Doc here https://docs.microsoft.com/en-us/windows/win32/sbscs/application-manifests#longpathaware

So would introducing this as a breaking change be acceptable?

If not, would it be possible to build 2 versions of paket.exe, one with the manifest and the other without it and add an option to paket.bootstrapper.exe to download the one with the manifest? This would be a way to make this change retro-compatible.

@0x53A
Copy link
Contributor

0x53A commented Apr 9, 2020

Yes, two exes with different manifests would probably work.

I think (but not 100% sure) that it is also possible to place the manifest next to the app (appname.manifest), if the app itself does not embed one.

@FrankGagnon
Copy link

From what I gather here, the Application Manifest has to be included as a resource in the assembly.

Would it be a big effort to have the 2 exes with different manifests (and one with a config file)?

@0x53A
Copy link
Contributor

0x53A commented Apr 9, 2020

As usual, the microsoft documentation is not complete. Embedding it as a resource is the preferred solution, but you can also put it next to the app.

There are a few caveats, for example it seems to be cached for some time:

https://csi-windows.com/blog/all/27-csi-news-general/245-find-out-why-your-external-manifest-is-being-ignored

The Microsoft documentation on external manifests would lead one to conclude that as long as an EXE does not have an embedded manifest, an external manifest will be read and used instead.

The truth, however, is that manifests are cached in the Activation Context Cache.

The secret sauce here is that if you change the EXE’s modification date time stamp, the loader will look again for a manifest. This seems to work reliably every time.

@FrankGagnon
Copy link

Yes you're right it didn't work in my tests because I didn't update the exe's modification time stamps. Ok so the bootstrapper could have an option to support long paths which would do 3 things :

  1. Download the paket.exe.manifest
  2. Download the paket.exe.config
  3. Update the paket.exe modification timestamp

This seems like a pretty good solution to me!

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.

None yet

5 participants