tarantool / mkrepo

Maintain DEB and RPM repositories on S3
69 stars 24 forks source link

Fail to sync `live/1.10/el/7/x86_64` #62

Closed LeonidVas closed 2 years ago

LeonidVas commented 2 years ago

The tarantool repositories specific bug. It is assumed that the problem will be fixed by the tarantool team with access to the repository.

2022-06-01T15:43:52.167754425Z app[web.1]: Traceback (most recent call last):
2022-06-01T15:43:52.167954314Z app[web.1]:   File "/app/.heroku/python/bin/mkrepo", line 5, in <module>
2022-06-01T15:43:52.167969891Z app[web.1]:     mkrepo.main()
2022-06-01T15:43:52.167979377Z app[web.1]:   File "/app/.heroku/python/lib/python3.9/site-packages/mkrepo.py", line 117, in main
2022-06-01T15:43:52.168122234Z app[web.1]:     update_repo(path, args)
2022-06-01T15:43:52.168157987Z app[web.1]:   File "/app/.heroku/python/lib/python3.9/site-packages/mkrepo.py", line 59, in update_repo
2022-06-01T15:43:52.168168105Z app[web.1]:     rpmrepo.update_repo(stor, args.sign, args.temp_dir, args.force)
2022-06-01T15:43:52.168177668Z app[web.1]:   File "/app/.heroku/python/lib/python3.9/site-packages/rpmrepo.py", line 1097, in update_repo
2022-06-01T15:43:52.168187173Z app[web.1]:     primary_str = dump_primary(primary)
2022-06-01T15:43:52.168196066Z app[web.1]:   File "/app/.heroku/python/lib/python3.9/site-packages/rpmrepo.py", line 495, in dump_primary
2022-06-01T15:43:52.168413070Z app[web.1]:     for key in sorted(fmt['provides']):
2022-06-01T15:43:52.168456583Z app[web.1]: TypeError: '<' not supported between instances of 'str' and 'NoneType'
2022-06-01T15:43:52.253078579Z app[web.1]: 2022-06-01 15:43:52,252 (WARNING) Synchronization failed: live/1.10/el/7/x86_64
LeonidVas commented 2 years ago

Possible patch

From: Leonid Vasiliev <lvasiliev@tarantool.org>
Date: Tue, 11 Jan 2022 16:17:56 +0300
Subject: [PATCH] fix something
Cc: tarantool-patches@dev.tarantool.org

---
 rpmrepo.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/rpmrepo.py b/rpmrepo.py
index a459c17..1cfd7af 100755
--- a/rpmrepo.py
+++ b/rpmrepo.py
@@ -492,6 +492,9 @@ def dump_primary(primary):

         res += '    <rpm:provides>\n'

+        print("----------------------------------------------")
+        print(fmt['provides'])
+        print("----------------------------------------------")
         for key in sorted(fmt['provides']):
             provides = fmt['provides'][key]
             entry = ['name="%s"' % provides['name']]
@@ -547,7 +550,7 @@ def parse_ver_str(ver_str):
         raise RuntimeError("Can't parse version: '%s'" % ver_str)
     epoch = match.group(1)[:-1] if match.group(1) else "0"
     ver = match.group(2)
-    rel = match.group(3)[1:] if match.group(3) else None
+    rel = match.group(3)[1:] if match.group(3) else ""
     return epoch, ver, rel

-- 
2.36.0
ylobankov commented 2 years ago

Looks like the fix works fine. I didn't see any issues with using '' instead of None. I will open a PR with this fix.

ylobankov commented 2 years ago

As I sad I didn't see any issues with using '', sync process is OK, but looking at the code I can see some logic for None values. Now I am not sure that using '' is a proper way to fix the bug because '' breaks the logic for None values. Looks like we just need to sort items in a safe way, for example, sort by the first element of the key (it is a package name, the key itself is a tuple), not by the whole key.

for key in sorted(fmt['provides'], key=lambda x: x[0]):
   ...
LeonidVas commented 2 years ago

Actually, I don't mind. But I have a few questions/thoughts: