merge with develop when using --from-pr#1838
Conversation
| try: | ||
| pr = int(pr) | ||
| except ValueError: | ||
| raise EasyBuildError("PR number is not an integer") |
There was a problem hiding this comment.
include pr (as a value) in the error message
|
@Caylo I briefly discussed this with @wpoely86, and he made a good point that we shouldn't just try to blindly apply the patch and fall back to using the PR branch as is, since we can actually check whether it makes sense to try and apply the patch... If the PR is not merged yet, applying the patch to If the PR is already merged, there's no point in trying to apply the patch to Long story short: this needs a bit more thought... :) |
|
|
|
||
| if not result: | ||
| raise EasyBuildError("Patching with patch %s failed", patch_file) | ||
| raise EasyBuildError("Couldn't apply patch") |
There was a problem hiding this comment.
keep mentioning name of patch file, and maybe also grab the output from the patch command and include it in the error message (you'll need to use simple=False in run_cmd and do out, ec = run_cmd(...)
| out, ec = run.run_cmd(patch_cmd, simple=False, path=adest, log_ok=False) | ||
|
|
||
| if ec: | ||
| raise EasyBuildError("Couldn't apply patch file %s. Process exited with code %s: %s", patch_file, out, result) |
There was a problem hiding this comment.
result is undefined here?
this points out that the tests should be enhanced to check for a failing patch, which should raise this error (rather than a traceback)
see #1838 (comment)
fixes #1292