Hi all,
I support Julius' proposition, I think this is a really good idea that would improve TF-A work flow, both for patch contributors and maintainers.
I too am unsure about the exact implications on the development of large features, long-standing patch stacks and the like. I can't think of any red flags, though, so I suggest we try to prototype something.
I can imagine we could use the staging instance of the OpenCI for that. For example, we could implement the rebase strategy on a branch of the tf-a-ci-scripts, and use that in the staging instance. When clicking the Allow-CI+1 label button, this would trigger the usual OpenCI job on the production CI servers with no visible change to TF-A contributors, but also trigger it on the staging instance in silent mode / private email mode. We could monitor that for some time before we take the plunge.
Cheers, Sandrine
On 8/16/22 17:17, Chris Kay via TF-A wrote:
I know that this will cause a patch that
actually doesn't rebase cleanly onto the current HEAD at the moment to fail the test, but isn't that a desirable feature?
I agree. I don’t think this would be a particularly difficult change to make, but I’m just wondering if there are any impacts that we have not considered, particularly for those working on larger features or refactoring work in high-traffic areas of the code-base.
I don't think this applies to the patches currently affected by this
problem. They fail because their parent is out of date with other repositories, not because the patch itself would somehow not apply cleanly. If it wasn't for the CI issue these patches should be able to get submitted (i.e. cherry-picked onto HEAD) cleanly by Gerrit without needing another rebase, because the rebase doesn't actually change anything about the patch itself, it just updates the parent.
Sure, I don’t mean that they would necessarily have to be rebased manually.
Chris
*From: *Julius Werner jwerner@chromium.org *Date: *Monday, 15 August 2022 at 22:21 *To: *Chris Kay Chris.Kay@arm.com *Cc: *Olivier Deprez Olivier.Deprez@arm.com, tf-a tf-a@lists.trustedfirmware.org, Glen Valante glen.valante@linaro.org, Don Harbin don.harbin@linaro.org, Julius Werner jwerner@chromium.org *Subject: *Re: [TF-A] Re: TF-A CI often fail due to missing rebase
Hi Chris, Olivier,
Thanks for the detailed explanation about the interaction with other repositories, I understand the cause of the issue better now.
However, I still feel like changing the strategy with which Jenkins tests the patches would be an elegant and hopefully not too complicated solution. I know that this will cause a patch that actually doesn't rebase cleanly onto the current HEAD at the moment to fail the test, but isn't that a desirable feature? If they don't rebase cleanly that means the author will need to manually intervene before submission anyway, and the CI is supposed to test whether the patch is ready for submission, so I think it makes sense for the CI to fail and notify the author of the problem in that case.
It’s usually not too much hassle, though, to just rebase, given that Gerrit can usually do it automatically with the click of a button. The patches will, inevitably, have to be rebased eventually before merging anyway.
I don't think this applies to the patches currently affected by this problem. They fail because their parent is out of date with other repositories, not because the patch itself would somehow not apply cleanly. If it wasn't for the CI issue these patches should be able to get submitted (i.e. cherry-picked onto HEAD) cleanly by Gerrit without needing another rebase, because the rebase doesn't actually change anything about the patch itself, it just updates the parent.
I know it's just one button click, but I think the problem is more that this issue is hard to understand for new contributors. Before you know to click that button you first have to dig through the CI interface, find the errors, be utterly confused why you're getting build errors in directories completely unrelated to your patch, ask a maintainer for help, and then be told to click the rebase button. If there is an easy automated solution to eliminate all this unnecessary friction, I think it would be nice to implement it.