From d0c24d24a633755393daa418c6b74950de75d317 Mon Sep 17 00:00:00 2001 From: Guy Afik <53861351+GuyAfik@users.noreply.github.com> Date: Sun, 10 Sep 2023 16:38:59 +0300 Subject: [PATCH] validate that git is installed before executing any command. (#3568) * validate that git is installed * pre-commit * add ruff rule * update to gitutil - first phase * use only GitUtil phase2 * exit if git is not installed * try to fix uts * add gitutil UT and remove cirucilar import * git content return repo * test * update * try to fix validations * fix content artifcats tests * pre-commit + linter fixes * pre-commit * try to fix uts now * update more uts * fix lint uts * update final tests * changelog * fix last uts * fixes * fix more unit-tests * update * ruff * remove from git_util * update err with click * pre-commit * make sure git is imported at top * ruff * update name of repo cls * cr fixes * change git method to git_util * readme * update * fix tests * fix tests * fix uts * changelog * update logger * use error with click * pre-commit --- CHANGELOG.md | 1 + README.md | 2 + demisto_sdk/__main__.py | 12 ++++-- .../commands/common/content/content.py | 34 +++++++---------- demisto_sdk/commands/common/git_util.py | 37 +++++++++++++------ .../content_entity_validator.py | 3 +- .../hook_validations/graph_validator.py | 3 +- .../hook_validations/pack_unique_files.py | 6 +-- .../commands/common/tests/git_config_test.py | 2 +- .../common/tests/pack_unique_files_test.py | 20 ++-------- .../commands/common/tests/tools_test.py | 5 +-- demisto_sdk/commands/common/tools.py | 17 ++++----- .../content_artifacts_creator.py | 2 +- .../tests/content_artifacts_creator_test.py | 10 +++-- .../commands/doc_reviewer/doc_reviewer.py | 3 +- .../init/tests/contribution_converter_test.py | 4 -- demisto_sdk/commands/lint/helpers.py | 2 +- demisto_sdk/commands/lint/lint_manager.py | 7 ++-- demisto_sdk/commands/lint/linter.py | 3 +- .../tests/test_linter/gather_facts_test.py | 12 ++++-- demisto_sdk/commands/secrets/secrets.py | 3 +- .../update_release_notes/update_rn.py | 3 +- .../validate/tests/validators_test.py | 2 +- .../commands/validate/validate_manager.py | 2 +- demisto_sdk/tests/conf_file_test.py | 9 +---- ...ntent_create_artifacts_integration_test.py | 4 +- .../format_integration_test.py | 9 +---- .../validate_integration_test.py | 23 +----------- pyproject.toml | 1 + tests_end_to_end/e2e_tests_utils.py | 5 ++- 30 files changed, 107 insertions(+), 139 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a90128fb2d7..83c92e8bb9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Changelog ## Unreleased +* Demisto-SDK will now exit gracefully with an appropriate error message when *git* is not installed. ## 1.20.2 * Updated the **pre-commit** command to run on all python versions in one run. diff --git a/README.md b/README.md index 3eac257a9bb..04847ed74ea 100644 --- a/README.md +++ b/README.md @@ -9,6 +9,8 @@ The Demisto SDK library can be used to manage your Cortex XSOAR content with ease and efficiency. The library supports Python 3.8-3.10. _Python 3.8 support will be removed soon._ +In order to function properly, the Demisto SDK requires git to be installed. If git isn't installed, an appropriate message will be raised. + ## Usage ### Installation diff --git a/demisto_sdk/__main__.py b/demisto_sdk/__main__.py index 5a846f17817..2788150c70c 100644 --- a/demisto_sdk/__main__.py +++ b/demisto_sdk/__main__.py @@ -1,14 +1,20 @@ # Site packages +import sys + +import click + +try: + import git +except ImportError: + sys.exit(click.style("Git executable cannot be found, or is invalid", fg="red")) + import copy import functools import logging import os -import sys from pathlib import Path from typing import IO, Any, Dict, Iterable, Tuple, Union -import click -import git import typer from pkg_resources import DistributionNotFound, get_distribution diff --git a/demisto_sdk/commands/common/content/content.py b/demisto_sdk/commands/common/content/content.py index 413a6115b3a..b43be990d88 100644 --- a/demisto_sdk/commands/common/content/content.py +++ b/demisto_sdk/commands/common/content/content.py @@ -3,7 +3,7 @@ import os from typing import Any, Iterator -from git import InvalidGitRepositoryError, Repo +from git import InvalidGitRepositoryError from wcmatch.pathlib import Path from demisto_sdk.commands.common.constants import ( @@ -24,6 +24,7 @@ Documentation, ) from demisto_sdk.commands.common.content.objects_factory import path_to_pack_object +from demisto_sdk.commands.common.git_util import GitUtil from demisto_sdk.commands.common.logger import logger @@ -53,17 +54,14 @@ def from_cwd(cls) -> Content: TODO: 1. Add attribute which init only changed objects by git. """ - repo = cls.git() - if repo: - content = Content(repo.working_tree_dir) # type: ignore - else: - content = Content(Path.cwd()) - - return content + try: + return Content(str(cls.git_util().repo.working_tree_dir)) + except InvalidGitRepositoryError: + return Content(Path.cwd()) @staticmethod - def git() -> Repo | None: - """Git repository object. + def git_util() -> GitUtil: + """Git Util object. Returns: Repo: Repo object of content repo if exists else retun None. @@ -74,17 +72,13 @@ def git() -> Repo | None: Notes: 1. Should be called when cwd inside content repository. """ - try: - if content_path := os.getenv("DEMISTO_SDK_CONTENT_PATH"): - repo = Repo(content_path) - logger.debug(f"Using content path: {content_path}") - else: - repo = Repo(Path.cwd(), search_parent_directories=True) - except InvalidGitRepositoryError: - logger.debug("Git repo was not found.") - repo = None + if content_path := os.getenv("DEMISTO_SDK_CONTENT_PATH"): + git_util = GitUtil(Path(content_path)) + logger.debug(f"Using content path: {content_path}") + else: + git_util = GitUtil(search_parent_directories=True) - return repo + return git_util @property def path(self) -> Path: diff --git a/demisto_sdk/commands/common/git_util.py b/demisto_sdk/commands/common/git_util.py index b524f7446b7..4c9bb65039b 100644 --- a/demisto_sdk/commands/common/git_util.py +++ b/demisto_sdk/commands/common/git_util.py @@ -1,11 +1,14 @@ import os import re from pathlib import Path -from typing import Set, Tuple, Union +from typing import Optional, Set, Tuple, Union import click import gitdb -from git import InvalidGitRepositoryError, Repo +from git import ( + InvalidGitRepositoryError, + Repo, # noqa: TID251: required to create GitUtil +) from git.diff import Lit_change_type from git.remote import Remote @@ -13,18 +16,28 @@ class GitUtil: - repo: Repo + # in order to use Repo class/static methods + REPO_CLS = Repo - def __init__(self, repo: Repo = None): - if not repo: - try: - self.repo = Repo(Path.cwd(), search_parent_directories=True) - except InvalidGitRepositoryError: - raise InvalidGitRepositoryError( - "Unable to find Repository from current working directory - aborting" - ) + def __init__( + self, + path: Optional[Path] = None, + search_parent_directories: bool = True, + ): + + if isinstance(path, str): + repo_path = Path(path) else: - self.repo = repo + repo_path = path or Path.cwd() + + try: + self.repo = Repo( + repo_path, search_parent_directories=search_parent_directories + ) + except InvalidGitRepositoryError: + raise InvalidGitRepositoryError( + f"Unable to find Repository from current {repo_path.absolute()} - aborting" + ) def get_all_files(self) -> Set[Path]: return set(map(Path, self.repo.git.ls_files().split("\n"))) diff --git a/demisto_sdk/commands/common/hook_validations/content_entity_validator.py b/demisto_sdk/commands/common/hook_validations/content_entity_validator.py index 1ecb6e9777c..0bb002eaaf7 100644 --- a/demisto_sdk/commands/common/hook_validations/content_entity_validator.py +++ b/demisto_sdk/commands/common/hook_validations/content_entity_validator.py @@ -28,7 +28,6 @@ from demisto_sdk.commands.common.content import Content from demisto_sdk.commands.common.content_constant_paths import CONF_PATH, CONTENT_PATH from demisto_sdk.commands.common.errors import Errors -from demisto_sdk.commands.common.git_util import GitUtil from demisto_sdk.commands.common.handlers import DEFAULT_JSON_HANDLER as json from demisto_sdk.commands.common.handlers import DEFAULT_YAML_HANDLER as yaml from demisto_sdk.commands.common.hook_validations.base_validator import ( @@ -303,7 +302,7 @@ def is_release_branch() -> bool: Returns: (bool): is release branch """ - git_util = GitUtil(repo=Content.git()) + git_util = Content.git_util() main_branch = git_util.handle_prev_ver()[1] if not main_branch.startswith("origin"): main_branch = "origin/" + main_branch diff --git a/demisto_sdk/commands/common/hook_validations/graph_validator.py b/demisto_sdk/commands/common/hook_validations/graph_validator.py index 8afd9254bbd..1a8d19cc4a4 100644 --- a/demisto_sdk/commands/common/hook_validations/graph_validator.py +++ b/demisto_sdk/commands/common/hook_validations/graph_validator.py @@ -5,7 +5,6 @@ from demisto_sdk.commands.common.constants import PACKS_DIR from demisto_sdk.commands.common.content.content import Content from demisto_sdk.commands.common.errors import Errors -from demisto_sdk.commands.common.git_util import GitUtil from demisto_sdk.commands.common.hook_validations.base_validator import ( BaseValidator, error_codes, @@ -251,7 +250,7 @@ def validate_deprecated_items_usage(self): For existing content, a warning is raised. """ is_valid = True - new_files = GitUtil(repo=Content.git()).added_files() + new_files = Content.git_util().added_files() items: List[dict] = self.graph.find_items_using_deprecated_items( self.file_paths ) diff --git a/demisto_sdk/commands/common/hook_validations/pack_unique_files.py b/demisto_sdk/commands/common/hook_validations/pack_unique_files.py index 3e7e664ddf4..4f7c67b5c85 100644 --- a/demisto_sdk/commands/common/hook_validations/pack_unique_files.py +++ b/demisto_sdk/commands/common/hook_validations/pack_unique_files.py @@ -9,7 +9,7 @@ from typing import Dict, List, Set, Tuple from dateutil import parser -from git import GitCommandError, Repo +from git import GitCommandError from packaging.version import Version, parse from demisto_sdk.commands.common import tools @@ -135,7 +135,7 @@ def __init__( self.metadata_content: Dict = dict() if not prev_ver: - git_util = GitUtil(repo=Content.git()) + git_util = Content.git_util() main_branch = git_util.handle_prev_ver()[1] self.prev_ver = ( f"origin/{main_branch}" @@ -981,7 +981,7 @@ def is_right_usage_of_usecase_tag(self): return True def get_master_private_repo_meta_file(self, metadata_file_path: str): - current_repo = Repo(Path.cwd(), search_parent_directories=True) + current_repo = GitUtil().repo # if running on master branch in private repo - do not run the test if current_repo.active_branch == "master": diff --git a/demisto_sdk/commands/common/tests/git_config_test.py b/demisto_sdk/commands/common/tests/git_config_test.py index 0ad446ae81a..a40cfa729c9 100644 --- a/demisto_sdk/commands/common/tests/git_config_test.py +++ b/demisto_sdk/commands/common/tests/git_config_test.py @@ -3,7 +3,7 @@ from typing import NamedTuple import pytest -from git import Repo +from git import Repo # noqa: TID251 from demisto_sdk.commands.common.git_content_config import ( GitContentConfig, diff --git a/demisto_sdk/commands/common/tests/pack_unique_files_test.py b/demisto_sdk/commands/common/tests/pack_unique_files_test.py index 179b8c2353a..6f2febb8d5e 100644 --- a/demisto_sdk/commands/common/tests/pack_unique_files_test.py +++ b/demisto_sdk/commands/common/tests/pack_unique_files_test.py @@ -760,7 +760,7 @@ class MyRepo: active_branch = "master" mocker.patch( - "demisto_sdk.commands.common.hook_validations.pack_unique_files.Repo", + "demisto_sdk.commands.common.git_util.Repo", return_value=MyRepo, ) res = self.validator.get_master_private_repo_meta_file( @@ -810,12 +810,10 @@ def rev_parse(self, var): git = gitClass() mocker.patch( - "demisto_sdk.commands.common.hook_validations.pack_unique_files.Repo", + "demisto_sdk.commands.common.git_util.Repo", return_value=MyRepo(), ) - mocker.patch( - "demisto_sdk.commands.common.tools.git.Repo", return_value=MyRepo() - ) + with ChangeCWD(repo.path): res = self.validator.get_master_private_repo_meta_file( str(pack.pack_metadata.path) @@ -864,12 +862,9 @@ def rev_parse(self, var): git = gitClass() mocker.patch( - "demisto_sdk.commands.common.hook_validations.pack_unique_files.Repo", + "demisto_sdk.commands.common.git_util.Repo", return_value=MyRepo(), ) - mocker.patch( - "demisto_sdk.commands.common.tools.git.Repo", return_value=MyRepo() - ) res = self.validator.get_master_private_repo_meta_file( str(pack.pack_metadata.path) ) @@ -925,13 +920,6 @@ def remote(self): git = gitClass() - mocker.patch( - "demisto_sdk.commands.common.hook_validations.pack_unique_files.Repo", - return_value=MyRepo(), - ) - mocker.patch( - "demisto_sdk.commands.common.tools.git.Repo", return_value=MyRepo() - ) mocker.patch("demisto_sdk.commands.common.git_util.Repo", return_value=MyRepo()) with ChangeCWD(repo.path): diff --git a/demisto_sdk/commands/common/tests/tools_test.py b/demisto_sdk/commands/common/tests/tools_test.py index 4006c03a43f..76e9b2c9375 100644 --- a/demisto_sdk/commands/common/tests/tools_test.py +++ b/demisto_sdk/commands/common/tests/tools_test.py @@ -6,7 +6,6 @@ from tempfile import NamedTemporaryFile, TemporaryDirectory from typing import Callable, List, Optional, Tuple, Union -import git import pytest import requests @@ -626,12 +625,12 @@ class TestGetRemoteFileLocally: FILE_NAME = "somefile.json" FILE_CONTENT = '{"id": "some_file"}' - git_util = GitUtil(repo=Content.git()) + git_util = Content.git_util() main_branch = git_util.handle_prev_ver()[1] def setup_method(self): # create local git repo - example_repo = git.Repo.init(self.REPO_NAME) + example_repo = GitUtil.REPO_CLS.init(self.REPO_NAME) origin_branch = self.main_branch if not origin_branch.startswith("origin"): origin_branch = "origin/" + origin_branch diff --git a/demisto_sdk/commands/common/tools.py b/demisto_sdk/commands/common/tools.py index 2d14ca3b7aa..4fd86f89b49 100644 --- a/demisto_sdk/commands/common/tools.py +++ b/demisto_sdk/commands/common/tools.py @@ -339,7 +339,7 @@ def src_root() -> Path: Returns: Path: src root path. """ - git_dir = git.Repo(Path.cwd(), search_parent_directories=True).working_tree_dir + git_dir = GitUtil().repo.working_tree_dir return Path(git_dir) / "demisto_sdk" # type: ignore @@ -436,10 +436,7 @@ def get_local_remote_file( tag: str = "master", return_content: bool = False, ): - repo = git.Repo( - search_parent_directories=True - ) # the full file path could be a git file path - repo_git_util = GitUtil(repo) + repo_git_util = GitUtil() git_path = repo_git_util.get_local_remote_file_path(full_file_path, tag) file_content = repo_git_util.get_local_remote_file_content(git_path) if return_content: @@ -1958,7 +1955,7 @@ def is_external_repository() -> bool: """ try: - git_repo = git.Repo(os.getcwd(), search_parent_directories=True) + git_repo = GitUtil().repo private_settings_path = os.path.join(git_repo.working_dir, ".private-repo-settings") # type: ignore return Path(private_settings_path).exists() except git.InvalidGitRepositoryError: @@ -2016,10 +2013,10 @@ def get_content_path(relative_path: Optional[Path] = None) -> Path: ) try: if content_path := os.getenv("DEMISTO_SDK_CONTENT_PATH"): - git_repo = git.Repo(content_path) + git_repo = GitUtil(Path(content_path), search_parent_directories=False).repo logger.debug(f"Using content path: {content_path}") else: - git_repo = git.Repo(Path.cwd(), search_parent_directories=True) + git_repo = GitUtil().repo remote_url = git_repo.remote().urls.__next__() is_fork_repo = "content" in remote_url @@ -2122,7 +2119,7 @@ def is_file_from_content_repo(file_path: str) -> Tuple[bool, str]: str: relative path of file in content repo. """ try: - git_repo = git.Repo(os.getcwd(), search_parent_directories=True) + git_repo = GitUtil().repo remote_url = git_repo.remote().urls.__next__() is_fork_repo = "content" in remote_url is_external_repo = is_external_repository() @@ -3150,7 +3147,7 @@ def extract_docker_image_from_text(text: str, with_no_tag: bool = False): def get_current_repo() -> Tuple[str, str, str]: try: - git_repo = git.Repo(os.getcwd(), search_parent_directories=True) + git_repo = GitUtil().repo parsed_git = giturlparse.parse(git_repo.remotes.origin.url) host = parsed_git.host if "@" in host: diff --git a/demisto_sdk/commands/create_artifacts/content_artifacts_creator.py b/demisto_sdk/commands/create_artifacts/content_artifacts_creator.py index d0471d3771f..e3f38bd4259 100644 --- a/demisto_sdk/commands/create_artifacts/content_artifacts_creator.py +++ b/demisto_sdk/commands/create_artifacts/content_artifacts_creator.py @@ -1446,7 +1446,7 @@ def content_files_handler( and content_object.code_path.name == "CommonServerPython.py" ): # Modify CommonServerPython.py global variables - repo = artifact_manager.content.git() + repo = artifact_manager.content.git_util().repo modify_common_server_constants( content_object.code_path, artifact_manager.content_version, diff --git a/demisto_sdk/commands/create_artifacts/tests/content_artifacts_creator_test.py b/demisto_sdk/commands/create_artifacts/tests/content_artifacts_creator_test.py index d776c77df9e..1b198b4fb20 100644 --- a/demisto_sdk/commands/create_artifacts/tests/content_artifacts_creator_test.py +++ b/demisto_sdk/commands/create_artifacts/tests/content_artifacts_creator_test.py @@ -134,8 +134,8 @@ def mock_git(mocker): from demisto_sdk.commands.common.content import Content # Mock git working directory - mocker.patch.object(Content, "git") - Content.git().working_tree_dir = TEST_CONTENT_REPO + mocker.patch.object(Content, "git_util") + Content.git_util().repo.working_tree_dir = TEST_CONTENT_REPO yield @@ -407,8 +407,10 @@ def mock_single_pack_git(mocker): from demisto_sdk.commands.common.content import Content # Mock git working directory - mocker.patch.object(Content, "git") - Content.git().working_tree_dir = TEST_DATA / "content_repo_with_alternative_fields" + mocker.patch.object(Content, "git_util") + Content.git_util().repo.working_tree_dir = ( + TEST_DATA / "content_repo_with_alternative_fields" + ) yield diff --git a/demisto_sdk/commands/doc_reviewer/doc_reviewer.py b/demisto_sdk/commands/doc_reviewer/doc_reviewer.py index c9cbeecf7ac..c7fb439136d 100644 --- a/demisto_sdk/commands/doc_reviewer/doc_reviewer.py +++ b/demisto_sdk/commands/doc_reviewer/doc_reviewer.py @@ -24,7 +24,6 @@ from demisto_sdk.commands.common.content.objects.pack_objects.abstract_pack_objects.yaml_content_object import ( YAMLContentObject, ) -from demisto_sdk.commands.common.git_util import GitUtil from demisto_sdk.commands.common.logger import logger from demisto_sdk.commands.common.tools import ( add_default_pack_known_words, @@ -85,7 +84,7 @@ def __init__( self.git_util = None if use_git: - self.git_util = GitUtil(repo=Content.git()) + self.git_util = Content.git_util() self.prev_ver = self.git_util.handle_prev_ver()[1] else: self.prev_ver = prev_ver if prev_ver else "demisto/master" diff --git a/demisto_sdk/commands/init/tests/contribution_converter_test.py b/demisto_sdk/commands/init/tests/contribution_converter_test.py index a92b1355d9e..14bf880a110 100644 --- a/demisto_sdk/commands/init/tests/contribution_converter_test.py +++ b/demisto_sdk/commands/init/tests/contribution_converter_test.py @@ -140,7 +140,6 @@ def test_convert_contribution_zip_updated_pack(tmp_path, mocker): - Ensure that readme file has not been changed. """ - mocker.patch.object(GitUtil, "__init__", return_value=None) mocker.patch.object(GitUtil, "added_files", return_value=set()) mocker.patch.object(GitUtil, "modified_files", return_value=set()) # Create all Necessary Temporary directories @@ -222,7 +221,6 @@ def test_convert_contribution_zip_outputs_structure(tmp_path, mocker): - Ensure the unified yaml files of the integration and script have been removed from the output created by converting the contribution zip file """ - mocker.patch.object(GitUtil, "__init__", return_value=None) mocker.patch.object(GitUtil, "added_files", return_value=set()) mocker.patch.object(GitUtil, "modified_files", return_value=set()) @@ -364,7 +362,6 @@ def test_convert_contribution_zip(tmp_path, mocker): - Ensure script and integration are componentized and in valid directory structure - Ensure readme_files is not empty and the generated docs exists. """ - mocker.patch.object(GitUtil, "__init__", return_value=None) mocker.patch.object(GitUtil, "added_files", return_value=set()) mocker.patch.object(GitUtil, "modified_files", return_value=set()) # Create all Necessary Temporary directories @@ -497,7 +494,6 @@ def test_convert_contribution_zip_with_args(tmp_path, mocker): - Ensure that the pack's 'pack_metadata.json' file's 'githubUser' field a list containing only 'octocat' - Ensure that the pack's 'pack_metadata.json' file's 'email' field is the empty string """ - mocker.patch.object(GitUtil, "__init__", return_value=None) mocker.patch.object(GitUtil, "added_files", return_value=set()) mocker.patch.object(GitUtil, "modified_files", return_value=set()) diff --git a/demisto_sdk/commands/lint/helpers.py b/demisto_sdk/commands/lint/helpers.py index 7b9350d6786..402482d49bb 100644 --- a/demisto_sdk/commands/lint/helpers.py +++ b/demisto_sdk/commands/lint/helpers.py @@ -111,7 +111,7 @@ def build_skipped_exit_code( def get_test_modules( - content_repo: Optional[git.Repo], is_external_repo: bool + content_repo: Optional[git.Repo], is_external_repo: bool # noqa: TID251 ) -> Dict[Path, bytes]: """Get required test modules from content repository - {remote}/master 1. Tests/demistomock/demistomock.py diff --git a/demisto_sdk/commands/lint/lint_manager.py b/demisto_sdk/commands/lint/lint_manager.py index cf7278b7131..b77bb75bc2b 100644 --- a/demisto_sdk/commands/lint/lint_manager.py +++ b/demisto_sdk/commands/lint/lint_manager.py @@ -25,6 +25,7 @@ ) from demisto_sdk.commands.common.content_constant_paths import CONTENT_PATH from demisto_sdk.commands.common.docker_helper import init_global_docker_client +from demisto_sdk.commands.common.git_util import GitUtil from demisto_sdk.commands.common.handlers import DEFAULT_JSON_HANDLER as json from demisto_sdk.commands.common.logger import logger from demisto_sdk.commands.common.timers import report_time_measurements @@ -185,7 +186,7 @@ def _gather_facts() -> Dict[str, Any]: # Get content repo object is_external_repo = False try: - git_repo = git.Repo(os.getcwd(), search_parent_directories=True) + git_repo = GitUtil().repo remote_url = git_repo.remote().urls.__next__() is_fork_repo = "content" in remote_url is_external_repo = is_external_repository() @@ -267,7 +268,7 @@ def _gather_facts() -> Dict[str, Any]: def _get_packages( self, - content_repo: git.Repo, + content_repo: git.Repo, # noqa: TID251 input: Union[str, List[str]], git: bool = False, all_packs: bool = False, @@ -359,7 +360,7 @@ def _get_packages_from_modified_files(modified_files): @staticmethod def _filter_changed_packages( - content_repo: git.Repo, pkgs: List[PosixPath], base_branch: str + content_repo: git.Repo, pkgs: List[PosixPath], base_branch: str # noqa: TID251 ) -> List[PosixPath]: """Checks which packages had changes in them and should run on Lint. The diff is calculated using git, and is done by the following cases: diff --git a/demisto_sdk/commands/lint/linter.py b/demisto_sdk/commands/lint/linter.py index 77a7debf91d..88570da283d 100644 --- a/demisto_sdk/commands/lint/linter.py +++ b/demisto_sdk/commands/lint/linter.py @@ -30,6 +30,7 @@ get_python_version, init_global_docker_client, ) +from demisto_sdk.commands.common.git_util import GitUtil from demisto_sdk.commands.common.handlers import DEFAULT_JSON_HANDLER as json from demisto_sdk.commands.common.handlers import YAML_Handler from demisto_sdk.commands.common.hook_validations.docker import DockerImageValidator @@ -519,7 +520,7 @@ def _remove_gitignore_files(self, log_prompt: str) -> None: """ try: - repo = git.Repo(self._content_repo) + repo = GitUtil(self._content_repo).repo files_to_ignore = repo.ignored(self._facts["lint_files"]) for file in files_to_ignore: logger.info(f"{log_prompt} - Skipping gitignore file {file}") diff --git a/demisto_sdk/commands/lint/tests/test_linter/gather_facts_test.py b/demisto_sdk/commands/lint/tests/test_linter/gather_facts_test.py index b422ebfd01d..994f7f34d84 100644 --- a/demisto_sdk/commands/lint/tests/test_linter/gather_facts_test.py +++ b/demisto_sdk/commands/lint/tests/test_linter/gather_facts_test.py @@ -77,7 +77,7 @@ def test_checks_common_server_python(self, repo): script = pack.create_script("CommonServerPython") script.create_default_script() script_path = Path(script.path) - runner = initiate_linter(pack.path, script_path) + runner = initiate_linter(Path(pack.path), script_path) runner._gather_facts(modules={}) common_server_python_path = runner._facts.get("lint_files")[0] assert "Packs/Base/Scripts/CommonServerPython/CommonServerPython.py" in str( @@ -101,7 +101,10 @@ def test_package_is_python_pack( def test_package_is_python_pack_api_module_script( self, demisto_content, pack, mocker ): + from demisto_sdk.commands.common.git_util import Repo + script = pack.create_script(name="TestApiModule") + mocker.patch.object(Repo, "ignored", return_value=[]) mocker.patch.object(linter.Linter, "_update_support_level") runner = initiate_linter(demisto_content, script.path) assert not runner._gather_facts(modules={}) @@ -259,7 +262,7 @@ def test_docker_images_according_to_docker_image_flag( # Run lint: with ChangeCWD(pack.repo_path): runner = initiate_linter( - pack.repo_path, + Path(pack.repo_path), test_integration.path, True, docker_image_flag=docker_image_flag, @@ -736,7 +739,10 @@ def test_deprecated_script( - Case A: gather facts should indicate script is skipped - Case B: gather father should indicate script is not skipped """ + from demisto_sdk.commands.common.git_util import Repo + script.yml.update({"deprecated": True}) + mocker.patch.object(Repo, "ignored", return_value=[]) mocker.patch.object(linter.Linter, "_update_support_level") runner = initiate_linter( demisto_content, script.path, True, all_packs=all_packs @@ -836,7 +842,7 @@ class GitMock: def ignored(self, files): return files[-1:] - mocker.patch("git.Repo", return_value=GitMock()) + mocker.patch("demisto_sdk.commands.common.git_util.Repo", return_value=GitMock()) runner = initiate_linter(demisto_content, "") runner._facts["lint_files"] = files_paths assert files_paths[-1] in runner._facts["lint_files"] diff --git a/demisto_sdk/commands/secrets/secrets.py b/demisto_sdk/commands/secrets/secrets.py index c5f8fe78864..43dee64b764 100644 --- a/demisto_sdk/commands/secrets/secrets.py +++ b/demisto_sdk/commands/secrets/secrets.py @@ -17,7 +17,6 @@ re, ) from demisto_sdk.commands.common.content import Content -from demisto_sdk.commands.common.git_util import GitUtil from demisto_sdk.commands.common.handlers import DEFAULT_JSON_HANDLER as json from demisto_sdk.commands.common.logger import logger from demisto_sdk.commands.common.tools import ( @@ -181,7 +180,7 @@ def get_all_diff_text_files(self, branch_name, is_circle): if is_circle: prev_ver = self.prev_ver if not prev_ver: - self.git_util = GitUtil(repo=Content.git()) + self.git_util = Content.git_util() prev_ver = self.git_util.handle_prev_ver()[1] if not prev_ver.startswith("origin"): prev_ver = "origin/" + prev_ver diff --git a/demisto_sdk/commands/update_release_notes/update_rn.py b/demisto_sdk/commands/update_release_notes/update_rn.py index 85dc67ae9ad..fc4acf29e8a 100644 --- a/demisto_sdk/commands/update_release_notes/update_rn.py +++ b/demisto_sdk/commands/update_release_notes/update_rn.py @@ -32,7 +32,6 @@ from demisto_sdk.commands.common.content.objects.pack_objects.abstract_pack_objects.yaml_content_object import ( YAMLContentObject, ) -from demisto_sdk.commands.common.git_util import GitUtil from demisto_sdk.commands.common.handlers import DEFAULT_JSON_HANDLER as json from demisto_sdk.commands.common.logger import logger from demisto_sdk.commands.common.tools import ( @@ -172,7 +171,7 @@ def __init__( self.should_delete_existing_rn = False self.pack_metadata_only = pack_metadata_only self.is_force = is_force - git_util = GitUtil(repo=Content.git()) + git_util = Content.git_util() self.main_branch = git_util.handle_prev_ver()[1] self.metadata_path = os.path.join(self.pack_path, "pack_metadata.json") self.master_version = self.get_master_version() diff --git a/demisto_sdk/commands/validate/tests/validators_test.py b/demisto_sdk/commands/validate/tests/validators_test.py index 4a2e5f26fb7..add25c13ba1 100644 --- a/demisto_sdk/commands/validate/tests/validators_test.py +++ b/demisto_sdk/commands/validate/tests/validators_test.py @@ -166,7 +166,7 @@ def remote(self): @pytest.fixture(autouse=False) def set_git_test_env(mocker): mocker.patch.object(ValidateManager, "setup_git_params", return_value=True) - mocker.patch.object(Content, "git", return_value=MyRepo()) + mocker.patch.object(Content, "git_util", return_value=GitUtil()) mocker.patch.object(ValidateManager, "setup_prev_ver", return_value="origin/master") mocker.patch.object(GitUtil, "_is_file_git_ignored", return_value=False) diff --git a/demisto_sdk/commands/validate/validate_manager.py b/demisto_sdk/commands/validate/validate_manager.py index 942067f2b90..8ef8d2e2266 100644 --- a/demisto_sdk/commands/validate/validate_manager.py +++ b/demisto_sdk/commands/validate/validate_manager.py @@ -268,7 +268,7 @@ def __init__( self.deprecation_validator = DeprecationValidator(id_set_file=self.id_set_file) try: - self.git_util = GitUtil(repo=Content.git()) + self.git_util = Content.git_util() self.branch_name = self.git_util.get_current_git_branch_or_hash() except (InvalidGitRepositoryError, TypeError): # if we are using git - fail the validation by raising the exception. diff --git a/demisto_sdk/tests/conf_file_test.py b/demisto_sdk/tests/conf_file_test.py index 9fe884aff8c..2cceee93613 100644 --- a/demisto_sdk/tests/conf_file_test.py +++ b/demisto_sdk/tests/conf_file_test.py @@ -13,13 +13,6 @@ from TestSuite.test_tools import ChangeCWD, str_in_call_args_list -class MyRepo: - active_branch = "not-master" - - def remote(self): - return "remote_path" - - def test_conf_file_custom(mocker, monkeypatch, repo): """ Given @@ -39,7 +32,7 @@ def test_conf_file_custom(mocker, monkeypatch, repo): mocker.patch.object(tools, "is_external_repository", return_value=True) mocker.patch.object(IntegrationValidator, "is_valid_category", return_value=True) mocker.patch.object(ValidateManager, "setup_git_params", return_value=True) - mocker.patch.object(Content, "git", return_value=MyRepo()) + mocker.patch.object(Content, "git_util", return_value=GitUtil()) mocker.patch.object(ValidateManager, "setup_prev_ver", return_value="origin/master") mocker.patch.object(GitUtil, "_is_file_git_ignored", return_value=False) pack = repo.create_pack("tempPack") diff --git a/demisto_sdk/tests/integration_tests/content_create_artifacts_integration_test.py b/demisto_sdk/tests/integration_tests/content_create_artifacts_integration_test.py index abb14bc7d1f..35951d4fc21 100644 --- a/demisto_sdk/tests/integration_tests/content_create_artifacts_integration_test.py +++ b/demisto_sdk/tests/integration_tests/content_create_artifacts_integration_test.py @@ -27,8 +27,8 @@ def mock_git(mocker): from demisto_sdk.commands.common.content import Content # Mock git working directory - mocker.patch.object(Content, "git") - Content.git().working_tree_dir = TEST_CONTENT_REPO + mocker.patch.object(Content, "git_util") + Content.git_util().repo.working_tree_dir = TEST_CONTENT_REPO yield diff --git a/demisto_sdk/tests/integration_tests/format_integration_test.py b/demisto_sdk/tests/integration_tests/format_integration_test.py index 2e432c149ce..43938d45b01 100644 --- a/demisto_sdk/tests/integration_tests/format_integration_test.py +++ b/demisto_sdk/tests/integration_tests/format_integration_test.py @@ -99,17 +99,10 @@ } -class MyRepo: - active_branch = "not-master" - - def remote(self): - return "remote_path" - - @pytest.fixture(autouse=True) def set_git_test_env(mocker): mocker.patch.object(ValidateManager, "setup_git_params", return_value=True) - mocker.patch.object(Content, "git", return_value=MyRepo()) + mocker.patch.object(Content, "git_util", return_value=GitUtil()) mocker.patch.object(ValidateManager, "setup_prev_ver", return_value="origin/master") mocker.patch.object(GitUtil, "_is_file_git_ignored", return_value=False) diff --git a/demisto_sdk/tests/integration_tests/validate_integration_test.py b/demisto_sdk/tests/integration_tests/validate_integration_test.py index 65fb45d01d2..2781793f3b7 100644 --- a/demisto_sdk/tests/integration_tests/validate_integration_test.py +++ b/demisto_sdk/tests/integration_tests/validate_integration_test.py @@ -116,17 +116,10 @@ } -class MyRepo: - active_branch = "not-master" - - def remote(self): - return "remote_path" - - @pytest.fixture(autouse=True) def set_git_test_env(mocker): mocker.patch.object(ValidateManager, "setup_git_params", return_value=True) - mocker.patch.object(Content, "git", return_value=MyRepo()) + mocker.patch.object(Content, "git_util", return_value=GitUtil()) mocker.patch.object(ValidateManager, "setup_prev_ver", return_value="origin/master") mocker.patch.object(GitUtil, "_is_file_git_ignored", return_value=False) mocker.patch.object( @@ -1065,7 +1058,6 @@ def test_invalid_modified_bc_deprecated_integration(self, mocker, repo): "get_changed_files_from_git", return_value=(modified_files, set(), set(), set(), True), ) - mocker.patch.object(GitUtil, "__init__", return_value=None) mocker.patch.object( GitUtil, "get_current_working_branch", return_value="MyBranch" ) @@ -1182,7 +1174,6 @@ def test_modified_invalid_bc_unsupported_toversion_integration(self, mocker, rep "get_changed_files_from_git", return_value=(modified_files, set(), set(), set(), True), ) - mocker.patch.object(GitUtil, "__init__", return_value=None) mocker.patch.object( GitUtil, "get_current_working_branch", return_value="MyBranch" ) @@ -3723,7 +3714,6 @@ def test_modified_invalid_bc_deprecated_playbook(self, mocker, repo): "get_changed_files_from_git", return_value=(modified_files, {}, set(), set(), True), ) - mocker.patch.object(GitUtil, "__init__", return_value=None) mocker.patch.object( GitUtil, "get_current_working_branch", return_value="MyBranch" ) @@ -3825,7 +3815,6 @@ def test_modified_invalid_bc_unsupported_toversion_playbook(self, mocker, repo): "get_changed_files_from_git", return_value=(modified_files, {}, set(), set(), True), ) - mocker.patch.object(GitUtil, "__init__", return_value=None) mocker.patch.object( GitUtil, "get_current_working_branch", return_value="MyBranch" ) @@ -4226,7 +4215,6 @@ def test_modified_invalid_bc_deprecated_script(self, mocker, repo): "get_changed_files_from_git", return_value=(modified_files, {}, set(), set(), True), ) - mocker.patch.object(GitUtil, "__init__", return_value=None) mocker.patch.object( GitUtil, "get_current_working_branch", return_value="MyBranch" ) @@ -4334,7 +4322,6 @@ def test_modified_invalid_bc_unsupported_toversion_script(self, mocker, repo): "get_changed_files_from_git", return_value=(modified_files, {}, set(), set(), True), ) - mocker.patch.object(GitUtil, "__init__", return_value=None) mocker.patch.object( GitUtil, "get_current_working_branch", return_value="MyBranch" ) @@ -4903,7 +4890,6 @@ def test_passing_validation_using_git(self, mocker, repo): "get_changed_files_from_git", return_value=(modified_files, added_files, set(), old_files, True), ) - mocker.patch.object(GitUtil, "__init__", return_value=None) mocker.patch.object( GitUtil, "get_current_working_branch", return_value="MyBranch" ) @@ -4994,7 +4980,6 @@ def test_failing_validation_using_git(self, mocker, repo): "get_changed_files_from_git", return_value=(modified_files, added_files, set(), set(), True), ) - mocker.patch.object(GitUtil, "__init__", return_value=None) mocker.patch.object( GitUtil, "get_current_working_branch", return_value="MyBranch" ) @@ -5078,7 +5063,6 @@ def test_validation_using_git_without_pack_dependencies(self, mocker, repo): "get_changed_files_from_git", return_value=(modified_files, set(), set(), set(), True), ) - mocker.patch.object(GitUtil, "__init__", return_value=None) mocker.patch.object( GitUtil, "get_current_working_branch", return_value="MyBranch" ) @@ -5155,7 +5139,6 @@ def test_validation_using_git_with_pack_dependencies(self, mocker, repo): "get_changed_files_from_git", return_value=(modified_files, set(), set(), set(), True), ) - mocker.patch.object(GitUtil, "__init__", return_value=None) mocker.patch.object( GitUtil, "get_current_working_branch", return_value="MyBranch" ) @@ -5301,7 +5284,6 @@ def test_validation_using_git_on_specific_file(self, mocker, repo): "get_changed_files_from_git", return_value=(modified_files, set(), set(), set(), True), ) - mocker.patch.object(GitUtil, "__init__", return_value=None) mocker.patch.object( GitUtil, "get_current_working_branch", return_value="MyBranch" ) @@ -5378,7 +5360,6 @@ def test_validation_using_git_on_specific_file_renamed(self, mocker, repo): "get_changed_files_from_git", return_value=(modified_files, set(), set(), set(), True), ) - mocker.patch.object(GitUtil, "__init__", return_value=None) mocker.patch.object( GitUtil, "get_current_working_branch", return_value="MyBranch" ) @@ -5466,7 +5447,6 @@ def test_validation_using_git_on_specific_pack(self, mocker, repo): "get_changed_files_from_git", return_value=(modified_files, set(), set(), set(), True), ) - mocker.patch.object(GitUtil, "__init__", return_value=None) mocker.patch.object( GitUtil, "get_current_working_branch", return_value="MyBranch" ) @@ -5697,7 +5677,6 @@ def test_modified_pack_files_with_ignored_validations(self, mocker, repo): "get_changed_files_from_git", return_value=(modified_files, set(), set(), set(), True), ) - mocker.patch.object(GitUtil, "__init__", return_value=None) mocker.patch.object( GitUtil, "get_current_working_branch", return_value="MyBranch" ) diff --git a/pyproject.toml b/pyproject.toml index 035b19d23a5..5409c7f0627 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -188,3 +188,4 @@ max-complexity = 30 "ruamel.yaml".msg = "use YAML_Handler instead" "distutils.version".msg = "Use packaging.version.Version instead" "packaging.version.LooseVersion".msg = "Use packaging.version.Version instead" +"git.Repo".msg = "Use GitUtil instead" diff --git a/tests_end_to_end/e2e_tests_utils.py b/tests_end_to_end/e2e_tests_utils.py index f4e4a8a86f1..25b762b2b62 100644 --- a/tests_end_to_end/e2e_tests_utils.py +++ b/tests_end_to_end/e2e_tests_utils.py @@ -9,10 +9,11 @@ def git_clone_demisto_sdk(destination_folder: str, sdk_git_branch: str = "master"): """Clone demisto-sdk from GitHub and add it to sys.path""" + from demisto_sdk.commands.common.git_util import GitUtil + logger.info(f"Cloning demisto-sdk to {destination_folder}") - import git - git.Repo.clone_from( + GitUtil.REPO_CLS.clone_from( url="https://github.com/demisto/demisto-sdk.git", to_path=destination_folder, multi_options=[f"-b {sdk_git_branch}", "--single-branch", "--depth 1"],