classification
Title: Expose private functions in marshal used by importlib
Type: enhancement Stage: needs patch
Components: Library (Lib) Versions: Python 3.4
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: brett.cannon, eric.snow, loewis, pitrou, terry.reedy
Priority: normal Keywords:

Created on 2012-10-12 15:13 by brett.cannon, last changed 2012-11-17 16:07 by brett.cannon. This issue is now closed.

Messages (11)
msg172751 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-10-12 15:13
IOW make _w_long, _r_long, and __fix_co_filename public so as to not be some special ability that only importlib has.
msg172754 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-10-12 16:00
Why is it a problem for importlib to use internal APIs?
I don't think support these low-level APIs as public helps anyone.
msg172757 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-10-12 16:23
Well, it means importlib becomes a special library and that no one could ever replicate it as a third-party library.

Now if we can expose the various APIs around this such that they are abstracted away then it isn't a big deal. That can be done for the _r_long and _w_long, but _fix_co_filename is still a rather special thing.
msg172758 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-10-12 16:27
Le vendredi 12 octobre 2012 à 16:23 +0000, Brett Cannon a écrit :
> Well, it means importlib becomes a special library and that no one
> could ever replicate it as a third-party library.

Well, it *is* a special library. It is tightly integrated with the
interpreter core, and it provides a fundamental service in the language.

Besides, since the pyc format is an implementation detail, I don't see
how exposing r_long, w_long and friends would help third-party
libraries, which would still have to rely on non-public details.
msg172763 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-10-12 17:02
http://bugs.python.org/issue15031 would deal with not needing to expose _r_long and _w_long, but that still means people are screwed for _fix_co_filename. You could argue that is a margin use-case, though.
msg172771 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2012-10-12 19:19
I don't understand this issue at all:

a) _bootstrap does not currently use any private API of marshal. Instead, it has functions _w_long and _r_long implemented in pure Python. So where is the special functionality that only importlib has? Anybody could easily replicate these functions.

b) Isn't it easy to implement it as such:

def _w_long(x):
  return x.to_bytes(4, 'little')

As for fix_co_filename, I think it would indeed be useful if marshal.load(s) supported an optional filename= parameter, which then fills rf.current_filename. It's better to load it into the correct form in the first place instead of fixing it after loading completed - in particular since marshal already has a mechanism to update all filenames.
msg172774 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-10-12 19:40
_w_long and _r_long originally came from marshal; forgot I re-implemented them in pure Python in the end.
msg172779 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-10-12 21:00
marshal currently has a simple, coherent, high-level interface: dump and load serializations.

_fix_co_filename sounds a bit hackish and ad hoc. Martin's suggestion looks to me like a better way to publicly expose filename setting.
Making a function public indefinitely freezes the api and behavior, so we need to consider if we might want to change it.

If Martin is correct as to the definition of _w_long, its a trivial one line convenience function that is not particularly specific to marshal and that could have been inlined in the code (and for all I know, is, when C compiled). Its existence in marshal really seems to me like an implementation detail that should remain private.
msg172781 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-10-12 21:06
> As for fix_co_filename, I think it would indeed be useful if
> marshal.load(s) supported an optional filename= parameter, which then
> fills rf.current_filename.

AFAIR, the problem is that you first need to import the locale module (and its dependencies) before you can decide what the filesystem encoding is. Until that, you aren't sure the filenames have been decoded correctly.
msg172789 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2012-10-12 22:33
> AFAIR, the problem is that you first need to import the locale  
> module (and its dependencies) before you can decide what the  
> filesystem encoding is.

I don't think this is true. The file system encoding works fine all along,
and also doesn't have to do much with co_filename.
msg175765 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2012-11-17 16:07
I've decided against this as issues #15031 and #16494 will alleviate the need to expose these functions.
History
Date User Action Args
2012-11-17 16:07:08brett.cannonsetstatus: open -> closed
resolution: rejected
dependencies: - Split .pyc parsing from module loading
messages: + msg175765
2012-11-13 06:49:21eric.snowsetnosy: + eric.snow
2012-10-12 22:33:51loewissetmessages: + msg172789
2012-10-12 21:06:43pitrousetmessages: + msg172781
2012-10-12 21:00:59terry.reedysetnosy: + terry.reedy
messages: + msg172779
2012-10-12 19:40:24brett.cannonsetmessages: + msg172774
2012-10-12 19:19:07loewissetnosy: + loewis
messages: + msg172771
2012-10-12 17:02:52brett.cannonsetdependencies: + Split .pyc parsing from module loading
messages: + msg172763
2012-10-12 16:27:18pitrousetmessages: + msg172758
2012-10-12 16:23:49brett.cannonsetmessages: + msg172757
2012-10-12 16:00:02pitrousetnosy: + pitrou
messages: + msg172754
2012-10-12 15:13:35brett.cannoncreate