mc1arke / sonarqube-community-branch-plugin

A plugin that allows branch analysis and pull request decoration in the Community version of Sonarqube
GNU Lesser General Public License v3.0
2.19k stars 513 forks source link

Allow an empty project to scan with a long-term branch #32

Closed rpdai closed 4 years ago

rpdai commented 5 years ago

Hi,

Our builds have for a long time populated the sonar branch properties; this was established some years ago and before sonarqube would error out when they were used without the developer version. I figure this plugin can help such workflows migrate to 7.9 LTS, but I ran into the requirement to scan the "main" branch first.

So I modified the plugin to proceed directly with the branch name rather than error out in the CommunityBranchConfigurationLoader:

    if (projectBranches.isEmpty()) {
        if (isTargetingDefaultBranch(localSettings)) {
            return new DefaultBranchConfiguration();
        } else {
            String branchName = localSettings.get(ScannerProperties.BRANCH_NAME);
            String branchTarget = localSettings.get(ScannerProperties.BRANCH_TARGET);
            final BranchType branchType = computeBranchType(localSettings.get(CoreProperties.LONG_LIVED_BRANCHES_REGEX), branchName);
            return new CommunityBranchConfiguration(branchName, branchType, branchTarget, branchTarget, null);
        }
    }

And I found it worked just fine. You can go on to scan the master at a later date, or rename and delete branches as appropriate.

So the above code is what I'm using, and I think that it might help others in the same situation.

I didn't feel confident enough to make this a PR though, because I'm not sure what the purpose of this limitation was in the first instance. Also, I wasn't sure whether there could be some subtleties with target branches for short-lived branches. If so perhaps the solution could be the following:

    if (projectBranches.isEmpty()) {
        if (isTargetingDefaultBranch(localSettings)) {
            return new DefaultBranchConfiguration();
        } else {
            String branchName = localSettings.get(ScannerProperties.BRANCH_NAME);
            final BranchType branchType = computeBranchType(localSettings.get(CoreProperties.LONG_LIVED_BRANCHES_REGEX), branchName);
            if (branchType == BranchType.LONG) {
                return new CommunityBranchConfiguration(branchName, branchType, branchName, branchName, null);
            } else {
                throw MessageException
                        .of("No branches currently exist in this project. Please either scan the main branch or a long-lived branch first");

            }
        }
    }

But my preference would be the first, simpler approach which would basically make this plugin a complete solution to allow existing workflows that use sonar branch properties update to 7.9 LTS.

Thoughts?

mc1arke commented 5 years ago

The reason the restriction existed was due to the plugin implementing the same features as those referenced in SonarQube's documentation. See my last comment on #1 for details. Since the plugin is now starting to introduce features that don't completely match SonarQube's default implementation, I'd be happy to consider adding a global configuration entry to allow users/administrators to turn on the creation of default branches where they don't exist. However, this then leads to further discussion around what the default branch should be:

rpdai commented 5 years ago

I can certainly buy into the community plugin's default behaviour matching the developer edition, then make everything else optional configuration.

At present, when creating a project from the UI, the main branch is automatically created as "master". I think that should remain the default main branch. It's sensible for a lot of people and keeps us closer to base sonar. That is to say:

  1. switching on the creation for empty projects would be a boolean option.
  2. The initial main branch name would be separately controlled and it's behaviour should default to pick the name "master"

I'd suggest also, the main branch should end up the same whether the project is created manually or via scanning. That would be more user friendly, less to remember between the different ways of creating a project.

That helps narrow things down a bit. It rules out all options based on first branch scanned, because there is no scanned branch when a project is created through UI or web API. We are left with:

My thinking depends a lot on whether plugins can modify the way projects are created in general. Can we rename the main branch to 'sonar.branch.default' as soon as any project is created? I've had a look at the plugin API but I haven't quite gotten my head around it. Ideally, we would plug into project creation, not duplicate behaviour between creating the project at scanning and project created through UI, or even worse a third time when created with web API. I don't think scanning should have to worry about creating any branch other than the one that is scanned. The hack I did above, does successfully result in a new project with the scanned branch, and the "master" main branch. It didn't have to create the "master" branch, that was done elsewhere. I hope sonarqube has good pluggability here, so we can maintain that separation of concerns.

What I'd really like to do read the default branch from gitlab's web API. How awesome would that be? I think that's too much to ask for in the community branch plugin, as not everyone uses gitlab, but it'd be great to take this into account. If other plugins can control the default main branch name, then the community branch plugin doesn't have to try to cover as many use cases on its own.

sverhagen commented 4 years ago

default branch should be: ... Always the first branch scanned

For our workflow at least, this does not work well. Our default branch is master, yet all our new microservices start their life on a story branch, like story/JIRA-ID.

A new sonar.branch.default parameter provided during scanning

This would work for us, since we have standardized on master, and we'd just put this property in our centralized configuration with a fixed value of master.

teake commented 4 years ago

A new sonar.branch.default parameter provided during scanning

That would work for my workflow as well (git flow with develop as default). I'm using a slightly different workaround than @rpdai at the moment: https://github.com/teake/sonarqube-community-branch-plugin/commit/899e7a0e49f3c6c9c627a3ffca3af5848da94f99. But I would very much like for this to work out of the box.

mc1arke commented 4 years ago

Sonarqube 8.1 has changed the way branches work. There is no longer being a concept of short of long branches, and all branches now target the main branch which defaults to the first scanned branch. As this change effectively delivers what this issue was asking for, I'm closing this issue as completed.

rpdai commented 4 years ago

What do you mean, that all branches target the main branch?

Is there a reference for this change as I couldn't see it in the 8.1 changelog?

mc1arke commented 4 years ago

See https://docs.sonarqube.org/8.1/branches/overview/. You'll notice that the sonar.branch.target parameter is no longer available, and the whole section on short/long lives branches that was in https://docs.sonarqube.org/8.0/branches/overview/ has been removed.