Skip to content

Commit da10c76

Browse files
authored
xtask: fix interplay between fmt-on-save and xsync (#925)
Fixing the underlying issue from #914 This PR does a few things: 1. The `xtask-path` logic has been fixed to always drop the file under `target/xtask-path`, regardless of whether a `--custom-root` is being used or not. 2. A new `--run-on-save` parameter has been added to `xtask`, which can be used to skip certain expensive checks when running `xtask` in response to a file save event. It is not currently being used by the open-source `xtask`, but plays an important role in the internal repo's `xtask fmt` implementation. 3. Updates the Guide to make it more clear why one may or may not want to opt into this functionality. **These changes are not particularly interesting in isolation, but make more sense when reviewed alongside the changes being made in the internal repo.** The fact we have this sort of coupling between the oss xtask and the closed-source xtask is unfortunate, and something we should try and move away from in the future...
1 parent 05bde6c commit da10c76

File tree

3 files changed

+48
-30
lines changed

3 files changed

+48
-30
lines changed

Guide/src/dev_guide/getting_started/suggested_dev_env.md

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -153,21 +153,31 @@ adding the following line to `keybindings.json`:
153153
}
154154
```
155155

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

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

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

165-
We recommend installing the
166-
[RunOnSave](https://marketplace.visualstudio.com/items?itemName=emeraldwalk.RunOnSave)
167-
extension, and configuring it to run these lints as part of your regular
168-
development flow.
165+
Aside from using `rustfmt`, the OpenVMM project also relies on a handful of
166+
extra formatting "house rules". e.g: enfocing the presence of copyright headers,
167+
enforcing single-trailing newlines, etc...
168+
169+
CI will fail if files are not formatted with `cargo xtask fmt house-rules`.
169170

170-
Set the following configuration in your `.vscode/settings.json`
171+
In general, there are 3 ways to fix "house rules" related lints:
172+
173+
1. Manually fixing issues in response to automated feedback
174+
2. Invoking `cargo xtask fmt house-rules --fix` to fix the whole project
175+
3. Invoking `cargo xtask fmt house-rules --fix [FILE]` to fix a given file
176+
177+
If you would prefer having "house-rules" enfoced whenever you save a file in
178+
VSCode, you can install the
179+
[RunOnSave](https://marketplace.visualstudio.com/items?itemName=emeraldwalk.RunOnSave)
180+
extension, and add the following configuration to `.vscode/settings.json`:
171181

172182
```json
173183
{
@@ -180,20 +190,13 @@ Set the following configuration in your `.vscode/settings.json`
180190
{
181191
"match": ".*",
182192
"isAsync": true,
183-
"cmd": "$(cat ./target/xtask-path) fmt house-rules --fix ${file}"
193+
"cmd": "$(cat ./target/xtask-path) --run-on-save fmt house-rules --fix ${file}"
184194
}
185195
]
186196
},
187197
}
188198
```
189199

190-
### GitHub Pull Request Integration
191-
192-
As the repo is hosted on GitHub, you might find convenient to use the
193-
[GitHub Pull Request](https://marketplace.visualstudio.com/items?itemName=GitHub.vscode-pull-request-github)
194-
VSCode extension. That allows working through the PR feedback and
195-
issues without leaving the comfort of VSCode.
196-
197200
## Setting up pre-commit and pre-push hooks
198201

199202
It's never fun having CI reject your changes due to some minor formatting issue,

xtask/src/main.rs

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ pub struct XtaskCtx {
3434
pub root: PathBuf,
3535
/// xtask is running within a hook
3636
pub in_git_hook: bool,
37+
/// xtask was invoked as part of a "run on save" operation
38+
pub in_run_on_save: bool,
3739
}
3840

3941
/// Common trait implemented by all Xtask subcommands.
@@ -59,6 +61,19 @@ struct Cli {
5961
/// repo, and any custom out-of-tree overlay repos.
6062
#[clap(long)]
6163
custom_root: Option<PathBuf>,
64+
65+
/// Signal that this `xtask` is being invoked as part of a "run on save"
66+
/// operation.
67+
///
68+
/// When set, certain tasks may choose to skip certain expensive checks in
69+
/// order to execute as quickly as possible.
70+
///
71+
/// e.g: instead of calling `cargo run` in order to execute a project-local
72+
/// tool, `xtask` may instead attempt to find and use an existing pre-built
73+
/// binary, if one is available. This will be faster, but may run the risk
74+
/// of executing a slightly stale binary.
75+
#[clap(long)]
76+
run_on_save: bool,
6277
}
6378

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

111+
let orig_root = Path::new(&env!("CARGO_MANIFEST_DIR"))
112+
.ancestors()
113+
.nth(1)
114+
.unwrap()
115+
.to_path_buf();
116+
96117
let root = cli
97118
.custom_root
98119
.map(std::path::absolute)
99120
.transpose()?
100-
.unwrap_or_else(|| {
101-
Path::new(&env!("CARGO_MANIFEST_DIR"))
102-
.ancestors()
103-
.nth(1)
104-
.unwrap()
105-
.to_path_buf()
106-
});
121+
.unwrap_or(orig_root.clone());
107122

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

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

128145
match cli.command {

xtask/src/tasks/verify_size.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4-
// Copyright (C) Microsoft Corporation. All rights reserved.
5-
64
use crate::Xtask;
75
use anyhow::Context;
86
use object::read::Object;

0 commit comments

Comments
 (0)