Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

twoliter: Remove Release.toml from variant build #112

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion tests/projects/project1/Release.toml

This file was deleted.

1 change: 1 addition & 0 deletions tests/projects/project1/Twoliter.toml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
schema-version = 1
release-version = "1.0.0"

[sdk]
registry = "twoliter.alpha"
Expand Down
8 changes: 0 additions & 8 deletions tests/projects/project1/variants/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions twoliter/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ hex = "0.4"
log = "0.4"
non-empty-string = { version = "0.2", features = [ "serde" ] }
serde = { version = "1", features = ["derive"] }
serde_json = "1"
sha2 = "0.10"
tar = "0.4"
tempfile = "3"
Expand Down
8 changes: 7 additions & 1 deletion twoliter/embedded/Makefile.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ BUILDSYS_VERSION_BUILD = { script = ["git describe --always --dirty --exclude '*
# later in this section. You have to edit the path here in Makefile.toml to
# use a different Release.toml.
BUILDSYS_RELEASE_CONFIG_PATH = "${BUILDSYS_ROOT_DIR}/Release.toml"
BUILDSYS_VERSION_IMAGE = { script = ["awk -F '[ =\"]+' '$1 == \"version\" {print $2}' ${BUILDSYS_RELEASE_CONFIG_PATH}"] }
# This can be overridden with -e to build a different variant from the variants/ directory
BUILDSYS_VARIANT = { script = ['echo "${BUILDSYS_VARIANT:-aws-k8s-1.24}"'] }
# Product name used for file and directory naming
Expand Down Expand Up @@ -263,6 +262,13 @@ if [[ -z "${TLPRIVATE_SDK_IMAGE}" || -z "{TLPRIVATE_TOOLCHAIN}" ]];then
exit 1
fi

# Ensure BUILDSYS_VERSION_IMAGE is set
if [[ -z "${BUILDSYS_VERSION_IMAGE}" ]];then
echo "BUILDSYS_VERSION_IMAGE must be defined and must be non-zero in length."
echo "Are you using Twoliter? It is a bug if Twoliter has invoked cargo make without this."
exit 1
fi

mkdir -p ${BUILDSYS_BUILD_DIR}
mkdir -p ${BUILDSYS_OUTPUT_DIR}
mkdir -p ${BUILDSYS_PACKAGES_DIR}
Expand Down
1 change: 1 addition & 0 deletions twoliter/src/cmd/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ impl BuildVariant {
.env("BUILDSYS_ARCH", &self.arch)
.env("BUILDSYS_VARIANT", &self.variant)
.env("BUILDSYS_SBKEYS_DIR", sbkeys_dir.display().to_string())
.env("BUILDSYS_VERSION_IMAGE", project.release_version())
.makefile(makefile_path)
.project_dir(project.project_dir())
.exec("build")
Expand Down
1 change: 1 addition & 0 deletions twoliter/src/cmd/make.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ impl Make {
CargoMake::new(&project, &self.arch)?
.env("CARGO_HOME", self.cargo_home.display().to_string())
.env("TWOLITER_TOOLS_DIR", tempdir.path().display().to_string())
.env("BUILDSYS_VERSION_IMAGE", project.release_version())
.makefile(makefile_path)
.project_dir(project.project_dir())
.exec_with_args(&self.makefile_task, self.additional_args.clone())
Expand Down
19 changes: 13 additions & 6 deletions twoliter/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,16 @@ pub(crate) async fn exec_log(cmd: &mut Command) -> Result<()> {
log::max_level(),
LevelFilter::Off | LevelFilter::Error | LevelFilter::Warn
);
exec(cmd, quiet).await
exec(cmd, quiet).await?;
Ok(())
}

/// Run a `tokio::process::Command` and return a `Result` letting us know whether or not it worked.
/// `quiet` determines whether or not the command output will be piped to `stdout/stderr`. When
/// `quiet=true`, no output will be shown.
pub(crate) async fn exec(cmd: &mut Command, quiet: bool) -> Result<()> {
/// `quiet=true`, no output will be shown and will be returned instead.
pub(crate) async fn exec(cmd: &mut Command, quiet: bool) -> Result<Option<String>> {
debug!("Running: {:?}", cmd);
if quiet {
Ok(if quiet {
// For quiet levels of logging we capture stdout and stderr
let output = cmd
.output()
Expand All @@ -30,6 +31,11 @@ pub(crate) async fn exec(cmd: &mut Command, quiet: bool) -> Result<()> {
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);

Some(
String::from_utf8(output.stdout)
.context("Unable to convert command output to `String`")?,
)
} else {
// For less quiet log levels we stream to stdout and stderr.
let status = cmd
Expand All @@ -42,6 +48,7 @@ pub(crate) async fn exec(cmd: &mut Command, quiet: bool) -> Result<()> {
"Command was unsuccessful, exit code {}",
status.code().unwrap_or(1),
);
}
Ok(())

None
})
}
3 changes: 2 additions & 1 deletion twoliter/src/docker/container.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ impl DockerContainer {
let mut args = vec!["cp".to_string()];
args.push(format!("{}:{}", self.name, src.as_ref().display()));
args.push(dest.as_ref().display().to_string());
exec(Command::new("docker").args(args), true).await
exec(Command::new("docker").args(args), true).await?;
Ok(())
}
}

Expand Down
1 change: 1 addition & 0 deletions twoliter/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ mod cmd;
mod common;
mod docker;
mod project;
mod schema_version;
mod tools;

/// Test code that should only be compiled when running tests.
Expand Down
158 changes: 89 additions & 69 deletions twoliter/src/project.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
use crate::docker::ImageArchUri;
use crate::schema_version::SchemaVersion;
use anyhow::{ensure, Context, Result};
use async_recursion::async_recursion;
use log::{debug, trace};
use log::{debug, info, trace, warn};
use non_empty_string::NonEmptyString;
use serde::de::Error;
use serde::{Deserialize, Deserializer, Serialize, Serializer};
use serde::{Deserialize, Serialize};
use sha2::{Digest, Sha512};
use std::fmt;
use std::path::{Path, PathBuf};
use tokio::fs;
use toml::Table;

/// Common functionality in commands, if the user gave a path to the `Twoliter.toml` file,
/// we use it, otherwise we search for the file. Returns the `Project` and the path at which it was
Expand All @@ -26,7 +26,7 @@ pub(crate) async fn load_or_find_project(user_path: Option<PathBuf>) -> Result<P
}

/// Represents the structure of a `Twoliter.toml` project file.
#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Serialize, Deserialize)]
#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Serialize)]
#[serde(rename_all = "kebab-case")]
pub(crate) struct Project {
#[serde(skip)]
Expand All @@ -37,6 +37,9 @@ pub(crate) struct Project {
/// The version of this schema struct.
schema_version: SchemaVersion<1>,

/// The version that will be given to released artifacts such as kits and variants.
release_version: String,

/// The Bottlerocket SDK container image.
sdk: Option<ImageName>,

Expand All @@ -51,20 +54,11 @@ impl Project {
let data = fs::read_to_string(&path)
.await
.context(format!("Unable to read project file '{}'", path.display()))?;
let mut project: Self = toml::from_str(&data).context(format!(
let unvalidated: UnvalidatedProject = toml::from_str(&data).context(format!(
"Unable to deserialize project file '{}'",
path.display()
))?;
project.filepath = path.into();
project.project_dir = project
.filepath
.parent()
.context(format!(
"Unable to find the parent directory of '{}'",
project.filepath.display(),
))?
.into();
Ok(project)
unvalidated.validate(path).await
}

/// Recursively search for a file named `Twoliter.toml` starting in `dir`. If it is not found,
Expand Down Expand Up @@ -101,6 +95,10 @@ impl Project {
self.project_dir.clone()
}

pub(crate) fn release_version(&self) -> &str {
self.release_version.as_str()
}

pub(crate) fn sdk_name(&self) -> Option<&ImageName> {
self.sdk.as_ref()
}
Expand Down Expand Up @@ -157,64 +155,85 @@ impl ImageName {
}
}

/// We need to constrain the `Project` struct to a valid version. Unfortunately `serde` does not
/// have an after-deserialization validation hook, so we have this struct to limit the version to a
/// single acceptable value.
#[derive(Default, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)]
pub(crate) struct SchemaVersion<const N: u32>;

impl<const N: u32> SchemaVersion<N> {
pub(crate) fn get(&self) -> u32 {
N
}

pub(crate) fn get_static() -> u32 {
N
}
}

impl<const N: u32> From<SchemaVersion<N>> for u32 {
fn from(value: SchemaVersion<N>) -> Self {
value.get()
}
}

impl<const N: u32> fmt::Debug for SchemaVersion<N> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> std::fmt::Result {
fmt::Debug::fmt(&self.get(), f)
}
}

impl<const N: u32> fmt::Display for SchemaVersion<N> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> std::fmt::Result {
fmt::Display::fmt(&self.get(), f)
}
/// This is used to `Deserialize` a project, then run validation code before returning a valid
/// [`Project`]. This is necessary both because there is no post-deserialization serde hook for
/// validation and, even if there was, we need to know the project directory path in order to check
/// some things.
#[derive(Debug, Clone, Deserialize)]
#[serde(rename_all = "kebab-case")]
struct UnvalidatedProject {
schema_version: SchemaVersion<1>,
release_version: String,
sdk: Option<ImageName>,
toolchain: Option<ImageName>,
}

impl<const N: u32> Serialize for SchemaVersion<N> {
fn serialize<S>(&self, serializer: S) -> std::result::Result<S::Ok, S::Error>
where
S: Serializer,
{
serializer.serialize_u32(self.get())
impl UnvalidatedProject {
/// Constructs a [`Project`] from an [`UnvalidatedProject`] after validating fields.
async fn validate(self, path: impl AsRef<Path>) -> Result<Project> {
let filepath: PathBuf = path.as_ref().into();
let project_dir = filepath
.parent()
.context(format!(
"Unable to find the parent directory of '{}'",
filepath.display(),
))?
.to_path_buf();

self.check_release_toml(&project_dir).await?;

Ok(Project {
filepath,
project_dir,
schema_version: self.schema_version,
release_version: self.release_version,
sdk: self.sdk,
toolchain: self.toolchain,
})
}
}

impl<'de, const N: u32> Deserialize<'de> for SchemaVersion<N> {
fn deserialize<D>(deserializer: D) -> Result<SchemaVersion<N>, D::Error>
where
D: Deserializer<'de>,
{
let value: u32 = Deserialize::deserialize(deserializer)?;
if value != Self::get_static() {
Err(Error::custom(format!(
"Incorrect project schema_version: got '{}', expected '{}'",
value,
Self::get_static()
)))
} else {
Ok(Self)
/// Issues a warning if `Release.toml` is found and, if so, ensures that it contains the same
/// version (i.e. `release-version`) as the `Twoliter.toml` project file.
async fn check_release_toml(&self, project_dir: &Path) -> Result<()> {
let path = project_dir.join("Release.toml");
if !path.exists() || !path.is_file() {
// There is no Release.toml file. This is a good thing!
trace!("This project does not have a Release.toml file (this is not a problem)");
return Ok(());
}
warn!(
"A Release.toml file was found. Release.toml is deprecated. Please remove it from \
your project."
);
Comment on lines +204 to +207
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this could encourage people building from the main repo to send a PR to remove Release.toml - might want to drop it.

let content = fs::read_to_string(&path).await.context(format!(
"Error while checking Release.toml file at '{}'",
path.display()
))?;
let toml: Table = match toml::from_str(&content) {
Ok(toml) => toml,
Err(e) => {
warn!(
"Unable to parse Release.toml to ensure that its version matches the \
release-version in Twoliter.toml: {e}",
);
return Ok(());
}
};
let version = match toml.get("version") {
Some(version) => version,
None => {
info!("Release.toml does not contain a version key. Ignoring it.");
return Ok(());
}
}
.to_string();
ensure!(
version == self.release_version,
"The version found in Release.toml, '{version}', does not match the release-version \
found in Twoliter.toml '{}'",
self.release_version
);
Ok(())
}
}

Expand Down Expand Up @@ -287,6 +306,7 @@ mod test {
filepath: Default::default(),
project_dir: Default::default(),
schema_version: Default::default(),
release_version: String::from("1.0.0"),
sdk: Some(ImageName {
registry: Some("example.com".try_into().unwrap()),
name: "foo-abc".try_into().unwrap(),
Expand Down
Loading