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

[Draft] Windows: Add longPathAware manifest file #1898

Closed
wants to merge 2 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions WinManifest.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0" xmlns:asmv3="urn:schemas-microsoft-com:asm.v3">
<!-- Uniquely identifiy the application (name + version) -->
<assemblyIdentity
type="win32"
name="BurntSushi.github.ripgrep"
version="13.0.0.0"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this required? I try not to put the version number in too many places. Right now, it's in exactly two places, and in the most recent release, I forgot to update one of them.

If it's unavoidable, then I think adding a step to RELEASE-CHECKLIST.md would be the right thing to do.

Copy link
Author

@ChrisDenton ChrisDenton Jun 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'm actually unsure here. The docs list it as "required" but excluding assemblyIdentity entirely still seems to work. I was just a bit nervous about breaking with the docs.

EDIT: In any case, I don't think the version actually has to be accurate or meaningful. It could just be set to 0.0.0.0

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go with 0.0.0.0 for now until someone complains with a good reason to change what we're doing. :-)

/>

<!-- Supported OS versions -->
<compatibility xmlns="urn:schemas-microsoft-com:compatibility.v1">
<application>
<!-- Windows 7 --><supportedOS Id="{35138b9a-5d96-4fbd-8e2d-a2440225f93a}"/>
<!-- Windows 8 --><supportedOS Id="{1f676c76-80e1-4239-95bb-83d0f6d0da78}"/>
<!-- Windows 10 --><supportedOS Id="{8e0f7a12-bfb3-4fe8-b9a5-48fd50a15a9a}"/>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a comment explaining how to update this stanza when a new version of Windows comes out?

</application>
</compatibility>

<!-- Application settings -->
<asmv3:application>
<asmv3:windowsSettings
xmlns:ws16="http://schemas.microsoft.com/SMI/2016/WindowsSettings"
xmlns:ws19="http://schemas.microsoft.com/SMI/2019/WindowsSettings"
>
<!-- Allows using longer paths without special handling -->
<!-- See: https://docs.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation?tabs=cmd#enable-long-paths-in-windows-10-version-1607-and-later -->
<ws16:longPathAware>true</ws16:longPathAware>
<ws19:activeCodePage>UTF-8</ws19:activeCodePage>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this have any harmful effects for versions of Windows older than Windows 10 v 1607?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case it will be unrecognised so it should be ignored. I will test on a Windows 7 VM to double check.

</asmv3:windowsSettings>
</asmv3:application>

<!-- Run the application at the same permission level as the process that started it -->
<asmv3:trustInfo>
<security>
<requestedPrivileges>
<requestedExecutionLevel level="asInvoker" uiAccess="false" />
</requestedPrivileges>
</security>
</asmv3:trustInfo>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this stanza necessary? What happens if we don't include it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, we can exclude it. The linker will add it back anyway.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great. General philosophy here is to just keep this file as small as possible so that it does the absolute minimum thing required.

</assembly>