ros-infrastructure / bloom

A release automation tool which makes releasing catkin (http://ros.org/wiki/catkin) packages easier.
Other
58 stars 93 forks source link

add meson support #691

Open christianrauch opened 1 year ago

christianrauch commented 1 year ago

This adds a new rosdebian and rosrpm template for meson. Fixes #690.

The templates are copied from the cmake templates and the rules.em was adapted for meson.

Requires: https://github.com/ros-infrastructure/bloom/pull/700

christianrauch commented 1 year ago

@wjwwood @nuclearsandwich Can you have a look at this?

christianrauch commented 1 year ago

@wjwwood @nuclearsandwich @cottsay Can you please have a look at this and let me know what is missing?

nuclearsandwich commented 1 year ago

@christianrauch thank you for patiently maintaining this branch pending review.

Unrelated to the quality of the proposed changes, one of the things that has been delaying my review is trying to process my thoughts on how we ought to be handling the introduction of new build types in bloom. Specifically, should we have new build types like this one for meson start out in some kind of "preview" state or give some other indication that they are relatively new and thus has different stability standards compared to the other bloom build types. I do want to stress that I'm considering a disclaimer like this around the potential for change not an expectation or assumption and around stability not quality.

What I want for the meson build type and those that follow it is to decouple the overall bloom stability requirements from the ones for individual build types in order to give build type contributors/maintainers increased latitude. I would like the bloom team to be able to review the integration with bloom and trust that the build type maintainers are the subject matter experts with their tools, then just "click merge" to allow iterating in subsequent releases. Right now, the thing that would help me feel comfortable moving towards that method of working would be some kind of disclaimer around the newly introduced build type.

What are your thoughts about something along those lines?

christianrauch commented 1 year ago

It makes sense to document and/or inform users that some build types are experimental and may fail. bloom could maintain a list of experimental build types and simply show a warning "\<build type> support is experimental. Please report errors to https://github.com/ros-infrastructure/bloom." when someone runs bloom to release a package.

However, I also expect users of bloom to know that meson is a new build type. ROS has always been using CMake for most of its packages and now regular python processes for python packages in ROS 2. I think that someone releasing a meson package will be aware of this novelty.

christianrauch commented 1 year ago

FYI, this PR was used to release libcamera as deb and rpm package into humble.

nuclearsandwich commented 1 year ago

FYI, this PR was used to release libcamera as deb and rpm package into humble.

I am significantly troubled that a custom branch of bloom was used to deploy to an official rosdistro without any indication. Especially because bloom reports that the released 0.11.2 version was used for https://github.com/ros/rosdistro/pull/37104

I found that https://github.com/ros/rosdistro/pull/36388#issuecomment-1454235654 references this fact. If in the future, you're using a non-standard version of bloom please call attention to it (although you do not need to cc me specifically in each case) on every review.

All the more reason to get this merged and released with an experimental tag.

christianrauch commented 1 year ago

I am significantly troubled that a custom branch of bloom was used to deploy to an official rosdistro without any indication. Especially because bloom reports that the released 0.11.2 version was used for ros/rosdistro#37104

Sorry that this comes as a surprise to you now. Since this PR is open for more than 5 months now without getting feedback after nagging people and also since my comment on the rosdep PR (https://github.com/ros/rosdistro/pull/36388#issuecomment-1454235654) went unnoticed, I assumed that there are no issues with the process. I am really not sure what I could have done more to notify people about this.

nuclearsandwich commented 1 year ago

I am really not sure what I could have done more to notify people about this.

I concur that you did everything in your power. It's presumptive of me to assume that you would wait indefinitely.

christianrauch commented 1 year ago

ping @nuclearsandwich

christianrauch commented 5 months ago

@nuclearsandwich @cottsay Can either of you have a look at this again, please?

codecov-commenter commented 1 month ago

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Please upload report for BASE (master@b42a1dd). Learn more about missing BASE report.

Files with missing lines Patch % Lines
bloom/generators/debian/generator.py 0.00% 2 Missing :warning:
bloom/generators/rpm/generator.py 0.00% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #691 +/- ## ========================================= Coverage ? 54.59% ========================================= Files ? 52 Lines ? 6367 Branches ? 1276 ========================================= Hits ? 3476 Misses ? 2534 Partials ? 357 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.