Skip to content

Commit

Permalink
Use compile_for_everything() to embed the manifest in the test binary
Browse files Browse the repository at this point in the history
And also do this for the harp test binary, which starts R and tests UTF-8 related capabilities.
  • Loading branch information
DavisVaughan committed Sep 26, 2024
1 parent 3495301 commit 2d9c0b5
Show file tree
Hide file tree
Showing 14 changed files with 186 additions and 50 deletions.
14 changes: 8 additions & 6 deletions Cargo.lock

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

2 changes: 1 addition & 1 deletion crates/ark/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ insta = { version = "1.39.0" }

[build-dependencies]
chrono = "0.4.23"
embed-resource = "2.4.0"
embed-resource = "2.5.0"

[package.metadata.generate-rpm]
assets = [{ source = "target/release/ark", dest = "/usr/bin/ark", mode = "755" }]
Expand Down
17 changes: 16 additions & 1 deletion crates/ark/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,23 @@ fn main() {

// Embed an Application Manifest file on Windows.
// Documented to do nothing on non-Windows.
// We also do this for harp to support its unit tests.
//
// We can't just use `compile()`, as that uses `cargo:rustc-link-arg-bins`,
// which targets the main `ark.exe` (good) but not the test binaries (bad).
// We need the application manifest to get embedded into the ark/harp test
// binaries too, so that the instance of R started by our tests also has
// UTF-8 support.
//
// We can't use `compile_for_tests()` because `cargo:rustc-link-arg-tests`
// only targets integration tests right now, not unit tests.
// https://github.com/rust-lang/cargo/issues/10937
//
// Instead we use `compile_for_everything()` which uses the kitchen sink
// instruction of `cargo:rustc-link-arg`, and that seems to work.
// https://github.com/nabijaczleweli/rust-embed-resource/issues/69
let resource = Path::new("resources")
.join("manifest")
.join("ark-manifest.rc");
embed_resource::compile(resource, embed_resource::NONE);
embed_resource::compile_for_everything(resource, embed_resource::NONE);
}
4 changes: 3 additions & 1 deletion crates/ark/resources/manifest/ark-manifest.rc
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// Autogenerated, do not modify!

// This is a C file that embed-resource compiles for us.
// It helps embed our Windows Application Manifest file.
// https://learn.microsoft.com/en-us/windows/win32/sbscs/application-manifests
Expand All @@ -7,5 +9,5 @@
// https://github.com/nabijaczleweli/rust-embed-resource/issues/11#issuecomment-779295722
#include <winuser.h>

// The `1` is a `resource_id` that specifies ark as an executable file
// The `1` is a `resource_id` that specifies us as an executable file
1 RT_MANIFEST "ark.exe.manifest"
48 changes: 12 additions & 36 deletions crates/ark/src/data_explorer/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,8 +439,6 @@ impl Into<String> for FormattedValue {

#[cfg(test)]
mod tests {
use harp::utils::r_envir_set;

use super::*;
use crate::test::r_test;

Expand Down Expand Up @@ -752,40 +750,18 @@ mod tests {
ColumnValue::FormattedValue("aa".to_string()),
]);

// options.max_value_length = 1000;
// let text = RObject::from(r#"x <- c("ボルテックス")"#);
// unsafe { r_envir_set("text", text.sexp, R_GlobalEnv) };
// let data = harp::parse_eval_global(r#"Encoding(text)"#).unwrap();
// let data = String::try_from(data).unwrap();
// let _ = harp::parse_eval_global(r#"rm(text)"#).unwrap();
// assert_eq!(data, "UTF-8".to_string());

// let data = harp::parse_eval_global("Sys.getlocale()").unwrap();
// let data = String::try_from(data).unwrap();
// assert_eq!(data, "wrong".to_string());

// let data =
// harp::parse_eval_global("paste0(capture.output(sessionInfo()), collapse = ' ')")
// .unwrap();
// let data = String::try_from(data).unwrap();
// assert_eq!(data, "wrong".to_string());

// let _ = harp::parse_eval_global(r#"x <- c("ボルテックス")"#).unwrap();
// let data = harp::parse_eval_global(r#"Encoding(x)"#).unwrap();
// let data = String::try_from(data).unwrap();
// let _ = harp::parse_eval_global(r#"rm(x)"#).unwrap();
// assert_eq!(data, "UTF-8".to_string());
// let formatted = format_column(data.sexp, &options);
// assert_eq!(formatted, vec![ColumnValue::FormattedValue(
// "ボルテックス".to_string()
// ),]);

// options.max_value_length = 4;
// let data = harp::parse_eval_global(r#"c("नमस्ते")"#).unwrap();
// let formatted = format_column(data.sexp, &options);
// assert_eq!(formatted, vec![ColumnValue::FormattedValue(
// "नमस्".to_string()
// ),]);
let data = harp::parse_eval_global(r#"c("ボルテックス")"#).unwrap();
let formatted = format_column(data.sexp, &options);
assert_eq!(formatted, vec![ColumnValue::FormattedValue(
"ボルテ".to_string()
),]);

options.max_value_length = 4;
let data = harp::parse_eval_global(r#"c("नमस्ते")"#).unwrap();
let formatted = format_column(data.sexp, &options);
assert_eq!(formatted, vec![ColumnValue::FormattedValue(
"नमस्".to_string()
),]);
})
}
}
1 change: 1 addition & 0 deletions crates/ark/src/sys/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
pub mod console;
pub mod control;
pub mod interface;
mod locale;
pub mod path;
pub mod signals;
mod strings;
Expand Down
33 changes: 33 additions & 0 deletions crates/ark/src/sys/windows/locale.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* locale.rs
*
* Copyright (C) 2024 Posit Software, PBC. All rights reserved.
*
*/

#[cfg(test)]
mod tests {
use crate::test::r_test;

#[test]
fn test_locale() {
// These tests assert that we've embedded our Application Manifest file correctly in `build.rs`
r_test(|| {
let latin1 = harp::parse_eval_base("l10n_info()$`Latin-1`").unwrap();
let latin1 = bool::try_from(latin1).unwrap();
assert!(!latin1);

let utf8 = harp::parse_eval_base("l10n_info()$`UTF-8`").unwrap();
let utf8 = bool::try_from(utf8).unwrap();
assert!(utf8);

let codepage = harp::parse_eval_base("l10n_info()$codepage").unwrap();
let codepage = i32::try_from(codepage).unwrap();
assert_eq!(codepage, 65001);

let system_codepage = harp::parse_eval_base("l10n_info()$system.codepage").unwrap();
let system_codepage = i32::try_from(system_codepage).unwrap();
assert_eq!(system_codepage, 65001);
})
}
}
5 changes: 0 additions & 5 deletions crates/ark/tests/data_explorer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1808,11 +1808,6 @@ fn test_histogram() {
#[test]
fn test_frequency_table() {
r_test(|| {
let data = harp::parse_eval_global("paste0(capture.output(sessionInfo()), collapse = ' ')")
.unwrap();
let data = String::try_from(data).unwrap();
assert_eq!(data, "wrong".to_string());

let socket =
open_data_explorer_from_expression("data.frame(x = rep(letters[1:10], 10:1))", None)
.unwrap();
Expand Down
3 changes: 3 additions & 0 deletions crates/harp/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,6 @@ serde = { version = "1.0.183", features = ["derive"] }
serde_json = { version = "1.0.94", features = ["preserve_order"]}
rust-embed = "8.2.0"
tracing-error = "0.2.0"

[build-dependencies]
embed-resource = "2.5.0"
22 changes: 22 additions & 0 deletions crates/harp/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
//
// build.rs
//
// Copyright (C) 2024 Posit Software, PBC. All rights reserved.
//
//

use std::path::Path;
extern crate embed_resource;

fn main() {
// Embed an Application Manifest file on Windows.
// Documented to do nothing on non-Windows.
// We also do this for ark.
// We don't generate a main `harp.exe` binary, but `cargo test` does generate a `harp-*.exe`
// binary for unit testing, and those unit tests also start R and test UTF-8 related capabilities!
// So we need that test executable to include a manifest file too.
let resource = Path::new("resources")
.join("manifest")
.join("harp-manifest.rc");
embed_resource::compile_for_everything(resource, embed_resource::NONE);
}
13 changes: 13 additions & 0 deletions crates/harp/resources/manifest/harp-manifest.rc
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Autogenerated, do not modify!

// This is a C file that embed-resource compiles for us.
// It helps embed our Windows Application Manifest file.
// https://learn.microsoft.com/en-us/windows/win32/sbscs/application-manifests
// https://learn.microsoft.com/en-us/cpp/build/reference/manifest-create-side-by-side-assembly-manifest?view=msvc-170

// This defines `RT_MANIFEST` to `24`. embed-resource should know how to ensure the header is available.
// https://github.com/nabijaczleweli/rust-embed-resource/issues/11#issuecomment-779295722
#include <winuser.h>

// The `1` is a `resource_id` that specifies us as an executable file
1 RT_MANIFEST "harp.exe.manifest"
40 changes: 40 additions & 0 deletions crates/harp/resources/manifest/harp.exe.manifest
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<assembly xmlns="urn:schemas-microsoft-com:asm.v1" manifestVersion="1.0">

<assemblyIdentity version="1.0.0.0" name="harp" type="win32"/>

<description>harp</description>

<!-- Support UTF-8 code page with R 4.2.0 and newer. -->
<application>
<windowsSettings>
<activeCodePage xmlns="http://schemas.microsoft.com/SMI/2019/WindowsSettings">UTF-8</activeCodePage>
</windowsSettings>
</application>

<!-- Identify the application security requirements. -->
<trustInfo xmlns="urn:schemas-microsoft-com:asm.v2">
<security>
<requestedPrivileges>
<requestedExecutionLevel level="asInvoker"/>
</requestedPrivileges>
</security>
</trustInfo>

<!-- Declare compatibility with different versions of Windows. -->
<compatibility xmlns="urn:schemas-microsoft-com:compatibility.v1">
<application>
<!-- Windows 10 and Windows 11 -->
<supportedOS Id="{8e0f7a12-bfb3-4fe8-b9a5-48fd50a15a9a}"/>
<!-- Windows 8.1 -->
<supportedOS Id="{1f676c76-80e1-4239-95bb-83d0f6d0da78}"/>
<!-- Windows Vista -->
<supportedOS Id="{e2011457-1546-43c5-a5fe-008deee3d3f0}"/>
<!-- Windows 7 -->
<supportedOS Id="{35138b9a-5d96-4fbd-8e2d-a2440225f93a}"/>
<!-- Windows 8 -->
<supportedOS Id="{4a2f28e3-53b9-4441-ba9c-d69d4a4a6e38}"/>
</application>
</compatibility>

</assembly>
1 change: 1 addition & 0 deletions crates/harp/src/sys/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@

pub mod library;
pub mod line_ending;
mod locale;
pub mod polled_events;
33 changes: 33 additions & 0 deletions crates/harp/src/sys/windows/locale.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/*
* locale.rs
*
* Copyright (C) 2024 Posit Software, PBC. All rights reserved.
*
*/

#[cfg(test)]
mod tests {
use crate::test::r_test;

#[test]
fn test_locale() {
// These tests assert that we've embedded our Application Manifest file correctly in `build.rs`
r_test(|| {
let latin1 = harp::parse_eval_base("l10n_info()$`Latin-1`").unwrap();
let latin1 = bool::try_from(latin1).unwrap();
assert!(!latin1);

let utf8 = harp::parse_eval_base("l10n_info()$`UTF-8`").unwrap();
let utf8 = bool::try_from(utf8).unwrap();
assert!(utf8);

let codepage = harp::parse_eval_base("l10n_info()$codepage").unwrap();
let codepage = i32::try_from(codepage).unwrap();
assert_eq!(codepage, 65001);

let system_codepage = harp::parse_eval_base("l10n_info()$system.codepage").unwrap();
let system_codepage = i32::try_from(system_codepage).unwrap();
assert_eq!(system_codepage, 65001);
})
}
}

0 comments on commit 2d9c0b5

Please sign in to comment.