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

Visual Studio Generator should probably prefer x64 tools when available #11307

Closed
zadjii-msft opened this issue Sep 22, 2021 · 10 comments · Fixed by #11326
Closed

Visual Studio Generator should probably prefer x64 tools when available #11307

zadjii-msft opened this issue Sep 22, 2021 · 10 comments · Fixed by #11326
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@zadjii-msft
Copy link
Member

From #7774

Sorry to bring back closed PR to life, but is there a way to setup it to run x64 prompt by default? I still have my custom profile which calls vcvars64.bat. It is nice addition, but well unless you want to build 32-bit binaries it is kind of useless :)

@kasper93 That's a good point! VS features 6 items in the start menu:
image

Sorry to bring back closed PR to life, but is there a way to setup it to run x64 prompt by default? I still have my custom profile which calls vcvars64.bat. It is nice addition, but well unless you want to build 32-bit binaries it is kind of useless :)

Let's file a new issue at this point to represent adding preferences to the profile generator like which architecture is prioritized. Thanks!

So, the way I found to do it is to edit the profile generated by this generator and add -DevCmdArguments "-arch=x64 -host_arch=x64" or something to the Enter-VsDevShell arguments, or add -arch ... directly to the vsdevcmd.cmd arguments.

<discuss>

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Sep 22, 2021
@zadjii-msft zadjii-msft added Area-Settings Issues related to settings and customizability, for console or terminal Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Discussion Something that requires a team discussion before we can proceed Product-Terminal The new Windows Terminal. labels Sep 22, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Sep 22, 2021
@zadjii-msft zadjii-msft added this to the Terminal v1.12 milestone Sep 22, 2021
@zadjii-msft
Copy link
Member Author

IMO adding 6 profiles per install vs 2 profiles per install seems a little heavy, and way more likely that I'm just gonna delete them all.

I'm gonna also summon @trippwill (as OP of the PR) and @heaths (as the literal vswhere expert) to this thread for discussion

@heaths
Copy link
Member

heaths commented Sep 23, 2021

I, too, was initial concerned about "too many" profiles but it's not hard to hide them. While I mostly agree with only showing x64 shells by default (when there are both), all those shells are designed for specific workloads where users may prefer one over another - even for different tasks.

As more and more possibilities are added (consider numerous WSL2 distros installed), what about some sort of grouping mechanism in WT? Discovery could attach some "group attribute" so they are put into a submenu together. In the case of VS, I'd recommend doing that per-version e.g. VS2019 vs. VS2022. Though, in that case, I imagine you'd need to make keyboard bindings explicit instead of purely-indexed based (or maybe that's possible now already).

@zadjii-msft
Copy link
Member Author

We've actually got a big-ol design for profile grouping in #1571 (there's a spec for that here:https://github.com/microsoft/terminal/blob/main/doc/specs/%231571%20-%20New%20Tab%20Menu%20Customization/%231571%20-%20New%20Tab%20Menu%20Customization.md). The original design was entirely outside of the list of profiles though. It simply provides a way for users to separately group their profiles, rather than allowing dynamic profile generators the ability to say "please group my profiles together".

Though, if I remember the discussion correctly, we did think it would be a cool idea for people to easily say "please put all the WSLs in this nested list". Maybe we should bump that spec with an addenda for something like "remainingProfiles but for a profile source"

@heaths
Copy link
Member

heaths commented Sep 23, 2021

something like "remainingProfiles but for a profile source"

Sounds like a good idea and is congruent with the concept I was thinking. Presumably people could still move profiles to another menu based on their stable guid property.

My main concern here is that deciding for the user what's relevant won't work for all users. Maybe in the meantime we can set "hidden": true by default for some of them e.g. x86 if x64 is available but still generate them.

@zadjii-msft
Copy link
Member Author

Maybe in the meantime we can set "hidden": true by default for some of them e.g. x86 if x64 is available but still generate them.

ryan-point

@heaths
Copy link
Member

heaths commented Sep 23, 2021

...and without nesting, one could also prioritize showing newer versions - even prereleases - over older versions. If we're only considering hiding other entries by default to clean them up (until WT supports nesting menus), then I see no harm in showing preferred entries, which one could assume that most users would prefer the latest version of what they installed, and the shell that supports the most targets and/or faster tools e.g. x64.

vswhere works similarly with the -latest flag, which is what many build pipelines use from what I've seen. This, in concept, would be similar to adding -prerelease which, for dev shells, I think makes sense. CIs and especially release pipelines might normally only want to stick with stable releases, but I doubt that's true for most devs who chose to install a prerelease.

@zadjii-msft
Copy link
Member Author

FWIW, the PowerShell Core generator does a similar thing where it prefers newer versions of pwsh.exe over older ones.

@heaths
Copy link
Member

heaths commented Sep 24, 2021

I'll take a stab at this with the idea of hiding some/most by default.

heaths added a commit to heaths/terminal that referenced this issue Sep 24, 2021
@ghost ghost added the In-PR This issue has a related PR label Sep 24, 2021
@kasper93
Copy link
Contributor

My main concern here is that deciding for the user what's relevant won't work for all users. Maybe in the meantime we can set "hidden": true by default for some of them e.g. x86 if x64 is available but still generate them.

Indeed I agree that polluting profile list with multiple profiles is not great and deciding for the user which ones to show/hide might not be possible reliably.

How about we provide automatic profile generation, but in fact hide them all by default? And let users enable easily whatever they need.

I'm kind of referring also to the #11326 PR. Well I have both VS2019 and VS2022 installed, but use mainly VS2019 so yeah, right of the gates I would need to switch it up.

Long story short I appreciate automatic generation of profiles (and future generation for each arch) it makes life easier to not do it manually. But those profiles should be imho opt-in whenever user wants to show them or not.

@heaths
Copy link
Member

heaths commented Sep 26, 2021

Based on telemetry, the vast majority of users only have a single version of VS installed, and the assumption is that most would want to use their newest installed version by default (they are also capable of targeting older versions in most cases). Why not provide the best experience automatically for the majority of users?

The PR shows the latest version of VS, and if VC is installed only shows the matching processor architecture by default. You can still toggle visibility on any discovered profiles.

@zadjii-msft zadjii-msft removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Discussion Something that requires a team discussion before we can proceed labels Sep 27, 2021
@ghost ghost closed this as completed in #11326 Sep 29, 2021
@ghost ghost removed the In-PR This issue has a related PR label Sep 29, 2021
ghost pushed a commit that referenced this issue Sep 29, 2021
## Summary of the Pull Request

Similar to `vswhere -latest`, show only the latest Visual Studio command prompts / developer PowerShell. This was tested by deleting the local package state and testing against fresh state with both VS2019 and VS2022 Preview installed, and indeed VS2022 Preview (both cmd and powershell) show. The other profiles were generated but hidden by default.

## References

Modification of PR #7774

## PR Checklist
* [x] Closes #11307
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [ ] Schema updated.
* [x] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

## Detailed Description of the Pull Request / Additional comments

The sort algorithm is the same basic algorithm I used in https://github.com/microsoft/vswhere. It sorts first by installation version with a secondary sort based on the install date in case the installation versions are the same.

## Validation Steps Performed

With both VS2019 and VS2022 Preview installed, I made sure the initial state was expected, and tried different combinations of hiding and unhiding generated entries, and restarted Terminal to make sure my settings "stuck".
@ghost ghost added the Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. label Sep 29, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants