Add 'aur' package loader
Details
- Reviewers
vlorentz - Group Reviewers
Reviewers - Commits
- rDLDBASE9493891b4888: Aur: Implements basic loader
Diff Detail
- Repository
- rDLDBASE Generic VCS/Package Loader
- Branch
- aur
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 30753 Build 48085: Phabricator diff pipeline on jenkins Jenkins console · Jenkins Build 48084: arc lint + arc unit
Event Timeline
Build is green
Patch application report for D8174 (id=29524)
Rebasing onto 0913e11f69...
First, rewinding head to replay your work on top of it... Applying: WIP: Aur: Implements basic loader
Changes applied before test
commit 53019e6cc463dded390936ff95d67b6a3ff4e84e Author: Franck Bret <franck.bret@octobus.net> Date: Wed Aug 3 16:16:02 2022 +0200 WIP: Aur: Implements basic loader Add 'aur' package loader
See https://jenkins.softwareheritage.org/job/DLDBASE/job/tests-on-diff/816/ for more details.
Looks good overall.
can pkgdesc contain multiple lines? If yes, please add a test
swh/loader/package/aur/loader.py | ||
---|---|---|
47–49 | can open as text if we always decode it anyway | |
48 | according to https://wiki.archlinux.org/title/.SRCINFO , this isn't always true | |
52–60 | this is more pythonic unrelatedly, wouldn't it be simpler to make data of type Dict[str, List[str]], so the type check can be skipped? It would replace this: if k in data: if type(data[k]) is not list: data[k] = [data[k]] data[k].append(v) else: data[k] = v with this: data.setdefault(k, []).append(v) and make the output format simpler by having consistent value types. | |
91 | ||
swh/loader/package/aur/tests/test_aur.py | ||
177–181 | What exception should this capture? It looks like loader.load() so neither assertion actually runs |
swh/loader/package/aur/loader.py | ||
---|---|---|
48 | Correct
| |
52–60 | The problem with the "setdefault" implementation is that it will turns everything as a list. Here is a print of the "PARSED" set of data, then "DATA" for current implementation and "DATA1" for the 'setdefault' one: 'PARSED' [('pkgbase', 'hg-evolve'), ('pkgdesc', 'Flexible evolution of Mercurial history'), ('pkgver', '10.5.2'), ('pkgrel', '1'), ('url', 'https://www.mercurial-scm.org/doc/evolution/'), ('arch', 'any'), ('license', 'GPL2'), ('makedepends', 'python-build'), ('makedepends', 'python-installer'), ('makedepends', 'python-wheel'), ('depends', 'mercurial'), ('source', 'https://files.pythonhosted.org/packages/source/h/hg-evolve/hg-evolve-10.5.2.tar.gz'), ('sha512sums', '81a1cc1202ffaf364fde70c6a36e32330e93aa69c9b9f7e11fbc11f988f7fb302d8b79414c644d274fedb7f0a67e10c4344c0206a1424f2bb97ae2cb11a51315'), ('pkgname', 'hg-evolve')] 'DATA' {'arch': 'any', 'depends': 'mercurial', 'license': 'GPL2', 'makedepends': ['python-build', 'python-installer', 'python-wheel'], 'pkgbase': 'hg-evolve', 'pkgdesc': 'Flexible evolution of Mercurial history', 'pkgname': 'hg-evolve', 'pkgrel': '1', 'pkgver': '10.5.2', 'sha512sums': '81a1cc1202ffaf364fde70c6a36e32330e93aa69c9b9f7e11fbc11f988f7fb302d8b79414c644d274fedb7f0a67e10c4344c0206a1424f2bb97ae2cb11a51315', 'source': 'https://files.pythonhosted.org/packages/source/h/hg-evolve/hg-evolve-10.5.2.tar.gz', 'url': 'https://www.mercurial-scm.org/doc/evolution/'} 'DATA1' {'arch': ['any'], 'depends': ['mercurial'], 'license': ['GPL2'], 'makedepends': ['python-build', 'python-installer', 'python-wheel'], 'pkgbase': ['hg-evolve'], 'pkgdesc': ['Flexible evolution of Mercurial history'], 'pkgname': ['hg-evolve'], 'pkgrel': ['1'], 'pkgver': ['10.5.2'], 'sha512sums': ['81a1cc1202ffaf364fde70c6a36e32330e93aa69c9b9f7e11fbc11f988f7fb302d8b79414c644d274fedb7f0a67e10c4344c0206a1424f2bb97ae2cb11a51315' ], 'source': ['https://files.pythonhosted.org/packages/source/h/hg-evolve/hg-evolve-10.5.2.tar.gz'], 'url': ['https://www.mercurial-scm.org/doc/evolution/']} Unless you ave a better idea will stick with the 'more pythonic' one | |
swh/loader/package/aur/tests/test_aur.py | ||
177–181 | Here I want to assert a status="failed" and a snapshot=None. Comparing with test_nixguix_url_not_found from nixguix loader tests if I run something like: load_status = loader.load() assert load_status == {"status": "failed"} I'm surprised about the result: > assert load_status == {"status": "failed"} E AssertionError: assert {'snapshot_id': '1a8893e6a86f444e8be8e7bda6cb34fb1735a00e',\n 'status': 'uneventful'} == {'status': 'failed'} E Differing items: E {'status': 'uneventful'} != {'status': 'failed'} E Left contains 1 more item: E {'snapshot_id': '1a8893e6a86f444e8be8e7bda6cb34fb1735a00e'} E Full diff: E { E - 'status': 'failed', E + 'snapshot_id': '1a8893e6a86f444e8be8e7bda6cb34fb1735a00e', E + 'status': 'uneventful', E } Why did it create a snapshot ? What should be the correct to way to test that? |
Build is green
Patch application report for D8174 (id=29699)
Rebasing onto 43597c4806...
First, rewinding head to replay your work on top of it... Applying: Aur: Implements basic loader
Changes applied before test
commit 2f7e81dfd80e313bf906aba983de6bd748c92632 Author: Franck Bret <franck.bret@octobus.net> Date: Wed Aug 3 16:16:02 2022 +0200 Aur: Implements basic loader Add 'aur' package loader
See https://jenkins.softwareheritage.org/job/DLDBASE/job/tests-on-diff/823/ for more details.
swh/loader/package/aur/loader.py | ||
---|---|---|
52–60 | I prefer "always a list" to "either a single element or a list with >= 2 elements", it's easier to deal with; because in the latter case you need to always check the type before using it. | |
swh/loader/package/aur/tests/test_aur.py | ||
177–181 | "failed" means the loader could not get anything, so there is no snapshot along with that status code. If the loader managed to produce a snapshot despite errors, it should return the "partial" status. |
swh/loader/package/aur/tests/test_aur.py | ||
---|---|---|
177–181 | although "uneventful" means everything went fine and there is no change; so it looks like it there was either no error (problem with the test) or it ignored it (problem in error handling in the loader) |
swh/loader/package/aur/loader.py | ||
---|---|---|
48 | It looks like simply stripping means the loader ignores the structure of the file, by merging the entries of all packages declared in the file together as if there was a single package in the whole file. |
swh/loader/package/aur/tests/test_aur.py | ||
---|---|---|
21 | yes |
swh/loader/package/aur/loader.py | ||
---|---|---|
48 | ok, there can be multiple package referenced in an .SRCINFO. From there my question is at which point that intrinsic metadata is important to get collected for a loader? And If yes what are the interesting attributes and how is the release object supposed to ingest or link to them? Regarding original comment about SRCINFO structure. The common case is one pkgbase, one pkgname. Another case is split package (one pkgbase, multiple pkgname). Here there must be one of the pkgname equal to pkgbase, and at least one different pkgname. To get the pkgdesc for this one, I must get it from the pkgname block not from the pkgbase one like in common case. Please note that the lister use 'pkgname' as package name, not 'pkgbase'. I've found kodi-stable-git as an example. Here is its .SRCINFO : pkgbase = kodi-stable-git pkgver = r57686.186f2f8614e pkgrel = 1 url = https://kodi.tv arch = x86_64 license = GPL2 makedepends = afpfs-ng makedepends = bluez-libs makedepends = cmake makedepends = curl makedepends = dav1d makedepends = doxygen makedepends = git makedepends = glew makedepends = gperf makedepends = hicolor-icon-theme makedepends = java-runtime makedepends = libaacs makedepends = libass makedepends = libbluray makedepends = libcdio makedepends = libcec makedepends = libgl makedepends = mariadb-libs makedepends = libmicrohttpd makedepends = libmodplug makedepends = libmpeg2 makedepends = libnfs makedepends = libplist makedepends = libpulse makedepends = libva makedepends = libva-vdpau-driver makedepends = libxrandr makedepends = libxslt makedepends = lirc makedepends = lzo makedepends = mesa makedepends = nasm makedepends = python-pycryptodomex makedepends = python-pillow makedepends = python-pybluez makedepends = python-simplejson makedepends = shairplay makedepends = smbclient makedepends = taglib makedepends = tinyxml makedepends = swig makedepends = upower makedepends = giflib makedepends = rapidjson makedepends = ghostscript makedepends = meson makedepends = gtest makedepends = graphviz makedepends = wayland-protocols makedepends = waylandpp makedepends = libxkbcommon makedepends = libinput noextract = libdvdcss-1.4.2-Leia-Beta-5.tar.gz noextract = libdvdnav-6.0.0-Leia-Alpha-3.tar.gz noextract = libdvdread-6.0.0-Leia-Alpha-3.tar.gz noextract = ffmpeg-4.3.2-Matrix-19.2.tar.gz noextract = fmt-6.1.2.tar.gz noextract = spdlog-1.5.0.tar.gz noextract = crossguid-8f399e8bd4.tar.gz noextract = fstrcmp-0.7.D001.tar.gz noextract = flatbuffers-1.12.0.tar.gz noextract = libudfread-1.1.0.tar.gz options = !lto source = git+https://github.com/xbmc/xbmc.git#branch=Matrix source = libdvdcss-1.4.2-Leia-Beta-5.tar.gz::https://github.com/xbmc/libdvdcss/archive/1.4.2-Leia-Beta-5.tar.gz source = libdvdnav-6.0.0-Leia-Alpha-3.tar.gz::https://github.com/xbmc/libdvdnav/archive/6.0.0-Leia-Alpha-3.tar.gz source = libdvdread-6.0.0-Leia-Alpha-3.tar.gz::https://github.com/xbmc/libdvdread/archive/6.0.0-Leia-Alpha-3.tar.gz source = ffmpeg-4.3.2-Matrix-19.2.tar.gz::https://github.com/xbmc/FFmpeg/archive/4.3.2-Matrix-19.2.tar.gz source = http://mirrors.kodi.tv/build-deps/sources/fmt-6.1.2.tar.gz source = http://mirrors.kodi.tv/build-deps/sources/spdlog-1.5.0.tar.gz source = http://mirrors.kodi.tv/build-deps/sources/crossguid-8f399e8bd4.tar.gz source = http://mirrors.kodi.tv/build-deps/sources/fstrcmp-0.7.D001.tar.gz source = http://mirrors.kodi.tv/build-deps/sources/flatbuffers-1.12.0.tar.gz source = http://mirrors.kodi.tv/build-deps/sources/libudfread-1.1.0.tar.gz source = cheat-sse-build.patch source = build-fix-for-dav1d-1.0.0.patch source = 0001-add-dav1d-patch-to-build-system.patch b2sums = SKIP b2sums = 283aa2cec0a2200d3569bc280cb9659e9224a6b3a77db8a35b269cd8caf1337ac9d8b92b806df66f63ef7458a46bd6261f0b8b14678b10e26644a79dcbeea5da b2sums = 7573434a0ae8e8ccabf48173f81fcde29074eb138e119a2ae9156cde3c3d8bfd716f5d0e605b97f2dcac21f570781137c8533c5ae306b51e3905822fda318355 b2sums = 0c206acdaf0776841ab792c74e023af07d9539eb72e03ae164382a31ed950f60e5e15f1d055979d28f1398924471b294d11f064b11b8373353b3962a3777ff3c b2sums = 60299be16d73fb689b698f5b322d9cd38093894222070d2f1c28a37477a338405938959440a3f5646d946f3c2287072fdfa582b644a36f3a23e7d637b51113fc b2sums = 36e7451a8732c62dcbf47e6d287ea582827b6196a468b8648803ea1bc9a37a5f681d87488f748d749183d97783ac7fb47a3f2aeed64fc6a684f9ee85b67ae28d b2sums = bac6c6650f8347458dd2dd66f318b43a769b0896d68f6a6f1310754527a69feaa52b2f6f48d67c7e811c2dafa5d3863a9a07c738df8c12abed2718fb06254b28 b2sums = e6f1f495adf541102e3b5ac11dfd14b770a52e23ef9d613bc6204f6493ff4df4da9ba290ad6c3a7e5c7fcf159cafdf355bfe668a4ddceb4329df934c65966d19 b2sums = a8b68fcb8613f0d30e5ff7b862b37408472162585ca71cdff328e3299ff50476fd265467bbd77b352b22bb88c590969044f74d91c5468475504568fd269fa69e b2sums = 441123be124ad851efa30bda0d828a764ebaf79ba6692a6e5904000b33818e9de78c3a964037ac93ef562890980c58169141e55354dce86857c02bcd917150d6 b2sums = e7fab72ebecb372c54af77b4907e53f77a5503af66e129bd2083ef7f4209ebfbed163ffd552e32b7181829664fff6ab82a1cdf00c81dc6f3cc6bfc8fa7242f6e b2sums = 6d647177380c619529fb875374ec46f1fff6273be1550f056c18cb96e0dea8055272b47664bb18cdc964496a3e9007fda435e67c4f1cee6375a80c048ae83dd0 b2sums = 6928d0fb1f4cb2609dee87c7078e02cecc37ddef263485b47be0ae5c281be67b403b39c95ea370c6b6667e1eceb1c7e6fb83ec9b04acd0bdbe4abec17fb84385 b2sums = f1769867f7bb998e9705cfe7709072436bc4824775ad6ccd151bb240798413a92c4a5c92452f5b781a439660a3d9a0846ebf6fefebdfa50b95f076ffdc6ff55e pkgname = kodi-stable-git pkgdesc = A software media player and entertainment hub for digital media (Matrix branch) depends = bluez-libs depends = curl depends = dav1d depends = desktop-file-utils depends = hicolor-icon-theme depends = lcms2 depends = libass depends = libbluray depends = libcdio depends = libcec depends = libmicrohttpd depends = libnfs depends = libplist depends = libpulse depends = libva depends = libvdpau depends = libxslt depends = lirc depends = mariadb-libs depends = mesa depends = python-pillow depends = python-pycryptodomex depends = python-simplejson depends = shairplay depends = smbclient depends = sqlite depends = taglib depends = tinyxml depends = libxrandr depends = libxkbcommon depends = waylandpp depends = libinput optdepends = afpfs-ng: Apple shares support optdepends = bluez: Blutooth support optdepends = python-pybluez: Bluetooth support optdepends = pulseaudio: PulseAudio support optdepends = upower: Display battery level provides = kodi=r57686.186f2f8614e provides = kodi-x11 provides = kodi-wayland provides = kodi-gbm conflicts = kodi conflicts = kodi-x11 conflicts = kodi-wayland conflicts = kodi-gbm conflicts = kodi-matrix-git replaces = kodi replaces = kodi-x11 replaces = kodi-wayland replaces = kodi-gbm replaces = kodi-matrix-git pkgname = kodi-stable-git-eventclients pkgdesc = Kodi Event Clients (Matrix branch) optdepends = kodi: local machine eventclient use optdepends = python: most eventclients are implemented in python provides = kodi-eventclients=r57686.186f2f8614e conflicts = kodi-eventclients pkgname = kodi-stable-git-tools-texturepacker pkgdesc = Kodi Texturepacker tool (Matrix branch) depends = libpng depends = giflib depends = libjpeg-turbo depends = lzo provides = kodi-tools-texturepacker=r57686.186f2f8614e conflicts = kodi-tools-texturepacker pkgname = kodi-stable-git-dev pkgdesc = Kodi dev files (Matrix branch) depends = kodi-stable-git provides = kodi-dev=r57686.186f2f8614e conflicts = kodi-dev The lister origin discovery data is : {"ID":1080455,"Name":"kodi-stable-git","PackageBaseID":174003,"PackageBase":"kodi-stable-git","Version":"r57686.186f2f8614e-1","Description":"A software media player and entertainment hub for digital media (Matrix branch)","URL":"https://kodi.tv","NumVotes":1,"Popularity":0.001125,"OutOfDate":null,"Maintainer":"graysky","FirstSubmitted":1639570648,"LastModified":1652616136,"URLPath":"/cgit/aur.git/snapshot/kodi-stable-git.tar.gz"}, But lister list also the split packages: {"ID":1080456,"Name":"kodi-stable-git-eventclients","PackageBaseID":174003,"PackageBase":"kodi-stable-git","Version":"r57686.186f2f8614e-1","Description":"Kodi Event Clients (Matrix branch)","URL":"https://kodi.tv","NumVotes":1,"Popularity":0.001125,"OutOfDate":null,"Maintainer":"graysky","FirstSubmitted":1639570648,"LastModified":1652616136,"URLPath":"/cgit/aur.git/snapshot/kodi-stable-git-eventclients.tar.gz"}, {"ID":1080457,"Name":"kodi-stable-git-tools-texturepacker","PackageBaseID":174003,"PackageBase":"kodi-stable-git","Version":"r57686.186f2f8614e-1","Description":"Kodi Texturepacker tool (Matrix branch)","URL":"https://kodi.tv","NumVotes":1,"Popularity":0.001125,"OutOfDate":null,"Maintainer":"graysky","FirstSubmitted":1639570648,"LastModified":1652616136,"URLPath":"/cgit/aur.git/snapshot/kodi-stable-git-tools-texturepacker.tar.gz"}, {"ID":1080458,"Name":"kodi-stable-git-dev","PackageBaseID":174003,"PackageBase":"kodi-stable-git","Version":"r57686.186f2f8614e-1","Description":"Kodi dev files (Matrix branch)","URL":"https://kodi.tv","NumVotes":1,"Popularity":0.001125,"OutOfDate":null,"Maintainer":"graysky","FirstSubmitted":1639570648,"LastModified":1652616136,"URLPath":"/cgit/aur.git/snapshot/kodi-stable-git-dev.tar.gz"}, {"ID":1076202,"Name":"kodi-standalone-service","PackageBaseID":87195,"PackageBase":"kodi-standalone-service","Version":"1.135-1","Description":"Systemd services to run kodi in stand-alone mode without a DE","URL":"https://github.com/graysky2/kodi-standalone-service","NumVotes":64,"Popularity":0.012128,"OutOfDate":null,"Maintainer":"graysky","FirstSubmitted":1415219129,"LastModified":1651774385,"URLPath":"/cgit/aur.git/snapshot/kodi-standalone-service.tar.gz"}, So in all case all of the package of a split package are referenced by the lister and will trigger a loader for it. If there is an interest in representing those split package related to a pkg base for a loader, what should be the structure of the release object? |
swh/loader/package/aur/loader.py | ||
---|---|---|
48 | If I understand this correctly, pkgbase sections are equivalent to Debian's source packages, and pkgname to Debian's binary packages; therefore we are only interested in the former. Therefore, it is fine to stop parsing .SRCINFO when reaching the first pkgname. |
swh/loader/package/aur/loader.py | ||
---|---|---|
48 | I think pkgbase is used only to represents something like a metapackage, a group of packages. The lister list packages like https://aur.archlinux.org/packages do. Each line of the json we parse corresponds to a unique package name. Look at the difference between a regular package and split package SRCINFO is generated from PKGBUILD and pkgbase is not required like pkgname is.
Check https://wiki.archlinux.org/title/PKGBUILD#Package_name and https://man.archlinux.org/man/PKGBUILD.5#PACKAGE_SPLITTING So I think using pkgname as the reference is the way to go. Are you ok with this? Also in the specific case of "aur" forge what we get as package (snapshot download) is always a tar.gz with two files, .SRCINFO and PKGBUILD. We don't download any binaries or software source code other than those two recipe files. |
swh/loader/package/aur/loader.py | ||
---|---|---|
48 | according to https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=kodi-stable-git it seems all packages defined by this PKGBUILD are compiled from the same source (by the build()) function, and the split happens with an install step to separate directories (the package_kodi-stable-git(), package_kodi-stable-git-eventclients(), ... functions) |
Build is green
Patch application report for D8174 (id=29793)
Rebasing onto 43597c4806...
First, rewinding head to replay your work on top of it... Applying: Aur: Implements basic loader
Changes applied before test
commit 417a10b9530c17710ac97cb3012ff2cfd79cda0e Author: Franck Bret <franck.bret@octobus.net> Date: Wed Aug 3 16:16:02 2022 +0200 Aur: Implements basic loader Add 'aur' package loader
See https://jenkins.softwareheritage.org/job/DLDBASE/job/tests-on-diff/824/ for more details.
Build is green
Patch application report for D8174 (id=29803)
Rebasing onto 43597c4806...
First, rewinding head to replay your work on top of it... Applying: Aur: Implements basic loader
Changes applied before test
commit 58ca3f307608d2ff424b28ba2525e4538ebe9d8d Author: Franck Bret <franck.bret@octobus.net> Date: Wed Aug 3 16:16:02 2022 +0200 Aur: Implements basic loader Add 'aur' package loader
See https://jenkins.softwareheritage.org/job/DLDBASE/job/tests-on-diff/825/ for more details.
swh/loader/package/aur/loader.py | ||
---|---|---|
48 | What should I conclude from this? | |
swh/loader/package/aur/tests/test_aur.py | ||
177–181 | I've put a breakpoint on package/loader.py on line 713. At this point I have: ipdb> load_exceptions [ValueError("Fail to query 'https://myforge.nowhere/42/42.tar.gz'. Reason: 404")] ipdb> status_visit 'partial' ipdb> tmp_releases {'0.0.1': []} Looks like tmp_releases is populated with a version number but no branches (as expected we test for 404 not found url). |
swh/loader/package/aur/tests/test_aur.py | ||
---|---|---|
177–181 | Hmm, looks like this edge case was not considered. if not tmp_releases should probably be replaced with if not any(tmp_releases.values()). Anyway, that's out of scope for this diff and you removed with pytest.raises so there is nothing left to do here, right? |
swh/loader/package/aur/tests/test_aur.py | ||
---|---|---|
177–181 | yep |
Build is green
Patch application report for D8174 (id=29815)
Rebasing onto 43597c4806...
First, rewinding head to replay your work on top of it... Applying: Aur: Implements basic loader
Changes applied before test
commit 5ee30a3fad39dad3474a5129ba667d792f114660 Author: Franck Bret <franck.bret@octobus.net> Date: Wed Aug 3 16:16:02 2022 +0200 Aur: Implements basic loader Add 'aur' package loader
See https://jenkins.softwareheritage.org/job/DLDBASE/job/tests-on-diff/826/ for more details.
Build is green
Patch application report for D8174 (id=29848)
Rebasing onto 43597c4806...
Current branch diff-target is up to date.
Changes applied before test
commit 9493891b4888876bfa799a7d1b46e6cdd296a97b Author: Franck Bret <franck.bret@octobus.net> Date: Wed Aug 3 16:16:02 2022 +0200 Aur: Implements basic loader Add 'aur' package loader
See https://jenkins.softwareheritage.org/job/DLDBASE/job/tests-on-diff/830/ for more details.