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

Windows GHA workflow #175

Merged
merged 23 commits into from
Dec 12, 2023
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
122 changes: 108 additions & 14 deletions .github/workflows/macos-release.yml → .github/workflows/release.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
name: "Build macOS Release"
name: "Build Ark Release"
on:
push:
branches:
Expand Down Expand Up @@ -76,8 +76,8 @@ jobs:
echo "Existing ${{ needs.get_version.outputs.ARK_VERSION }} release: ${{steps.check_tag.outputs.exists}}"
echo "result=${{steps.check_tag.outputs.exists}}" >> $GITHUB_OUTPUT

# Build ARK for both arm64 (Apple Silicon) and x64 (Intel) hosts.
build_archs:
# Build ARK for macOS. Both arm64 (Apple Silicon) and x64 (Intel) hosts.
build_macos:
name: Build macOS
runs-on: [self-hosted, macos, arm64]
needs: [revive_agent, get_version]
Expand Down Expand Up @@ -183,10 +183,79 @@ jobs:
name: ark-${{ matrix.flavor }}-darwin-${{ matrix.arch }}-archive
path: ark-${{ needs.get_version.outputs.ARK_VERSION }}-${{ matrix.flavor }}-darwin-${{ matrix.arch }}.zip

build_windows:
name: Build Windows
runs-on: windows-latest
timeout-minutes: 40
needs: [get_version]

env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

strategy:
matrix:
arch: [x64]
flavor: [debug, release]
include:
- arch: x64
rust_target_prefix: x86_64

steps:
- name: Checkout sources
uses: actions/checkout@v4

- name: Setup R
uses: r-lib/actions/setup-r@v2
with:
r-version: '4.3.2'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we are going to eventually have to decide what our minimum supported R version is, and then build libR-sys against that.

Then for any C utilities that comes from a newer version of R than that, we would dynamically load those out of the user's R.dll at runtime using dlopen or the Windows equivalent. RStudio seems to take this approach in a few places (For example, with R_DefParamsEx(), introduced in R 4.0.0 ish but R really wants you to use that now on startup)

If libR-sys doesn't support that version of R that we pin against, we may have to maintain our own bindings 😢

But otherwise what currently happens is that on Windows we probably can't load up R 4.0.0 or earlier right now due to us using some utilities that came from R versions after that (even if we guard against using those with a runtime R version check, that isn't enough, because the linker needs to resolve the symbols at runtime and it can't)


- name: Find R installation
id: r_installation
shell: bash
run: |
R_HOME="C:\R"
R_SCRIPT="${R_HOME}\bin\x64\Rscript.exe"
echo "Using R at ${R_HOME}"
echo "Using Rscript at ${R_SCRIPT}"
# Output the result for consumption in later steps
echo "r_home=${R_HOME}" >> $GITHUB_OUTPUT
echo "r_script=${R_SCRIPT}" >> $GITHUB_OUTPUT

- name: Generate LIB from DLL
shell: cmd
run: |
${{ steps.r_installation.outputs.r_script }} "scripts\windows\dll2lib.R"

- name: Compile ARK
env:
ARK_BUILD_TYPE: ${{ matrix.flavor }}
RUST_TARGET: ${{ matrix.rust_target_prefix }}-pc-windows-msvc
R_HOME: ${{ steps.r_installation.outputs.r_home }}
shell: cmd
run: |
cargo clean
cargo build ${{ matrix.flavor == 'release' && '--release' || '' }} --target ${{ matrix.rust_target_prefix }}-pc-windows-msvc

- name: Create archive
shell: pwsh
run: |
# Compress the kernel to an archive
$params = @{
Path = "target\${{ matrix.rust_target_prefix }}-pc-windows-msvc\${{ matrix.flavor }}\ark.exe"
DestinationPath = "ark-${{ needs.get_version.outputs.ARK_VERSION }}-${{ matrix.flavor }}-windows-${{ matrix.arch }}.zip"
}
Compress-Archive @params

- name: Upload client archive
uses: actions/upload-artifact@v3
with:
name: ark-${{ matrix.flavor }}-windows-${{ matrix.arch }}-archive
path: ark-${{ needs.get_version.outputs.ARK_VERSION }}-${{ matrix.flavor }}-windows-${{ matrix.arch }}.zip

create_release:
name: Create Release
runs-on: [self-hosted, macos, arm64]
needs: [get_version, build_archs, check_release]
needs: [get_version, build_macos, build_windows, check_release]
if: ${{ needs.check_release.outputs.EXISTING_RELEASE == 'false' }}
env:
GITHUB_TOKEN: ${{ github.token }}
Expand Down Expand Up @@ -217,19 +286,24 @@ jobs:
flavor: [debug, release]

steps:
# Download arm64 and x64 binaries
- name: Download arm64 kernel (${{ matrix.flavor }})
# Download all binaries
- name: Download macOS arm64 kernel (${{ matrix.flavor }})
uses: actions/download-artifact@v3
with:
name: ark-${{ matrix.flavor }}-darwin-arm64-archive

- name: Download x64 kernel (${{ matrix.flavor}})
- name: Download macOS x64 kernel (${{ matrix.flavor}})
uses: actions/download-artifact@v3
with:
name: ark-${{ matrix.flavor }}-darwin-x64-archive

# Combine them to a single binary with lipo
- name: Create universal binary
- name: Download Windows x64 kernel (${{ matrix.flavor}})
uses: actions/download-artifact@v3
with:
name: ark-${{ matrix.flavor }}-windows-x64-archive

# Combine macOS binaries to a single binary with lipo
- name: Create macOS universal binary
run: |
# Decompress x64 builds
rm -rf x64 && mkdir x64 && pushd x64
Expand All @@ -248,13 +322,23 @@ jobs:
ARCHIVE="$GITHUB_WORKSPACE/ark-${{ needs.get_version.outputs.ARK_VERSION }}${{ env.DEBUG_FLAG }}-darwin-universal.zip"
zip -Xry $ARCHIVE ark

# Add the R modules (these aren't architecture dependent)
echo "Adding R modules ..."
# Add the R modules (these aren't architecture dependent)
- name: Add Ark R modules
run: |
ARCHIVE_MAC="$GITHUB_WORKSPACE/ark-${{ needs.get_version.outputs.ARK_VERSION }}${{ env.DEBUG_FLAG }}-darwin-universal.zip"
ARCHIVE_WINDOWS="$GITHUB_WORKSPACE/ark-${{ needs.get_version.outputs.ARK_VERSION }}-${{ matrix.flavor }}-windows-x64.zip"

pushd crates/ark/src
zip -Xry $ARCHIVE modules

echo "Adding R modules to macOS binary ..."
zip -Xry $ARCHIVE_MAC modules

echo "Adding R modules to Windows binary ..."
zip -Xry $ARCHIVE_WINDOWS modules

popd

- name: Upload release artifact (universal)
- name: Upload macOS release artifact (universal)
uses: actions/upload-release-asset@v1
env:
GITHUB_TOKEN: ${{ github.token }}
Expand All @@ -264,10 +348,20 @@ jobs:
asset_name: ark-${{ needs.get_version.outputs.ARK_VERSION }}${{ env.DEBUG_FLAG }}-darwin-universal.zip
asset_content_type: application/octet-stream

- name: Upload Windows release artifact (x64)
uses: actions/upload-release-asset@v1
env:
GITHUB_TOKEN: ${{ github.token }}
with:
upload_url: ${{ needs.create_release.outputs.upload_url }}
asset_path: ark-${{ needs.get_version.outputs.ARK_VERSION }}-${{ matrix.flavor }}-windows-x64.zip
asset_name: ark-${{ needs.get_version.outputs.ARK_VERSION }}${{ env.DEBUG_FLAG }}-windows-x64.zip
asset_content_type: application/octet-stream

status:
if: ${{ failure() }}
runs-on: self-hosted
needs: [build_archs, get_version]
needs: [build_macos, build_windows, get_version]
steps:
- name: Notify slack if build fails
uses: slackapi/[email protected]
Expand Down
2 changes: 1 addition & 1 deletion 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
@@ -1,6 +1,6 @@
[package]
name = "ark"
version = "0.1.32"
version = "0.1.33"
edition = "2021"
rust-version = "1.70.0"
description = """
Expand Down
4 changes: 2 additions & 2 deletions crates/ark/src/modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,8 @@ pub unsafe fn initialize(testing: bool) -> anyhow::Result<()> {
// look in the source tree (found via the 'CARGO_MANIFEST_DIR' environment
// variable).
if !root.exists() {
let source = format!("{}/src/modules", env!("CARGO_MANIFEST_DIR"));
root = Path::new(&source).to_path_buf();
let source = env!("CARGO_MANIFEST_DIR");
root = Path::new(&source).join("src").join("modules").to_path_buf();
Comment on lines -169 to +170
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed a bug that only showed up in release builds. Need \ on Windows!

}

// Import all module files.
Expand Down
4 changes: 2 additions & 2 deletions crates/harp/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -648,8 +648,8 @@ fn init_modules() {
// look in the source tree (found via the 'CARGO_MANIFEST_DIR' environment
// variable).
if !root.exists() {
let source = format!("{}/src/modules", env!("CARGO_MANIFEST_DIR"));
root = std::path::Path::new(&source).to_path_buf();
let source = env!("CARGO_MANIFEST_DIR");
root = Path::new(&source).join("src").join("modules").to_path_buf();
}

log::info!("Loading modules from directory: {}", root.display());
Expand Down
84 changes: 45 additions & 39 deletions scripts/windows/dll2lib.R
Original file line number Diff line number Diff line change
Expand Up @@ -11,55 +11,61 @@

# Typically something like:
# "C:\\Program Files\\Microsoft Visual Studio\\2022\\Community\\VC\\Tools\\MSVC\\14.37.32822\\bin\\Hostx86\\x86"
# We try to dynamically look up the year (2022) and exact version (14.37.32822)
# in case they change on us
# A number of the pieces are unstable (year, Community vs Enterprise, exact version)
# so we try and use `vswhere.exe` and `vsdevcmd.bat` to find the exact path
Comment on lines -14 to +15
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you can mostly ignore the specifics of the changes in this file. I thought I had it right before, but that was with a Community version of the vs tools, and the Enterprise version is on the runner.

Anyways, I think it should be more robust now through the use of the "official" vswhere utility


get_visual_studio_tools_directory <- function() {
path <- file.path("C:", "Program Files", "Microsoft Visual Studio")
# First find `vswhere.exe`, which is supposedly always in the same spot
vswhere <- file.path("C:", "Program Files (x86)", "Microsoft Visual Studio", "Installer")

if (!dir.exists(path)) {
stop("Microsoft Visual Studio has not been installed.")
if (!dir.exists(vswhere)) {
stop("Microsoft Visual Studio Installer folder does not exist.")
}

# Dynamically look up the next folder, which should be a year like `2022`
files <- dir(path)
n_files <- length(files)

if (n_files == 0L) {
stop("Expected at least 1 version of Microsoft Visual Studio.")
} else if (n_files == 1L) {
year <- files
} else {
warning("Expected exactly 1 version of Microsoft Visual Studio. Using the last (hopefully newest) version.")
year <- files[[n_files]]
}
vswhere <- file.path(vswhere, "vswhere.exe")
vswhere <- normalizePath(vswhere, mustWork = TRUE)
vswhere <- shQuote(vswhere)
vswhere <- paste(vswhere, "-prerelease -latest -property installationPath")

path <- file.path(path, year, "Community", "VC", "Tools", "MSVC")
# `vswhere` tells us where Microsoft Visual Studio lives
visualstudio <- system(vswhere, intern = TRUE)

if (!dir.exists(path)) {
stop("Microsoft Visual Studio tools have not been installed.")
if (!is.character(visualstudio) && length(visualstudio) != 1L && !is.na(visualstudio) && !dir.exists(visualstudio)) {
stop("`vswhere` failed to find Microsoft Visual Studio")
}

# Dynamically look up the next folder, which should be a very specific version
# of the tools like `14.38.33130`
files <- dir(path)
n_files <- length(files)

if (n_files == 0L) {
stop("Expected at least 1 version of the Microsoft Visual Studio tools.")
} else if (n_files == 1L) {
version <- files
} else {
warning("Expected exactly 1 version of the Microsoft Visual Studio tools. Using the last (hopefully newest) version.")
version <- files[[n_files]]
# Next we navigate to `vsdevcmd.bat`, which also has a stable path, according
# to https://github.com/microsoft/vswhere/wiki/Start-Developer-Command-Prompt
vscmdbat <- file.path(visualstudio, "Common7", "Tools", "VsDevCmd.bat")
vscmdbat <- normalizePath(vscmdbat, mustWork = TRUE)
vscmdbat <- shQuote(vscmdbat)
vscmdbat <- paste(vscmdbat, "-arch=amd64 -startdir=none -host_arch=amd64 -no_logo")

where <- "where dumpbin.exe"

# Running `VsDevCmd.bat` puts tools like `dumpbin.exe` and `link.exe` on the
# PATH in the current command prompt, so we run that and then ask `where` to
# find `dumpbin.exe` (finding `link.exe` also finds one from RTools).
command <- paste(vscmdbat, "&&", where)
dumpbin <- system(command, intern = TRUE)

if (length(dumpbin) > 1L) {
warning("Found multiple `dumpbin.exe`. Looking for one tied to Visual Studio.")
dumpbin <- dumpbin[grepl("Microsoft Visual Studio", dumpbin)]

if (length(dumpbin) > 1L) {
warning("Still have multiple `dumpbin.exe`. Taking the first.")
dumpbin <- dumpbin[[1L]]
}
}

path <- file.path(path, version, "bin", "Hostx86", "x86")

if (!dir.exists(path)) {
stop("Microsoft Visual Studio tools directory is incorrect or missing.")
if (!is.character(dumpbin) && length(dumpbin) != 1L && !is.na(dumpbin) && !file.exists(dumpbin)) {
stop("`where` failed to find `dumpbin.exe`.")
}

normalizePath(path, mustWork = TRUE)
# Now just look up one level
path <- normalizePath(file.path(dumpbin, ".."))

path
}

# Get the Visual Studio tools directory where `dumpbin.exe` and `lib.exe` live
Expand Down Expand Up @@ -112,7 +118,7 @@ for (dll in dlls) {

# Call 'lib.exe' to generate the library file.
outfile <- sub("dll$", "lib", dll)
fmt <- "lib.exe /def:%s /out:%s /machine:%s"
fmt <- "lib.exe /nologo /def:%s /out:%s /machine:%s"
cmd <- sprintf(fmt, def, outfile, .Platform$r_arch)
system(cmd)

Expand Down