Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace __file__ with newer APIs #674

Open
jayvdb opened this issue Nov 11, 2019 · 3 comments
Open

Replace __file__ with newer APIs #674

jayvdb opened this issue Nov 11, 2019 · 3 comments

Comments

@jayvdb
Copy link

jayvdb commented Nov 11, 2019

#533 replaced custom data lookup with __file__, which works only if the resources are exported to a filesystem. See indygreg/PyOxidizer#69 for more info about why, but the tl;dr version is __file__ is an optional attribute and should not be relied upon. It also requires each zip/pack tool to fiddle with __file__.

Here is the current backtrace when __file__ is not provided:

  File "babel", line 20, in <module>
    from babel.core import UnknownLocaleError, Locale, default_locale, \
  File "babel.core", line 14, in <module>
    from babel import localedata
  File "babel.localedata", line 24, in <module>
    _dirname = os.path.join(os.path.dirname(__file__), 'locale-data')
NameError: name '__file__' is not defined

There are stdlib APIs for this type of access, especially the now ancient pkgutil.get_data, and the newer importlib.resources and backport importlib_resources.

Loading of resources from the runtime package should ideally be done using importlib.resources and its backport , however pkgutil could be used to avoid the need for the backport. Using pkgutil doesn't work under PyOxidizer 0.4, however I expect that will be fixed soon.

@akx
Copy link
Member

akx commented Nov 11, 2019

#533 is actually not to "blame" here – the package has been using __file__ to locate locale data since forever (13 years ago and counting – I didn't feel like clicking back through blames further than that).

I wouldn't mind a patch that has Babel attempt to use, say, importlib.resources.open_binary() when available and fall back to the current __file__ behavior, but more drastic changes would have to be Babel 3.x+ (see #569) and probably related to the vaguely discussed custom loader stuff (#454 (comment)).

@akx akx added the area/CLDR label Nov 11, 2019
@jayvdb
Copy link
Author

jayvdb commented Nov 12, 2019

Apologies for seeming to allocate blame. I was trying to say that #533 brought Babel to the point that tools which uses __file__ hacks, like PyInstaller, would work reliably, but the next step is to also work with tools which rely on stdlib.

I wouldn't mind a patch that has Babel attempt to use, say, importlib.resources.open_binary() when available and fall back to the current __file__ behavior

That should be sufficient. I'll try it.

@akx
Copy link
Member

akx commented Nov 12, 2019

Apologies for seeming to allocate blame.

I didn't mean it like that – let me explain:

I was trying to say that #533 brought Babel to the point that tools which uses __file__ hacks, like PyInstaller, would work reliably, but the next step is to also work with tools which rely on stdlib.

#533 reverted some changes to that directory code that had been ultimately unnecessary; PyInstaller had had __file__ hacks/fixups for Babel since time immemorial, and Babel itself has used the same pattern using __file__ since 1.0 and probably before it.

That should be sufficient. I'll try it.

Come to think of it, if importlib.resources doesn't have an API to iterate over resources of a certain kind, that may be a bit of a hurdle. The code for enumerating locales needs that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants