Skip to content

Commit

Permalink
Automatically run Django's collectstatic command (#108)
Browse files Browse the repository at this point in the history
The classic Heroku Python buildpack automatically runs the Django
`collectstatic` command:
https://github.com/heroku/heroku-buildpack-python/blob/main/bin/steps/collectstatic

This adds equivalent support, with a couple of improvements:
- This implementation performs more checks to see whether the app is
  actually using the static files feature before trying to run it
  (reducing the number of cases where users would need to manually
  disable it).
- The collectstatic symlink feature has been enabled, as requested in
  heroku/heroku-buildpack-python#1060.
- Symlinked `manage.py` files are now supported, as requested in
  heroku/heroku-buildpack-python#972.
- The error messages are finer grained/more useful.
- There are many more tests (including now testing legacy vs latest
  Django versions, to check the CLI arguments used work for both ends of
  the spectrum).

There is currently no way to force disable the feature (beyond removing
`django.contrib.staticfiles` from `INSTALLED_APPS` in the app's Django
config, or removing the `manage.py` script). Supporting this depends on
us deciding how best to handle buildpack options, so will be added
later, in #109.

The build log output and error messages are fairly reasonable already
(and a significant improvement over the classic buildpack), and will be
further polished as part of the future build output overhaul.

The implementation uses the new `utils::run_command_and_capture_output`
added in #106.

See:
* https://docs.djangoproject.com/en/4.2/howto/static-files/
* https://docs.djangoproject.com/en/4.2/ref/contrib/staticfiles/
* https://docs.djangoproject.com/en/4.2/ref/settings/#settings-staticfiles

Fixes #5.
GUS-W-9538294.
  • Loading branch information
edmorley committed Sep 13, 2023
1 parent 20d51b1 commit 64d5c7c
Show file tree
Hide file tree
Showing 33 changed files with 548 additions and 3 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Added

- Django's `collectstatic` command is now automatically run for Django apps that use static files. ([#108](https://github.com/heroku/buildpacks-python/pull/108))

## [0.6.0] - 2023-08-25

### Changed
Expand Down
118 changes: 118 additions & 0 deletions src/django.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
use crate::utils::{self, CapturedCommandError, StreamedCommandError};
use indoc::indoc;
use libcnb::Env;
use libherokubuildpack::log::log_info;
use std::io;
use std::path::Path;
use std::process::Command;

const MANAGEMENT_SCRIPT_NAME: &str = "manage.py";

pub(crate) fn is_django_installed(dependencies_layer_dir: &Path) -> io::Result<bool> {
dependencies_layer_dir.join("bin/django-admin").try_exists()
}

pub(crate) fn run_django_collectstatic(
app_dir: &Path,
command_env: &Env,
) -> Result<(), DjangoCollectstaticError> {
if !has_management_script(app_dir)
.map_err(DjangoCollectstaticError::CheckManagementScriptExists)?
{
log_info(indoc! {"
Skipping automatic static file generation since no Django 'manage.py'
script (or symlink to one) was found in the root directory of your
application."
});
return Ok(());
}

if !has_collectstatic_command(app_dir, command_env)
.map_err(DjangoCollectstaticError::CheckCollectstaticCommandExists)?
{
log_info(indoc! {"
Skipping automatic static file generation since the 'django.contrib.staticfiles'
feature is not enabled in your app's Django configuration."
});
return Ok(());
}

log_info("Running 'manage.py collectstatic'");
utils::run_command_and_stream_output(
Command::new("python")
.args([
MANAGEMENT_SCRIPT_NAME,
"collectstatic",
"--link",
// Using `--noinput` instead of `--no-input` since the latter requires Django 1.9+.
"--noinput",
])
.current_dir(app_dir)
.env_clear()
.envs(command_env),
)
.map_err(DjangoCollectstaticError::CollectstaticCommand)
}

fn has_management_script(app_dir: &Path) -> io::Result<bool> {
app_dir.join(MANAGEMENT_SCRIPT_NAME).try_exists()
}

fn has_collectstatic_command(
app_dir: &Path,
command_env: &Env,
) -> Result<bool, CapturedCommandError> {
utils::run_command_and_capture_output(
Command::new("python")
.args([MANAGEMENT_SCRIPT_NAME, "help", "collectstatic"])
.current_dir(app_dir)
.env_clear()
.envs(command_env),
)
.map_or_else(
|error| match error {
// We need to differentiate between the command not existing (due to the staticfiles app
// not being installed) and the Django config or mange.py script being broken. Ideally
// we'd inspect the output of `manage.py help --commands` but that command unhelpfully
// exits zero even if the app's `DJANGO_SETTINGS_MODULE` wasn't a valid module.
CapturedCommandError::NonZeroExitStatus(output)
if String::from_utf8_lossy(&output.stderr).contains("Unknown command") =>
{
Ok(false)
}
_ => Err(error),
},
|_| Ok(true),
)
}

/// Errors that can occur when running the Django collectstatic command.
#[derive(Debug)]
pub(crate) enum DjangoCollectstaticError {
CheckCollectstaticCommandExists(CapturedCommandError),
CheckManagementScriptExists(io::Error),
CollectstaticCommand(StreamedCommandError),
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn has_management_script_django_project() {
assert!(has_management_script(Path::new(
"tests/fixtures/django_staticfiles_latest_django"
))
.unwrap());
}

#[test]
fn has_management_script_empty() {
assert!(!has_management_script(Path::new("tests/fixtures/empty")).unwrap());
}

#[test]
fn has_management_script_io_error() {
assert!(has_management_script(Path::new("tests/fixtures/empty/.gitkeep")).is_err());
}
}
75 changes: 74 additions & 1 deletion src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use crate::django::DjangoCollectstaticError;
use crate::layers::pip_dependencies::PipDependenciesLayerError;
use crate::layers::python::PythonLayerError;
use crate::package_manager::DeterminePackageManagerError;
use crate::python_version::{PythonVersion, PythonVersionError, DEFAULT_PYTHON_VERSION};
use crate::runtime_txt::{ParseRuntimeTxtError, RuntimeTxtError};
use crate::utils::{DownloadUnpackArchiveError, StreamedCommandError};
use crate::utils::{CapturedCommandError, DownloadUnpackArchiveError, StreamedCommandError};
use crate::BuildpackError;
use indoc::{formatdoc, indoc};
use libherokubuildpack::log::log_error;
Expand Down Expand Up @@ -46,6 +47,8 @@ fn on_buildpack_error(error: BuildpackError) {
&io_error,
),
BuildpackError::DeterminePackageManager(error) => on_determine_package_manager_error(error),
BuildpackError::DjangoCollectstatic(error) => on_django_collectstatic_error(error),
BuildpackError::DjangoDetection(error) => on_django_detection_error(&error),
BuildpackError::PipDependenciesLayer(error) => on_pip_dependencies_layer_error(error),
BuildpackError::PythonLayer(error) => on_python_layer_error(error),
BuildpackError::PythonVersion(error) => on_python_version_error(error),
Expand Down Expand Up @@ -217,6 +220,76 @@ fn on_pip_dependencies_layer_error(error: PipDependenciesLayerError) {
};
}

fn on_django_detection_error(error: &io::Error) {
log_io_error(
"Unable to determine if this is a Django-based app",
"checking if the 'django-admin' command exists",
error,
);
}

fn on_django_collectstatic_error(error: DjangoCollectstaticError) {
match error {
DjangoCollectstaticError::CheckCollectstaticCommandExists(error) => match error {
CapturedCommandError::Io(io_error) => log_io_error(
"Unable to inspect Django configuration",
"running 'python manage.py help collectstatic' to inspect the Django configuration",
&io_error,
),
CapturedCommandError::NonZeroExitStatus(output) => log_error(
"Unable to inspect Django configuration",
formatdoc! {"
The 'python manage.py help collectstatic' Django management command
(used to check whether Django's static files feature is enabled)
failed ({exit_status}).
Details:
{stderr}
This indicates there is a problem with your application code or Django
configuration. Try running the 'manage.py' script locally to see if the
same error occurs.
",
exit_status = &output.status,
stderr = String::from_utf8_lossy(&output.stderr)
},
),
},
DjangoCollectstaticError::CheckManagementScriptExists(io_error) => log_io_error(
"Unable to inspect Django configuration",
"checking if the 'manage.py' script exists",
&io_error,
),
DjangoCollectstaticError::CollectstaticCommand(error) => match error {
StreamedCommandError::Io(io_error) => log_io_error(
"Unable to generate Django static files",
"running 'python manage.py collectstatic' to generate Django static files",
&io_error,
),
StreamedCommandError::NonZeroExitStatus(exit_status) => log_error(
"Unable to generate Django static files",
formatdoc! {"
The 'python manage.py collectstatic --link --noinput' Django management
command to generate static files failed ({exit_status}).
This is most likely due an issue in your application code or Django
configuration. See the log output above for more information.
If you are using the WhiteNoise package to optimize the serving of static
files with Django (recommended), check that your app is using the Django
config options shown here:
https://whitenoise.readthedocs.io/en/stable/django.html
Or, if you do not need to use static files in your app, disable the
Django static files feature by removing 'django.contrib.staticfiles'
from 'INSTALLED_APPS' in your app's Django configuration.
"},
),
},
};
}

fn log_io_error(header: &str, occurred_whilst: &str, io_error: &io::Error) {
// We don't suggest opening a support ticket, since a subset of I/O errors can be caused
// by issues in the application. In the future, perhaps we should try and split these out?
Expand Down
19 changes: 17 additions & 2 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#![allow(clippy::large_enum_variant)]
#![allow(clippy::result_large_err)]

mod django;
mod errors;
mod layers;
mod package_manager;
Expand All @@ -13,6 +14,7 @@ mod python_version;
mod runtime_txt;
mod utils;

use crate::django::DjangoCollectstaticError;
use crate::layers::pip_cache::PipCacheLayer;
use crate::layers::pip_dependencies::{PipDependenciesLayer, PipDependenciesLayerError};
use crate::layers::python::{PythonLayer, PythonLayerError};
Expand Down Expand Up @@ -80,7 +82,7 @@ impl Buildpack for PythonBuildpack {

// Create the layers for the application dependencies and package manager cache.
// In the future support will be added for package managers other than pip.
match package_manager {
let (dependencies_layer_dir, dependencies_layer_env) = match package_manager {
PackageManager::Pip => {
log_header("Installing dependencies using Pip");
let pip_cache_layer = context.handle_layer(
Expand All @@ -97,9 +99,18 @@ impl Buildpack for PythonBuildpack {
pip_cache_dir: pip_cache_layer.path,
},
)?;
pip_layer.env
(pip_layer.path, pip_layer.env)
}
};
command_env = dependencies_layer_env.apply(Scope::Build, &command_env);

if django::is_django_installed(&dependencies_layer_dir)
.map_err(BuildpackError::DjangoDetection)?
{
log_header("Generating Django static files");
django::run_django_collectstatic(&context.app_dir, &command_env)
.map_err(BuildpackError::DjangoCollectstatic)?;
}

BuildResultBuilder::new().build()
}
Expand All @@ -115,6 +126,10 @@ pub(crate) enum BuildpackError {
DetectIo(io::Error),
/// Errors determining which Python package manager to use for a project.
DeterminePackageManager(DeterminePackageManagerError),
/// Errors running the Django collectstatic command.
DjangoCollectstatic(DjangoCollectstaticError),
/// IO errors when detecting whether Django is installed.
DjangoDetection(io::Error),
/// Errors installing the project's dependencies into a layer using Pip.
PipDependenciesLayer(PipDependenciesLayerError),
/// Errors installing Python and required packaging tools into a layer.
Expand Down
Loading

0 comments on commit 64d5c7c

Please sign in to comment.