diff --git a/.github/workflows/amalthea-ci.yml b/.github/workflows/amalthea-ci.yml deleted file mode 100644 index d02aee107..000000000 --- a/.github/workflows/amalthea-ci.yml +++ /dev/null @@ -1,45 +0,0 @@ -name: "Amalthea Tests" - -on: - push: - branches: - - main - pull_request: - -jobs: - - linux: - runs-on: ubuntu-latest - name: "Unit Tests on Linux (rust: ${{ matrix.config.rust }})" - strategy: - fail-fast: false - matrix: - config: - - { rust: 'stable' } - - { rust: 'nightly' } - timeout-minutes: 30 - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - steps: - - uses: actions/checkout@v4 - - - uses: dtolnay/rust-toolchain@nightly - if: matrix.config.rust == 'nightly' - - - name: Setup Build Environment - run: | - sudo apt-get update - sudo apt-get install -y build-essential r-base-dev libsodium-dev - - - name: Build - id: amalthea-build - run: | - cargo build --verbose - - # Ubuntu runners already have a version of R installed. - # Unit tests "automatically" find and set `R_HOME` if it isn't set by - # calling `R RHOME`. See `harp::fixtures::r_test_init()`. - - name: Run Unit Tests - id: amalthea-test - run: | - cargo test --verbose diff --git a/.github/workflows/test-linux.yml b/.github/workflows/test-linux.yml new file mode 100644 index 000000000..db719769f --- /dev/null +++ b/.github/workflows/test-linux.yml @@ -0,0 +1,60 @@ +name: "Test Ark - Linux" + +on: + workflow_call: + workflow_dispatch: + inputs: + ssh: + description: 'Set up an SSH session before running `cargo test`?' + type: boolean + required: true + default: false + +jobs: + linux: + runs-on: ubuntu-latest + name: "Rust: ${{ matrix.config.rust }}, R: ${{ matrix.config.r }}" + strategy: + fail-fast: false + matrix: + config: + - { rust: 'stable', r: 'release' } + # Oldest supported R version + - { rust: 'stable', r: '4.2' } + # Nightly rust + - { rust: 'nightly', r: 'release' } + timeout-minutes: 30 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + steps: + - uses: actions/checkout@v4 + + - uses: dtolnay/rust-toolchain@nightly + if: matrix.config.rust == 'nightly' + + - name: Install R + uses: r-lib/actions/setup-r@v2 + with: + r-version: ${{ matrix.config.r }} + use-public-rspm: true + + - name: Install R Packages Required For Tests + uses: r-lib/actions/setup-r-dependencies@v2 + with: + packages: + data.table + tibble + + - name: Setup Build Environment + run: | + sudo apt-get update + + - name: Setup SSH access + uses: mxschmitt/action-tmate@v3 + if: ${{ inputs.ssh }} + timeout-minutes: 30 + + - name: Run Unit Tests + id: test + run: | + cargo test --verbose diff --git a/.github/workflows/test-macos.yml b/.github/workflows/test-macos.yml new file mode 100644 index 000000000..42f5ff388 --- /dev/null +++ b/.github/workflows/test-macos.yml @@ -0,0 +1,38 @@ +name: "Test Ark - macOS" + +on: + workflow_call: + workflow_dispatch: + +jobs: + macos: + runs-on: macos-latest + name: "Rust: ${{ matrix.config.rust }}, R: ${{ matrix.config.r }}" + strategy: + fail-fast: false + matrix: + config: + - { rust: 'stable', r: 'release' } + timeout-minutes: 30 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + steps: + - uses: actions/checkout@v4 + + - name: Install R + uses: r-lib/actions/setup-r@v2 + with: + r-version: ${{ matrix.config.r }} + use-public-rspm: true + + - name: Install R Packages Required For Tests + uses: r-lib/actions/setup-r-dependencies@v2 + with: + packages: + data.table + tibble + + - name: Run Unit Tests + id: test + run: | + cargo test --verbose diff --git a/.github/workflows/test-windows.yml b/.github/workflows/test-windows.yml new file mode 100644 index 000000000..09fbaf17e --- /dev/null +++ b/.github/workflows/test-windows.yml @@ -0,0 +1,38 @@ +name: "Test Ark - Windows" + +on: + workflow_call: + workflow_dispatch: + +jobs: + windows: + runs-on: windows-latest + name: "Rust: ${{ matrix.config.rust }}, R: ${{ matrix.config.r }}" + strategy: + fail-fast: false + matrix: + config: + - { rust: 'stable', r: 'release' } + timeout-minutes: 30 + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + steps: + - uses: actions/checkout@v4 + + - name: Install R + uses: r-lib/actions/setup-r@v2 + with: + r-version: ${{ matrix.config.r }} + use-public-rspm: true + + - name: Install R Packages Required For Tests + uses: r-lib/actions/setup-r-dependencies@v2 + with: + packages: + data.table + tibble + + - name: Run Unit Tests + id: test + run: | + cargo test --verbose diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 000000000..4fb947530 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,23 @@ +name: "Test Ark" +on: + push: + branches: + - main + pull_request: + workflow_dispatch: + +jobs: + test_macos: + name: Test macOS + uses: ./.github/workflows/test-macos.yml + secrets: inherit + + test_windows: + name: Test Windows + uses: ./.github/workflows/test-windows.yml + secrets: inherit + + test_linux: + name: Test Linux + uses: ./.github/workflows/test-linux.yml + secrets: inherit diff --git a/Cargo.lock b/Cargo.lock index 18b296d8f..11ae1bac7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -830,11 +830,12 @@ checksum = "7fcaabb2fef8c910e7f4c7ce9f67a1283a1715879a7c230ca9d6d1ae31f16d91" [[package]] name = "embed-resource" -version = "2.4.0" +version = "2.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f54cc3e827ee1c3812239a9a41dede7b4d7d5d5464faa32d71bd7cba28ce2cb2" +checksum = "f4e24052d7be71f0efb50c201557f6fe7d237cfd5a64fd5bcd7fd8fe32dbbffa" dependencies = [ "cc", + "memchr", "rustc_version", "toml 0.8.8", "vswhom", @@ -1108,6 +1109,7 @@ dependencies = [ "anyhow", "cfg-if", "ctor", + "embed-resource", "harp-macros", "itertools", "libc", @@ -1640,9 +1642,9 @@ checksum = "2532096657941c2fea9c289d370a250971c689d4f143798ff67113ec042024a5" [[package]] name = "memchr" -version = "2.6.4" +version = "2.7.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f665ee40bc4a3c5590afb1e9677db74a508659dfd71e126420da8274909a0167" +checksum = "78ca9ab1a0babb1e7d5695e3530886289c18cf2f87ec19a575a0abdce112e3a3" [[package]] name = "memoffset" @@ -3731,9 +3733,9 @@ dependencies = [ [[package]] name = "winreg" -version = "0.51.0" +version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "937f3df7948156640f46aacef17a70db0de5917bda9c92b0f751f3a955b588fc" +checksum = "a277a57398d4bfa075df44f501a17cfdf8542d224f0d36095a2adc7aee4ef0a5" dependencies = [ "cfg-if", "windows-sys 0.48.0", diff --git a/crates/ark/Cargo.toml b/crates/ark/Cargo.toml index 72d69e945..fb8082887 100644 --- a/crates/ark/Cargo.toml +++ b/crates/ark/Cargo.toml @@ -68,7 +68,7 @@ stdext = { path = "../stdext", features = ["testing"]} [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" }] diff --git a/crates/ark/build.rs b/crates/ark/build.rs index cf0a1a9f3..ed31800d1 100644 --- a/crates/ark/build.rs +++ b/crates/ark/build.rs @@ -36,9 +36,27 @@ fn main() { println!("cargo:rustc-env=BUILD_DATE={}", build_date); // Embed an Application Manifest file on Windows. + // Turns on UTF-8 support and declares our Windows version compatibility. // Documented to do nothing on non-Windows. + // See . + // + // 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); } diff --git a/crates/ark/resources/manifest/ark-manifest.rc b/crates/ark/resources/manifest/ark-manifest.rc index 571caf3f7..4df2a418e 100644 --- a/crates/ark/resources/manifest/ark-manifest.rc +++ b/crates/ark/resources/manifest/ark-manifest.rc @@ -1,3 +1,5 @@ +// Autogenerated, do not modify! See `template.rc`. + // 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 @@ -7,5 +9,5 @@ // https://github.com/nabijaczleweli/rust-embed-resource/issues/11#issuecomment-779295722 #include -// 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" diff --git a/crates/ark/resources/manifest/ark.exe.manifest b/crates/ark/resources/manifest/ark.exe.manifest index 5c68fb45b..8be536132 100644 --- a/crates/ark/resources/manifest/ark.exe.manifest +++ b/crates/ark/resources/manifest/ark.exe.manifest @@ -3,6 +3,7 @@ + ark diff --git a/crates/ark/src/modules/positron/r_data_explorer.R b/crates/ark/src/modules/positron/r_data_explorer.R index 98e5e928f..a18ee40fc 100644 --- a/crates/ark/src/modules/positron/r_data_explorer.R +++ b/crates/ark/src/modules/positron/r_data_explorer.R @@ -342,13 +342,38 @@ export_selection <- function(x, format = c("csv", "tsv", "html"), include_header } write_delim <- function(x, delim, include_header) { - tmp <- tempfile() - defer(unlink(tmp)) + path <- tempfile() + defer(unlink(path)) - utils::write.table(x, tmp, sep = delim, row.names = FALSE, col.names = include_header, quote = FALSE, na = "") - # We use size - 1 because we don't want to read the last newline character - # that creates problems when pasting the content in spreadsheets - readChar(tmp, file.info(tmp)$size - 1L) + write_delim_impl(x, path, delim, include_header) + + # We use `size - 1` because we don't want to read the last newline character + # as that creates problems when pasting the content in spreadsheets. + # `file.info()$size` reports the size in bytes, hence `useBytes = TRUE`. + readChar(path, file.info(path)$size - 1L, useBytes = TRUE) +} + +write_delim_impl <- function(x, path, delim, include_header) { + # Scope the `con` lifetime to just this helper. + # We need to `close()` the connection before we try and get the + # `file.info()$size`. + + # Must open in binary write mode, otherwise even though we set + # `eol = "\n"` on Windows it will still write `\r\n` due to the C + # level `vfprintf()` call. + con <- file(path, open = "wb") + defer(close(con)) + + utils::write.table( + x = x, + file = con, + sep = delim, + eol = "\n", + row.names = FALSE, + col.names = include_header, + quote = FALSE, + na = "" + ) } write_html <- function(x, include_header) { @@ -415,7 +440,9 @@ profile_histogram <- function(x, method = c("fixed", "sturges", "fd", "scott"), # For dates, we convert back the breaks to the date representation. if (inherits(x, "POSIXct")) { - bin_edges <- as.POSIXct(h$breaks, tz = attr(x, "tzone")) + # Must supply an `origin` on R <= 4.2 + origin <- as.POSIXct("1970-01-01", tz = "UTC") + bin_edges <- as.POSIXct(h$breaks, tz = attr(x, "tzone"), origin = origin) } list( diff --git a/crates/ark/src/sys/windows.rs b/crates/ark/src/sys/windows.rs index 6651ef627..19571fe85 100644 --- a/crates/ark/src/sys/windows.rs +++ b/crates/ark/src/sys/windows.rs @@ -8,6 +8,7 @@ pub mod console; pub mod control; pub mod interface; +mod locale; pub mod path; pub mod signals; mod strings; diff --git a/crates/ark/src/sys/windows/locale.rs b/crates/ark/src/sys/windows/locale.rs new file mode 100644 index 000000000..4280a4404 --- /dev/null +++ b/crates/ark/src/sys/windows/locale.rs @@ -0,0 +1,33 @@ +/* + * locale.rs + * + * Copyright (C) 2024 Posit Software, PBC. All rights reserved. + * + */ + +#[cfg(test)] +mod tests { + use crate::r_task; + + #[test] + fn test_locale() { + // These tests assert that we've embedded our Application Manifest file correctly in `build.rs` + r_task(|| { + 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); + }) + } +} diff --git a/crates/harp/Cargo.toml b/crates/harp/Cargo.toml index 224356510..e0dbf6f6f 100644 --- a/crates/harp/Cargo.toml +++ b/crates/harp/Cargo.toml @@ -29,3 +29,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" diff --git a/crates/harp/build.rs b/crates/harp/build.rs new file mode 100644 index 000000000..888d7341f --- /dev/null +++ b/crates/harp/build.rs @@ -0,0 +1,27 @@ +// +// build.rs +// +// Copyright (C) 2024 Posit Software, PBC. All rights reserved. +// +// + +use std::path::Path; +extern crate embed_resource; + +/// [main()] +fn main() { + // Embed an Application Manifest file on Windows. + // Turns on UTF-8 support and declares our Windows version compatibility. + // Documented to do nothing on non-Windows. + // See . + // + // 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); +} diff --git a/crates/harp/resources/manifest/harp-manifest.rc b/crates/harp/resources/manifest/harp-manifest.rc new file mode 100644 index 000000000..b5d647516 --- /dev/null +++ b/crates/harp/resources/manifest/harp-manifest.rc @@ -0,0 +1,13 @@ +// Autogenerated, do not modify! See `template.rc`. + +// 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 + +// The `1` is a `resource_id` that specifies us as an executable file +1 RT_MANIFEST "harp.exe.manifest" diff --git a/crates/harp/resources/manifest/harp.exe.manifest b/crates/harp/resources/manifest/harp.exe.manifest new file mode 100644 index 000000000..f0055309a --- /dev/null +++ b/crates/harp/resources/manifest/harp.exe.manifest @@ -0,0 +1,41 @@ + + + + + + + harp + + + + + UTF-8 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/crates/harp/src/sys/windows.rs b/crates/harp/src/sys/windows.rs index 95932ce46..dff45856b 100644 --- a/crates/harp/src/sys/windows.rs +++ b/crates/harp/src/sys/windows.rs @@ -7,4 +7,5 @@ pub mod library; pub mod line_ending; +mod locale; pub mod polled_events; diff --git a/crates/harp/src/sys/windows/locale.rs b/crates/harp/src/sys/windows/locale.rs new file mode 100644 index 000000000..4280a4404 --- /dev/null +++ b/crates/harp/src/sys/windows/locale.rs @@ -0,0 +1,33 @@ +/* + * locale.rs + * + * Copyright (C) 2024 Posit Software, PBC. All rights reserved. + * + */ + +#[cfg(test)] +mod tests { + use crate::r_task; + + #[test] + fn test_locale() { + // These tests assert that we've embedded our Application Manifest file correctly in `build.rs` + r_task(|| { + 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); + }) + } +} diff --git a/scripts/manifest/generate.R b/scripts/manifest/generate.R new file mode 100644 index 000000000..f116098eb --- /dev/null +++ b/scripts/manifest/generate.R @@ -0,0 +1,42 @@ +#' Write out templated Windows Application Manifest files +#' +#' We use two files to embed an application manifest on Windows: +#' - `{crate}-manifest.rc` +#' - `{crate}.exe.manifest` +#' +#' And we generate these two files for both `harp` and `ark`. +#' +#' They are effectively the same, we just need to swap out the crate names, +#' so we use this script to write the files in a consistent manner. +write_manifest <- function(root, crate) { + crate_exe <- glue::glue("{crate}.exe.manifest") + + dest_folder <- file.path(root, "crates", crate, "resources", "manifest") + dest_path_rc <- file.path(dest_folder, glue::glue("{crate}-manifest.rc")) + dest_path_manifest <- file.path(dest_folder, crate_exe) + + src_folder <- file.path(root, "scripts", "manifest") + src_path_rc <- file.path(src_folder, "template.rc") + src_path_manifest <- file.path(src_folder, "template.manifest") + + # Write `{crate}-manifest.rc` + data <- list(name = crate_exe) + text <- brio::read_file(src_path_rc) + text <- whisker::whisker.render(text, data = data) + brio::write_file(text, dest_path_rc) + + # Write `{crate}.exe.manifest` + data <- list(name = crate) + text <- brio::read_file(src_path_manifest) + text <- whisker::whisker.render(text, data = data) + brio::write_file(text, dest_path_manifest) + + invisible(NULL) +} + +root <- here::here() +crates <- c("ark", "harp") + +for (crate in crates) { + write_manifest(root, crate) +} diff --git a/scripts/manifest/template.manifest b/scripts/manifest/template.manifest new file mode 100644 index 000000000..647b87f88 --- /dev/null +++ b/scripts/manifest/template.manifest @@ -0,0 +1,41 @@ + + + + + + + {{name}} + + + + + UTF-8 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/scripts/manifest/template.rc b/scripts/manifest/template.rc new file mode 100644 index 000000000..25cc16a5c --- /dev/null +++ b/scripts/manifest/template.rc @@ -0,0 +1,13 @@ +// Autogenerated, do not modify! See `template.rc`. + +// 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 + +// The `1` is a `resource_id` that specifies us as an executable file +1 RT_MANIFEST "{{name}}"