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

feat: refactor cli parsing #1200

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Shubhranshu153
Copy link
Contributor

@Shubhranshu153 Shubhranshu153 commented Nov 29, 2024

Issue #, if available:

Description of changes:
For macos and windows and OS specific command prefix is attached to execute in the vm plane. As there is a vm plane, environment variables and credentials needs to be passed from the host to the vm. The limactl plane doesnt automatically handle this [Issue: https://github.com/lima-vm/lima/issues/1419]. To address this, the current finch handler used to parse or env flags [‘-e’,‘—env’,‘—env-file’], consolidated it and added it. For windows, the host path need to be translated to equivalent mount paths in the wsl environment.

In the proposed design, For macOS and windows we parse all env flags and add those env variables in the prefix of the limactl command. Here we dont modify the ordering or the contents of the command args reducing the complexity and chances of error getting introduced.

For windows path we do an inplace substitution based on regex pattern search across the args. This involves modifying the command args as windows to wsl path translation support for limactl shell execution is not provided. We use regexp to do a pattern match in substring of args split by separators. To validate this, we would rely on unit test and e2e tests.

Testing done:

  1. Automated Tests

TODO:
Devcontainers qualification tests.

  • I've reviewed the guidance in CONTRIBUTING.md

License Acceptance

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@Shubhranshu153 Shubhranshu153 force-pushed the refactor-remote-parsing branch 20 times, most recently from 9f211cf to 5ea6b14 Compare December 1, 2024 11:32
@Shubhranshu153 Shubhranshu153 reopened this Dec 1, 2024
@Shubhranshu153 Shubhranshu153 force-pushed the refactor-remote-parsing branch 8 times, most recently from d5b2a85 to 181583b Compare December 1, 2024 23:56
@Shubhranshu153 Shubhranshu153 force-pushed the refactor-remote-parsing branch 22 times, most recently from 79011b7 to 3f21783 Compare December 6, 2024 17:58
@Shubhranshu153 Shubhranshu153 force-pushed the refactor-remote-parsing branch 3 times, most recently from 4f1e4aa to a471cac Compare December 15, 2024 02:47
Signed-off-by: Shubharanshu Mahapatra <[email protected]>
@Shubhranshu153 Shubhranshu153 force-pushed the refactor-remote-parsing branch from a471cac to f52ab39 Compare December 15, 2024 04:38
@Shubhranshu153 Shubhranshu153 marked this pull request as ready for review December 30, 2024 16:11
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.

1 participant