Skip to content

Remove incorrect annotation on GitSCM.getBrowser() - #1731

Merged
MarkEWaite merged 1 commit into
jenkinsci:masterfrom
MarkEWaite:remove-wrong-getBrowser-annotation
Mar 11, 2025
Merged

Remove incorrect annotation on GitSCM.getBrowser()#1731
MarkEWaite merged 1 commit into
jenkinsci:masterfrom
MarkEWaite:remove-wrong-getBrowser-annotation

Conversation

@MarkEWaite

@MarkEWaite MarkEWaite commented Mar 11, 2025

Copy link
Copy Markdown
Contributor

Remove incorrect annotation on GitSCM.getBrowser()

Jesse Glick notes in JENKINS-75288:

adding @Whitelisted to a method already marked @Override is clearly a mistake—this would never have any effect.

I didn't realize that was a mistake when I added the annotation. Let's remove the annotation so that the code does not mislead someone to think that this can be used from a Pipeline without changes to hudson.scm.SCM.getBrowser().

In a multibranch Pipeline, the default checkout assigns current values to the scm object, including scm.browser. That value can't be read unless the hudson.scm.SCM.getBrowser() method is approved through script security.

Testing done

None. Already confirmed that the annotation has no effect, as noted in JENKINS-75288. Removing the annotation will continue to have no effect.

Submitter checklist

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

Jesse Glick notes in https://issues.jenkins.io/browse/JENKINS-75288:

> adding @Whitelisted to a method already marked @OverRide is clearly
> a mistake—this would never have any effect.

I didn't realize that was a mistake when I added the annotation.
Let's remove the annotation so that the code does not mislead someone
to think that this can be used from a Pipeline without changes to
hudson.scm.SCM.getBrowser().

In a multibranch Pipeline, the default checkout assigns current values
to the scm object, including scm.browser.  That value can't be read
unless the hudson.scm.SCM.getBrowser() method is approved through script
security.
@MarkEWaite
MarkEWaite requested a review from a team as a code owner March 11, 2025 00:50
@MarkEWaite MarkEWaite added the chore Reduces maintenance effort by changes not directly visible to users label Mar 11, 2025
@MarkEWaite
MarkEWaite merged commit 343b167 into jenkinsci:master Mar 11, 2025
@MarkEWaite
MarkEWaite deleted the remove-wrong-getBrowser-annotation branch March 11, 2025 01:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Reduces maintenance effort by changes not directly visible to users

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant