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

resolve #771 #772

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

realchrisolin
Copy link

This PR resolves issue #771, which is ultimately the result of multiple independent issues related to building.

Because there are a couple different ways to resolve the issues I encountered, there's room for discussion on exactly how fixes should be implemented upstream. I'll try to reference specific issues and pertinent details in this PR and then update BUILDING.md in a subsequent commit to document/reflect the agreed on changes to what the build process should look like.

First and foremost are the changes proposed in WhitewaterFoundry/legacy-rootfs-build-scripts#1 -- getting pengwin-base built successfully first is critical. Some changes were needed so the script would continue to work as intended. As proposed, it is intended to work with the linux_files/setup file already in this repository instead of the one in the legacy-rootfs-build-scripts. Alternatively, perhaps one of the non-legacy scripts in https://github.com/WhitewaterFoundry/pengwin-rootfs-builds can referenced in build docs and used instead.

The next major issue is dependencies and the VS build environment. Current documentation recommends VS 2019, but platform tools v143 is not compatible/available on VS 2019. See the changes to .vcxproj files.

Past that, I encountered extensive NuGet dependency issues that were ultimately resolved by the changes made to build.bat to automatically restore dependencies. Other issues that were resolved without changes to specific scripts were where to modify the package thumbprint and exact steps related to trusting the signing certificate before the msixbundle is allowed to be installed.

Copy link
Collaborator

@crramirez crramirez left a comment

Choose a reason for hiding this comment

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

As the automated process to publish the program to the store uses Visual Studio 2022, it is preferable to update the BUILDING.md to use it.

@@ -30,27 +30,27 @@
<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'" Label="Configuration">
<ConfigurationType>Application</ConfigurationType>
<UseDebugLibraries>true</UseDebugLibraries>
<PlatformToolset>v143</PlatformToolset>
<PlatformToolset>v142</PlatformToolset>
Copy link
Collaborator

Choose a reason for hiding this comment

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

As the automated process to publish the program to the store uses Visual Studio 2022, it is preferable to update the BUILDING.md to use it.

Suggested change
<PlatformToolset>v142</PlatformToolset>
<PlatformToolset>v143</PlatformToolset>

Copy link
Author

Choose a reason for hiding this comment

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

Alright. I'll revert these changes in the next push to this PR.

I'm working on build instructions using VS 2022 and the non-legacy rootfs build repo, but I'm running into unrelated issues with debootstrap failing to successfully install sudo in the rootfs.

<CharacterSet>Unicode</CharacterSet>
</PropertyGroup>
<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|ARM64'" Label="Configuration">
<ConfigurationType>Application</ConfigurationType>
<UseDebugLibraries>true</UseDebugLibraries>
<PlatformToolset>v143</PlatformToolset>
<PlatformToolset>v142</PlatformToolset>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<PlatformToolset>v142</PlatformToolset>
<PlatformToolset>v143</PlatformToolset>

<CharacterSet>Unicode</CharacterSet>
<WindowsSDKDesktopARM64Support>true</WindowsSDKDesktopARM64Support>
</PropertyGroup>
<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'" Label="Configuration">
<ConfigurationType>Application</ConfigurationType>
<UseDebugLibraries>false</UseDebugLibraries>
<PlatformToolset>v143</PlatformToolset>
<PlatformToolset>v142</PlatformToolset>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<PlatformToolset>v142</PlatformToolset>
<PlatformToolset>v143</PlatformToolset>

<WholeProgramOptimization>true</WholeProgramOptimization>
<CharacterSet>Unicode</CharacterSet>
</PropertyGroup>
<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|ARM64'" Label="Configuration">
<ConfigurationType>Application</ConfigurationType>
<UseDebugLibraries>false</UseDebugLibraries>
<PlatformToolset>v143</PlatformToolset>
<PlatformToolset>v142</PlatformToolset>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<PlatformToolset>v142</PlatformToolset>
<PlatformToolset>v143</PlatformToolset>

@@ -16,7 +16,7 @@
<Keyword>Win32Proj</Keyword>
<WindowsTargetPlatformVersion>10.0</WindowsTargetPlatformVersion>
<ConfigurationType>Application</ConfigurationType>
<PlatformToolset>v143</PlatformToolset>
<PlatformToolset>v142</PlatformToolset>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<PlatformToolset>v142</PlatformToolset>
<PlatformToolset>v143</PlatformToolset>

@@ -29,7 +29,7 @@
<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'" Label="Configuration">
<ConfigurationType>Application</ConfigurationType>
<UseDebugLibraries>true</UseDebugLibraries>
<PlatformToolset>v143</PlatformToolset>
<PlatformToolset>v142</PlatformToolset>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<PlatformToolset>v142</PlatformToolset>
<PlatformToolset>v143</PlatformToolset>

@@ -41,7 +41,7 @@
<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'" Label="Configuration">
<ConfigurationType>Application</ConfigurationType>
<UseDebugLibraries>false</UseDebugLibraries>
<PlatformToolset>v143</PlatformToolset>
<PlatformToolset>v142</PlatformToolset>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<PlatformToolset>v142</PlatformToolset>
<PlatformToolset>v143</PlatformToolset>

@@ -102,14 +102,15 @@ goto :ARGS_LOOP

:POST_ARGS_LOOP
%MSBUILD% %~dp0\DistroLauncher.sln /t:%_MSBUILD_TARGET% /m /nr:true ^
/restore ^
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use spaces

Suggested change
/restore ^
/restore ^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants