1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
|
//! A quick and dirty command-line tool to enforce certain properties about
//! Arti's Cargo.toml files.
//!
//!
//! Definitions.
//!
//! - An **experimental** feature is one for which we do not provide semver guarantees.
//! - A **non-additive** feature is one whose behavior does something other than
//! add functionality to its crate. (For example, building statically or
//! switching out a default is non-additive.)
//! - The **meta** features are `default`, `full`, `experimental`,
//! `__is_nonadditive`, and `__is_experimental`.
//! - The **toplevel** features are `default`, `full`, and `experimental`.
//! - A feature A "is reachable from" some feature B if there is a nonempty path from A
//! to B in the feature graph.
//! - A feature A "directly depends on" some feature B if there is an edge from
//! A to B in the feature graph. We also say that feature B "is listed in"
//! feature A.
//!
//! The properties that we want to enforce are:
//!
//! 1. Every crate has a "full" feature.
//! 2. For every crate within Arti, if we depend on that crate, our "full"
//! includes that crate's "full".
//! 3. Every feature listed in `experimental` depends on `__is_experimental`.
//! Every feature that depends on `__is_experimental` is reachable from `experimental`.
//! Call such features "experimental" features.
//! 4. Call a feature "non-additive" if and only if it depends directly on `__is_nonadditive`.
//! Every non-meta feature we declare is reachable from "full" or "experimental",
//! or it is non-additive.
//! 5. Every feature reachable from `default` is reachable from `full`.
//! 6. No non-additive feature is reachable from `full` or `experimental`.
//! 7. No experimental is reachable from `full`.
//! 8. No in-workspace dependency uses the `*` wildcard version.
//! 9. Only unpublished crates may depend on unpublished crates.
//!
//! This tool can edit Cargo.toml files to enforce the rules 1-3
//! automatically. For rules 4-7, it can annotate any offending features with
//! comments complaining about how they need to be fixed. For rules 8 and 9,
//! it generates warnings.
//!
//! # To use:
//!
//! Run this tool with the top-level Cargo.toml as an argument.
//! Run with `--no-annotate` if you don't want any comments added.
//!
//! # Limitations
//!
//! This is not very efficient, and is not trying to be.
mod changes;
mod graph;
use anyhow::{Context, Result, anyhow};
use std::collections::{HashMap, HashSet};
use std::path::{Path, PathBuf};
use toml_edit::{DocumentMut, Item, Table, Value};
use changes::{Change, Changes};
/// A warning we return from our linter.
///
/// It's a newtype so I don't confuse it with other strings.
#[derive(Debug, Clone)]
struct Warning(String);
/// A dependency from a crate.
///
/// All we care about is the dependency's name, its version,
/// and whether it is optional.
#[derive(Debug, Clone)]
struct Dependency {
name: String,
optional: bool,
version: Option<String>,
}
/// Stored information about a crate.
#[derive(Debug, Clone)]
struct Crate {
/// name of the crate
name: String,
/// path to the crate's Cargo.toml
toml_file: PathBuf,
/// Parsed and manipulated copy of Cargo.toml
toml_doc: DocumentMut,
/// Parsed and un-manipulated copy of Cargo.toml.
toml_doc_orig: DocumentMut,
}
/// Information about a crate that we use in other crates.
#[derive(Debug, Clone)]
struct CrateInfo {
published: bool,
}
/// Given a `[dependencies]` table from a Cargo.toml, find all of the
/// dependencies that are also part of arti.
///
/// We do this by looking for ones that have `path` set.
fn arti_dependencies(dependencies: &Table) -> Vec<Dependency> {
let mut deps = Vec::new();
for (depname, info) in dependencies {
let table = match info {
// Cloning is "inefficient", but we don't care.
Item::Value(Value::InlineTable(info)) => info.clone().into_table(),
Item::Table(info) => info.clone(),
_ => continue, // Not part of arti.
};
if !table.contains_key("path") {
continue; // Not part of arti.
}
let optional = table
.get("optional")
.and_then(Item::as_value)
.and_then(Value::as_bool)
.unwrap_or(false);
let version = table
.get("version")
.and_then(Item::as_value)
.and_then(Value::as_str)
.map(str::to_string);
deps.push(Dependency {
name: depname.to_string(),
optional,
version,
});
}
deps
}
impl Crate {
/// Try to read a crate's Cargo.toml from a given filename.
fn load(p: impl AsRef<Path>) -> Result<Self> {
let toml_file = p.as_ref().to_owned();
let s = std::fs::read_to_string(&toml_file)?;
let toml_doc = s.parse::<DocumentMut>()?;
let toml_doc_orig = toml_doc.clone();
let name = toml_doc["package"]["name"]
.as_str()
.ok_or_else(|| anyhow!("package.name was not a string"))?
.to_string();
Ok(Crate {
name,
toml_file,
toml_doc,
toml_doc_orig,
})
}
/// Extract information about this crate that other crates will need.
fn info(&self) -> CrateInfo {
let package = self
.toml_doc
.get("package")
.expect("no package table!")
.as_table()
.expect("[package] was not a table");
let publish_option = package
.get("publish")
.and_then(Item::as_value)
.and_then(Value::as_bool);
let published = publish_option != Some(false);
CrateInfo { published }
}
/// Try to fix all the issues we find with a Cargo.toml. Return a list of warnings.
fn fix(
&mut self,
no_annotate: bool,
other_crates: &HashMap<String, CrateInfo>,
) -> Result<Vec<Warning>> {
let mut warnings = Vec::new();
let my_info = self.info();
let mut w = |s| warnings.push(Warning(s));
let dependencies = self
.toml_doc
.entry("dependencies")
.or_insert_with(|| Item::Table(Table::new()));
let dependencies = arti_dependencies(
dependencies
.as_table()
.ok_or_else(|| anyhow!("dependencies was not a table"))?,
);
let features = self
.toml_doc
.entry("features")
.or_insert_with(|| Item::Table(Table::new()))
.as_table_mut()
.ok_or_else(|| anyhow!("Features was not table"))?;
let graph = graph::FeatureGraph::from_features_table(features)?;
let mut changes = Changes::default();
// Build a few sets that will be useful a few times below.
let all_features: HashSet<_> = graph.all_features().collect();
let reachable_from_experimental: HashSet<_> =
graph.all_reachable_from("experimental").collect();
let nonadditive: HashSet<_> = graph.edges_to("__is_nonadditive").collect();
let reachable_from_full: HashSet<_> = graph.all_reachable_from("full").collect();
// Enforce rule 1. (There is a "Full" feature.)
if !graph.contains_feature("full") {
w("full feature does not exist. Adding.".to_string());
changes.push(Change::AddFeature("full".to_string()));
}
// Enforce rule 2. (for every arti crate that we depend on, our 'full' should include that crate's full.
for dep in dependencies.iter() {
let wanted = if dep.optional {
format!("{}?/full", dep.name)
} else {
format!("{}/full", dep.name)
};
if !graph.contains_edge("full", wanted.as_str()) {
w(format!("full should contain {wanted}. Fixing."));
changes.push(Change::AddExternalEdge("full".to_string(), wanted));
}
}
// Enforce rule 3 (relationship between "experimental" and
// "__is_experimental")
let defined_experimental: HashSet<_> = {
let in_experimental: HashSet<_> = graph.edges_from("experimental").collect();
let is_experimental: HashSet<_> = graph.edges_to("__is_experimental").collect();
// Every feature listed in `experimental` depends on `__is_experimental`.
for f in in_experimental.difference(&is_experimental) {
if all_features.contains(f) {
w(format!("{f} should depend on __is_experimental. Fixing."));
changes.push(Change::AddEdge(f.clone(), "__is_experimental".into()));
}
}
// Every feature that depends on `__is_experimental` is reachable from `experimental`.
for f in is_experimental.difference(&reachable_from_experimental) {
w(format!(
"{f} is marked as __is_experimental, but is not reachable from experimental. Fixing."
));
changes.push(Change::AddEdge("experimental".into(), f.clone()))
}
&in_experimental | &is_experimental
};
// Enforce rule 4: Every non-meta feature is reachable from full, or
// from experimental, or is nonadditive.
{
let complaint: &str = "# XX\x58X Mark as full, experimental, or non-additive!\n";
let all_features: HashSet<_> = graph.all_features().collect();
let meta: HashSet<_> = [
"__is_nonadditive",
"__is_experimental",
"full",
"default",
"experimental",
]
.into_iter()
.map(String::from)
.collect();
let mut not_found = all_features;
for set in [
&reachable_from_full,
&meta,
&reachable_from_experimental,
&nonadditive,
] {
not_found = ¬_found - set;
}
for f in not_found {
w(format!(
"{f} is not experimental, reachable from full, or nonadditive."
));
changes.push(Change::Annotate(f.clone(), complaint.to_string()));
}
}
// 5. Every feature reachable from `default` is reachable from `full`.
{
let complaint = "# XX\x58X This is reachable from 'default', but from 'full'.\n";
let default: HashSet<_> = graph.edges_from("default").collect();
for f in default.difference(&reachable_from_full) {
if all_features.contains(f) {
w(format!("{f} is reachable from default, but not from full."));
changes.push(Change::Annotate(f.clone(), complaint.to_string()));
}
}
}
// 6. No non-additive feature is reachable from `full` or
// `experimental`.
{
let complaint = "# XX\x58X This is non-additive, but reachable from 'full'.\n";
for f in nonadditive.intersection(&reachable_from_full) {
w(format!("nonadditive feature {f} is reachable from full."));
changes.push(Change::Annotate(f.clone(), complaint.to_string()));
}
let complaint = "# XX\x58X This is non-additive, but reachable from 'experimental'.\n";
for f in nonadditive.intersection(&reachable_from_experimental) {
w(format!(
"nonadditive feature {f} is reachable from experimental."
));
changes.push(Change::Annotate(f.clone(), complaint.to_string()));
}
}
// 7. No experimental is reachable from `full`.
{
let complaint = "# XX\x58X This is experimental, but reachable from 'full'.\n";
for f in reachable_from_full.intersection(&defined_experimental) {
w(format!("experimental feature {f} is reachable from full!"));
changes.push(Change::Annotate(f.clone(), complaint.to_string()));
}
}
// 8. Every dependency is a real version.
for dep in &dependencies {
match dep.version.as_deref() {
Some("*") => w(format!(
"Dependency for {:?} is given as version='*'",
&dep.name
)),
None => w(format!(
"No version found for dependency on {:?}",
&dep.name
)),
_ => {}
}
}
// Enforce rule 9. (every arti crate we depend on is published if
// we are published.)
if my_info.published {
println!("in {}", self.name);
for dep in &dependencies {
match other_crates.get(&dep.name) {
None => w(format!(
"Dependency on crate {:?}, which I could not find.",
&dep.name
)),
Some(info) if !info.published => w(format!(
"Dependency on crate {:?}, which has `publish = false`",
&dep.name
)),
_ => {}
}
}
}
if no_annotate {
changes.drop_annotations();
}
// We have to look this up again, or else it isn't &mut.
let features = self
.toml_doc
.get_mut("features")
.ok_or_else(|| anyhow!("I thought we added 'features' earlier!"))?
.as_table_mut()
.ok_or_else(|| anyhow!("Features was not table"))?;
changes.apply(features)?;
Ok(warnings)
}
/// If we made changes to this crate's cargo.toml, flush it to disk.
fn save_if_changed(&self) -> Result<()> {
let old_text = self.toml_doc_orig.to_string();
let new_text = self.toml_doc.to_string();
if new_text != old_text {
println!("{} changed. Replacing.", self.name);
let tmpname = self.toml_file.with_extension("toml.tmp");
std::fs::write(&tmpname, new_text.as_str())?;
std::fs::rename(&tmpname, &self.toml_file)?;
}
Ok(())
}
}
/// Look at a toplevel Cargo.toml and find all of the paths in workplace.members
fn list_crate_paths(
toplevel: impl AsRef<Path>,
exclusion_prefixes: &[String],
) -> Result<Vec<String>> {
let s = std::fs::read_to_string(toplevel.as_ref())?;
let toml_doc = s.parse::<DocumentMut>()?;
Ok(toml_doc["workspace"]["members"]
.as_array()
.ok_or_else(|| anyhow!("workplace.members is not an array!?"))?
.iter()
.map(|v| {
v.as_str()
.expect("Some member of workplace.members is not a string!?")
.to_owned()
})
.filter(|s| {
// Iterate through all exclusion prefixes and check if there isn't one that `s` starts with.
!exclusion_prefixes
.iter()
.any(|prefix| s.starts_with(prefix))
})
.collect())
}
fn main() -> Result<()> {
let mut pargs = pico_args::Arguments::from_env();
const HELP: &str = "fixup-features [--no-annotate] [--exclude <PREFIX1> --exclude <PREFIX2> ...] <toplevel Cargo.toml>";
if pargs.contains(["-h", "--help"]) {
println!("{HELP}");
return Ok(());
}
let no_annotate = pargs.contains("--no-annotate");
let exclusion_prefixes: Vec<String> = pargs.values_from_str("--exclude").unwrap();
let toplevel_toml_file: PathBuf = pargs.free_from_str()?;
if !pargs.finish().is_empty() {
println!("{HELP}");
return Ok(());
}
let toplevel_dir = toplevel_toml_file
.parent()
.expect("How is your Cargo.toml file `/`?")
.to_path_buf();
let mut crates = Vec::new();
let mut crate_info = HashMap::new();
for p in list_crate_paths(&toplevel_toml_file, &exclusion_prefixes)? {
let mut crate_toml_path = toplevel_dir.clone();
crate_toml_path.push(p);
crate_toml_path.push("Cargo.toml");
let cr =
Crate::load(&crate_toml_path).with_context(|| format!("In {crate_toml_path:?}"))?;
crate_info.insert(cr.name.clone(), cr.info());
crates.push(cr);
}
for cr in crates.iter_mut() {
for w in cr
.fix(no_annotate, &crate_info)
.with_context(|| format!("In {}", cr.name))?
{
println!("{}: {}", cr.name, w.0);
}
cr.save_if_changed()?;
}
Ok(())
}
|