-
Notifications
You must be signed in to change notification settings - Fork 33
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.
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
Various improvements to support generating recipes for Noetic #280
Various improvements to support generating recipes for Noetic #280
Conversation
eead3ab
to
e0b59c0
Compare
@allenh1 @herb-kuta-lge Please review and merge |
1806769
to
b46d2ab
Compare
b46d2ab
to
17c2e11
Compare
17c2e11
to
b5b1ac3
Compare
1995657
to
f704598
Compare
* otherwise it won't be parsed correctly by superflore as explained in: "bitbake: use own condition_context instead of whole os.environ" commit from: ros-infrastructure/superflore#280 Signed-off-by: Martin Jansa <martin.jansa@lge.com>
…5146 Regenerate with superflore modified to better parse <export><build_type> elements as implemented in: ros-infrastructure/superflore#280 Recipes generated by superflore for all packages in ROS distribution melodic. This pull request was generated by running the following command: superflore-gen-oe-recipes --dry-run --no-branch --ros-distro melodic --output-repository-path . --upstream-branch HEAD Signed-off-by: Martin Jansa <martin.jansa@lge.com>
…3600 Regenerate with superflore modified to better parse <export><build_type> elements as implemented in: ros-infrastructure/superflore#280 Recipes generated by superflore for all packages in ROS distribution dashing. This pull request was generated by running the following command: superflore-gen-oe-recipes --dry-run --no-branch --ros-distro dashing --output-repository-path . --upstream-branch HEAD Signed-off-by: Martin Jansa <martin.jansa@lge.com>
…171906 Regenerate with superflore modified to better parse <export><build_type> elements as implemented in: ros-infrastructure/superflore#280 Recipes generated by superflore for all packages in ROS distribution eloquent. This pull request was generated by running the following command: superflore-gen-oe-recipes --dry-run --no-branch --ros-distro eloquent --output-repository-path . --upstream-branch HEAD Signed-off-by: Martin Jansa <martin.jansa@lge.com>
Regenerate with superflore modified to better parse <export><build_type> elements as implemented in: ros-infrastructure/superflore#280 Recipes generated by superflore for all packages in ROS distribution foxy. This pull request was generated by running the following command: superflore-gen-oe-recipes --dry-run --no-branch --ros-distro foxy --output-repository-path . --upstream-branch HEAD Signed-off-by: Martin Jansa <martin.jansa@lge.com>
…5327 Regenerate with superflore modified to better parse <export><build_type> elements as implemented in: ros-infrastructure/superflore#280 Recipes generated by superflore for all packages in ROS distribution rolling. This pull request was generated by running the following command: superflore-gen-oe-recipes --dry-run --no-branch --ros-distro rolling --output-repository-path . --upstream-branch HEAD Signed-off-by: Martin Jansa <martin.jansa@lge.com>
…5146 Regenerate with superflore modified to even better parse <export><build_type> elements and handling ROS_UNRESOLVED_PLATFORM_PKG_* variables as implemented in: ros-infrastructure/superflore#280 Recipes generated by superflore for all packages in ROS distribution melodic. This pull request was generated by running the following command: superflore-gen-oe-recipes --dry-run --no-branch --ros-distro melodic --output-repository-path . --upstream-branch HEAD Signed-off-by: Martin Jansa <martin.jansa@lge.com>
…5146 Regenerate with superflore from: ros-infrastructure/superflore#280 and rosdistro/rosdep changes from: ros/rosdistro#27623 Recipes generated by superflore for all packages in ROS distribution melodic. This pull request was generated by running the following command: superflore-gen-oe-recipes --dry-run --no-branch --ros-distro melodic --output-repository-path . --upstream-branch HEAD Signed-off-by: Martin Jansa <martin.jansa@lge.com>
…3600 Regenerate with superflore modified to even better parse <export><build_type> elements and handling ROS_UNRESOLVED_PLATFORM_PKG_* variables as implemented in: ros-infrastructure/superflore#280 Recipes generated by superflore for all packages in ROS distribution dashing. This pull request was generated by running the following command: superflore-gen-oe-recipes --dry-run --no-branch --ros-distro dashing --output-repository-path . --upstream-branch HEAD Signed-off-by: Martin Jansa <martin.jansa@lge.com>
…3600 Regenerate with superflore from: ros-infrastructure/superflore#280 and rosdistro/rosdep changes from: ros/rosdistro#27623 Recipes generated by superflore for all packages in ROS distribution dashing. This pull request was generated by running the following command: superflore-gen-oe-recipes --dry-run --no-branch --ros-distro dashing --output-repository-path . --upstream-branch HEAD Signed-off-by: Martin Jansa <martin.jansa@lge.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@herb-kuta-lge please take a look at this one when you can |
f704598
to
0a0d9d5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've unresolved 3 of the conversations.
Please DON'T SQUASH the commits when merging, it will be impossible to review and understand the commits after they are squashed. |
0a0d9d5
to
926aee2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unresolved 1 conversation.
* noetic should use 3 Signed-off-by: Martin Jansa <martin.jansa@lge.com>
* when regenerating the recipes I often see change like this: suitesparse: -- suitesparse-cholmod@meta-ros - suitesparse-cxsparse@meta-ros +- suitesparse-cholmod@meta-ros * because the order of the list is not defined Signed-off-by: Martin Jansa <martin.jansa@lge.com>
* it will be used for creating context for conditional evaluations in rosdistro as well Signed-off-by: Martin Jansa <martin.jansa@lge.com>
* Use only ROS_OS_OVERRIDE, ROS_DISTRO, ROS_PYTHON_VERSION, ROS_VERSION variables with ROS_PYTHON_VERSION, ROS_VERSION derived from distro value instead of depending on ros-generate-recipes.sh script from meta-ros to export correct value in shell before calling superflore * this is needed by DependencyWalker evaluate_condition_context parameter to correctly resolve the conditionals when evaluating dependencies and build_type * there are 3 variables used in currently generated package.xml files not covered by this context: "ros_version" (lowercase) in lgsvl-msgs (all ROS distributions) https://raw.githubusercontent.com/ros2-gbp/lgsvl_msgs-release/release/rolling/lgsvl_msgs/0.0.3-2/package.xml "BUILD_WITH_LDMRS_SUPPORT" in sick_scan2 (only in foxy) "IGNITION_VERSION" in ros_ign_image (only in noetic) https://raw.githubusercontent.com/ros-gbp/ros_ign-release/release/noetic/ros_ign_image/0.111.0-1/package.xml Signed-off-by: Martin Jansa <martin.jansa@lge.com>
…ound * the package names aren't the same as generated recipes, so it's easy to make mistake and call e.g. sh scripts/ros-generate-recipes.sh noetic ros-core instead of sh scripts/ros-generate-recipes.sh noetic ros_core * the KeyError doesn't seem to be thrown from regenerate_pkg in this case as it raises RuntimeError, but extend the error message in both cases (as it's possible that KeyError might be raised in some other code-paths) Signed-off-by: Martin Jansa <martin.jansa@lge.com>
* set preserve_existing to False in regenerate_pkg call otherwise it will consider old recipe to be up to date (even without comparing its version) * call add_generated_files before commit, otherwise it generates a commit which removes the old version, but the new version is left untracked Signed-off-by: Martin Jansa <martin.jansa@lge.com>
…ot ROS 1 (ros-infrastructure#267) * _get_ros_version() expects just a string (as in distro.name) not whole RosDistro class This is a bit confusing, because in many places we use the same distro variable as this string, rename it to make it more clear in the next commit. Signed-off-by: Martin Jansa <martin.jansa@lge.com>
ros-infrastructure#267) * if get_build_type() returns 'catkin' and we're generating some ROS 2 distribution change it to 'ament_cmake' and show an error message * there are couple cases where <export><build_type> is missing completely and then catkin_pkg get_build_type() will return the default value as 'catkin' as specified in https://www.ros.org/reps/rep-0149.html#build-type-multiple "If no <build_type> is provided, catkin is assumed." but this assumption doesn't work for ROS 2 distributions and in meta-ros we had to override these cases (as of 2020-12-05): dashing: https://raw.github.com/ros2-gbp/async_web_server_cpp-release/release/dashing/async_web_server_cpp/1.0.0-1/package.xml https://raw.github.com/boschresearch/fmilibrary_vendor-release/release/dashing/fmilibrary_vendor/0.2.0-1/package.xml https://raw.github.com/ros-geographic-info/geographic_info-release/release/dashing/geographic_info/1.0.1-1/package.xml https://raw.github.com/swri-robotics-gbp/gps_umd-release/release/dashing/gps_umd/1.0.4-1/package.xml https://raw.github.com/ros2-gbp/web_video_server-release/release/dashing/web_video_server/1.0.0-1/package.xml https://raw.github.com/KIT-MRT/mrt_cmake_modules-release/release/dashing/mrt_cmake_modules/1.0.8-1/package.xml eloquent: https://raw.github.com/boschresearch/fmilibrary_vendor-release/release/eloquent/fmilibrary_vendor/0.2.0-1/package.xml https://raw.github.com/ros-geographic-info/geographic_info-release/release/eloquent/geographic_info/1.0.1-1/package.xml https://raw.github.com/swri-robotics-gbp/gps_umd-release/release/eloquent/gps_umd/1.0.2-1/package.xml foxy: https://raw.github.com/boschresearch/fmilibrary_vendor-release/release/foxy/fmilibrary_vendor/0.2.0-1/package.xml https://raw.github.com/swri-robotics-gbp/gps_umd-release/release/foxy/gps_umd/1.0.4-1/package.xml https://raw.github.com/KIT-MRT/mrt_cmake_modules-release/release/foxy/mrt_cmake_modules/1.0.8-1/package.xml rolling: https://raw.github.com/ros2-gbp/fmilibrary_vendor-release/release/rolling/fmilibrary_vendor/0.2.0-2/package.xml https://raw.github.com/ros2-gbp/gps_umd-release/release/rolling/gps_umd/1.0.3-2/package.xml Signed-off-by: Martin Jansa <martin.jansa@lge.com>
* e.g. _get_ros_version() expects just a string (as in distro.name) not whole RosDistro class this is a bit confusing, because in many places we use the same distro variable as this string, rename it to make it more clear Signed-off-by: Martin Jansa <martin.jansa@lge.com>
…pe names * with https://raw.githubusercontent.com/ros-gbp/moveit-release/release/noetic/moveit_kinematics/1.1.0-1/package.xml <depend condition="$ROS_DISTRO != noetic">orocos_kdl</depend> <depend condition="$ROS_DISTRO == noetic">liborocos-kdl-dev</depend> we have accidentally got orocos_kdl dependency (because ROS_DISTRO was missing in evaluate_condition_context) which results in dependency on ROS_UNRESOLVED_PLATFORM_PKG_orocos_kdl * but ROS_UNRESOLVED_PLATFORM_PKG_orocos_kdl = "UNRESOLVED-orocos_kdl" never got generated in generated/superflore-ros-distro.inc simply because https://github.com/ros-infrastructure/superflore/blob/649543e232fe1f44fcdf12c90ed5e0fc19bad4b8/superflore/generators/bitbake/yocto_recipe.py#L671 takes last underscore-separated field as PN, so it generated ROS_UNRESOLVED_PLATFORM_PKG_kdl = "UNRESOLVED-kdl" instead which in the end caused unexpanded dependency issue like: ERROR: Nothing PROVIDES '${ROS_UNRESOLVED_PLATFORM_PKG_orocos_kdl}' (but meta-ros/meta-ros1-noetic/generated-recipes/robot-calibration/robot-calibration_0.6.4-1.bb, meta-ros/meta-ros1-noetic/generated-recipes/moveit/moveit-kinematics_1.1.0-1.bb DEPENDS on or otherwise requires it) * similar issue is reproducible e.g. with jsk_pcl_ros: https://raw.githubusercontent.com/tork-a/jsk_recognition-release/release/noetic/jsk_pcl_ros/1.2.15-1/package.xml which has: <exec_depend>ml_classifiers</exec_depend> but there is no ml_classifiers package in noetic, it's only in melodic and dashing where it also generated: ROS_UNRESOLVED_PLATFORM_PKG_classifiers = "UNRESOLVED-classifiers" instead of: ROS_UNRESOLVED_PLATFORM_PKG_ml-classifiers = "UNRESOLVED-ml-classifiers" and similarly openni_launch which also doesn't exist in noetic, only in melodic ROS_UNRESOLVED_PLATFORM_PKG_launch = "UNRESOLVED-launch" ROS_UNRESOLVED_PLATFORM_PKG_openni-launch = "UNRESOLVED-openni-launch" * we could easily fix that by using whole "ROS_UNRESOLVED_PLATFORM_PKG_" prefix as a separator before PN, but there is still a risk of using underscore in bitbake variable names, because if some build happens to include one of these underscore separated strings (e.g. 'kdl') in OVERRIDES variable, then the ROS_UNRESOLVED_PLATFORM_PKG_orocos_kdl = "UNRESOLVED-orocos_kdl" assignment will be expanded as assignment to variable "ROS_UNRESOLVED_PLATFORM_PKG" with "kdl" and "orocos" as an overrides and ${ROS_UNRESOLVED_PLATFORM_PKG_orocos_kdl} will still end undefined - using fewer underscores here is better * but there is still an option that the whole ROS package name will match with something in OVERRIDES (e.g. MACHINE name can be pretty much anything) and then these variables will fail the same, to fix it properly we should use uppercase for dependency name as well (while possibly changing the prefix to use dash as separators for easier readability), e.g.: ROS-UNRESOLVED-PLATFORM-PKG_OROCOS-KDL = "UNRESOLVED-OROCOS-KDL" but that would require migration of all manually overriden mappings: $ git grep ^ROS_UNRESOLVED_PLATFORM_PKG_ | grep '/ros-distro\.inc' | wc -l 491 which is easy, but would cause unwelcome diff between newer and older meta-ros branches. To mitigate this we plan to upstream all these manual overrides from meta-ros to rosdistro rosdep files and then we can re-evaluate this change to upper case when we don't use so many overrides Signed-off-by: Martin Jansa <martin.jansa@lge.com>
* use DEPENDENCY instead of PLATFORM_PKG in UNRESOLVED_* variables in some cases as documented in previous commit the unresolved dependency might be ROS package which just happens to be missing in pkg_names, so it's considered as external/system/platform package * parse dependency name by skipping whole UNRESOLVED_DEPENDENCY_REF_PREFIX instead of relying on last underscore separated string Signed-off-by: Martin Jansa <martin.jansa@lge.com>
* without split() it was trying to remove all these files as one path in quotes and failing as shown after removing --ignore-unmatch git.exc.GitCommandError: Cmd('git') failed due to: exit code(128) cmdline: git rm -rf meta-ros2-eloquent/generated-recipes meta-ros2-eloquent/conf/ros-distro/include/eloquent/generated meta-ros2-eloquent/files/eloquent/generated/newer-platform-components.list meta-ros2-eloquent/files/eloquent/generated/rosdep-resolve.yaml meta-ros2-eloquent/files/eloquent/generated/superflore-change-summary.txt stderr: 'fatal: pathspec 'meta-ros2-eloquent/generated-recipes meta-ros2-eloquent/conf/ros-distro/include/eloquent/generated meta-ros2-eloquent/files/eloquent/generated/newer-platform-components.list meta-ros2-eloquent/files/eloquent/generated/rosdep-resolve.yaml meta-ros2-eloquent/files/eloquent/generated/superflore-change-summary.txt ' did not match any files' with split() it correctly complains only about superflore-change-summary.txt which will be fixed by returning --ignore-unmatch git.exc.GitCommandError: Cmd('git') failed due to: exit code(128) cmdline: git rm -rf meta-ros2-eloquent/generated-recipes meta-ros2-eloquent/conf/ros-distro/include/eloquent/generated meta-ros2-eloquent/files/eloquent/generated/newer-platform-components.list meta-ros2-eloquent/files/eloquent/generated/rosdep-resolve.yaml meta-ros2-eloquent/files/eloquent/generated/superflore-change-summary.txt stderr: 'fatal: pathspec 'meta-ros2-eloquent/files/eloquent/generated/superflore-change-summary.txt' did not match any files' Signed-off-by: Martin Jansa <martin.jansa@lge.com>
926aee2
to
18cbbe2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
* this way the lowercase dependency name won't ever be parsed as an override * use the same prefix in the value (instead of 'UNRESOLVED-') * there was still a risk of using underscore in bitbake variable names, because if some build happens to include one of these underscore separated strings (e.g. 'orocos-kdl') in OVERRIDES variable, then the ROS_UNRESOLVED_PLATFORM_PKG_orocos_kdl = "UNRESOLVED-orocos_kdl" assignment will be expanded as assignment to variable "ROS_UNRESOLVED_PLATFORM_PKG" with "orocos-kdl" as an override and ${ROS_UNRESOLVED_PLATFORM_PKG_orocos_kdl} will still end undefined - using fewer underscores here is better * there was still an option that the whole ROS package name will match with something in OVERRIDES (e.g. MACHINE name can be pretty much anything) and then these variables will fail, to fix it properly we should either use uppercase for dependency name or change the prefix separator from underscore to dash. * this will require migration of all manually overriden mappings: $ git grep ^ROS_UNRESOLVED_PLATFORM_PKG_ | grep '/ros-distro\.inc' | wc -l 491 so I've updated rosdistro/rosdep to include most of them first and regenerated all ROS distributions with updated rosdistro. Signed-off-by: Martin Jansa <martin.jansa@lge.com>
18cbbe2
to
bbe1d07
Compare
* otherwise it won't be parsed correctly by superflore as explained in: "bitbake: use own condition_context instead of whole os.environ" commit from: ros-infrastructure/superflore#280 Signed-off-by: Martin Jansa <martin.jansa@lge.com>
No description provided.