diff --git a/end-to-end-tests/features/from_ambiguous_reference.feature b/end-to-end-tests/features/from_ambiguous_reference.feature new file mode 100644 index 0000000..086ff60 --- /dev/null +++ b/end-to-end-tests/features/from_ambiguous_reference.feature @@ -0,0 +1,85 @@ +Feature: When the provided argument matches more than one thing, the same one Git itself resolves to is used. + + + # The branch is named after the shortened commit hash of the commit before HEAD, but points at HEAD. + # So only when the shortened commit hash shadows the branch is there a commit within the range. + Scenario Outline: The Git reference is used in preference to the shortened Git commit hash. + Given the repository "" is cloned and checked out at the commit "". + Given the branch "" is created pointing at "". + When linting from the "". + Then their is a no commits within the provided range error. + + + Examples: + | repository | checkout_commit | branch | pointing_at | + | https://github.com/haunt98/changeloguru.git | a768f1329b07db76566e0aa3009182a42d2bfe01 | d828a41 | HEAD | + + + Scenario Outline: A warning is emitted as the provided argument is both a Git reference and a shortened Git commit hash. + Given the repository "" is cloned and checked out at the commit "". + Given the branch "" is created pointing at "". + When the argument --verbose is provided. + When linting from the "". + Then their is an ambiguous reference and commit hash warning for "". + + + Examples: + | repository | checkout_commit | branch | pointing_at | + | https://github.com/haunt98/changeloguru.git | a768f1329b07db76566e0aa3009182a42d2bfe01 | d828a41 | HEAD | + + + # Unlike a shortened commit hash, Git uses a full commit hash in preference to a reference of the same name. + # The branch is named after the full commit hash of the commit before HEAD, but points at HEAD. + # So only when the branch shadows the full commit hash is there no commit within the range. + Scenario Outline: The full Git commit hash is used in preference to the Git reference. + Given the repository "" is cloned and checked out at the commit "". + Given the branch "" is created pointing at "". + When linting from the "". + Then the Git history is clean. + + + Examples: + | repository | checkout_commit | branch | pointing_at | + | https://github.com/haunt98/changeloguru.git | a768f1329b07db76566e0aa3009182a42d2bfe01 | d828a41601b950843639535489c57b855c9852dd | HEAD | + + + Scenario Outline: A warning is emitted as the provided argument is both a full Git commit hash and a Git reference. + Given the repository "" is cloned and checked out at the commit "". + Given the branch "" is created pointing at "". + When the argument --verbose is provided. + When linting from the "". + Then their is an ambiguous full commit hash and reference warning for "". + + + Examples: + | repository | checkout_commit | branch | pointing_at | + | https://github.com/haunt98/changeloguru.git | a768f1329b07db76566e0aa3009182a42d2bfe01 | d828a41601b950843639535489c57b855c9852dd | HEAD | + + + # Git matches a tag before a branch, the tag points at the commit before HEAD and the branch points at HEAD. + # So only when the branch is matched before the tag is there no commit within the range. + Scenario Outline: The Git tag is used in preference to the Git branch of the same name. + Given the repository "" is cloned and checked out at the commit "". + Given the tag "" is created pointing at "". + Given the branch "" is created pointing at "". + When linting from the "". + Then the Git history is clean. + + + Examples: + | repository | checkout_commit | name | tag_pointing_at | branch_pointing_at | + | https://github.com/haunt98/changeloguru.git | a768f1329b07db76566e0aa3009182a42d2bfe01 | ambiguous | HEAD~1 | HEAD | + + + Scenario Outline: A warning is emitted as the provided argument matches multiple Git references. + Given the repository "" is cloned and checked out at the commit "". + Given the tag "" is created pointing at "". + Given the branch "" is created pointing at "". + When the argument --verbose is provided. + When linting from the "". + Then their is an ambiguous references warning for "". + + + Examples: + | repository | checkout_commit | name | tag_pointing_at | branch_pointing_at | + | https://github.com/haunt98/changeloguru.git | a768f1329b07db76566e0aa3009182a42d2bfe01 | ambiguous | HEAD~1 | HEAD | diff --git a/end-to-end-tests/features/from_shortened_commit_hash.feature b/end-to-end-tests/features/from_shortened_commit_hash.feature index 0c53a50..eb09c97 100644 --- a/end-to-end-tests/features/from_shortened_commit_hash.feature +++ b/end-to-end-tests/features/from_shortened_commit_hash.feature @@ -48,4 +48,16 @@ Feature: A shortened Git commit hash can be provided as an argument to indicate Examples: | repository | checkout_commit | shortened_commit_hash | - | https://gitlab.com/DSASanFrancisco/membership_api | bf7dacdba6d030250e0ac26805d80be1feb62012 | ff6 | + | https://gitlab.com/DSASanFrancisco/membership_api | bf7dacdba6d030250e0ac26805d80be1feb62012 | 0213 | + + + # Git itself will not match a shortened commit hash of fewer than four characters. + Scenario Outline: The shortened Git commit hash is too short, so an error is returned. + Given the repository "" is cloned and checked out at the commit "". + When linting from the "". + Then their is a too short commit hash "" error. + + + Examples: + | repository | checkout_commit | shortened_commit_hash | + | https://github.com/SergioBenitez/Rocket.git | 549c9241c41320fc5af76b53c2ffc3bd8db88f8c | ecf | diff --git a/end-to-end-tests/features/steps/given.py b/end-to-end-tests/features/steps/given.py index 1090837..2bff3d1 100644 --- a/end-to-end-tests/features/steps/given.py +++ b/end-to-end-tests/features/steps/given.py @@ -48,6 +48,26 @@ def clone_remote_repository_and_checkout_commit(context, remote_repository, comm os.chdir(context.behave_directory) +@given('the branch "{branch}" is created pointing at "{pointing_at}".') +def create_branch(context, branch, pointing_at): + os.chdir(context.remote_repository_cache) + + result = execute_command(f"git branch --force {branch} {pointing_at}") + + os.chdir(context.behave_directory) + assert_command_successful(result) + + +@given('the tag "{tag}" is created pointing at "{pointing_at}".') +def create_tag(context, tag, pointing_at): + os.chdir(context.remote_repository_cache) + + result = execute_command(f"git tag --force {tag} {pointing_at}") + + os.chdir(context.behave_directory) + assert_command_successful(result) + + @given('the GIT_DIR environment variable is set to the cloned repository.') def set_git_dir(context): os.environ["GIT_DIR"] = str(context.remote_repository_cache + "/.git") diff --git a/end-to-end-tests/features/steps/then.py b/end-to-end-tests/features/steps/then.py index 97a29f0..adf3110 100644 --- a/end-to-end-tests/features/steps/then.py +++ b/end-to-end-tests/features/steps/then.py @@ -83,6 +83,66 @@ def assert_ambiguous_shortened_commit_hash_error(context, shortened_commit_hash) assert_error_matches_regex(result, ambiguous_shortened_commit_hash_error) +@then('their is a no commits within the provided range error.') +def assert_no_commits_within_the_provided_range_error(context): + # Given + no_commits_within_the_provided_range_error = "No Git commits within the provided range.\n" # fmt: off + + # When/Then + result = assert_git_history_is_not_clean(context) + + # Then + assert_error_contains(result, no_commits_within_the_provided_range_error) + + +@then('their is a too short commit hash "{shortened_commit_hash}" error.') +def assert_too_short_commit_hash_error(context, shortened_commit_hash): + # Given + too_short_commit_hash_error = f"The provided short commit hash \"{shortened_commit_hash}\" is shorter than the minimum of 4 characters.\n" # fmt: off + + # When/Then + result = assert_git_history_is_not_clean(context) + + # Then + assert_error_contains(result, too_short_commit_hash_error) + + +@then('their is an ambiguous reference and commit hash warning for "{ambiguous}".') +def assert_ambiguous_reference_and_commit_hash_warning(context, ambiguous): + # Given + ambiguous_reference_and_commit_hash_warning = f"The provided \"{ambiguous}\" is ambiguous, it is both a reference pointing at the commit " # fmt: off + + # When + result = execute_clean_git_history(context) + + # Then + assert_error_contains(result, ambiguous_reference_and_commit_hash_warning) + + +@then('their is an ambiguous full commit hash and reference warning for "{ambiguous}".') +def assert_ambiguous_full_commit_hash_and_reference_warning(context, ambiguous): + # Given + ambiguous_full_commit_hash_and_reference_warning = f"The provided \"{ambiguous}\" is ambiguous, it is both a full commit hash and the reference " # fmt: off + + # When + result = execute_clean_git_history(context) + + # Then + assert_error_contains(result, ambiguous_full_commit_hash_and_reference_warning) + + +@then('their is an ambiguous references warning for "{ambiguous}".') +def assert_ambiguous_references_warning(context, ambiguous): + # Given + ambiguous_references_warning = f"The provided \"{ambiguous}\" is ambiguous, it matches the references " # fmt: off + + # When + result = execute_clean_git_history(context) + + # Then + assert_error_contains(result, ambiguous_references_warning) + + @then('their is an invalid max commits value "{max_commits}" error.') def assert_invalid_max_commits_value_error(context, max_commits): # Given diff --git a/end-to-end-tests/features/steps/when.py b/end-to-end-tests/features/steps/when.py index 46e608d..c55e48e 100644 --- a/end-to-end-tests/features/steps/when.py +++ b/end-to-end-tests/features/steps/when.py @@ -6,6 +6,11 @@ def set_linting_from_the(context, git): context.from_ref = f"\"{git}\"" +@when('the argument --verbose is provided.') +def set_verbose(context): + context.arguments += " --verbose " + + @when('the argument --max-commits is provided as "{max_commits}".') def set_max_commits(context, max_commits): context.arguments += f" --max-commits {max_commits} " diff --git a/src/commits/mod.rs b/src/commits/mod.rs index 6a89816..adba369 100644 --- a/src/commits/mod.rs +++ b/src/commits/mod.rs @@ -9,6 +9,12 @@ use crate::linting_results::{CommitErrors, CommitsError, CommitsErrors, LintingR pub mod commit; pub use commit::Commit; +/// The length of a full commit hash. +const FULL_COMMIT_HASH_LENGTH: usize = 40; + +/// The minimum length of a short commit hash Git will match, as per Git's own MINIMUM_ABBREV. +const MINIMUM_SHORT_COMMIT_HASH_LENGTH: usize = 4; + /// A representation of a range of commits within a Git repository, which can have various lints performed upon it after construction. pub struct Commits { commits: VecDeque, @@ -16,9 +22,7 @@ pub struct Commits { impl Commits { pub fn from_git>(repository: &Repository, git: T) -> Result { - let oid = parse_to_oid(repository, git.as_ref()).or_else(|error| { - get_reference_oid(repository, git.as_ref()).map_err(|e| error.context(e)) - })?; + let oid = resolve_to_oid(repository, git.as_ref())?; get_commits_till_head_from_oid(repository, oid) } @@ -94,23 +98,109 @@ fn get_commits_till_head_from_oid( Ok(Commits { commits }) } +/// Resolve to the Oid of a commit, matching how Git itself resolves a name which is both a reference and a commit hash. +fn resolve_to_oid(repository: &Repository, git: &str) -> Result { + // Git resolves a full commit hash to that commit, even when a reference shares its name. + if is_full_commit_hash(git) { + let commit_oid = parse_to_oid(repository, git)?; + + if let Some((reference_name, _)) = get_matching_references(repository, git).first() { + warn!( + "The provided {git:?} is ambiguous, it is both a full commit hash and the reference {reference_name:?}, using the commit hash as Git does." + ); + } + + info!("Using the commit hash '{commit_oid}'."); + return Ok(commit_oid); + } + + match get_reference_oid(repository, git) { + Ok(reference_oid) => { + // Git resolves an ambiguous name to the reference, only warning that the name is also a short commit hash. + if let Ok(commit_oid) = parse_to_oid(repository, git) { + warn!( + "The provided {git:?} is ambiguous, it is both a reference pointing at the commit '{reference_oid}' and the commit hash '{commit_oid}', using the reference as Git does." + ); + } + + info!("Using the reference {git:?}, which points at the commit '{reference_oid}'."); + Ok(reference_oid) + } + Err(reference_error) => { + let commit_oid = + parse_to_oid(repository, git).map_err(|error| error.context(reference_error))?; + info!("Using the commit hash '{commit_oid}'."); + Ok(commit_oid) + } + } +} + fn get_reference_oid(repository: &Repository, matching: &str) -> Result { - let reference = repository - .resolve_reference_from_short_name(matching) - .context(format!( - "Could not find a reference with the name {matching:?}." - ))?; - debug!( - "Matched {matching:?} to the reference {:?}.", - reference.name().unwrap() - ); - let commit = reference.peel_to_commit()?; - Ok(commit.id()) + let matched_references = get_matching_references(repository, matching); + + let (reference_name, reference_oid) = matched_references.first().context(format!( + "Could not find a reference with the name {matching:?}." + ))?; + + // Git resolves a name matching several references to the first one, only warning that the name is ambiguous. + if matched_references.len() > 1 { + let reference_names: Vec<&String> = matched_references + .iter() + .map(|(reference_name, _)| reference_name) + .collect(); + warn!( + "The provided {matching:?} is ambiguous, it matches the references {reference_names:?}, using the reference {reference_name:?} as Git does." + ); + } + + debug!("Matched {matching:?} to the reference {reference_name:?}."); + Ok(*reference_oid) +} + +/// All the references a name matches, in the order Git itself matches them. +fn get_matching_references(repository: &Repository, matching: &str) -> Vec<(String, Oid)> { + [ + matching.to_string(), + format!("refs/{matching}"), + format!("refs/tags/{matching}"), + format!("refs/heads/{matching}"), + format!("refs/remotes/{matching}"), + format!("refs/remotes/{matching}/HEAD"), + ] + .into_iter() + .filter_map(|reference_name| { + let commit = repository + .find_reference(&reference_name) + .ok()? + .peel_to_commit() + .ok()?; + Some((reference_name, commit.id())) + }) + .collect() +} + +fn is_full_commit_hash(oid: &str) -> bool { + oid.len() == FULL_COMMIT_HASH_LENGTH && is_commit_hash_characters(oid) +} + +fn is_commit_hash_characters(oid: &str) -> bool { + !oid.is_empty() && oid.chars().all(|character| character.is_ascii_hexdigit()) } fn parse_to_oid(repository: &Repository, oid: &str) -> Result { + // Avoid searching the history for anything which can not be a commit hash, such as a reference's name. + if !is_commit_hash_characters(oid) { + bail!("{oid:?} is not a valid commit hash."); + } + match oid.len() { - 1..=39 => { + // Git does not match a short commit hash of fewer characters, so neither do we. + ..MINIMUM_SHORT_COMMIT_HASH_LENGTH => { + bail!( + "The provided short commit hash {oid:?} is shorter than the minimum of {MINIMUM_SHORT_COMMIT_HASH_LENGTH} characters." + ); + } + MINIMUM_SHORT_COMMIT_HASH_LENGTH..FULL_COMMIT_HASH_LENGTH => { debug!("Attempting to find a match for the short commit hash {oid:?}."); let matching_oid_lowercase = oid.to_lowercase();