WIP: Use package-specific archive and subdir #3
Conversation
When a repo contains multiple released packages, each individual package can be downloaded as a standalone repo from the release repository. This commit prefers downloading from the release repository and downloads a package-only archive as source archive. This source archive contains (from my understanding) only one directory containing the corresponding package, so we can use this as the _dir directly. This is implemented quite hacky ATM and should serve as a PoC only.
|
I tested this on |
| if 'source' in d: | ||
| src = d['source']['url'] | ||
| elif 'release' in d: | ||
| if 'release' in d: |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| pkgname = 'ros-melodic-{}'.format(re.sub('_', '-', pkg)) | ||
| pkg_url = '/'.join([re.sub(r'\.git', '', src), | ||
| 'archive/release/melodic', |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I don't think, so, as those will be generated automatically, by the bloom release process, right?
There was a problem hiding this comment.
archive/release/melodic is specific to the release pages of ros-gbp. That's the point I was trying to make.
| 'archive/release/melodic', | ||
| pkg, | ||
| '${pkgver}.tar.gz']) | ||
| pkg_dl = '/'.join([re.sub(r'\.git', '', src), |
There was a problem hiding this comment.
Not sure if the escape backslash in the rawstring is correct, but I could be wrong.
There was a problem hiding this comment.
This should be fine, as escaping the . in the regular expression should be necessary.
| # - 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) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
This is exactly why I proposed using the release repositories instead.
And yes, commonpath would be the right function to use.
|
And please udpate |
Is this necessary, as tarfile is a standard library? |
My bad again with the |
This implements #1
When a repo contains multiple released packages, each individual package
can be downloaded as a standalone repo from the release repository.
This commit prefers downloading from the release repository and downloads
a package-only archive as source archive.
This source archive contains (from my understanding) only one directory
containing the corresponding package, so we can use this as the _dir directly.
This is implemented quite hacky ATM and should serve as a PoC only.