add initial cargo ci #3409
add initial cargo ci #3409
Conversation
|
The tests are failing. I think several of them just need a |
bfops
added
release-any
| run: | | ||
| apt-get update | ||
| apt-get install -y acl curl ca-certificates |
There was a problem hiding this comment.
this part is already done below?
There was a problem hiding this comment.
did we mean to replace parts of the below with cargo ci?
|
|
||
| match cli.cmd { | ||
| Some(CiCmd::Test) => { | ||
| run!("sudo mkdir -p /stdb && sudo chmod 777 /stdb")?; |
There was a problem hiding this comment.
I think this should be part of the CI still? I'm pretty sure we don't want to do this if running it on local machines.
There was a problem hiding this comment.
I looked locally and it looks like this directory isn't even used by the test. I think we can probably remove it
| Some(CiCmd::Smoketests { args }) => { | ||
| // Note: clear_database and replication only work in private | ||
| run!(&format!( | ||
| "python -m smoketests {} -x clear_database replication", |
There was a problem hiding this comment.
I kind of think that the tests to skip should be a parameter, since we may not always want to skip the same things
| default_value = "true", | ||
| long_help = "Whether to enable github token authentication feature when building the update binary. By default this is enabled." | ||
| )] | ||
| github_token_auth: bool, |
There was a problem hiding this comment.
nit: I kind of think this should default to false so that people running this outside of CI don't get errors about the env var not being defined.
| run!("for p in \"$GITHUB_WORKSPACE\" \"${RUNNER_TEMP:-/__t}\" \"${RUNNER_TOOL_CACHE:-/__t}\"; do [ -d \"$p\" ] && setfacl -R -m u:ue4:rwX -m d:u:ue4:rwX \"$p\" || true; done")?; | ||
|
|
||
| run!("export CARGO_HOME=\"${RUNNER_TOOL_CACHE:-/__t}/cargo\"")?; | ||
| run!("export RUSTUP_HOME=\"${RUNNER_TOOL_CACHE:-/__t}/rustup\"")?; |
There was a problem hiding this comment.
FYI I think a lot of this doesn't apply to users running these tests locally
|
|
||
| Some(CiCmd::CliDocs) => { | ||
| run!("pnpm install --recursive")?; | ||
| run!("cargo run --features markdown-docs -p spacetimedb-cli > docs/docs/cli-reference.md")?; |
There was a problem hiding this comment.
note that this assumes we're running from the root directory
| run: | | ||
| apt-get update | ||
| apt-get install -y acl curl ca-certificates |
There was a problem hiding this comment.
did we mean to replace parts of the below with cargo ci?
| run: cargo ci self-docs --check | ||
|
|
||
| cli_docs: | ||
| name: Check CLI docs |
There was a problem hiding this comment.
I assume we want to replace this one with a cargo ci invocation as well?
| @@ -0,0 +1,4 @@ | |||
| #[allow(clippy::disallowed_macros)] | |||
| fn main() { | |||
| println!("cargo:rustc-env=TARGET={}", std::env::var("TARGET").unwrap_or_default()); | |||
There was a problem hiding this comment.
this allows for the default parameter for target on the update-flow. Mostly for running it locally without needing to specify that flag manually.
SpacetimeDB/tools/ci/src/main.rs
Line 177 in 1c0406d
There was a problem hiding this comment.
ah gotcha. in that case, nit: we could either use the variables that are provided already:
const ARCH: &str = env!("CARGO_CFG_TARGET_ARCH");
const OS: &str = env!("CARGO_CFG_TARGET_OS");
const ENV: &str = env!("CARGO_CFG_TARGET_ENV");
or we could only pass --target {target} to cargo build if it's provided by the user.
I kind of prefer that last option. It seems intuitive still.
There was a problem hiding this comment.
(honestly I'm not 100% sure that we even need the target param.. I think the default on each platform might be the one we want anyway.. but we should keep it for now just for parity with the original CI).
| let mut env = env::vars().collect::<HashMap<_, _>>(); | ||
| env.extend(additional_env.iter().map(|(k, v)| (k.to_string(), v.to_string()))); | ||
| log::debug!("$ {cmdline}"); | ||
| let status = cmd!("bash", "-lc", cmdline).full_env(env).run()?; |
There was a problem hiding this comment.
FYI I'm not 100% sure how this will work on windows
|
|
||
| use crate::Cli; | ||
|
|
||
| pub fn generate_cli_docs() -> String { |
There was a problem hiding this comment.
nit: any chance we could use clap_markdown like we do for the SpacetimeDB CLI docs?
There was a problem hiding this comment.
indeed. I didn't notice that we had that
2 participants
Description of Changes
API and ABI breaking changes
Expected complexity level and risk
Testing