Summary

Fix extension packaging with the new pipeline changes.

Review Request #8067 — Created March 21, 2016 and submitted April 1, 2016, 4:39 p.m.

Information

Owner

chipx86

Repository

Djblets

Branch

release-0.10.x

Bugs

Depends On

Commit

52330b2...

Reviewers

Groups

djblets

People

Description

The new pipeline changes broke any extension-provided .less files that
needed to include or reference anything from another module, along with
any global variables provided by the app. There were also problems
loading static media from installed packages.

The arguments to lessc were still being set in the old setting, which
Pipeline no longer respects, and it wasn't in a suitable format (it
needed to be a list of strings, not a string).

This fixes that setting, plus ensures that we don't have broken
conflicts between what the app must now provide for normal use and
what's needed for packaging. It does this by filtering out app-provided
settings, replacing them with ours.

It also disables the use of the special Djblets LessCompiler's import
script for checking outdated dependencies. This isn't needed during
packaging, and in fact fails due to static file storage access control
checks.

To fix the package static media, we now have specialized versions of the
Pipeline CSS/JavaScript template nodes that change the rendering
behavior to fall back on outputted (compiled) static media files if the
sources are unavailable. This depends on a pending change in Pipeline
(https://github.com/jazzband/django-pipeline/pull/552).

Testing Done

Tested against Power Pack, which wasn't working with the new Pipeline

changes due to referencing stylesheets in Review Board. I was able to

successfully compile them without problems.
Tested loading Power Pack pages when installed via a package, and

tested when the package was in develop mode. Wiped my htdocs/static/

tree each time, and verified that the page was loading the appropriate

files. Also verified that if I modified a file (source or output,

depending on which was being used), the changes were picked up when

next reloading the page.
Existing unit tests pass.

Issues

Description	From	Last Updated
Don't we want to return True here (always recompile)?	david	March 30, 2016, 5:03 p.m.
Maybe say "ALWAYS_REBUILD" rather than "ALWAYS_OUTDATED"?	david	March 31, 2016, 2:35 p.m.
We could probably stick this into the pipeline settings dictionary.	david	March 31, 2016, 2:41 p.m.
'settings' imported but unused	reviewbot	March 31, 2016, 3:12 p.m.

Tool: Pyflakes
Processed Files:
    djblets/extensions/templatetags/djblets_extensions.py
    djblets/extensions/packaging.py
    djblets/pipeline/compilers/less.py



Tool: PEP8 Style Checker
Processed Files:
    djblets/extensions/templatetags/djblets_extensions.py
    djblets/extensions/packaging.py
    djblets/pipeline/compilers/less.py

Change Summary:

Fixed a missing default variable declaration.

Commit:

-	6f5166454a79373bc4fa5431ab81559bd19baae2
+	152457155f944adecb06308b0e0405786b64adc6

Diff:

Revision 2 (+119 -16)

Show changes

	djblets/extensions/packaging.py
	djblets/extensions/templatetags/djblets_extensions.py
	djblets/pipeline/compilers/less.py

Tool: Pyflakes
Processed Files:
    djblets/extensions/templatetags/djblets_extensions.py
    djblets/extensions/packaging.py
    djblets/pipeline/compilers/less.py



Tool: PEP8 Style Checker
Processed Files:
    djblets/extensions/templatetags/djblets_extensions.py
    djblets/extensions/packaging.py
    djblets/pipeline/compilers/less.py

It looks like this is the same change as /r/8066/ ?

djblets/pipeline/compilers/less.py (Diff revision 2)

The issue has been resolved. Show all issues

Don't we want to return True here (always recompile)?

chipx86 March 30, 2016, 5:03 p.m.

Had to think about this for a bit, piece everything together in my head, but I agree.
The intent was to just use LessCompiler's default behavior (which is checked above), and since we know it didn't find it to be outdated, we'd return that result. However, that doesn't consider the dependencies, so it's not safe..
Going to change this and rename the setting to DJBLETS_PIPELINE_LESS_ALWAYS_OUTDATED.
This is only really used for packaging, which is going to wipe and rebuild anyway, so it's sort of academic, but it'll help future-proof this.

Change Summary:

Packaging extensions now forces all .less files to be considered outdated.

Commit:

-	152457155f944adecb06308b0e0405786b64adc6
+	678d1e6d058f4563109101955284b09a351a1270

Diff:

Revision 3 (+119 -18)

Show changes

	djblets/extensions/packaging.py
	djblets/extensions/templatetags/djblets_extensions.py
	djblets/pipeline/compilers/less.py

Tool: Pyflakes
Processed Files:
    djblets/extensions/templatetags/djblets_extensions.py
    djblets/extensions/packaging.py
    djblets/pipeline/compilers/less.py



Tool: PEP8 Style Checker
Processed Files:
    djblets/extensions/templatetags/djblets_extensions.py
    djblets/extensions/packaging.py
    djblets/pipeline/compilers/less.py

djblets/extensions/packaging.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Maybe say "ALWAYS_REBUILD" rather than "ALWAYS_OUTDATED"?
```
1. chipx86 March 31, 2016, 2:29 p.m.
  That's a better name.

djblets/pipeline/compilers/less.py (Diff revision 3)

The issue has been resolved. Show all issues

We could probably stick this into the pipeline settings dictionary.

chipx86 March 31, 2016, 2:29 p.m.

I didn't want to invade on their namespace too much, particularly for something internal.

david March 31, 2016, 2:33 p.m.

Yeah it's internal, but it's both our compiler and our settings.py

Change Summary:

Renamed the internal setting to _DJBLETS_LESS_ALWAYS_REBUILD and put it in pipeline_settings.

Commit:

-	678d1e6d058f4563109101955284b09a351a1270
+	211a7d1e0d9d0dd5cbddc0e88b81cf31cb912f07

Diff:

Revision 4 (+120 -18)

Show changes

	djblets/extensions/packaging.py
	djblets/extensions/templatetags/djblets_extensions.py
	djblets/pipeline/compilers/less.py

Tool: Pyflakes
Processed Files:
    djblets/extensions/templatetags/djblets_extensions.py
    djblets/extensions/packaging.py
    djblets/pipeline/compilers/less.py



Tool: PEP8 Style Checker
Processed Files:
    djblets/extensions/templatetags/djblets_extensions.py
    djblets/extensions/packaging.py
    djblets/pipeline/compilers/less.py

djblets/pipeline/compilers/less.py (Diff revision 4)
The issue has been resolved. Show all issues
```
 'settings' imported but unused
```

Change Summary:

Removed an unused import.

Commit:

-	211a7d1e0d9d0dd5cbddc0e88b81cf31cb912f07
+	52330b210cede83aba2a61812e9b1ecfddd3bb1e

Diff:

Revision 5 (+119 -18)

Show changes

	djblets/extensions/packaging.py
	djblets/extensions/templatetags/djblets_extensions.py
	djblets/pipeline/compilers/less.py

Tool: PEP8 Style Checker
Processed Files:
    djblets/extensions/templatetags/djblets_extensions.py
    djblets/extensions/packaging.py
    djblets/pipeline/compilers/less.py



Tool: Pyflakes
Processed Files:
    djblets/extensions/templatetags/djblets_extensions.py
    djblets/extensions/packaging.py
    djblets/pipeline/compilers/less.py

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-0.10.x (3bafb69)