diff mbox series

[1/1] check-package: add version format check

Message ID 20191025081459.32429-1-heiko.thiery@gmail.com
State Superseded
Headers show
Series [1/1] check-package: add version format check | expand

Commit Message

Heiko Thiery Oct. 25, 2019, 8:15 a.m. UTC 
From: Heiko Thiery <heiko.thiery@kontron.com>

To meet the requirement of "Anitya", the tool used by
https://release-monitoring.org/, the <pkg>_VERSION variable
should not contain a prefix 'v'.

Signed-off-by: Heiko Thiery <heiko.thiery@kontron.com>
---
 utils/checkpackagelib/lib_mk.py | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

Comments

Thomas Petazzoni Oct. 25, 2019, 9 a.m. UTC | #1 
Hello,

On Fri, 25 Oct 2019 10:15:00 +0200
Heiko Thiery <heiko.thiery@gmail.com> wrote:

> From: Heiko Thiery <heiko.thiery@kontron.com>
> 
> To meet the requirement of "Anitya", the tool used by
> https://release-monitoring.org/, the <pkg>_VERSION variable
> should not contain a prefix 'v'.

Wow, that was fast! Thanks!

> +class Version(_CheckFunction):
> +    VERSION = re.compile(r"^([A-Z0-9_]+)_VERSION\s*[\+|:|]*=\s*(.*)")
> +    GIT_HASH = re.compile("([A-Za-f0-9]{40})")
> +
> +    def _is_git_hash(self, value):
> +        return True if self.GIT_HASH.match(value) else False
> +
> +    def check_line(self, lineno, text):
> +        m = self.VERSION.search(text)
> +        if m is None:
> +            return
> +
> +        variable, assignment = m.group(1, 2)
> +
> +        if self._is_git_hash(assignment):
> +            return

Do we really need this check? After all, we're checking below if it
starts with "v" and "v" is not a valid digit in hexadecimal, so it will
never appear in a Git hash, so a Git hash will anyway never match the
below condition "version starts with v".

> +        if assignment.startswith('v'):

The only concern I have with such a "simple" approach is what if
someone creates a package like this:

FOO_SITE = git://git.somewhere.com
FOO_VERSION = very-last-tag-before-1.0-release

Of course, that's a very stupid Git tag name, but I guess you see my
point: are there cases where a version starting with "v" would be valid?

That's why I was suggesting something like a check that it looks like
"vX.Y.Z". As Arnout said, it could be "vX-Y-Z". Perhaps we should check
for "v[0-9]" ?

Best regards,

Thomas
diff mbox series

Patch

diff --git a/utils/checkpackagelib/lib_mk.py b/utils/checkpackagelib/lib_mk.py
index dd04ffd58f..ffde48e05b 100644
--- a/utils/checkpackagelib/lib_mk.py
+++ b/utils/checkpackagelib/lib_mk.py
@@ -325,3 +325,28 @@  class VariableWithBraces(_CheckFunction):
             return ["{}:{}: use $() to delimit variables, not ${{}}"
                     .format(self.filename, lineno),
                     text]
+
+
+class Version(_CheckFunction):
+    VERSION = re.compile(r"^([A-Z0-9_]+)_VERSION\s*[\+|:|]*=\s*(.*)")
+    GIT_HASH = re.compile("([A-Za-f0-9]{40})")
+
+    def _is_git_hash(self, value):
+        return True if self.GIT_HASH.match(value) else False
+
+    def check_line(self, lineno, text):
+        m = self.VERSION.search(text)
+        if m is None:
+            return
+
+        variable, assignment = m.group(1, 2)
+
+        if self._is_git_hash(assignment):
+            return
+
+        if assignment.startswith('v'):
+            return ["{}:{}: remove 'v' prefix from {} of variable {}_VERSION "
+                    "and add to {}_SITE"
+                    .format(self.filename, lineno, assignment, variable,
+                            variable),
+                    text]