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

Added an option to select architecture to install. #1666

Merged
merged 2 commits into from
Nov 5, 2021

Conversation

jedieaston
Copy link
Contributor

@jedieaston jedieaston commented Nov 2, 2021


Related to #906.

This PR adds a option, --architecture, to winget install. This option allows the user to (you guessed it) tell winget what architecture they want to install. If the architecture isn't available, winget does the same thing it would do if there wasn't a installer entry for the user's platform (No applicable installer error). The user is only allowed to provide architectures their system can run (if they ask for arm64 on x64, a CommandException is thrown).

This may have implications for upgrades (I'm not 100% sure on whether upgrade checks the architecture of the currently installed version. If it doesn't, winget might upgrade a 32-bit version to a 64-bit version even if the user didn't want that). If it breaks too much stuff I can close this.

Tested: manually. If I need to write unit tests I can.

image

Microsoft Reviewers: Open in CodeFlow

@jedieaston jedieaston requested a review from a team as a code owner November 2, 2021 17:13
@ghost ghost added the Issue-Feature This is a feature request for the Windows Package Manager client. label Nov 2, 2021
@jedieaston jedieaston changed the title Btw i use arch Added an option to select architecture to install. Nov 2, 2021
@jedieaston jedieaston changed the title Added an option to select architecture to install. Added an option to select architecture to install Nov 2, 2021
@jedieaston jedieaston changed the title Added an option to select architecture to install Added an option to select architecture to install Nov 2, 2021
@jedieaston jedieaston changed the title Added an option to select architecture to install Added an option to select architecture to install. Nov 2, 2021
@denelon
Copy link
Contributor

denelon commented Nov 2, 2021

Issue #906 was intended to include adding this to settings so a user wouldn't necessarily need to specify on the command line. I wouldn't close that issue until we have a setting (preference and requirement) for architecture. We also may have some pain if we run into a situation where we have an x64 package that requires an x86 dependency and the user has a setting to require only x64 architecture.

@Masamune3210
Copy link

Absolutely love the original title

@jedieaston
Copy link
Contributor Author

jedieaston commented Nov 2, 2021

Issue #906 was intended to include adding this to settings so a user wouldn't necessarily need to specify on the command line.

My memory was dim, I thought that issue was purely for adding the argument. Unlinked, sorry for the trouble.

Absolutely love the original title

TIL that GitHub uses the branch name to name a PR if you didn't specify a title 😄

@JohnMcPMS
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JohnMcPMS
Copy link
Member

This may have implications for upgrades (I'm not 100% sure on whether upgrade checks the architecture of the currently installed version. If it doesn't, winget might upgrade a 32-bit version to a 64-bit version even if the user didn't want that).

I wanted upgrade to take that into account, but is difficult to determine the true architecture of an installed package based on ARP data alone. This is actually something we should store in the tracking catalog that I've recently added; the preference/requirement and the architecture itself. That way we can at try to honor the original request during upgrade. I don't expect you to do that as part of this change though.

@denelon, are you ok with taking just the command line argument, or do you want the settings implemented as well so as to have a more complete feature?

@denelon
Copy link
Contributor

denelon commented Nov 4, 2021

I think this is a great enhancement al by itself. I'd suggest accepting the feature as implemented, and we will keep the feature open until we build the settings and integrate that with the other work being done.

@jedieaston you rock! This is a fantastic enhancement, and I'm sure will be very well received. I've noticed the ask for 64bit adobe in another manifest. I love being able to eliminate the x64 and x86 variants of the "same" package. We still need to think about what this means in terms of an upgrade when the preferred architecture isn't available in the latest version, but that is a solvable problem.

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants