renovatebot / renovate

Home of the Renovate CLI: Cross-platform Dependency Automation by Mend.io
https://mend.io/renovate
GNU Affero General Public License v3.0
17.89k stars 2.37k forks source link

Improve branchName structure #7737

Open rarkins opened 4 years ago

rarkins commented 4 years ago

What would you like Renovate to be able to do?

Today, branchTopic is defined as so:

https://github.com/renovatebot/renovate/blob/ad50398fb3981a7c5d04b96bddce3dbad45a7d21/lib/config/definitions.ts#L1264-L1271

It is also redefined in a few scenarios like. https://github.com/renovatebot/renovate/blob/ad50398fb3981a7c5d04b96bddce3dbad45a7d21/lib/config/definitions.ts#L1093 and https://github.com/renovatebot/renovate/blob/ad52ac9b13e7fdd87fbfcedfdfbfcf44a052ae0e/lib/workers/repository/updates/branch-name.ts#L44

I think we should make this simpler and more understandable if we divided it into something like branchTopic and branchTopicDivider. The divider is things like major, patch, digest, etc. The topic could then be computed in order of priority:

And an optional one we could let people group by is scopeName, which could be e.g. npm scopes like @angular or maven group IDs. We could have a new field branchTopicPriority like ['groupName', 'variableName', 'depName', 'scopeName'] and allow them to reordered by config.

Did you already have any implementation ideas?

This whole FR is essentially an implementation idea, so see above.

noelmccrory commented 3 years ago

Currently it doesn't seem like there is a way to customize the branch name when a Maven property is used for the version, e.g.

  <properties>
    <testcontainers.version>1.15.0</testcontainers.version>
  </properties>

Even though I have branchTopic set to {{{depNameSanitized}}}-{{{toVersion}}} , it still results in a branch being created with renovate/testcontainers.version as the name.

If this scenario could be included in this improvement, that'd be great. Thanks.

viceice commented 3 years ago

@noelmccrory please open a new config help discussion. You should be able to override the default behavior. So please share you config, maybe there is something wrong.

rarkins commented 3 years ago

In most cases our branch structure is like {{branchPrefix}}{{branchTopic}}{{branchBucket}}, where:

One other semi-common requirement is known as the additionalBranchPrefix. It has had two main uses:

The choice for this additional prefix in future is:

rarkins commented 3 years ago

The above idea would mean that the default behaviour would result in 1 or 2 buckets per topic - -non-major and -major. Enabling separateMinorPatch=true would mean -patch, -minor, and -major. Using a groupName would mean that non-major and major updates would be grouped separately, even if they are on different major versions. Similarly, setting separateMinorPatch=true while grouping would mean potentially three PRs - -patch, -non-major, -major.

rarkins commented 3 years ago

Per the above, setting an additional branch prefix is most closely related to branchTopic

rarkins commented 3 years ago

We should also think about depNameSanitized. Which is currently:

depName
                  .replace('@types/', '')
                  .replace('@', '')
                  .replace(/\//g, '-')
                  .replace(/\s+/g, '-')
                  .toLowerCase();

If we were to move the @types hack somewhere else, then essentially it's just replacing @, / and whitespace with - and then lowering case - not too complex.

viceice commented 3 years ago

We should also think about depNameSanitized. Which is currently:

depName
                  .replace('@types/', '')
                  .replace('@', '')
                  .replace(/\//g, '-')
                  .replace(/\s+/g, '-')
                  .toLowerCase();

If we were to move the @types hack somewhere else, then essentially it's just replacing @, / and whitespace with - and then lowering case - not too complex.

We should replace -+ with - as last.

rarkins commented 3 years ago

Another thing is to consider depNameShort in this context. e.g. if the dep is https://github.com/foo/bar, should we call depName foo/bar? And same if https://bitbucket.org/foo/bar? Same if https://enterprise.renovatebot.com/foo/bar?

I don't think the chances of name collision are very high, but we could make sure there's a way to fix it if it happens.

What about quay.io/foo/bar and docker.io/foo/bar? BTW we could already deconstruct these into registryUrls/depName anyway. One downside of this though: Docker images are pretty common in regex, and it could make the regex manager harder to write.

rarkins commented 3 years ago

Maybe we can have an option shortenDepName which defaults to true and when that's present we have algorithms for creating a simpler depName, e.g. removing the host. If it's configurable then people could still disable it, including per-package.

rarkins commented 3 years ago

I forgot something important. I want to default to using a shortened version of the sourceUrl instead of depName, to achieve:

So then the priority of topic would be:

Instead of requiring it to be in a template (e.g with lots of if/else) we could have a priority array, which defaults to ["groupName", "variableName", "sourceRepoShort", "depName"] but would let people reorder, e.g. to ["groupName", "depName"] to keep today's functionality.

viceice commented 3 years ago

Sounds good. 🤗

rarkins commented 3 years ago

We need to work out how lock file maintenance fits in. Today we treat it as a topic with no bucket.

Additionally when we group automatically by sourceRepoShort then we will need a way to generate what the PR title should be. e.g. instead of "Update babel/babel packages to v7.1.0" it would ideally be "Update babel packages to v7.1.0". We should have ways to intelligently detect things like the package.json>name value but also allow it to be overridden by config. Simply using the repo name stripped of org name could also work quite well in the majority of cases, which we can verify here: https://github.com/renovatebot/renovate/blob/master/lib/config/presets/internal/monorepo.ts

rarkins commented 3 years ago

branchBucket should also distinguish between updates of the same dependency with different versions. e.g. if you have two copies of a dependency - one of them 1.0.0 and second one 2.0.0 and then 3.0.0 is available then we should separate 1->3 and 2->3

rarkins commented 1 year ago

Current thinking:

branchName = {{branchPrefix}}{{branchDivider}}{{branchTopic}}-{{branchBucket}}

Of direct relevance, it is my goal to reduce the amount of explicit grouping required in Renovate by defaulting branchTopic to default to a slug of the source repo, e.g. jestjs-jest for https://github.com/jestjs/jest. This way, monorepo grouping is Renovate's default behavior and not a manually crowd-sourced add-on.

When there are major updates of a single package, we would want to separate buckets based on from/to major version, e.g.

The above would apply regardless of whether separateMultipleMajor=true or false. It would also usually apply to monorepo groups, such as all angular upgrades.

Other example buckets would be:

However when unrelated packages are grouped together then we'd want the bucket to simply be "major", "non-major", etc. so that you can get one branch. It seems easy to divide grouped PRs into major and non-major like our defaults, but hard to know what to do if the user has configured non-default separateMinorPatch=true or separateMultipleMajor=true settings, and even harder if they are mixed between packages in the same group. It's also challenging because grouping might be applied after buckets are decided, e.g. someone groups non-major updates. We could have a simpleBucket concept which is applied by default for grouped updates.

rarkins commented 1 year ago

With default config (separateMajorMinor=true, separateMultipleMajor=false): we get two buckets, major and non-major.

If separateMajorMinor=false, then there's only one bucket - could be latest or ''. No need to look at other configs

If separateMinorPatch=true, then buckets are major, minor and patch.

If separateMultipleMajor=true, there's no major bucket and instead v3, v4, etc.

If (future) separateMultipleMinor=true, then the buckets can be v2.1, v2.2, etc.

If (possible future) separateMultiplePatch=true, then the buckets will be v2.3.4, v2.3.5, etc.

I think we also need to clarify that if you separate major, minor or patch AND try to group (e.g. groupAll) then you're usually going to get undesirable behavior and should not separate.

I have left out a few others buckets though:

I think we probably want to get rid of the -lockfile suffix which exists today for rangeStrategy=update-lockfile and instead make those updates one of the above.

We also need to consider lock file maintenance (which should be renamed). I would like for that to now be formally discouraged to group together with other updates, although still possible to separate by file or manager if desired. By using "refresh" as the "bucket" we could maybe achieve this separation.

rarkins commented 1 year ago

Now thinking to use subject instead of topic as it was already used:

branchName = {{branchPrefix}}{{branchDivider}}{{branchSubject}}-{{branchBucket}}

Also thinking we should do branchNameStrict better along with this change, because we don't have to worry about renames when we're already renaming.

For my use, I'd only want branches with english lower case letters (a-z), digits (0-9), hyphen (-), and period (.). I can see others might not want periods. All characters which can't be replaced with a-z could be replaced with hyphens and then multiple hyphens reduced plus any trailing hyphen removed.

rarkins commented 1 year ago

Blocked by #19980