This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: PyImport_GetMagicTag() should use the same const char * as sys.implementation.cache_tag
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, amaury.forgeotdarc, brett.cannon, eric.snow, larry, python-dev
Priority: normal Keywords: patch

Created on 2012-07-03 06:54 by eric.snow, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
cache_tag_via_configure.diff eric.snow, 2012-07-03 06:54 review
cache_tag_via_sys_var.diff eric.snow, 2012-07-07 04:44 review
Messages (10)
msg164582 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2012-07-03 06:54
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.
msg164587 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2012-07-03 07:54
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.
msg164631 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2012-07-04 01:43
> 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.

Agreed.  I'd like to pull them into Python/sysmodule.h, though.  I'm also going to drop the part that messes with SOABI.
msg164675 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2012-07-05 07:38
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".
msg164683 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-07-05 15:41
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. =)
msg164783 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2012-07-07 04:30
Here's a new patch that keeps things simpler, but still cleans up the concerns about using sys.implementation.cache_tag in PyImport_GetMagicTag().
msg164785 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2012-07-07 04:44
Patch updated to include a doc addition.
msg165104 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-07-09 18:22
New changeset ee01fd98b5b0 by Brett Cannon in branch 'default':
Issue #15242: Have PyImport_GetMagicTag() return a const char *
http://hg.python.org/cpython/rev/ee01fd98b5b0
msg165105 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-07-09 18:23
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.
msg165111 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2012-07-09 19:18
It's not necessary.  The fix looks good as is.
History
Date User Action Args
2022-04-11 14:57:32adminsetgithub: 59447
2012-07-10 05:21:02eric.snowsetstatus: open -> closed
stage: patch review -> resolved
2012-07-09 19:18:42amaury.forgeotdarcsetmessages: + msg165111
2012-07-09 18:23:24brett.cannonsetresolution: fixed
messages: + msg165105
2012-07-09 18:22:19python-devsetnosy: + python-dev
messages: + msg165104
2012-07-07 04:44:37eric.snowsetfiles: + cache_tag_via_sys_var.diff

messages: + msg164785
2012-07-07 04:44:01eric.snowsetfiles: - cache_tag_via_sys_var.diff
2012-07-07 04:30:45eric.snowsetfiles: + cache_tag_via_sys_var.diff

messages: + msg164783
2012-07-05 15:41:56brett.cannonsetmessages: + msg164683
2012-07-05 07:38:06larrysetnosy: + larry
messages: + msg164675
2012-07-04 23:20:56Arfreversetnosy: + Arfrever
2012-07-04 01:43:38eric.snowsetmessages: + msg164631
2012-07-03 07:54:32amaury.forgeotdarcsetmessages: + msg164587
2012-07-03 06:54:07eric.snowcreate