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

xtask: fix interplay between fmt-on-save and xsync #925

Merged
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
41 changes: 22 additions & 19 deletions Guide/src/dev_guide/getting_started/suggested_dev_env.md
Original file line number Diff line number Diff line change
Expand Up @@ -153,21 +153,31 @@ adding the following line to `keybindings.json`:
}
```

### Running `cargo xtask fmt house-rules` on-save
### GitHub Pull Request Integration

The OpenVMM project includes a handful of custom "house rule" lints that are
external to `rustfmt`. These are things like checking for the presence of
copyright headers, enforcing single-trailing newlines, etc...
As the repo is hosted on GitHub, you might find convenient to use the
[GitHub Pull Request](https://marketplace.visualstudio.com/items?itemName=GitHub.vscode-pull-request-github)
VSCode extension. That allows working through the PR feedback and
issues without leaving the comfort of VSCode.

These lints are enfoced using `cargo xtask fmt house-rules`, and can be
automatically fixed by passing the `--fix` flag.
### (Possibly Useful) Enabling 'house-rules' formatting on-save

We recommend installing the
[RunOnSave](https://marketplace.visualstudio.com/items?itemName=emeraldwalk.RunOnSave)
extension, and configuring it to run these lints as part of your regular
development flow.
Aside from using `rustfmt`, the OpenVMM project also relies on a handful of
extra formatting "house rules". e.g: enfocing the presence of copyright headers,
enforcing single-trailing newlines, etc...

CI will fail if files are not formatted with `cargo xtask fmt house-rules`.

Set the following configuration in your `.vscode/settings.json`
In general, there are 3 ways to fix "house rules" related lints:

1. Manually fixing issues in response to automated feedback
2. Invoking `cargo xtask fmt house-rules --fix` to fix the whole project
3. Invoking `cargo xtask fmt house-rules --fix [FILE]` to fix a given file

If you would prefer having "house-rules" enfoced whenever you save a file in
VSCode, you can install the
[RunOnSave](https://marketplace.visualstudio.com/items?itemName=emeraldwalk.RunOnSave)
extension, and add the following configuration to `.vscode/settings.json`:

```json
{
Expand All @@ -180,20 +190,13 @@ Set the following configuration in your `.vscode/settings.json`
{
"match": ".*",
"isAsync": true,
"cmd": "$(cat ./target/xtask-path) fmt house-rules --fix ${file}"
"cmd": "$(cat ./target/xtask-path) --run-on-save fmt house-rules --fix ${file}"
}
]
},
}
```

### GitHub Pull Request Integration

As the repo is hosted on GitHub, you might find convenient to use the
[GitHub Pull Request](https://marketplace.visualstudio.com/items?itemName=GitHub.vscode-pull-request-github)
VSCode extension. That allows working through the PR feedback and
issues without leaving the comfort of VSCode.

## Setting up pre-commit and pre-push hooks

It's never fun having CI reject your changes due to some minor formatting issue,
Expand Down
35 changes: 26 additions & 9 deletions xtask/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ pub struct XtaskCtx {
pub root: PathBuf,
/// xtask is running within a hook
pub in_git_hook: bool,
/// xtask was invoked as part of a "run on save" operation
pub in_run_on_save: bool,
}

/// Common trait implemented by all Xtask subcommands.
Expand All @@ -59,6 +61,19 @@ struct Cli {
/// repo, and any custom out-of-tree overlay repos.
#[clap(long)]
custom_root: Option<PathBuf>,

/// Signal that this `xtask` is being invoked as part of a "run on save"
/// operation.
///
/// When set, certain tasks may choose to skip certain expensive checks in
/// order to execute as quickly as possible.
///
/// e.g: instead of calling `cargo run` in order to execute a project-local
/// tool, `xtask` may instead attempt to find and use an existing pre-built
/// binary, if one is available. This will be faster, but may run the risk
/// of executing a slightly stale binary.
#[clap(long)]
run_on_save: bool,
}

#[expect(clippy::large_enum_variant)]
Expand Down Expand Up @@ -93,25 +108,26 @@ fn main() {
fn try_main() -> anyhow::Result<()> {
let cli = Cli::parse();

let orig_root = Path::new(&env!("CARGO_MANIFEST_DIR"))
.ancestors()
.nth(1)
.unwrap()
.to_path_buf();

let root = cli
.custom_root
.map(std::path::absolute)
.transpose()?
.unwrap_or_else(|| {
Path::new(&env!("CARGO_MANIFEST_DIR"))
.ancestors()
.nth(1)
.unwrap()
.to_path_buf()
});
.unwrap_or(orig_root.clone());

// for consistency, always run xtasks as though they were run from the root
std::env::set_current_dir(&root)?;

// drop the path to the xtask binary in an easy-to-find place. this gets
// used by the pre-commit hook to avoid rebuilding the xtask.
// used by the pre-commit hook, as well as the fmt-on-save dev-flow to avoid
// rebuilding the xtask.
if let Ok(path) = std::env::current_exe() {
if let Err(e) = fs_err::write(XTASK_PATH_FILE, path.display().to_string()) {
if let Err(e) = fs_err::write(orig_root.join(XTASK_PATH_FILE), path.display().to_string()) {
log::debug!("Unable to create XTASK_PATH_FILE: {:#}", e)
}
}
Expand All @@ -123,6 +139,7 @@ fn try_main() -> anyhow::Result<()> {
let ctx = XtaskCtx {
root,
in_git_hook: matches!(cli.command, Commands::Hook(..)),
in_run_on_save: cli.run_on_save,
};

match cli.command {
Expand Down
2 changes: 0 additions & 2 deletions xtask/src/tasks/verify_size.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

// Copyright (C) Microsoft Corporation. All rights reserved.

use crate::Xtask;
use anyhow::Context;
use object::read::Object;
Expand Down