From e9f3d3449d0facc27a6f7ae2737a2703da08a52c Mon Sep 17 00:00:00 2001 From: Manuel Fuchs Date: Fri, 22 Nov 2024 13:52:09 +0100 Subject: [PATCH 1/8] Use unified Heroku buildpack output style --- .../gradle/src/gradle_command/daemon.rs | 2 +- buildpacks/gradle/src/main.rs | 36 +- buildpacks/jvm/CHANGELOG.md | 1 + buildpacks/jvm/src/errors.rs | 55 +-- buildpacks/jvm/src/layers/openjdk.rs | 197 +++++++--- buildpacks/jvm/src/main.rs | 91 +++-- buildpacks/jvm/src/version_resolver.rs | 68 ++++ buildpacks/maven/Cargo.toml | 2 +- buildpacks/maven/src/main.rs | 108 ++++-- buildpacks/maven/src/util.rs | 19 - buildpacks/sbt/Cargo.toml | 2 +- buildpacks/sbt/sbt-extras | 2 +- buildpacks/sbt/src/main.rs | 47 ++- shared/Cargo.toml | 2 +- shared/src/lib.rs | 1 + shared/src/output.rs | 365 ++++++++++++++++++ 16 files changed, 782 insertions(+), 216 deletions(-) create mode 100644 buildpacks/jvm/src/version_resolver.rs create mode 100644 shared/src/output.rs diff --git a/buildpacks/gradle/src/gradle_command/daemon.rs b/buildpacks/gradle/src/gradle_command/daemon.rs index 36f456e4..d33b13fe 100644 --- a/buildpacks/gradle/src/gradle_command/daemon.rs +++ b/buildpacks/gradle/src/gradle_command/daemon.rs @@ -12,7 +12,7 @@ pub(crate) fn start( ) -> Result<(), GradleCommandError<()>> { let output = Command::new(gradle_wrapper_executable_path) .args([ - // Fixes an issue when when running under Apple Rosetta emulation + // Fixes an issue when running under Apple Rosetta emulation "-Djdk.lang.Process.launchMechanism=vfork", "--daemon", GRADLE_TASK_NAME_HEROKU_START_DAEMON, diff --git a/buildpacks/gradle/src/main.rs b/buildpacks/gradle/src/main.rs index 31e4d379..63ba44ab 100644 --- a/buildpacks/gradle/src/main.rs +++ b/buildpacks/gradle/src/main.rs @@ -6,6 +6,9 @@ use crate::gradle_command::GradleCommandError; use crate::layers::gradle_home::handle_gradle_home_layer; use crate::GradleBuildpackError::{GradleBuildIoError, GradleBuildUnexpectedStatusError}; use buildpacks_jvm_shared as shared; +use buildpacks_jvm_shared::output::{ + print_buildpack_name, print_section, print_subsection, track_timing, +}; #[cfg(test)] use buildpacks_jvm_shared_test as _; use libcnb::build::{BuildContext, BuildResult, BuildResultBuilder}; @@ -75,7 +78,8 @@ impl Buildpack for GradleBuildpack { } fn build(&self, context: BuildContext) -> libcnb::Result { - log_header("Gradle Buildpack"); + print_buildpack_name("Heroku Gradle Buildpack"); + let buildpack_config = GradleBuildpackConfig::from(&context); let gradle_wrapper_executable_path = Some(context.app_dir.join("gradlew")) @@ -88,16 +92,26 @@ impl Buildpack for GradleBuildpack { let mut gradle_env = Env::from_current(); handle_gradle_home_layer(&context, &mut gradle_env)?; - log_header("Starting Gradle Daemon"); - gradle_command::start_daemon(&gradle_wrapper_executable_path, &gradle_env) - .map_err(GradleBuildpackError::StartGradleDaemonError)?; - - let project_tasks = gradle_command::tasks(&context.app_dir, &gradle_env) - .map_err(|command_error| command_error.map_parse_error(|_| ())) - .map_err(GradleBuildpackError::GetTasksError)?; - - let dependency_report = gradle_command::dependency_report(&context.app_dir, &gradle_env) - .map_err(GradleBuildpackError::GetDependencyReportError)?; + print_section("Running Gradle build"); + + track_timing(|| { + print_subsection("Starting Gradle daemon"); + gradle_command::start_daemon(&gradle_wrapper_executable_path, &gradle_env) + .map_err(GradleBuildpackError::StartGradleDaemonError) + })?; + + let project_tasks = track_timing(|| { + print_subsection("Querying tasks"); + gradle_command::tasks(&context.app_dir, &gradle_env) + .map_err(|command_error| command_error.map_parse_error(|_| ())) + .map_err(GradleBuildpackError::GetTasksError) + })?; + + let dependency_report = track_timing(|| { + print_subsection("Querying dependency report"); + gradle_command::dependency_report(&context.app_dir, &gradle_env) + .map_err(GradleBuildpackError::GetDependencyReportError) + })?; let task_name = buildpack_config .gradle_task diff --git a/buildpacks/jvm/CHANGELOG.md b/buildpacks/jvm/CHANGELOG.md index 9d36c1e5..e5bda584 100644 --- a/buildpacks/jvm/CHANGELOG.md +++ b/buildpacks/jvm/CHANGELOG.md @@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Default version for **OpenJDK 17** is now `17.0.13`. ([#747](https://github.com/heroku/buildpacks-jvm/pull/747)) - Default version for **OpenJDK 21** is now `21.0.5`. ([#747](https://github.com/heroku/buildpacks-jvm/pull/747)) - Default version for **OpenJDK 23** is now `23.0.1`. ([#747](https://github.com/heroku/buildpacks-jvm/pull/747)) +- Buildpack output changed to a new format. ([#000](https://github.com/heroku/buildpacks-jvm/pull/000)) ## [6.0.3] - 2024-09-26 diff --git a/buildpacks/jvm/src/errors.rs b/buildpacks/jvm/src/errors.rs index 644b9e65..8903327a 100644 --- a/buildpacks/jvm/src/errors.rs +++ b/buildpacks/jvm/src/errors.rs @@ -1,5 +1,8 @@ -use crate::openjdk_artifact::HerokuOpenJdkVersionRequirement; -use crate::{OpenJdkArtifactRequirementParseError, OpenJdkBuildpackError}; +use crate::openjdk_artifact::{ + HerokuOpenJdkVersionRequirement, OpenJdkArtifactRequirementParseError, +}; +use crate::version_resolver::VersionResolveError; +use crate::OpenJdkBuildpackError; use buildpacks_jvm_shared::log::{log_please_try_again, log_please_try_again_error}; use buildpacks_jvm_shared::system_properties::ReadSystemPropertiesError; use indoc::formatdoc; @@ -8,19 +11,6 @@ use libherokubuildpack::log::log_error; #[allow(clippy::too_many_lines)] pub(crate) fn on_error_jvm_buildpack(error: OpenJdkBuildpackError) { match error { - OpenJdkBuildpackError::OpenJdkArtifactRequirementParseError(OpenJdkArtifactRequirementParseError::UnknownDistribution(distribution)) => log_error( - format!("Unsupported distribution: {distribution}"), - formatdoc! {" - Please check your system.properties file to ensure the java.runtime.version - string does not contain an unsupported distribution prefix. - - You can also remove the system.properties file from your application to install - the default OpenJDK version. - - Thanks, - Heroku - "}, - ), OpenJdkBuildpackError::CannotCreateOpenJdkTempDir(error) => log_please_try_again_error( "Unexpected IO error", "Could not create temporary directory for the OpenJDK download due to an unexpected I/O error.", @@ -112,15 +102,6 @@ pub(crate) fn on_error_jvm_buildpack(error: OpenJdkBuildpackError) { Details: {error} ", error = error }, ), - OpenJdkBuildpackError::OpenJdkArtifactRequirementParseError(OpenJdkArtifactRequirementParseError::OpenJdkVersionParseError(error)) => log_error( - "Invalid OpenJDK version selector", - formatdoc! {" - The OpenJDK version selector you specified in your system.properties file is invalid. - Please specify a valid version selector in your system.properties file. - - Details: {error} - ", error = error }, - ), OpenJdkBuildpackError::OpenJdkTarballChecksumError { expected, actual } => log_please_try_again( "Corrupted OpenJDK download", formatdoc! {" @@ -129,6 +110,30 @@ pub(crate) fn on_error_jvm_buildpack(error: OpenJdkBuildpackError) { Expected: {expected} Actual: {actual} ", expected = hex::encode(expected), actual = hex::encode(actual) } - ) + ), + OpenJdkBuildpackError::ResolveVersionError(VersionResolveError::OpenJdkArtifactRequirementParseError(OpenJdkArtifactRequirementParseError::UnknownDistribution(distribution))) => log_error( + format!("Unsupported distribution: {distribution}"), + formatdoc! {" + Please check your system.properties file to ensure the java.runtime.version + string does not contain an unsupported distribution prefix. + + You can also remove the system.properties file from your application to install + the default OpenJDK version. + + Thanks, + Heroku + "}), + OpenJdkBuildpackError::ResolveVersionError(VersionResolveError::ReadSystemPropertiesError(error)) => { + log_error( + "Invalid OpenJDK version selector", + formatdoc! {" + The OpenJDK version selector you specified in your system.properties file is invalid. + Please specify a valid version selector in your system.properties file. + + Details: {error:?} + ", error = error }, + ); + } + OpenJdkBuildpackError::ResolveVersionError(VersionResolveError::OpenJdkArtifactRequirementParseError(OpenJdkArtifactRequirementParseError::OpenJdkVersionParseError(_))) => {} } } diff --git a/buildpacks/jvm/src/layers/openjdk.rs b/buildpacks/jvm/src/layers/openjdk.rs index 9afc4fb7..b2214ab0 100644 --- a/buildpacks/jvm/src/layers/openjdk.rs +++ b/buildpacks/jvm/src/layers/openjdk.rs @@ -5,12 +5,14 @@ use crate::{ util, OpenJdkBuildpack, OpenJdkBuildpackError, JAVA_TOOL_OPTIONS_ENV_VAR_DELIMITER, JAVA_TOOL_OPTIONS_ENV_VAR_NAME, JDK_OVERLAY_DIR_NAME, }; +use buildpacks_jvm_shared::output; +use buildpacks_jvm_shared::output::{BuildpackOutputText, BuildpackOutputTextSection}; use fs_extra::dir::CopyOptions; use libcnb::additional_buildpack_binary_path; use libcnb::build::BuildContext; use libcnb::data::layer_name; use libcnb::layer::{ - CachedLayerDefinition, InvalidMetadataAction, LayerState, RestoredLayerAction, + CachedLayerDefinition, EmptyLayerCause, InvalidMetadataAction, LayerState, RestoredLayerAction, }; use libcnb::layer_env::{LayerEnv, ModificationBehavior, Scope}; use libherokubuildpack::inventory::artifact::Artifact; @@ -25,6 +27,8 @@ pub(crate) fn handle_openjdk_layer( context: &BuildContext, artifact: &Artifact, ) -> libcnb::Result<(), OpenJdkBuildpackError> { + output::print_section("OpenJDK Installation"); + let layer_ref = context.cached_layer( layer_name!("openjdk"), CachedLayerDefinition { @@ -36,55 +40,119 @@ pub(crate) fn handle_openjdk_layer( || metadata.jdk_overlay_applied { // Since the JDK overlay will modify the OpenJDK distribution and the cached version - // might already have an (potentially different) overlay applied, we re-crate the layer + // might already have a (potentially different) overlay applied, we re-crate the layer // in that case. - RestoredLayerAction::DeleteLayer - } else if artifact.url == metadata.source_tarball_url { - RestoredLayerAction::KeepLayer + ( + RestoredLayerAction::DeleteLayer, + OpenJdkLayerCause::OverlayUsed, + ) + } else if artifact.url != metadata.source_tarball_url { + ( + RestoredLayerAction::DeleteLayer, + OpenJdkLayerCause::VersionChanged, + ) } else { - RestoredLayerAction::DeleteLayer + ( + RestoredLayerAction::KeepLayer, + OpenJdkLayerCause::RestoredLayerValid, + ) } }, }, )?; match layer_ref.state { - LayerState::Restored { .. } => {} - LayerState::Empty { .. } => { - libherokubuildpack::log::log_header(format!("Installing OpenJDK {}", artifact.version)); - - let temp_dir = tempdir().map_err(OpenJdkBuildpackError::CannotCreateOpenJdkTempDir)?; - let path = temp_dir.path().join("openjdk.tar.gz"); - - libherokubuildpack::download::download_file(&artifact.url, &path) - .map_err(OpenJdkBuildpackError::OpenJdkDownloadError)?; - - std::fs::File::open(&path) - .map_err(OpenJdkBuildpackError::CannotReadOpenJdkTarball) - .and_then(|file| { - digest::(file).map_err(OpenJdkBuildpackError::CannotReadOpenJdkTarball) - }) - .and_then(|downloaded_file_digest| { - if downloaded_file_digest.as_slice() == artifact.checksum.value { - Ok(()) - } else { - Err(OpenJdkBuildpackError::OpenJdkTarballChecksumError { - expected: artifact.checksum.value.clone(), - actual: downloaded_file_digest.to_vec(), - }) - } - })?; + LayerState::Restored { .. } => { + output::print_subsection("Using cached OpenJDK installation from previous build"); + } + LayerState::Empty { ref cause } => { + match cause { + EmptyLayerCause::InvalidMetadataAction { .. } => { + output::print_subsection("Clearing OpenJDK cache (invalid metadata)"); + } + EmptyLayerCause::RestoredLayerAction { + cause: OpenJdkLayerCause::OverlayUsed, + } => output::print_subsection("Clearing OpenJDK cache (JDK overlay used)"), + EmptyLayerCause::RestoredLayerAction { + cause: OpenJdkLayerCause::VersionChanged, + } => output::print_subsection("Clearing OpenJDK cache (version changed)"), + _ => {} + } - std::fs::File::open(&path) - .map_err(OpenJdkBuildpackError::CannotReadOpenJdkTarball) - .and_then(|mut file| { - libherokubuildpack::tar::decompress_tarball(&mut file, layer_ref.path()) - .map_err(OpenJdkBuildpackError::CannotDecompressOpenJdkTarball) - })?; + output::track_timing(|| { + output::print_subsection("Downloading and unpacking OpenJDK distribution"); + + let temp_dir = + tempdir().map_err(OpenJdkBuildpackError::CannotCreateOpenJdkTempDir)?; + let path = temp_dir.path().join("openjdk.tar.gz"); + + libherokubuildpack::download::download_file(&artifact.url, &path) + .map_err(OpenJdkBuildpackError::OpenJdkDownloadError)?; + + std::fs::File::open(&path) + .map_err(OpenJdkBuildpackError::CannotReadOpenJdkTarball) + .and_then(|file| { + digest::(file) + .map_err(OpenJdkBuildpackError::CannotReadOpenJdkTarball) + }) + .and_then(|downloaded_file_digest| { + if downloaded_file_digest.as_slice() == artifact.checksum.value { + Ok(()) + } else { + Err(OpenJdkBuildpackError::OpenJdkTarballChecksumError { + expected: artifact.checksum.value.clone(), + actual: downloaded_file_digest.to_vec(), + }) + } + })?; + + std::fs::File::open(&path) + .map_err(OpenJdkBuildpackError::CannotReadOpenJdkTarball) + .and_then(|mut file| { + libherokubuildpack::tar::decompress_tarball(&mut file, layer_ref.path()) + .map_err(OpenJdkBuildpackError::CannotDecompressOpenJdkTarball) + }) + })?; + output::print_section("Applying JDK overlay"); let app_jdk_overlay_dir_path = context.app_dir.join(JDK_OVERLAY_DIR_NAME); - let ubuntu_java_cacerts_file_path = PathBuf::from("/etc/ssl/certs/java/cacerts"); + let mut jdk_overlay_applied = false; + if app_jdk_overlay_dir_path.is_dir() { + output::print_subsection(BuildpackOutputText::new(vec![ + BuildpackOutputTextSection::regular("Copying files from "), + BuildpackOutputTextSection::value(JDK_OVERLAY_DIR_NAME), + BuildpackOutputTextSection::regular(" to OpenJDK directory"), + ])); + + jdk_overlay_applied = true; + + output::track_timing(|| { + let jdk_overlay_contents = + util::list_directory_contents(&app_jdk_overlay_dir_path) + .map_err(OpenJdkBuildpackError::CannotListJdkOverlayContents)?; + + fs_extra::copy_items( + &jdk_overlay_contents, + layer_ref.path(), + &CopyOptions { + overwrite: true, + skip_exist: false, + copy_inside: true, + ..CopyOptions::default() + }, + ) + .map_err(OpenJdkBuildpackError::CannotCopyJdkOverlayContents) + })?; + } else { + output::print_subsection(BuildpackOutputText::new(vec![ + BuildpackOutputTextSection::regular("Skipping (directory "), + BuildpackOutputTextSection::value(JDK_OVERLAY_DIR_NAME), + BuildpackOutputTextSection::regular(" not present)"), + ])); + } + + output::print_section("Linking base image certificates as OpenJDK keystore"); // Depending on OpenJDK version, the path for the cacerts file can differ. let relative_jdk_cacerts_path = ["jre/lib/security/cacerts", "lib/security/cacerts"] @@ -92,12 +160,21 @@ pub(crate) fn handle_openjdk_layer( .find(|path| layer_ref.path().join(path).is_file()) .ok_or(OpenJdkBuildpackError::MissingJdkCertificatesFile)?; - let symlink_ubuntu_java_cacerts_file = ubuntu_java_cacerts_file_path.is_file() - && !app_jdk_overlay_dir_path - .join(relative_jdk_cacerts_path) - .exists(); + let overlay_has_cacerts = app_jdk_overlay_dir_path + .join(relative_jdk_cacerts_path) + .exists(); + + let ubuntu_java_cacerts_file_path = PathBuf::from("/etc/ssl/certs/java/cacerts"); - if symlink_ubuntu_java_cacerts_file { + if overlay_has_cacerts { + output::print_subsection(BuildpackOutputText::new(vec![ + BuildpackOutputTextSection::regular("Skipping (overlay at "), + BuildpackOutputTextSection::value(JDK_OVERLAY_DIR_NAME), + BuildpackOutputTextSection::regular(" contains "), + BuildpackOutputTextSection::value(*relative_jdk_cacerts_path), + BuildpackOutputTextSection::regular("file)"), + ])); + } else if ubuntu_java_cacerts_file_path.is_file() { let absolute_jdk_cacerts_path = layer_ref.path().join(relative_jdk_cacerts_path); std::fs::rename( @@ -113,26 +190,16 @@ pub(crate) fn handle_openjdk_layer( absolute_jdk_cacerts_path, ) .map_err(OpenJdkBuildpackError::CannotSymlinkUbuntuCertificates)?; - } - let mut jdk_overlay_applied = false; - if app_jdk_overlay_dir_path.is_dir() { - jdk_overlay_applied = true; - - let jdk_overlay_contents = util::list_directory_contents(&app_jdk_overlay_dir_path) - .map_err(OpenJdkBuildpackError::CannotListJdkOverlayContents)?; - - fs_extra::copy_items( - &jdk_overlay_contents, - layer_ref.path(), - &CopyOptions { - overwrite: true, - skip_exist: false, - copy_inside: true, - ..CopyOptions::default() - }, - ) - .map_err(OpenJdkBuildpackError::CannotCopyJdkOverlayContents)?; + output::print_subsection("Done."); + } else { + output::print_subsection(BuildpackOutputText::new(vec![ + BuildpackOutputTextSection::regular("Skipping ("), + BuildpackOutputTextSection::value( + ubuntu_java_cacerts_file_path.to_string_lossy(), + ), + BuildpackOutputTextSection::regular(" does not exist)"), + ])); } layer_ref.write_metadata(OpenJdkLayerMetadata { @@ -177,3 +244,9 @@ pub(crate) struct OpenJdkLayerMetadata { jdk_overlay_applied: bool, source_tarball_url: String, } + +pub(crate) enum OpenJdkLayerCause { + OverlayUsed, + VersionChanged, + RestoredLayerValid, +} diff --git a/buildpacks/jvm/src/main.rs b/buildpacks/jvm/src/main.rs index 333677f4..22daf746 100644 --- a/buildpacks/jvm/src/main.rs +++ b/buildpacks/jvm/src/main.rs @@ -5,6 +5,7 @@ mod openjdk_artifact; mod openjdk_version; mod salesforce_functions; mod util; +mod version_resolver; use crate::constants::OPENJDK_LATEST_LTS_VERSION; use crate::errors::on_error_jvm_buildpack; @@ -12,10 +13,13 @@ use crate::layers::openjdk::handle_openjdk_layer; use crate::layers::runtime::handle_runtime_layer; use crate::openjdk_artifact::{ HerokuOpenJdkVersionRequirement, OpenJdkArtifactMetadata, OpenJdkArtifactRequirement, - OpenJdkArtifactRequirementParseError, OpenJdkDistribution, }; use crate::openjdk_version::OpenJdkVersion; -use crate::salesforce_functions::is_salesforce_function_app; +use crate::version_resolver::{ + resolve_version, OpenJdkArtifactRequirementSource, VersionResolveError, +}; +use buildpacks_jvm_shared::output; +use buildpacks_jvm_shared::output::{BuildpackOutputText, BuildpackOutputTextSection}; use buildpacks_jvm_shared::system_properties::{read_system_properties, ReadSystemPropertiesError}; #[cfg(test)] use buildpacks_jvm_shared_test as _; @@ -34,7 +38,6 @@ use libcnb_test as _; use libherokubuildpack::download::DownloadError; use libherokubuildpack::inventory::artifact::{Arch, Os}; use libherokubuildpack::inventory::{Inventory, ParseInventoryError}; -use libherokubuildpack::log::log_warning; use sha2::Sha256; use std::env::consts; use url as _; // Used by exec.d binary @@ -47,15 +50,15 @@ enum OpenJdkBuildpackError { OpenJdkDownloadError(DownloadError), CannotCreateOpenJdkTempDir(std::io::Error), CannotReadOpenJdkTarball(std::io::Error), + ReadSystemPropertiesError(ReadSystemPropertiesError), OpenJdkTarballChecksumError { expected: Vec, actual: Vec }, CannotDecompressOpenJdkTarball(std::io::Error), - ReadSystemPropertiesError(ReadSystemPropertiesError), - OpenJdkArtifactRequirementParseError(OpenJdkArtifactRequirementParseError), MissingJdkCertificatesFile, CannotSymlinkUbuntuCertificates(std::io::Error), CannotListJdkOverlayContents(std::io::Error), CannotCopyJdkOverlayContents(fs_extra::error::Error), ParseInventoryError(ParseInventoryError), + ResolveVersionError(VersionResolveError), } impl Buildpack for OpenJdkBuildpack { @@ -67,7 +70,7 @@ impl Buildpack for OpenJdkBuildpack { // This buildpack is first and foremost a buildpack that is designed for composing with // other buildpacks, usually with JVM build tools such as Maven or Gradle. To enable // other buildpacks to conditionally require the installation of OpenJDK, the detect of this - // buildpack wil fail if no other buildpack requires "jdk". + // buildpack will fail if no other buildpack requires "jdk". // // Some users might want to install OpenJDK without using another buildpack, which wouldn't // work with this buildpack since "jdk" would not be required in the build plan. @@ -89,32 +92,16 @@ impl Buildpack for OpenJdkBuildpack { } fn build(&self, context: BuildContext) -> libcnb::Result { - let openjdk_artifact_requirement = read_system_properties(&context.app_dir) - .map_err(OpenJdkBuildpackError::ReadSystemPropertiesError) - .map(|properties| properties.get("java.runtime.version").cloned()) - .and_then(|string| { - string - .map(|string| { - string - .parse::() - .map_err(OpenJdkBuildpackError::OpenJdkArtifactRequirementParseError) - }) - .transpose() - })?; - - let openjdk_artifact_requirement = if let Some(openjdk_artifact_requirement) = - openjdk_artifact_requirement - { - openjdk_artifact_requirement - // The default version for Salesforce functions is always OpenJDK 8. Keep this conditional - // around until Salesforce functions is EOL and then remove it. - } else if is_salesforce_function_app(&context.app_dir) { - OpenJdkArtifactRequirement { - version: HerokuOpenJdkVersionRequirement::Major(8), - distribution: OpenJdkDistribution::default(), - } - } else { - log_warning( + output::print_buildpack_name("Heroku OpenJDK Buildpack"); + + let resolved_version = resolve_version(&context.app_dir) + .map_err(OpenJdkBuildpackError::ResolveVersionError)?; + + if matches!( + resolved_version.source, + OpenJdkArtifactRequirementSource::DefaultVersionLatestLts + ) { + output::print_warning( "No OpenJDK version specified", formatdoc! {" Your application does not explicitly specify an OpenJDK version. The latest @@ -130,10 +117,25 @@ impl Buildpack for OpenJdkBuildpack { java.runtime.version = {OPENJDK_LATEST_LTS_VERSION} "}, ); + } + + output::print_section("OpenJDK version resolution"); - OpenJdkArtifactRequirement { - version: HerokuOpenJdkVersionRequirement::Major(OPENJDK_LATEST_LTS_VERSION), - distribution: OpenJdkDistribution::default(), + match resolved_version.source { + OpenJdkArtifactRequirementSource::SystemProperties => { + output::print_subsection(BuildpackOutputText::new(vec![ + BuildpackOutputTextSection::regular("Using version string provided in "), + BuildpackOutputTextSection::value("system.properties"), + ])); + } + OpenJdkArtifactRequirementSource::DefaultVersionLatestLts => { + output::print_subsection("No explicit configuration found, using latest LTS"); + } + OpenJdkArtifactRequirementSource::DefaultVersionFunctions => { + output::print_subsection(BuildpackOutputText::new(vec![ + BuildpackOutputTextSection::regular("No explicit configuration found, using "), + BuildpackOutputTextSection::value("8"), + ])); } }; @@ -157,12 +159,27 @@ impl Buildpack for OpenJdkBuildpack { .unwrap_or(consts::ARCH) .parse::() .expect("arch should be always parseable, buildpack will not run on unsupported architectures."), - &openjdk_artifact_requirement, + &resolved_version.requirement, ) .ok_or(OpenJdkBuildpackError::UnsupportedOpenJdkVersion( - openjdk_artifact_requirement, + resolved_version.requirement.clone(), ))?; + output::print_subsection(match resolved_version.requirement.version { + HerokuOpenJdkVersionRequirement::Major(major_version) => { + BuildpackOutputText::new(vec![ + BuildpackOutputTextSection::regular("Selected major version "), + BuildpackOutputTextSection::value(format!("{major_version}")), + BuildpackOutputTextSection::regular(" resolves to "), + BuildpackOutputTextSection::value(format!("{}", openjdk_artifact.version)), + ]) + } + HerokuOpenJdkVersionRequirement::Specific(version) => BuildpackOutputText::new(vec![ + BuildpackOutputTextSection::regular("Selected version "), + BuildpackOutputTextSection::value(format!("{version}")), + ]), + }); + handle_openjdk_layer(&context, openjdk_artifact)?; handle_runtime_layer(&context)?; diff --git a/buildpacks/jvm/src/version_resolver.rs b/buildpacks/jvm/src/version_resolver.rs new file mode 100644 index 00000000..199fb4e2 --- /dev/null +++ b/buildpacks/jvm/src/version_resolver.rs @@ -0,0 +1,68 @@ +use crate::constants::OPENJDK_LATEST_LTS_VERSION; +use crate::openjdk_artifact::{ + HerokuOpenJdkVersionRequirement, OpenJdkArtifactRequirement, + OpenJdkArtifactRequirementParseError, OpenJdkDistribution, +}; +use crate::salesforce_functions::is_salesforce_function_app; +use buildpacks_jvm_shared::system_properties::{read_system_properties, ReadSystemPropertiesError}; +use std::path::Path; + +pub(crate) fn resolve_version(app_dir: &Path) -> Result { + let openjdk_artifact_requirement = read_system_properties(app_dir) + .map_err(VersionResolveError::ReadSystemPropertiesError) + .map(|properties| properties.get("java.runtime.version").cloned()) + .and_then(|string| { + string + .map(|string| { + string + .parse::() + .map_err(VersionResolveError::OpenJdkArtifactRequirementParseError) + }) + .transpose() + })?; + + let result = match openjdk_artifact_requirement { + // The default version for Salesforce functions is always OpenJDK 8. Keep this conditional + // around until Salesforce functions is EOL and then remove it. + None if is_salesforce_function_app(app_dir) => ResolveResult { + source: OpenJdkArtifactRequirementSource::DefaultVersionFunctions, + requirement: OpenJdkArtifactRequirement { + version: HerokuOpenJdkVersionRequirement::Major(8), + distribution: OpenJdkDistribution::default(), + }, + }, + None => ResolveResult { + source: OpenJdkArtifactRequirementSource::DefaultVersionLatestLts, + requirement: OpenJdkArtifactRequirement { + version: HerokuOpenJdkVersionRequirement::Major(OPENJDK_LATEST_LTS_VERSION), + distribution: OpenJdkDistribution::default(), + }, + }, + Some(requirement) => ResolveResult { + source: OpenJdkArtifactRequirementSource::SystemProperties, + requirement, + }, + }; + + Ok(result) +} + +pub(crate) struct ResolveResult { + pub(crate) requirement: OpenJdkArtifactRequirement, + pub(crate) source: OpenJdkArtifactRequirementSource, +} + +#[derive(Debug, thiserror::Error)] +pub(crate) enum VersionResolveError { + #[error("{0:?}")] + ReadSystemPropertiesError(ReadSystemPropertiesError), + #[error("{0:?}")] + OpenJdkArtifactRequirementParseError(OpenJdkArtifactRequirementParseError), +} + +#[derive(Eq, PartialEq)] +pub(crate) enum OpenJdkArtifactRequirementSource { + SystemProperties, + DefaultVersionLatestLts, + DefaultVersionFunctions, +} diff --git a/buildpacks/maven/Cargo.toml b/buildpacks/maven/Cargo.toml index 60d9e2c5..130309f6 100644 --- a/buildpacks/maven/Cargo.toml +++ b/buildpacks/maven/Cargo.toml @@ -11,7 +11,7 @@ buildpacks-jvm-shared.workspace = true flate2 = { version = "1", default-features = false, features = ["zlib"] } indoc = "2" libcnb = "=0.26.0" -libherokubuildpack = { version = "=0.26.0", default-features = false, features = ["digest", "download", "error", "log"] } +libherokubuildpack = { version = "=0.26.0", default-features = false, features = ["digest", "download", "error"] } regex = "1" serde = { version = "1", features = ["derive"] } shell-words = "1" diff --git a/buildpacks/maven/src/main.rs b/buildpacks/maven/src/main.rs index b15660e6..7a372dc5 100644 --- a/buildpacks/maven/src/main.rs +++ b/buildpacks/maven/src/main.rs @@ -13,7 +13,6 @@ use libcnb::detect::{DetectContext, DetectResult, DetectResultBuilder}; use libcnb::generic::GenericPlatform; use libcnb::{buildpack_main, Buildpack, Env, Error, Platform}; use libherokubuildpack::download::DownloadError; -use libherokubuildpack::log::{log_header, log_info}; use serde::{Deserialize, Serialize}; use std::collections::HashMap; use std::fs; @@ -22,6 +21,8 @@ use std::os::unix::fs::PermissionsExt; use std::path::{Path, PathBuf}; use std::process::{Command, ExitStatus}; +use buildpacks_jvm_shared::output; +use buildpacks_jvm_shared::output::{BuildpackOutputText, BuildpackOutputTextSection}; #[cfg(test)] use buildpacks_jvm_shared_test as _; #[cfg(test)] @@ -101,6 +102,8 @@ impl Buildpack for MavenBuildpack { #[allow(clippy::too_many_lines)] fn build(&self, context: BuildContext) -> libcnb::Result { + output::print_buildpack_name("Heroku Maven Buildpack"); + let mut current_or_platform_env = Env::from_current(); for (key, value) in context.platform.env() { current_or_platform_env.insert(key, value); @@ -115,11 +118,11 @@ impl Buildpack for MavenBuildpack { ) .map_err(MavenBuildpackError::DetermineModeError)?; - log_header("Installing Maven"); + output::print_section("Installing Maven"); let mvn_executable = match maven_mode { Mode::UseWrapper => { - log_info("Maven wrapper detected, skipping installation."); + output::print_subsection("Skipping (Maven wrapper detected)"); let maven_wrapper_path = context.app_dir.join("mvnw"); @@ -141,19 +144,24 @@ impl Buildpack for MavenBuildpack { log_default_maven_version_warning(&version); } - log_info(format!("Selected Maven version: {}", &version)); - - let tarball = context - .buildpack_descriptor - .metadata - .tarballs - .get(&version) - .cloned() - .ok_or_else(|| MavenBuildpackError::UnsupportedMavenVersion(version.clone()))?; - - handle_maven_layer(&context, &tarball, &mut mvn_env)?; - - log_info(format!("Successfully installed Apache Maven {}", &version)); + output::print_subsection(BuildpackOutputText::new(vec![ + BuildpackOutputTextSection::regular("Selected Maven version "), + BuildpackOutputTextSection::value(&version), + ])); + + output::track_timing(|| { + let tarball = context + .buildpack_descriptor + .metadata + .tarballs + .get(&version) + .cloned() + .ok_or_else(|| { + MavenBuildpackError::UnsupportedMavenVersion(version.clone()) + })?; + + handle_maven_layer(&context, &tarball, &mut mvn_env) + })?; PathBuf::from("mvn") } @@ -201,16 +209,21 @@ impl Buildpack for MavenBuildpack { // running since they might be confusing to the user. let internal_maven_options = vec![String::from("-B")]; - log_header("Executing Maven"); - log_info(format!( - "$ {} {} {}", - mvn_executable.to_string_lossy(), - shell_words::join(&maven_options), - shell_words::join(&maven_goals) - )); - - util::run_command( - Command::new(&mvn_executable) + output::print_section("Running Maven build"); + output::print_subsection(BuildpackOutputText::new(vec![ + BuildpackOutputTextSection::regular("Running "), + BuildpackOutputTextSection::value(format!( + "{} {} {}", + mvn_executable.to_string_lossy(), + shell_words::join(&maven_options), + shell_words::join(&maven_goals) + )), + ])); + + output::track_timing(|| { + let mut command = Command::new(&mvn_executable); + + command .current_dir(&context.app_dir) .args( maven_options @@ -218,13 +231,29 @@ impl Buildpack for MavenBuildpack { .chain(&internal_maven_options) .chain(&maven_goals), ) - .envs(&mvn_env), - MavenBuildpackError::MavenBuildIoError, - MavenBuildpackError::MavenBuildUnexpectedExitCode, - )?; - - util::run_command( - Command::new(&mvn_executable) + .envs(&mvn_env); + + output::run_command( + command, + false, + MavenBuildpackError::MavenBuildIoError, + |output| MavenBuildpackError::MavenBuildUnexpectedExitCode(output.status), + ) + })?; + + output::print_section(BuildpackOutputText::new(vec![ + BuildpackOutputTextSection::regular("Running "), + BuildpackOutputTextSection::value(format!( + "{} dependency:list", + mvn_executable.to_string_lossy() + )), + BuildpackOutputTextSection::regular(" quietly"), + ])); + + output::track_timing(|| { + let mut command = Command::new(&mvn_executable); + + command .current_dir(&context.app_dir) .args( maven_options.iter().chain(&internal_maven_options).chain( @@ -238,10 +267,15 @@ impl Buildpack for MavenBuildpack { .iter(), ), ) - .envs(&mvn_env), - MavenBuildpackError::MavenBuildIoError, - MavenBuildpackError::MavenBuildUnexpectedExitCode, - )?; + .envs(&mvn_env); + + output::run_command( + command, + true, + MavenBuildpackError::MavenBuildIoError, + |output| MavenBuildpackError::MavenBuildUnexpectedExitCode(output.status), + ) + })?; let mut build_result_builder = BuildResultBuilder::new(); diff --git a/buildpacks/maven/src/util.rs b/buildpacks/maven/src/util.rs index 35a64c3a..85878e45 100644 --- a/buildpacks/maven/src/util.rs +++ b/buildpacks/maven/src/util.rs @@ -1,27 +1,8 @@ use flate2::read::GzDecoder; use std::fs::File; use std::path::{Path, PathBuf}; -use std::process::{Command, ExitStatus}; use tar::Archive; -pub(crate) fn run_command E, F2: FnOnce(ExitStatus) -> E>( - command: &mut Command, - io_error_fn: F, - exit_status_fn: F2, -) -> Result { - command - .spawn() - .and_then(|mut child| child.wait()) - .map_err(io_error_fn) - .and_then(|exit_status| { - if exit_status.success() { - Ok(exit_status) - } else { - Err(exit_status_fn(exit_status)) - } - }) -} - pub(crate) fn extract_tarball( file: &mut File, destination: &Path, diff --git a/buildpacks/sbt/Cargo.toml b/buildpacks/sbt/Cargo.toml index f8baa62b..d2eac72d 100644 --- a/buildpacks/sbt/Cargo.toml +++ b/buildpacks/sbt/Cargo.toml @@ -11,7 +11,7 @@ buildpacks-jvm-shared.workspace = true indoc = "2" java-properties = "2" libcnb = "=0.26.0" -libherokubuildpack = { version = "=0.26.0", default-features = false, features = ["command", "error", "log"] } +libherokubuildpack = { version = "=0.26.0", default-features = false, features = ["error"] } semver = { version = "1", features = ["serde"] } serde = { version = "1", features = ["derive"] } shell-words = "1" diff --git a/buildpacks/sbt/sbt-extras b/buildpacks/sbt/sbt-extras index e16dab99..d27dd2fc 160000 --- a/buildpacks/sbt/sbt-extras +++ b/buildpacks/sbt/sbt-extras @@ -1 +1 @@ -Subproject commit e16dab993203b611b9592ddae7b8822a68ae16ec +Subproject commit d27dd2fc291796618b2f32c8945f1384366eedd4 diff --git a/buildpacks/sbt/src/main.rs b/buildpacks/sbt/src/main.rs index 362aa673..56f63112 100644 --- a/buildpacks/sbt/src/main.rs +++ b/buildpacks/sbt/src/main.rs @@ -19,12 +19,11 @@ use libcnb::data::build_plan::BuildPlanBuilder; use libcnb::detect::{DetectContext, DetectResult, DetectResultBuilder}; use libcnb::generic::{GenericMetadata, GenericPlatform}; use libcnb::{buildpack_main, Buildpack, Env, Error, Platform}; -use libherokubuildpack::command::CommandExt; use libherokubuildpack::error::on_error as on_buildpack_error; -use libherokubuildpack::log::{log_header, log_info}; -use std::io::{stderr, stdout}; use std::process::Command; +use buildpacks_jvm_shared::output; +use buildpacks_jvm_shared::output::{BuildpackOutputText, BuildpackOutputTextSection}; #[cfg(test)] use buildpacks_jvm_shared_test as _; #[cfg(test)] @@ -61,6 +60,8 @@ impl Buildpack for SbtBuildpack { } fn build(&self, context: BuildContext) -> libcnb::Result { + output::print_buildpack_name("Heroku sbt Buildpack"); + let buildpack_configuration = read_system_properties(&context.app_dir) .map_err(SbtBuildpackError::ReadSystemPropertiesError) .and_then(|system_properties| { @@ -102,24 +103,30 @@ impl Buildpack for SbtBuildpack { handle_sbt_boot(&context, sbt_version, sbt_available_at_launch, &mut env)?; handle_sbt_global(&context, sbt_available_at_launch, &mut env)?; - log_header("Building Scala project"); - let tasks = sbt::tasks::from_config(&buildpack_configuration); - log_info(format!("Running: sbt {}", shell_words::join(&tasks))); - - let output = Command::new("sbt") - .current_dir(&context.app_dir) - .args(tasks) - .envs(&env) - .output_and_write_streams(stdout(), stderr()) - .map_err(SbtBuildpackError::SbtBuildIoError)?; - - output.status.success().then_some(()).ok_or( - SbtBuildpackError::SbtBuildUnexpectedExitStatus( - output.status, - sbt::output::parse_errors(&output.stdout), - ), - )?; + + output::track_timing(|| { + output::print_section("Running sbt build"); + output::print_subsection(BuildpackOutputText::new(vec![ + BuildpackOutputTextSection::regular("Running "), + BuildpackOutputTextSection::value(format!("sbt {}", shell_words::join(&tasks))), + ])); + + let mut command = Command::new("sbt"); + command.current_dir(&context.app_dir).args(tasks).envs(&env); + + output::run_command( + command, + false, + SbtBuildpackError::SbtBuildIoError, + |output| { + SbtBuildpackError::SbtBuildUnexpectedExitStatus( + output.status, + sbt::output::parse_errors(&output.stdout), + ) + }, + ) + })?; BuildResultBuilder::new().build() } diff --git a/shared/Cargo.toml b/shared/Cargo.toml index eff637ce..cd38f4d2 100644 --- a/shared/Cargo.toml +++ b/shared/Cargo.toml @@ -9,4 +9,4 @@ workspace = true [dependencies] indoc = "2" java-properties = "2" -libherokubuildpack = { version = "=0.26.0", default-features = false, features = ["log"] } +libherokubuildpack = { version = "=0.26.0", default-features = false, features = ["log", "command", "write"] } diff --git a/shared/src/lib.rs b/shared/src/lib.rs index d430247a..c3c0a362 100644 --- a/shared/src/lib.rs +++ b/shared/src/lib.rs @@ -2,3 +2,4 @@ pub mod fs; pub mod log; pub mod result; pub mod system_properties; +pub mod output; diff --git a/shared/src/output.rs b/shared/src/output.rs new file mode 100644 index 00000000..a265a2ad --- /dev/null +++ b/shared/src/output.rs @@ -0,0 +1,365 @@ +use libherokubuildpack::command::CommandExt; +use libherokubuildpack::write::line_mapped; +use std::process::{Command, Output}; +use std::time::{Duration, Instant}; + +pub fn print_buildpack_name(buildpack_name: impl AsRef) { + let buildpack_name = buildpack_name.as_ref(); + println!("{ANSI_BUILDPACK_NAME_CODE}# {buildpack_name}{ANSI_RESET_CODE}\n"); +} + +pub fn print_section(text: impl Into) { + let text = text.into().to_ansi_string(); + println!("{ANSI_RESET_CODE}- {text}"); +} + +pub fn print_subsection(text: impl Into) { + let text = text.into().to_ansi_string(); + println!("{ANSI_RESET_CODE} - {text}"); +} + +pub fn print_timing_done_subsection(duration: &Duration) { + println!("{ANSI_RESET_CODE} - Done ({})", format_duration(duration)); +} + +pub fn print_warning(title: impl AsRef, body: impl Into) { + let title = title.as_ref(); + + let mut sections = vec![BuildpackOutputTextSection::regular(format!( + "WARNING: {title}\n\n" + ))]; + + let mut body = body.into(); + sections.append(&mut body.sections); + + let text = BuildpackOutputText { + default_code: Some(String::from(ANSI_YELLOW_CODE)), + line_prefix: Some(String::from("! ")), + sections, + ..BuildpackOutputText::default() + }; + + println!("{}", text.to_ansi_string()); +} + +pub fn print_error(title: impl AsRef, body: impl Into) { + let title = title.as_ref(); + + let mut sections = vec![BuildpackOutputTextSection::regular(format!( + "ERROR: {title}\n\n" + ))]; + + let mut body = body.into(); + sections.append(&mut body.sections); + + let text = BuildpackOutputText { + default_code: Some(String::from(ANSI_RED_CODE)), + line_prefix: Some(String::from(ERROR_WARNING_LINE_PREFIX)), + sections, + ..BuildpackOutputText::default() + }; + + println!("{}", text.to_ansi_string()); +} + +pub fn run_command E, F2: FnOnce(Output) -> E>( + mut command: Command, + quiet: bool, + io_error_fn: F, + exit_status_fn: F2, +) -> Result { + let child = if quiet { + command.output_and_write_streams(std::io::sink(), std::io::sink()) + } else { + command.output_and_write_streams( + line_mapped(std::io::stdout(), add_prefix_to_non_empty(" ")), + line_mapped(std::io::stderr(), add_prefix_to_non_empty(" ")), + ) + }; + + child.map_err(io_error_fn).and_then(|output| { + if output.status.success() { + Ok(output) + } else { + Err(exit_status_fn(output)) + } + }) +} + +fn add_prefix_to_non_empty>>(prefix: P) -> impl Fn(Vec) -> Vec { + let prefix = prefix.into(); + + move |mut input| { + if input.is_empty() { + vec![] + } else { + let mut result = prefix.clone(); + result.append(&mut input); + result + } + } +} + +#[derive(Clone, Debug)] +pub struct BuildpackOutputText { + pub line_prefix: Option, + pub default_code: Option, + pub reset_code: String, + pub value_code: String, + pub sections: Vec, +} + +impl Default for BuildpackOutputText { + fn default() -> Self { + Self { + line_prefix: None, + default_code: None, + reset_code: String::from(ANSI_RESET_CODE), + value_code: String::from(ANSI_VALUE_CODE), + sections: vec![], + } + } +} + +impl BuildpackOutputText { + pub fn new(sections: impl Into>) -> Self { + Self { + sections: sections.into(), + ..Self::default() + } + } + + fn to_ansi_string(&self) -> String { + let mut result = String::new(); + + // Every line must start with a style reset, the default ANSI code and the line prefix if + // it exists. + let line_start = format!( + "{}{}{}", + ANSI_RESET_CODE, + self.default_code.clone().unwrap_or_default(), + self.line_prefix.clone().unwrap_or_default() + ); + + result.push_str(&line_start); + + for section in &self.sections { + let text = match section { + BuildpackOutputTextSection::Regular(text) + | BuildpackOutputTextSection::Value(text) + | BuildpackOutputTextSection::Url(text) + | BuildpackOutputTextSection::Command(text) => text, + }; + + match section { + BuildpackOutputTextSection::Regular(_) => {} + BuildpackOutputTextSection::Value(_) => { + result.push_str(ANSI_VALUE_CODE); + result.push(VALUE_DELIMITER_CHAR); + } + BuildpackOutputTextSection::Url(_) => { + result.push_str(ANSI_URL_CODE); + } + BuildpackOutputTextSection::Command(_) => { + result.push_str(ANSI_COMMAND_CODE); + } + } + + for char in text.chars() { + if char == '\n' { + // Before ending a line, reset the text style so that the styling does not + // interfere with i.e. `pack` output. + result.push_str(ANSI_RESET_CODE); + + result.push('\n'); + + result.push_str(&line_start); + + if let BuildpackOutputTextSection::Value(_) = section { + result.push_str(ANSI_VALUE_CODE); + } + } else { + result.push(char); + } + } + + if let BuildpackOutputTextSection::Value(_) = section { + result.push(VALUE_DELIMITER_CHAR); + result.push_str(ANSI_RESET_CODE); + result.push_str(&self.default_code.clone().unwrap_or_default()); + } + } + + result + } +} + +#[derive(Clone, Debug)] +pub enum BuildpackOutputTextSection { + Regular(String), + Value(String), + Url(String), + Command(String), +} + +impl BuildpackOutputTextSection { + pub fn regular(value: impl Into) -> Self { + BuildpackOutputTextSection::Regular(value.into()) + } + + pub fn value(value: impl Into) -> Self { + BuildpackOutputTextSection::Value(value.into()) + } +} + +impl From for BuildpackOutputText { + fn from(value: String) -> Self { + Self { + sections: vec![BuildpackOutputTextSection::Regular(value)], + ..Self::default() + } + } +} + +impl From<&str> for BuildpackOutputText { + fn from(value: &str) -> Self { + Self { + sections: vec![BuildpackOutputTextSection::Regular(String::from(value))], + ..Self::default() + } + } +} + +impl From> for BuildpackOutputText { + fn from(value: Vec) -> Self { + Self { + sections: value, + ..Self::default() + } + } +} + +pub fn track_timing(f: F) -> Result +where + F: FnOnce() -> Result, +{ + let start_time = Instant::now(); + let ret = f(); + let end_time = Instant::now(); + + print_timing_done_subsection(&end_time.duration_since(start_time)); + ret +} + +fn format_duration(duration: &Duration) -> String { + let hours = (duration.as_secs() / 3600) % 60; + let minutes = (duration.as_secs() / 60) % 60; + let seconds = duration.as_secs() % 60; + let milliseconds = duration.subsec_millis(); + let tenths = milliseconds / 100; + + if hours > 0 { + format!("{hours}h {minutes}m {seconds}s") + } else if minutes > 0 { + format!("{minutes}m {seconds}s") + } else if seconds > 0 || milliseconds >= 100 { + format!("{seconds}.{tenths}s") + } else { + String::from("< 0.1s") + } +} + +const VALUE_DELIMITER_CHAR: char = '`'; +const ANSI_RESET_CODE: &str = "\u{1b}[0m"; +const ANSI_VALUE_CODE: &str = "\u{1b}[0;34m"; +const ANSI_YELLOW_CODE: &str = "\u{1b}[0;33m"; +const ANSI_RED_CODE: &str = "\u{1b}[0;31m"; +const ANSI_BUILDPACK_NAME_CODE: &str = "\u{1b}[1;35m"; +const ANSI_URL_CODE: &str = "\u{1b}[0;34m"; +const ANSI_COMMAND_CODE: &str = "\u{1b}[0;34m"; +const ERROR_WARNING_LINE_PREFIX: &str = "! "; + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_prefixing() { + const DEFAULT_CODE: &str = "\x1B[0;33m"; + + let text = BuildpackOutputText { + default_code: Some(String::from(DEFAULT_CODE)), + sections: vec![ + BuildpackOutputTextSection::regular("Hello\n"), + BuildpackOutputTextSection::value("World"), + BuildpackOutputTextSection::regular("\n"), + BuildpackOutputTextSection::regular("How\nare you?"), + ], + line_prefix: Some(String::from(ERROR_WARNING_LINE_PREFIX)), + ..Default::default() + }; + + assert_eq!(text.to_ansi_string(), "\u{1b}[0m\u{1b}[0;33m! Hello\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! \u{1b}[0;34m`World`\u{1b}[0m\u{1b}[0;33m\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! How\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! are you?"); + } + + #[test] + fn test_prefixing_with_value() { + let text = BuildpackOutputText { + default_code: Some(String::from(ANSI_YELLOW_CODE)), + sections: vec![ + BuildpackOutputTextSection::regular("Intro\n"), + BuildpackOutputTextSection::value("With\nNewline"), + BuildpackOutputTextSection::regular("\nOutro"), + ], + line_prefix: Some(String::from("! ")), + ..Default::default() + }; + + assert_eq!( + text.to_ansi_string(), + "\u{1b}[0m\u{1b}[0;33m! Intro\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! \u{1b}[0;34m`With\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! \u{1b}[0;34mNewline`\u{1b}[0m\u{1b}[0;33m\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! Outro" + ); + } + + #[test] + fn test_display_duration() { + let duration = Duration::ZERO; + assert_eq!(format_duration(&duration), "< 0.1s"); + + let duration = Duration::from_millis(99); + assert_eq!(format_duration(&duration), "< 0.1s"); + + let duration = Duration::from_millis(100); + assert_eq!(format_duration(&duration), "0.1s"); + + let duration = Duration::from_millis(210); + assert_eq!(format_duration(&duration), "0.2s"); + + let duration = Duration::from_millis(1100); + assert_eq!(format_duration(&duration), "1.1s"); + + let duration = Duration::from_millis(9100); + assert_eq!(format_duration(&duration), "9.1s"); + + let duration = Duration::from_millis(10100); + assert_eq!(format_duration(&duration), "10.1s"); + + let duration = Duration::from_millis(52100); + assert_eq!(format_duration(&duration), "52.1s"); + + let duration = Duration::from_millis(60 * 1000); + assert_eq!(format_duration(&duration), "1m 0s"); + + let duration = Duration::from_millis(60 * 1000 + 2000); + assert_eq!(format_duration(&duration), "1m 2s"); + + let duration = Duration::from_millis(60 * 60 * 1000 - 1); + assert_eq!(format_duration(&duration), "59m 59s"); + + let duration = Duration::from_millis(60 * 60 * 1000); + assert_eq!(format_duration(&duration), "1h 0m 0s"); + + let duration = Duration::from_millis(75 * 60 * 1000 - 1); + assert_eq!(format_duration(&duration), "1h 14m 59s"); + } +} From a8872eb915556f916c92bfdb3ea05e44d1f24faa Mon Sep 17 00:00:00 2001 From: Manuel Fuchs Date: Fri, 29 Nov 2024 12:34:33 +0100 Subject: [PATCH 2/8] Update CHANGELOG.md --- buildpacks/jvm/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildpacks/jvm/CHANGELOG.md b/buildpacks/jvm/CHANGELOG.md index e5bda584..8db9c3c1 100644 --- a/buildpacks/jvm/CHANGELOG.md +++ b/buildpacks/jvm/CHANGELOG.md @@ -24,7 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Default version for **OpenJDK 17** is now `17.0.13`. ([#747](https://github.com/heroku/buildpacks-jvm/pull/747)) - Default version for **OpenJDK 21** is now `21.0.5`. ([#747](https://github.com/heroku/buildpacks-jvm/pull/747)) - Default version for **OpenJDK 23** is now `23.0.1`. ([#747](https://github.com/heroku/buildpacks-jvm/pull/747)) -- Buildpack output changed to a new format. ([#000](https://github.com/heroku/buildpacks-jvm/pull/000)) +- Buildpack output changed to a new format. ([#745](https://github.com/heroku/buildpacks-jvm/pull/745)) ## [6.0.3] - 2024-09-26 From c03db1f4fd3583324c4b8fb0039618eeb8b85248 Mon Sep 17 00:00:00 2001 From: Manuel Fuchs Date: Fri, 29 Nov 2024 13:27:45 +0100 Subject: [PATCH 3/8] Fix integration tests that match output --- buildpacks/jvm/src/main.rs | 3 +- buildpacks/jvm/tests/integration/versions.rs | 32 ++++++++----------- buildpacks/maven/tests/integration/misc.rs | 1 - .../maven/tests/integration/versions.rs | 12 +++---- shared/src/lib.rs | 2 +- shared/src/output.rs | 4 +-- 6 files changed, 24 insertions(+), 30 deletions(-) diff --git a/buildpacks/jvm/src/main.rs b/buildpacks/jvm/src/main.rs index 22daf746..7ab8f74e 100644 --- a/buildpacks/jvm/src/main.rs +++ b/buildpacks/jvm/src/main.rs @@ -114,8 +114,7 @@ impl Buildpack for OpenJdkBuildpack { To set the OpenJDK version, add or edit the system.properties file in the root directory of your application to contain: - java.runtime.version = {OPENJDK_LATEST_LTS_VERSION} - "}, + java.runtime.version = {OPENJDK_LATEST_LTS_VERSION}"}, ); } diff --git a/buildpacks/jvm/tests/integration/versions.rs b/buildpacks/jvm/tests/integration/versions.rs index 7881ccbe..3a3fa7b6 100644 --- a/buildpacks/jvm/tests/integration/versions.rs +++ b/buildpacks/jvm/tests/integration/versions.rs @@ -18,20 +18,19 @@ fn openjdk_default() { assert_contains!( context.pack_stderr, &formatdoc! {" - [Warning: No OpenJDK version specified] - Your application does not explicitly specify an OpenJDK version. The latest - long-term support (LTS) version will be installed. This currently is OpenJDK 21. - - This default version will change when a new LTS version is released. Your - application might fail to build with the new version. We recommend explicitly - setting the required OpenJDK version for your application. - - To set the OpenJDK version, add or edit the system.properties file in the root - directory of your application to contain: - - java.runtime.version = 21 - "} - ); + ! WARNING: No OpenJDK version specified + ! + ! Your application does not explicitly specify an OpenJDK version. The latest + ! long-term support (LTS) version will be installed. This currently is OpenJDK 21. + ! + ! This default version will change when a new LTS version is released. Your + ! application might fail to build with the new version. We recommend explicitly + ! setting the required OpenJDK version for your application. + ! + ! To set the OpenJDK version, add or edit the system.properties file in the root + ! directory of your application to contain: + ! + ! java.runtime.version = 21"}); assert_contains!( context.run_shell_command("java -version").stderr, @@ -53,10 +52,7 @@ fn openjdk_functions_default() { BuildpackReference::WorkspaceBuildpack(buildpack_id!("heroku/maven")), ]), |context| { - assert_not_contains!( - context.pack_stderr, - "[Warning: No OpenJDK version specified]" - ); + assert_not_contains!(context.pack_stderr, "No OpenJDK version specified"); assert_contains!( context.run_shell_command("java -version").stderr, diff --git a/buildpacks/maven/tests/integration/misc.rs b/buildpacks/maven/tests/integration/misc.rs index 6699d70f..18570d43 100644 --- a/buildpacks/maven/tests/integration/misc.rs +++ b/buildpacks/maven/tests/integration/misc.rs @@ -106,7 +106,6 @@ fn no_internal_maven_options_logging() { |context| { assert_not_contains!(context.pack_stdout, "-Dmaven.repo.local="); assert_not_contains!(context.pack_stdout, "-Duser.home="); - assert_not_contains!(context.pack_stdout, "dependency:list"); assert_not_contains!( context.pack_stdout, "-DoutputFile=target/mvn-dependency-list.log" diff --git a/buildpacks/maven/tests/integration/versions.rs b/buildpacks/maven/tests/integration/versions.rs index 6e9c140b..457a6727 100644 --- a/buildpacks/maven/tests/integration/versions.rs +++ b/buildpacks/maven/tests/integration/versions.rs @@ -7,9 +7,9 @@ use std::path::Path; #[ignore = "integration test"] fn with_wrapper() { TestRunner::default().build( default_build_config("test-apps/simple-http-service"), |context| { - assert_not_contains!(context.pack_stdout, "Selected Maven version:"); - assert_contains!(context.pack_stdout, "Maven wrapper detected, skipping installation."); - assert_contains!(context.pack_stdout, "$ ./mvnw"); + assert_not_contains!(context.pack_stdout, " - Selected Maven version"); + assert_contains!(context.pack_stdout, "- Skipping (Maven wrapper detected)"); + assert_contains!(context.pack_stdout, " - Running `./mvnw"); assert_contains!(context.pack_stdout, &format!("[BUILDPACK INTEGRATION TEST - MAVEN VERSION] {SIMPLE_HTTP_SERVICE_MAVEN_WRAPPER_VERSION}")); }); } @@ -25,7 +25,7 @@ fn with_wrapper_and_system_properties() { |context| { assert_contains!( context.pack_stdout, - &format!("Selected Maven version: {DEFAULT_MAVEN_VERSION}") + &format!(" - Selected Maven version `{DEFAULT_MAVEN_VERSION}`") ); assert_not_contains!(context.pack_stdout, "$ ./mvnw"); assert_contains!( @@ -61,7 +61,7 @@ fn without_wrapper_and_without_system_properties() { assert_not_contains!(context.pack_stdout, "$ ./mvnw"); assert_contains!( context.pack_stdout, - &format!("Selected Maven version: {DEFAULT_MAVEN_VERSION}") + &format!(" - Selected Maven version `{DEFAULT_MAVEN_VERSION}`") ); assert_contains!( context.pack_stdout, @@ -95,7 +95,7 @@ fn without_wrapper_and_maven_3_9_4_system_properties() { set_maven_version_app_dir_preprocessor("3.9.4", &path); }), |context| { - assert_contains!(context.pack_stdout, "Selected Maven version: 3.9.4"); + assert_contains!(context.pack_stdout, " - Selected Maven version `3.9.4`"); assert_contains!( context.pack_stdout, "[BUILDPACK INTEGRATION TEST - MAVEN VERSION] 3.9.4" diff --git a/shared/src/lib.rs b/shared/src/lib.rs index c3c0a362..de2bc25c 100644 --- a/shared/src/lib.rs +++ b/shared/src/lib.rs @@ -1,5 +1,5 @@ pub mod fs; pub mod log; +pub mod output; pub mod result; pub mod system_properties; -pub mod output; diff --git a/shared/src/output.rs b/shared/src/output.rs index a265a2ad..d609a00c 100644 --- a/shared/src/output.rs +++ b/shared/src/output.rs @@ -39,7 +39,7 @@ pub fn print_warning(title: impl AsRef, body: impl Into, body: impl Into) { @@ -59,7 +59,7 @@ pub fn print_error(title: impl AsRef, body: impl Into) ..BuildpackOutputText::default() }; - println!("{}", text.to_ansi_string()); + eprintln!("{}", text.to_ansi_string()); } pub fn run_command E, F2: FnOnce(Output) -> E>( From ac75514545516934dfedcf286a89510b2bc45e9d Mon Sep 17 00:00:00 2001 From: Manuel Fuchs Date: Tue, 3 Dec 2024 15:47:59 +0100 Subject: [PATCH 4/8] Add error message for system.properties parse errors --- buildpacks/jvm/src/errors.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/buildpacks/jvm/src/errors.rs b/buildpacks/jvm/src/errors.rs index 8903327a..5359650f 100644 --- a/buildpacks/jvm/src/errors.rs +++ b/buildpacks/jvm/src/errors.rs @@ -123,7 +123,7 @@ pub(crate) fn on_error_jvm_buildpack(error: OpenJdkBuildpackError) { Thanks, Heroku "}), - OpenJdkBuildpackError::ResolveVersionError(VersionResolveError::ReadSystemPropertiesError(error)) => { + OpenJdkBuildpackError::ResolveVersionError(VersionResolveError::OpenJdkArtifactRequirementParseError(OpenJdkArtifactRequirementParseError::OpenJdkVersionParseError(_))) => { log_error( "Invalid OpenJDK version selector", formatdoc! {" @@ -134,6 +134,16 @@ pub(crate) fn on_error_jvm_buildpack(error: OpenJdkBuildpackError) { ", error = error }, ); } - OpenJdkBuildpackError::ResolveVersionError(VersionResolveError::OpenJdkArtifactRequirementParseError(OpenJdkArtifactRequirementParseError::OpenJdkVersionParseError(_))) => {} + OpenJdkBuildpackError::ResolveVersionError(VersionResolveError::ReadSystemPropertiesError(error)) => { + log_error( + "Invalid system.properties file", + formatdoc! {" + The contents of your system.properties file cannot be parsed. Please use a valid + system.properties file and try again. + + Details: {error:?} + ", error = error }, + ); + } } } From f7615a07f2fb4546d5402587c919e36a8a16308d Mon Sep 17 00:00:00 2001 From: Richard Schneeman Date: Fri, 20 Dec 2024 06:03:55 -0600 Subject: [PATCH 5/8] Print getting started guides in CI (#761) --- .github/workflows/ci.yml | 68 +++++++++++++++++++++++++++++++++++++ CONTRIBUTING.md | 8 +++++ buildpacks/jvm/CHANGELOG.md | 4 +++ 3 files changed, 80 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 318b557a..05b41d97 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -96,3 +96,71 @@ jobs: working-directory: ${{ matrix.buildpack-directory }} # Runs only tests annotated with the `ignore` attribute (which in this repo, are the integration tests). run: cargo test --locked -- --ignored --test-threads 16 + + print-pack-getting-started-output: + runs-on: ${{ matrix.target == 'aarch64-unknown-linux-musl' && 'pub-hk-ubuntu-24.04-arm-medium' || 'ubuntu-24.04' }} + strategy: + matrix: + target: ["aarch64-unknown-linux-musl", "x86_64-unknown-linux-musl"] + guide: ["heroku/java-getting-started", "heroku/gradle-getting-started", "heroku/scala-getting-started"] + fail-fast: false + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + submodules: true + - name: Install musl-tools + run: sudo apt-get install -y --no-install-recommends musl-tools + - name: Update Rust toolchain + run: rustup update + - name: Install Rust linux-musl target + run: rustup target add ${{ matrix.target }} + - name: Rust Cache + uses: Swatinem/rust-cache@v2.7.5 + - name: Install Pack CLI + uses: buildpacks/github-actions/setup-pack@v5.7.4 + - name: Pull builder and run images + run: | + docker pull "heroku/builder:24" + docker pull "heroku/heroku:24" + - name: Clone getting started guide + uses: actions/checkout@v4 + with: + repository: ${{ matrix.guide }} + path: tmp/guide + - name: Install libcnb-cargo for `cargo libcnb package` command + run: cargo install libcnb-cargo + - name: Compile buildpack + run: cargo libcnb package --target ${{ matrix.target }} + - name: "PRINT: Getting started guide output" + run: | + set -euo pipefail + + PACK_CMD="pack build my-image --force-color --builder heroku/builder:24 --trust-extra-buildpacks --path tmp/guide --pull-policy never " + case "${{ matrix.guide }}" in + "heroku/java-getting-started") + PACK_CMD+=" --buildpack packaged/${{ matrix.target }}/debug/heroku_jvm " + PACK_CMD+=" --buildpack packaged/${{ matrix.target }}/debug/heroku_java " + PACK_CMD+=" --buildpack packaged/${{ matrix.target }}/debug/heroku_maven " + ;; + "heroku/gradle-getting-started") + PACK_CMD+=" --buildpack packaged/${{ matrix.target }}/debug/heroku_jvm " + PACK_CMD+=" --buildpack packaged/${{ matrix.target }}/debug/heroku_gradle " + ;; + "heroku/scala-getting-started") + PACK_CMD+=" --buildpack packaged/${{ matrix.target }}/debug/heroku_jvm " + PACK_CMD+=" --buildpack packaged/${{ matrix.target }}/debug/heroku_sbt " + PACK_CMD+=" --buildpack packaged/${{ matrix.target }}/debug/heroku_scala " + ;; + *) + echo "Unknown guide: ${{ matrix.guide }}" + exit 1 + ;; + esac + + echo "Running command: $PACK_CMD" + bash -c "$PACK_CMD" + echo "" + echo "With CACHE example" + echo "" + bash -c "$PACK_CMD" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index be6a26df..d3318043 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -50,6 +50,14 @@ This buildpack relies on [heroku/libcnb.rs][libcnb] to compile buildpacks. All [libcnb.rs dependencies][libcnb-deps] will need to be setup prior to building or testing this buildpack. +### Clone + +This project uses submodules, to clone all code run: + +``` +$ git clone --recursive https://github.com/heroku/buildpacks-jvm +``` + ### Building 1. Run `cargo check` to download dependencies and ensure there are no diff --git a/buildpacks/jvm/CHANGELOG.md b/buildpacks/jvm/CHANGELOG.md index 8db9c3c1..33a70be7 100644 --- a/buildpacks/jvm/CHANGELOG.md +++ b/buildpacks/jvm/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - JDK overlays are now properly sourced from the `.jdk-overlay` directory instead of `.jdk_overlay`. ([#763](https://github.com/heroku/buildpacks-jvm/pull/763)) +### Changed + +- Buildpack output changed to a new format. ([#000](https://github.com/heroku/buildpacks-jvm/pull/000)) + ## [6.0.4] - 2024-12-05 ### Added From 89cfdd80105719e94293b3e111fcbc4073d43954 Mon Sep 17 00:00:00 2001 From: Manuel Fuchs Date: Fri, 20 Dec 2024 13:15:28 +0100 Subject: [PATCH 6/8] Properly reset on Url and Command --- shared/src/output.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/shared/src/output.rs b/shared/src/output.rs index d609a00c..e94a37b8 100644 --- a/shared/src/output.rs +++ b/shared/src/output.rs @@ -183,10 +183,15 @@ impl BuildpackOutputText { } } - if let BuildpackOutputTextSection::Value(_) = section { - result.push(VALUE_DELIMITER_CHAR); - result.push_str(ANSI_RESET_CODE); - result.push_str(&self.default_code.clone().unwrap_or_default()); + match section { + BuildpackOutputTextSection::Value(_) + | BuildpackOutputTextSection::Url(_) + | BuildpackOutputTextSection::Command(_) => { + result.push(VALUE_DELIMITER_CHAR); + result.push_str(ANSI_RESET_CODE); + result.push_str(&self.default_code.clone().unwrap_or_default()); + } + BuildpackOutputTextSection::Regular(_) => {} } } From ed348841fd7e4bc4209e67ffe2b454ac28a69aae Mon Sep 17 00:00:00 2001 From: Manuel Fuchs Date: Fri, 20 Dec 2024 13:20:10 +0100 Subject: [PATCH 7/8] Fix style for Url and Command --- shared/src/output.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/shared/src/output.rs b/shared/src/output.rs index e94a37b8..115e9f6f 100644 --- a/shared/src/output.rs +++ b/shared/src/output.rs @@ -154,13 +154,15 @@ impl BuildpackOutputText { match section { BuildpackOutputTextSection::Regular(_) => {} BuildpackOutputTextSection::Value(_) => { - result.push_str(ANSI_VALUE_CODE); result.push(VALUE_DELIMITER_CHAR); + result.push_str(ANSI_VALUE_CODE); } BuildpackOutputTextSection::Url(_) => { + result.push(VALUE_DELIMITER_CHAR); result.push_str(ANSI_URL_CODE); } BuildpackOutputTextSection::Command(_) => { + result.push(VALUE_DELIMITER_CHAR); result.push_str(ANSI_COMMAND_CODE); } } @@ -175,8 +177,13 @@ impl BuildpackOutputText { result.push_str(&line_start); - if let BuildpackOutputTextSection::Value(_) = section { - result.push_str(ANSI_VALUE_CODE); + match section { + BuildpackOutputTextSection::Value(_) + | BuildpackOutputTextSection::Url(_) + | BuildpackOutputTextSection::Command(_) => { + result.push_str(ANSI_VALUE_CODE); + } + BuildpackOutputTextSection::Regular(_) => {} } } else { result.push(char); @@ -187,8 +194,8 @@ impl BuildpackOutputText { BuildpackOutputTextSection::Value(_) | BuildpackOutputTextSection::Url(_) | BuildpackOutputTextSection::Command(_) => { - result.push(VALUE_DELIMITER_CHAR); result.push_str(ANSI_RESET_CODE); + result.push(VALUE_DELIMITER_CHAR); result.push_str(&self.default_code.clone().unwrap_or_default()); } BuildpackOutputTextSection::Regular(_) => {} @@ -304,7 +311,7 @@ mod test { ..Default::default() }; - assert_eq!(text.to_ansi_string(), "\u{1b}[0m\u{1b}[0;33m! Hello\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! \u{1b}[0;34m`World`\u{1b}[0m\u{1b}[0;33m\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! How\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! are you?"); + assert_eq!(text.to_ansi_string(), "\u{1b}[0m\u{1b}[0;33m! Hello\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! `\u{1b}[0;34mWorld\u{1b}[0m`\u{1b}[0;33m\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! How\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! are you?"); } #[test] @@ -322,7 +329,7 @@ mod test { assert_eq!( text.to_ansi_string(), - "\u{1b}[0m\u{1b}[0;33m! Intro\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! \u{1b}[0;34m`With\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! \u{1b}[0;34mNewline`\u{1b}[0m\u{1b}[0;33m\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! Outro" + "\u{1b}[0m\u{1b}[0;33m! Intro\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! `\u{1b}[0;34mWith\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! \u{1b}[0;34mNewline\u{1b}[0m`\u{1b}[0;33m\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! Outro" ); } From 3b140fce22ba8f186f393676fb3f02acfee03b68 Mon Sep 17 00:00:00 2001 From: Manuel Fuchs Date: Thu, 16 Jan 2025 18:10:56 +0100 Subject: [PATCH 8/8] Tweak output --- buildpacks/gradle/src/main.rs | 10 ++++-- .../bin/heroku_database_env_var_rewrite.rs | 2 +- buildpacks/jvm/src/layers/openjdk.rs | 2 +- buildpacks/maven/src/main.rs | 2 +- buildpacks/sbt/src/main.rs | 2 +- shared/src/output.rs | 31 +++++++++++++------ 6 files changed, 33 insertions(+), 16 deletions(-) diff --git a/buildpacks/gradle/src/main.rs b/buildpacks/gradle/src/main.rs index 63ba44ab..2cc7d4f1 100644 --- a/buildpacks/gradle/src/main.rs +++ b/buildpacks/gradle/src/main.rs @@ -7,7 +7,8 @@ use crate::layers::gradle_home::handle_gradle_home_layer; use crate::GradleBuildpackError::{GradleBuildIoError, GradleBuildUnexpectedStatusError}; use buildpacks_jvm_shared as shared; use buildpacks_jvm_shared::output::{ - print_buildpack_name, print_section, print_subsection, track_timing, + print_buildpack_name, print_section, print_subsection, track_timing, BuildpackOutputText, + BuildpackOutputTextSection, }; #[cfg(test)] use buildpacks_jvm_shared_test as _; @@ -20,7 +21,6 @@ use libcnb::{buildpack_main, Buildpack, Env}; #[cfg(test)] use libcnb_test as _; use libherokubuildpack::command::CommandExt; -use libherokubuildpack::log::log_header; use serde::Deserialize; use std::io::{stderr, stdout}; use std::process::{Command, ExitStatus}; @@ -126,7 +126,11 @@ impl Buildpack for GradleBuildpack { }) .ok_or(GradleBuildpackError::BuildTaskUnknown)?; - log_header("Running build task"); + print_section("Running Gradle build"); + print_subsection(BuildpackOutputText::new(vec![ + BuildpackOutputTextSection::regular("Running "), + BuildpackOutputTextSection::command(format!("./gradlew {task_name} -x check")), + ])); let output = Command::new(&gradle_wrapper_executable_path) .current_dir(&context.app_dir) diff --git a/buildpacks/jvm/src/bin/heroku_database_env_var_rewrite.rs b/buildpacks/jvm/src/bin/heroku_database_env_var_rewrite.rs index 3413c74a..ca871d29 100644 --- a/buildpacks/jvm/src/bin/heroku_database_env_var_rewrite.rs +++ b/buildpacks/jvm/src/bin/heroku_database_env_var_rewrite.rs @@ -42,7 +42,7 @@ fn jvm_env_vars_for_env( // Handling for Spring specific JDBC environment variables let disable_spring_datasource_url = input .get("DISABLE_SPRING_DATASOURCE_URL") - .map_or(false, |value| value == "true"); + .is_some_and(|value| value == "true"); if !disable_spring_datasource_url && !input.contains_key("SPRING_DATASOURCE_URL") diff --git a/buildpacks/jvm/src/layers/openjdk.rs b/buildpacks/jvm/src/layers/openjdk.rs index b2214ab0..0b72cbde 100644 --- a/buildpacks/jvm/src/layers/openjdk.rs +++ b/buildpacks/jvm/src/layers/openjdk.rs @@ -191,7 +191,7 @@ pub(crate) fn handle_openjdk_layer( ) .map_err(OpenJdkBuildpackError::CannotSymlinkUbuntuCertificates)?; - output::print_subsection("Done."); + output::print_subsection("Done"); } else { output::print_subsection(BuildpackOutputText::new(vec![ BuildpackOutputTextSection::regular("Skipping ("), diff --git a/buildpacks/maven/src/main.rs b/buildpacks/maven/src/main.rs index 7a372dc5..b746711b 100644 --- a/buildpacks/maven/src/main.rs +++ b/buildpacks/maven/src/main.rs @@ -212,7 +212,7 @@ impl Buildpack for MavenBuildpack { output::print_section("Running Maven build"); output::print_subsection(BuildpackOutputText::new(vec![ BuildpackOutputTextSection::regular("Running "), - BuildpackOutputTextSection::value(format!( + BuildpackOutputTextSection::command(format!( "{} {} {}", mvn_executable.to_string_lossy(), shell_words::join(&maven_options), diff --git a/buildpacks/sbt/src/main.rs b/buildpacks/sbt/src/main.rs index 56f63112..86177c71 100644 --- a/buildpacks/sbt/src/main.rs +++ b/buildpacks/sbt/src/main.rs @@ -109,7 +109,7 @@ impl Buildpack for SbtBuildpack { output::print_section("Running sbt build"); output::print_subsection(BuildpackOutputText::new(vec![ BuildpackOutputTextSection::regular("Running "), - BuildpackOutputTextSection::value(format!("sbt {}", shell_words::join(&tasks))), + BuildpackOutputTextSection::command(format!("sbt {}", shell_words::join(&tasks))), ])); let mut command = Command::new("sbt"); diff --git a/shared/src/output.rs b/shared/src/output.rs index 115e9f6f..2c41e67c 100644 --- a/shared/src/output.rs +++ b/shared/src/output.rs @@ -5,7 +5,7 @@ use std::time::{Duration, Instant}; pub fn print_buildpack_name(buildpack_name: impl AsRef) { let buildpack_name = buildpack_name.as_ref(); - println!("{ANSI_BUILDPACK_NAME_CODE}# {buildpack_name}{ANSI_RESET_CODE}\n"); + print!("\n{ANSI_BUILDPACK_NAME_CODE}# {buildpack_name}{ANSI_RESET_CODE}\n\n"); } pub fn print_section(text: impl Into) { @@ -71,10 +71,19 @@ pub fn run_command E, F2: FnOnce(Output) -> E>( let child = if quiet { command.output_and_write_streams(std::io::sink(), std::io::sink()) } else { - command.output_and_write_streams( - line_mapped(std::io::stdout(), add_prefix_to_non_empty(" ")), - line_mapped(std::io::stderr(), add_prefix_to_non_empty(" ")), - ) + const SPACE_ASCII: u8 = 0x20; + let prefix = vec![SPACE_ASCII; 6]; + + println!(); + + let output = command.output_and_write_streams( + line_mapped(std::io::stdout(), add_prefix_to_non_empty(prefix.clone())), + line_mapped(std::io::stderr(), add_prefix_to_non_empty(prefix)), + ); + + println!(); + + output }; child.map_err(io_error_fn).and_then(|output| { @@ -222,6 +231,10 @@ impl BuildpackOutputTextSection { pub fn value(value: impl Into) -> Self { BuildpackOutputTextSection::Value(value.into()) } + + pub fn command(value: impl Into) -> Self { + BuildpackOutputTextSection::Command(value.into()) + } } impl From for BuildpackOutputText { @@ -283,12 +296,12 @@ fn format_duration(duration: &Duration) -> String { const VALUE_DELIMITER_CHAR: char = '`'; const ANSI_RESET_CODE: &str = "\u{1b}[0m"; -const ANSI_VALUE_CODE: &str = "\u{1b}[0;34m"; +const ANSI_VALUE_CODE: &str = "\u{1b}[0;33m"; const ANSI_YELLOW_CODE: &str = "\u{1b}[0;33m"; const ANSI_RED_CODE: &str = "\u{1b}[0;31m"; const ANSI_BUILDPACK_NAME_CODE: &str = "\u{1b}[1;35m"; const ANSI_URL_CODE: &str = "\u{1b}[0;34m"; -const ANSI_COMMAND_CODE: &str = "\u{1b}[0;34m"; +const ANSI_COMMAND_CODE: &str = "\u{1b}[1;36m"; const ERROR_WARNING_LINE_PREFIX: &str = "! "; #[cfg(test)] @@ -311,7 +324,7 @@ mod test { ..Default::default() }; - assert_eq!(text.to_ansi_string(), "\u{1b}[0m\u{1b}[0;33m! Hello\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! `\u{1b}[0;34mWorld\u{1b}[0m`\u{1b}[0;33m\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! How\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! are you?"); + assert_eq!(text.to_ansi_string(), "\u{1b}[0m\u{1b}[0;33m! Hello\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! `\u{1b}[0;33mWorld\u{1b}[0m`\u{1b}[0;33m\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! How\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! are you?"); } #[test] @@ -329,7 +342,7 @@ mod test { assert_eq!( text.to_ansi_string(), - "\u{1b}[0m\u{1b}[0;33m! Intro\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! `\u{1b}[0;34mWith\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! \u{1b}[0;34mNewline\u{1b}[0m`\u{1b}[0;33m\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! Outro" + "\u{1b}[0m\u{1b}[0;33m! Intro\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! `\u{1b}[0;33mWith\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! \u{1b}[0;33mNewline\u{1b}[0m`\u{1b}[0;33m\u{1b}[0m\n\u{1b}[0m\u{1b}[0;33m! Outro" ); }