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

Launcher msi bug fix #2302

Conversation

alekhyareddy28
Copy link
Contributor

Summary of the Pull Request

Changes made in the following PR-

  • Added the runtime folders to the MSI.
  • Modified the PowerLauncher.UI.csproj file so that the PowerLauncher.csproj can be published. This is to ensure that all the required dlls are produced. This was done by setting the configuration to x64.
  • Code for the folder plugin has been removed as it is no longer needed.

PR Checklist

Validation Steps Performed

  • Manually tested it.
  • Verified that all the folders are created in the C:/Program Files/PowerToys/modules/launcher folder.

Outstanding tasks

  • Right now, To be safe, I ensured that the runtime folders of all the architectures are copied. All of them are not required and we need to verify which ones are needed and which one's are not before removing them.

@alekhyareddy28 alekhyareddy28 added the Product-PowerToys Run Improved app launch PT Run (Win+R) Window label Apr 21, 2020
<Directory Id="Launcherwin10x64nativeFolder" Name="native">
<Directory Id="Launcherwin10x64releaseFolder" Name="release">
</Directory>
<Directory Id="Launcherwin10x64debugFolder" Name="debug">
Copy link
Contributor

Choose a reason for hiding this comment

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

@alekhyareddy28 @jyuwono. What was the error that was requiring debug. I think this is only for the sqllite interop, but I don't know what uses that. It doesn't make sense that we need debug assemblies, and I'm worried that this is indicative that our build is configured incorrectly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Description: A .NET Core application failed. Application: PowerLauncher.exe Path: C:\Program Files\PowerToys\modules\launcher\PowerLauncher.exe Message: Error: An assembly specified in the application dependencies manifest (PowerLauncher.deps.json) was
not found: package: 'Microsoft.VCRTForwarders.140', version: '1.0.2-rc' path: 'runtimes/win10-x64/native/debug/concrt140d_app.dll'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SQLite.Interop.dll is a type of DLL file associated with System.Data.SQLite.
Reference

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have the full context here, but please make sure that no partial debug options will eventually end up in master.

Copy link
Contributor

@ryanbodrug-microsoft ryanbodrug-microsoft Apr 22, 2020

Choose a reason for hiding this comment

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

I believe that assembly is brought in by the xaml islands. We don't reference it explicitly. I don't want to block this from getting in much longer, but can create a new issue to remove the debug and ios/linux runtimes. Ideally none of this is generated on build, but we have to investigate further. @enricogior does that work for you?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm seeing the same issue for adding in SettingsV2 to the MSI as well. It looks like the deps.json file references ARM, x64 and x86 versions of all the dependencies and debug and release versions of each of them. The program launches only if all these referenced dlls are present (even though the ARM one for example would not be used). This readme has more context on that file.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ryanbodrug-microsoft sure that sounds good.

Copy link
Contributor

@ryanbodrug-microsoft ryanbodrug-microsoft left a comment

Choose a reason for hiding this comment

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

This works, and I'd like to get the ability for keyboard remapper changes and launcher changes to be in the same branch and msi. Lets create a cleanup task to make sure we aren't bloating the installer unecessarily.

@alekhyareddy28
Copy link
Contributor Author

@arjunbalgovind and I are working on publishing to a specific architecture and there is also an option to publish only one file. That way we don't have to do set all the dlls (hopefully). I'll merge this in if we're still not able to figure out a better alternative in a few mins.

@alekhyareddy28
Copy link
Contributor Author

alekhyareddy28 commented Apr 23, 2020

Closing this PR as publishing the PowerLauncher proj by setting the target framework to win-x64 would mitigate having debug and runtime dlls. New PR - #2355

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Product-PowerToys Run Improved app launch PT Run (Win+R) Window
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants