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

Use latest prerelease features #145

Merged

Conversation

mathias-luedtke
Copy link
Member

@mathias-luedtke mathias-luedtke commented Mar 16, 2017

  • works for repositories that are not (yet) listed in rosdistro
  • adds mockup feature
  • slightly changes meaning of PRERELEASE_REPONAME
  • add travis folding

The latter prevents testing packages other than the target repo as we did it before.

@mathias-luedtke
Copy link
Member Author

mathias-luedtke commented Mar 16, 2017

@mathias-luedtke
Copy link
Member Author

mathias-luedtke commented Mar 16, 2017

43657b2 can be removed as soon as ros-infrastructure/ros_buildfarm#405 was merged.
09f95e4 should get removed or reverted after the release (ros-infrastructure/ros_buildfarm#394)

@mathias-luedtke mathias-luedtke changed the title [WiP] Use latest prerelease features Use latest prerelease features Mar 16, 2017
@mathias-luedtke mathias-luedtke force-pushed the enhancement/prerelease-updates branch 4 times, most recently from 2e9c72a to 02f4c4f Compare March 17, 2017 07:55
@130s
Copy link
Member

130s commented Mar 17, 2017

Will try looking into this over the weekend!

Copy link
Member

@130s 130s left a comment

Choose a reason for hiding this comment

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

This looks great! Added test jobs seem to be working nicely too. I left some comments inline.

Btw just curious, the fold in the fold (e.g. this) is what's added in industrial_ci or does it come from somewhere upstrem? That's nice too (if only it could be colorized when error occurs inside, since finding errors may be a bit tricky without it. But may not be a big deal).

doc/index.rst Outdated
* `PRERELEASE_REPONAME` (default: not set): The target of Prerelease Test (that you select at `<http://prerelease.ros.org/indigo>`_, `<http://prerelease.ros.org/kinetic>`_ etc.).
* If not set then it tests the package of the repository's name. You can specify this by your ROS package name format (with underscore e.g. `industrial_core`), not Debian package name format. NOTE that this package name must be listed in the `rosdistro/distribution.yaml` (e.g. [for ROS Indigo](https://github.com/ros/rosdistro/blob/master/indigo/distribution.yaml)) (this requirement comes from ROS buildfarm's Prerelease Test).
* (As of Dec 2016) when this variable is set, development branch listed in `rosdistro/distribution.yaml` is tested. See [detail](https://github.com/ros-industrial/industrial_ci/pull/85#issue-196409011).
* `PRERELEASE_REPONAME` (default: TARGET_REPO_NAME): The name of the target of Prerelease Test in rosdistro (that you select at `<http://prerelease.ros.org>`_). You can specify this if your repository name differs from the corresponding rosdisto entry.
Copy link
Member

Choose a reason for hiding this comment

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

You can specify this if your repository name differs from the corresponding rosdisto entry.

This is a bit unclear. Readers may want to know, in which cases specifying different name here could be useful, if the name specified is different from the tested repo name, where is the specified package retrieved from? etc.

Copy link
Member Author

@mathias-luedtke mathias-luedtke Mar 26, 2017

Choose a reason for hiding this comment

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

It does not get retrieved. It is just the name in rosdistro.
E.g. if your key in rosdistro is my_pkg and for some reason your source is in https://github.com/me/my_pkg_impl.git, you have to set PRERELEASE_REPONAME=my_pkg.
I guess this case is prettry rare.

Copy link
Member

Choose a reason for hiding this comment

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

I see. That usecase is a good example. We should add it to the doc or at least link here. Please review mathias-luedtke#8

run_in_prerelease_docker catkin_test_results $WORKSPACE/catkin_workspace_overlay/test_results
ici_time_end # prerelease_build_overlay.sh
fi
run_in_prerelease_docker generate_prerelease_script.py https://raw.githubusercontent.com/ros-infrastructure/ros_buildfarm_config/production/index.yaml "$ROS_DISTRO" default ubuntu "$UBUNTU_OS_CODE_NAME" amd64 --level "$downstream_depth" --output-dir . --custom-repo "$reponame::::"
Copy link
Member

Choose a reason for hiding this comment

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

"$reponame::::"

Just curious, are the trailing colons still needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, all of them.

repo : type : url_scheme : url_path : branch
For us type, url_scheme, url_patch, branch are empty.
This way the prerelease will fail if the target source code is missing.

@@ -26,8 +26,9 @@ env:
- ROS_DISTRO=indigo NOT_TEST_BUILD='true' NOT_TEST_INSTALL='true' POST_PROCESS='I_am_supposed_to_fail'
- ROS_DISTRO=indigo CATKIN_PARALLEL_JOBS='-p1' ROS_PARALLEL_JOBS='-j1' # Intend build on low-power platform
# - env: ROS_DISTRO=indigo PRERELEASE=true ## Comment out because this is meaningless for always failing without prerelease testable contents in industrial_ci.
- ROS_DISTRO=indigo PRERELEASE=true PRERELEASE_REPONAME=std_msgs PRERELEASE_DOWNSTREAM_DEPTH=0
- ROS_DISTRO=kinetic PRERELEASE=true PRERELEASE_REPONAME=std_msgs
- ROS_DISTRO=indigo PRERELEASE=true PRERELEASE_DOWNSTREAM_DEPTH=1
Copy link
Member

Choose a reason for hiding this comment

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

Don't we (also) want to add a job for a package that is actually depended on by maybe just one or a few packages so that we can test PRERELEASE_DOWNSTREAM_DEPTH? We can even add another package to mockups and inject dependency.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is not possible for now. With this patch, we cannot "fake" the identity anymore, and industrial_ci does not have any dowstream depencies.
In a later versions we might add a full rosinstall workspace as underlay, then we can test it again

Copy link
Member

Choose a reason for hiding this comment

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

So I assume this new job does not test anything distinct for now other than testing PRERELEASE_DOWNSTREAM_DEPTH option to not cause error.

@130s 130s merged commit 89da19e into ros-industrial:master Mar 26, 2017
@mathias-luedtke mathias-luedtke deleted the enhancement/prerelease-updates branch November 5, 2017 16:27
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.

2 participants