Skip to content

Commit 4247d13

Browse files
committed
[host/{build,lib,uninit}] added feature flag to remove logging build details
git2 uses deep C bindings to libgit2 that might not be available on every target, we should allow disabling logging build details. Signed-off-by: danbugs <[email protected]>
1 parent 6d67b97 commit 4247d13

File tree

6 files changed

+94
-70
lines changed

6 files changed

+94
-70
lines changed

Justfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ test-like-ci config=default-target hypervisor="kvm":
6262
just test {{config}} {{ if hypervisor == "mshv3" {"mshv3"} else {""} }}
6363

6464
@# with only one driver enabled + seccomp + inprocess
65-
just test {{config}} inprocess,seccomp,{{ if hypervisor == "mshv" {"mshv2"} else if hypervisor == "mshv3" {"mshv3"} else {"kvm"} }}
65+
just test {{config}} inprocess,seccomp,build-metadata,{{ if hypervisor == "mshv" {"mshv2"} else if hypervisor == "mshv3" {"mshv3"} else {"kvm"} }}
6666

6767
@# make sure certain cargo features compile
6868
cargo check -p hyperlight-host --features crashdump

src/hyperlight_host/Cargo.toml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,10 @@ proc-maps = "0.4.0"
113113
[build-dependencies]
114114
anyhow = { version = "1.0.98" }
115115
cfg_aliases = "0.2.1"
116-
built = { version = "0.8.0", features = ["chrono", "git2"] }
116+
built = { version = "0.8.0", optional = true, features = ["chrono", "git2"] }
117117

118118
[features]
119-
default = ["kvm", "mshv2", "seccomp"]
119+
default = ["kvm", "mshv2", "seccomp", "build-metadata"]
120120
seccomp = ["dep:seccompiler"]
121121
function_call_metrics = []
122122
executable_heap = []
@@ -130,6 +130,7 @@ inprocess = []
130130
# This enables easy debug in the guest
131131
gdb = ["dep:gdbstub", "dep:gdbstub_arch"]
132132
fuzzing = ["hyperlight-common/fuzzing"]
133+
build-metadata = ["dep:built"]
133134

134135
[[bench]]
135136
name = "benchmarks"

src/hyperlight_host/build.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ limitations under the License.
1515
*/
1616

1717
use anyhow::Result;
18+
#[cfg(feature = "build-metadata")]
1819
use built::write_built_file;
1920

2021
fn main() -> Result<()> {
@@ -106,6 +107,7 @@ fn main() -> Result<()> {
106107
mshv3: { all(feature = "mshv3", target_os = "linux") },
107108
}
108109

110+
#[cfg(feature = "build-metadata")]
109111
write_built_file()?;
110112

111113
Ok(())

src/hyperlight_host/src/lib.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,10 @@ limitations under the License.
2222
#![cfg_attr(not(any(test, debug_assertions)), warn(clippy::unwrap_used))]
2323
#![cfg_attr(any(test, debug_assertions), allow(clippy::disallowed_macros))]
2424

25+
#[cfg(feature = "build-metadata")]
2526
use std::sync::Once;
2627

27-
use log::info;
28+
#[cfg(feature = "build-metadata")]
2829
/// The `built` crate is used to generate a `built.rs` file that contains
2930
/// information about the build environment. This information is used to
3031
/// populate the `built_info` module, which is re-exported here.
@@ -149,9 +150,12 @@ macro_rules! debug {
149150
}
150151

151152
// LOG_ONCE is used to log information about the crate version once
153+
#[cfg(feature = "build-metadata")]
152154
static LOG_ONCE: Once = Once::new();
153155

156+
#[cfg(feature = "build-metadata")]
154157
pub(crate) fn log_build_details() {
158+
use log::info;
155159
LOG_ONCE.call_once(|| {
156160
info!("Package name: {}", built_info::PKG_NAME);
157161
info!("Package version: {}", built_info::PKG_VERSION);

src/hyperlight_host/src/sandbox/uninitialized.rs

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,15 @@ use super::mem_mgr::MemMgrWrapper;
3030
use super::run_options::SandboxRunOptions;
3131
use super::uninitialized_evolve::evolve_impl_multi_use;
3232
use crate::func::host_functions::{HostFunction, IntoHostFunction};
33+
#[cfg(feature = "build-metadata")]
34+
use crate::log_build_details;
3335
use crate::mem::exe::ExeInfo;
3436
use crate::mem::mgr::{SandboxMemoryManager, STACK_COOKIE_LEN};
3537
use crate::mem::shared_mem::ExclusiveSharedMemory;
3638
use crate::sandbox::SandboxConfiguration;
3739
use crate::sandbox_state::sandbox::EvolvableSandbox;
3840
use crate::sandbox_state::transition::Noop;
39-
use crate::{log_build_details, log_then_return, new_error, MultiUseSandbox, Result};
41+
use crate::{log_then_return, new_error, MultiUseSandbox, Result};
4042

4143
/// A preliminary `Sandbox`, not yet ready to execute guest code.
4244
///
@@ -129,6 +131,7 @@ impl UninitializedSandbox {
129131
sandbox_run_options: Option<SandboxRunOptions>,
130132
host_print_writer: Option<&dyn HostFunction<i32, (String,)>>,
131133
) -> Result<Self> {
134+
#[cfg(feature = "build-metadata")]
132135
log_build_details();
133136

134137
// hyperlight is only supported on Windows 11 and Windows Server 2022 and later
@@ -332,28 +335,18 @@ fn check_windows_version() -> Result<()> {
332335

333336
#[cfg(test)]
334337
mod tests {
335-
use std::path::PathBuf;
336338
use std::sync::{Arc, Mutex};
337339
use std::time::Duration;
338340
use std::{fs, thread};
339341

340342
use crossbeam_queue::ArrayQueue;
341343
use hyperlight_common::flatbuffer_wrappers::function_types::{ParameterValue, ReturnValue};
342-
use hyperlight_testing::logger::{Logger as TestLogger, LOGGER as TEST_LOGGER};
343344
use hyperlight_testing::simple_guest_as_string;
344-
use hyperlight_testing::tracing_subscriber::TracingSubscriber as TestSubscriber;
345-
use log::Level;
346-
use serde_json::{Map, Value};
347-
use tracing::Level as tracing_level;
348-
use tracing_core::callsite::rebuild_interest_cache;
349-
use tracing_core::Subscriber;
350-
use uuid::Uuid;
351345

352346
use crate::sandbox::uninitialized::GuestBinary;
353347
use crate::sandbox::SandboxConfiguration;
354348
use crate::sandbox_state::sandbox::EvolvableSandbox;
355349
use crate::sandbox_state::transition::Noop;
356-
use crate::testing::log_values::{test_value_as_str, try_to_strings};
357350
use crate::{new_error, MultiUseSandbox, Result, SandboxRunOptions, UninitializedSandbox};
358351

359352
#[test]
@@ -844,7 +837,19 @@ mod tests {
844837
// from their workstation to be successful without needed to know about test interdependencies
845838
// this test will be run explicitly as a part of the CI pipeline
846839
#[ignore]
840+
#[cfg(feature = "build-metadata")]
847841
fn test_trace_trace() {
842+
use hyperlight_testing::logger::Logger as TestLogger;
843+
use hyperlight_testing::tracing_subscriber::TracingSubscriber as TestSubscriber;
844+
use serde_json::{Map, Value};
845+
use tracing::Level as tracing_level;
846+
use tracing_core::callsite::rebuild_interest_cache;
847+
use tracing_core::Subscriber;
848+
use uuid::Uuid;
849+
850+
use crate::testing::log_values::build_metadata_testing::try_to_strings;
851+
use crate::testing::log_values::test_value_as_str;
852+
848853
TestLogger::initialize_log_tracer();
849854
rebuild_interest_cache();
850855
let subscriber = TestSubscriber::new(tracing_level::TRACE);
@@ -943,7 +948,14 @@ mod tests {
943948
#[ignore]
944949
// Tests that traces are emitted as log records when there is no trace
945950
// subscriber configured.
951+
#[cfg(feature = "build-metadata")]
946952
fn test_log_trace() {
953+
use std::path::PathBuf;
954+
955+
use hyperlight_testing::logger::{Logger as TestLogger, LOGGER as TEST_LOGGER};
956+
use log::Level;
957+
use tracing_core::callsite::rebuild_interest_cache;
958+
947959
{
948960
TestLogger::initialize_test_logger();
949961
TEST_LOGGER.set_max_level(log::LevelFilter::Trace);

src/hyperlight_host/src/testing/log_values.rs

Lines changed: 60 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -61,61 +61,66 @@ fn try_to_string<'a>(values: &'a Map<String, Value>, key: &'a str) -> Result<&'a
6161
}
6262
}
6363

64-
/// A single value in the parameter list for the `try_to_strings`
65-
/// function.
66-
pub(crate) type MapLookup<'a> = (&'a Map<String, Value>, &'a str);
64+
#[cfg(feature = "build-metadata")]
65+
pub(crate) mod build_metadata_testing {
66+
use super::*;
6767

68-
/// Given a constant-size slice of `MapLookup`s, attempt to look up the
69-
/// string value in each `MapLookup`'s map (the first tuple element) for
70-
/// that `MapLookup`'s key (the second tuple element). If the lookup
71-
/// succeeded, attempt to convert the resulting value to a string. Return
72-
/// `Ok` with all the successfully looked-up string values, or `Err`
73-
/// if any one or more lookups or string conversions failed.
74-
pub(crate) fn try_to_strings<'a, const NUM: usize>(
75-
lookups: [MapLookup<'a>; NUM],
76-
) -> Result<[&'a str; NUM]> {
77-
// Note (from arschles) about this code:
78-
//
79-
// In theory, there's a way to write this function in the functional
80-
// programming (FP) style -- e.g. with a fold, map, flat_map, or
81-
// something similar -- and without any mutability.
82-
//
83-
// In practice, however, since we're taking in a statically-sized slice,
84-
// and we are expected to return a statically-sized slice of the same
85-
// size, we are more limited in what we can do. There is a way to design
86-
// a fold or flat_map to iterate over the lookups parameter and attempt to
87-
// transform each MapLookup into the string value at that key.
88-
//
89-
// I wrote that code, which I'll called the "FP code" hereafter, and
90-
// noticed two things:
91-
//
92-
// - It required several places where I had to explicitly deal with long
93-
// and complex (in my opinion) types
94-
// - It wasn't much more succinct or shorter than the code herein
95-
//
96-
// The FP code is functionally "pure" and maybe fun to write (if you like
97-
// Rust or you love FP), but not fun to read. In fact, because of all the
98-
// explicit type ceremony, I bet it'd make even the most hardcore Haskell
99-
// programmer blush.
100-
//
101-
// So, I've decided to use a little bit of mutability to implement this
102-
// function in a way I think most programmers would agree is easier to
103-
// reason about and understand quickly.
104-
//
105-
// Final performance note:
106-
//
107-
// It's likely, but not certain, that the FP code is probably not
108-
// significantly more memory efficient than this, since the compiler knows
109-
// the size of both the input and output slices. Plus, this is test code,
110-
// so even if this were 2x slower, I'd still argue the ease of
111-
// understanding is more valuable than the (relatively small) memory
112-
// savings.
113-
let mut ret_slc: [&'a str; NUM] = [""; NUM];
114-
for (idx, lookup) in lookups.iter().enumerate() {
115-
let map = lookup.0;
116-
let key = lookup.1;
117-
let val = try_to_string(map, key)?;
118-
ret_slc[idx] = val
68+
/// A single value in the parameter list for the `try_to_strings`
69+
/// function.
70+
pub(crate) type MapLookup<'a> = (&'a Map<String, Value>, &'a str);
71+
72+
/// Given a constant-size slice of `MapLookup`s, attempt to look up the
73+
/// string value in each `MapLookup`'s map (the first tuple element) for
74+
/// that `MapLookup`'s key (the second tuple element). If the lookup
75+
/// succeeded, attempt to convert the resulting value to a string. Return
76+
/// `Ok` with all the successfully looked-up string values, or `Err`
77+
/// if any one or more lookups or string conversions failed.
78+
pub(crate) fn try_to_strings<'a, const NUM: usize>(
79+
lookups: [MapLookup<'a>; NUM],
80+
) -> Result<[&'a str; NUM]> {
81+
// Note (from arschles) about this code:
82+
//
83+
// In theory, there's a way to write this function in the functional
84+
// programming (FP) style -- e.g. with a fold, map, flat_map, or
85+
// something similar -- and without any mutability.
86+
//
87+
// In practice, however, since we're taking in a statically-sized slice,
88+
// and we are expected to return a statically-sized slice of the same
89+
// size, we are more limited in what we can do. There is a way to design
90+
// a fold or flat_map to iterate over the lookups parameter and attempt to
91+
// transform each MapLookup into the string value at that key.
92+
//
93+
// I wrote that code, which I'll called the "FP code" hereafter, and
94+
// noticed two things:
95+
//
96+
// - It required several places where I had to explicitly deal with long
97+
// and complex (in my opinion) types
98+
// - It wasn't much more succinct or shorter than the code herein
99+
//
100+
// The FP code is functionally "pure" and maybe fun to write (if you like
101+
// Rust or you love FP), but not fun to read. In fact, because of all the
102+
// explicit type ceremony, I bet it'd make even the most hardcore Haskell
103+
// programmer blush.
104+
//
105+
// So, I've decided to use a little bit of mutability to implement this
106+
// function in a way I think most programmers would agree is easier to
107+
// reason about and understand quickly.
108+
//
109+
// Final performance note:
110+
//
111+
// It's likely, but not certain, that the FP code is probably not
112+
// significantly more memory efficient than this, since the compiler knows
113+
// the size of both the input and output slices. Plus, this is test code,
114+
// so even if this were 2x slower, I'd still argue the ease of
115+
// understanding is more valuable than the (relatively small) memory
116+
// savings.
117+
let mut ret_slc: [&'a str; NUM] = [""; NUM];
118+
for (idx, lookup) in lookups.iter().enumerate() {
119+
let map = lookup.0;
120+
let key = lookup.1;
121+
let val = try_to_string(map, key)?;
122+
ret_slc[idx] = val
123+
}
124+
Ok(ret_slc)
119125
}
120-
Ok(ret_slc)
121126
}

0 commit comments

Comments
 (0)