bloom 0.5.1 still generates invalid pull reuqests

[dirk-thomas]

https://github.com/ros/rosdistro/pull/3293 https://github.com/ros/rosdistro/pull/3356

[ahendrix]

In particular, this seems to happen when making several releases quickly in sequence, when the first release is merged around the same time that the PR for the second release is generated.

[wjwwood]

I added a check, but it seems to insufficient at catching some of the scenarios. I would have to look at the whole process and consider places where it could get out of sync... I don't have time at the moment though.

[dirk-thomas]

We need to address this issue. The number of wrong PRs is just ridiculous and we pay with more effort on a daily base for every day we don't address this.

[mikepurvis]

Would it be worth thinking again about #208 for this?

If a user knows they're releasing a bunch of repos all together, allow bloom to create a single PR for the bunch of them, or at least augment the user's already-in-queue PR rather than creating a separate one based from master.

[tfoote]

208 is orthogonal to this. If it was batched, you'd have batched diffs which downgraded parallel releases (note it's a race condition against all releases, not just yourself) And I'd still rather have the granularity to consider accepting or rejecting each package. Or if there's a regression rolling back a specific commit. If releases are bundled they must be unbundled by manually.

[mikepurvis]

Another possibility is a webhook script which auto-merges the PR as soon as its created, at least for some combination of trusted releaser + simple version-up PR.

You still get the ability to go back and fix it later, and you're not giving the entire world write-access to ros/rosdistro, but you also considerably narrow the window for the race condition to occur.

[wjwwood]

Merging them faster is actually more like to cause the issue, the problem occurs when a pull request is merged during a bloom release. In the common scenario people are releasing multiple times in a row and one gets merged as they are running the others. Instead it would probably be better to wait until there has been no activity for a while and then merge them all in batch.

However, this is just addressing a symptom, the problem is that the distro file which bloom starts with has no associated git hash, if it did I could assert in bloom that the pull request was made against the correct rosdistro commit hash and the problem would go away. Getting a rosdistro distribution file associated with a commit hash of upstream is the hard part. I estimate that it would take 4-8 hours to implement, test, and release this. We'll see if we can allocate some time for it in the near term since so many people are being affected.

[tfoote]

Auto-merging is not a good idea. The whole point of this process is to be able to review the changes as they come in. We are quite liberal with our merging, but if it's something like a beginner user screws up and types in "bloom-release ros_comm -r hydro -t hydro --edit" and enters all their info we'll catch that level of gross error. We also are checking for good naming practices etc.

[chadrockey]

Just got hit by this. Tried to release again with -p, but it seems like I can't release the repo anymore, with or without -p.

==> Pushing changes to fork
To https://c846737da56abc0ec20d86cda3e8893d9f556589:x-oauth-basic@github.com/chadrockey/rosdistro.git
 ! [rejected]        bloom-urg_c-0 -> bloom-urg_c-0 (fetch first)
error: failed to push some refs to 'https://c846737da56abc0ec20d86cda3e8893d9f556589:x-oauth-basic@github.com/chadrockey/rosdistro.git'
hint: Updates were rejected because the remote contains work that you do
hint: not have locally. This is usually caused by another repository pushing
hint: to the same ref. You may want to first integrate the remote changes
hint: (e.g., 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.
Failed to open pull request: CalledProcessError - Command 'git push https://c846737da56abc0ec20d86cda3e8893d9f556589:x-oauth-basic@github.com/chadrockey/rosdistro.git bloom-urg_c-0' returned non-zero exit status 1

[wjwwood]

It doesn't seem like it would be possible to get that error repeatedly @chadrockey, can you confirm that running again with --pull-request-only does not work?

[chadrockey]

It was very weird, multiple attempts with and without -p did not work. I ended up merging outstanding pull requests, then deleting my fork, and letting bloom recreate it, then the problem went away.

[wjwwood]

I've made some changes in 0.5.20 which should resolve this (hopefully). I'll come back and reopen this in a few days if we see it anymore.

[dirk-thomas]

While much less common this is still happening with the current version 0.5.20: https://github.com/ros/rosdistro/pull/9930/

[norro]

This one can be closed, I suppose? Bloom is creating working PRs by now ;)