Merge remote-tracking branch 'origin/main' into malax/cargo

CHANGELOG.md

@@ -47,6 +47,8 @@
 - `BuildpackTomlError::InvalidStarStack` has been replaced by `BuildpackTomlError::InvalidAnyStack`.
 - Update the Ruby example buildpack to no longer use anyhow and better demonstrate the intended way to work with errors.
 - `BuildpackTomlError` has been split into `BuildpackApiError` and `StackError`.
+- `BuildpackApi` no longer implements `FromStr`, use `BuildpackApi::try_from()` instead.
+- Fixed file extension for delimiters when writing `LayerEnv` to disk.
 - Add an external Cargo command for packaging libcnb buildpacks. See the README for usage.
 
 ## [0.3.0] 2021/09/17

README.md

@@ -138,7 +138,7 @@ impl Buildpack for HelloWorldBuildpack {
     // according to the returned value, handle both writing the build plan and exiting with
     // the correct status code for you.
     fn detect(&self, _context: DetectContext<Self>) -> libcnb::Result<DetectResult, Self::Error> {
-        Ok(DetectResultBuilder::pass().build())
+        DetectResultBuilder::pass().build()
     }
 
     // Similar to detect, this method will be called when the CNB lifecycle executes the
@@ -147,15 +147,15 @@ impl Buildpack for HelloWorldBuildpack {
         println!("Hello World!");
         println!("Build runs on stack {}!", context.stack_id);
 
-        Ok(BuildResultBuilder::new()
+        BuildResultBuilder::new()
             .launch(Launch::new().process(Process::new(
                 process_type!("web"),
                 "echo",
                 vec!["Hello World!"],
                 false,
                 true,
             )))
-            .build())
+            .build()
     }
 }
 

examples/example-01-basics/buildpack.toml

@@ -0,0 +1,9 @@
+api = "0.6"
+
+[buildpack]
+id = "libcnb-examples/basics"
+version = "0.1.0"
+name = "Example libcnb buildpack: basics"
+
+[[stacks]]
+id = "heroku-20"

examples/example-01-basics/src/main.rs

@@ -10,12 +10,12 @@ impl Buildpack for BasicBuildpack {
     type Error = GenericError;
 
     fn detect(&self, _context: DetectContext<Self>) -> libcnb::Result<DetectResult, Self::Error> {
-        Ok(DetectResultBuilder::pass().build())
+        DetectResultBuilder::pass().build()
     }
 
     fn build(&self, context: BuildContext<Self>) -> libcnb::Result<BuildResult, Self::Error> {
         println!("Build runs on stack {}!", context.stack_id);
-        Ok(BuildResultBuilder::new().build())
+        BuildResultBuilder::new().build()
     }
 }
 

examples/example-02-ruby-sample/buildpack.toml

@@ -1,13 +1,10 @@
-# Buildpack API version
 api = "0.6"
 
-# Buildpack ID and metadata
 [buildpack]
-id = "com.examples.buildpacks.ruby"
-version = "0.0.1"
-name = "Ruby Buildpack"
+id = "libcnb-examples/ruby"
+version = "0.1.0"
+name = "Example libcnb buildpack: ruby"
 
-# Stacks that the buildpack will work with
 [[stacks]]
 id = "heroku-20"
 

examples/example-02-ruby-sample/src/main.rs

@@ -22,13 +22,11 @@ impl Buildpack for RubyBuildpack {
     type Error = RubyBuildpackError;
 
     fn detect(&self, context: DetectContext<Self>) -> libcnb::Result<DetectResult, Self::Error> {
-        let result = if context.app_dir.join("Gemfile.lock").exists() {
+        if context.app_dir.join("Gemfile.lock").exists() {
             DetectResultBuilder::pass().build()
         } else {
             DetectResultBuilder::fail().build()
-        };
-
-        Ok(result)
+        }
     }
 
     fn build(&self, context: BuildContext<Self>) -> libcnb::Result<BuildResult, Self::Error> {
@@ -43,7 +41,7 @@ impl Buildpack for RubyBuildpack {
             },
         )?;
 
-        Ok(BuildResultBuilder::new()
+        BuildResultBuilder::new()
             .launch(
                 Launch::new()
                     .process(Process::new(
@@ -61,7 +59,7 @@ impl Buildpack for RubyBuildpack {
                         false,
                     )),
             )
-            .build())
+            .build()
     }
 }
 

libcnb-data/Cargo.toml

@@ -12,10 +12,12 @@ readme = "../README.md"
 include = ["src/**/*", "../LICENSE", "../README.md"]
 
 [dependencies]
-lazy_static = "^1.4.0"
 fancy-regex = "^0.7.1"
 semver = { version = "^1.0.4", features = ["serde"] }
 serde = { version = "^1.0.126", features = ["derive"] }
 thiserror = "^1.0.26"
 toml = "^0.5.8"
 libcnb-proc-macros = { path = "../libcnb-proc-macros", version = "0.1.0" }
+
+[dev-dependencies]
+serde_test = "1.0.130"

libcnb-data/src/buildpack/api.rs

@@ -1,61 +1,34 @@
 use std::convert::TryFrom;
+use std::fmt;
 use std::fmt::{Display, Formatter};
-use std::{fmt, str::FromStr};
 
-use fancy_regex::Regex;
-use lazy_static::lazy_static;
 use serde::Deserialize;
 
-// Used as a "shadow" struct to store
-// potentially invalid `BuildpackApi` data when deserializing
-// <https://dev.to/equalma/validate-fields-and-types-in-serde-with-tryfrom-c2n>
-#[derive(Deserialize)]
-struct BuildpackApiUnchecked(String);
-
-impl TryFrom<BuildpackApiUnchecked> for BuildpackApi {
-    type Error = BuildpackApiError;
-
-    fn try_from(value: BuildpackApiUnchecked) -> Result<Self, Self::Error> {
-        Self::from_str(value.0.as_str())
-    }
-}
-
+/// The Buildpack API version.
+///
+/// This MUST be in form `<major>.<minor>` or `<major>`, where `<major>` is equivalent to `<major>.0`.
 #[derive(Deserialize, Debug, Eq, PartialEq)]
-#[serde(try_from = "BuildpackApiUnchecked")]
+#[serde(try_from = "&str")]
 pub struct BuildpackApi {
     pub major: u32,
     pub minor: u32,
 }
 
-impl FromStr for BuildpackApi {
-    type Err = BuildpackApiError;
-
-    fn from_str(value: &str) -> Result<Self, Self::Err> {
-        lazy_static! {
-            static ref RE: Regex = Regex::new(r"^(?P<major>\d+)(\.(?P<minor>\d+))?$").unwrap();
-        }
-
-        if let Some(captures) = RE.captures(value).unwrap_or_default() {
-            if let Some(major) = captures.name("major") {
-                // these should never panic since we check with the regex unless it's greater than
-                // `std::u32::MAX`
-                let major = major
-                    .as_str()
-                    .parse::<u32>()
-                    .map_err(|_| Self::Err::InvalidBuildpackApi(String::from(value)))?;
-
-                // If no minor version is specified default to 0.
-                let minor = captures
-                    .name("minor")
-                    .map_or("0", |s| s.as_str())
-                    .parse::<u32>()
-                    .map_err(|_| Self::Err::InvalidBuildpackApi(String::from(value)))?;
-
-                return Ok(Self { major, minor });
-            }
-        }
+impl TryFrom<&str> for BuildpackApi {
+    type Error = BuildpackApiError;
 
-        Err(Self::Err::InvalidBuildpackApi(String::from(value)))
+    fn try_from(value: &str) -> Result<Self, Self::Error> {
+        // We're not using the `semver` crate, since it only supports non-range versions of form `X.Y.Z`.
+        // If no minor version is specified, it defaults to `0`.
+        let (major, minor) = value.split_once('.').unwrap_or((value, "0"));
+        Ok(Self {
+            major: major
+                .parse()
+                .map_err(|_| Self::Error::InvalidBuildpackApi(String::from(value)))?,
+            minor: minor
+                .parse()
+                .map_err(|_| Self::Error::InvalidBuildpackApi(String::from(value)))?,
+        })
     }
 }
 
@@ -67,38 +40,82 @@ impl Display for BuildpackApi {
 
 #[derive(thiserror::Error, Debug)]
 pub enum BuildpackApiError {
-    #[error("Found `{0}` but value MUST be in the form `<major>.<minor>` or `<major>` and only contain numbers.")]
+    #[error("Invalid Buildpack API version: `{0}`")]
     InvalidBuildpackApi(String),
 }
 
 #[cfg(test)]
 mod tests {
+    use serde_test::{assert_de_tokens, assert_de_tokens_error, Token};
+
     use super::*;
 
     #[test]
-    fn buildpack_api_from_str_major_minor() {
-        let result = BuildpackApi::from_str("0.4");
-        assert!(result.is_ok());
-        if let Ok(api) = result {
-            assert_eq!(0, api.major);
-            assert_eq!(4, api.minor);
-        }
+    fn deserialize_valid_api_versions() {
+        assert_de_tokens(
+            &BuildpackApi { major: 1, minor: 3 },
+            &[Token::BorrowedStr("1.3")],
+        );
+        assert_de_tokens(
+            &BuildpackApi { major: 0, minor: 0 },
+            &[Token::BorrowedStr("0.0")],
+        );
+        assert_de_tokens(
+            &BuildpackApi {
+                major: 2020,
+                minor: 10,
+            },
+            &[Token::BorrowedStr("2020.10")],
+        );
+        assert_de_tokens(
+            &BuildpackApi { major: 2, minor: 0 },
+            &[Token::BorrowedStr("2")],
+        );
     }
 
     #[test]
-    fn buildpack_api_from_str_major() {
-        let result = BuildpackApi::from_str("1");
-        assert!(result.is_ok());
-        if let Ok(api) = result {
-            assert_eq!(1, api.major);
-            assert_eq!(0, api.minor);
-        }
+    fn reject_invalid_api_versions() {
+        assert_de_tokens_error::<BuildpackApi>(
+            &[Token::BorrowedStr("1.2.3")],
+            "Invalid Buildpack API version: `1.2.3`",
+        );
+        assert_de_tokens_error::<BuildpackApi>(
+            &[Token::BorrowedStr("1.2-dev")],
+            "Invalid Buildpack API version: `1.2-dev`",
+        );
+        assert_de_tokens_error::<BuildpackApi>(
+            &[Token::BorrowedStr("-1")],
+            "Invalid Buildpack API version: `-1`",
+        );
+        assert_de_tokens_error::<BuildpackApi>(
+            &[Token::BorrowedStr(".1")],
+            "Invalid Buildpack API version: `.1`",
+        );
+        assert_de_tokens_error::<BuildpackApi>(
+            &[Token::BorrowedStr("1.")],
+            "Invalid Buildpack API version: `1.`",
+        );
+        assert_de_tokens_error::<BuildpackApi>(
+            &[Token::BorrowedStr("1..2")],
+            "Invalid Buildpack API version: `1..2`",
+        );
+        assert_de_tokens_error::<BuildpackApi>(
+            &[Token::BorrowedStr("")],
+            "Invalid Buildpack API version: ``",
+        );
     }
 
     #[test]
     fn buildpack_api_display() {
         assert_eq!(BuildpackApi { major: 1, minor: 0 }.to_string(), "1.0");
         assert_eq!(BuildpackApi { major: 1, minor: 2 }.to_string(), "1.2");
-        assert_eq!(BuildpackApi { major: 0, minor: 5 }.to_string(), "0.5");
+        assert_eq!(
+            BuildpackApi {
+                major: 0,
+                minor: 10
+            }
+            .to_string(),
+            "0.10"
+        );
     }
 }

libcnb-data/src/buildpack/id.rs

@@ -40,23 +40,46 @@ libcnb_newtype!(
     /// ```
     BuildpackId,
     BuildpackIdError,
-    r"^(?!app$|config$)[[:alnum:]./-]+$"
+    r"^(?!(app|config)$)[[:alnum:]./-]+$"
 );
 
 #[cfg(test)]
 mod tests {
     use super::*;
-    use std::str::FromStr;
 
     #[test]
-    fn buildpack_id_does_not_allow_app() {
-        let result = BuildpackId::from_str("app");
-        assert!(result.is_err());
+    fn buildpack_id_validation_valid() {
+        assert!("heroku/jvm".parse::<BuildpackId>().is_ok());
+        assert!("Abc123./-".parse::<BuildpackId>().is_ok());
+        assert!("app-foo".parse::<BuildpackId>().is_ok());
+        assert!("foo-app".parse::<BuildpackId>().is_ok());
     }
 
     #[test]
-    fn buildpack_id_does_not_allow_config() {
-        let result = BuildpackId::from_str("config");
-        assert!(result.is_err());
+    fn buildpack_id_validation_invalid() {
+        assert_eq!(
+            "heroku_jvm".parse::<BuildpackId>(),
+            Err(BuildpackIdError::InvalidValue(String::from("heroku_jvm")))
+        );
+        assert_eq!(
+            "heroku:jvm".parse::<BuildpackId>(),
+            Err(BuildpackIdError::InvalidValue(String::from("heroku:jvm")))
+        );
+        assert_eq!(
+            "heroku jvm".parse::<BuildpackId>(),
+            Err(BuildpackIdError::InvalidValue(String::from("heroku jvm")))
+        );
+        assert_eq!(
+            "app".parse::<BuildpackId>(),
+            Err(BuildpackIdError::InvalidValue(String::from("app")))
+        );
+        assert_eq!(
+            "config".parse::<BuildpackId>(),
+            Err(BuildpackIdError::InvalidValue(String::from("config")))
+        );
+        assert_eq!(
+            "".parse::<BuildpackId>(),
+            Err(BuildpackIdError::InvalidValue(String::new()))
+        );
     }
 }