ros-infrastructure / buildfarm

Build scripts and notes for catkin debian build pipeline.
6 stars 15 forks source link

Mark packages with missing package.xml unstable #205

Closed cottsay closed 10 years ago

cottsay commented 10 years ago

This was discussed in https://github.com/ros/catkin/issues/622

Since I don't have access to the debian buildfarm, someone will need to test this.

Feedback is appreciated.

Thanks,

--scott

dirk-thomas commented 10 years ago

Each job builds exactly one package. Please update the text for that. Also please modify the check for the package.xml file to test the verify specific location where the package.xml file for that package must be (instead of searching for any file with that name).

cottsay commented 10 years ago

The name of the ROS package wasn't available in the binary build template, so I had to add it. Please take another look and let me know where there are other concerns.

Thanks,

--scott

dirk-thomas commented 10 years ago

After the minor comments +1 from me.

cottsay commented 10 years ago

Updated again after @dirk-thomas's astute observations.

Please note once again that I haven't tested this. I'd rather see someone try a package or two on the buildfarm before it is merged...

Thanks,

--scott

dirk-thomas commented 10 years ago

There is sadly no good way to test that except on the farm. We have reviewed the PR and will merge it. We will watch the next releases and see if there is any regression.

Thank you!

dirk-thomas commented 10 years ago

Boom! http://jenkins.ros.org/job/ros-indigo-rosserial-windows_binarydeb_trusty_amd64/

I am looking into it...

tfoote commented 10 years ago

Should this not be a dpkg -L packagename

cottsay commented 10 years ago

@tfoote dpkg -L is for a package that is already installed. dpkg -c is used to list the contents of a .deb file.

cottsay commented 10 years ago

I see this in the logs:

tar: write error
dpkg-deb: error: subprocess tar returned error exit status 2

I believe this is caused by grep closing stdin before the internal call to tar is done dumping the contents of the archive.

man grep says this for -q:

Exit immediately with zero status if any match is found, even if an error was detected.

...so I think the -q must be removed. I can reproduce this behaviour on my local machine by loading the system down (some while(1)'s did the trick) and then trying to dpkg -c *.deb | grep package.xml with and without the -q. With it, it often displays errors about a broken pipe.

The truth here is that it doesn't really matter. A match was found so grep exited, and we really don't care about the rest of the results of the dpkg call. If we really want to suppress the excess output, we can remove the -q and pipe stdout to /dev/null.

On an unrelated note, maybe we should escape the $ and quote the string to grep for clarity sake:

grep "./opt/ros/$ROSDISTRO/share/$SHORT_NAME/package.xml\$"
dirk-thomas commented 10 years ago

Sounds good. I guess the two dots should also be escaped?

I tried the following line with the job http://jenkins.ros.org/job/ros-indigo-rosserial-windows_binarydeb_trusty_amd64/13/ and it worked well:

dpkg -c $output_dir/*$distro*.deb | grep "\./opt/ros/$ROSDISTRO/share/$SHORT_NAME/package\.xml\$" > /dev/null || PACKAGE_XML_MISSING=$?

@cottsay If you agree I will commit that change directly referencing this ticket.

cottsay commented 10 years ago

Yes @dirk-thomas - I think that would be the best solution. Thanks for working with me to tweak this check.

dirk-thomas commented 10 years ago

Done.