publish_toolstate: don't use 'new' from inside the loop

[RalfJung]

I think I made a mistake in #61938 by using new outside the inner loop. This PR fixes that.

However, no issue got created at all for #62003 (comment), and I don't know why that is. The script should be triggered by Traivs, and I can find no trace of that in the build log.

[rust-highfive]

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

[RalfJung]

Cc @kennytm @rust-lang/infra

[RalfJung]

Oh I see, the push would happen elsewhere, namely when the branch actually hits master. That would be this log. And that log contains I/O error: HTTP Error 422: Unprocessable Entity. Not sure why though.

[ehuss]

Yea, I noticed the same thing in #61693. Clippy has never opened a new issue when it has failed. Unfortunately the body of the error is not logged. I recommend at least updating the error handling to do that to understand why it is hitting 422.

I thought about running the command locally with my own api key to see what the message is, but I was concerned about creating issues and pinging a bunch of people if it worked.

[alexcrichton]

r? @kennytm

[RalfJung]

I recommend at least updating the error handling to do that to understand why it is hitting 422.

How would I do that? I don't know the involved python libs very well.

[ehuss]

Something like the following should work.

diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py
index 9e7c18b7f5..c7bfd8a1ab 100755
--- a/src/tools/publish_toolstate.py
+++ b/src/tools/publish_toolstate.py
@@ -164,6 +164,8 @@ def update_latest(
                         tool, new, MAINTAINERS.get(tool, ''),
                         relevant_pr_number, relevant_pr_user, pr_reviewer,
                     )
+                except urllib2.HTTPError as e:
+                    print("HTTPError: {0} {1}".format(e, e.read()))
                 except IOError as e:
                     # network errors will simply end up not creating an issue, but that's better
                     # than failing the entire build job

[RalfJung]

Okay, I added something like that.

[rust-highfive]

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

travis_time:end:0446759a:start=1561137888923664985,finish=1561138036291350656,duration=147367685671
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
$ export AWS_ACCESS_KEY_ID=AKIA46X5W6CZEJZ6XT55
---

[00:03:51] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:03:51] tidy error: /checkout/src/tools/publish_toolstate.py:172: line longer than 100 chars
[00:03:56] some tidy checks failed
[00:03:56] 
[00:03:56] 
[00:03:56] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:03:56] 
[00:03:56] 
[00:03:56] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:03:56] Build completed unsuccessfully in 0:01:14
---
travis_time:end:14fa9ac0:start=1561138283340483535,finish=1561138283345289398,duration=4805863
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:12f39a2c
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:01864245
travis_time:start:01864245
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:115ffda0
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

[kennytm]

{"message":"Validation Failed","errors":[{"value":"llogiq","resource":"Issue","field":"assignees","code":"invalid"}],"documentation_url":"https://developer.github.com/v3/issues/#create-an-issue"}

🤔

Edit: Seems unassignable users cannot be present in the assignees field. After removing all non-team members in 902aeb2, highfive is able to create the issue #62034. Is the GitHub API docs lying?

Logins for Users to assign to this issue. NOTE: Only users with push access can set assignees for new issues. Assignees are silently dropped otherwise.

[ehuss]

That's weird, perhaps the assignee must be at least a collaborator? I can't find anything that mentions any restrictions like that. However, that makes sense because if you go to a github issue and click "assign", the only options are members/collaborators.

I read that statement in the docs that if you don't have "push" access, all assignees will be dropped. Presumably the bot has push access, but this is hitting the additional requirements.

[RalfJung]

Oooh so this is actually unrelated to my PR, and related to whether the author of the breaking PR is a team member? That explains so much! I had noticed a missing Miri issue some time ago already but it worked the next time so I thought it was fixed.

So probably having the PR author as assignee is not a good idea then?

[RalfJung]

@kennytm so this should fix the problem of not being able to assign non-team-members, and it also adds some more logging for the next time we have a problem. Does that seem reasonable?

[kennytm]

@bors r+ rollup

[bors]

📌 Commit 02863a3 has been approved by kennytm