-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Consider new syntax or a new option for the --dir
CLI option
#7309
Comments
According to docker/cli#1480 the Docker CLI uses a CSV parser for |
I'm not sure that using CSV as a way to reduce ambiguity is a design choice I'd copy... 😜 Jokes aside, though, I do like the As for escaping, I'm guessing there's some great art out there from which to draw inspiration, but my gut feeling is that something as simple as possible (like repeating the delimiter if you want it as a literal) would be good to avoid creating exotic corner cases for people to stumble upon. |
(Yes, originally Wasmtime used I agree with @jeffparsons ; what if we went with just the |
I'm a bit ambivalent myself about only having the With only the Also with |
I wouldn't disregard your ambivalence. I know my personal preference in things like this sits all the way up one end of the spectrum, and what feels right to me is often perceived as unnecessarily cumbersome to my colleagues. If I try to put on my "everyone else hat" I do feel that having a simple
Tomayto / tomahto. If all functionality was somehow squished into one arg (see below), I figure you could just keep
I was taking a shot at CSV specifically because it's so poorly specified/understood. (Ask three people what "CSV" means and you'll get four different answers.) And I've always been irked by tools that say "the format of this is whatever libwhatever accepts, where libwhatever has its own quirks and changes unpredictably over time. I have no such qualms about JSON, especially for the "other tools calling Wasmtime" use case. Given that the existing Does Wasmtime support some kind of config file or "response file" as an alternative to passing all options as command line arguments? (I don't recall seeing one.) If so, or if it might in future, what if command line arguments like
The parsing rule for any such argument would be:
Then I imagine interactive users would mostly use the shorthand, people like me would use the JSON format always, and other tools calling Wasmtime would mostly write/pipe a config file instead of passing command line flags. Is that too weird? I guess it opens a huge can of worms around config/response files, too, so it might be too much to bite off just for the sake of improving the |
I think we shouldn't use |
I think @jeffparsons makes a compelling argument myself. Coupled with @tschneidereit's thoughts, which I also agree with, I think the proposal at this point is basically adding JSON support to |
i guess it would be nicer to keep it similar among wasi-supporting wasm runtimes. |
This issue is inspired by discussion on #7301 and Zulip. To summarize a bit, the
--dir
syntax is inconsistent between Wasmtime and other runtimes, and a notable inconsistency of Wasmtime is the usage of::
as a delimiter as opposed to a single character such as:
. I'll also personally note that there's inconsistency with Docker which is another common tool for a similar operatin which uses:
.As of #7301 and Wasmtime 14/15 the CLI for Wasmtime will support:
--dir foo
- opens the host directoryfoo
and mounts it in the guest asfoo
--dir foo::bar
- opens the host directoryfoo
and mounts it in the guest asbar
I find the discussion over on Docker's documentation pretty illuminating here. The gist, as it applies to Wasmtime, appears to be:
-v
is the "old" syntax and is acknowledged to be cryptic and error-prone, and this is basically the same as the--dir
syntax Wasmtime has today (and the subject of this comment)--mount
is the "new" syntax which uses--mount key=val,key2=val,key3=val
which is more verbose but more clearWhile I wasn't personally present (@sunfishcode maybe you were?) when this option was originally added to Wasmtime my guess is that the choice of
::
delimiter for Wasmtime assists with Windows-style paths which have colons in them with absolute paths such asC:\foo\bar
. Such a concern does not affect Docker, however, where Docker appears to supportdocker run -v z:\foo:c:\dest ...
according to its documentation.With all that in mind, there's a few questions in my mind of what to do with Wasmtime:
--dir
be kept at all? Should this be replaced entirely with a separate option with more obvious syntax to readers? (albeit more verbose to writers)--dir
is kept, can its delimiter be changed to:
to be more consistent with Docker? If so how do Windows paths work?,
-delimited list ofkey=value
pairs, but I'm also not sure how this handles paths-with-commas in them.Personally I think that while the CLI was overhauled for Wasmtime 14 we can still perform this change when we want. It's probably not too too hard to have broad messaging of a change in syntax if
--dir
changes or gets replaced. So in that sense while we can't change anything for Wasmtime 14, I do still think changes should be considered for 15+The text was updated successfully, but these errors were encountered: