From 958a85fc6f180d10c494692a6b4d94a7497ac137 Mon Sep 17 00:00:00 2001 From: Ed Morley <501702+edmorley@users.noreply.github.com> Date: Wed, 3 Jan 2024 10:30:44 +0000 Subject: [PATCH] Fix CI failures on `main` (#757) Fixes the following CI failures on `main`: 1. Lint failures on the newly released Rust 1.75 (see #756) (plus pre-emptively fixes another for Rust 1.76 beta). 2. Integration test failures due to the Ruby example having picked up newer bundler, which is not compatible with the Ruby 2.7 it uses. I've opted to delete the Ruby example entirely, since: 1. This is currently blocking libcnb.rs PRs/development. 2. It has multiple issues that really require a complete rewrite: - https://github.com/heroku/libcnb.rs/issues/398 - https://github.com/heroku/libcnb.rs/issues/479 - https://github.com/heroku/libcnb.rs/issues/746 - https://github.com/heroku/libcnb.rs/issues/755 3. IMO including a full language CNB example in this repo is not something we should do, since it's both never going to fully support the language in a best practice way, and also will be too complicated to demonstrate libcnb concepts in the simplified way that we should be using in an example. Instead, we should stick to simple examples of concepts, and then link out to our real-world CNB repos for users who want to do further reading. As an added bonus, the removal will speed up CI a fair amount, since the Ruby integration test was very slow (due to it bundler plus the test using a different builder image, so another docker pull). Fixes #756. Fixes #755. Closes #398. Closes #479. Closes #746. GUS-W-14739082. GUS-W-14739086. --- Cargo.toml | 1 - examples/ruby-sample/Cargo.toml | 21 --- examples/ruby-sample/buildpack.toml | 12 -- examples/ruby-sample/src/layers/bundler.rs | 125 ------------------ examples/ruby-sample/src/layers/mod.rs | 5 - examples/ruby-sample/src/layers/ruby.rs | 61 --------- examples/ruby-sample/src/main.rs | 87 ------------ examples/ruby-sample/src/util.rs | 75 ----------- .../tests/fixtures/simple-ruby-app/Gemfile | 2 - .../fixtures/simple-ruby-app/Gemfile.lock | 13 -- .../tests/fixtures/simple-ruby-app/app.rb | 12 -- .../ruby-sample/tests/integration_test.rs | 86 ------------ libcnb-test/src/test_runner.rs | 1 + libcnb/src/layer_env.rs | 12 +- 14 files changed, 7 insertions(+), 506 deletions(-) delete mode 100644 examples/ruby-sample/Cargo.toml delete mode 100644 examples/ruby-sample/buildpack.toml delete mode 100644 examples/ruby-sample/src/layers/bundler.rs delete mode 100644 examples/ruby-sample/src/layers/mod.rs delete mode 100644 examples/ruby-sample/src/layers/ruby.rs delete mode 100644 examples/ruby-sample/src/main.rs delete mode 100644 examples/ruby-sample/src/util.rs delete mode 100644 examples/ruby-sample/tests/fixtures/simple-ruby-app/Gemfile delete mode 100644 examples/ruby-sample/tests/fixtures/simple-ruby-app/Gemfile.lock delete mode 100644 examples/ruby-sample/tests/fixtures/simple-ruby-app/app.rb delete mode 100644 examples/ruby-sample/tests/integration_test.rs diff --git a/Cargo.toml b/Cargo.toml index e37e6ccc..25b90a7c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,6 @@ resolver = "2" members = [ "examples/basics", "examples/execd", - "examples/ruby-sample", "libcnb", "libcnb-cargo", "libcnb-common", diff --git a/examples/ruby-sample/Cargo.toml b/examples/ruby-sample/Cargo.toml deleted file mode 100644 index 47a169b4..00000000 --- a/examples/ruby-sample/Cargo.toml +++ /dev/null @@ -1,21 +0,0 @@ -[package] -name = "examples-ruby-sample" -version = "0.0.0" -edition.workspace = true -rust-version.workspace = true -publish = false - -[lints] -workspace = true - -[dependencies] -flate2 = { version = "1.0.28", default-features = false, features = ["zlib"] } -libcnb.workspace = true -serde = "1.0.193" -sha2 = "0.10.8" -tar = { version = "0.4.40", default-features = false } -tempfile = "3.8.1" -ureq = { version = "2.9.1", default-features = false, features = ["tls"] } - -[dev-dependencies] -libcnb-test.workspace = true diff --git a/examples/ruby-sample/buildpack.toml b/examples/ruby-sample/buildpack.toml deleted file mode 100644 index 2419aea4..00000000 --- a/examples/ruby-sample/buildpack.toml +++ /dev/null @@ -1,12 +0,0 @@ -api = "0.9" - -[buildpack] -id = "libcnb-examples/ruby" -version = "0.1.0" -name = "Example libcnb buildpack: ruby" - -[[stacks]] -id = "heroku-20" - -[metadata] -ruby_url = "https://heroku-buildpack-ruby.s3.us-east-1.amazonaws.com/heroku-20/ruby-2.7.4.tgz" diff --git a/examples/ruby-sample/src/layers/bundler.rs b/examples/ruby-sample/src/layers/bundler.rs deleted file mode 100644 index 34e8338c..00000000 --- a/examples/ruby-sample/src/layers/bundler.rs +++ /dev/null @@ -1,125 +0,0 @@ -use crate::RubyBuildpack; -use crate::{util, RubyBuildpackError}; -use libcnb::build::BuildContext; -use libcnb::data::layer_content_metadata::LayerTypes; -use libcnb::layer::{ExistingLayerStrategy, Layer, LayerData, LayerResult, LayerResultBuilder}; -use libcnb::Env; -use serde::Deserialize; -use serde::Serialize; -use std::path::Path; -use std::process::Command; - -#[derive(Deserialize, Serialize, Debug, Clone)] -pub(crate) struct BundlerLayerMetadata { - gemfile_lock_checksum: String, -} - -pub(crate) struct BundlerLayer { - pub(crate) ruby_env: Env, -} - -impl Layer for BundlerLayer { - type Buildpack = RubyBuildpack; - type Metadata = BundlerLayerMetadata; - - fn types(&self) -> LayerTypes { - LayerTypes { - build: true, - launch: true, - cache: true, - } - } - - // TODO: Remove use of unwrap(): https://github.com/heroku/libcnb.rs/issues/746 - #[allow(clippy::unwrap_used)] - fn create( - &self, - context: &BuildContext, - layer_path: &Path, - ) -> Result, RubyBuildpackError> { - println!("---> Installing bundler"); - - util::run_simple_command( - Command::new("gem") - .args(["install", "bundler", "--force"]) - .envs(&self.ruby_env), - RubyBuildpackError::GemInstallBundlerCommandError, - RubyBuildpackError::GemInstallBundlerUnexpectedExitStatus, - )?; - - println!("---> Installing gems"); - - util::run_simple_command( - Command::new("bundle") - .args([ - "install", - "--path", - layer_path.to_str().unwrap(), - "--binstubs", - layer_path.join("bin").to_str().unwrap(), - ]) - .envs(&self.ruby_env), - RubyBuildpackError::BundleInstallCommandError, - RubyBuildpackError::BundleInstallUnexpectedExitStatus, - )?; - - LayerResultBuilder::new(BundlerLayerMetadata { - gemfile_lock_checksum: util::sha256_checksum(context.app_dir.join("Gemfile.lock")) - .map_err(RubyBuildpackError::CouldNotGenerateChecksum)?, - }) - .build() - } - - fn existing_layer_strategy( - &self, - context: &BuildContext, - layer: &LayerData, - ) -> Result { - util::sha256_checksum(context.app_dir.join("Gemfile.lock")) - .map_err(RubyBuildpackError::CouldNotGenerateChecksum) - .map(|checksum| { - if checksum == layer.content_metadata.metadata.gemfile_lock_checksum { - ExistingLayerStrategy::Keep - } else { - ExistingLayerStrategy::Update - } - }) - } - - // TODO: Remove use of unwrap(): https://github.com/heroku/libcnb.rs/issues/746 - #[allow(clippy::unwrap_used)] - fn update( - &self, - context: &BuildContext, - layer: &LayerData, - ) -> Result, RubyBuildpackError> { - println!("---> Reusing gems"); - - util::run_simple_command( - Command::new("bundle") - .args(["config", "--local", "path", layer.path.to_str().unwrap()]) - .envs(&self.ruby_env), - RubyBuildpackError::BundleConfigCommandError, - RubyBuildpackError::BundleConfigUnexpectedExitStatus, - )?; - - util::run_simple_command( - Command::new("bundle") - .args([ - "config", - "--local", - "bin", - layer.path.join("bin").as_path().to_str().unwrap(), - ]) - .envs(&self.ruby_env), - RubyBuildpackError::BundleConfigCommandError, - RubyBuildpackError::BundleConfigUnexpectedExitStatus, - )?; - - LayerResultBuilder::new(BundlerLayerMetadata { - gemfile_lock_checksum: util::sha256_checksum(context.app_dir.join("Gemfile.lock")) - .map_err(RubyBuildpackError::CouldNotGenerateChecksum)?, - }) - .build() - } -} diff --git a/examples/ruby-sample/src/layers/mod.rs b/examples/ruby-sample/src/layers/mod.rs deleted file mode 100644 index a3566ff5..00000000 --- a/examples/ruby-sample/src/layers/mod.rs +++ /dev/null @@ -1,5 +0,0 @@ -mod bundler; -mod ruby; - -pub(crate) use bundler::*; -pub(crate) use ruby::*; diff --git a/examples/ruby-sample/src/layers/ruby.rs b/examples/ruby-sample/src/layers/ruby.rs deleted file mode 100644 index 07c6608f..00000000 --- a/examples/ruby-sample/src/layers/ruby.rs +++ /dev/null @@ -1,61 +0,0 @@ -use crate::util; -use crate::{RubyBuildpack, RubyBuildpackError}; -use libcnb::build::BuildContext; -use libcnb::data::layer_content_metadata::LayerTypes; -use libcnb::generic::GenericMetadata; -use libcnb::layer::{Layer, LayerResult, LayerResultBuilder}; -use libcnb::layer_env::{LayerEnv, ModificationBehavior, Scope}; -use std::path::Path; -use tempfile::NamedTempFile; - -pub(crate) struct RubyLayer; - -impl Layer for RubyLayer { - type Buildpack = RubyBuildpack; - type Metadata = GenericMetadata; - - fn types(&self) -> LayerTypes { - LayerTypes { - build: true, - launch: true, - cache: false, - } - } - - fn create( - &self, - context: &BuildContext, - layer_path: &Path, - ) -> Result, RubyBuildpackError> { - println!("---> Download and extracting Ruby"); - - let ruby_tgz = - NamedTempFile::new().map_err(RubyBuildpackError::CouldNotCreateTemporaryFile)?; - - util::download( - &context.buildpack_descriptor.metadata.ruby_url, - ruby_tgz.path(), - ) - .map_err(RubyBuildpackError::RubyDownloadError)?; - - util::untar(ruby_tgz.path(), layer_path).map_err(RubyBuildpackError::RubyUntarError)?; - - LayerResultBuilder::new(GenericMetadata::default()) - .env( - LayerEnv::new() - .chainable_insert( - Scope::All, - ModificationBehavior::Prepend, - "PATH", - context.app_dir.join(".gem/ruby/2.6.6/bin"), - ) - .chainable_insert( - Scope::All, - ModificationBehavior::Prepend, - "LD_LIBRARY_PATH", - layer_path, - ), - ) - .build() - } -} diff --git a/examples/ruby-sample/src/main.rs b/examples/ruby-sample/src/main.rs deleted file mode 100644 index 776659f5..00000000 --- a/examples/ruby-sample/src/main.rs +++ /dev/null @@ -1,87 +0,0 @@ -use crate::layers::{BundlerLayer, RubyLayer}; -use crate::util::{DownloadError, UntarError}; -use libcnb::build::{BuildContext, BuildResult, BuildResultBuilder}; -use libcnb::data::launch::{LaunchBuilder, ProcessBuilder}; -use libcnb::data::{layer_name, process_type}; -use libcnb::detect::{DetectContext, DetectResult, DetectResultBuilder}; -use libcnb::generic::GenericPlatform; -use libcnb::layer_env::Scope; -use libcnb::{buildpack_main, Buildpack}; -use serde::Deserialize; -use std::process::ExitStatus; - -// Suppress warnings due to the `unused_crate_dependencies` lint not handling integration tests well. -#[cfg(test)] -use libcnb_test as _; - -mod layers; -mod util; - -pub(crate) struct RubyBuildpack; - -impl Buildpack for RubyBuildpack { - type Platform = GenericPlatform; - type Metadata = RubyBuildpackMetadata; - type Error = RubyBuildpackError; - - fn detect(&self, context: DetectContext) -> libcnb::Result { - if context.app_dir.join("Gemfile.lock").exists() { - DetectResultBuilder::pass().build() - } else { - DetectResultBuilder::fail().build() - } - } - - fn build(&self, context: BuildContext) -> libcnb::Result { - println!("---> Ruby Buildpack"); - - let ruby_layer = context.handle_layer(layer_name!("ruby"), RubyLayer)?; - - context.handle_layer( - layer_name!("bundler"), - BundlerLayer { - ruby_env: ruby_layer.env.apply_to_empty(Scope::Build), - }, - )?; - - BuildResultBuilder::new() - .launch( - LaunchBuilder::new() - .process( - ProcessBuilder::new(process_type!("web"), ["bundle"]) - .args(["exec", "ruby", "app.rb"]) - .default(true) - .build(), - ) - .process( - ProcessBuilder::new(process_type!("worker"), ["bundle"]) - .args(["exec", "ruby", "worker.rb"]) - .build(), - ) - .build(), - ) - .build() - } -} - -#[derive(Deserialize, Debug)] -#[serde(deny_unknown_fields)] -pub(crate) struct RubyBuildpackMetadata { - pub(crate) ruby_url: String, -} - -#[derive(Debug)] -pub(crate) enum RubyBuildpackError { - RubyDownloadError(DownloadError), - RubyUntarError(UntarError), - CouldNotCreateTemporaryFile(std::io::Error), - CouldNotGenerateChecksum(std::io::Error), - GemInstallBundlerCommandError(std::io::Error), - GemInstallBundlerUnexpectedExitStatus(ExitStatus), - BundleInstallCommandError(std::io::Error), - BundleInstallUnexpectedExitStatus(ExitStatus), - BundleConfigCommandError(std::io::Error), - BundleConfigUnexpectedExitStatus(ExitStatus), -} - -buildpack_main!(RubyBuildpack); diff --git a/examples/ruby-sample/src/util.rs b/examples/ruby-sample/src/util.rs deleted file mode 100644 index ece4cf0e..00000000 --- a/examples/ruby-sample/src/util.rs +++ /dev/null @@ -1,75 +0,0 @@ -use flate2::read::GzDecoder; -use sha2::Digest; -use std::fs; -use std::io; -use std::path::Path; -use std::process::{Command, ExitStatus}; -use tar::Archive; - -pub(crate) fn download( - uri: impl AsRef, - destination: impl AsRef, -) -> Result<(), DownloadError> { - let mut response_reader = ureq::get(uri.as_ref()) - .call() - .map_err(|err| DownloadError::RequestError(Box::new(err)))? - .into_reader(); - - let mut destination_file = fs::File::create(destination.as_ref()) - .map_err(DownloadError::CouldNotCreateDestinationFile)?; - - io::copy(&mut response_reader, &mut destination_file) - .map_err(DownloadError::CouldNotWriteDestinationFile)?; - - Ok(()) -} - -#[derive(Debug)] -pub(crate) enum DownloadError { - // Boxed to prevent `large_enum_variant` errors since `ureq::Error` is massive. - RequestError(Box), - CouldNotCreateDestinationFile(std::io::Error), - CouldNotWriteDestinationFile(std::io::Error), -} - -pub(crate) fn untar( - path: impl AsRef, - destination: impl AsRef, -) -> Result<(), UntarError> { - let file = fs::File::open(path.as_ref()).map_err(UntarError::CouldNotOpenFile)?; - - Archive::new(GzDecoder::new(file)) - .unpack(destination.as_ref()) - .map_err(UntarError::CouldNotUnpack) -} - -#[derive(Debug)] -pub(crate) enum UntarError { - CouldNotOpenFile(std::io::Error), - CouldNotUnpack(std::io::Error), -} - -pub(crate) fn sha256_checksum(path: impl AsRef) -> Result { - fs::read(path).map(|bytes| format!("{:x}", sha2::Sha256::digest(bytes))) -} - -/// Helper to run very simple commands where we just need to handle I/O errors and non-zero exit -/// codes. Not very useful in complex scenarios, but can cut down the amount of code in simple -/// cases. -pub(crate) fn run_simple_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)) - } - }) -} diff --git a/examples/ruby-sample/tests/fixtures/simple-ruby-app/Gemfile b/examples/ruby-sample/tests/fixtures/simple-ruby-app/Gemfile deleted file mode 100644 index 17d753e1..00000000 --- a/examples/ruby-sample/tests/fixtures/simple-ruby-app/Gemfile +++ /dev/null @@ -1,2 +0,0 @@ -source 'https://rubygems.org' -gem 'minitest' diff --git a/examples/ruby-sample/tests/fixtures/simple-ruby-app/Gemfile.lock b/examples/ruby-sample/tests/fixtures/simple-ruby-app/Gemfile.lock deleted file mode 100644 index 7477f852..00000000 --- a/examples/ruby-sample/tests/fixtures/simple-ruby-app/Gemfile.lock +++ /dev/null @@ -1,13 +0,0 @@ -GEM - remote: https://rubygems.org/ - specs: - minitest (5.14.4) - -PLATFORMS - ruby - -DEPENDENCIES - minitest - -BUNDLED WITH - 2.1.2 diff --git a/examples/ruby-sample/tests/fixtures/simple-ruby-app/app.rb b/examples/ruby-sample/tests/fixtures/simple-ruby-app/app.rb deleted file mode 100644 index b0d09a9f..00000000 --- a/examples/ruby-sample/tests/fixtures/simple-ruby-app/app.rb +++ /dev/null @@ -1,12 +0,0 @@ -require 'socket' - -port = ENV["PORT"] -port = 12345 if port.nil? - -server = TCPServer.new(port) - -loop do - socket = server.accept - socket.print(socket.gets.delete("\n").reverse) - socket.close -end diff --git a/examples/ruby-sample/tests/integration_test.rs b/examples/ruby-sample/tests/integration_test.rs deleted file mode 100644 index 15ebeebf..00000000 --- a/examples/ruby-sample/tests/integration_test.rs +++ /dev/null @@ -1,86 +0,0 @@ -//! All integration tests are skipped by default (using the `ignore` attribute) -//! since performing builds is slow. To run them use: `cargo test -- --ignored`. - -// Required due to: https://github.com/rust-lang/rust/issues/95513 -#![allow(unused_crate_dependencies)] - -use libcnb_test::{ - assert_contains, assert_not_contains, BuildConfig, ContainerConfig, PackResult, TestRunner, -}; -use std::io::{Read, Write}; -use std::net; -use std::net::ToSocketAddrs; -use std::time::Duration; -use std::{fs, io}; - -#[test] -#[ignore = "integration test"] -fn basic() { - let config = BuildConfig::new("heroku/builder:20", "tests/fixtures/simple-ruby-app"); - - TestRunner::default().build(&config, |context| { - assert_contains!(context.pack_stdout, "---> Ruby Buildpack"); - assert_contains!(context.pack_stdout, "---> Installing bundler"); - assert_contains!(context.pack_stdout, "---> Installing gems"); - - context.start_container( - ContainerConfig::new() - .env("PORT", TEST_PORT.to_string()) - .expose_port(TEST_PORT), - |container| { - std::thread::sleep(Duration::from_secs(2)); - - assert_eq!( - call_test_fixture_service( - container.address_for_port(TEST_PORT), - "Hello World!" - ) - .unwrap(), - "!dlroW olleH" - ); - }, - ); - - assert_contains!( - context.run_shell_command("ruby --version").stdout, - "ruby 2.7.0p0" - ); - - context.rebuild(&config, |context| { - assert_not_contains!(context.pack_stdout, "---> Installing bundler"); - assert_not_contains!(context.pack_stdout, "---> Installing gems"); - }); - }); -} - -#[test] -#[ignore = "integration test"] -fn missing_gemfile_lock() { - TestRunner::default().build( - BuildConfig::new("heroku/builder:20", "tests/fixtures/simple-ruby-app") - .app_dir_preprocessor(|path| fs::remove_file(path.join("Gemfile.lock")).unwrap()) - .expected_pack_result(PackResult::Failure), - |context| { - assert_contains!( - context.pack_stdout, - "ERROR: No buildpack groups passed detection." - ); - }, - ); -} - -fn call_test_fixture_service(a: A, payload: impl AsRef) -> io::Result -where - A: ToSocketAddrs, -{ - let mut stream = net::TcpStream::connect(a)?; - - stream.write_all(format!("{}\n", payload.as_ref()).as_bytes())?; - - let mut buffer = Vec::new(); - stream.read_to_end(&mut buffer)?; - - Ok(String::from_utf8_lossy(&buffer).to_string()) -} - -const TEST_PORT: u16 = 12346; diff --git a/libcnb-test/src/test_runner.rs b/libcnb-test/src/test_runner.rs index 08de7782..c2707023 100644 --- a/libcnb-test/src/test_runner.rs +++ b/libcnb-test/src/test_runner.rs @@ -176,6 +176,7 @@ impl TestRunner { } } +#[allow(clippy::struct_field_names)] pub(crate) struct TemporaryDockerResources { pub(crate) build_cache_volume_name: String, pub(crate) image_name: String, diff --git a/libcnb/src/layer_env.rs b/libcnb/src/layer_env.rs index 542163f5..8b2529f7 100644 --- a/libcnb/src/layer_env.rs +++ b/libcnb/src/layer_env.rs @@ -594,6 +594,12 @@ impl LayerEnvDelta { } } +#[cfg(target_family = "unix")] +const PATH_LIST_SEPARATOR: &str = ":"; + +#[cfg(target_family = "windows")] +const PATH_LIST_SEPARATOR: &str = ";"; + #[cfg(test)] mod tests { use std::cmp::Ordering; @@ -925,9 +931,3 @@ mod tests { result } } - -#[cfg(target_family = "unix")] -const PATH_LIST_SEPARATOR: &str = ":"; - -#[cfg(target_family = "windows")] -const PATH_LIST_SEPARATOR: &str = ";";