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

[ARM64]CI and build pipelines #18337

Merged
merged 7 commits into from
May 19, 2022
Merged

Conversation

jaimecbernardo
Copy link
Collaborator

@jaimecbernardo jaimecbernardo commented May 17, 2022

Summary of the Pull Request

What is this about:

Builds both x64 and arm64 on CI and build Pipelines.

What is included in the PR:

  • Run both x64 and arm64 jobs in CI and release pipelines.
  • Add GenerateSatelliteAssembliesForCore to the projects that require localization, to avoid using ALINK (al.exe) in the CI, which doesn't support arm64.
  • Builds installer with actual arm64 name and output paths (hacks for the x86 installer workaround should be placed locally nearer the place of the x86 hack than farther on the pipelines). This should be cleared once we update Wix to a version that supports arm64 installers.
  • Deletes build-installer-MSI.cmd, which is no longer used.
  • Some minor white space fixes in touched solution files.

How does someone test / validate:
CI and release pipelines are able to build both x64 and arm64. (No arm64 tests run in CI yet. Not sure we have agents for that)

Quality Checklist

  • Linked issue: Support ARM platform #490
  • Communication: I've discussed this with core contributors in the issue.
  • Tests: Added/updated and all pass
  • Installer: Added/updated and all pass
  • Localization: All end user facing strings can be localized
  • Docs: Added/ updated
  • Binaries: Any new files are added to WXS / YML

@jaimecbernardo
Copy link
Collaborator Author

jaimecbernardo commented May 17, 2022

(opened the draft PR in order to run the CI pipelines to check for errors)

@jaimecbernardo jaimecbernardo force-pushed the dev/jaimecbernardo/arm64-pipelines branch from 88a699f to 102bef7 Compare May 17, 2022 09:10
@microsoft microsoft deleted a comment from github-actions bot May 17, 2022
@jaimecbernardo jaimecbernardo marked this pull request as ready for review May 17, 2022 12:45
@jaimecbernardo
Copy link
Collaborator Author

@yuyoyuppe @crutkas , It's ready for review.

@crutkas , please check the changes to publishing on nuget. Is arm64 nuget something we want to do?

@@ -6,6 +6,7 @@
<OutputPath>$(SolutionDir)$(Platform)\$(Configuration)\modules\ColorPicker</OutputPath>
<AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>
<AppendRuntimeIdentifierToOutputPath>false</AppendRuntimeIdentifierToOutputPath>
<GenerateSatelliteAssembliesForCore>true</GenerateSatelliteAssembliesForCore>
Copy link
Contributor

Choose a reason for hiding this comment

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

should we add it as part of Directory.props file instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is project specific, though. Only for C# projects with localization. No idea if there's a downside otherwise.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Plus, directory.props doesn't propagate for publishing directly from the csproj, so it would still need it in the project files for projects that are published.

@crutkas
Copy link
Member

crutkas commented May 18, 2022

so excited!!

@@ -21,7 +21,7 @@ jobs:
$github = Invoke-RestMethod -uri "https://api.github.com/repos/Microsoft/PowerToys/releases"

$targetRelease = $github | Where-Object -Property name -match 'Release'| Select -First 1
$installerUrl = $targetRelease | Select -ExpandProperty assets -First 1 | Where-Object -Property name -match 'PowerToysSetup' | Select -ExpandProperty browser_download_url
$installerUrl = $targetRelease | Select -ExpandProperty assets -First 1 | Where-Object -Property name -match 'PowerToysSetup.*x64' | Select -ExpandProperty browser_download_url
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you elaborate this change?

Copy link
Member

Choose a reason for hiding this comment

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

This is grabbing only x64. The script needs larger tweaks after a manual step to include arm. I created an issue to track.

@snickler
Copy link
Collaborator

😍😍

@jaimecbernardo jaimecbernardo merged commit 1375018 into main May 19, 2022
@jaimecbernardo jaimecbernardo deleted the dev/jaimecbernardo/arm64-pipelines branch June 14, 2022 23:32
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.

5 participants