Re: [Django] #32302: Permit migrations in non-namespace packages that don't have __file__

[Django]

#32302: Permit migrations in non-namespace packages that don't have __file__
-+-
 Reporter:  William Schwartz |Owner:  William
 Type:   |  Schwartz
  Cleanup/optimization   |   Status:  closed
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:  fixed
 Keywords:  migrations freezers  | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-

Comment (by William Schwartz):

 == Appendix: Research Notes

 'This ticket is closed. You need read no further unless you're
 researching a different issue related to the Python import system or
 Django's migration loading.'

 While doing my research this ticket, I took a lot of notes that didn't
 make it into the actual report. Here they are, in case it helps someone
 else down the line.

 === More background on `__file__`

 `__file__` is set by the import machinery from
 
[https://docs.python.org/3.9/library/importlib.html#importlib.machinery.ModuleSpec.origin
 importlib.machinery.ModuleSpec.origin], which may be `None`:

 > Name of the place from which the module is loaded, e.g. “builtin” for
 built-in modules and the filename for modules loaded from source. Normally
 “origin” should be set, but it may be None (the default) which indicates
 it is unspecified (e.g. for namespace packages).

 One frozen Python environment, PyOxidizer,
 
[https://pyoxidizer.readthedocs.io/en/stable/oxidized_importer_behavior_and_compliance.html
 #file-and-cached-module-attributes relies] on this API rule.

 === Proving a module is a namespace package

 [https://docs.python.org/3.9/whatsnew/3.7.html#other-cpython-
 implementation-changes Since Python 3.7] fixed
 [https://bugs.python.org/issue32303 bpo-32303], it is no longer possible
 to prove definitively that a module is a namespace package. In Python 3.6
 and earlier, a module's
 [https://docs.python.org/3.9/reference/import.html#__loader__ __loader__]
 was `None` if and only if the module was a namespace package. This was
 because [https://www.python.org/dev/peps/pep-0451/#namespace-packages PEP
 451] required that a [https://docs.python.org/3.9/glossary.html#term-
 finder finder]'s
 
[https://docs.python.org/3.9/library/importlib.html#importlib.abc.MetaPathFinder.find_spec
 find_spec] set
 
[https://docs.python.org/3.9/library/importlib.html#importlib.machinery.ModuleSpec.loader
 loader] to `None`:

 > Currently a path entry finder may return (None, portions) from
 
[https://docs.python.org/3.9/library/importlib.html#importlib.abc.PathEntryFinder.find_loader
 find_loader()] to indicate it found part of a possible namespace package.
 To achieve the same effect, find_spec() must return a spec with "loader"
 set to None (a.k.a. not set) and with submodule_search_locations set

 In Python 3.9, calling
 [https://docs.python.org/3.9/library/importlib.html#importlib.util.find_spec
 importlib.util.find_spec] on the name of a not-yet-imported namespace
 package returned a spec whose `loader` is `None`, but once the module is
 imported, the returned spec's `loader` is an instance of
 `_frozen_importlib_external._NamespaceLoader`. Note that
 `_NamespaceLoader` is an implementation detail of CPython.

 We must resist the temptation to rely on Python's
 [https://docs.python.org/3.9/reference/import.html#loading documented code
 for detecting namespace packages]:

 > {{{#!python
 > if spec.origin is None and spec.submodule_search_locations is not None:
 > # namespace package
 > sys.modules[spec.name] = module
 > }}}

 This documentation is patently falsified by a cursory review of Python's
 
[https://github.com/python/cpython/blob/4140f10a16f06c32fd49f9e21fb2a53abe7357f0/Lib/importlib/_bootstrap.py#L511-L538
 source code], which clearly relies on `loader` being `None`.

 See also [https://www.python.org/dev/peps/pep-0420/#differences-between-
 namespace-packages-and-regular-packages PEP-420's summary of differences
 between namespace and regular packages], which is now somewhat outdated.

 === History of the migration loader's no-namespace check

 The reason, given both in the
 
[https://github.com/django/django/blob/2a76f4313423a3b91caade4fce71790630ef9152/django/db/migrations/loader.py#L91-L95
 source code comments] and in [https://groups.google.com/g/django-
 developers/c/GVHMH2ciAnk/m/vbIPbZuSBQAJ discussions among maintainers],
 for the migration loader's check that an app's `migrations` package is not
 a namespace package is to discourage the latter's use.

 The current 

Re: [Django] #32302: Permit migrations in non-namespace packages that don't have __file__

[Django]

#32302: Permit migrations in non-namespace packages that don't have __file__
-+-
 Reporter:  William Schwartz |Owner:  William
 Type:   |  Schwartz
  Cleanup/optimization   |   Status:  closed
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:  fixed
 Keywords:  migrations freezers  | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Mariusz Felisiak ):

 * status:  assigned => closed
 * resolution:   => fixed


Comment:

 In [changeset:"e64c1d8055a3e476122633da141f16b50f0c4a2d" e64c1d80]:
 {{{
 #!CommitTicketReference repository=""
 revision="e64c1d8055a3e476122633da141f16b50f0c4a2d"
 Fixed #32302 -- Allowed migrations to be loaded from regular packages with
 no __file__ attribute.

 The migrations loader prevents the use of PEP-420 namespace packages
 for holding apps' migrations modules. Previously the loader tested for
 this only by checking that app.migrations.__file__ is present. This
 prevented migrations' being found in frozen Python environments that
 don't set __file__ on any modules. Now the loader *additionally* checks
 whether app.migrations.__path__ is a list because namespace packages
 use a different type for __path__. Namespace packages continue to be
 forbidden, and, in fact, users of normal Python environments should
 experience no change whatsoever.
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/068.f8aaac19a4757086432d0ea70eee1624%40djangoproject.com.

Re: [Django] #32302: Permit migrations in non-namespace packages that don't have __file__

[Django]

#32302: Permit migrations in non-namespace packages that don't have __file__
-+-
 Reporter:  William Schwartz |Owner:  William
 Type:   |  Schwartz
  Cleanup/optimization   |   Status:  assigned
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  migrations freezers  | Triage Stage:  Ready for
 |  checkin
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Mariusz Felisiak):

 * stage:  Accepted => Ready for checkin


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/068.0d244a5af9fb4d13bd59f31039e827fa%40djangoproject.com.

Re: [Django] #32302: Permit migrations in non-namespace packages that don't have __file__

[Django]

#32302: Permit migrations in non-namespace packages that don't have __file__
-+-
 Reporter:  William Schwartz |Owner:  William
 Type:   |  Schwartz
  Cleanup/optimization   |   Status:  assigned
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  migrations freezers  | Triage Stage:  Accepted
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by Mariusz Felisiak):

 * keywords:  migrations => migrations freezers
 * type:  New feature => Cleanup/optimization
 * stage:  Unreviewed => Accepted


Comment:

 Thanks for the rationale. Sounds reasonable.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/068.4ccb88456f101dd4b87c4b2fe518f573%40djangoproject.com.

Re: [Django] #32302: Permit migrations in non-namespace packages that don't have __file__

[Django]

#32302: Permit migrations in non-namespace packages that don't have __file__
-+-
 Reporter:  William Schwartz |Owner:  William
 |  Schwartz
 Type:  New feature  |   Status:  assigned
Component:  Migrations   |  Version:  master
 Severity:  Normal   |   Resolution:
 Keywords:  migrations   | Triage Stage:
 |  Unreviewed
Has patch:  1|  Needs documentation:  0
  Needs tests:  0|  Patch needs improvement:  0
Easy pickings:  0|UI/UX:  0
-+-
Changes (by William Schwartz):

 * owner:  nobody => William Schwartz
 * status:  new => assigned


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/068.49159f4f2504b5bcfe5a4bb4e1fa82d0%40djangoproject.com.

[Django] #32302: Permit migrations in non-namespace packages that don't have __file__

[Django]

#32302: Permit migrations in non-namespace packages that don't have __file__
+
   Reporter:  William Schwartz  |  Owner:  nobody
   Type:  New feature   | Status:  new
  Component:  Migrations|Version:  master
   Severity:  Normal|   Keywords:  migrations
   Triage Stage:  Unreviewed|  Has patch:  1
Needs documentation:  0 |Needs tests:  0
Patch needs improvement:  0 |  Easy pickings:  0
  UI/UX:  0 |
+
 == Summary

 This feature request, for which I will post a PR shortly, aims to improve
 the specificity of the migration loader's check for and rejection of
 [http://www.python.org/dev/peps/pep-0420/ PEP-420] namespace packages. **I
 am NOT asking to allow namespace packages for apps' migrations.** I merely
 want to make the existing check more compliant with Python's documented
 import API. This would remove one impediment to using Django in so-called
 ''frozen'' Python environments (such as those mentioned in #30950) that do
 not set [https://docs.python.org/3/reference/import.html#__file__
 __file__] on ''regular'' packages by default.

 **This narrow proposal does not change Django's behavior at all for normal
 Python environments.** The only change for frozen environments is that
 Django will learn how to find existing migrations. In particular, at this
 time **I am not proposing to enable any other Django feature that does not
 already work in frozen environments**.

 I would love for this feature to land in Django 3.2.

 == Details

 I initially broached this idea on the [https://groups.google.com/g/django-
 developers/c/UXsG5xZNRgY django-developers mailing list]. This is my
 second ticket related to frozen Python environments, the first being
 #32177.

 The
 
[https://github.com/django/django/blob/2a76f4313423a3b91caade4fce71790630ef9152/django/db/migrations/loader.py#L91-L95
 current implementation] of the migration loader's no-namespace-package
 check in `django.db.migrations.loader.MigrationLoader.load_disk` skips
 searching for migrations in a module `m` if `getattr(m, '__file__', None)`
 is false.

 The trouble with this implementation is that namespace packages are not
 the only modules with no `__file__`. Indeed, the Python
 [https://docs.python.org/3/reference/import.html#__file__ documentation
 states] that

 > `__file__` is optional. If set, this attribute's value must be a string.
 The import system may opt to leave `__file__` unset if it has no semantic
 meaning (e.g. a module loaded from a database).

 However, Python's [https://docs.python.org/3/reference/import.html
 #namespace-packages documentation also states]

 > Namespace packages do not use an ordinary list for their `__path__`
 attribute. They instead use a custom iterable type

 The class of namespace packages' `__path__` in CPython is
 
[https://github.com/python/cpython/blob/4140f10a16f06c32fd49f9e21fb2a53abe7357f0/Lib/importlib/_bootstrap_external.py#L1158-L1217
 _NamespacePath], but that is a CPython implementation detail. Instead, I
 propose to augment `getattr(m, '__file__', None)` with `and
 isinstance(m.__path__, list)`.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/053.d1de81d007bbb4dd36e143bb0e5508d3%40djangoproject.com.