Fix #37: raise errors on build failures

[movitto]

Also handle a few edge cases / add some syntactic sugar

[movitto]

@Fryguy @jrafanie Updated, symlink is so gem is available as a source when building the rpm. Can send a follow up flushing out the TODO / extracting the URL w/ another PR. Shout out if there is anything else

[Fryguy]

I don't understand the symlink comment. Since you are doing it in the repo, doesn't that change the repo directory? If so it leaves a dirty tree around which wreaks havoc on code that is expecting a clean tree.

[movitto]

@Fryguy the gem needs to be in the local directory for the srpm command. Agree, ideally it should be in that method but added it to the current location in the update_to method as that is where the gem is readily available.

With PR #75 we add a "upstream_gem" method to the spec class to reference the gem which the spec corresponds to. Once that is in I can leverage that in the 'srpm' method to access the gem and will remove it from here.

[Fryguy]

Merged #75

[movitto]

@Fryguy rebased to move the ln to the correct location / use the newly added feature.

Koji doesn't output anything on stderr afaik, if the build fails you just get the message that a task failed and to check the url previously output for the logs.

As far as using run! / capturing the error vs using run / checking the status, I guess I'm not quite sure why the preference of one over the other. Can change it if its a consistency thing, but would think not raising / catching an exception needlessly would be desired. In any case am fine either way so long as we detect the error here (eg not just blindly pass it through).

[jrafanie]

@movitto @Fryguy I think it's a toss-up. Ideally, you'd use run! when non-zero return codes are the exception. Some commands like grep have multiple non-zero return code conditions so you might have to use run for these... For example grep returns 0 (found), 1 (not found), and 2 (for an error), where 0 and 1 are both non-exceptional cases.

Regardless, the exceptional cases need to raise consistent error classes with a common interface for getting information from the exception. For example, if you use parted, I believe it writes error information to stdout, so our exception might not have anything useful in it.

[Fryguy]

@jrafanie...The CommandResultError exception class has access to both stdout and stderr, so if you know a library acts inconsistently, like parted, you can access it via err.result.output instead of err.result.error.

[cfme-bot]

This pull request is not mergeable. Please rebase and repush.

[movitto]

@chessbyte updated, link is being created in repo, needed to build srpm. Added an ensure block after the srpm is built to cleanup link. Shout out if anything else needs tbd to see this in!

[cfme-bot]

This pull request is not mergeable. Please rebase and repush.

[coveralls]

Coverage increased (+0.58%) when pulling 326600a683070ee63f2747eabab59e8370b0174d on build_enhancements into 689b36d9e06e37d139c3f08eafcf87fb56b01237 on master.

[cfme-bot]

This pull request is not mergeable. Please rebase and repush.

[cfme-bot]

Checked commits https://github.com/ManageIQ/polisher/commit/3700dcda39c162212a68767763bb346dd4207b32 .. https://github.com/ManageIQ/polisher/commit/e6b9a5f4284eb077995a5dce296e3bc8b1044271 with rubocop 0.21.0 10 files checked, 4 offenses detected

lib/polisher/gem.rb

• [ ] Style - Line 65, Col 11 - PredicateName - Rename has_file_satisfied_by? to file_satisfied_by?.

spec/core_spec.rb

• [ ] Warn - Line 65, Col 28 - Void - Operator == used in void context.
• [ ] Warn - Line 66, Col 27 - Void - Operator == used in void context.

spec/rpm/spec_spec.rb

• [ ] Warn - Line 58, Col 34 - Void - Operator == used in void context.

[coveralls]

Coverage decreased (-0.24%) when pulling e6b9a5f4284eb077995a5dce296e3bc8b1044271 on build_enhancements into e42498ec1fd18d50d61e551fd9f0f3a2f3d906e4 on master.

[Fryguy]

@movitto I've lost track of where this PR began and where is had moved to with new features having been added. Can you split off the "Fix #37: raise errors on build failures" into its own PR, and perhaps split off the other commits if they are isolated into other PRs as well?

[movitto]

@Fryguy split / sent separate PR's. Closing this one.