Skip to content
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

Update Intel RealSense team and repositories #297

Open
wants to merge 1 commit into
base: latest
Choose a base branch
from

Conversation

SamerKhshiboun
Copy link

No description provided.

@Nir-Az
Copy link

Nir-Az commented Jul 13, 2023

@nuclearsandwich can someone please help with reviewing / merging this PR.
We have a release pending for it and we may loss the sync..

Thanks :)

@nuclearsandwich
Copy link
Collaborator

@Nir-Az thanks for the ping and my apologies for the delay.

Are you changing the names of the release repositories here or attempting to remove/retire old releases and start new ones?
Changing release repo names requires some extra support since the terraform scripts do not track renames directly.

Comment on lines +3 to +5
"SamerKhshiboun",
"IntelRealSense",
"Nir-Az"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to change team members, can you please provide links to the relevant package.xml <maintainer> tags in the source repository for these individuals or otherwise provide a link demonstrating that they are authorized maintainers/releasers of these ROS packages.

Copy link
Author

@SamerKhshiboun SamerKhshiboun Jul 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines -6 to +9
"librealsense-release",
"ros2_intel_realsense-release",
"librealsense2-release",
"realsense-ros-release",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these "renames" of the current release repositories or the addition of two repositories and the removal of two others?

Release repositories cannot be deleted so the old repositories must be archived rather than removed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these "renames" of the current release repositories or the addition of two repositories and the removal of two others?

Release repositories cannot be deleted so the old repositories must be archived rather than removed.

Renaming of the old release repositories. The new ones links:
https://github.com/IntelRealSense/librealsense2-release
https://github.com/IntelRealSense/realsense-ros-release

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renaming of the old release repositories. The new ones links:

I think that we may be miscommunicating, IntelRealSense/librealsense2-release does not appear to have any common commits from the master branch (bloom's central config branch) of ros2-gbp/librealsense-release which suggests to me that this is a new release repository and not an extension of ros2-gbp/librealsense-release.

The same is the case for ros2-gbp/ros2_intel_realsense-release and IntelRealSense/realsense-ros-release

I think the right thing to do is:

  1. Archive both
  • ros2-gbp/librealsense-release
  • ros2-gbp/ros2_intel_realsense-release
  1. and then create new ros2-gbp repositories:
  • ros2-gbp/librealsense2-release
  • ros2-gbp/realsense-ros-release
  1. and mirror push the current contents of the IntelRealSense hosted repositories:
  1. finally, once we've verified that you can push releases to the ros2-gbp repositories it's a good idea to archive the ones in IntelRealSense to avoid accidentally pushing to them in the future.

If that plan works for you, let me know and I'll refactor this PR to include the archivals and mirror the current repositories.

Copy link
Author

@SamerKhshiboun SamerKhshiboun Jul 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @nuclearsandwich,

Thanks for the detailed solution you are proposing.
Before doing the steps above, I need some clarifications from your side:

  • How this is going to affect current/old release for the different distros ? (humble, iron, and even EOL distro.. foxy, galactic...)
  • What about writ access to gbp repo ? will we have the permissions to write/edit patches and files of our releases ?
  • How does this change will affect our future releases ? specially when new ROS distros are out

Thanks,
Samer

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How this is going to affect current/old release for the different distros ? (humble, iron, and even EOL distro.. foxy, galactic...)

Archiving the repositories will make them read-only but keep them available for people using those earlier packages.

What about writ access to gbp repo ? will we have the permissions to write/edit patches and files of our releases ?

Release repositories are generally not modified by hand, the bloom release tool is primarily used to update release repositories based on changes in the upstream source repository.

How does this change will affect our future releases ? specially when new ROS distros are out

The release process is the same for most ROS 2 packages. Once granted access to the ros2-gbp repositories you'll be able to follow http://docs.ros.org/en/rolling/How-To-Guides/Releasing/Subsequent-Releases.html when making releases.

@nuclearsandwich nuclearsandwich added the needs-maintainer-links package.xml maintainer entries or other links supporting the addition of team members are needed. label Jul 15, 2023
@SamerKhshiboun
Copy link
Author

@nuclearsandwich, any update on this?
Should I edit/add something ?

@Nir-Az
Copy link

Nir-Az commented Aug 11, 2023

There are 3 issues in the process that will make our lives hard.

  1. When a brach is changed we saw the bloom release is not modifing it, so we puch the change manually.

  2. When we publish a release we sometimes need control on patches, delete some, create some.
    It probably be harder doing it this way.

Any known work method to get by this?

@nuclearsandwich
Copy link
Collaborator

Okay, what's going on with these is actually really hard to follow. So I'm going to try and spell it all out.

The source repositories to be released are:

  1. https://github.com/IntelRealSense/librealsense
  2. https://github.com/IntelRealSense/realsense-ros

There are two existing release repositories in ros2-gbp
A. https://github.com/ros2-gbp/librealsense-release which was previously used to release https://github.com/IntelRealSense/librealsense
B. https://github.com/ros2-gbp/ros2_intel_realsense-release which was previously used to release https://github.com/intel/ros2_intel_realsense (now archived upstream)

The two external release repositories proposed for import are:
i. https://github.com/IntelRealSense/librealsense2-release for https://github.com/IntelRealSense/librealsense
ii. https://github.com/IntelRealSense/realsense-ros-release for https://github.com/IntelRealSense/realsense-ros

Both A and i use 1 as their upstream but have completely distinct release artifacts.
The upstream source repository for B has been archived and is apparently succeeded by 2.

So the proposal I mentioned in #297 (comment) is still valid:

  1. Archive both A and B in ros2-gbp.
  2. Mirror push i and ii into the ros2-gbp organization
  3. Archive the external repositories https://github.com/IntelRealsense/librealsense2-release and https://github.com/IntelRealsense/realsense-ros-release to prevent accidental pushes to them instead of the new ros2-gbp repositories).

The questions raised in that thread are answered in #297 (comment) but I'm happy to go into more detail if there are follow-up questions.


Separately, there is the issue of maintainership evidence. There is no formal set of pass/fail logic for demonstrating maintainership. "claiming" a reference to a maintainer entry in package.xml is usually enough that if access is ever questioned we can point to the link as evidence. Other links such as PRs merged by the specified GitHub user (merged by in order to demonstrate write access, not authored by) or endorsement from an existing maintainer would both be acceptable substitutes.

@nuclearsandwich
Copy link
Collaborator

There are 3 issues in the process that will make our lives hard.

Just to make sure I am not misunderstanding, I only see two points below this sentence not three.

  1. When a branch is changed we saw the bloom release is not modifying it, so we push the change manually.

Are you pushing changes to the source repository and not seeing those changes reflected in Bloom? Bloom is a tool for generating platform-specific packaging information for releases. Which means that it will look for tagged releases in the source repository. Updating a branch without creating a new release would not change what bloom creates release information for.
The steps Updating changelog and Bump the package version cover the usual method for performing these operations on ROS packages but you can do it manually as well.

2. When we publish a release we sometimes need control on patches, delete some, create some.
It probably be harder doing it this way.

What kinds of patching are you talking about? Are you referring to updates to the release once it is already tagged or platform / architecture-specific patches that need to be applied during the release?

@Nir-Az
Copy link

Nir-Az commented Aug 31, 2023

There are 3 issues in the process that will make our lives hard.

Just to make sure I am not misunderstanding, I only see two points below this sentence not three.

  1. When a branch is changed we saw the bloom release is not modifying it, so we push the change manually.

Are you pushing changes to the source repository and not seeing those changes reflected in Bloom? Bloom is a tool for generating platform-specific packaging information for releases. Which means that it will look for tagged releases in the source repository. Updating a branch without creating a new release would not change what bloom creates release information for. The steps Updating changelog and Bump the package version cover the usual method for performing these operations on ROS packages but you can do it manually as well.

  1. When we publish a release we sometimes need control on patches, delete some, create some.
    It probably be harder doing it this way.

What kinds of patching are you talking about? Are you referring to updates to the release once it is already tagged or platform / architecture-specific patches that need to be applied during the release?

I will try to explain,

  1. An example to our patches is this: https://github.com/IntelRealSense/librealsense2-release/blob/patches/release/humble/librealsense2/0001-update-lrs-options.patch

We need control over this patches, modify/delete/add
The package source code is : https://github.com/IntelRealSense/librealsense and it is cross compile SDK.
For ROS we need to override some code (mostly change some CMake flags)

How can we do it with the new process?

  1. We had to release from a different branch and this was missed on bloom release process.
    We had to manually push a commit to fix it (ros/rosdistro@a080a91)
    Is this a known bug? or did we do something wrong?

Thanks!

@nuclearsandwich
Copy link
Collaborator

An example to our patches is this: https://github.com/IntelRealSense/librealsense2-release/blob/patches/release/humble/librealsense2/0001-update-lrs-options.patch

We need control over this patches, modify/delete/add

Thanks for the context. That patch looks like it was created using bloom's built-in patching workflow, which is definitely under-documented, but referenced in the tutorial here: http://wiki.ros.org/bloom/Tutorials/ReleaseThirdParty#Adding_an_Install_Rule_as_a_Patch

How can we do it with the new process?

This patch is introduced via IntelRealSense/librealsense2-release@f1d1af2 and subsequent patches can be added the same way using the same workflow when using the ros2-gbp repository. Your ability to update these patches is not affected by updating the release repository location.

We had to release from a different branch and this was missed on bloom release process.
We had to manually push a commit to fix it (ros/rosdistro@a080a91)
Is this a known bug? or did we do something wrong?

Bloom is configured by the tracks.yaml file on the master branch. The rolling stanza for librealsense2-release has no devel_branch specified. Bloom will check the devel_branch for tags when automating a bloom release. However, the release version in your rolling track is currently hard-coded to 2.54.1 so no matter which branch is specified the tag corresponding to that release will be used until the tracks.yaml is updated.

Bloom releases are run from source repository tags only, not branches. Bloom can use source branches to find the appropriate tag for a release but your repository is not set up for this currently. ros/rosdistro@a080a91 updates the rosdistro index to specify which branch should be considered for upstream source and documentation builds. However, bloom does not actually use those values (ros-infrastructure/bloom#708) and instead looks exclusively at the branch specified by devel_branch if the release version for the track is set to :{auto}. With the hard-coded version field you'll have to use bloom-release --edit ... whenever you want to change the version you intend to release. That is also a valid workflow.

@SamerKhshiboun
Copy link
Author

SamerKhshiboun commented Jun 3, 2024

@kadiredd
Please take a moment to read the whole thread and comment here when we are ready to do the requested steps.
We should just make sure that the current releases in humble/iron will not be affected when we mirror our release repositors.

The steps are well-explained above by @nuclearsandwich :
#297 (comment)

@SamerKhshiboun
Copy link
Author

SamerKhshiboun commented Jun 11, 2024

Hi @nuclearsandwich, @kadiredd is a ROS Wrapper for RealSense cameras developer, and will continue this task from here.

Just a quick question, can we set the maintainer as Librealsense ROS Team ? or should it be a real GitHub user ?
if that is okay, @kadiredd will submit a new PR for changing the realsense team.

In all of our three packages this is the maintainer:
<maintainer email="[email protected]">LibRealSense ROS Team</maintainer>

Attached links to the 3 packages xmls

  1. https://github.com/IntelRealSense/realsense-ros/blob/ros2-development/realsense2_description/package.xml
  2. https://github.com/IntelRealSense/realsense-ros/blob/ros2-development/realsense2_camera_msgs/package.xml
  3. https://github.com/IntelRealSense/realsense-ros/blob/ros2-development/realsense2_camera/package.xml

Thanks,
Samer

@nuclearsandwich
Copy link
Collaborator

Hi @nuclearsandwich, @kadiredd is a ROS Wrapper for RealSense cameras developer, and will continue this task from here.

Hello @kadiredd, I'm really happy to get this all sorted out. If it will help to get on a video chat once you've made your first pass through the thread. I'm in the US/Pacific timezone but can accommodate an early morning or later evening if required.

@kadiredd
Copy link

kadiredd commented Jun 21, 2024 via email

@kadiredd
Copy link

kadiredd commented Jul 1, 2024

@nuclearsandwich - friendly reminder

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-maintainer-links package.xml maintainer entries or other links supporting the addition of team members are needed.
Development

Successfully merging this pull request may close these issues.

4 participants