`trigger_files` doesn't seem to work when a PR is opened

[camelid]

For example: rust-lang/rust#91833

That PR was force-pushed here: rust-lang/rust#91833 (comment)

Both the old and the new versions modified rustdoc, but only after the force-push did the PR get autolabeled:

@Mark-Simulacrum I suspect there's something wrong in diff_between(). For one, it uses unwrap_or_default for the commit hashes, which seems like it'd introduce weird behavior. If the value isn't there, it'll just use an empty commit hash.

When you get a chance, could you look at the logs for that PR perhaps? Other than that, I don't know how to fix this.

[camelid]

Actually, autolabeling just seems broken altogether; it just added T-rustdoc to rust-lang/rust#91819.

[klensy]

This rust-lang/rust#91825 (comment) can be related?

[camelid]

No, that was fixed already.

[Mark-Simulacrum]

Okay, after some trial and error I think this is fixed on the current deploy; tests with force-pushes worked here rust-lang/rust#72571 (comment)

I haven't tested opening a new PR yet -- but I think we can move ahead on rust-lang/rust#91819 at least.

[camelid]

PR approved :)

[camelid]

I just realized a potential issue that may come up with this feature: If rustbot incorrectly labels a PR with, e.g., T-compiler, then every time the PR is pushed to, the label will have to be manually removed. I imagine it won't be an issue too often in practice though.

[Mark-Simulacrum]

Without storing metadata in the database I'm not sure how much better we can do. It'd theoretically be pretty cheap to add the database state but I'm a little reluctant to introduce that complexity.

That said it would be relatively simple to check just the pre-sync commit, I suppose, to see if that was already enough that the label should have been added..

In general it doesn't seem worth it to me though.

[camelid]

Yeah, I think we should only try to "fix" it if we run into issues.

[camelid]

I haven't tested opening a new PR yet

Ok, I just opened a new PR and the autolabeling didn't work. So I think there's still a bug in that part of the code.

Also, highfive thinks I'm a new contributor for some reason?

[Mark-Simulacrum]

Also, highfive thinks I'm a new contributor for some reason?

IIRC, this is based on the last ~100 PRs or something like that? Not sure.

Ok, I just opened a new PR and the autolabeling didn't work. So I think there's still a bug in that part of the code.

Yeah, seems so. Looking at that particular event, it looks like we .. just didn't deserialize the pull_request field. I think that's another case of serde not having compile-time duplicate detection, pull_request was already alias'd to the issue field. Pushing a fix shortly (done, ~15 minutes till production).

[camelid]

Fix confirmed to work: rust-lang/rust#91876

[Mark-Simulacrum]

rust-lang/rust#89831 (comment) has T-rustdoc being added spuriously(?)