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

Handle DLLs for Symlinked executables #2711

Closed
caiohamamura opened this issue Nov 19, 2022 · 20 comments
Closed

Handle DLLs for Symlinked executables #2711

caiohamamura opened this issue Nov 19, 2022 · 20 comments
Assignees
Labels
Issue-Feature This is a feature request for the Windows Package Manager client. Portable Issue related to portable package Zipped-Binary Depends on .dlls that aren't available via symlink
Milestone

Comments

@caiohamamura
Copy link

caiohamamura commented Nov 19, 2022

Description of the new feature / enhancement

With current zip support, the only types of installer allowed are:

  1. Portable standalone exe files
  2. Compressed installers

But what about supporting regular compressed binary packages, such as those having exe along with dynamic dll libraries?

Proposed technical implementation details

To use the dynamic dll libraries, they must either be:

  1. in PATH environment variable
  2. linked by specifying Files -> RelativeFilePath within the installer manifest

It is not possible to link dll files to make it work using the 2 approach, because the links are forced to have .exe extension.

This could be fixed either by:

  1. Adding the dlls folder to PATH environment variable
  2. Allowing to link dll files specifying something like NestedInstallerType: library. Then it would also be useful to provide pattern matching RelativeFilePath, such as *.dll
  3. Instead of making a symlink inside WinGet\Links, make an executable bat with the command @call "path\to\exe" %*, then the working directory will be the same where the exe actually is (this would the easiest path in my opinion)
@caiohamamura caiohamamura added the Issue-Feature This is a feature request for the Windows Package Manager client. label Nov 19, 2022
@ghost ghost added the Needs-Triage Issue need to be triaged label Nov 19, 2022
@Trenly
Copy link
Contributor

Trenly commented Nov 21, 2022

iirc, the entire Zip folder is extracted and moved to the default install location. Take the Paint.Net Pull Request for example. If you comment out everything except for the zip-portable installer entry, it still installs and runs correctly, meaning that all the DLLs needed to run it are referenced. The DLLs aren’t needed in the installer files because they aren’t installers

@denelon denelon removed the Needs-Triage Issue need to be triaged label Nov 21, 2022
@caiohamamura
Copy link
Author

caiohamamura commented Nov 21, 2022

iirc, the entire Zip folder is extracted and moved to the default install location. Take the Paint.Net Pull Request for example. If you comment out everything except for the zip-portable installer entry, it still installs and runs correctly, meaning that all the DLLs needed to run it are referenced. The DLLs aren’t needed in the installer files because they aren’t installers

Maybe I wasnt' clear enough. By binary files I mean plain binary, not an exe installer, that's the case for Paint.Net. What I mean is plain binary packages, which is just a bunch of exe files along with dll files, many development tools are deployed this way such as nginx, php, etc. And they won't run unless:

  1. They are launched from the same directory where they were extracted, which a symlink will break
  2. They are added to PATH
  3. They are launched through a .bat file as I proposed: this strategy is used throughout many different packages.

If the community agrees with the bat implementation I would gladly like to contribute. I think it is much better than polluting the PATH or forcing the manifest to declare a whole bunch of DLL files which could also drive other side effects such as DLL versions conflicts.

@Trenly
Copy link
Contributor

Trenly commented Nov 21, 2022

iirc, the entire Zip folder is extracted and moved to the default install location. Take the Paint.Net Pull Request for example. If you comment out everything except for the zip-portable installer entry, it still installs and runs correctly, meaning that all the DLLs needed to run it are referenced. The DLLs aren’t needed in the installer files because they aren’t installers

Maybe I wasnt' clear enough. By binary files I mean plain binary, not an exe installer, that's the case for https://github.com/microsoft/winget-pkgs/pull/88065/files. What I mean is plain binary packages, which is just a bunch of exe files along with dll files, many development tools are deployed this way such as nginx, php, etc. And they won't run unless:

  1. They are launched from the same directory where they were extracted, which a symlink will break
  2. They are added to PATH
  3. They are launched through a .bat file as I proposed: this strategy is used throughout many different packages.

If the community agrees with the bat implementation I would gladly like to contribute. I think it is much better than polluting the PATH or forcing the manifest to declare a whole bunch of DLL files which could also drive other side effects such as DLL versions conflicts.

I think you should look at the Paint.Net manifest and zip file more closely. Specifically, this entry -

- Architecture: x64
   NestedInstallerType: portable
   NestedInstallerFiles:
   - RelativeFilePath: paintdotnet.exe
     PortableCommandAlias: paint.net
   InstallerUrl: https://github.com/paintdotnet/release/releases/download/v4.3.12/paint.net.4.3.12.portable.x64.zip
   InstallerSha256: AF58C12B92BC759F8E38C8623E356C8B8A2534809C44058AB76055DFA15DE4F8

That specific entry is exactly what you described - a loose .exe file with a bunch of DLLs and other assets included.

If you comment out all the other entries in the manifest, and then use winget to install this specific entry, it will still run perfectly fine

@caiohamamura
Copy link
Author

caiohamamura commented Nov 21, 2022

@Trenly, it does work if you install it and launch it through run dialog (Win + R) paint.net.exe, but that's because it looks like it follows the symlink working directory somehow, but if you try to launch it through cmd or powershell it won't work at all!

Although this is fine for Desktop Apps, it is not the expected behavior for development tools and won't work for CLI apps.

But even for Desktop Apps you need to specify paint.net.exe which is not what I would expect to type in the run dialog, I usually just type paint.net, code, etc. I do not type the .exe, I think this is another issue with the SymLinking strategy.

@Trenly
Copy link
Contributor

Trenly commented Nov 21, 2022

@Trenly, it does work if you install it and launch it through run dialog (Win + R) paint.net.exe, but that's because somehow it looks like it follows the symlink working directory somehow, but if you try to launch it through cmd or powershell it won't work at all!

Although this is fine for Desktop Apps, it is not the expected behavior for development tools and won't work for CLI apps.

But even for Desktop Apps you need to specify paint.net.exe which is not what I would expect to type in the run dialog, I usually just type paint.net, code, etc. I do not type the .exe, I think this is another issue with the SymLinking strategy.

It works fine for me from powershell when I use start paint.net

@caiohamamura
Copy link
Author

caiohamamura commented Nov 21, 2022

That's my point, start shouldn't be needed at all, you won't be running development tools using start by default, besides you probably won't be launching these development tools yourself but they will be launched by other development tools which you don't have control over.

@Trenly
Copy link
Contributor

Trenly commented Nov 21, 2022

Ahhh, I see. Thank you for your patience and the clarification!

@sitiom
Copy link

sitiom commented Jan 28, 2023

The issue title should be better renamed to something like "Handle DLLs for symlinked executables."

This could be fixed either by:

  1. Adding the dlls folder to PATH environment variable
  2. Allowing to link dll files specifying something like NestedInstallerType: library. Then it would also be useful to provide pattern matching RelativeFilePath, such as *.dll
  3. Instead of making a symlink inside WinGet\Links, make an executable bat with the command @call "path\to\exe" %*, then the working directory will be the same where the exe actually is (this would the most easier path in my opinion)

Another solution would be to use shims, like Scoop or Chocolatey:

@ddspj23
Copy link

ddspj23 commented Feb 18, 2023

iirc, the entire Zip folder is extracted and moved to the default install location. Take the Paint.Net Pull Request for example. If you comment out everything except for the zip-portable installer entry, it still installs and runs correctly, meaning that all the DLLs needed to run it are referenced. The DLLs aren’t needed in the installer files because they aren’t installers

Maybe I wasnt' clear enough. By binary files I mean plain binary, not an exe installer, that's the case for Paint.Net. What I mean is plain binary packages, which is just a bunch of exe files along with dll files, many development tools are deployed this way such as nginx, php, etc. And they won't run unless:

  1. They are launched from the same directory where they were extracted, which a symlink will break
  2. They are added to PATH
  3. They are launched through a .bat file as I proposed: this strategy is used throughout many different packages.

If the community agrees with the bat implementation I would gladly like to contribute. I think it is much better than polluting the PATH or forcing the manifest to declare a whole bunch of DLL files which could also drive other side effects such as DLL versions conflicts.

Use bat might cause problem while execute it in another bat script.

@denelon
Copy link
Contributor

denelon commented Oct 9, 2024

I believe we've resolved this with:

It would likely involve uninstalling the package and reinstalling it (if it's installed and not working), but this should unblock a fairly large number of packages in the community repository.

@denelon denelon closed this as completed Oct 9, 2024
@felipecrs
Copy link
Contributor

Should be worth mentioning that existing packages affected by this issue like scrcpy needs to have their manifests patched with this new option, and only then reinstalling them with the new winget version should work.

@MrHinsh
Copy link

MrHinsh commented Oct 10, 2024

@denelon do you have an example of where specifically to add ArchiveBinariesDependOnPath and i'll go patch my packages today?

never mind... I trawled the code and found:

PackageIdentifier: AppInstallerTest.ArchivePortableWithBinariesDependentOnPath
PackageVersion: 1.0.0.0
PackageName: TestArchivePortableWithBinariesDependentOnPath
PackageLocale: en-US
Publisher: AppInstallerTest
License: Test
ShortDescription: E2E test for installing a zip containing a portable with binaries that depend on the PATH variable.
Installers:
  - Architecture: x64
    InstallerUrl: https://localhost:5001/TestKit/AppInstallerTestZipInstaller/AppInstallerTestZipInstaller.zip
    InstallerType: zip
    InstallerSha256: <ZIPHASH>
    NestedInstallerType: portable
    NestedInstallerFiles:
      - RelativeFilePath: AppInstallerTestExeInstaller.exe
        PortableCommandAlias: TestPortable
    ArchiveBinariesDependOnPath: true
ManifestType: singleton
ManifestVersion: 1.9.0

Where is ManifestVersion 1.9.0 documented?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Feature This is a feature request for the Windows Package Manager client. Portable Issue related to portable package Zipped-Binary Depends on .dlls that aren't available via symlink
Projects
Status: Released
Development

No branches or pull requests