Skip to content

Commit

Permalink
Fix build for windows and osx.
Browse files Browse the repository at this point in the history
OSX had an issue with max UDP packet size
Windows has all sorts of file related issues. On the plus side the special handling for windows in circle has been removed as the root cause of the hang on windows is now known to be: rust-lang/rust#45572
  • Loading branch information
bryn committed Apr 19, 2022
1 parent 6be0629 commit ec1e960
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 57 deletions.
38 changes: 9 additions & 29 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ parameters:
cache_version:
type: string
# increment that one to invalidate all the caches
default: v3.{{ checksum "rust-toolchain.toml" }}
default: v5.{{ checksum "rust-toolchain.toml" }}

commands:
initialize_submodules:
Expand Down Expand Up @@ -88,6 +88,12 @@ commands:
- run:
name: Assert npm version
command: test "$(npm --version)" = "${NPM_VERSION}"
- run:
# The jaeger exporter won't work without this
name: Increase udp packet size
command: |
sudo sysctl net.inet.udp.maxdgram=65536
sudo sysctl net.inet.udp.maxdgram
linux_prepare_node_env:
steps:
#TODO[igni]: check for node version before we try to install it
Expand Down Expand Up @@ -143,33 +149,6 @@ commands:
command: |
if ((npm --version) -Ne "${Env:NPM_VERSION}") { exit 1 }
windows_prepare_test_env:
steps:
- restore_cache:
keys:
- subgraph-node-modules-v2-windows-{{ checksum "dockerfiles/federation-demo/federation-demo/package.json" }}-{{ checksum "dockerfiles/federation-demo/federation-demo/package-lock.json" }}
- subgraph-node-modules-v2-windows-{{ checksum "dockerfiles/federation-demo/federation-demo/package.json" }}
- run:
name: npm clean-install
working_directory: dockerfiles/federation-demo/federation-demo
command: npm clean-install
- save_cache:
key: subgraph-node-modules-v2-windows-{{ checksum "dockerfiles/federation-demo/federation-demo/package.json" }}-{{ checksum "dockerfiles/federation-demo/federation-demo/package-lock.json" }}
paths:
- dockerfiles/federation-demo/federation-demo/node_modules

# TODO: normally xtask can run the federation by itself and it
# works on GitHub Actions on Windows. Unfortunately it
# doesn't work here on CircleCI on Windows only.
- run:
name: start federation-demo (background)
working_directory: dockerfiles/federation-demo/federation-demo
command: npm start
background: true
- run:
name: wait for federation demo to start
command: npx wait-on tcp:4001 tcp:4002 tcp:4003 tcp:4004 tcp:4100

windows_prepare_rust_env:
steps:
- run:
Expand Down Expand Up @@ -501,6 +480,7 @@ jobs:
condition:
equal: [*rust_windows_executor, << parameters.platform >>]
steps:
- windows_install_baseline
- windows_prepare_node_env
- windows_prepare_rust_env
- windows_build_workspace
Expand Down Expand Up @@ -537,8 +517,8 @@ jobs:
condition:
equal: [*rust_windows_executor, << parameters.platform >>]
steps:
- windows_install_baseline
- windows_prepare_node_env
- windows_prepare_test_env
- windows_prepare_rust_env
- windows_test_workspace
- when:
Expand Down
3 changes: 3 additions & 0 deletions NEXT_CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,4 +104,7 @@ Jaeger tracing end to end test including client->router->subgraphs
### Router tracing span cleanup [PR #850](https://github.com/apollographql/router/pull/850)
Spans generated by the Router are now aligned with plugin services.

### Simplified CI for windows [PR #850](https://github.com/apollographql/router/pull/850)
All windows processes are spawned via xtask rather than a separate CircleCI stage.

## 📚 Documentation
44 changes: 31 additions & 13 deletions apollo-router/tests/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use opentelemetry::propagation::TextMapPropagator;
use opentelemetry::sdk::trace::Tracer;
use opentelemetry_http::HttpClient;
use serde_json::Value;
use std::path::Path;
use std::path::{Path, PathBuf};
use std::process::{Child, Command, Stdio};
use std::time::Duration;
use tower::BoxError;
Expand Down Expand Up @@ -39,25 +39,29 @@ impl TracingTest {
tracing::subscriber::set_global_default(subscriber).unwrap();
global::set_text_map_propagator(propagator);

let config_location = Path::new("apollo-router/src/testdata").join(config_location);
tracing::debug!(
"starting router with config: {}",
config_location.to_string_lossy()
);
let router_location = if cfg!(windows) {
PathBuf::from_iter(["..", "target", "debug", "router.exe"])
} else {
PathBuf::from_iter(["..", "target", "debug", "router"])
};

let mut command = Command::new(router_location);
command = set_command_log(command);

Self {
router: Command::new("target/debug/router")
.current_dir("..")
.env("RUST_LOG", "INFO")
router: command
.args([
"--hr",
"--config",
&config_location.to_string_lossy().to_string(),
&PathBuf::from_iter(["..", "apollo-router", "src", "testdata"])
.join(config_location)
.to_string_lossy()
.to_string(),
"--supergraph",
"examples/graphql/local.graphql",
&PathBuf::from_iter(["..", "examples", "graphql", "local.graphql"])
.to_string_lossy()
.to_string(),
])
.stdout(Stdio::piped())
.stderr(Stdio::piped())
.spawn()
.expect("Router should start"),
}
Expand Down Expand Up @@ -103,6 +107,20 @@ impl TracingTest {
}
}

#[cfg(windows)]
fn set_command_log(mut command: Command) -> Command {
// Pipe to NULL is required for Windows to not hang
// https://github.com/rust-lang/rust/issues/45572
command.stdout(Stdio::null()).stderr(Stdio::null());
command
}

#[cfg(not(windows))]
fn set_command_log(mut command: Command) -> Command {
command.stdout(Stdio::piped()).stderr(Stdio::piped());
command
}

impl Drop for TracingTest {
fn drop(&mut self) {
self.router.kill().expect("router could not be halted");
Expand Down
10 changes: 1 addition & 9 deletions xtask/src/commands/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,7 @@ impl Test {
);

// NOTE: it worked nicely on GitHub Actions but it hangs on CircleCI on Windows
let _guard: Box<dyn std::any::Any> = if !std::env::var("CIRCLECI")
.ok()
.unwrap_or_default()
.is_empty()
&& cfg!(windows)
{
eprintln!("Not running federation-demo because it makes the step hang on Circle CI.");
Box::new(())
} else if self.no_demo {
let _guard: Box<dyn std::any::Any> = if self.no_demo {
eprintln!("Flag --no-demo is the default now. Not running federation-demo.");
Box::new(())
} else if !self.with_demo {
Expand Down
7 changes: 5 additions & 2 deletions xtask/src/federation_demo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,14 @@ impl FederationDemoRunner {

eprintln!("Running federation-demo in background...");
let mut command = Command::new(which::which("npm")?);

// Pipe to NULL is required for Windows to not hang
// https://github.com/rust-lang/rust/issues/45572
command
.current_dir(&self.path)
.args(["run", "start"])
.stdout(Stdio::piped())
.stderr(Stdio::piped());
.stdout(Stdio::null())
.stderr(Stdio::null());
let task = BackgroundTask::new(command)?;

eprintln!("Waiting for federation-demo services and gateway to be ready...");
Expand Down
20 changes: 16 additions & 4 deletions xtask/src/jaeger.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::*;
use anyhow::Result;
use camino::Utf8PathBuf;
use std::path::Path;
use std::path::PathBuf;
use std::{
process::{Command, Stdio},
thread::sleep,
Expand All @@ -21,11 +21,23 @@ impl JaegerRunner {

pub fn start_background(&self) -> Result<BackgroundTask> {
eprintln!("Running jaeger in background...");
let mut command = Command::new(Path::new("jaeger/jaeger-all-in-one"));
let jaeger_path = if cfg!(windows) {
PathBuf::from_iter(["jaeger", "jaeger-all-in-one.exe"])
} else {
PathBuf::from_iter(["jaeger", "jaeger-all-in-one"])
};

let mut command = Command::new(jaeger_path);

// Pipe to NULL is required for Windows to not hang
// https://github.com/rust-lang/rust/issues/45572
command
.current_dir(&self.path)
.stdout(Stdio::piped())
.stderr(Stdio::piped());
.stdout(Stdio::null())
.stderr(Stdio::null());

if cfg!(windows) {}

let task = BackgroundTask::new(command)?;

loop {
Expand Down

0 comments on commit ec1e960

Please sign in to comment.