unix/main: Use standard pyexec/repl for unix port. #12802

andrewleech · 2023-10-25T23:40:29Z

This improves repl usage consistency across ports.
Only enabled when MICROPY_USE_READLINE == 1 (default).

In particular I noticed that aiorepl didn't work properly on the unix port currently, it would echo the "just entered line" on a newline each time you hit enter, eg.

>>> import aiorepl, asyncio;asyncio.run(aiorepl.task())
Starting asyncio REPL...
--> print("hello")
print("hello")
hello

I feel there were also a few other inconsistencies with how I'm used to the repl working on the stm port / via mpremote which are alsos hopefully resolved now.

I've got an optional couple of lines to restore/reset stdio_raw mode when running code - however if raw mode is disabled the aiorepl issue above remains. I think for consistency with other ports it's better to keep stdio in raw mode, but not sure if this is likely to have other side effects?

This PR consilidates the repl functionality and fixes the above aiorepl glitch.

dpgeorge · 2023-10-25T23:51:56Z

Thanks for this, it's a good clean up.

I actually attempted it before, see #6008. The approach here is much simpler, so maybe it's a good first step to make.

andrewleech · 2023-10-26T01:34:26Z

Ah interesting, I did see some of the return handling and kind of assumed it shouldn't matter - if someone is dealing with the repl (live coding) they're unlikely to care about the exit code?

I didn't consider the SystemExit interrupt at all, haven't looked at how that behaves but probably should!

If running a python file directly then certainly the exit code is important but that shouldn't be impacted by this.

andrewleech · 2023-10-26T01:43:03Z

Ah, I might need to read through the other PR discussion a bit more... but in the mean time this is a little off:

$ ~/micropython/ports/unix/build-standard/micropython
MicroPython v1.22.0-preview.32.g2c1c7d8b30 on 2023-10-26; linux [GCC 10.2.1] version
Type "help()" for more information.
>>> raise ValueError()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError:
>>> raise SystemExit()
>>>

Edit:
Actually that behaviour above is consistent with running a script on Unix port in that SystemExit will drop out of the running script back to console without printing anything, other exceptions drop out while printing the traceback.

jimmo · 2023-10-26T07:36:27Z

See also #12807

andrewleech · 2023-11-16T09:49:35Z

I ran into a tricky issue when trying to fix the unit tests; the banner on Unix port has changed a little with this MR (now consistent with other ports) so unit test exp needed to be updated to match.

I initially tried just copying the results/*.out files to replace the original .exp ones, but the tests still failed! There were inconsistent newlines in the files that tripped up the test.
I narrowed the problem down to the pty usage:

micropython was outputting stdout with '\r\n`
pty was converting \n to \r\n, creating a few \r\r\n
the run_tests script does convert \r\n back to \n but I think it was only doing that once before writing failed test *.out files, then converting a second time to do the actual comparison to *.exp.
Or it was the offer way around... or maybe subprocess was doing some conversions too.
Either way I was getting some \r\n and some \n in the test out files.
The same issue is discussed here which is where I found reference to how to disable the pty newline conversions: https://www.thecodingforums.com/threads/new-line-conversion-with-popen-attached-to-a-pty.961960/post-5100925

stinos · 2023-11-16T10:03:49Z

ports/windows/micropython.vcxproj

@@ -88,6 +88,7 @@
    <ClCompile Include="@(PyCoreSource)" />
    <ClCompile Include="@(PyExtModSource)" />
    <ClCompile Include="$(PyBaseDir)shared\readline\*.c" />
+    <ClCompile Include="$(PyBaseDir)shared\runtime\pyexec.c" />


Super minor but can you move this one line down so it's aplhabetically?

Ah whoops, I honestly thought I was alreadyt putting it in order, eyes blurred readline/runtime and completely missed runtime\gchelper_generic below!

stinos · 2023-11-16T10:06:25Z

tests/run-tests.py

@@ -232,6 +233,11 @@ def send_get(what):
                    with open(test_file, "rb") as f:
                        # instead of: output_mupy = subprocess.check_output(args, stdin=f)
                        master, slave = pty.openpty()
+                        # disable auto newline conversion on the pty terminal.
+                        attrs = termios.tcgetattr(slave)


You're probably aware but this doesn't exist on windows. Isn't there a way to just do a replace("\r\n", "\n") before comparing results, or something like that?

Oh, I thought pty itself wasn't avaialble either so it would be a moot point?
I initially thought of just adding another newline replace, but thought fixing the problem at the source was cleaner than further fiddling with test results.

Yeah it looks like these repl_* tests are already skipped on windows master builds https://ci.appveyor.com/project/dpgeorge/micropython/builds/48536058/job/4o16bx3lnvsu7nbn

Oh, I thought pty itself wasn't avaialble either so it would be a moot point?

Yes, but

I initially thought of just adding another newline replace, but thought fixing the problem at the source was cleaner than further fiddling with test results.

Cleaner perhaps, but: reaonsing that suppose we do get cmdline tests working for the windows port, like with some other code, then that would also need a similar fix. Whereas the more general case of the tests being written like 'whatever you throw at me I'm going to make newlines consistent first because I know that can be an issue' seems the more robust way plus keeps everything nicely in one place?

codecov · 2023-11-16T10:53:23Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.54%. Comparing base (f315a37) to head (5d39ed7).

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #12802   +/-   ##
=======================================
  Coverage   98.54%   98.54%           
=======================================
  Files         169      169           
  Lines       21877    21877           
=======================================
  Hits        21558    21558           
  Misses        319      319

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

dpgeorge · 2024-07-05T06:24:34Z

Regarding the comment above about raise SystemExit: actually this PR changes the behaviour, now if you raise SystemExit from the REPL it does nothing. Currently on master it will exit MicroPython back to the shell.

We need to retain the existing behaviour, so we don't break existing usage, and because that's how CPython works.

Then it's a separate question how to handle this on bare-metal targets. But for now we shouldn't change the unix behaviour on this.

github-actions · 2024-07-09T07:06:44Z

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    -4 -0.002% 
   unix x64: +1577 +0.184% standard[incl +8(bss)]
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32

andrewleech · 2024-07-09T07:07:09Z

Regarding the comment above about raise SystemExit: actually this PR changes the behaviour, now if you raise SystemExit from the REPL it does nothing. Currently on master it will exit MicroPython back to the shell.

We need to retain the existing behaviour, so we don't break existing usage, and because that's how CPython works.

Thanks, yes I hadn't realised this behavior had been changed inadvertantly. I've now fixed it in the latest push.
There was an associates variable here pyexec_system_exit which I did a global search for - it was never actually being used (anymore) so I've stripped that out as well.
Any variable passed to SystemExit is now returned for unix port.

anl@STEP: ~ $ micropython
MicroPython v1.24.0-preview.102.g03e992d59d.dirty on 2024-07-09; linux [GCC 9.4.0] version
Type "help()" for more information.
>>> raise SystemExit
anl@STEP: ~ $ echo $?
0
anl@STEP: ~ $ micropython
MicroPython v1.24.0-preview.102.g03e992d59d.dirty on 2024-07-09; linux [GCC 9.4.0] version
Type "help()" for more information.
>>> raise SystemExit(-1)
anl@STEP: ~ $ echo $?
255

... which matches cpython correctly:

anl@ANL2-LAP:~$ python
Python 3.8.10 (default, Nov 22 2023, 10:22:35)
[GCC 9.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> raise SystemExit(-1)
anl@ANL2-LAP:~$ echo $?
255

Then it's a separate question how to handle this on bare-metal targets. But for now we shouldn't change the unix behaviour on this.

Well it looks to me like this should be treated as a soft reset.

micropython/extmod/modmachine.c

Line 66 in 358e501

mp_raise_type(&mp_type_SystemExit);

The current push also works this way:

anl@STEP: ~/micropython $ mpremote
Connected to MicroPython at /dev/ttyACM0
Use Ctrl-] or Ctrl-x to exit this shell
MicroPython v1.24.0-preview.104.g161a9c3097.dirty on 2024-07-09; WEACTV20_F411 with STM32F411CE
Type "help()" for more information.
>>> raise SystemExit
MPY: sync filesystems
MPY: soft reboot
MicroPython v1.24.0-preview.104.g161a9c3097.dirty on 2024-07-09; WEACTV20_F411 with STM32F411CE
Type "help()" for more information.
>>>

dpgeorge · 2024-07-18T04:03:19Z

There was an associates variable here pyexec_system_exit which I did a global search for - it was never actually being used (anymore) so I've stripped that out as well.

Right, it would be a nice clean up to get rid of that variable, and also change the behaviour of SystemExit on bare-metal ports to match unix/CPython.

Because this is a change in user-facing behaviour, I've split that change out in #15486.

andrewleech · 2024-08-13T12:46:15Z

This has been rebased to master, taking into account the SystemExit stuff already merged.

dpgeorge · 2024-08-13T13:03:12Z

Testing this on the unix coverage variant, passing -v -v -v no longer prints the parse tree out. That needs to be fixed.

I suggest just copying the following bit into the relevant place in pyexec.c:

        #if defined(MICROPY_UNIX_COVERAGE)
        // allow to print the parse tree in the coverage build
        if (mp_verbose_flag >= 3) {
            printf("----------------\n");
            mp_parse_node_print(&mp_plat_print, parse_tree.root, 0);
            printf("----------------\n");
        }
        #endif

andrewleech · 2024-08-13T20:25:41Z

prints the parse tree out

Thanks, I wasn't aware of this development feature. I also hadn't noticed the amount of overlap between pyexec/parse_compile_execute function and unix/main/execute_from_lexer() but there's probably not much to be really gained by consolidating the two.

I've made the change now and assume it's working:

anl@STEP: ~/micropython $ ./ports/unix/build-coverage/micropython -v -v -v
MicroPython v1.24.0-preview.187.g95bce29344.dirty on 2024-08-14; linux [GCC 9.4.0] version
Type "help()" for more information.
>>> import os
----------------
[   1] import_name(14) (n=1)
         id(os)
----------------
File <stdin>, code block '<module>' (descriptor: 7f9fea2a9320, bytecode @7f9fea2a9180 11 bytes)
Raw bytecode (code_info_size=3, bytecode_size=8):
 08 02 01 80 51 1b 02 16 02 51 63
arg names:
(N_STATE 2)
(N_EXC_STACK 0)
  bc=0 line=1
00 LOAD_CONST_SMALL_INT 0
...

dpgeorge · 2024-08-14T02:02:32Z

It looks like there are still a couple of things missing in pyexec.c that the unix port has:

support the compile_only flag
set __file__ when MICROPY_PY___FILE__ is enabled

dpgeorge · 2024-08-14T02:03:44Z

It looks like there are still a couple of things missing in pyexec.c that the unix port has

This is why #6008 is quite a bit bigger than this PR, and why it was still in progress, because of all these details 😄

Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>

This improves repl usage consistency across ports. Only enabled when MICROPY_USE_READLINE == 1 (default). Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>

dpgeorge added the port-unix label Oct 25, 2023

andrewleech force-pushed the unix_pyexec branch 2 times, most recently from e4cc4f5 to 2c1c7d8 Compare October 26, 2023 01:38

andrewleech force-pushed the unix_pyexec branch from 2c1c7d8 to 55032e9 Compare October 26, 2023 01:44

jimmo mentioned this pull request Oct 26, 2023

ports/unix: Add embedded mode. #12807

Open

andrewleech force-pushed the unix_pyexec branch 2 times, most recently from 12f5770 to 7a0825f Compare November 16, 2023 09:37

stinos reviewed Nov 16, 2023

View reviewed changes

andrewleech force-pushed the unix_pyexec branch from 7a0825f to ad27be1 Compare November 16, 2023 10:24

andrewleech force-pushed the unix_pyexec branch from ad27be1 to 02fb334 Compare November 16, 2023 21:53

mattytrentini added the maintainer-queue label Feb 27, 2024

dpgeorge added this to the release-1.24.0 milestone Mar 26, 2024

andrewleech force-pushed the unix_pyexec branch 3 times, most recently from d08fe6e to 554ba06 Compare March 26, 2024 01:01

andrewleech force-pushed the unix_pyexec branch from 554ba06 to 03e992d Compare July 9, 2024 06:58

dpgeorge mentioned this pull request Jul 18, 2024

Make raising SystemExit do a soft reset on bare-metal targets #15486

Merged

andrewleech force-pushed the unix_pyexec branch from 03e992d to 95bce29 Compare August 13, 2024 12:26

andrewleech force-pushed the unix_pyexec branch from 95bce29 to 8b8b730 Compare August 13, 2024 20:20

projectgus removed the maintainer-queue label Aug 30, 2024

dpgeorge modified the milestones: release-1.24.0, release-1.25.0 Sep 10, 2024

dpgeorge mentioned this pull request Nov 4, 2024

ports/unix: Add raw REPL support. #16141

Open

dpgeorge modified the milestones: release-1.25.0, release-1.26.0 Jan 28, 2025

andrewleech added 2 commits March 26, 2025 11:38

tests/run-tests: Fix newline handling on repl/pty tests.

d9e9195

Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>

unix/main: Use standard pyexec/repl for unix & windows ports.

5d39ed7

This improves repl usage consistency across ports. Only enabled when MICROPY_USE_READLINE == 1 (default). Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>

andrewleech force-pushed the unix_pyexec branch from 8b8b730 to 5d39ed7 Compare March 26, 2025 00:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

unix/main: Use standard pyexec/repl for unix port. #12802

unix/main: Use standard pyexec/repl for unix port. #12802

andrewleech commented Oct 25, 2023

dpgeorge commented Oct 25, 2023

andrewleech commented Oct 26, 2023

andrewleech commented Oct 26, 2023 •

edited

Loading

jimmo commented Oct 26, 2023

andrewleech commented Nov 16, 2023 •

edited

Loading

stinos Nov 16, 2023

andrewleech Nov 16, 2023

stinos Nov 16, 2023 •

edited

Loading

andrewleech Nov 16, 2023

andrewleech Nov 16, 2023

stinos Nov 17, 2023

codecov bot commented Nov 16, 2023 •

edited

Loading

dpgeorge commented Jul 5, 2024

github-actions bot commented Jul 9, 2024 •

edited

Loading

andrewleech commented Jul 9, 2024 •

edited

Loading

dpgeorge commented Jul 18, 2024

andrewleech commented Aug 13, 2024

dpgeorge commented Aug 13, 2024

andrewleech commented Aug 13, 2024 •

edited

Loading

dpgeorge commented Aug 14, 2024

dpgeorge commented Aug 14, 2024

unix/main: Use standard pyexec/repl for unix port. #12802

Are you sure you want to change the base?

unix/main: Use standard pyexec/repl for unix port. #12802

Conversation

andrewleech commented Oct 25, 2023

dpgeorge commented Oct 25, 2023

andrewleech commented Oct 26, 2023

andrewleech commented Oct 26, 2023 • edited Loading

jimmo commented Oct 26, 2023

andrewleech commented Nov 16, 2023 • edited Loading

stinos Nov 16, 2023

Choose a reason for hiding this comment

andrewleech Nov 16, 2023

Choose a reason for hiding this comment

stinos Nov 16, 2023 • edited Loading

Choose a reason for hiding this comment

andrewleech Nov 16, 2023

Choose a reason for hiding this comment

andrewleech Nov 16, 2023

Choose a reason for hiding this comment

stinos Nov 17, 2023

Choose a reason for hiding this comment

codecov bot commented Nov 16, 2023 • edited Loading

Codecov Report

dpgeorge commented Jul 5, 2024

github-actions bot commented Jul 9, 2024 • edited Loading

andrewleech commented Jul 9, 2024 • edited Loading

dpgeorge commented Jul 18, 2024

andrewleech commented Aug 13, 2024

dpgeorge commented Aug 13, 2024

andrewleech commented Aug 13, 2024 • edited Loading

dpgeorge commented Aug 14, 2024

dpgeorge commented Aug 14, 2024

andrewleech commented Oct 26, 2023 •

edited

Loading

andrewleech commented Nov 16, 2023 •

edited

Loading

stinos Nov 16, 2023 •

edited

Loading

codecov bot commented Nov 16, 2023 •

edited

Loading

github-actions bot commented Jul 9, 2024 •

edited

Loading

andrewleech commented Jul 9, 2024 •

edited

Loading

andrewleech commented Aug 13, 2024 •

edited

Loading