Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-57587] Prevent GitSCMFileSystem NPE on '*' branch name #914

Merged

Conversation

MarkEWaite
Copy link
Contributor

@MarkEWaite MarkEWaite commented Jun 20, 2020

JENKINS-57587 - Prevent GitSCMFileSystem NPE on '*' branch

When a Pipeline job that is not a multibranch Pipeline job is defined to use '*' as its branch name and is configured with lightweight checkout, it reports a null pointer exception and fails the job. A branch name '*' should be disallowed for Pipeline jobs that are not multibranch Pipeline jobs because a wildcard branch name would cause the job to switch between different branches each time it detects a change on one of the branches in the repository. Switching branches within a job makes the change history and general user experience confusing for the user.

This has the questionable result that by avoiding the null pointer exception it now allows a Pipeline job that is not a multibranch Pipeline to switch from one branch to another inside the same job.

The technique of using a single job to validate multiple branches was commonly used with Freestyle jobs because they did not have the concept of multibranch. It is generally much better to allow Jenkins to manage the Pipeline jobs with multibranch rather than having a single job that switches between branches. However, a null pointer exception is not a friendly way to tell users that they should not use a specific technique.

Special thanks to Sven Hickstein for his suggestion to use the supports method to detect this case. It is a much simpler change thanks to his suggestion.

Checklist

  • I have read the CONTRIBUTING doc
  • I have referenced the Jira issue related to my changes in one or more commit messages
  • I have added tests that verify my changes
  • Unit tests pass locally with my changes
  • I have added documentation as necessary
  • No Javadoc warnings were introduced with my changes
  • No spotbugs warnings were introduced with my changes
  • I have interactively tested my changes
  • Any dependent changes have been merged and published in upstream modules (like git-client-plugin)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

@MarkEWaite MarkEWaite added the bugfix Fixes a bug - used by Release Drafter label Jun 21, 2020
This has the questionable result that by avoiding the null pointer
exception it now allows a Pipeline job that is not a multibranch
Pipeline to switch from one branch to another inside the same job.

The technique of using a single job to validate multiple branches was
commonly used with Freestyle jobs because they did not have the
concept of multibranch.  It is generally much better to allow Jenkins
to manage the Pipeline jobs with multibranch rather than having a
single job that switches between branches.  However, a null pointer
exception is not a friendly way to tell users that they should not
use a specific technique.
@MarkEWaite MarkEWaite force-pushed the avoid-NPE-on-wildcard-branch-name branch from d4650ed to 39e2962 Compare June 21, 2020 14:18
Copy link
Member

@fcojfernandez fcojfernandez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has the questionable result that by avoiding the null pointer exception it now allows a Pipeline job that is not a multibranch Pipeline to switch from one branch to another inside the same job.

I'm not a pipeline expert and I might be misunderstanding something, but I think we can consider this behaviour as the lesser evil. Fine with me

@MarkEWaite MarkEWaite merged commit fee2130 into jenkinsci:master Jun 22, 2020
@MarkEWaite MarkEWaite deleted the avoid-NPE-on-wildcard-branch-name branch June 22, 2020 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fixes a bug - used by Release Drafter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants