gh-127405: Add ABIFLAGS to sysconfig.get_config_vars() on Windows
#131799
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Add ``ABIFLAGS`` to :func:`sysconfig.get_config_vars` on Windows. Patch by Xuehai Pan. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,6 +55,23 @@ _sysconfig_config_vars_impl(PyObject *module) | |
| Py_DECREF(config); | ||
| return NULL; | ||
| } | ||
|
|
||
| // On Unix, the `ABIFLAGS` key is defined via a different logic. | ||
| // | ||
| // Emulate `sys.abiflags` value on Unix for Windows. ABIFLAGS here is only | ||
| // an emulated value. It is not present during build on Windows. | ||
| if (add_string_value(config, "ABIFLAGS", | ||
| # ifdef Py_GIL_DISABLED | ||
| "t" | ||
| # endif | ||
| # ifdef _DEBUG | ||
| "d" | ||
|
There was a problem hiding this comment. I think making this There was a problem hiding this comment.
Can't we use something like this? def add_underscore_d(x):
if os.name == 'nt':
return x.replace('d', '_d')
return x
executable = f'python{version}{add_underscore_d(ABIFLAGS)}.exe'Explicit is better than implicit. There was a problem hiding this comment. It isn't "we" - it's "every single user ever for the rest of time". Our job as core maintainers is to do more work and make less obvious decisions so that every single user doesn't have to. Don't bother quoting the zen at me either. "Special cases aren't special enough" applies just as well here, as does "one obvious way" (which is to use If you want to run with the "explicit > implicit" argument, here's a more explicit version: Inserting an underscore is neither explicit, nor obvious, and is definitely a special case that isn't special enough to not just use a There was a problem hiding this comment. I added the underscore prefix in the Python code so that developers can introspect the source more easily.
Contributor
Author
There was a problem hiding this comment. A constraint of the There was a problem hiding this comment.
Users can't split up the value anyway, that's not what it's for. You can parse it specifically, which basically amounts to an There was a problem hiding this comment. I still don't understand why this definition needs to be in the native module, instead of generating it in the Python code. There was a problem hiding this comment. Ideally, as you say, all these variables should be defined in On Windows, we don't parse If not set in So basically, tier 2 is closer to ideal than tier 3 would be, and importantly it makes it clearer that this is exposing an internally defined value, rather than trying to adhere to some kind of definition of what Footnotes
|
||
| # endif | ||
| "") | ||
| < 0) { | ||
| Py_DECREF(config); | ||
| return NULL; | ||
| } | ||
|
There was a problem hiding this comment. Instead of setting There was a problem hiding this comment. Agree with adding We don't need or use it, though, so I don't see an issue with constructing it here. But I'd rather have build variables be closer to the build rather than further away. There was a problem hiding this comment. IMO, But ignoring that, I think it would be more maintainable to have all variables that need construction in the same place. Having part of them here, and part of them in the Python module should be avoidable if we can. Especially for Windows specific variables emulating POSIX ones, There was a problem hiding this comment. I do not have a strong preference for whether the code should live in Python or C. Both are fine for me. There was a problem hiding this comment.
I move the definition from C to Python in the latest commit. I can either keep it or revert it if we reach a consensus. |
||
| #endif | ||
|
|
||
| #ifdef Py_GIL_DISABLED | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.