Skip to content

Commit

Permalink
move away from directory listings for finding generated crates (#1509)
Browse files Browse the repository at this point in the history
As identified in #1479, we do not currently handle deleted/renamed specifications well.

This update addresses this via the following:
* Moves to using parsing `cargo_toml` to parse services/Cargo.toml to know what crates already exist
* Replaces all uses of `list_crate_names` with using the results of `gen_crates`

As a byproduct, this identifies that the previously existing spec that would result in `azure_svc_codesigning` was removed.

ref: Azure/azure-rest-api-specs#26635
  • Loading branch information
demoray committed Dec 8, 2023
1 parent e04f5c3 commit 7273f11
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 66 deletions.
16 changes: 0 additions & 16 deletions .github/workflows/check-all-services.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion services/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,6 @@ members = [
"svc/attestation",
"svc/batch",
"svc/blobstorage",
"svc/codesigning",
"svc/confidentialledger",
"svc/containerregistry",
"svc/datalakeanalytics",
Expand Down
84 changes: 51 additions & 33 deletions services/autorust/azure-autorust/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@
// cargo run --release -p azure-autorust -- -p azure_svc_queuestorage

use autorust_codegen::{
crates::{list_crate_names, list_dirs},
crates::list_crates,
gen, get_mgmt_readmes, get_svc_readmes,
jinja::{CheckAllServicesYml, PublishSdksYml, PublishServicesYml, WorkspaceCargoToml},
Error, ErrorKind, Result, ResultExt, RunConfig, SpecReadme,
};
use clap::Parser;
use rayon::prelude::*;
use std::{collections::BTreeSet, path::PathBuf};

#[derive(Debug, clap::Parser)]
struct Args {
Expand All @@ -35,15 +36,34 @@ impl Args {
fn main() -> Result<()> {
let args = Args::parse();
let packages = &args.packages();
gen_crates(packages)?;
gen_services_workspace(packages)?;

let existing_crates = list_crates(&PathBuf::from("../"))?;
let generated = gen_crates(packages)?;
gen_services_workspace(&generated)?;

if packages.is_empty() {
gen_workflow_check_all_services()?;
gen_workflow_check_all_services(&generated)?;
if args.publish {
gen_workflow_publish_sdks()?;
gen_workflow_publish_services()?;
gen_workflow_publish_services(&generated)?;
}

let removed = existing_crates.difference(&generated).collect::<Vec<_>>();
let added = generated.difference(&existing_crates).collect::<Vec<_>>();
if !removed.is_empty() {
println!("the following crates are no longer generated:");
for name in removed {
println!("- {name}");
}
}
if !added.is_empty() {
println!("the following crates are newly generated:");
for name in added {
println!("- {name}");
}
}
}

Ok(())
}

Expand All @@ -63,7 +83,7 @@ fn gen_crate(only_packages: &[&str], crate_type: &str, spec: &SpecReadme) -> Res
}
}

fn gen_crates(only_packages: &[&str]) -> Result<()> {
fn gen_crates(only_packages: &[&str]) -> Result<BTreeSet<String>> {
let svc = get_svc_readmes()?.into_iter().map(|x| ("svc".to_string(), x));
let mgmt = get_mgmt_readmes()?.into_iter().map(|x| ("mgmt".to_string(), x));
let crate_iters = svc.chain(mgmt).collect::<Vec<_>>();
Expand All @@ -77,12 +97,17 @@ fn gen_crates(only_packages: &[&str]) -> Result<()> {

let mut errors = vec![];
let mut completed = vec![];
let mut skipped = vec![];

for result in results {
match result {
Ok(result) => {
if let Some(result) = result {
completed.push(result);
if let Some((name, tags)) = result {
if tags.is_empty() {
skipped.push(name);
} else {
completed.push((name, tags));
}
}
}
Err(err) => {
Expand All @@ -98,40 +123,34 @@ fn gen_crates(only_packages: &[&str]) -> Result<()> {
return Err(Error::new(ErrorKind::CodeGen, "Failed to generate some crates"));
}

for (package_name, tags) in completed {
for (package_name, tags) in &completed {
println!("{package_name}");
if tags.is_empty() {
println!(" No tags");
} else {
for tag in tags {
println!("- {tag}");
}
for tag in tags {
println!("- {tag}");
}
}

Ok(())
}
if !skipped.is_empty() {
println!("the following crates were not generated due to configuration:");
for name in &skipped {
println!("- {name}");
}
}

fn gen_services_workspace(only_packages: &[&str]) -> Result<()> {
let dirs: Vec<String> = if only_packages.is_empty() {
list_dirs()?
.iter()
.map(|dir| dir.as_str().replace('\\', "/").replace("../", ""))
.collect()
} else {
only_packages
.iter()
.map(|p| p.replace("azure_mgmt_", "mgmt/").replace("azure_svc_", "svc/"))
.collect()
};
Ok(completed.into_iter().map(|(package_name, _)| package_name).collect())
}

fn gen_services_workspace(only_packages: &BTreeSet<String>) -> Result<()> {
let dirs = only_packages
.iter()
.map(|p| p.replace("azure_mgmt_", "mgmt/").replace("azure_svc_", "svc/"))
.collect();
let toml = WorkspaceCargoToml { dirs };
toml.create("../Cargo.toml")?;
Ok(())
}

fn gen_workflow_check_all_services() -> Result<()> {
let packages = list_crate_names()?;
fn gen_workflow_check_all_services(packages: &BTreeSet<String>) -> Result<()> {
let packages = &packages.iter().map(String::as_str).collect();

let yml = CheckAllServicesYml { packages };
Expand Down Expand Up @@ -159,8 +178,7 @@ fn gen_workflow_publish_sdks() -> Result<()> {
Ok(())
}

fn gen_workflow_publish_services() -> Result<()> {
let packages = list_crate_names()?;
fn gen_workflow_publish_services(packages: &BTreeSet<String>) -> Result<()> {
let packages = &packages.iter().map(String::as_str).collect();
let yml = PublishServicesYml { packages };
yml.create("../../.github/workflows/publish-services.yml")?;
Expand Down
1 change: 1 addition & 0 deletions services/autorust/codegen/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ camino = "1.1"
askama = "0.12"
toml = "0.8"
qstring = "0.7"
cargo_toml = "0.17"

[dev-dependencies]
thiserror = "1.0"
Expand Down
37 changes: 21 additions & 16 deletions services/autorust/codegen/src/crates.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
use crate::{ErrorKind, Result, ResultExt};
use camino::{Utf8Path, Utf8PathBuf};
use cargo_toml::Manifest;
use serde::Deserialize;
use std::io::BufRead;
use std::{
collections::BTreeSet,
fs::{self, File},
io::BufReader,
io::{BufRead, BufReader},
path::Path,
str::FromStr,
};

Expand All @@ -22,27 +24,30 @@ fn list_dirs_in(dir: impl AsRef<Utf8Path>) -> Result<Vec<Utf8PathBuf>> {
Ok(dirs)
}

pub fn list_crates(services_dir: &Path) -> Result<BTreeSet<String>> {
let mut package_names = BTreeSet::new();
let base_path = services_dir.join("Cargo.toml");
let manifest = Manifest::from_path(base_path)?;
if let Some(workspaces) = manifest.workspace {
for member in workspaces.members {
let member_path = services_dir.join(member).join("Cargo.toml");
let Ok(manifest) = Manifest::from_path(member_path) else { continue };
let Some(package) = manifest.package else {
continue;
};
package_names.insert(package.name);
}
}
Ok(package_names)
}

pub fn list_dirs() -> Result<Vec<Utf8PathBuf>> {
let mut names: Vec<_> = list_dirs_in("../mgmt")?.into_iter().collect();
names.extend(list_dirs_in("../svc")?);
names.sort();
Ok(names)
}

pub fn list_crate_names() -> Result<Vec<String>> {
let mut names: Vec<_> = list_dirs_in("../mgmt")?
.into_iter()
.filter_map(|d| d.file_name().map(|d| format!("azure_mgmt_{d}")))
.collect();
names.extend(
list_dirs_in("../svc")?
.into_iter()
.filter_map(|d| d.file_name().map(|d| format!("azure_svc_{d}"))),
);
names.sort();
Ok(names)
}

pub fn has_version(name: &str, version: &str) -> Result<bool> {
Ok(get_versions(name)?.iter().any(|v| v.vers.as_str() == version))
}
Expand Down
6 changes: 6 additions & 0 deletions services/autorust/codegen/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,12 @@ impl From<syn::Error> for Error {
}
}

impl From<cargo_toml::Error> for Error {
fn from(error: cargo_toml::Error) -> Self {
Self::new(ErrorKind::Parse, error)
}
}

impl From<autorust_openapi::Error> for Error {
fn from(error: autorust_openapi::Error) -> Self {
Self::new(ErrorKind::Parse, error)
Expand Down

0 comments on commit 7273f11

Please sign in to comment.