Skip to content

Commit f9a87e3

Browse files
committed
cli: only allow file path in run and compile commands
This changes the run and compile commands to only accept a file name as an argument. Before this, if a file name was misspelled, it was treated as the body of a MicroPython script and compiled instead of raising a file not found error. Since we are using argparse.FileType, `-` is accepted as an argument meaning read from standard input. This will allow the previous behavior of passing a script without actually creating a file, albeit with a different syntax, i.e. piping via shell. Fixes: pybricks/support#411
1 parent 8579db4 commit f9a87e3

File tree

2 files changed

+58
-22
lines changed

2 files changed

+58
-22
lines changed

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
88

99
### Changed
1010
- Updated `bleak` dependency to v0.12.1.
11+
- `run` and `compile` scripts now accept `-` as an argument to mean stdin.
12+
13+
### Removed
14+
- Removed script command line args in `run` and `compile` commands. Only accepts
15+
file name now.
1116

1217
## [1.0.0-alpha.11] - 2021-07-05
1318

pybricksdev/cli/__init__.py

Lines changed: 53 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,12 @@
55

66
import argparse
77
import asyncio
8+
import contextlib
89
import logging
10+
import os
911
import sys
12+
from tempfile import NamedTemporaryFile
13+
from typing import ContextManager, TextIO
1014
import validators
1115

1216
from abc import ABC, abstractmethod
@@ -50,13 +54,43 @@ async def run(self, args: argparse.Namespace):
5054
pass
5155

5256

53-
def _parse_script_arg(script_arg):
54-
"""Save user script argument to a file if it is a Python one-liner."""
55-
from ..compile import save_script
57+
def _get_script_path(file: TextIO) -> ContextManager:
58+
"""
59+
Gets the path to a script on the file system.
5660
57-
if not path.exists(script_arg):
58-
return save_script(script_arg)
59-
return script_arg
61+
If the file is ``sys.stdin``, the contents are copied to a temporary file
62+
and the path to the temporary file is returned. Otherwise, the file is closed
63+
and the path is returned.
64+
65+
The context manager will delete the temporary file, if applicable.
66+
"""
67+
if file is sys.stdin:
68+
69+
# Have to close the temp file so that mpy-cross can read it, so we
70+
# create our own context manager to delete the file when we are done
71+
# using it.
72+
73+
@contextlib.contextmanager
74+
def temp_context():
75+
try:
76+
with NamedTemporaryFile(suffix=".py", delete=False) as temp:
77+
temp.write(file.buffer.read())
78+
79+
yield temp.name
80+
finally:
81+
try:
82+
os.remove(temp.name)
83+
except NameError:
84+
# if NamedTemporaryFile() throws, temp is not defined
85+
pass
86+
except OSError:
87+
# file was already deleted or other strangeness
88+
pass
89+
90+
return temp_context()
91+
92+
file.close()
93+
return contextlib.nullcontext(file.name)
6094

6195

6296
class Compile(Tool):
@@ -65,21 +99,19 @@ def add_parser(self, subparsers: argparse._SubParsersAction):
6599
"compile",
66100
help="compile a Pybricks program without running it",
67101
)
68-
parser.tool = self
69-
# The argument is a filename or a Python one-liner.
70102
parser.add_argument(
71-
"script",
72-
metavar="<script>",
73-
help="path to a MicroPython script or inline script",
103+
"file",
104+
metavar="<file>",
105+
help="path to a MicroPython script or `-` for stdin",
106+
type=argparse.FileType(),
74107
)
108+
parser.tool = self
75109

76110
async def run(self, args: argparse.Namespace):
77111
from ..compile import compile_file, print_mpy
78112

79-
script_path = _parse_script_arg(args.script)
80-
81-
# Compile the script and print the result
82-
mpy = await compile_file(script_path)
113+
with _get_script_path(args.file) as script_path:
114+
mpy = await compile_file(script_path)
83115
print_mpy(mpy)
84116

85117

@@ -97,9 +129,10 @@ def add_parser(self, subparsers: argparse._SubParsersAction):
97129
choices=["ble", "usb", "ssh"],
98130
)
99131
parser.add_argument(
100-
"script",
101-
metavar="<script>",
102-
help="path to a MicroPython script or inline script",
132+
"file",
133+
metavar="<file>",
134+
help="path to a MicroPython script or `-` for stdin",
135+
type=argparse.FileType(),
103136
)
104137
parser.add_argument(
105138
"-n",
@@ -142,9 +175,6 @@ async def run(self, args: argparse.Namespace):
142175
USBRPCConnection,
143176
)
144177

145-
# Convert script argument to valid path
146-
script_path = _parse_script_arg(args.script)
147-
148178
# Pick the right connection
149179
if args.conntype == "ssh":
150180
# So it's an ev3dev
@@ -181,7 +211,8 @@ async def run(self, args: argparse.Namespace):
181211
# Connect to the address and run the script
182212
await hub.connect(device_or_address)
183213
try:
184-
await hub.run(script_path, args.wait)
214+
with _get_script_path(args.file) as script_path:
215+
await hub.run(script_path, args.wait)
185216
finally:
186217
await hub.disconnect()
187218

0 commit comments

Comments
 (0)