jenkins icon indicating copy to clipboard operation
jenkins copied to clipboard

Feat: First Backporting for LTS 2.361.2

Open krisstern opened this issue 3 years ago • 18 comments

The below are the LTS candidate stats for the firt

Latest core version: jenkins-2.363

Postponed
---------

JENKINS-69534           Major                   2.369
        File descriptor leak on slave.log when using cloud agents (regression in 2.294)
        regression
        https://issues.jenkins.io/browse/JENKINS-69534

JENKINS-69257           Minor                   2.369
        Model link chevron is not in center in user build cause on console log
        regression
        https://issues.jenkins.io/browse/JENKINS-69257

JENKINS-67864           Minor                   2.369
        Table columns get wider and smaller if you click on buttons
        https://issues.jenkins.io/browse/JENKINS-67864

Fixed
-----

JENKINS-69570           Minor                   2.368
        Java version check incorrect in init.d/jenkins
        https://issues.jenkins.io/browse/JENKINS-69570

JENKINS-69543           Critical                2.368
        CPU pegged handling WebSocket agent connection due to thread-unsafe code
        regression
        https://issues.jenkins.io/browse/JENKINS-69543

JENKINS-69467           Minor                   2.367
        Plugin manager selection controller buttons appearance issue
        https://issues.jenkins.io/browse/JENKINS-69467

JENKINS-69320           Minor                   2.365
        Resize behavior of Execute Shell build steps is broken (regression in 2.321 or 2.357, depending)
        regression
        https://issues.jenkins.io/browse/JENKINS-69320

JENKINS-68805           Minor                   2.367
        Symbol cache does not properly reset attributes
        https://issues.jenkins.io/browse/JENKINS-68805

JENKINS-67624           Minor                   2.366
        FileAlreadyExistsException /root/.jenkins/updates
        regression
        https://issues.jenkins.io/browse/JENKINS-67624

Desired reviewers

@timja @basil @MarkEWaite @NotMyFault

krisstern avatar Sep 17 '22 11:09 krisstern

Yeah, I had some issues with the file war/src/main/webapp/scripts/hudson-behavior.js while doing a cherry pick for https://github.com/jenkinsci/jenkins/commit/4d5fd8958cc5d7cc444d6a9f4422d7bd046b1fe0, as there were some merge conflicts. Let me see how I can best resolve this.

krisstern avatar Sep 18 '22 10:09 krisstern

I am not sure if this is the best way to resolve the merge conflicts... Is there any alternative suggestion? It seems like every time when I do git cherry-pick -x 4d5fd8958cc5d7cc444d6a9f4422d7bd046b1fe0 I got the following:

Auto-merging war/src/main/webapp/scripts/hudson-behavior.js
CONFLICT (content): Merge conflict in war/src/main/webapp/scripts/hudson-behavior.js
error: could not apply 4d5fd8958c... [JENKINS-69320] Resize behavior of Execute Shell build steps is broken (#6999)

krisstern avatar Sep 18 '22 10:09 krisstern

The backport of 31329fb is not correct. The origin commit, 4d5fd89, is a one-liner when the backported one is not.

Ensure you're using the post-prettier version, given it's present on the stable branch, when resolving conflicts, instead of reintroducing the old formatting, which causes the build failure.

I did check again. I think it was effectively a one-liner change, except for some reason my IDE formatted some other parts of the code in the same file as well. So something else is causing the failure.

krisstern avatar Sep 18 '22 16:09 krisstern

I suspect it could be due to the some Prettier changes as I see a similar traceback in the logs.

krisstern avatar Sep 18 '22 16:09 krisstern

except for some reason my IDE formatted some other parts of the code in the same file as well

I can't tell what went wrong on your end, but my cherry-pick of https://github.com/jenkinsci/jenkins/commit/4d5fd8958cc5d7cc444d6a9f4422d7bd046b1fe0 was much cleaner than yours 👀 Possibly caused by an outdated local branch?

Anyway, I went ahead and rebased the PR to a) warrant a clean commit history and b) to include the correct part of https://github.com/jenkinsci/jenkins/commit/4d5fd8958cc5d7cc444d6a9f4422d7bd046b1fe0 without your additional edits.

NotMyFault avatar Sep 18 '22 16:09 NotMyFault

Postponed

To be reconsidered for 2.361.3?

Or any later LTS release. The script adds the entry, if you label a Jira issue as rejected, in case you were unaware of that 👀 I'm not sure why the Jira issue was labeled as rejected, because it's not eligible for a backport anyway.

I'd also be in favor of dropping the couple of commits, that have not been tested via weeklys before, given that's the typical workflow. Although, you could consider them as marginal changes, tested by PR reviewers and now Basil, I don't think we should diverge from the common workflow of testing changes in weeklys prior to backporting.

NotMyFault avatar Sep 18 '22 18:09 NotMyFault

Thanks @NotMyFault It's probably because my local branches are behind...

krisstern avatar Sep 19 '22 11:09 krisstern

Just made a PR for JENKINS-69570 at https://github.com/jenkinsci/packaging/pull/336.

krisstern avatar Sep 19 '22 12:09 krisstern

Postponed

To be reconsidered for 2.361.3?

JENKINS-69570 Minor 2.368 Java version check incorrect in init.d/jenkins

I see no evidence that jenkinsci/packaging#335 has been backported to stable-2.361.

JENKINS-69257 Minor 2.369 Model link chevron is not in center in user build cause on console log

Note that this fix has not yet shipped in a weekly, but I did verify the fix manually on the main branch in #6970 (comment).

JENKINS-68805 Minor 2.367 Symbol cache does not properly reset attributes

This fix consisted of three commits on the main branch: commit d977ee8, commit 01e637e, and commit fd33b5f. IconSetTest#getSymbol_notSettingTooltipDoesntAddTooltipAttribute_evenWithAmpersand should be deleted to fully backport commit 01e637e, as the test was originally added to the wrong file and later moved into a separate file as required to exercise the use case.

JENKINS-67864 Minor 2.369 Table columns get wider and smaller if you click on buttons

Note that this fix has not yet shipped in a weekly, but as a courtesy I verified the fix manually on the main branch in #6702 (comment). In the future, please ensure that verification has taken place prior to filing a backport PR.

Okay 👍🏼 noted, and I will be more mindful about the review process next time.

krisstern avatar Sep 19 '22 12:09 krisstern

I can and will fix the JIRA issues labeling once this PR is approved.

krisstern avatar Sep 19 '22 12:09 krisstern

Thanks for the comment @basil

Regarding the remaining open issues:

  1. Yes, I think it is better to postpone JENKINS-69534 to be reconsidered for LTS 2.361.3.
  2. Should I undo the cherry pick for JENKINS-68805 and follow the instructions here?:

    This fix consisted of three commits on the main branch: commit d977ee8, commit 01e637e, and commit fd33b5f. IconSetTest#getSymbol_notSettingTooltipDoesntAddTooltipAttribute_evenWithAmpersand should be deleted to fully backport commit 01e637e, as the test was originally added to the wrong file and later moved into a separate file as required to exercise the use case.

krisstern avatar Sep 20 '22 14:09 krisstern

The reasons behind the postponement is based on the following comment made on the JIRA ticket for JENKINS-69534 by @basil:

I am applying the lts-candidate label with the following justification: this is a fix for a regression in 2.294 that, while impacting a low number of users, is high in severity for the impacted users. While the fix carries some degree of risk, we are early in the LTS development cycle, and there is plenty of time for these changes to be deployed and tested by users of weekly releases. If a regression is discovered between now and the next LTS backporting window, this justification can be reconsidered.

To be on the safe side I have labeled this lts-candidate as 2.361.2-rejected, so that it will be reconsidered for the LTS 2.361.3 release.

krisstern avatar Sep 20 '22 15:09 krisstern

Can we drop the commits that haven't been tested in the weekly please?

https://issues.jenkins.io/browse/JENKINS-69257 https://issues.jenkins.io/browse/JENKINS-67864

timja avatar Sep 20 '22 15:09 timja

Can we drop the commits that haven't been tested in the weekly please?

https://issues.jenkins.io/browse/JENKINS-69257 https://issues.jenkins.io/browse/JENKINS-67864

Yup, sure. It's done.

krisstern avatar Sep 20 '22 15:09 krisstern

Should I undo the cherry pick for JENKINS-68805 and follow the instructions here?

What you have done already is not incorrect but just incomplete, so to complete it you need to delete IconSetTest#getSymbol_notSettingTooltipDoesntAddTooltipAttribute_evenWithAmpersand per my previous comment as in commit 01e637ec08fddc9a032837ebd838cccdefe4da83.

basil avatar Sep 20 '22 16:09 basil

Please update the PR description as accordingly, after addressing the review comments.

NotMyFault avatar Sep 20 '22 16:09 NotMyFault

Please update the PR description as accordingly, after addressing the review comments.

No problem, PR description has been updated.

krisstern avatar Sep 20 '22 16:09 krisstern

Should I undo the cherry pick for JENKINS-68805 and follow the instructions here?

What you have done already is not incorrect but just incomplete, so to complete it you need to delete IconSetTest#getSymbol_notSettingTooltipDoesntAddTooltipAttribute_evenWithAmpersand per my previous comment as in commit 01e637e.

Thanks for the clarification! Think I was able to fix it.

krisstern avatar Sep 20 '22 16:09 krisstern

Thanks guys for the approvals!

krisstern avatar Sep 22 '22 10:09 krisstern