bpo-31904: Add posix module support for VxWorks RTOS by pxinwr · Pull Request #12118 · python/cpython

pxinwr · 2019-03-01T10:07:20Z

This is the successive PR after #11968. This PR enables posix module support for VxWorks RTOS. VxWorks doesn't support fork()/exec(). Instead, we use rtpSpawn() to spawn new processes.
More and full support on modules for VxWorks will continuously be added by the coming PRs.
VxWorks is a product developed and owned by Wind River. For VxWorks introduction or more details, go to https://www.windriver.com/products/vxworks/
Wind River will have a dedicated engineering team to contribute to the support as maintainers.
We already have a working buildbot worker internally, but has not bound to master. We will check the process for the buildbot, then add it.

https://bugs.python.org/issue31904

vstinner · 2019-03-01T15:30:44Z

@pablogsal @izbyshev @nanjekyejoannah: Would you min to review this change?

izbyshev

Is there any plan to update documentation to reflect that some functions that are normally available on Unix are not actually available on VxWorks? With this patch, they are os.fork, os.exec* and os.spawn*p*.

pxinwr · 2019-03-08T04:12:38Z

@izbyshev

Is there any plan to update documentation to reflect that some functions that are normally available on Unix are not actually available on VxWorks? With this patch, they are os.fork, os.exec* and os.spawn*p*.

Have updated os.rst about this.

pxinwr · 2019-03-11T07:12:18Z

@izbyshev @pablogsal I've fixed all the issues reviewed out. Could you help to review again when available? Thanks a lot.

vstinner · 2019-04-16T08:50:34Z

See also #12319 " bpo-31904: Fix test_os failures for VxWorks RTOS #12319 ".

… test_os.py

pxinwr · 2019-05-17T09:39:53Z

I've combined #12319 into this PR.

vstinner

Your change seems reasonable, but I have a few comments.

vstinner · 2019-05-17T09:37:25Z

@@ -1419,9 +1419,20 @@ def test_urandom_failure(self):
            import errno
            import os
            import resource


I dislike this code. If the limit is high (ex: larger than 1024), the test can take a lot of time.

I suggest to simply skip the test on VxWorks.

Following your comments, skip this test on VxWorks. By default the RLIMIT_NOFILE on VxWorks is 8. But on other OS, perhaps there will be the issues you concerned.

vstinner · 2019-05-17T09:37:47Z

+            else:
+                pass
            try:
                os.urandom(16)


nitpick: please align the string to the start of hasattr.

vstinner · 2019-05-17T09:40:13Z

+   On VxWorks, if *mode* is :const:`P_NOWAIT`, this function returns the process id
+   of the new process; if *mode* is :const:`P_WAIT`, only returns the new process's
+   exit code if it exits normally. The process id will actually be the process
+   handle, so can be used with the :func:`waitpid` function.


This description seems to be redundant with the previous paragraph. I don't see how it's different from the previous paragraph. And it seems obvious to that the a process is can be passed to waitpid(), I don't see the point of saying it here.

Really some confusion here. The difference we want to express here is in P_WAIT mode if the process is killed by others, VxWorks doesn't return "-signal" as unix-like OS does. VxWorks just returns -1 in this case.

vstinner · 2019-05-17T09:42:21Z

+     if ((rtpid != RTP_ID_ERROR) && (mode == _P_WAIT)) {
+         do {
+             res = waitpid((pid_t)rtpid, &status, 0);
+         } while (res < 0 && errno == EINTR);


You have to call PyErr_CheckSignals() on EINTR to respect the PEP 475. See how other functions implement that.

vstinner · 2019-05-17T09:47:25Z

I've combined #12319 into this PR.

Thanks, I like that :-)

vstinner

Your PR mostly LGTM, but I have a few more comments before being able to approve it.

vstinner · 2019-05-17T12:51:14Z

@@ -0,0 +1 @@
+Skip ExecTests and URandomFDTests in test_os.py for VxWorks.


Honestly, I don't think that this NEWS entry is really helpful to users. Can you please remove it?

vstinner · 2019-05-21T09:33:04Z

   process.  On Windows, the process id will actually be the process handle, so can
   be used with the :func:`waitpid` function.

+   Note on VxWorks, this function desn't return ``-signal`` when the new process is


typo!

Suggested change

Note on VxWorks, this function desn't return ``-signal`` when the new process is

Note on VxWorks, this function doesn't return ``-signal`` when the new process is

vstinner · 2019-05-21T09:33:40Z


-
+@unittest.skipUnless(hasattr(os, 'execv'),
+                     "No os.execv or os.execve function to test.")


nitpick: usually, we just say "need os.execv", no need to elaborate ;-)

vstinner · 2019-05-21T09:35:51Z

LGTM except if the typo. Please fix the typo, so I can merge your PR ;-)

vstinner · 2019-05-21T10:47:15Z

Thanks @pxinwr! I know that this one was painful, but well, the os ("posix") module is where you can find most differences between operating systems.

pxinwr added 4 commits March 1, 2019 17:49

support posix module on VxWorks

cce75ba

update clinic for Modules/clinic/posixmodule.c.h

f548045

Run autoreconf

b41b4d0

add news file

3462991

the-knights-who-say-ni added the CLA signed label Mar 1, 2019

bedevere-bot added the awaiting review label Mar 1, 2019

izbyshev reviewed Mar 2, 2019

View reviewed changes

Comment thread Modules/posixmodule.c Resolved

Comment thread Modules/posixmodule.c Resolved

Comment thread Modules/posixmodule.c Resolved

izbyshev reviewed Mar 2, 2019

View reviewed changes

Comment thread Modules/posixmodule.c Resolved

pablogsal self-requested a review March 3, 2019 20:59

yuesun1 and others added 3 commits March 6, 2019 16:30

bpo-31904: Add posix module support for VxWorks RTOS python#12118

8de1134

add the period mark in the os.rst

7354c9f

revert aclocal.m4

8668612

tune the lines in os.rst

6f8e048

yuesun1 mentioned this pull request Mar 14, 2019

bpo-31904: Fix test_os failures for VxWorks RTOS #12319

Closed

pxinwr added 2 commits March 18, 2019 14:06

Merge remote-tracking branch 'upstream/master' into fix-issue-31904-os

961f435

update Modules/clinic/posixmodule.c.h

86091ad

vstinner reviewed Mar 18, 2019

View reviewed changes

Comment thread Doc/library/os.rst Resolved

Fix os.rst for spawnv() and spawnve() in OS module

5ea0755

pxinwr and others added 3 commits May 17, 2019 17:07

Merge remote-tracking branch 'upstream/master' into fix-issue-31904-os

30e9049

improve the test_urandom_failure and skip the ExecTests test cases in…

cf1a844

… test_os.py

Add news file

728165f

update clinic for Modules/clinic/posixmodule.c.h

36e955f

vstinner reviewed May 17, 2019

View reviewed changes

skip URandomFDTests in test_os.py for VxWorks

3716550

pxinwr added 3 commits May 17, 2019 19:31

call PyErr_CheckSignals() on EINTR to respect the PEP 475 for waitpid

c14d326

rephrase VxWorks' behaviour for spawn-like methods

a508f47

improve Doc/library/os.rst for VxWorks special behaviour

201212d

vstinner reviewed May 17, 2019

View reviewed changes

pxinwr added 2 commits May 20, 2019 10:15

remove the separate NEWS entry for test part

da73b91

add comments for calling rtpSpawn()

b380c49

vstinner reviewed May 21, 2019

View reviewed changes

pxinwr added 2 commits May 21, 2019 18:21

fix the typo in Doc/library/os.rst

7ada2ad

refine the skip message for ExecTests

eb063a8

vstinner merged commit f2d7ac7 into python:master May 21, 2019

bedevere-bot removed the awaiting review label May 21, 2019

pxinwr deleted the fix-issue-31904-os branch July 12, 2021 09:42

kuhlenough mannequin mentioned this pull request Jan 12, 2024

Python should support VxWorks RTOS #76085

Closed

		@@ -0,0 +1 @@
		Skip ExecTests and URandomFDTests in test_os.py for VxWorks.

	Note on VxWorks, this function desn't return ``-signal`` when the new process is
	Note on VxWorks, this function doesn't return ``-signal`` when the new process is



		@unittest.skipUnless(hasattr(os, 'execv'),
		"No os.execv or os.execve function to test.")

Conversation

pxinwr commented Mar 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vstinner commented Mar 1, 2019

Uh oh!

izbyshev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pxinwr commented Mar 8, 2019

Uh oh!

pxinwr commented Mar 11, 2019

Uh oh!

Uh oh!

vstinner commented Apr 16, 2019 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pxinwr commented May 17, 2019

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vstinner commented May 17, 2019

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vstinner commented May 21, 2019

Uh oh!

vstinner commented May 21, 2019

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

pxinwr commented Mar 1, 2019 •

edited

Loading

vstinner commented Apr 16, 2019 •

edited by bedevere-bot

Loading