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

Fixes automated WSL installation on ARM #16997

Merged

Conversation

n1hility
Copy link
Member

@n1hility n1hility commented Jan 5, 2023

This PR builds on #16987, although the merge order does not matter, since it simply updates the installer.

This PR changes the custom action used to check the need for WSL installation to be a simple native C based win32 implementation as opposed to the cgo c-shared DLL approach used previously. This change is necessary to resolve compatibility problems with the Windows x86_64 emulation layer on ARM, which struggles with hot-loading a translated goruntime into an existing process (in contrast, standard executables and hard-linking work fine). Once this issue is resolved, this could be converted back to cgo for project consistency.

While the language has changed for the podman-msihooks.dll artifact, the existing process, pre-requisites, and build steps remain the same (cgo already required we had a win32 c compiler toolchain installed)

As mentioned in #16987, future PRs will update the installer to bundle arm64 podman remote client binaries. However, this is not necessary at this stage.

The installer itself, and it's supporting code will remain x86, even though it will eventually also package ARM binaries, for two reasons:

  1. We can have one installer download, no matter the platform (prevents picking the wrong one)
  2. Since the incentives are low (no discernable perf difference for an installer), it will be some time before the mainstream Windows installer toolchains (e.g. Wix) fully support the option.
Fixes WSL auto-installation when run under Windows ARM x86_64 emulation

[NO NEW TESTS NEEDED]

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 5, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: n1hility

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 5, 2023
@n1hility n1hility force-pushed the winstaller-arm-compat branch from faaa922 to 85e2ec1 Compare January 5, 2023 07:28
Fixes automated WSL installation on ARM

Signed-off-by: Jason T. Greene <[email protected]>
@n1hility n1hility force-pushed the winstaller-arm-compat branch from 85e2ec1 to 54afda2 Compare January 5, 2023 07:50
@rhatdan
Copy link
Member

rhatdan commented Jan 5, 2023

LGTM
@gbraad PTAL

@rhatdan rhatdan added the 4.4 label Jan 7, 2023
@baude
Copy link
Member

baude commented Jan 9, 2023

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 9, 2023
@openshift-merge-robot openshift-merge-robot merged commit 31e22aa into containers:main Jan 9, 2023
@edsantiago
Copy link
Member

All PRs are failing CI today:

Downloading https://api.cirrus-ci.com/v1/artifact/build/5589793266991104/Windows Cross/repo/repo.tbz to repo.tbz2
arc : The term 'arc' is not recognized as the name of a cmdlet, function, 
script file, or operable program. Check the spelling of the name, or if a path 
was included, verify that the path is correct and try again.
At C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\contrib\c
irrus\win-installer-main.ps1:42 char:1
+ arc unarchive repo.tbz2 .; CheckExit
+ ~~~
    + CategoryInfo          : ObjectNotFound: (arc:String) [], ParentContainsE 
   rrorRecordException
    + FullyQualifiedErrorId : CommandNotFoundException

@n1hility could this be related, or is it just coincidence?

@n1hility
Copy link
Member Author

All PRs are failing CI today:

Downloading https://api.cirrus-ci.com/v1/artifact/build/5589793266991104/Windows Cross/repo/repo.tbz to repo.tbz2
arc : The term 'arc' is not recognized as the name of a cmdlet, function, 
script file, or operable program. Check the spelling of the name, or if a path 
was included, verify that the path is correct and try again.
At C:\Users\ContainerAdministrator\AppData\Local\Temp\cirrus-ci-build\contrib\c
irrus\win-installer-main.ps1:42 char:1
+ arc unarchive repo.tbz2 .; CheckExit
+ ~~~
    + CategoryInfo          : ObjectNotFound: (arc:String) [], ParentContainsE 
   rrorRecordException
    + FullyQualifiedErrorId : CommandNotFoundException

@n1hility could this be related, or is it just coincidence?

@edsantiago i think this is unrelated. Will take a look,

@baude
Copy link
Member

baude commented Feb 3, 2023

/cherry-pick v4.4

@openshift-cherrypick-robot
Copy link
Collaborator

@baude: new pull request could not be created: failed to create pull request against containers/podman#v4.4 from head openshift-cherrypick-robot:cherry-pick-16997-to-v4.4: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"No commits between containers:v4.4 and openshift-cherrypick-robot:cherry-pick-16997-to-v4.4"}],"documentation_url":"https://docs.github.com/rest/reference/pulls#create-a-pull-request"}

In response to this:

/cherry-pick v4.4

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 12, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants