From 97106025c9348150b08e4d2c2039711fe51ff1e1 Mon Sep 17 00:00:00 2001 From: Julio Gonzalez Date: Mon, 26 Aug 2024 17:37:38 +0200 Subject: [PATCH] Solve PR comments. * Fix report paths. * Add documentation. * Rerun build if any environment variable changes. * Use determine_paths to figure out source directory. --- .github/workflows/test.yml | 2 +- Cargo.lock | 1 + build-common/src/cbindgen.rs | 8 +++-- builder/Cargo.toml | 1 + builder/build/main.rs | 60 ++++++++++++++++++++++++++++++++---- builder/build/module.rs | 39 +++++++++++++++++++++++ 6 files changed, 101 insertions(+), 10 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 154b52278..c72d0b98f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -69,7 +69,7 @@ jobs: if: success() || failure() uses: mikepenz/action-junit-report@v4 with: - report_paths: "target/nextestgci/junit.xml" + report_paths: "target/nextest/ci/junit.xml" check_name: "[${{ matrix.platform }}:${{ matrix.rust_version }}] test report" include_passed: true diff --git a/Cargo.lock b/Cargo.lock index 065ebecdf..35cde9be1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -789,6 +789,7 @@ name = "builder" version = "12.0.0" dependencies = [ "anyhow", + "build_common", "cmake", "datadog-crashtracker-ffi", "datadog-profiling-ffi", diff --git a/build-common/src/cbindgen.rs b/build-common/src/cbindgen.rs index 80eb1416c..4f89f61bf 100644 --- a/build-common/src/cbindgen.rs +++ b/build-common/src/cbindgen.rs @@ -7,12 +7,14 @@ use std::path::{Path, PathBuf}; use std::process::Command; use std::str; +pub const HEADER_PATH: &str = "include/datadog"; + /// Determines the cargo target directory and deliverables directory. /// /// # Returns /// /// * `(PathBuf, PathBuf)` - The cargo target directory and deliverables directory. -fn determine_paths() -> (PathBuf, PathBuf) { +pub fn determine_paths() -> (PathBuf, PathBuf) { let cargo_target_dir = match env::var_os("CARGO_TARGET_DIR") { Some(dir) => PathBuf::from(dir), None => { @@ -94,7 +96,7 @@ pub fn generate_header(crate_dir: PathBuf, header_name: &str, output_base_dir: P output_base_dir.is_absolute(), "output_base_dir must be an absolute path" ); - let output_path = output_base_dir.join("include/datadog/").join(header_name); + let output_path = output_base_dir.join(HEADER_PATH).join(header_name); if let Some(parent) = output_path.parent() { fs::create_dir_all(parent).expect("Failed to create output directory"); @@ -148,7 +150,7 @@ pub fn copy_and_configure_headers() { /// * `destination` - The destination path fn copy_header(source: &Path, destination: &Path) { let output_path = destination - .join("include/datadog/") + .join(HEADER_PATH) .join(source.file_name().unwrap()); if let Some(parent) = output_path.parent() { diff --git a/builder/Cargo.toml b/builder/Cargo.toml index 743d90385..dddf4196f 100644 --- a/builder/Cargo.toml +++ b/builder/Cargo.toml @@ -19,6 +19,7 @@ symbolizer = ["datadog-profiling-ffi?/symbolizer"] [build-dependencies] anyhow = { version = "1.0" } +build_common = { path = "../build-common" } cmake = "0.1.50" tools = { path = "../tools" } datadog-crashtracker-ffi = { path = "../crashtracker-ffi", optional = true } diff --git a/builder/build/main.rs b/builder/build/main.rs index 0a61fe7f5..9eb3829fe 100644 --- a/builder/build/main.rs +++ b/builder/build/main.rs @@ -17,12 +17,12 @@ mod profiling; mod symbolizer; use anyhow::Result; -use core::panic; use std::path::{Path, PathBuf}; use std::process::Command; use std::rc::Rc; use std::{env, fs}; +use build_common::{determine_paths, HEADER_PATH}; use tools::headers::dedup_headers; use crate::common::Common; @@ -36,6 +36,27 @@ use crate::profiling::Profiling; use crate::symbolizer::Symbolizer; use module::Module; +/// [`Builder`] is a structure that holds all the information required to assemble the final +/// workspace artifact. It will manage the different modules which will be in charge of producing +/// the different binaries and source files that will be part of the artifact. The builder will +/// provide the needed information: paths, version, etc, to the different modules so they can +/// install their sub-artifacts on the target folder. +/// The target folder is set through `LIBDD_OUTPUT_FOLDER` environment variable if it is not +/// provided the default target folder will be the builder output directory. +/// +/// # Example +/// +/// ```rust +/// use crate::core::Core; +/// +/// let mut builder = Builder::new(&path, &profile, &version); +/// let core = Box::new(Core { +/// version: builder.version.clone(), +/// }); +/// builder.add_module(core); +/// builder.build()?; +/// builder.pack()?; +/// ``` struct Builder { modules: Vec>, main_header: Rc, @@ -50,21 +71,33 @@ struct Builder { } impl Builder { - fn new(target_dir: &str, profile: &str, version: &str) -> Self { + /// Creates a new Builder instance + /// + /// # Aguments + /// + /// * `target_dir`: artifact folder. + /// * `profile`: Release configuration: debug or release; + /// * `version`: artifact's version. + /// + /// # Returns + /// + /// A new Builder instance. + fn new(source_dir: &str, target_dir: &str, profile: &str, version: &str) -> Self { Builder { modules: Vec::new(), main_header: "common.h".into(), - source_lib: ("../target".to_string() + "/" + profile + "/deps").into(), - source_inc: "../target/include/datadog".into(), + source_lib: (source_dir.to_string() + "/" + profile + "/deps").into(), + source_inc: (source_dir.to_string() + "/" + HEADER_PATH).into(), target_dir: target_dir.into(), target_lib: (target_dir.to_string() + "/lib").into(), - target_include: (target_dir.to_string() + "/include/datadog").into(), + target_include: (target_dir.to_string() + "/" + HEADER_PATH).into(), target_bin: (target_dir.to_string() + "/bin").into(), target_pkconfig: (target_dir.to_string() + "/lib/pkgconfig").into(), version: version.into(), } } + /// Adds a boxed object which implements Module trait. fn add_module(&mut self, module: Box) { self.modules.push(module); } @@ -128,6 +161,11 @@ impl Builder { fs::write(cmake_path, output.stdout).expect("writing cmake file failed"); } + /// Builds the final artifact by going through all modules and instancing their install method. + /// + /// #Returns + /// + /// Ok(()) if success Err(_) if failure. fn build(&self) -> Result<()> { for module in &self.modules { module.install()?; @@ -135,6 +173,11 @@ impl Builder { Ok(()) } + /// Generate a tar file with all the intermediate artifacts generated by all the modules.k + /// + /// #Returns + /// + /// Ok(()) if success Err(_) if failure. fn pack(&self) -> Result<()> { let tarname = "libdatadog".to_string() + "_v" + &self.version + ".tar"; let path: PathBuf = [self.target_dir.as_ref(), &tarname].iter().collect(); @@ -149,6 +192,11 @@ impl Builder { } fn main() { + // Rerun build script if any of the env vars change. + println!("cargo:rerun-if-env-changed=LIBDD_OUTPUT_FOLDER"); + println!("cargo:rerun-if-env-changed=PROFILE"); + + let (_, source_path) = determine_paths(); let mut path = env::var("OUT_DIR").unwrap(); if let Ok(libdd_path) = env::var("LIBDD_OUTPUT_FOLDER") { path = libdd_path; @@ -156,7 +204,7 @@ fn main() { let profile = env::var("PROFILE").unwrap(); let version = env::var("CARGO_PKG_VERSION").unwrap(); - let mut builder = Builder::new(&path, &profile, &version); + let mut builder = Builder::new(source_path.to_str().unwrap(), &path, &profile, &version); builder.create_dir_structure(); builder.deduplicate_headers(); diff --git a/builder/build/module.rs b/builder/build/module.rs index ffc917f83..c7fd5ff22 100644 --- a/builder/build/module.rs +++ b/builder/build/module.rs @@ -3,6 +3,45 @@ use anyhow::Result; +/// The trait Module is used to handle different modules under the same interface. A [`Module`] is +/// a installation unit used by the builder to install specific artifacts pertaining to certain +/// crate belonging to the workspace. +/// +/// # Examples +/// +/// Assuming there is a crate inside the workspace named core-ffi and this crate produces a library +/// `libcore_ffi.so` and a header file `core-ffi.h`: +/// ``` +/// struct Core { +/// pub source_include: Rc, +/// pub source_lib: Rc, +/// pub target_include: Rc, +/// pub target_lib: Rc, +/// } +/// +/// impl Core { +/// fn add_header(&self) -> Result<()> { +/// let mut origin_path: PathBuf = [&self.source_include, "core.h"].iter().collect(); +/// let mut target_path: PathBuf = [&self.target_include, "core.h"].iter().collect(); +/// fs::copy(&origin_path, &target_path).expect("Failed to copy the header"); +/// Ok(()) +/// } +/// +/// fn add_lib(&self) -> Result<()> { +/// let mut origin_path: PathBuf = [&self.source_lib, "libcore_ffi.so"].iter().collect(); +/// let mut target_path: PathBuf = [&self.target_lib, "libcore_ffi.so"].iter().collect(); +/// fs::copy(&origin_path, &target_path).expect("Failed to copy the library"); +/// } +/// } +/// +/// impl Module for Core { +/// fn install(&self) -> Result<()> { +/// self.add_header()?; +/// self.add_lib()?; +/// Ok(()) +/// } +/// } +/// ``` pub trait Module { fn install(&self) -> Result<()>; }