Skip to content

Fix issue in inferred caller when resourcse package has no source. - #314

Merged
jaraco merged 7 commits into
mainfrom
gh-123085/inferred-compiled
Aug 17, 2024
Merged

Fix issue in inferred caller when resourcse package has no source.#314
jaraco merged 7 commits into
mainfrom
gh-123085/inferred-compiled

Conversation

@jaraco

@jaracojaraco commented Aug 17, 2024

Copy link
Copy Markdown
Member

Closespython/cpython#123085

  • gh-121735: Fix inferring caller when resolving importlib.resources.files()
  • Adapt changes for new fixtures.
  • Extract a function for computing 'this filename' once.
  • Just do a simple textual substitution to avoid under matching when the file is compiled.

@jaraco

Copy link
Copy Markdown
MemberAuthor

@Gatsik I've cherry-picked your commit here and adapted it for the recent changes and to work with importlib_resources, since I can iterate much faster in this codebase. I've added some additional commits, one which explored extracting a function for resolving the filename, and the last one which explores just doing a textual substitution on __file__. This last option seems the simplest and does allow the tests to pass. What's your opinion?

@Gatsik

Copy link
Copy Markdown
Contributor

@jaraco Great! Thanks for including me as an author of one of the commits!

My concern is that compiled python files 'freeze' their source filename, and this 'frozen' filename is referenced in frame, but not in __file__. Thus, if 'source' and 'compiled' directories of importlib differ, then simple string replacement won't work

Reproducer, which fails with textual substitution:

importimportlibimportosimportpathlibimportpy_compileimportshutilimportsysimporttempfileimporttextwrapdefcompile(tempdir):
orig_importlib=pathlib.Path(importlib.__file__).parentsource_dir=pathlib.Path(tempdir) /"source_importlib"target_dir=pathlib.Path(tempdir) /'cimportlib'shutil.copytree(orig_importlib, source_dir, ignore=lambda*_: ['__pycache__'])
fordirpath, _, filenamesinos.walk(source_dir):
forfilenameinfilenames:
source_path=pathlib.Path(dirpath) /filenamecfilename=source_path.with_suffix('.pyc').namecfile=target_dir/pathlib.Path(dirpath).name/cfilenamepy_compile.compile(source_path, cfile)
defcreate_package(tempdir):
package_dir=pathlib.Path(tempdir) /'somepkg'package_dir.mkdir()
contents= {
"__init__.py": textwrap.dedent(
""" import cimportlib.resources as res val = res.files().joinpath('resource.txt').read_text(encoding='utf-8') """
),
"resource.txt": "data",
}
forfile, contentincontents.items():
path=pathlib.Path(package_dir) /filepath.write_text(content)
defmain():
withtempfile.TemporaryDirectory() astempdir:
compile(tempdir)
create_package(tempdir)
sys.path.insert(0, str(tempdir))
print(importlib.import_module('somepkg').val)
if__name__=="__main__":
raiseSystemExit(main())

@jaraco
jaracoforce-pushed the gh-123085/inferred-compiled branch from 792da5a to 4ea81bfCompareAugust 17, 2024 11:40
@jaraco

Copy link
Copy Markdown
MemberAuthor

Thanks for clarifying. That makes sense. I've pushed that last commit to refs/archive/314-text-sub and force-pushed the previous commit (retaining the frame-based check).

@jaraco

Copy link
Copy Markdown
MemberAuthor

The next thing I need to determine - is this a bugfix or a new feature. On one hand, it's the correction of a behavior that was presumed to work broadly but didn't. On the other hand, it's expanding the scope of tested/supported scenarios. I see you've tagged the downstream issue as "type-bug". Would you like to advocate for it being a bugfix and not a feature? What use cases would be affected if this behavior were only available here and in Python 3.14+?

Comment threadimportlib_resources/_common.py Outdated
@jaraco
jaracoforce-pushed the gh-123085/inferred-compiled branch from 5c3e385 to d618902CompareAugust 17, 2024 12:06
@jaraco
jaraco merged commit 0ecbc3b into mainAug 17, 2024
@jaraco
jaraco deleted the gh-123085/inferred-compiled branch August 17, 2024 12:11
@jaraco

Copy link
Copy Markdown
MemberAuthor

Releasing as v6.4.3. Please give it a try and let me know if you encounter any issues. This will get ported to CPython as well.

jaraco added a commit to jaraco/cpython that referenced this pull request Aug 17, 2024
@Gatsik

Copy link
Copy Markdown
Contributor

Would you like to advocate for it being a bugfix and not a feature?

In my opinion, it's a bug, because previously files worked both for non-compiled and compiled code, and update, which allows to omit anchor, broke behaviour for compiled code

What use cases would be affected if this behavior were only available here and in Python 3.14+?

Applications, which use libraries, which use bare files with importlib will stop working correctly. Currently I have no idea how I would workaround this issue with irc==20.5.0 rather than downgrading it to the version that doesn't use bare files

jaraco added a commit to jaraco/cpython that referenced this pull request Sep 12, 2024
jaraco added a commit to jaraco/cpython that referenced this pull request Sep 12, 2024
jaraco added a commit to python/cpython that referenced this pull request Sep 12, 2024
… source (#123102)
gh-123085: Fix issue in inferred caller when resources package has no source.
From importlib_resources 6.4.3 (python/importlib_resources#314).
jaraco added a commit to jaraco/cpython that referenced this pull request Sep 12, 2024
…ackage has no source (pythonGH-123102)
pythongh-123085: Fix issue in inferred caller when resources package has no source.
From importlib_resources 6.4.3 (python/importlib_resourcesGH-314).
(cherry picked from commit a53812d)
Co-authored-by: Jason R. Coombs <jaraco@jaraco.com>
jaraco added a commit to python/cpython that referenced this pull request Jan 5, 2025
… has no source (GH-123102) (#124021)
gh-123085: Fix issue in inferred caller when resources package has no source.
From importlib_resources 6.4.3 (python/importlib_resourcesGH-314).
(cherry picked from commit a53812d)
github-actionsBot pushed a commit to m-aciek/python-docs-weblate that referenced this pull request Jan 5, 2025
… has no source (GH-123102) (#124021)
gh-123085: Fix issue in inferred caller when resources package has no source.
From importlib_resources 6.4.3 (python/importlib_resourcesGH-314).
(cherry picked from commit a53812df126b99bca25187441a123c7785ee82a0)
CPython-sync-commit-latest: d712ece43feeed7f930954d40b046cdaeed7c414
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

importlib.resources.files() doesn't work correctly when importlib library is compiled

2 participants

@jaraco@Gatsik