Deprecation warnings skip past packages named "django_*"

[Mike Edmunds]

The django_file_prefixes() utility doesn't include a trailing path separator, so also matches other packages whose installation directories start with "django". As a result, use of deprecated features in those packages will be mis-attributed to some other code outside those packages.

Django's implementation frequently has code like this:

# django.some_module

import warnings
from django.utils.deprecation import RemovedInDjangoXXWarning, django_file_prefixes

def _some_helper():
    ...
    warnings.warn(
        "This is deprecated.",
        RemovedInDjangoXXWarning,
        skip_file_prefixes=django_file_prefixes(),
    )

def some_django_feature():
    ...
    _some_helper()
    ...

The intent is a caller to some_django_feature() outside Django gets a deprecation warning pointed at their own code. (Not pointing at the call to _some_helper() in Django's implementation.)

As ​currently implemented, django_file_prefixes() returns the directory where Django is installed (in a tuple). E.g., ("/venv/site-packages/django",).

If some_django_feature() is called from a (hypothetical) django_goodies package installed in /venv/site-packages/django_goodies/__init__.py, the warning will skip it too: that filename ​startswith the django_file_prefixes. The resulting warning will point to the first stack frame outside django_goodies, which may be completely disconnected from the deprecated feature usage.

[Fashad Ahmed]

Hi Artyom, I’m interested in helping with this ticket. It seems like a straightforward fix in django.utils.deprecation by ensuring django_file_prefixes adds a trailing os.sep to the paths. Are you currently working on a patch, or would you mind if I took a shot at it? Thanks!

[Artyom Kotovskiy]

Replying to Fashad Ahmed:

Hi Artyom, I’m interested in helping with this ticket. It seems like a straightforward fix in django.utils.deprecation by ensuring django_file_prefixes adds a trailing os.sep to the paths. Would you mind if I took a shot at it? Thanks!

I already fixed it just have to do a pr

[Artyom Kotovskiy]

PR --> ​https://github.com/django/django/pull/21185

[Jacob Walls]

Bug in f42b89f1bf49a5b89ed852b60f79342320a81c5e and 34bd3ed944bf38792c631b55e581963d44d52284 before refactor in 7b26b64a63b5fc15134426a6ee0534dca2a1a855.

[Jacob Walls]

Fashad, although I'm merging your PR in the interest of efficiency, since it took fewer manual edits to polish, next time please respect the existing owner on Trac. It had only been a few hours since it was claimed.

[Mike Edmunds]

Just to clarify, the deprecation warnings are still issued even with the bug. They just show a less-helpful stack frame than intended.

[I'm not sure I follow the history references, but they seem to confirm code using skip_file_prefixes had the same problem before the centralized django_file_prefixes() was created.]

[Mike Edmunds]

(Sorry, Trac bug changed more than I intended)

[Jacob Walls]

Thanks for the review!

Replying to Mike Edmunds:

[I'm not sure I follow the history references, but they seem to confirm code using skip_file_prefixes had the same problem before the centralized django_file_prefixes() was created.]

Right, all I mean was that it's a "bug" in the "new features" contributed in those commits that they cite the less appropriate stack frames, so we should backport.

[Jacob Walls <jacobtylerwalls@…>]

In 60a9c70:

Fixed #37067 -- Added trailing slash in django_file_prefixes().

Ensure skip_file_prefixes does not match sibling packages like django*.

Bug in f42b89f1bf49a5b89ed852b60f79342320a81c5e
and 34bd3ed944bf38792c631b55e581963d44d52284.

[Jacob Walls <jacobtylerwalls@…>]

In 8bcd15b:

[6.0.x] Fixed #37067 -- Added trailing slash in django_file_prefixes().

Ensure skip_file_prefixes does not match sibling packages like django*.

Bug in f42b89f1bf49a5b89ed852b60f79342320a81c5e
and 34bd3ed944bf38792c631b55e581963d44d52284.

Backport of 60a9c70496e5d7b971928ce3da5b47c8836a4def from main.

[Fashad Ahmed]

Replying to Jacob Walls:

Fashad, although I'm merging your PR in the interest of efficiency, since it took fewer manual edits to polish, next time please respect the existing owner on Trac. It had only been a few hours since it was claimed.

Thanks for merging and polishing the PR, I appreciate it. Apologies for stepping in while the ticket was already assigned. I’ll make sure to check ownership more carefully before contributing next time. Glad this fix made it in!