Fix several issues with building static media files.

Review Request #12413 — Created June 24, 2022 and submitted

Information

Review Board
release-5.0.x

Reviewers

We had issues that popped up with building of static media files. These
didn't prevent building static media, but mostly because we're lucky.

The first issue is that FORCE_BUILD_MEDIA wasn't being respected. We
were setting that using os.putenv() in build-media.py, but
settings.py was getting an empty string out when using os.getenv().

When calling os.putenv(), we don't actually modify the current
process, just subprocesses. The correct API to use is just os.environ,
which stores the variable in the local process's environment and wraps
os.putenv() for subprocesses.

This has been updated to just use os.environ in all cases.

Another is that we had conflicts when building files. Both
AppDirectoriesFinder and FileSystemFinder would find the exact same
static media files, and this would log complaints about duplicates.
Likely this isn't new behavior, but the logging is new.

The reason for the conflict is that we defined specific directories
containing static media files to process (for rb, djblets, and
lib), where the djblets version was also found in the djblets app
and rb and lib were found in the reviewboard app (both in
static/ directories).

We need reviewboard and djblets apps to be in INSTALLED_APPS,
since we reference templates and other state from there. Given that we
need these, and both contain static/ directories, we can get by with
just AppDirectoriesFinder. There's no need to keep FileSystemFinder
anymore, or maintain a list of STATICFILES_DIRS.

Tested a complete static files rebuild with both finders, without
FileSystemFinder, and without AppDirectoriesFinder. Backed up each
version and compared directory contents.

Removing FileSystemFinder had no effect on the contents of the directory,
but it did remove the errors.

Removing AppDirectoriesFinder caused a change in what files we collected,
ruling that out as an option.

Successfully built packages, ran the server without any static media issues,
and ran unit tests.

Summary ID
Fix several issues with building static media files.
We had issues that popped up with building of static media files. These didn't prevent building static media, but mostly because we're lucky. The first issue is that `FORCE_BUILD_MEDIA` wasn't being respected. We were setting that using `os.putenv()` in `build-media.py`, but `settings.py` was getting an empty string out when using `os.getenv()`. When calling `os.putenv()`, we don't actually modify the current process, just subprocesses. The correct API to use is just `os.environ`, which stores the variable in the local process's environment and wraps `os.putenv()` for subprocesses. This has been updated to just use `os.environ` in all cases. Another is that we had conflicts when building files. Both `AppDirectoriesFinder` and `FileSystemFinder` would find the exact same static media files, and this would log complaints about duplicates. Likely this isn't new behavior, but the logging is new. The reason for the conflict is that we defined specific directories containing static media files to process (for `rb`, `djblets`, and `lib`), where the `djblets` version was also found in the `djblets` app and `rb` and `lib` were found in the `reviewboard` app (both in `static/` directories). We need `reviewboard` and `djblets` apps to be in `INSTALLED_APPS`, since we reference templates and other state from there. Given that we need these, and both contain `static/` directories, we can get by with just `AppDirectoriesFinder`. There's no need to keep `FileSystemFinder` anymore, or maintain a list of `STATICFILES_DIRS`.
bfe9131d2d84f022ba11942a95d522ee8b07186e
david
  1. Ship It!
  2. 
      
chipx86
Review request changed
Status:
Completed
Change Summary:
Pushed to release-5.0.x (48e8806)