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

Add support to create PR against ROS gz_*_vendor repositories in release.py #1151

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

j-rivero
Copy link
Contributor

This PR adds support in release.py to automatically create PRs in the gz_*_vendor repositories when a new release for any gazebo libraries is being created:

  • To detect if a vendor repository is affected by a the release in process, the release.py uses the script jenkins-scripts/dsl/tools/get_collections_from_package_and_version.py in Include a script to obtain gazebo releases for a given library #1148 that is able to return Gazebo releases given a library a version. To known the vendor repository from a Gazebo releases the script uses a simple approach by now defining a map of gazebo-release <-> ros-distro ROS_VENDOR = {'harmonic': 'rolling'} that assumes also that vendor repository branches are named in the same way than the ROS release names.
  • For creating a PR that updates the version in the gz_*_vendor repositories:
    • First step is to create a temporary directory and clone the corresponding gz_*_vendor packages and the tool https://github.com/gazebo-tooling/gz_vendor/. That will update the versions needed.
    • Second step is to commit the results in the cloned vendor repositories, push a new branch named releasepy/{version} and use gh create pr to send the PR.

The PR also implements an output parsing for detecting if the ROS vendor commands are being processed correctly for a variety of options.

For testing the whole pipeline, I've been using the following patch that enables all new actions under the --dry-run and run a release like: release.py gz-cmake3 3.2.3 foo --dry-run --no-sanity-check

diff --git a/release.py b/release.py
index 34945f11..7c7c51b5 100755
--- a/release.py
+++ b/release.py
@@ -540,6 +540,7 @@ def get_collections_for_package(package_name, version) -> list:
 
 def get_vendor_github_repo(package_name) -> str:
     canonical_name = get_canonical_package_name(package_name)
+    return f"j-rivero/{canonical_name.replace('-', '_')}_vendor"
     return f"gazebo-release/{canonical_name.replace('-', '_')}_vendor"
 
 
@@ -572,7 +573,7 @@ def execute_update_vendor_package_tool(vendor_tool_path, vendor_repo_path) -> No
                f"{vendor_tool_path}/create_gz_vendor_pkg/create_vendor_package.py",
                'package.xml',
                '--output_dir', vendor_repo_path]
-    _, _err_run = check_call(run_cmd)
+    _, _err_run = check_call(run_cmd, IGNORE_DRY_RUN)
     if _err_run:
         print("Problems running the create_vendor_package.py script:")
         print(_err_run.decode())
@@ -592,18 +593,18 @@ def create_pr_for_vendor_package(args, repo_path, base_branch) -> str:
                   'commit',
                   '-m', f'Bump version to {args.version}',
                   'CMakeLists.txt', 'package.xml']
-    _, _ = check_call(commit_cmd)
+    _, _ = check_call(commit_cmd, IGNORE_DRY_RUN)
     push_cmd = ['git', "-C", repo_path,
                 'push', '--force',
                 vendor_repo, branch_name]
-    _, _ = check_call(push_cmd)
+    _, _ = check_call(push_cmd, IGNORE_DRY_RUN)
     pr_cmd = ['gh', 'pr', 'create',
               '--base', base_branch,
               '--head', branch_name,
               '--title', f'Bump version to {args.version}',
               '--body', 'PR automatically created by release.py']
     try:
-        _out, _err = check_call(pr_cmd, cwd=repo_path)
+        _out, _err = check_call(pr_cmd, IGNORE_DRY_RUN, cwd=repo_path)
     except ErrorAlreadyExists:
         return 'there is already a PR for the branch.'\

Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
…version

Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
@azeey azeey self-requested a review June 17, 2024 18:38
@azeey
Copy link
Contributor

azeey commented Jun 25, 2024

I just tested this and it failed due to a missing dependency: jinja2

ROS vendor packages that can be updated:
 * Github gazebo-release/gz_physics_vendor part of harmonic in ROS 2 rolling
   + Preparing a PR: Error running command (python3 /tmp/tmppwudeu3w/gz_vendor/create_gz_vendor_pkg/create_vendor_package.py package.xml --output_dir /tmp/tmppwudeu3w/gz_vendor_repo).
stdout:
stderr: Traceback (most recent call last):
  File "/tmp/tmppwudeu3w/gz_vendor/create_gz_vendor_pkg/create_vendor_package.py", line 12, in <module>
    import jinja2
ModuleNotFoundError: No module named 'jinja2'

Traceback (most recent call last):
  File "~/ws/release/release-tools-upstream/release.py", line 782, in <module>
    go(sys.argv)
  File "~/ws/release/release-tools-upstream/release.py", line 777, in go
    pr_url = create_pr_in_gz_vendor_repo(args, ros_distro)
  File "~/ws/release/release-tools-upstream/release.py", line 633, in create_pr_in_gz_vendor_repo
    execute_update_vendor_package_tool(
  File "~/ws/release/release-tools-upstream/release.py", line 581, in execute_update_vendor_package_tool
    _, _err_run = check_call(run_cmd)
  File "~/ws/release/release-tools-upstream/release.py", line 409, in check_call
    raise Exception('subprocess call failed')
Exception: subprocess call failed

Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
@j-rivero
Copy link
Contributor Author

I just tested this and it failed due to a missing dependency: jinja2

Implemented an installation of venv to be used to handle dependencies in 80475de. Dependencies are added here manually by now, would be better handled if we a requirements.txt file in tha gz_vendor repository.

I've also added a --only-bump-ros-vendor-package option that skip almost everything and just deal with the PR generation for ROS packages.

@azeey
Copy link
Contributor

azeey commented Aug 20, 2024

I was able to use this when releasing gz-fuel-tools 9.1.0 and it created gazebo-release/gz_fuel_tools_vendor#3, so it seems to work well. I still haven't had a chance to do a full review though. I also noticed that it created on PR toward the rolling branch, but right now (until Ionic) both the rolling and jazzy branches use Harmonic versions so we probably need to open two PRs

release.py Outdated Show resolved Hide resolved
release.py Outdated Show resolved Hide resolved
'CMakeLists.txt', 'package.xml']
_, _ = check_call(commit_cmd)
push_cmd = ['git', "-C", repo_path,
'push', '--force',
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it safe to use --force here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I remember correctly, this --force was added to handle the overwrite of previous existing branches (branch_name = f'releasepy/{args.version}') created by the tool in previous runs without the need of removing the branch to make the tool to work. This is useful to correct erroneous/incomplete PRs created by the own tool but if we can stabilize the code could be removed. Whatever you prefer.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. Let's keep for now then. Maybe add a TODO to remove it later?

azeey and others added 4 commits September 26, 2024 08:23
…ill report there were no changes in the vendor package

Signed-off-by: Addisu Z. Taddese <addisu@openrobotics.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

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

LGTM! Just one unresolved comment left.

@iche033
Copy link
Contributor

iche033 commented Sep 26, 2024

I just tried releasing gz-sensors9 and it seems to use http instead of ssh to push to the vendor repo?

Error running command (git -C /tmp/tmpz13zdsnx/gz_vendor_repo push --force https://github.com/gazebo-release/gz_sensors_vendor releasepy/9.0.0).

It then asks for username / pw but fails to push as it's is no longer supported. I manually pushed using ssh and created the PR. Just checking if there's some configuration that I needed to do first?

@azeey
Copy link
Contributor

azeey commented Sep 27, 2024

Ah, that's definitely an issue. It worked fine for me because I have the following in my ~/.gitconfig

[url "ssh://git@github.com/gazebo-release"]
  insteadOf = https://github.com/gazebo-release

@iche033
Copy link
Contributor

iche033 commented Sep 27, 2024

there's no gz_ionic_vendor so maybe skip the vendor PR creation if it's doing a release for a gz collection package?

Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
@j-rivero
Copy link
Contributor Author

there's no gz_ionic_vendor so maybe skip the vendor PR creation if it's doing a release for a gz collection package?

24d6a83


It then asks for username / pw but fails to push as it's is no longer supported. I manually pushed using ssh and created the PR. Just checking if there's some configuration that I needed to do first?

I think that we can default to ssh but keeping https for testing and dry-run, did that on 4180170

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants