[issue39461] [RFE] os.environ should support Path-like values, like subprocess(..., env=...)

[Ethan Furman]

Change by Ethan Furman :


--
resolution:  -> rejected
stage:  -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39461] [RFE] os.environ should support Path-like values, like subprocess(..., env=...)

[Terry J. Reedy]

Terry J. Reedy  added the comment:

os.environ is initially a copy of the os string-string mapping.  That some 
string values happen to represent file paths is opaque to the mapping.  So, to 
me, looking at os.environ by itself, there is no particular reason to 
autoconvert Path values but not anything else nor to autoconvert values but not 
keys.

If os.environ['key'] = Path('boo') worked, I would expect os.environ['key2'] = 
333 to work.  I would consider a new inconsistency here to be worse than the 
existing one between os.environ and subprocess.Popen(env=...).  It would be OK 
with me if the latter were fixed.

It is not unheard of for CPython to accept objects that conceptually should not 
be.  Some instances have be 'grandparented', others not.

--
nosy: +terry.reedy

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39461] [RFE] os.environ should support Path-like values, like subprocess(..., env=...)

[Ethan Furman]

Ethan Furman  added the comment:

Well, I would prefer if Path objects were seamless to use since at one time 
they were actually strings, but I can live with Brett's rationale that they are 
only seamless where paths are explicitly expected.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39461] [RFE] os.environ should support Path-like values, like subprocess(..., env=...)

[STINNER Victor]

STINNER Victor  added the comment:

I understand that os.fsencode() was modified to support the fspath protocol to 
be consistent with the C implementation PyUnicode_FSConverter() which calls 
PyOS_FSPath().

I agree that at least in C, we need two functions: one which accepts (str, 
bytes), another which also supports the fspath protocol.

IMHO PyUnicode_FSConverter() was modified to support fspath because it was 
convenient to only modify one function. But this change wasn't designed to 
decide in each function if fspath should be accepted or not.

For me, os.putenv() should not accept fspath neither.

This issue is not specific to os.environ. It's more general about the PEP 519 
implementation. IMHO the implementation should be "adjusted".

I like the deprecation idea ;-) We did something similar with functions 
accepting float by mistake. First a deprecation warning was emited, and then it 
became a hard error (exception).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39461] [RFE] os.environ should support Path-like values, like subprocess(..., env=...)

[Raymond Hettinger]

Raymond Hettinger  added the comment:

I don't think proposal this makes sense.  os.environ is a dict-like object 
providing clean access to the environment variables.  Use of Paths is an 
orthogonal problem unrelated to environment variables.

--
nosy: +rhettinger

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39461] [RFE] os.environ should support Path-like values, like subprocess(..., env=...)

[Eryk Sun]

Eryk Sun  added the comment:

> as long as the behavior is *consistent* with the env kwarg to 
> subprocess.run() 

subprocess isn't consistent with itself across platforms. The env parameter in 
Windows is strictly an str->str mapping, like os.environ. This is coded in 
getenvironment in Modules/_winapi.c:

if (! PyUnicode_Check(key) || ! PyUnicode_Check(value)) {
PyErr_SetString(PyExc_TypeError,
"environment can only contain strings");
goto error;
}

At a lower level, should the env parameter of os.spawnve and os.execve  allow 
path-like objects? Should (Unix) os.putenv allow the name and value to be 
path-like?

Currently these cases use PyUnicode_FSConverter and PyUnicode_FSDecoder, which 
support __fspath__ via PyOS_FSPath. PyUnicode_FSDecoder also supports the 
buffer protocol, with a warning. 

Since we have cases like this where the filesystem encoding is used for string 
data that's not actually a file path, should alternate ParseTuple converters be 
added that are limited to just str and bytes? Maybe name them 
PyUnicode_FSStringEncoder and PyUnicode_FSStringDecoder, where "String" 
emphasizes that fspath and buffer objects are not allowed. For example:

int
PyUnicode_FSStringEncoder(PyObject *path, void *addr)
{
PyObject *output = NULL;

if (path == NULL) {
Py_DECREF(*(PyObject **)addr);
*(PyObject **)addr = NULL;
return 1;
}

if (PyBytes_Check(path)) {
output = path;
Py_INCREF(output);
}
else if (PyUnicode_Check(path)) {
output = PyUnicode_EncodeFSDefault(path);
if (!output)
return 0;
}
else {
PyErr_Format(PyExc_TypeError, "path should be str or bytes, not "
"%.200s", _PyType_Name(Py_TYPE(path)));
return 0;
}

if ((size_t)PyBytes_GET_SIZE(output) !=
strlen(PyBytes_AS_STRING(output)))
{
PyErr_SetString(PyExc_ValueError, "embedded null byte");
Py_DECREF(output);
return 0;
}

*(PyObject **)addr = output;
return Py_CLEANUP_SUPPORTED;
}


int
PyUnicode_FSStringDecoder(PyObject *path, void *addr)
{
PyObject *output = NULL;

if (arg == NULL) {
Py_DECREF(*(PyObject **)addr);
*(PyObject **)addr = NULL;
return 1;
}

if (PyUnicode_Check(path)) {
output = path;
Py_INCREF(output);
}
else if (PyBytes_Check(path)) {
output = PyUnicode_DecodeFSDefaultAndSize(PyBytes_AS_STRING(path),
PyBytes_GET_SIZE(path));
if (!output)
return 0;
}
else {
PyErr_Format(PyExc_TypeError, "path should be str or bytes, not "
"%.200s", _PyType_Name(Py_TYPE(path)));
return 0;
}

if (PyUnicode_READY(output) == -1) {
Py_DECREF(output);
return 0;
}

if (findchar(PyUnicode_DATA(output), PyUnicode_KIND(output),
 PyUnicode_GET_LENGTH(output), 0, 1) >= 0) {
PyErr_SetString(PyExc_ValueError, "embedded null character");
Py_DECREF(output);
return 0;
}

*(PyObject **)addr = output;
return Py_CLEANUP_SUPPORTED;
}

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39461] [RFE] os.environ should support Path-like values, like subprocess(..., env=...)

[Antony Lee]

Antony Lee  added the comment:

FWIW, I'm actually fine with not supporting Path objects in os.environ, as long 
as the behavior is *consistent* with the env kwarg to subprocess.run() -- note 
that the original title of the thread only pointed out to the inconsistency, 
and did not strongly request that os.environ support Paths.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39461] [RFE] os.environ should support Path-like values, like subprocess(..., env=...)

[Brett Cannon]

Brett Cannon  added the comment:

> but now it's too change to change it again :-)

I actually don't think it is if we want to revert this. We can raise a 
deprecation warning if the call to os.fsencode() leads to a value different 
than its argument and say that people are expected to handle conversions 
themselves.

> The idea behind PEP 519 was to alleviate str(path_obj) calls between the 
> os/program interface

... where file paths were explicitly expected. os.environ is not a place where 
file paths are explicitly expected, just a place where they _might_ end up. 
Basically I only consider PEP 519 valid in places where file paths are the only 
thing that are expected (e.g. all the path manipulation functions in os.path). 
Everywhere else where paths are only a "maybe" I would say it shouldn't be 
relied upon to do implicit conversions.

> Out of curiosity, any idea how often non-strings are used to set os.environ, 
> and so need explicit conversion?

I have anecdotal evidence from outside of Python that people will do this when 
they can and then are surprised when the conversion doesn't work the way they 
expect (e.g. floats).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39461] [RFE] os.environ should support Path-like values, like subprocess(..., env=...)

[Ethan Furman]

Ethan Furman  added the comment:

The idea behind PEP 519 was to alleviate str(path_obj) calls between the 
os/program interface.  We can either make that as consistent as we can as we 
find places that still require the str(path_obj) idiom, or we can make users 
remember which ones do, and which ones don't, easily use a Path object.

In my opinion, having to remember is a very unpleasant and frustrating user 
experience.

Serhiy Storchaka:

> Path-like objects are now unintentionally accepted for many non-path
> things, like hostname in socket.sethostname() and username in
> os.initgroups(). I also think it was a mistake, and we should not
> make new mistakes.

There will always be ways for users to make mistakes when using API's.  Whether 
they passed a Path path to os.sethostname() or a string path to 
os.sethostbyname(), it's still their bug, and linters/type-checkers exist to 
help catch those kinds of bugs.

Brett Canon:
---
> Environment variables are strings, period, so they should be specified
> as such;

Out of curiosity, any idea how often non-strings are used to set os.environ, 
and so need explicit conversion?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39461] [RFE] os.environ should support Path-like values, like subprocess(..., env=...)

[Eryk Sun]

Eryk Sun  added the comment:

> I honestly would have disagreed with the Popen change for its 'env' 
> argument or any other place that is dealing with environment
> variables

os.spawnve (Windows) and os.execve support __fspath__ for the env  dict 
partially due to me (bpo-28114), so sorry if that was the wrong decision. 
However, at least on the POSIX side, I was restoring previous behavior that 
aligned with POSIX os.putenv. An example of what's supported currently:

Windows:

>>> path = os.environ['comspec']
>>> argv = ['cmd', '/c', 'echo %SPAM%']
>>> env = os.environ.copy()
>>> env[Path('SPAM')] = Path('eggs')
>>> os.spawnve(0, path, argv, env)
eggs
0

POSIX:

>>> path = '/bin/bash'
>>> argv = ['bash', '-c', 'echo $SPAM']
>>> env = os.environ.copy()
>>> env[Path('SPAM')] = Path('eggs')
>>> os.spawnve(0, path, argv, env)
eggs
0

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39461] [RFE] os.environ should support Path-like values, like subprocess(..., env=...)

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Path-like objects are now unintentionally accepted for many non-path things, 
like hostname in socket.sethostname() and username in os.initgroups(). I also 
think it was a mistake, and we should not make new mistakes.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39461] [RFE] os.environ should support Path-like values, like subprocess(..., env=...)

[STINNER Victor]

STINNER Victor  added the comment:

> I don't think this should be done (and I honestly would have disagreed with 
> the Popen change for its 'env' argument or any other place that is dealing 
> with environment variables). Environment variables are strings, period, so 
> they should be specified as such; explicit is better than implicit (and there 
> isn't enough pragmatic benefit to override this in my opinion).

I don't think that subprocess accepts fspath on purpose. It uses fsencode() 
because it's convenient to encode Unicode to the filesystem encoding using the 
right error handler.

It's a side effect of the commit c1cbeedf0c650c3f7c64f04479070d39e15e1baf. In 
fact, I'm opposed to this commit, but now it's too change to change it again :-)

os.putenv() is unrelated to os.environ. Calling os.putenv() does not update 
os.environ, whereas setting a key in os.environ calls internally os.putenv().

I like the idea of keeping os.environ as simple as a dict mapping strings to 
trings (in term of API, internally it's more complicated).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39461] [RFE] os.environ should support Path-like values, like subprocess(..., env=...)

[Brett Cannon]

Brett Cannon  added the comment:

I don't think this should be done (and I honestly would have disagreed with the 
Popen change for its 'env' argument or any other place that is dealing with 
environment variables). Environment variables are strings, period, so they 
should be specified as such; explicit is better than implicit (and there isn't 
enough pragmatic benefit to override this in my opinion).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39461] [RFE] os.environ should support Path-like values, like subprocess(..., env=...)

[Eryk Sun]

Eryk Sun  added the comment:

Supporting __fspath__ for os.environ[b] makes it consistent with POSIX 
os.putenv, which already supports it via PyUnicode_FSConverter. For example:

os.putenv(Path('spam'), Path('eggs'))
getenv = ctypes.CDLL('libc.so.6').getenv
getenv.restype = ctypes.c_char_p

>>> getenv(b'spam')
b'eggs'

For symmetry, os.putenv in Windows could be updated to support __fspath__. An 
old patch for bpo-28188 implements this, along with bytes support, via 
PyUnicode_FSDecoder. But bytes support would be limited to UTF-8 strings, so it 
wouldn't be as flexible as os.environb in POSIX, which supports arbitrary bytes 
except for embedded nulls.

--
nosy: +eryksun

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39461] [RFE] os.environ should support Path-like values, like subprocess(..., env=...)

[Ethan Furman]

Ethan Furman  added the comment:

True, but this example of implicit conversion is only for Path objects which 
are currently implicitly converted throughout the stdlib where appropriate, and 
this looks like one of those appropriate places.

I don't know enough about os.environb to offer an opinion on auto-conversion 
there.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39461] [RFE] os.environ should support Path-like values, like subprocess(..., env=...)

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

Implicit conversion can also hide some bugs.

Semantically os.environ is an str to str mapping and os.environb is a bytes to 
bytes mapping.

--
nosy: +serhiy.storchaka

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39461] [RFE] os.environ should support Path-like values, like subprocess(..., env=...)

[Ethan Furman]

Ethan Furman  added the comment:

True, but so is having Path objects not seemlessly usable.

Also, isn't os.environ a special case where all values should be strings?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39461] [RFE] os.environ should support Path-like values, like subprocess(..., env=...)

[STINNER Victor]

STINNER Victor  added the comment:

Currently, os.environ behaves as a dictionary. When you put value into 
os.environ[key], os.environ[key] gives you back this value. If we start to 
convert value to a different type (convert something to str), it can be 
surprising.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39461] [RFE] os.environ should support Path-like values, like subprocess(..., env=...)

[Ethan Furman]

Ethan Furman  added the comment:

Adding `os.environ` support makes sense to me.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39461] [RFE] os.environ should support Path-like values, like subprocess(..., env=...)

[STINNER Victor]

Change by STINNER Victor :


--
title: os.environ does not support Path-like values, but subprocess(..., 
env=...) does -> [RFE] os.environ should support Path-like values, like 
subprocess(..., env=...)
type:  -> enhancement

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com