Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Use package-specific archive and subdir #3

Closed
wants to merge 1 commit into from
Closed

WIP: Use package-specific archive and subdir #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions aurci/general.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ def build_metainfo_dict():
for repo in rosdistro:
#Go through distro, and make entry for each package in a repository
d = rosdistro[repo]
if 'source' in d:
src = d['source']['url']
elif 'release' in d:
if 'release' in d:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that the release repos are always heavily outdated, a lot of fixes take longer and that's why we use the upstream dev repos. See this thread ros-arch/ros-package-tools#1 or many others why ros-gbp is bad to use for Arch.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why should the release repositories be outdated? Outdated relative to what?

From my understanding, the intention is to mirror the state of the ubuntu binary packages, so the versions available on AUR match the versions released to the rosdistro as noted in the distribution.yaml. As the release repositories are only managed automatically by the bloom release process, they should reflect the state being released to the respective ROS distribution.

I completely share the points raised in this post, but I also see the problem with a delayed distribution.yaml. However, this will probably be an issue for the time that the distribution syncs are done (which from my understanding is done once a month). I agree that this isn't optimal, but it would keep things much closer to the automated workflow. However, with the quite large list of outdated ROS packages in AUR (and not only for melodic because of a failing CI, but also noetic) this might be a sane way to go.

That being said, I also agree on this post as a strong argument against using the release repositories.

In the end, this is only a suggestion and I would be fine not implementing this any further if the disadvantages outweigh the advantages of my proposal.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And, as mentioned by @stertingen a newer release version could be fetched from the tracks.yaml file in order to circumvent the cached distribution problems. However, this might lead to failing builds, as those versions are not necessarily passing the buildfarm CI.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I completely share the points raised in this post, but I also see the problem with a delayed distribution.yaml. However, this will probably be an issue for the time that the distribution syncs are done (which from my understanding is done once a month). I agree that this isn't optimal, but it would keep things much closer to the automated workflow. However, with the quite large list of outdated ROS packages in AUR (and not only for melodic because of a failing CI, but also noetic) this might be a sane way to go.

One thing why we originally added CI was because the releases actually only work for Ubuntu/Debian and we had to find out where we would need new releases to get things working without patching everything on Arch. And the CI failed a lot because of the old releases in the melodic sync, sometimes thing even didn't went into sync. I'm working (or already finished but don't have time to test) on a big CI improvement to actually make package building more sane and clear. We started the CI/CD in first line bc of all the comments in about things not working on machines that didn't had certain deps or even weirder config stuff and it improved a lot faster bc of the CI, even though it doesn't look like that currently. You have valid reasons for ros-gbp as they existed always, but we changed from them to the upstream releases bc ros-gbp is made for Ubuntu and even there often requires lots of patches to work.

src = d['release']['url']
elif 'source' in d:
src = d['source']['url']
target = re.sub(r'\.git', '', src.split('/')[3] + '/' + src.split('/')[4])
pkgver = d.get('release', {'version': None}).get('version', None)
if pkgver:
Expand All @@ -49,9 +49,19 @@ def build_metainfo_dict():
pkg_list = d.get('release', {'packages': [repo]}).get('packages', [repo])
for pkg in pkg_list:
siblings = len(pkg_list)-1
# TODO: Replace hard-coded melodic
pkgname = 'ros-melodic-{}'.format(re.sub('_', '-', pkg))
pkg_url = '/'.join([re.sub(r'\.git', '', src),
'archive/release/melodic',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can only guess the url paths on ros-gbp, so that's a bad idea. IMHO a specific instead of a general url generation always will be.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think, so, as those will be generated automatically, by the bloom release process, right?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

archive/release/melodic is specific to the release pages of ros-gbp. That's the point I was trying to make.

pkg,
'${pkgver}.tar.gz'])
pkg_dl = '/'.join([re.sub(r'\.git', '', src),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if the escape backslash in the rawstring is correct, but I could be wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be fine, as escaping the . in the regular expression should be necessary.

'archive/release/melodic',
pkg,
str(pkgver)]) + '.tar.gz'
ros_dict[pkgname] = {'repo': repo, 'siblings': siblings, 'orig_name': pkg,
'pkgname': pkgname, 'src': src, 'pkgver': pkgver, 'dl': dl, 'url': url}
'pkgname': pkgname, 'src': src, 'pkgver': pkgver, 'dl': pkg_dl,
'url': pkg_url}
return ros_dict

@staticmethod
Expand Down
13 changes: 11 additions & 2 deletions aurci/update.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import sys
import re
import subprocess
import tarfile
import urllib


Expand Down Expand Up @@ -43,8 +44,6 @@ def update_pkgbuild(self):
Maybe diffrent quotes than needed for regex?")

new_pkgver = "pkgver='{}'".format(self.package_info['pkgver'])
new_dir = '_dir="{}-${{pkgver}}/{}"'.format(self.package_info['repo'],
'{}'.format(self.get_nested_package_path()) if self.package_info['siblings'] else '')
new_src = 'source=("${{pkgname}}-${{pkgver}}.tar.gz"::"{}"'.format(self.package_info['url'])

if old_pkgver == new_pkgver and old_dir == new_dir and old_src == new_src:
Expand All @@ -61,6 +60,16 @@ def update_pkgbuild(self):

sha256 = subprocess.run(['sha256sum', fname], check=True, capture_output=True)
new_sha = "sha256sums=('{}'".format(sha256.stdout.decode('utf-8').split(' ')[0])

# TODO:
# - Error checking
# - Probably use with ...: notation
# - Check, whether main_dir is only one string (should be)
archive = tarfile.open(fname, "r:gz")
main_dir = os.path.commonprefix(archive.getnames())
new_dir = '_dir="{}"'.format(main_dir)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't work in (source of course) repos like ros_comm, as the packages are internally grouped into several folders. Additionally, os.path.commonprefix() is generally bad, because it returns the string prefix and I guess it's a bug in here, too. I guess os.path.commonpath() would be the correct one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is exactly why I proposed using the release repositories instead.

And yes, commonpath would be the right function to use.

archive.close()

os.remove(fname)

with open('PKGBUILD', 'r') as f:
Expand Down