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

Convert additional build context paths on Windows #23391

Merged
merged 1 commit into from
Aug 6, 2024

Conversation

l0rd
Copy link
Member

@l0rd l0rd commented Jul 24, 2024

This PR partially fixes issues with the option --build-context <label>=<path> of podman build.

This is enough to fix #17313 on macOS and Windows WSL with the default machine mounts.

A complete fix of the option --build-context would require packaging the additional build contexts in some additional tars and update the /libpod/build endpoint to accept multi-part HTTP requests. But that's outside the scope of this PR.

Does this PR introduce a user-facing change?

Fix VisualStudio Dev Containers Features on Windows

@rhatdan
Copy link
Member

rhatdan commented Jul 24, 2024

LGTM
@n1hility @baude @ashley-cui PTAL

@mheon
Copy link
Member

mheon commented Jul 24, 2024

Is this something we can test?

@l0rd l0rd force-pushed the build-contexts branch from c32e283 to e690efd Compare July 24, 2024 20:35
@l0rd l0rd marked this pull request as draft July 25, 2024 16:53
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 25, 2024
@l0rd
Copy link
Member Author

l0rd commented Jul 25, 2024

Is this something we can test?

I am going to add an e2e test. Converted it back to draft mode.

@n1hility
Copy link
Member

LGTM

Signed-off-by: Mario Loriedo <mario.loriedo@gmail.com>
@l0rd l0rd force-pushed the build-contexts branch from 3939719 to bf3f207 Compare July 29, 2024 15:44
@l0rd
Copy link
Member Author

l0rd commented Jul 29, 2024

I have added a new e2e machine test podman build contexts in pkg/machine/e2e/basic_tests.go.

I have also updated pkg/machine/e2e/README.md with more instructions to run the tests on windows. There are also a couple of small fixes in build_windows.md that are unrelated to the additional contexts but too small to deserve a dedicated PR.

Tests are green. I am switching this PR back to ready for review.

@l0rd l0rd marked this pull request as ready for review July 29, 2024 22:02
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 29, 2024
@l0rd
Copy link
Member Author

l0rd commented Aug 6, 2024

@containers/podman-maintainers PTAL

@rhatdan
Copy link
Member

rhatdan commented Aug 6, 2024

/approve
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 6, 2024
Copy link
Contributor

openshift-ci bot commented Aug 6, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: l0rd, rhatdan

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 Aug 6, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit b1d111c into containers:main Aug 6, 2024
90 checks passed
@worldofgeese
Copy link

Does this mean Podman will now just work with Dev Containers? I can scarcely contain my excitement!

@l0rd
Copy link
Member Author

l0rd commented Aug 6, 2024

Does this mean Podman will now just work with Dev Containers? I can scarcely contain my excitement!

A few clarifications:

  • This is now merged in main branch and will be included in next stable version of Podman (i.e. v5.3.0)
  • The machine provider needs to be WSL (Hyper-V supports needs Error running podman build with option --build-context on Hyper-V #23429)
  • The devcontainer.json should include the following runArgs and containerEnv (but I think it may be avoided in rootless mode):
    "runArgs": [
    "--userns=keep-id"
    ],
    "containerEnv": {
    "HOME": "/home/node"
    }

@JustinGrote
Copy link

@l0rd thank you!

@stale-locking-app stale-locking-app bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Nov 5, 2024
@stale-locking-app stale-locking-app bot locked as resolved and limited conversation to collaborators Nov 5, 2024
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. machine release-note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: vscode devcontainers "features" feature fails on Windows and MacOS
6 participants