Update sourcemap paths when concatenating source files.

[chipx86]

When building a package from source files, the built source files get concatenated together before being post-processed by Django. Prior to Django 4.0, the post-processing step would normalize url(...) entries in CSS by looking it up in storage and replacing the path with the hashed version.

Starting in Django 4.0, post-processing would do the same for sourcemaps. This can break when concatenating either CSS or JavaScript files, since Pipeline may produce a built package file that's in a different directory from one or more built source files. Django would fail to find the file and raise an error.

We now include sourcemap normalization as part of the concatenation process. This is using a similar approach to url(...) normalization, but now consolidated into the Compressor.concatenate() function. This has been updated to take arguments controlling the contenation process, such as a regex for capturing paths to normalize.

The regex for capturing sourcemap lines is built to be spec-compliant, and is currently more broad than what Django looks for during post-processing. This will help avoid potential issues as Django makes changes to their process.

The old functions (concatenate_and_rewrite()) and old default behavior has been left intact, but with runtime deprecation warnings, so that any code specializing Pipeline will continue to work. This helps ensure this change is API-compatible and non-breaking.

See issue #808 for more details on the problem and the solution.

Testing

Ran tox. All unit tests passed.

Tested this locally in three different projects using Pipeline and exhibiting this
problem on Django 4.0+. Verified that packaging worked as expected, with corrected
sourcemap paths.

[codecov]

Codecov Report

Attention: Patch coverage is 95.12195% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 82.55%. Comparing base (2018c11) to head (58f9f99).

Files	Patch %	Lines
pipeline/compressors/__init__.py	95.12%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #809      +/-   ##
==========================================
+ Coverage   82.35%   82.55%   +0.20%     
==========================================
  Files          33       33              
  Lines        1139     1158      +19     
==========================================
+ Hits          938      956      +18     
- Misses        201      202       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[chipx86]

Regarding the Ruff output:

1. Some of those results are about my type changes, but don't seem to reflect the actual code I've provided (complaints about X | Y unions, which I don't use). Pipeline doesn't use type hints today, and I can remove those, though there may be value in progressively working toward that.
2. The substantive complaint involves the concatenate() function having 6 arguments instead of a max of 5 (self, paths, file_sep, output_filename, rewrite_path_re, variant). These are necessary to allow for the same logic to be applied in both the CSS and JavaScript cases. Attempting to reduce the number of arguments would likely mean moving to independent CSS and JavaScript concatenation methods, which would then need to duplicate some logic. I can reexamine this if the 5 argument max is a hard requirement, but I think it'll lead to more redundancy in the code.
3. The code coverage lines mentioned are not actually related to my change. It's just impacted by conversion of a regex string to a compiled regex (which I changed to be consistent with the related regexes modified/introduced in this change). I could just undo that part if that's the best approach.

Thoughts on these points?

[davidt]

• Some of those results are about my type changes, but don't seem to reflect the actual code I've provided (complaints about X | Y unions, which I don't use). Pipeline doesn't use type hints today, and I can remove those, though there may be value in progressively working toward that.

Ruff's complaint about X | Y is because of Optional[X], which it wants to be X | None. Theoretically, we can use the X | Y syntax if we're careful to always use from __future__ import annotations, but it may be worth waiting until 3.10 is the minimum version. Regarding the larger question of whether to include type hints at all, my opinion is in favor, but hopefully other pipeline users will chime in.

• The substantive complaint involves the concatenate() function having 6 arguments instead of a max of 5 (self, paths, file_sep, output_filename, rewrite_path_re, variant). These are necessary to allow for the same logic to be applied in both the CSS and JavaScript cases. Attempting to reduce the number of arguments would likely mean moving to independent CSS and JavaScript concatenation methods, which would then need to duplicate some logic. I can reexamine this if the 5 argument max is a hard requirement, but I think it'll lead to more redundancy in the code.

This is just the default pylint PLR09 limit, which I think is unreasonable for real-world, complex software. The recommendation in pylint to "fix" this is to create NamedTuples to group some arguments. In some cases, that might make sense, but here I think all it would do is obfuscate the code to satisfy the linter.

[davidt]

This looks good to me now. Not sure if anyone else wants to review as well.

[tomlinsj]

would it be possible to get this released to pypi soon? I have a number of sites impacted by this, and would like to update them to django 4.2 before 3.2 losses LTS

[chipx86]

would it be possible to get this released to pypi soon? I have a number of sites impacted by this, and would like to update them to django 4.2 before 3.2 losses LTS

We're just waiting for approval on being able to perform a release. We'd like to get this released quickly as well.