fix: javascript workspace bugs

Cargo.toml

@@ -12,12 +12,12 @@ edition = "2021"
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/axodotdev/cargo-dist"
 homepage = "https://opensource.axo.dev/cargo-dist/"
-version = "0.20.0"
+version = "0.21.0-prerelease.1"
 
 [workspace.dependencies]
 # intra-workspace deps (you need to bump these versions when you cut releases too!
-cargo-dist-schema = { version = "=0.20.0", path = "cargo-dist-schema" }
-axoproject = { version = "=0.20.0", path = "axoproject", default-features = false, features = ["cargo-projects", "generic-projects", "npm-projects"] }
+cargo-dist-schema = { version = "=0.21.0-prerelease.1", path = "cargo-dist-schema" }
+axoproject = { version = "=0.21.0-prerelease.1", path = "axoproject", default-features = false, features = ["cargo-projects", "generic-projects", "npm-projects"] }
 
 # first-party deps
 axocli = { version = "0.2.0" }

axoproject/src/errors.rs

@@ -60,7 +60,7 @@ pub enum AxoprojectError {
 
     /// We found a package.json but it didn't have "name" set
     #[cfg(feature = "npm-projects")]
-    #[error("your package doesn't have a name: {manifest}")]
+    #[error("your package doesn't have a name:\n{manifest}")]
     #[diagnostic(help("is it a workspace? We don't support that yet."))]
     NamelessNpmPackage {
         /// path to the package.json
@@ -69,7 +69,7 @@ pub enum AxoprojectError {
 
     /// We tried to get the bins from a package.json but something went wrong
     #[cfg(feature = "npm-projects")]
-    #[error("Failed to read the binaries from your package.json: {manifest_path}")]
+    #[error("Failed to read the binaries from your package.json:\n{manifest_path}")]
     BuildInfoParse {
         /// Path to the package.json
         manifest_path: Utf8PathBuf,
@@ -79,7 +79,7 @@ pub enum AxoprojectError {
     },
 
     /// Your workspace gave several different values for "repository"
-    #[error("your workspace has inconsistent values for 'repository', refusing to select one:\n  {file1}: {url1}\n  {file2}: {url2}")]
+    #[error("your workspace has inconsistent values for 'repository', refusing to select one:\n  {file1}:\n    {url1}\n  {file2}:\n    {url2}")]
     #[diagnostic(severity("warning"))]
     InconsistentRepositoryKey {
         /// Path to the first manifest
@@ -93,7 +93,7 @@ pub enum AxoprojectError {
     },
 
     /// An error that occured while trying to find READMEs and whatnot in your project dir
-    #[error("couldn't search for files in {dir}")]
+    #[error("couldn't search for files in\n{dir}")]
     AutoIncludeSearch {
         /// path to the dir we were searching
         dir: Utf8PathBuf,

axoproject/src/generic.rs

@@ -195,7 +195,7 @@ fn workspace_from(manifest_path: &Utf8Path) -> Result<WorkspaceStructure> {
             let paired_manifest = package.package_root.join(DIST_PACKAGE_TOML);
             if paired_manifest.exists() {
                 let generic = raw_package_from(&paired_manifest)?;
-                merge_package_with_raw_generic(package, generic);
+                merge_package_with_raw_generic(package, generic, paired_manifest);
             }
 
             crate::merge_auto_includes(package, &root_auto_includes);
@@ -210,6 +210,7 @@ fn workspace_from(manifest_path: &Utf8Path) -> Result<WorkspaceStructure> {
             target_dir: workspace_dir.join(DIST_TARGET_DIR),
             workspace_dir,
             manifest_path: manifest_path.to_owned(),
+            dist_manifest_path: Some(manifest_path.to_owned()),
             root_auto_includes,
             #[cfg(feature = "cargo-projects")]
             cargo_metadata_table: None,
@@ -229,6 +230,7 @@ fn single_package_workspace_from(manifest_path: &Utf8Path) -> Result<WorkspaceSt
             target_dir: package.package_root.join(DIST_TARGET_DIR),
             workspace_dir: package.package_root.clone(),
             manifest_path: package.manifest_path.clone(),
+            dist_manifest_path: Some(package.manifest_path.clone()),
             root_auto_includes,
 
             #[cfg(feature = "cargo-projects")]
@@ -276,6 +278,7 @@ fn package_from(manifest_path: &Utf8Path) -> Result<PackageInfo> {
         true_name: name.clone(),
         true_version: version.clone(),
         manifest_path: manifest_path.clone(),
+        dist_manifest_path: Some(manifest_path.clone()),
         package_root: manifest_path.parent().unwrap().to_owned(),
         name,
         version,
@@ -298,6 +301,7 @@ fn package_from(manifest_path: &Utf8Path) -> Result<PackageInfo> {
         cargo_metadata_table: None,
         #[cfg(feature = "cargo-projects")]
         cargo_package_id: None,
+        npm_scope: None,
     };
 
     // Load and apply auto-includes
@@ -321,7 +325,11 @@ fn load_package_dist_toml(manifest_path: &Utf8Path) -> Result<PackageManifest> {
     Ok(manifest)
 }
 
-fn merge_package_with_raw_generic(package: &mut PackageInfo, generic: Package) {
+fn merge_package_with_raw_generic(
+    package: &mut PackageInfo,
+    generic: Package,
+    generic_manifest_path: Utf8PathBuf,
+) {
     let Package {
         name,
         repository,
@@ -339,6 +347,9 @@ fn merge_package_with_raw_generic(package: &mut PackageInfo, generic: Package) {
         build_command,
         version,
     } = generic;
+
+    package.dist_manifest_path = Some(generic_manifest_path);
+
     if let Some(val) = name {
         package.name = val;
     }

axoproject/src/javascript.rs

@@ -53,11 +53,18 @@ fn read_workspace(manifest_path: &Utf8Path) -> Result<WorkspaceStructure> {
     let root_auto_includes = crate::find_auto_includes(&root)?;
 
     // Not having a name is common for virtual manifests, but we don't handle those!
-    let Some(package_name) = manifest.name else {
+    let Some(true_package_name) = manifest.name else {
         return Err(crate::errors::AxoprojectError::NamelessNpmPackage {
             manifest: manifest_path.to_owned(),
         });
     };
+
+    let (package_scope, package_name) =
+        if let Some((scope, name)) = true_package_name.split_once('/') {
+            (Some(scope.to_owned()), name.to_owned())
+        } else {
+            (None, true_package_name.clone())
+        };
     let version = manifest.version.map(Version::Npm);
     let authors = manifest
         .author
@@ -118,11 +125,13 @@ fn read_workspace(manifest_path: &Utf8Path) -> Result<WorkspaceStructure> {
     };
 
     let mut info = PackageInfo {
-        true_name: package_name.clone(),
+        true_name: true_package_name,
         true_version: version.clone(),
         name: package_name,
+        npm_scope: package_scope,
         version,
         manifest_path: manifest_path.to_owned(),
+        dist_manifest_path: None,
         package_root: root.clone(),
         description: manifest.description,
         authors,
@@ -164,6 +173,7 @@ fn read_workspace(manifest_path: &Utf8Path) -> Result<WorkspaceStructure> {
             workspace_dir: root,
 
             manifest_path: manifest_path.to_owned(),
+            dist_manifest_path: None,
             root_auto_includes,
             #[cfg(feature = "cargo-projects")]
             cargo_metadata_table: None,

axoproject/src/lib.rs

@@ -355,10 +355,16 @@ pub struct WorkspaceInfo {
     pub workspace_dir: Utf8PathBuf,
     /// Path to the root manifest of the workspace
     ///
-    /// This can be either a Cargo.toml or package.json. In either case this manifest
-    /// may or may not represent a "real" package. Both systems have some notion of
-    /// "virtual" manifest which exists only to list the actual packages in the workspace.
+    /// This can be either a Cargo.toml, package.json, dist.toml, or dist-workspace.toml.
+    ///
+    /// In most cases this manifest may or may not represent a "real" package. Many systems
+    /// have some notion of "virtual" manifest which exists only to list the actual packages
+    /// in the workspace.
     pub manifest_path: Utf8PathBuf,
+    /// Path to the dist.toml or dist-workspace.toml for the workspace
+    ///
+    /// If this is ever set, then this will be the same as manifest_path.
+    pub dist_manifest_path: Option<Utf8PathBuf>,
     /// If the workspace root has some auto-includeable files, here they are!
     ///
     /// This is currently what is use for top-level Announcement contents.
@@ -406,6 +412,8 @@ impl RepositoryUrl {
                 if let Some(cur_url) = &repo_url {
                     if &normalized_new_url == cur_url {
                         // great! consensus!
+                    } else if cur_url.github_repo().ok() == normalized_new_url.github_repo().ok() {
+                        // good enough! consensus on the github repo!
                     } else {
                         return Err(AxoprojectError::InconsistentRepositoryKey {
                             file1: repo_url_origin.as_ref().unwrap().to_owned(),
@@ -443,8 +451,13 @@ pub struct PackageInfo {
     ///
     /// This may be needed when e.g. talking to cargo about the package.
     pub true_version: Option<Version>,
-    /// Path to the manifest for this package
+    /// Path to the primary manifest for this package
     pub manifest_path: Utf8PathBuf,
+    /// Path to a dist.toml overriding this package's contents
+    ///
+    /// NOTE: this WILL be the same path as the manifest_path for a pure dist.toml
+    /// package. Don't assume these are disjoint!
+    pub dist_manifest_path: Option<Utf8PathBuf>,
     /// Path to the root dir for this package
     pub package_root: Utf8PathBuf,
     /// Name of the package
@@ -547,6 +560,8 @@ pub struct PackageInfo {
     /// A unique id used by Cargo to refer to the package
     #[cfg(feature = "cargo-projects")]
     pub cargo_package_id: Option<PackageId>,
+    /// npm scope (with the @, like "@axodotdev")
+    pub npm_scope: Option<String>,
     /// Command to run to build this package
     pub build_command: Option<Vec<String>>,
 }

axoproject/src/repo.rs

@@ -11,7 +11,7 @@ pub enum GithubRepoInput {
 }
 
 /// Represents a GitHub repository that we can query things about.
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub struct GithubRepo {
     /// The repository owner.
     pub owner: String,

axoproject/src/rust.rs

@@ -95,6 +95,7 @@ fn workspace_info(pkg_graph: &PackageGraph) -> Result<WorkspaceStructure> {
             workspace_dir,
 
             manifest_path,
+            dist_manifest_path: None,
             root_auto_includes,
             cargo_metadata_table,
             cargo_profiles,
@@ -203,6 +204,7 @@ fn package_info(_workspace_root: &Utf8Path, package: &PackageMetadata) -> Result
         name: package.name().to_owned(),
         version,
         manifest_path,
+        dist_manifest_path: None,
         package_root: package_root.clone(),
         description: package.description().map(ToOwned::to_owned),
         authors: package.authors().to_vec(),
@@ -224,6 +226,7 @@ fn package_info(_workspace_root: &Utf8Path, package: &PackageMetadata) -> Result
         cstaticlibs,
         cargo_metadata_table,
         cargo_package_id,
+        npm_scope: None,
         build_command: None,
     };
 

cargo-dist/src/config.rs

@@ -4,7 +4,6 @@ use std::collections::BTreeMap;
 
 use axoasset::{toml_edit, SourceFile};
 use axoproject::local_repo::LocalRepo;
-use axoproject::WorkspaceKind;
 use camino::{Utf8Path, Utf8PathBuf};
 use semver::Version;
 use serde::{Deserialize, Serialize};
@@ -21,7 +20,7 @@ use crate::{
 #[derive(Debug, Deserialize)]
 struct GenericConfig {
     /// The dist field within dist.toml
-    dist: DistMetadata,
+    dist: Option<DistMetadata>,
 }
 
 /// Contents of METADATA_DIST in Cargo.toml files
@@ -1668,25 +1667,40 @@ impl std::fmt::Display for ProductionMode {
 }
 
 pub(crate) fn parse_metadata_table_or_manifest(
-    workspace_type: WorkspaceKind,
     manifest_path: &Utf8Path,
+    dist_manifest_path: Option<&Utf8Path>,
     metadata_table: Option<&serde_json::Value>,
 ) -> DistResult<DistMetadata> {
-    match workspace_type {
-        WorkspaceKind::Javascript => unimplemented!("npm packages not yet supported here"),
-        // Pre-parsed Rust metadata table
-        WorkspaceKind::Rust => parse_metadata_table(manifest_path, metadata_table),
+    if let Some(dist_manifest_path) = dist_manifest_path {
+        reject_metadata_table(manifest_path, dist_manifest_path, metadata_table)?;
         // Generic dist.toml
-        WorkspaceKind::Generic => {
-            let src = SourceFile::load_local(manifest_path)?;
-            parse_generic_config(src)
-        }
+        let src = SourceFile::load_local(dist_manifest_path)?;
+        parse_generic_config(src)
+    } else {
+        // Pre-parsed Rust metadata table
+        parse_metadata_table(manifest_path, metadata_table)
     }
 }
 
 pub(crate) fn parse_generic_config(src: SourceFile) -> DistResult<DistMetadata> {
     let config: GenericConfig = src.deserialize_toml()?;
-    Ok(config.dist)
+    Ok(config.dist.unwrap_or_default())
+}
+
+pub(crate) fn reject_metadata_table(
+    manifest_path: &Utf8Path,
+    dist_manifest_path: &Utf8Path,
+    metadata_table: Option<&serde_json::Value>,
+) -> DistResult<()> {
+    let has_dist_metadata = metadata_table.and_then(|t| t.get(METADATA_DIST)).is_some();
+    if has_dist_metadata {
+        Err(DistError::UnusedMetadata {
+            manifest_path: manifest_path.to_owned(),
+            dist_manifest_path: dist_manifest_path.to_owned(),
+        })
+    } else {
+        Ok(())
+    }
 }
 
 pub(crate) fn parse_metadata_table(

cargo-dist/src/errors.rs

@@ -105,7 +105,7 @@ pub enum DistError {
     },
 
     /// Error parsing metadata in Cargo.toml (json because it's from cargo-metadata)
-    #[error("Malformed metadata.dist in {manifest_path}")]
+    #[error("Malformed metadata.dist in\n{manifest_path}")]
     #[diagnostic(help("you can find a reference for the configuration schema at https://opensource.axo.dev/cargo-dist/book/reference/config.html"))]
     CargoTomlParse {
         /// path to file
@@ -320,7 +320,7 @@ pub enum DistError {
     GitArchiveError {},
 
     /// An error running `git -C path rev-parse HEAD`
-    #[error("We failed to query information about the git submodule at {path}")]
+    #[error("We failed to query information about the git submodule at\n{path}")]
     #[diagnostic(help("Does a submodule exist at that path? Has it been fetched with `git submodule update --init`?"))]
     GitSubmoduleCommitError {
         /// The path we failed to fetch
@@ -465,6 +465,18 @@ pub enum DistError {
         /// Error messsage details
         message: String,
     },
+
+    /// Something has metadata.dist but shouldn't
+    #[error("The metadata.dist entry in this Cargo.toml isn't being used:\n{manifest_path}")]
+    #[diagnostic(help(
+        "You probably want to move them to the [dist] section in\n{dist_manifest_path}"
+    ))]
+    UnusedMetadata {
+        /// The manifest that had the metadata.dist
+        manifest_path: Utf8PathBuf,
+        /// The dist.toml/dist-workspace.toml that means it's ignored
+        dist_manifest_path: Utf8PathBuf,
+    },
 }
 
 /// This error indicates we tried to deserialize some YAML with serde_yml

cargo-dist/src/init.rs

@@ -387,8 +387,8 @@ fn has_metadata_table(workspace_info: &WorkspaceInfo) -> bool {
             .unwrap_or(false)
     } else {
         config::parse_metadata_table_or_manifest(
-            workspace_info.kind,
             &workspace_info.manifest_path,
+            workspace_info.dist_manifest_path.as_deref(),
             workspace_info.cargo_metadata_table.as_ref(),
         )
         .is_ok()
@@ -410,8 +410,8 @@ fn get_new_dist_metadata(
 
     let mut meta = if has_config {
         config::parse_metadata_table_or_manifest(
-            root_workspace.kind,
             &root_workspace.manifest_path,
+            root_workspace.dist_manifest_path.as_deref(),
             root_workspace.cargo_metadata_table.as_ref(),
         )?
     } else {