On 1 July 2017 at 10:06, Zero King <[email protected]> wrote: > On Sat, Jul 01, 2017 at 09:21:42AM +0200, Mojca Miklavec wrote: >> >> I would say that >> portindex || exit 1 >> should work. > > Does this make a difference? `portindex` returns 0 (success) even if > parse error occurred.
I'm sorry, I didn't know that. I only assumed that a non-zero value wouldn't prevent the bootstrap script to continue and exit with 0 at the end. In that case portindex should be fixed as a high priority and return non-zero value when it fails. (I still didn't test it). I guess nobody really cared about this so far because commits should generally never generate a parse error. >> When individual steps could fail and when I wanted to proceed >> executing the script to the end, I usually wrote a function that >> checked for failure status and only reported / generated a failure at >> the end. But I guess you can safely exit if portindex fails >> (potentially add some explicit sentence saying so if it's not clear >> from the error message). > > If portindex couldn't parse the Portfile, I'd say we shouldn't waste > Travis's resource on the PR. True. I wouldn't start the build anyway, I just wasn't sure whether you wanted to continue in some other way. > Another problem is that if our master branch contains broken Portfiles > (committed between PR parent commit and Travis build), should the build > fail and warn us or should we filter the irrelevant errors? This is a completely separate "can of worms" :) We should actually decide whether to build from master or from the point where the commit was branched. It can be that some dependencies were upgraded in the meantime (after PR was branched). Should the build use the new or the old dependencies then? One could argue that new ones make more sense because that's where the changes will eventually end up. But then this makes non-deterministic builds. Travis could launch three builds, some of them with dependencies already updated and some with the old ones. Or someone could commit something broken that gets fixed a few minutes later, but the travis build would start building from the broken commit and report a broken PR for no good reason. I would be slightly in favour of taking the parent of PR as a reference (that is: the exact state of the branch of the PR, so not even trying to create portindex from master) unless that unnecessarily complicates matters (if so, we can simply take trunk and stop worrying about anything; in most cases that won't make a difference anyway). What exactly did you have in mind with filtering? Mojca
