From 3f463e4edd4f287b24f6a4a97bb639321709bbe0 Mon Sep 17 00:00:00 2001 From: Cedric Zheng Date: Tue, 19 Dec 2023 14:42:06 -0800 Subject: [PATCH 1/4] Add validation for pants_version format --- tools/src/scie_pants/pants_version.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tools/src/scie_pants/pants_version.py b/tools/src/scie_pants/pants_version.py index be3ec68..6c6390f 100644 --- a/tools/src/scie_pants/pants_version.py +++ b/tools/src/scie_pants/pants_version.py @@ -99,6 +99,11 @@ def determine_find_links( def determine_tag_version( ptex: Ptex, pants_version: str, find_links_dir: Path, github_api_bearer_token: str | None = None ) -> ResolveInfo: + # `pants_version` should be in the format `major.minor.micro`, e.g., `2.17.0`. + # Raise an error if it does not follow this format. + if not pants_version.count(".") == 2: + fatal(f"Expected a pants version like `major.minor.micro`, but got `{pants_version}`.\n\n") + stable_version = Version(pants_version) if stable_version >= PANTS_PEX_GITHUB_RELEASE_VERSION: return ResolveInfo(stable_version, sha_version=None, find_links=None) From d6b72f6af38adf892065fa8ca5e796c9a2edc721 Mon Sep 17 00:00:00 2001 From: Cedric Zheng Date: Wed, 20 Dec 2023 12:30:20 -0800 Subject: [PATCH 2/4] improve error message with patch version suggestion --- tools/src/scie_pants/install_pants.py | 7 +++++++ tools/src/scie_pants/pants_version.py | 5 ----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/tools/src/scie_pants/install_pants.py b/tools/src/scie_pants/install_pants.py index cd4f0ad..d50e21a 100644 --- a/tools/src/scie_pants/install_pants.py +++ b/tools/src/scie_pants/install_pants.py @@ -105,8 +105,15 @@ def install_pants_from_pex( try: ptex.fetch_to_fp(pex_url, pants_pex.file) except subprocess.CalledProcessError as e: + # if there's only one dot in version, specifically suggest adding the `.patch`) + suggestion = ( + "Pants version format not recognized. Please add `.` to the end of the version. For example: `2.18` -> `2.18.0`" + if version.base_version.count(".") < 2 + else "" + ) fatal( f"Wasn't able to fetch the Pants PEX at {pex_url}.\n\n" + f"{suggestion}\n\n" "Check to see if the URL is reachable (i.e. GitHub isn't down) and if" f" {pex_name} asset exists within the release." " If the asset doesn't exist it may be that this platform isn't yet supported." diff --git a/tools/src/scie_pants/pants_version.py b/tools/src/scie_pants/pants_version.py index 6c6390f..be3ec68 100644 --- a/tools/src/scie_pants/pants_version.py +++ b/tools/src/scie_pants/pants_version.py @@ -99,11 +99,6 @@ def determine_find_links( def determine_tag_version( ptex: Ptex, pants_version: str, find_links_dir: Path, github_api_bearer_token: str | None = None ) -> ResolveInfo: - # `pants_version` should be in the format `major.minor.micro`, e.g., `2.17.0`. - # Raise an error if it does not follow this format. - if not pants_version.count(".") == 2: - fatal(f"Expected a pants version like `major.minor.micro`, but got `{pants_version}`.\n\n") - stable_version = Version(pants_version) if stable_version >= PANTS_PEX_GITHUB_RELEASE_VERSION: return ResolveInfo(stable_version, sha_version=None, find_links=None) From b26dbf24db8fb4b19415dbafc871ab248feabb82 Mon Sep 17 00:00:00 2001 From: Cedric Zheng Date: Tue, 26 Dec 2023 18:48:04 -0800 Subject: [PATCH 3/4] add test for bad pex version --- package/src/test.rs | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/package/src/test.rs b/package/src/test.rs index a73ccdb..08c4f51 100644 --- a/package/src/test.rs +++ b/package/src/test.rs @@ -129,6 +129,7 @@ pub(crate) fn run_integration_tests( test_ignore_empty_pants_version_pants_sha(scie_pants_scie); test_pants_from_pex_version(scie_pants_scie); + test_pants_from_bad_pex_version(scie_pants_scie); let clone_root = create_tempdir()?; test_use_in_repo_with_pants_script(scie_pants_scie, &clone_root); @@ -460,6 +461,36 @@ fn test_pants_from_pex_version(scie_pants_scie: &Path) { ); } +fn test_pants_from_bad_pex_version(scie_pants_scie: &Path) { + integration_test!( + "Verify the output of scie-pants is user-friendly if they provide an invalid pants version" + ); + + let tmpdir = create_tempdir().unwrap(); + + let pants_release = "2.18"; + let pants_toml_content = format!( + r#" + [GLOBAL] + pants_version = "{pants_release}" + "# + ); + let pants_toml = tmpdir.path().join("pants.toml"); + write_file(&pants_toml, false, pants_toml_content).unwrap(); + + let err = execute( + Command::new(scie_pants_scie) + .arg("-V") + .current_dir(&tmpdir) + .stderr(Stdio::piped()), + ) + .unwrap_err(); + + let error_text = err.to_string(); + assert!(error_text.contains("Wasn't able to fetch the Pants PEX at")); + assert!(error_text.contains("Pants version format not recognized. Please add `.` to the end of the version. For example: `2.18` -> `2.18.0`")); +} + fn test_use_in_repo_with_pants_script(scie_pants_scie: &Path, clone_root: &TempDir) { integration_test!("Verify scie-pants can be used as `pants` in a repo with the `pants` script"); // This verifies a fix for https://github.com/pantsbuild/scie-pants/issues/28. From 01169d4f32cfc052ae1f03245602f377e92304c5 Mon Sep 17 00:00:00 2001 From: Cedric Zheng Date: Thu, 28 Dec 2023 10:01:25 -0800 Subject: [PATCH 4/4] modify the suggestion in error message --- tools/src/scie_pants/install_pants.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tools/src/scie_pants/install_pants.py b/tools/src/scie_pants/install_pants.py index d50e21a..4846b11 100644 --- a/tools/src/scie_pants/install_pants.py +++ b/tools/src/scie_pants/install_pants.py @@ -107,13 +107,12 @@ def install_pants_from_pex( except subprocess.CalledProcessError as e: # if there's only one dot in version, specifically suggest adding the `.patch`) suggestion = ( - "Pants version format not recognized. Please add `.` to the end of the version. For example: `2.18` -> `2.18.0`" + "Pants version format not recognized. Please add `.` to the end of the version. For example: `2.18` -> `2.18.0`.\n\n" if version.base_version.count(".") < 2 else "" ) fatal( - f"Wasn't able to fetch the Pants PEX at {pex_url}.\n\n" - f"{suggestion}\n\n" + f"Wasn't able to fetch the Pants PEX at {pex_url}.\n\n{suggestion}" "Check to see if the URL is reachable (i.e. GitHub isn't down) and if" f" {pex_name} asset exists within the release." " If the asset doesn't exist it may be that this platform isn't yet supported."