Bug 33220

Summary:	Optional RPM task succeeds even if RPM process fails
Product:	Ant	Reporter:	Julian Simpson <julian.simpson>
Component:	Optional Tasks	Assignee:	Ant Notifications List <notifications>
Status:	RESOLVED FIXED
Severity:	normal	Keywords:	ErrorMessage
Priority:	P2
Version:	unspecified
Target Milestone:	---
Hardware:	All
OS:	Linux
Attachments:	Proposed patch to close this bug - examines exit code and throws buildException when neccessary. proposed patch - context diff Unit Test for bug fix. Context Diff patch for Rpm.java

Description Julian Simpson 2005-01-24 16:25:58 UTC

When making RPM's with the Ant task, the 'rpm' or 'rpmbuild' executables that get called by the rpm 
task can fail (because of errors in the spec file, for example), exit with a non-zero return code, and not 
break the build.  This is because the return code from the rpm executable isn't examined by the task.

I tested this with Ant 1.6.1 and 1.62 on debian linux, as well as ant 1.6.1 on Solaris 8 (with rpm 
installed locally).

I will attempt to post a patch.

Comment 1 Julian Simpson 2005-01-25 08:04:53 UTC

Created attachment 14088 [details]
Proposed patch to close this bug - examines exit code and throws buildException when neccessary.

I tested this under debian testing with a build of Ant from yesterday.	 When
rpmbuild doesn't exit cleanly, the build fails.  When rpmbuild does exit
cleanly, the build succeeds.

Comment 2 Julian Simpson 2005-02-01 15:41:14 UTC

Created attachment 14150 [details]
proposed patch -  context diff

sorry, forgot to do context diff - here it is.

Comment 3 Steve Loughran 2005-02-08 20:06:23 UTC

Fixed in CVS. Please test. Better yet. please can you give us a unit test?

Comment 4 Julian Simpson 2005-02-13 21:50:28 UTC

Created attachment 14273 [details]
Unit Test for bug fix.

This is the unit test for the bug.  I had to change the code under test
slightly so I could return the correct value without actually trying to execute
something.  That would have slowed the test down. I extracted a private method
out of the big "execute" method in Rpm.java and overrode that.	A context diff
for Rpm.java follows this attachment.

Comment 5 Julian Simpson 2005-02-13 21:51:57 UTC

Created attachment 14274 [details]
Context Diff patch for Rpm.java

This is a non-functional change to the class - I just pulled some of the
execute method into a private method so as to make the code more testable.

Comment 6 Julian Simpson 2005-02-13 22:55:34 UTC

(In reply to comment #3)
> Fixed in CVS. Please test. Better yet. please can you give us a unit test?

Thanks.  Unit test supplied as requested.
J.

Comment 7 Stefan Bodewig 2005-03-21 14:51:25 UTC

I've committed a modified version of the test, thanks.

Modified since we've now added a failonerror attribute to <rpm> (with default false
for backwards compatibility) and I've used the test as a basis to check both
states of this attribute.