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

Introduce a Universal package for Windows Terminal #3236

Merged
merged 28 commits into from Nov 26, 2019
Merged

Conversation

miniksa
Copy link
Member

@miniksa miniksa commented Oct 17, 2019

This PR creates a Universal entrypoint for the Windows Terminal solution in search of our goals to run everywhere, on all Windows platforms.

The Universal entrypoint is relatively straightforward and mostly just invokes the App without any of the other islands and win32 boilerplate required for the centennial route. The Universal project is also its own packaging project all in one and will emit a relevant APPX.

A few things were required to make this work correctly:

  • Vcxitems reuse of resources (and link instructions on all of them for proper pkg layout)
  • Move all Terminal project CRT usages to the app ones (and ensure forwarders are only Nugetted to the Centennial package to not pollute the Universal one)
  • Fix/delay dependencies in TerminalApp that are not available in the core platform (or don't have an appropriate existing platform forwarder... do a loader snaps check)
  • vcpkg needs updating for the Azure connection parser
  • font fallbacks because Consolas isn't necessarily there
  • fallbacks because there are environments without a window handle

Some of those happened in other small PRs in the past week or two. They were relevant to this.

Note, this isn't useful as such yet. You can run the Terminal in this context and even get some of the shells to work. But they don't do a whole lot yet. Scoping which shells appear in the profiles list and only offering those that contextually make sense is future work.

@miniksa miniksa added the Product-Terminal The new Windows Terminal. label Oct 17, 2019
@miniksa
Copy link
Member Author

miniksa commented Oct 17, 2019

@ocalvo, did I break it apart to your satisfaction here?

<Link>defaults.json</Link>
</Content>
</ItemGroup>
<!-- Reprint all content as Nones for Universal project type to include it in the resources -->
Copy link
Member Author

Choose a reason for hiding this comment

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

@DHowett-MSFT, this reprint as None and the DeploymentContent on every item are killing me, but it's the only way I could make it work. If you know some MSBuild shorthand to clean this up, we can do that before submit (as long as both projects still play nice with it.)

@ocalvo
Copy link
Contributor

ocalvo commented Nov 12, 2019 via email

@miniksa
Copy link
Member Author

miniksa commented Nov 12, 2019

@ocalvo, that'll make it smaller for the top stuff, thanks!

But the Universal project still won't accept a Content, only a None/Image/etc as something to pack into the package. (Where as the Centennial project only accepts the inverse: it will only pack a Content and not a None/Image/etc).

@ocalvo
Copy link
Contributor

ocalvo commented Nov 12, 2019

@ocalvo, that'll make it smaller for the top stuff, thanks!

But the Universal project still won't accept a Content, only a None/Image/etc as something to pack into the package. (Where as the Centennial project only accepts the inverse: it will only pack a Content and not a None/Image/etc).

We have resolved this issue for Xaml Islands and WAP project, please use this targets file in your main EXE project.

@miniksa miniksa marked this pull request as ready for review November 16, 2019 00:04
src/cascadia/CascadiaResources.build.items Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/AppLogic.h Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/TerminalApp.vcxproj Outdated Show resolved Hide resolved
src/cascadia/TerminalApp/lib/TerminalAppLib.vcxproj Outdated Show resolved Hide resolved
src/renderer/dx/DxRenderer.cpp Show resolved Hide resolved
@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Nov 16, 2019
@ghost ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Nov 20, 2019
Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

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

What's up with the move to Microsoft.Toolkit.Win32.UI.XamlApplication.6.0.0 (from the preview version)? I thought there was a reason we were using that preview build (though I don't remember it now)

@miniksa
Copy link
Member Author

miniksa commented Nov 20, 2019

What's up with the move to Microsoft.Toolkit.Win32.UI.XamlApplication.6.0.0 (from the preview version)? I thought there was a reason we were using that preview build (though I don't remember it now)

6.0.0 is now the release build from last week. I talked to Oscar about it and there were some updates to how resources are packaged.

@zadjii-msft
Copy link
Member

@miniksa Ah okay, that makes sense. I assumed the minor version would have been bumped or something.

@miniksa miniksa changed the title Dev/miniksa/uwp Introduce a Universal package for Windows Terminal Nov 20, 2019
…age identity to match proposed license information.
Copy link
Contributor

@DHowett-MSFT DHowett-MSFT left a comment

Choose a reason for hiding this comment

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

Pop all that good good into the commit message and we're golden, G

@DHowett-MSFT DHowett-MSFT merged commit 2e9e4a5 into master Nov 26, 2019
@DHowett-MSFT DHowett-MSFT deleted the dev/miniksa/uwp branch November 26, 2019 00:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants