microsoft / winget-pkgs

The Microsoft community Windows Package Manager manifest repository
MIT License
8.76k stars 4.58k forks source link

[Package Issue]: AutoHotkey.AutoHotkey asks for install location #103198

Open lackovic opened 1 year ago

lackovic commented 1 year ago

Please confirm these before moving forward

Category of the issue

Installation issue.

Brief description of your issue

When I run winget install --id AutoHotkey.AutoHotkey I am asked to specify an installation root (3 times).

Steps to reproduce

Run:

winget install --id AutoHotkey.AutoHotkey

Actual behavior

> winget install --id AutoHotkey.AutoHotkey
Found AutoHotkey [AutoHotkey.AutoHotkey] Version 2.0.2
This application is licensed to you by its owner.
Microsoft is not responsible for, nor does it grant any licenses to, third-party packages.
This package requires an install location
Specify the install root:
Specify the install root:
Specify the install root:

Expected behavior

The application is installed like any other, without asking for an install location.

Environment

> winget --info
Windows Package Manager v1.4.10173
Copyright (c) Microsoft Corporation. All rights reserved.

Windows: Windows.Desktop v10.0.22621.1555
System Architecture: X64
Package: Microsoft.DesktopAppInstaller v1.19.10173.0

Logs: %LOCALAPPDATA%\Packages\Microsoft.DesktopAppInstaller_8wekyb3d8bbwe\LocalState\DiagOutputDir

User Settings: %LOCALAPPDATA%\Packages\Microsoft.DesktopAppInstaller_8wekyb3d8bbwe\LocalState\settings.json

Links
---------------------------------------------------------------------------
Privacy Statement   https://aka.ms/winget-privacy
License Agreement   https://aka.ms/winget-license
Third Party Notices https://aka.ms/winget-3rdPartyNotice
Homepage            https://aka.ms/winget
Windows Store Terms https://www.microsoft.com/en-us/storedocs/terms-of-sale

Screenshots and Logs

WinGet-2023-04-23-00-22-08.868.log

Trenly commented 1 year ago

This is intentional behavior, as the package requires the install location in order to run properly. If the install location is not specified, the installer will install to the root directory of the executable, which happens to be inside the TEMP folder when installed with winget, and so would be deleted after some time

lackovic commented 1 year ago

Would it be possible to make the installer to install by default to %PROGRAMFILES% instead of %TEMP%? Is this a question for the package maintainer?

The prompt Specify the install root: does not accept %PROGRAMFILES% as input.

When I entered C:\Program Files it installed the application inside that folder, instead of installing it to a AutoHotkey subfolder. The word "root" in the prompt is misleading, the prompt should say Specify the install directory:.

Trenly commented 1 year ago

Would it be possible to make the installer to install by default to %PROGRAMFILES% instead of %TEMP%? Is this a question for the package maintainer?

This isn't something winget can change. You would have to reach out to the developers of AutoHotkey to ask them to change the default install location

The prompt Specify the install root: does not accept %PROGRAMFILES% as input.

Sounds like a great feature request for the CLI

When I entered C:\Program Files it installed the application inside that folder, instead of installing it to a AutoHotkey subfolder. The word "root" in the prompt is misleading, the prompt should say Specify the install directory:.

That might be a bug in the CLI, or it might be with the installer. I'll do some more digging to figure that one out

BrandonWanHuanSheng commented 1 year ago

It does not accept /AllUser /CurrentUser. That silly installer created an installer.ahk.

asolopovas commented 1 year ago

there has to be default value rather than prompt, just chose the best option and if somebody is not happy with it they can always change it with parameters.

Trenly commented 1 year ago

there has to be default value rather than prompt, just chose the best option and if somebody is not happy with it they can always change it with parameters.

Unfortunately the default value is set by the publisher. If winget uses its own default value, users would never be able to use the --location switch to override it

asolopovas commented 1 year ago

Unfortunately the default value is set by the publisher. If winget uses its own default value, users would never be able to use the --location switch to override it

don't you think that tool owner defines how publish such packages? If there will be requirement to specify default path of installation the publishers will obey.

Trenly commented 1 year ago

Unfortunately the default value is set by the publisher. If winget uses its own default value, users would never be able to use the --location switch to override it

don't you think that tool owner defines how publish such packages? If there will be requirement to specify default path of installation the publishers will obey.

Thats exactly what the problem is. They have specified a default path, it's just that the default path happens to be relative to where the installer is located

cheater commented 1 year ago

Sorry, this is clearly a packaging issue. It is possible for the user to manually specify that the thing should be installed in Program Files. AHK should provide that exact path as the install path, and if another path is passed, then that path should be passed to the installer instead. If winget doesn't have the ability to do that, this functionality needs to be added. Leaving things as it is, or requiring the upstream maintainer of Autohotkey to change things, is not acceptable for a packaging software that needs to be able to package all sorts of programs. This is one of the behaviors programs will exhibit, and winget has to be able to cope with it in a way that the installation UX is the same for this package as for every other package.

cheater commented 1 year ago

If by default a program installs into the same directory the installation package is located in, then it is clearly nonsensical to use that behavior in a situation where the installation package is put in a programmatically generated random temporary directory. No user wants this, no user would choose to do it. Therefore in such cases a path should be specified by winget at all times, and if the user doesn't use a command line argument to specify a path, it should be %PROGRAMFILES%.

It doesn't matter one bit what the msi/exe/etc wants to do in its own internal logic. None of that is relevant, because users need to be presented with a unified UX that makes sense and doesn't have stupid exceptions like Autohotkey.Autohotkey is right now.

Trenly commented 1 year ago

Sorry, this is clearly a packaging issue. It is possible for the user to manually specify that the thing should be installed in Program Files. AHK should provide that exact path as the install path, and if another path is passed, then that path should be passed to the installer instead. If winget doesn't have the ability to do that, this functionality needs to be added. Leaving things as it is, or requiring the upstream maintainer of Autohotkey to change things, is not acceptable for a packaging software that needs to be able to package all sorts of programs. This is one of the behaviors programs will exhibit, and winget has to be able to cope with it in a way that the installation UX is the same for this package as for every other package.

But the thing is, winget is not a packaging tool, it is a package manager. Winget doesn’t change how an .exe file or .msi installer works, it just runs the file with the appropriate command line arguments for an install.

If you were to go out and download AutoHotkey's exe file, open up a command line, and run the exe file using the silent switch, AutoHotkey wouldn't install to ProgramFiles, it would install to the directory the installer was in unless the location argument was provided. Effectively, that is all winget does - download the package and run it with the silent installl argument. It's not wingets choice that it installs to the same location as the installer, it is the choice of the creator of AutoHotkey.

If by default a program installs into the same directory the installation package is located in, then it is clearly nonsensical to use that behavior in a situation where the installation package is put in a programmatically generated random temporary directory. No user wants this, no user would choose to do it. Therefore in such cases a path should be specified by winget at all times, and if the user doesn't use a command line argument to specify a path, it should be %PROGRAMFILES%.

The use of %PROGRAMFILES% assumes the user has write access to the shared folder, which requires administrator. I can agree that having a default value would make sense, but then again, there are users who believe that a default value is not proper and that they should be informed the location is required.

There is, however, a way for each user to choose to set a default value for the install location for packages that require a location to be specified by setting it in the winget settings file.

https://github.com/microsoft/winget-cli/blob/master/doc/Settings.md#default-install-root

Perhaps some recommendation should be added to the cli to make it clear this option exists. Something like "To bypass this prompt in the future, configure the defaultInstallRoot using winget settings"

It doesn't matter one bit what the msi/exe/etc wants to do in its own internal logic. None of that is relevant, because users need to be presented with a unified UX that makes sense and doesn't have stupid exceptions like Autohotkey.Autohotkey is right now.

There are always exceptions, and sometimes there are exceptions to exceptions. No solution is ever perfect, and the developers of winget have to deal with an enormous amount of non-standard installers, anti-patterns, and hack-jobs on top of all the other items like dependencies and side-by-side installations. The philosophy has always been to meet developers where they are, and to have a single UX cover everything is not a small task. I believe that the team has done a fairly good job at making the UX as seamless as possible - even in the cases where it doesn't "just work"

cheater commented 1 year ago

Effectively, that is all winget does

then winget needs to do more.

cheater commented 1 year ago

There is absolutely no technical reason why winget install -e --accept-package-agreements --id AutoHotkey.AutoHotkey shouldn't install into program files. All you're talking about is what people expect in your opinion and your arguments seem bad because you're only talking about "this one user who doesn't like the default" and other special cases like "the user is using winget but doesn't have admin rights". This is not the right place to focus on edge cases. This is where you focus on the UX of the 99% of users, not of the disagreeable or otherwise special 1%. This is an absolutely stupid reason to block this issue and it's just stopping people from installing Autohotkey. It's completely disappointing that it's been 2 weeks and people can't figure out that winget should, in essence, just do the thing. Just do the thing, stop thinking about stuff that barely ever happens, figure out how to handle those weird cases with extra input, and meanwhile make the standard invokation - the code I posted above - Just Do The Thing. The vastly most common case should not require extra interaction, extra switches, extra hidden configuration, or anything else. It's the special cases that should require that.

winget needs to have the ability to special case installers like this. Some syntax in a manifest or whatever, I don't think anyone cares how it's done, as long as the line of code I posted above installs AHK 2 into program files for a user who isn't in the 1% of not having admin access like nearly everyone does who'll be using winget.

R4ygen commented 1 year ago

Hello, I'm re-installing every software on my PC with Winget, and Autohotkey is the ONLY one that get installed wrongly in the TEMP folder or asks for the root installation directory.

I'm fairly sure this can be fixed and made it so it installs silently like any other software on Winget in the ProgramFiles folder. The need to specify a root installation directory should not be mandatory for a standard silent installation via CLI.

DarrenSchwartz commented 1 year ago

I have to agree the default install directory should be configured to Program Files. The user is free to override with --location but there should be a default to enable a silent install. That is the whole point of using a tool like this. So far, AutoHotkey is the only offender I have encountered.

asolopovas commented 1 year ago

Its pretty much pointless argument the solution can be done by either party Autohotkey or Winget its the question who will pick it up and fix it, but people tend to point fingers at each other rather than do something about the problem.

Bert-Proesmans commented 1 year ago

The latest v2 Autohotkey installer, which is also packaged on the winget index, is broken on system and user install. It complains about filepaths not existing during installation.
There is no way for me to create an issue at the AutoHotkey repository itself and I cannot link this issue, so I'm explicitly mentioning here.

An updated package (possibly after a solution to the default --location request) needs to be preceded by thoroughly testing the installer file itself.

EDIT; I'm talking about the installer itself. Download the installer manually from github and attempt to install it. That doesn't work on all of my systems in SYSTEM context, and some of them in USER context. I'll take this up to the forums next week with more details, as suggested by @cheater.

Log ```text Error: File or manifest not found Specifically: v2\AutoHotkey32_UIA.exe 226: xml.setProperty("SelectionNamespaces" , "xmlns:v1='urn:schemas-microsoft-com:asm.v1' " . "xmlns:v3='urn:schemas-microsoft-com:asm.v3'") 229: If !xml.load("res://" ExePath "/#24/#1") ▶ 230: Throw Error("File or manifest not found",, ExePath) 231: node := xml.selectSingleNode("/v1:assembly/v3:trustInfo/v3:security" . "/v3:requestedPrivileges/v3:requestedExecutionLevel") 233: If !node The current thread will exit. Call stack: *#1 (230) : [EnableUIAccess_SetManifest] Throw Error("File or manifest not found",, ExePath) *#1 (219) : [EnableUIAccess] EnableUIAccess_SetManifest(ExePath) *#1 (1066) : [Installation.Prototype.MakeUIA] EnableUIAccess(newPath) *#1 (500) : [Installation.Prototype.Apply] item(this) *#1 (560) : [Installation.Prototype.InstallFull] this.Apply() *#1 (393) : [Install_Main] inst.%method%(params*) *#1 (1168) : [] Install_Main() > Auto-execute ``` ![image](https://github.com/microsoft/winget-pkgs/assets/2027877/ae99ab49-210a-4db8-b36b-3a9902de72c0)
cheater commented 1 year ago

The AHK bug tracker is on their forum. I created an issue about this, and I'm waiting for clarification, but it looks like the AHK installer should be working, and winget might be screwing something up.

https://www.autohotkey.com/boards/viewtopic.php?p=520952#p520952

woodyinwoodley commented 1 year ago

I can only assume the designers of Winget have never used a package manager before. The ONLY ACCEPTABLE behaviour here is for winget to just install/upgrade the application. If the package doesn't behave like that, don't accept it into the winget repo. You are the gatekeepers. If I can't just run "winget upgrade -all --silent" on a schedule, then you have failed. I can do exactly that with apt, yum, brew, dnf...

cheater commented 1 year ago

as regrettable as that is, I have to agree. maybe not the "never used a package manager before" bit (but it's tempting) - maybe people just got lost in the weeds trying to make things work. thigns that look trivial from the outside can be loaded with massive amounts of mental baggage for someone working on the code. that said, woody's message is right: the only acceptable behavior is what he points out

BastionNtB commented 1 year ago

Throwing my two cents in here as well. Mainly just an agreement with the fact that all packages should be installed with defaults and, it is not expected that a prompt would be required for install of a standard package. If this was apt, it wouldn't be allowed. This also causes a problem when chaining multiple packages. If I specify to upgrade all packages, I then have to tell ALL packages to install with --location? But surely not every package goes in the same location... But now, because of that, I either have to run multiple commands, with separate locations, or just not use a tool that can be wonderful for Windows users.

Lexikos commented 1 year ago

AutoHotkey/AutoHotkeyUX@29ffb4e should fix this.

Does someone also need to change the winget package to not require an install location?

cheater commented 1 year ago

@Lexikos well it seems that the script was updated to use AHK 2.0.8, but after installing it using the instructions, now I receive the following error:

--------------------------- AutoHotkey_2.0.8_setup.exe ---------------------------
Unable to create AutoHotkey64. Try adding an exclusion in your antivirus software. If that doesn't work, please report the issue.  Error: (6) The handle is invalid.
--------------------------- Abort   Retry   Ignore    ---------------------------

No idea what to do here, what to set, where to set it, how to set it, or why I should be setting anything at all.

Clicking Retry creates the following error popup instead:

Error: Error loading manifest from v2\AutoHotkey64_UIA.exe

Specifically: (2) The system cannot find the file specified.
  @ *#1:257

    231: Throw Error("Invalid manifest")
    232: Catch as e
▶   233: Throw Error("Error loading manifest from " ExePath,, e.Message "
  @ " e.File ":" e.Line)
    234: node := xml.selectSingleNode("/v1:assembly/v3:trustInfo/v3:security" . "/v3:requestedPrivileges/v3:requestedExecutionLevel")
    236: If !node

The current thread will exit.

Call stack:
*#1 (233) : [EnableUIAccess_SetManifest] Throw Error("Error loading manifest from " ExePath,, e.Message "
  @ " e.File ":" e.Line)
*#1 (219) : [EnableUIAccess] EnableUIAccess_SetManifest(ExePath)
*#1 (1095) : [Installation.Prototype.MakeUIA] EnableUIAccess(newPath)
*#1 (527) : [Installation.Prototype.Apply] item(this)
*#1 (587) : [Installation.Prototype.InstallFull] this.Apply()
*#1 (412) : [Install_Main] inst.%method%(params*)
*#1 (1199) : [] Install_Main()
> Auto-execute

There's still crucial improvement to be had in the AHK+winget experience.