Fix go tags (#544)

Fixes #502

---------

Co-authored-by: Dylan Anthony <[email protected]>

.changeset/use_the_correct_tags_for_gomod_files_in_subdirectories.md

@@ -0,0 +1,18 @@
+---
+default: patch
+---
+
+#### Use the correct tags for `go.mod` files in subdirectories
+
+PR #544 fixed issue #502 by @BatmanAoD.
+
+Previously, the version for a `go.mod` file was determined by the package tag, named `v{Version}` for single packages or `{PackageName}/v{Version}` for named packages. This worked when the `go.mod` file was in the root of the repository or a directory named `{PackageName}` (respectively), but not when it was in a different directory. Now, the version tag, both for determining the current version and creating a new release, will correctly be determined by the name of the directory the `go.mod` file is in (relative to the working directory). The existing package tagging strategy remains unchanged.
+
+For example, consider this `knope.toml` file:
+
+```toml
+[package]
+versioned_files = ["some_dir/go.mod"]
+```
+
+Previous to this release, creating the version `1.2.3` would only create a tag `v1.2.3`. Now, it will _additionally_ create the tag `some_dir/v1.2.3`.

src/fs.rs

@@ -0,0 +1,42 @@
+//! Proxies to FS utils that _either_ actually write to files or print to stdout (for dry runs).
+
+use std::{
+    fmt::Display,
+    io,
+    io::Write,
+    path::{Path, PathBuf},
+};
+
+use log::trace;
+use miette::Diagnostic;
+use thiserror::Error;
+
+/// Writes to a file if this is not a dry run, or prints just the diff to stdout if it is.
+pub(crate) fn write<C: AsRef<[u8]> + Display>(
+    dry_run: &mut Option<Box<dyn Write>>,
+    diff: &str,
+    path: &Path,
+    contents: C,
+) -> Result<(), Error> {
+    if let Some(stdout) = dry_run {
+        writeln!(stdout, "Would write {} to {}", diff, path.display()).map_err(Error::Stdout)
+    } else {
+        trace!("Writing {} to {}", contents, path.display());
+        std::fs::write(path, contents).map_err(|source| Error::File {
+            path: path.into(),
+            source,
+        })
+    }
+}
+
+#[derive(Debug, Diagnostic, Error)]
+pub(crate) enum Error {
+    #[error("Error writing to {path}: {source}")]
+    File {
+        path: PathBuf,
+        #[source]
+        source: io::Error,
+    },
+    #[error("Error writing to stdout: {0}")]
+    Stdout(#[from] io::Error),
+}

src/lib.rs

@@ -38,6 +38,7 @@ use crate::{
 mod app_config;
 mod command;
 mod config;
+mod fs;
 mod git;
 mod issues;
 mod prompt;

src/releases/cargo.rs

@@ -1,25 +1,59 @@
+use std::path::{Path, PathBuf};
+
+use miette::Diagnostic;
 use serde::Deserialize;
+use thiserror::Error;
 use toml::Spanned;
 
-pub(crate) fn get_version(content: &str) -> Result<String, toml::de::Error> {
-    toml::from_str::<Cargo>(content).map(|cargo| cargo.package.version.into_inner())
+use crate::fs;
+
+pub(crate) fn get_version(content: &str, path: &Path) -> Result<String, Error> {
+    toml::from_str::<Cargo>(content)
+        .map(|cargo| cargo.package.version.into_inner())
+        .map_err(|source| Error::Deserialize {
+            path: path.into(),
+            source,
+        })
 }
 
 pub(crate) fn set_version(
+    dry_run: &mut Option<Box<dyn std::io::Write>>,
     mut cargo_toml: String,
     new_version: &str,
-) -> Result<String, toml::de::Error> {
-    let doc: Cargo = toml::from_str(&cargo_toml)?;
+    path: &Path,
+) -> Result<String, Error> {
+    let doc: Cargo = toml::from_str(&cargo_toml).map_err(|source| Error::Deserialize {
+        path: path.into(),
+        source,
+    })?;
 
     // Account for quotes with +- 1
     let start = doc.package.version.span().start + 1;
     let end = doc.package.version.span().end - 1;
 
     cargo_toml.replace_range(start..end, new_version);
+    fs::write(dry_run, new_version, path, &cargo_toml)?;
 
     Ok(cargo_toml)
 }
 
+#[derive(Debug, Diagnostic, Error)]
+pub(crate) enum Error {
+    #[error("Error deserializing {path}: {source}")]
+    #[diagnostic(
+        code(cargo::deserialize),
+        help("knope expects the Cargo.toml file to have a `package.version` property. Workspace support is coming soon!"),
+        url("https://knope-dev.github.io/knope/config/packages.html#supported-formats-for-versioning")
+    )]
+    Deserialize {
+        path: PathBuf,
+        #[source]
+        source: toml::de::Error,
+    },
+    #[error(transparent)]
+    Fs(#[from] fs::Error),
+}
+
 #[derive(Debug, Deserialize)]
 struct Cargo {
     package: Package,
@@ -32,6 +66,8 @@ struct Package {
 
 #[cfg(test)]
 mod tests {
+    use std::path::Path;
+
     use super::*;
 
     #[test]
@@ -42,7 +78,10 @@ mod tests {
         version = "0.1.0-rc.0"
         "###;
 
-        assert_eq!(get_version(content).unwrap(), "0.1.0-rc.0".to_string());
+        assert_eq!(
+            get_version(content, Path::new("")).unwrap(),
+            "0.1.0-rc.0".to_string()
+        );
     }
 
     #[test]
@@ -53,7 +92,14 @@ mod tests {
         version = "0.1.0-rc.0"
         "###;
 
-        let new = set_version(String::from(content), "1.2.3-rc.4").unwrap();
+        let stdout = Box::<Vec<u8>>::default();
+        let new = set_version(
+            &mut Some(stdout),
+            String::from(content),
+            "1.2.3-rc.4",
+            Path::new(""),
+        )
+        .unwrap();
 
         let expected = content.replace("0.1.0-rc.0", "1.2.3-rc.4");
         assert_eq!(new, expected);

src/releases/git.rs

@@ -17,30 +17,34 @@ pub(crate) fn tag_name(version: &Version, package_name: &Option<PackageName>) ->
 }
 
 pub(crate) fn release(
-    dry_run_stdout: Option<&mut Box<dyn Write>>,
+    dry_run_stdout: &mut Option<Box<dyn Write>>,
     version: &Version,
     package_name: &Option<PackageName>,
 ) -> Result<(), StepError> {
     let tag = tag_name(version, package_name);
 
-    if let Some(stdout) = dry_run_stdout {
-        writeln!(stdout, "Would create Git tag {tag}")?;
-        return Ok(());
-    }
+    create_tag(dry_run_stdout, tag)?;
+
+    Ok(())
+}
 
-    let repo = open(current_dir()?).map_err(|_e| StepError::NotAGitRepo)?;
+pub(crate) fn create_tag(dry_run: &mut Option<Box<dyn Write>>, name: String) -> Result<(), Error> {
+    if let Some(stdout) = dry_run {
+        return writeln!(stdout, "Would create Git tag {name}").map_err(Error::Stdout);
+    }
+    let repo = open(current_dir().map_err(Error::CurrentDirectory)?)
+        .map_err(|err| Error::OpenGitRepo(Box::new(err)))?;
     let head = repo.head_commit()?;
     repo.tag(
-        tag,
+        name,
         head.id,
         Kind::Commit,
         repo.committer()
             .transpose()
-            .map_err(|_| StepError::NoCommitter)?,
+            .map_err(|_| Error::NoCommitter)?,
         "",
         PreviousValue::Any,
     )?;
-
     Ok(())
 }
 
@@ -90,4 +94,23 @@ pub(crate) enum Error {
     GitReferences(#[from] gix::reference::iter::Error),
     #[error("Could not get Git tags: {0}")]
     Tags(#[from] gix::reference::iter::init::Error),
+    #[error("Could not find head commit: {0}")]
+    HeadCommit(#[from] gix::reference::head_commit::Error),
+    #[error("Could not determine Git committer to commit changes")]
+    #[diagnostic(
+        code(git::no_committer),
+        help(
+            "We couldn't determine who to commit the changes as. Please set the `user.name` and \
+                `user.email` Git config options."
+        )
+    )]
+    NoCommitter,
+    #[error("Could not create a tag: {0}")]
+    #[diagnostic(
+        code(git::tag_failed),
+        help("A Git tag could not be created for the release.")
+    )]
+    CreateTagError(#[from] gix::tag::Error),
+    #[error("Failed to write to stdout")]
+    Stdout(#[source] std::io::Error),
 }

src/releases/go.rs

@@ -1,7 +1,12 @@
+use std::path::Path;
+
 use miette::Diagnostic;
 use thiserror::Error;
 
-use crate::releases::semver::Version;
+use crate::{
+    fs,
+    releases::{git, semver::Version},
+};
 
 #[derive(Debug, Diagnostic, Error)]
 pub(crate) enum Error {
@@ -17,16 +22,34 @@ pub(crate) enum Error {
         help("The go.mod file contains an invalid module line.")
     )]
     MalformedModuleLine(String),
+    #[error(transparent)]
+    Git(#[from] git::Error),
+    #[error(transparent)]
+    Fs(#[from] fs::Error),
 }
 
-pub(crate) fn set_version(go_mod: String, new_version: &Version) -> Result<String, Error> {
+pub(crate) fn set_version(
+    dry_run: &mut Option<Box<dyn std::io::Write>>,
+    content: String,
+    new_version: &Version,
+    path: &Path,
+) -> Result<String, Error> {
+    let parent_dir = path.parent().map(Path::to_string_lossy);
+    if let Some(parent_dir) = parent_dir {
+        if !parent_dir.is_empty() {
+            let tag = format!("{parent_dir}/v{new_version}");
+            git::create_tag(dry_run, tag)?;
+        }
+        // If there's not a nested dir, the tag will equal the release tag, so creating it here would cause a conflict later.
+    }
+
     let new_major = new_version.stable_component().major;
     if new_major == 0 || new_major == 1 {
         // These major versions aren't recorded in go.mod
-        return Ok(go_mod);
+        return Ok(content);
     }
 
-    let module_line = go_mod
+    let module_line = content
         .lines()
         .find(|line| line.starts_with("module "))
         .ok_or(Error::MissingModuleLine)?;
@@ -46,17 +69,23 @@ pub(crate) fn set_version(go_mod: String, new_version: &Version) -> Result<Strin
     } else {
         None
     };
-    if let Some(existing_version) = existing_version {
+    let new_contents = if let Some(existing_version) = existing_version {
         if existing_version == new_version.stable_component().major {
-            // Major version has not changed—keep existing content
-            return Ok(go_mod);
+            None
+        } else {
+            let new_version_string = format!("v{new_major}");
+            let new_module_line = format!("module {uri}/{new_version_string}");
+            Some(content.replace(module_line, &new_module_line))
         }
-        let new_version_string = format!("v{new_major}");
-        let new_module_line = format!("module {uri}/{new_version_string}");
-        Ok(go_mod.replace(module_line, &new_module_line))
     } else {
         // No existing version found—add new line
         let new_module_line = format!("module {module}/v{new_major}");
-        Ok(go_mod.replace(module_line, &new_module_line))
+        Some(content.replace(module_line, &new_module_line))
+    };
+    if let Some(new_contents) = new_contents {
+        fs::write(dry_run, &new_version.to_string(), path, &new_contents)?;
+        Ok(new_contents)
+    } else {
+        Ok(content)
     }
 }

src/releases/mod.rs

@@ -252,7 +252,7 @@ pub(crate) fn release(run_type: RunType) -> Result<RunType, StepError> {
                     dry_run_stdout.as_mut(),
                 )?;
             } else {
-                git::release(dry_run_stdout.as_mut(), &release.new_version, &package.name)?;
+                git::release(&mut dry_run_stdout, &release.new_version, &package.name)?;
             }
         }
     }