Repository navigation
PyImport_GetMagicTag() should use the same const char * as sys.implementation.cache_tag #59447
Description
| BPO | 15242 |
|---|---|
| Nosy | @brettcannon, @amauryfa, @larryhastings, @ericsnowcurrently |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = <Date 2012-07-10.05:21:02.363>
created_at = <Date 2012-07-03.06:54:06.919>
labels = ['interpreter-core', 'type-bug']
title = 'PyImport_GetMagicTag() should use the same const char * as sys.implementation.cache_tag'
updated_at = <Date 2012-07-10.05:21:02.363>
user = 'https://github.com/ericsnowcurrently'bugs.python.org fields:
activity = <Date 2012-07-10.05:21:02.363>
actor = 'eric.snow'
assignee = 'none'
closed = True
closed_date = <Date 2012-07-10.05:21:02.363>
closer = 'eric.snow'
components = ['Interpreter Core']
creation = <Date 2012-07-03.06:54:06.919>
creator = 'eric.snow'
dependencies = []
files = ['26237', '26281']
hgrepos = []
issue_num = 15242
keywords = ['patch']
message_count = 10.0
messages = ['164582', '164587', '164631', '164675', '164683', '164783', '164785', '165104', '165105', '165111']
nosy_count = 6.0
nosy_names = ['brett.cannon', 'amaury.forgeotdarc', 'larry', 'Arfrever', 'python-dev', 'eric.snow']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue15242'
versions = ['Python 3.3']
Activity
ericsnowcurrently commentedon Jul 3, 2012
There was some concern with PyImport_GetMagicTag() extracting its value from sys.implementation.cache_tag. One solution is to have the two use a common underlying value. However, that's basically what was already in import.c and I'd rather minimize that file at this point.
An alternative is to have the underlying value defined in a more amenable spot, but I'm not convinced that buys a whole lot over simply exposing the underlying value in PyImport_GetMagicTag(). If we could deprecate that function...
Another possibility is to move the underlying value to configure. The down-side is that it's platform-specific. I've attached a patch that does this. While it may do too much as-is, it demonstrates my point.
amauryfa commentedon Jul 3, 2012
Can the #defines appear in pyconfig.h instead? I find it easier to discover them this way, and will also simplify the implementation on Windows.
ericsnowcurrently commentedon Jul 4, 2012
Agreed. I'd like to pull them into Python/sysmodule.h, though. I'm also going to drop the part that messes with SOABI.
larryhastings commentedon Jul 5, 2012
Sorry for the pedantry, but: I read the title of this bug as wanting these two values to be the exact same pointer. If you're talking about putting a constant string in a header file, you'll get an (identical) copy of that string in every .o that references it (unless your linker collapses identical strings, which I don't know if they do).
From the description, it sounds like you want them to merely be guaranteed identical strings, but they don't have to be the same exact pointer.
tl;dr: do you mean "=="? the title implies you mean "is".
brettcannon commentedon Jul 5, 2012
tl;dr: "==", not "is"
Shouldn't tl;dr go first, else it seems a little pointless since I already read the whole thing. At which point the tl;dr is really just a summary of what I just read, not a "didn't read" line. Just being pedantic. =)
ericsnowcurrently commentedon Jul 7, 2012
Here's a new patch that keeps things simpler, but still cleans up the concerns about using sys.implementation.cache_tag in PyImport_GetMagicTag().
ericsnowcurrently commentedon Jul 7, 2012
Patch updated to include a doc addition.
python-dev commentedon Jul 9, 2012
New changeset ee01fd98b5b0 by Brett Cannon in branch 'default':
Issue bpo-15242: Have PyImport_GetMagicTag() return a const char *
http://hg.python.org/cpython/rev/ee01fd98b5b0
brettcannon commentedon Jul 9, 2012
I went ahead and committed Eric's patch. Amaury, if you want to move the macros to a header file I see no reason not to, but I also don't see a need so I didn't want to spend the time doing it myself.
amauryfa commentedon Jul 9, 2012
It's not necessary. The fix looks good as is.