From eb715735bbb79e5edd39e29c1b0e5587d2d3712c Mon Sep 17 00:00:00 2001 From: Ritwik Puri Date: Wed, 27 Apr 2022 20:17:04 +0530 Subject: [PATCH 1/3] fix: Misc fixes (#1294) * fix: use better api endpoint for is_valid_frappe_branch * fix: dont allow appnames starting with number and/or dot in them * fix: dont convert response to json * fix: using ls-remote to check for valid branches * refactor: changed the message displayed on invalid branch and invalid frappe path Co-authored-by: Aradhya Co-authored-by: Aradhya Tripathi <67282231+Aradhya-Tripathi@users.noreply.github.com> --- bench/app.py | 7 +++++++ bench/utils/__init__.py | 40 +++++++++++++++++++++------------------- 2 files changed, 28 insertions(+), 19 deletions(-) diff --git a/bench/app.py b/bench/app.py index de5b1d92..e0da4e18 100755 --- a/bench/app.py +++ b/bench/app.py @@ -486,6 +486,13 @@ def new_app(app, no_git=None, bench_path="."): # For backwards compatibility app = app.lower().replace(" ", "_").replace("-", "_") + if app[0].isdigit() or "." in app: + click.secho( + "App names cannot start with numbers(digits) or have dot(.) in them", + fg="red" + ) + return + apps = os.path.abspath(os.path.join(bench_path, "apps")) args = ["make-app", apps, app] if no_git: diff --git a/bench/utils/__init__.py b/bench/utils/__init__.py index 45cd1dce..d0bb0383 100644 --- a/bench/utils/__init__.py +++ b/bench/utils/__init__.py @@ -51,10 +51,9 @@ def is_frappe_app(directory: str) -> bool: def is_valid_frappe_branch(frappe_path:str, frappe_branch:str): - """ Check if a branch exists in a repo. Throws InvalidRemoteException if branch is not found + """Check if a branch exists in a repo. Throws InvalidRemoteException if branch is not found - Uses github's api without auth to query branch. - If rate limited by gitapi, requests are sent to github.com + Uses native git command to check for branches on a remote. :param frappe_path: git url :type frappe_path: str @@ -62,23 +61,26 @@ def is_valid_frappe_branch(frappe_path:str, frappe_branch:str): :type frappe_branch: str :raises InvalidRemoteException: branch for this repo doesn't exist """ - if "http" in frappe_path and frappe_branch: - frappe_path = frappe_path.replace(".git", "") + import subprocess + + if frappe_branch: try: - owner, repo = frappe_path.split("/")[3], frappe_path.split("/")[4] - except IndexError: - raise InvalidRemoteException("Invalid git url") - git_api_req = f"https://api.github.com/repos/{owner}/{repo}/branches" - res = requests.get(git_api_req).json() - - if "message" in res: - # slower alternative with no rate limit - github_req = f'https://github.com/{owner}/{repo}/tree/{frappe_branch}' - if requests.get(github_req).status_code != 200: - raise InvalidRemoteException("Invalid git url") - - elif frappe_branch not in [x["name"] for x in res]: - raise InvalidRemoteException("Frappe branch does not exist") + ret = subprocess.check_output( + ( + "git", + "ls-remote", + "--heads", + frappe_path, + frappe_branch, + ), + encoding="UTF-8", + ) + if not ret: + raise InvalidRemoteException( + f"Invalid {frappe_branch} for the remote {frappe_path}" + ) + except subprocess.CalledProcessError: + raise InvalidRemoteException(f"Invalid frappe path {frappe_path}") def log(message, level=0, no_log=False): From 31d9de4decacf62a5e8aec9d88855174dc929b51 Mon Sep 17 00:00:00 2001 From: Gavin D'souza Date: Thu, 28 Apr 2022 11:03:25 +0530 Subject: [PATCH 2/3] ci: Remove install script tests The script has been untested and unmaintained for a while. We've asked for help in maintenance but nobody seems to want to undertake it. With newer versions of distros and packages coming out each day, setups get more and more volatile. Making these tests run in the CI don't reflect how it would go down irl as no server comes with the same base config. Apart from the base sanity, these tests are just a waste of carbon. Removing them. --- .travis.yml | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/.travis.yml b/.travis.yml index 97296634..f1d8b83f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -60,21 +60,6 @@ matrix: env: TEST=bench script: python -m unittest -v bench.tests.test_utils && python -m unittest -v bench.tests.test_init - - name: "Python 3.7 Easy Install" - python: 3.7 - env: TEST=easy_install - script: sudo python $TRAVIS_BUILD_DIR/install.py --user travis --run-travis --production --verbose - - - name: "Python 3.8 Easy Install" - python: 3.8 - env: TEST=easy_install - script: sudo python $TRAVIS_BUILD_DIR/install.py --user travis --run-travis --production --verbose - - - name: "Python 3.9 Easy Install" - python: 3.9 - env: TEST=easy_install - script: sudo python $TRAVIS_BUILD_DIR/install.py --user travis --run-travis --production --verbose - install: - pip3 install urllib3 pyOpenSSL ndg-httpsclient pyasn1 From 7b8f16bcb43e5390aabb06225870887cfda376fc Mon Sep 17 00:00:00 2001 From: Kelvin zawala <87924723+Zawala@users.noreply.github.com> Date: Tue, 3 May 2022 13:52:10 +0200 Subject: [PATCH 3/3] Get-app from port specific ssh git server (#1299) * Get-app from port specific ssh git server Current bench package fails to get an app from a private git server with a specific ssh port other than the normal 22. * test: Added tests for ssh ports Co-authored-by: Aradhya Tripathi <67282231+Aradhya-Tripathi@users.noreply.github.com> Co-authored-by: Aradhya --- bench/app.py | 2 +- bench/tests/test_utils.py | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/bench/app.py b/bench/app.py index e0da4e18..6a9d4e97 100755 --- a/bench/app.py +++ b/bench/app.py @@ -116,7 +116,7 @@ class AppMeta: name = url if url else self.name if name.startswith("git@") or name.startswith("ssh://"): self.use_ssh = True - _first_part, _second_part = name.split(":") + _first_part, _second_part = self.name.rsplit(":", 1) self.remote_server = _first_part.split("@")[-1] self.org, _repo = _second_part.rsplit("/", 1) else: diff --git a/bench/tests/test_utils.py b/bench/tests/test_utils.py index 3c2b3307..f91e8785 100644 --- a/bench/tests/test_utils.py +++ b/bench/tests/test_utils.py @@ -73,3 +73,7 @@ class TestUtils(unittest.TestCase): self.assertEqual("11.0", fake_bench.apps.states["frappe"]["version"]) shutil.rmtree(bench_dir) + + def test_ssh_ports(self): + app = App("git@github.com:22:frappe/frappe") + self.assertEqual((app.use_ssh, app.org, app.repo), (True, "frappe", "frappe")) \ No newline at end of file