Skip to content

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented Jun 19, 2024

Soft deprecate os.popen(), os.spawn*() and os.system() functions.


📚 Documentation preview 📚: https://cpython-previews--120744.org.readthedocs.build/

Soft deprecate os.popen(), os.spawn*() and os.system() functions.
@vstinner
Copy link
Member Author

cc @gpshead

@zooba
Copy link
Member

zooba commented Jun 24, 2024

I don't like system being a deprecation. I have no concerns about recommending people use subprocess.run or equivalent if they're unsure, but there's no harm in os.system being used. If there were problems, we'd have heard reports of them by now.

Comment on lines 4995 to 4998
.. deprecated:: 3.14
The function is :term:`soft deprecated` and should no longer be used to
write new code. The :mod:`subprocess` module is recommended instead.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.. deprecated:: 3.14
The function is :term:`soft deprecated` and should no longer be used to
write new code. The :mod:`subprocess` module is recommended instead.

I don't like this one

@gpshead
Copy link
Member

gpshead commented Jun 24, 2024

but there's no harm in os.system being used. If there were problems, we'd have heard reports of them by now.

Anything silently using a shell opens the world up for problems that are not obvious to many code authors. Shell injection, quoting hell, spaces, etc. It is fair to say that the world has heard so much about that over the decades that many of us don't even notice that screaming noise anymore.

this is just a soft deprecation in the docs to point people towards the well lit path of subprocess.run().

@zooba
Copy link
Member

zooba commented Jun 24, 2024

Anything silently using a shell opens the world up for problems that are not obvious to many code authors.

If you can recognise the implications of shell=True but not os.system, then I don't think there's anything we can do on our side short of removing both.

There isn't a case where they behave featurefully different, right? Like, os.system isn't going to choose not to create a shell in some cases?

@adorilson
Copy link
Contributor

Is it time to add a new Superseded functions section to the Superseded page and put these functions there, or is this restricted to modules?

@vstinner
Copy link
Member Author

vstinner commented Jul 1, 2024

I removed os.system() deprecation. We can revisit its deprecation once subprocess.shell() will be added or not: #121093

I kept the non-controversial deprecations.

@vstinner vstinner merged commit d44c550 into python:main Jul 1, 2024
@vstinner vstinner deleted the soft_depr_os branch July 1, 2024 16:27
@vstinner
Copy link
Member Author

vstinner commented Jul 1, 2024

Is it time to add a new Superseded functions section to the Superseded page and put these functions there, or is this restricted to modules?

It seems like a huge maintenance burden to maintain such hypothetical (long) list in the documentation. I don't think that it's worth it, but you can open an issue if you disagree and consider that it's a good idea :-)

Akasurde pushed a commit to Akasurde/cpython that referenced this pull request Jul 3, 2024
Soft deprecate os.popen() and os.spawn*() functions.
noahbkim pushed a commit to hudson-trading/cpython that referenced this pull request Jul 11, 2024
Soft deprecate os.popen() and os.spawn*() functions.
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
Soft deprecate os.popen() and os.spawn*() functions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation in the Doc dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants