search

ros-infrastructure / superflore

An extended platform release manager for Robot Operating System
Apache License 2.0
52 stars 33 forks source link

Implement changes for meta-ros layer version 3 (phase 1) #256

Closed shr-project closed 4 years ago

shr-project commented 4 years ago

I've updated this to include fixed version of all 3 open PRs from Herb and 1 from me. This branch is what we're using in meta-ros layer version 3 now.

herb-kuta-lge commented 4 years ago

@shr-project < Please defer https://github.com/ros-infrastructure/superflore/pull/256/commits/7820ceebb56b24e5b77281fba361db0d6db1866e until we have a complete solution for building ROS 1 packages for ROS 2 distros.

shr-project commented 4 years ago

@shr-project < Please defer 7820cee until we have a complete solution for building ROS 1 packages for ROS 2 distros.

But this fixes the parsing issues and allows to start building and testing. Without this we'll be again blocked until superflore is updated and all distros regenerated to start working on this.

With the number for recipe re-generations and updates we have in queue for various milestones it would be really useful to have this already included to be able to work on ROS1+ROS2 in parallel to other initiatives.

herb-kuta-lge commented 4 years ago

Won't it have to be changed anyway to set the other generated variables correctly? Some of them will need to be appended, but others, eg, ROS_VERSION will need ?=. Wouldn't it be better to make all of the changes to support ROS 1 + ROS 2 at the same time once we've decided what this will be?

But this fixes the parsing issues and allows to start building and testing.

You can always add a TEMP commit to make the changes you need to fix the parsing issues.

herb-kuta-lge commented 4 years ago

generators/bitbake/yocto_recipe.py:539: conf_file_name = 'ros-distro.inc'

This needs to be changed to 'superflore-ros-distro.inc' and the /generated subdirectory added to the line 1 comment, eg. # melodic/generated/superflore-ros-distro.inc.

shr-project commented 4 years ago

Won't it have to be changed anyway to set the other generated variables correctly? Some of them will need to be appended, but others, eg, ROS_VERSION will need ?=. Wouldn't it be better to make all of the changes to support ROS 1 + ROS 2 at the same time once we've decided what this will be?

Without this not even bitbake -e doesn't work, because it doesn't parse due to missing native dependencies. What's wrong with solving this incrementally? The main fix would be to change the order in which ros-distro.inc files are parsed - this shouldn't need a change in superflore. At least it would be good enough to investigate what else needs to change with bitbake -e, but if you reject this one, then the very first step would need to be this superflore change or similar TEMP commit just to get started.

herb-kuta-lge commented 4 years ago

What's wrong with solving this incrementally?

Nothing, I suppose, but then place it in a separate PR so that this one contains only the changes required to implement part 1 of meta-ros layer version 3. And change the title of this PR to "Implement changes for meta-ros layer version 3 (part 1)". @allenh1 < The PR merge policy that's being used for this repo is "rebase and squash", right? (I don't see where this is shown in the UI.) So the title of the PR matters because it is what will appear as the commit summary line.

shr-project commented 4 years ago

Added another PR https://github.com/ros-infrastructure/superflore/pull/259 which includes all of this PR + that one commit on top.

herb-kuta-lge commented 4 years ago

Please squash "yocto_recipe.py: rename the generated ros-distro.inc to superflore-ro" into "Generate recipes in meta-ros- instead of generated-recipes-<d…"

herb-kuta-lge commented 4 years ago

Added another PR #259 which includes all of this PR + that one commit on top. 31b491d747c0fda5f4ce9fa694c93454794b4c98

herb-kuta-lge commented 4 years ago

Added another PR #259 which includes all of this PR + that one commit on top.

31b491d747c0fda5f4ce9fa694c93454794b4c98 still appears in the list shown by https://github.com/ros-infrastructure/superflore/pull/256/commits . Is this correct?

allenh1 commented 4 years ago

@shr-project please resolve the merge conflicts, and I'll give it a look

shr-project commented 4 years ago

@shr-project please resolve the merge conflicts, and I'll give it a look

The merge conflicts are caused by other 3 changes from other open PRs which I've included yesterday to resolve the merge conflicts :). Once the https://github.com/ros-infrastructure/superflore/pull/233 https://github.com/ros-infrastructure/superflore/pull/243 are merged, I'll rebase this and only last 3 commits should remain without any conflicts.

shr-project commented 4 years ago

@shr-project please resolve the merge conflicts, and I'll give it a look

The merge conflicts are caused by other 3 changes from other open PRs which I've included yesterday to resolve the merge conflicts :). Once the #233 #243 are merged, I'll rebase this and only last 3 commits should remain without any conflicts.

Thanks for merging the other 2, I've rebased this one (and the follow-up one) now.