On Sat, Jul 01, 2017 at 10:53:41PM +0200, Mojca Miklavec wrote:
On 1 July 2017 at 18:51, Clemens Lang wrote:On Sat, Jul 01, 2017 at 10:37:04AM +0000, Zero King wrote:Travis only fetches the merged ref `git fetch origin +refs/pull/542/merge:`, so it does complicate matters.Isn't that exactly what we want?
Except when it's a fast-forward merge, Travis would not fetch the commits in the PR. It only fetches the merge commit and some more commits in master.
I thought of `grep`ing the output for "Failed to parse file" and ignore broken Portfiles not touched by the PR like this one > Failed to parse file python/py-pydot/Portfile: can't read "_name": no such variable in https://travis-ci.org/macports/macports-ports/jobs/248896726.Is this the correct link?
Yes. Two ports failed and one of them is on our master branch.
Rather than parsing the output of portindex, could we just run 'port info' in the directory of the modified portfile? That would catch the parse error as well.If commit changed 20 directories (hopefully nobody submits another PR like the one to remove $Id lines in a couple of thousand ports :), the command would have to be executed in each and every one of them.
It could be executed once with all ports changed as arguments, but `portindex` is called before downloading CI bot and its dependencies. I'd rather kill a build early if it contains broken Portfiles.
I simply cannot decide which approach is less "hacky", but anything that works ...Changing portindex to return non-zero on parse error may not be what we want. As a user, I don't want my portindex to fail (and potentially abort further processing) if somebody accidentially commits a broken Portfile that I don't care about.OK, that's a somewhat valid point. Maybe something like "portindex --strict" would be suitable then :) Something that would return non-zero value the :) :) :) Just brainstorming, don't take my proposal too seriously.
We can patch the file on CI and leave it as it in -base. Please review https://github.com/macports/macports-ports/pull/550.
I would say that this is not something that we should spend too much time on for unrelated ports. I don't care if the complete build fails if another unrelated port was in a bad shape. It may be that the broken port is one of dependencies. The committer could have run "portindex" himself, or in any case can act upon the problem once discovered pretty quickly. What is really important is to fail the build when portindex fails on the submitted port. The use case that triggered this discussion was an example where I could perhaps easily push the changes if it was another much less relevant port submitted by maintainer. (I didn't see the problem when first inspecting the changes. If maintainer presumably tested it and the build succeeded ...)
The question is should I spend some time on filtering out irrelevant broken ports. The important thing is done in PR#550.
Mojca
-- Best regards, Zero King Don't trust the From address.
smime.p7s
Description: S/MIME cryptographic signature
