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

fix add rosdep_update_options #684 #890

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions ros_buildfarm/devel_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,8 @@ def configure_devel_job(
build_targets=None,
dry_run=False,
run_abichecker=None,
require_gpu_support=None):
require_gpu_support=None,
custom_rosdep_update_options=None):
"""
Configure a single Jenkins devel job.

Expand Down Expand Up @@ -332,7 +333,8 @@ def configure_devel_job(
build_file, os_name, os_code_name, arch, source_repository,
repo_name, pull_request, job_name, dist_cache=dist_cache,
is_disabled=is_disabled, run_abichecker=run_abichecker,
require_gpu_support=require_gpu_support)
require_gpu_support=require_gpu_support,
custom_rosdep_update_options=custom_rosdep_update_options)
# jenkinsapi.jenkins.Jenkins evaluates to false if job count is zero
if isinstance(jenkins, object) and jenkins is not False:
from ros_buildfarm.jenkins import configure_job
Expand All @@ -353,7 +355,7 @@ def _get_devel_job_config(
build_file, os_name, os_code_name, arch, source_repo_spec,
repo_name, pull_request, job_name, dist_cache=None,
is_disabled=False, run_abichecker=None,
require_gpu_support=None):
require_gpu_support=None, custom_rosdep_update_options=None):
template_name = 'devel/devel_job.xml.em'

repository_args, script_generating_key_files = \
Expand Down Expand Up @@ -425,6 +427,7 @@ def _get_devel_job_config(
'build_tool_test_args': build_file.build_tool_test_args,
'ros_version': ros_version,
'build_environment_variables': build_environment_variables,
'custom_rosdep_update_options': custom_rosdep_update_options,

'run_abichecker': run_abichecker,
'require_gpu_support': require_gpu_support,
Expand Down
1 change: 1 addition & 0 deletions ros_buildfarm/templates/devel/devel_job.xml.em
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ if pull_request:
' --ros-version ' + str(ros_version) +
(' --run-abichecker' if run_abichecker else '') +
(' --require-gpu-support' if require_gpu_support else '') +
(' --custom-rosdep-update-options="' + ' '.join(custom_rosdep_update_options) + '"' if custom_rosdep_update_options else '') +
Copy link
Member

Choose a reason for hiding this comment

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

We should consider changing the argument type for this to use nargs=argparse.REMAINDER to avoid the need to wrap the value in quotes. The approach here is fragile and will break if the arguments need to contain quotes or other characters meaningful to the shell.

I'm not entirely sure if we should block this PR on that, and I'm not sure if it might be a breaking change to do so, but as someone who is clearly utilizing this feature already, I'm hoping you can tell me!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I use this feature on https://github.com/ros-perception/openslam_gmapping/blob/melodic-devel/.travis.yml#L29, but do not know other people who utilizing this. may be @ipa-mdl is interested in ros-industrial/industrial_ci#579, but does not seem to actually using this.

' --env-vars ' + ' '.join(build_environment_variables) +
' --dockerfile-dir $WORKSPACE/docker_generating_dockers' +
' --build-tool-args $build_tool_args' +
Expand Down
3 changes: 2 additions & 1 deletion scripts/prerelease/generate_prerelease_script.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,8 @@ def beforeInclude(self, *_, **kwargs):
index=index, dist_file=dist_file, dist_cache=dist_cache,
jenkins=False, views=False,
source_repository=source_repository,
build_targets=release_targets_combined)
build_targets=release_targets_combined,
custom_rosdep_update_options=args.custom_rosdep_update_options)

templates.template_hooks = None

Expand Down