Repository: frappe/frappe
Pull request: fix: error on trying to check semantic version
The semantic version check fails if an app does not have any releases (in a fork, for example).

<hr>

**Traceback:**

```python
ValueError: Call either Version('1.2.3') or Version(major=1, ...).
  File "rq/worker.py", line 793, in perform_job
    rv = job.perform()
  File "rq/job.py", line 599, in perform
    self._result = self._execute()
  File "rq/job.py", line 605, in _execute
    return self.func(*self.args, **self.kwargs)
  File "frappe/utils/background_jobs.py", line 102, in execute_job
    method(**kwargs)
  File "frappe/utils/change_log.py", line 146, in check_for_update
    app_details = check_release_on_github(app)
  File "frappe/utils/change_log.py", line 209, in check_release_on_github
    return Version(lastest_non_beta_release), org_name
  File "semantic_version/base.py", line 102, in __init__
    raise ValueError("Call either Version('1.2.3') or Version(major=1, ...).")
```

Also fixes https://github.com/frappe/frappe/issues/11097

Problem statement:
error: frappe.utils.change_log.check_for_update
```
{'method_name': 'frappe.utils.change_log.check_for_update', 'log': <function log at 0x7f837e01f730>, 'retry': 0, 'is_async': True, 'user': 'Administrator', 'kwargs': {}, 'job_name': 'frappe.utils.change_log.check_for_update', 'event': 'daily_long', 'method': <function check_for_update at 0x7f837e54cae8>, 'site': 'my.erp.com'}
Traceback (most recent call last):
  File "/home/frappe/frappe-bench/apps/frappe/frappe/utils/background_jobs.py", line 102, in execute_job
    method(**kwargs)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/utils/change_log.py", line 144, in check_for_update
    app_details = check_release_on_github(app)
  File "/home/frappe/frappe-bench/apps/frappe/frappe/utils/change_log.py", line 207, in check_release_on_github
    return Version(lastest_non_beta_release), org_name
  File "/home/frappe/frappe-bench/env/lib/python3.6/site-packages/semantic_version/base.py", line 102, in __init__
    raise ValueError("Call either Version('1.2.3') or Version(major=1, ...).")
ValueError: Call either Version('1.2.3') or Version(major=1, ...).
```

Frappe Framework: v12.8.0 (version-12)

Hints from issue discussion:

Commit message: fix: add git URL check

Patch to review:
diff --git a/frappe/utils/change_log.py b/frappe/utils/change_log.py
index 29fee2bac024..75421c43ea7f 100644
--- a/frappe/utils/change_log.py
+++ b/frappe/utils/change_log.py
@@ -1,15 +1,16 @@
 # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors
 # MIT License. See license.txt
 
-from __future__ import unicode_literals
-from six.moves import range
-import json, os
-from semantic_version import Version
+import json
+import os
+import subprocess  # nosec
+
 import frappe
 import requests
-import subprocess # nosec
-from frappe.utils import cstr
 from frappe import _, safe_decode
+from frappe.utils import cstr, is_git_url
+from semantic_version import Version
+from six.moves import range
 
 
 def get_change_log(user=None):
@@ -165,9 +166,10 @@ def check_for_update():
 
 	add_message_to_redis(updates)
 
+
 def parse_latest_non_beta_release(response):
 	"""
-	Pasrses the response JSON for all the releases and returns the latest non prerelease
+	Parses the response JSON for all the releases and returns the latest non prerelease
 
 	Parameters
 	response (list): response object returned by github
@@ -182,32 +184,37 @@ def parse_latest_non_beta_release(response):
 
 	return None
 
+
 def check_release_on_github(app):
-	# Check if repo remote is on github
 	from subprocess import CalledProcessError
+
 	try:
+		# Check if repo remote is on github
 		remote_url = subprocess.check_output("cd ../apps/{} && git ls-remote --get-url".format(app), shell=True).decode()
 	except CalledProcessError:
 		# Passing this since some apps may not have git initializaed in them
-		return None
+		return
 
 	if isinstance(remote_url, bytes):
 		remote_url = remote_url.decode()
 
 	if "github.com" not in remote_url:
-		return None
+		return
 
 	# Get latest version from github
 	if 'https' not in remote_url:
-		return None
+		return
+
+	if is_git_url(remote_url):
+		return
 
 	org_name = remote_url.split('/')[3]
 	r = requests.get('https://api.github.com/repos/{}/{}/releases'.format(org_name, app))
 	if r.ok:
-		lastest_non_beta_release = parse_latest_non_beta_release(r.json())
-		return Version(lastest_non_beta_release), org_name
-	# In case of an improper response or if there are no releases
-	return None
+		latest_non_beta_release = parse_latest_non_beta_release(r.json())
+		if latest_non_beta_release:
+			return Version(latest_non_beta_release), org_name
+
 
 def add_message_to_redis(update_json):
 	# "update-message" will store the update message string
diff --git a/frappe/utils/data.py b/frappe/utils/data.py
index 41f247da45e0..1ddf59a82c94 100644
--- a/frappe/utils/data.py
+++ b/frappe/utils/data.py
@@ -1376,3 +1376,9 @@ def validate_json_string(string):
 		json.loads(string)
 	except (TypeError, ValueError):
 		raise frappe.ValidationError
+
+
+def is_git_url(url):
+	# modified to allow without the tailing .git from https://github.com/jonschlinkert/is-git-url.git
+	pattern = r"(?:git|ssh|https?|git@[-\w.]+):(\/\/)?(.*?)(\.git)?(\/?|\#[-\d\w._]+?)$"
+	return bool(re.match(pattern, url))


Code review comments:
diff_hunk: @@ -182,32 +184,37 @@ def parse_latest_non_beta_release(response):
 
 	return None
 
+
 def check_release_on_github(app):
-	# Check if repo remote is on github
 	from subprocess import CalledProcessError
+
 	try:
+		# Check if repo remote is on github
 		remote_url = subprocess.check_output("cd ../apps/{} && git ls-remote --get-url".format(app), shell=True).decode()
 	except CalledProcessError:
 		# Passing this since some apps may not have git initializaed in them
-		return None
+		return
 
 	if isinstance(remote_url, bytes):
 		remote_url = remote_url.decode()
 
 	if "github.com" not in remote_url:
-		return None
+		return
 
 	# Get latest version from github
 	if 'https' not in remote_url:
-		return None
+		return
path: frappe/utils/change_log.py
text: @user1:
We probably don't need these checks now? Seems sane to remove them as it directly exits if its an SSH url, making the `is_git_url` check redundant