ros-noetic-arch / ros-noetic-desktop-full

ros-noetic-desktop-full AUR package
18 stars 2 forks source link

Split packages #2

Open stertingen opened 4 years ago

stertingen commented 4 years ago

I made a version of the ros-noetic-roscpp-core packages as split packages instead of multiple PKGBUILDs for each package. You can see it here: https://github.com/ros-noetic-arch/ros-noetic-roscpp-core/blob/split-packages/PKGBUILD

This concrete PKGBUILD creates five packages, doing the same for the ros (https://github.com/ros/ros) and ros_comm (https://github.com/ros/ros_comm) repository would reduce the maintenance effort for updating significantly.

On the downside, the new PKGBUILD does not rely on cmake only, instead, I use catkin_make_isolated to build the packages in the right order. (Since all ROS packages depend on catkin anyway, I do not mind.)

As a bonus, the PKGBUILD runs the catkin test suite to verify the build in the check() function, but to save time, we could leave that out.

Please let me hear your opinions on migrating repositories with multiple packages to split packages to reduce maintenance effort. I'd like to discuss whether and how we could introduce PKGBUILDs for split packages how a template PKGBUILD would look like.

acxz commented 4 years ago

Reducing the maintenance burden and incorporating checks is very nice. This seems like a very good idea. What do you think @bionade24 ?

stertingen commented 4 years ago

The following repositories/packages will be affected (probably non-exhaustive): (going through https://ros.org/reps/rep-0150.html)

ros_core

ros_base

robot

perception

simulators

viz

desktop

acxz commented 4 years ago

If we do go through with this change would you prefer we get it out before we sync the packages to the AUR or would you be okay if we change it after. If you think this will speed up the syncing to the AUR process than that is great, but I would rather have the packages out on the AUR before performing Nonessential fixes/cleaning. Getting it out on the AUR helps users use package managers and makes testing the arch-ROS stack much easier.

stertingen commented 4 years ago

If we do go through with this change would you prefer we get it out before we sync the packages to the AUR or would you be okay if we change it after. If you think this will speed up the syncing to the AUR process than that is great, but I would rather have the packages out on the AUR before performing Nonessential fixes/cleaning. Getting it out on the AUR helps users use package managers and makes testing the arch-ROS stack much easier.

I do think that this will speed up the porting process since we have to update less PKGBUILDs, less version numbers, less checksums to update. Also, it will be less likely to oversee a package that has a new version number for noetic.

The repositories containing single packages will be updated the old way and we might port them to using catkin_make_isolated later for the sake of consistency.

In addition to that, I do not know how good the transition from One-PKGBUILD-per-package to One-PKGBUILD-per-repo will work out once our packages are on the AUR.

acxz commented 4 years ago

will speed up the porting

I see, I still think that porting will be slower, because right now all we have to do is change melodic to noetic and sync. Boom we are on the AUR.

Before moving on with this approach let us wait for @bionade24 's response.

In addition to that, I do not know how good the transition from One-PKGBUILD-per-package to One-PKGBUILD-per-repo will work out once our packages are on the AUR.

The process is the exact same as for both cases, when you push a pkgbase to the AUR, it will also push the respective packages.

stertingen commented 4 years ago

will speed up the porting

I see, I still think that porting will be slower, because right now all we have to do is change melodic to noetic and sync. Boom we are on the AUR.

Since a lot packages release extra versions for noetic, this is not true. (seen often: melodic: 1.14.x, noetic: 1.15.x) We might s/melodic/noetic/g all packages, but then we would just ship melodic packages and declare them as noetic. So right now I'm checking if a package has a noetic version and edit the PKGBUILD accordingly.

acxz commented 4 years ago

Yes, my original plan was to just update the packages once they are already out on the AUR, so they are easier to work with.

bionade24 commented 4 years ago

You could use the update command out of my toolchain https://github.com/bionade24/aur-ci (I know name ist unfitting and I also refactored a lot of the shit but haven't merged it yet), change the settings to melodic and it should update the PKGBUILD if noetic has a newer version.

bionade24 commented 4 years ago

Or wait I'll merge and push soon the you can configure the gh organization and the ROS version in a *.ini file

acxz commented 4 years ago

Yep after going through the syncing process and updating some packages, I find this idea much more appealing. I'll let you take the lead on this @stertingen

While we can use bionade24's solution for now, I think what stertigen has proposed is a cleaner solution overall. Not necessary but it is nice and if it doesn't break anything feel free to merge your changes in, @stertingen

bionade24 commented 4 years ago

Sorry for forgetting this, I've merged and pushed now what I changed in between. I also like @stertingen 's idea because it's more the Arch way , but didn't want to change this when I overtook melodic. Sorry if that we uploaded that mess, somehow I had some problems with deletion via the API, but maybe it just bc PyGithub isn't maintained well and it's better to use the JSON API directly. Or I just was too dumb to set the API token rights correctly, even though I enabled nearly everything at my last try.

acxz commented 4 years ago

no worries, it was weird that some packages were not populated, but the ones necessary for ros-noetic-desktop-full are all there now. Thx for the initial porting from melodic to noetic.

acxz commented 2 years ago

@stertingen just want to follow up with this. I found a case where a PKGBUILD that has this structure actually causes essentially a circular dependency. See: https://github.com/ros-noetic-arch/ros-noetic-panda-moveit-config/issues/3 and https://github.com/ros-noetic-arch/ros-noetic-franka-ros/issues/2.

Even though franka-ros does not depend on panda-moveit-config, since franka-description which is in the franka-ros PKGBUILD is a dependency of panda-moveit-config and franka-example-controller which is also in the franka-ros PKGBUILD is dependent on panda-moveit-config, it causes a circ dep, where I can't makepkg the franka-ros PKGBUILD.

Do you have any ideas on how to work around this?

stertingen commented 2 years ago

I see two possible ways to handle this:

1. Split up

I would split the set of packages into multiple PKGBUILD files, so we have some sets of packages A1, A2 and B. Packages in B depend on A1, Packages on A2 depend on A1 and B. This is the current status quo, I believe, with franka-description in set A1.

This is the more elegant approach, having the feature split packages as a performance opt-in when appropriate and falling back to one PKGBUILD per package when needed.

I would somehow ensure that only the relevant packages are built during makepkg for performance.

2. Unify

Throw all packages (franka-ros and panda-moveit-config) into the build tree and let the build system (catkin) sort the dependencies out. This is more the sledgehammer approach, but might be faster when building all packages together.

This might not be as bad as it sounds because it seems to me like you want all of these packages installed anyway.

Notes

IMHO this is a design flaw that should be reported and fixed upstream, either by splitting or unifying the repositories.

Apparently, panda-moveit-config was split out at some point in time for faster (?) adoption to MoveIt! changes. (https://github.com/ros-planning/panda_moveit_config/blob/melodic-devel/README.md)

The Panda robot is the flagship MoveIt! integration robot used in the MoveIt! tutorials. Any changes to MoveIt! need to be propagated into this config fast, so this package is co-located under the ros-planning Github organization here.