diff --git a/lib/modules/platform/default-scm.spec.ts b/lib/modules/platform/default-scm.spec.ts index 28826974dcd88ad45f4c571e2ca895a7f59d870a..023948fa3dacaec6f2e22f0474dd94533029ba16 100644 --- a/lib/modules/platform/default-scm.spec.ts +++ b/lib/modules/platform/default-scm.spec.ts @@ -45,7 +45,7 @@ describe('modules/platform/default-scm', () => { it('delegate isBranchModified to util/git', async () => { git.isBranchModified.mockResolvedValueOnce(true); - await defaultGitScm.isBranchModified('branchName', 'main'); + await defaultGitScm.isBranchModified('branchName'); expect(git.isBranchModified).toHaveBeenCalledTimes(1); }); }); diff --git a/lib/modules/platform/default-scm.ts b/lib/modules/platform/default-scm.ts index cf30e1e6bbd674ba6b2b11c59a20e21708984d4f..d21f3cff0b832eb826322c473558475dc2309f1a 100644 --- a/lib/modules/platform/default-scm.ts +++ b/lib/modules/platform/default-scm.ts @@ -27,7 +27,7 @@ export class DefaultGitScm implements PlatformScm { return git.isBranchConflicted(baseBranch, branch); } - isBranchModified(branchName: string, baseBranch?: string): Promise<boolean> { - return git.isBranchModified(branchName, baseBranch); + isBranchModified(branchName: string): Promise<boolean> { + return git.isBranchModified(branchName); } } diff --git a/lib/modules/platform/types.ts b/lib/modules/platform/types.ts index 1cd330290f8141331c92f32f8c0a1ea1761a06d3..d37eae8db6c50a30f114545e4728619c17b62243 100644 --- a/lib/modules/platform/types.ts +++ b/lib/modules/platform/types.ts @@ -218,7 +218,7 @@ export interface Platform { export interface PlatformScm { isBranchBehindBase(branchName: string, baseBranch: string): Promise<boolean>; - isBranchModified(branchName: string, baseBranch?: string): Promise<boolean>; + isBranchModified(branchName: string): Promise<boolean>; isBranchConflicted(baseBranch: string, branch: string): Promise<boolean>; branchExists(branchName: string): Promise<boolean>; getBranchCommit(branchName: string): Promise<CommitSha | null>; diff --git a/lib/util/git/index.spec.ts b/lib/util/git/index.spec.ts index 7cdbf5e507fa1684935c046629ef3504a7920052..1f205f85bd489361f379d7e9d95ffff6b0dca90a 100644 --- a/lib/util/git/index.spec.ts +++ b/lib/util/git/index.spec.ts @@ -77,20 +77,6 @@ describe('util/git/index', () => { await repo.addConfig('user.email', 'custom@example.com'); await repo.commit('custom message'); - await repo.checkoutBranch('renovate/multiple_commits', defaultBranch); - await fs.writeFile(base.path + '/commit1', 'commit1'); - await repo.add(['commit1']); - await repo.addConfig('user.email', 'author1@example.com'); - await repo.commit('commit1 message'); - await fs.writeFile(base.path + '/commit2', 'commit2'); - await repo.add(['commit2']); - await repo.addConfig('user.email', 'author2@example.com'); - await repo.commit('commit2 message'); - await fs.writeFile(base.path + '/commit3', 'commit3'); - await repo.add(['commit3']); - await repo.addConfig('user.email', 'author1@example.com'); - await repo.commit('commit3 message'); - await repo.checkoutBranch('renovate/nested_files', defaultBranch); await fs.mkdirp(base.path + '/bin/'); await fs.writeFile(base.path + '/bin/nested', 'nested'); @@ -289,16 +275,11 @@ describe('util/git/index', () => { it('should return false when branch is not found', async () => { expect(await git.isBranchModified('renovate/not_found')).toBeFalse(); - expect( - await git.isBranchModified('renovate/not_found', defaultBranch) - ).toBeFalse(); }); it('should return false when author matches', async () => { expect(await git.isBranchModified('renovate/future_branch')).toBeFalse(); - expect( - await git.isBranchModified('renovate/future_branch', defaultBranch) - ).toBeFalse(); + expect(await git.isBranchModified('renovate/future_branch')).toBeFalse(); }); it('should return false when author is ignored', async () => { @@ -306,42 +287,15 @@ describe('util/git/index', () => { gitIgnoredAuthors: ['custom@example.com'], }); expect(await git.isBranchModified('renovate/custom_author')).toBeFalse(); - expect( - await git.isBranchModified('renovate/custom_author', defaultBranch) - ).toBeFalse(); - }); - - it('should return false if every author is ignored with multiple commits', async () => { - git.setUserRepoConfig({ - gitIgnoredAuthors: ['author1@example.com', 'author2@example.com'], - }); - expect( - await git.isBranchModified('renovate/multiple_commits', defaultBranch) - ).toBeFalse(); }); it('should return true when custom author is unknown', async () => { expect(await git.isBranchModified('renovate/custom_author')).toBeTrue(); - expect( - await git.isBranchModified('renovate/custom_author', defaultBranch) - ).toBeTrue(); - }); - - it('should return true if any commit is modified', async () => { - git.setUserRepoConfig({ - gitIgnoredAuthors: ['author1@example.com'], - }); - expect( - await git.isBranchModified('renovate/multiple_commits', defaultBranch) - ).toBeTrue(); }); it('should return value stored in modifiedCacheResult', async () => { modifiedCache.getCachedModifiedResult.mockReturnValue(true); expect(await git.isBranchModified('renovate/future_branch')).toBeTrue(); - expect( - await git.isBranchModified('renovate/future_branch', defaultBranch) - ).toBeTrue(); }); }); diff --git a/lib/util/git/index.ts b/lib/util/git/index.ts index 8aea27da4d2c0690b17b64a4c93d13cc07c6be44..2df34dfdcd43b9dfc305fbb759f4fd340929b38f 100644 --- a/lib/util/git/index.ts +++ b/lib/util/git/index.ts @@ -603,10 +603,7 @@ export async function isBranchBehindBase( } } -export async function isBranchModified( - branchName: string, - baseBranch?: string -): Promise<boolean> { +export async function isBranchModified(branchName: string): Promise<boolean> { if (!branchExists(branchName)) { logger.debug('branch.isModified(): no cache'); return false; @@ -626,48 +623,21 @@ export async function isBranchModified( return isModified; } + logger.debug('branch.isModified(): using git to calculate'); + await syncGit(); - // Retrieve the commit authors - let branchAuthors: string[] = []; + // Retrieve the author of the most recent commit + let lastAuthor: string | undefined; try { - if (baseBranch) { - logger.debug( - `branch.isModified(): using git to calculate authors between ${branchName} and ${baseBranch}` - ); - branchAuthors = [ - ...new Set( - ( - await git.raw([ - 'log', - '--pretty=format:%ae', - `origin/${branchName}...origin/${baseBranch}`, - '--', - ]) - ) - .trim() - .split('\n') - ), - ]; - } else { - logger.debug( - `branch.isModified(): checking last author of ${branchName}` - ); - branchAuthors = [ - ...new Set( - ( - await git.raw([ - 'log', - '-1', - '--pretty=format:%ae', - `origin/${branchName}`, - '--', - ]) - ) - .trim() - .split('\n') - ), - ]; - } + lastAuthor = ( + await git.raw([ + 'log', + '-1', + '--pretty=format:%ae', + `origin/${branchName}`, + '--', + ]) + ).trim(); } catch (err) /* istanbul ignore next */ { if (err.message?.includes('fatal: bad revision')) { logger.debug( @@ -676,19 +646,26 @@ export async function isBranchModified( ); throw new Error(REPOSITORY_CHANGED); } - logger.warn({ err }, 'Error retrieving git authors for isBranchModified'); + logger.warn({ err }, 'Error checking last author for isBranchModified'); } const { gitAuthorEmail } = config; - let branchIsModified = false; - for (const author of branchAuthors) { - if (author !== gitAuthorEmail && !config.ignoredAuthors.includes(author)) { - branchIsModified = true; - } + if ( + lastAuthor === gitAuthorEmail || + config.ignoredAuthors.some((ignoredAuthor) => lastAuthor === ignoredAuthor) + ) { + // author matches - branch has not been modified + logger.debug('branch.isModified() = false'); + config.branchIsModified[branchName] = false; + setCachedModifiedResult(branchName, false); + return false; } - logger.debug(`branch.isModified() = ${branchIsModified}`); - config.branchIsModified[branchName] = branchIsModified; - setCachedModifiedResult(branchName, branchIsModified); - return branchIsModified; + logger.debug( + { branchName, lastAuthor, gitAuthorEmail }, + 'branch.isModified() = true' + ); + config.branchIsModified[branchName] = true; + setCachedModifiedResult(branchName, true); + return true; } export async function isBranchConflicted( diff --git a/lib/workers/repository/config-migration/branch/rebase.ts b/lib/workers/repository/config-migration/branch/rebase.ts index 041a6cf48539bcf9feef311b1604b5d8af0c116b..cb932c114842442c1e3e5b3738723e06a0851387 100644 --- a/lib/workers/repository/config-migration/branch/rebase.ts +++ b/lib/workers/repository/config-migration/branch/rebase.ts @@ -16,7 +16,7 @@ export async function rebaseMigrationBranch( ): Promise<string | null> { logger.debug('Checking if migration branch needs rebasing'); const branchName = getMigrationBranchName(config); - if (await scm.isBranchModified(branchName, config.baseBranch)) { + if (await scm.isBranchModified(branchName)) { logger.debug('Migration branch has been edited and cannot be rebased'); return null; } diff --git a/lib/workers/repository/onboarding/branch/rebase.ts b/lib/workers/repository/onboarding/branch/rebase.ts index 04f28a4c01b81176e1b07ba17ad631c094794165..4cb1d27345db4d68bf4434b38b7144da67d3f88d 100644 --- a/lib/workers/repository/onboarding/branch/rebase.ts +++ b/lib/workers/repository/onboarding/branch/rebase.ts @@ -25,9 +25,7 @@ export async function rebaseOnboardingBranch( } // TODO #7154 - if ( - await scm.isBranchModified(config.onboardingBranch!, config.defaultBranch) - ) { + if (await scm.isBranchModified(config.onboardingBranch!)) { logger.debug('Onboarding branch has been edited and cannot be rebased'); return null; } diff --git a/lib/workers/repository/onboarding/pr/index.ts b/lib/workers/repository/onboarding/pr/index.ts index 04aa5e753a83c4de9cf66206bdc4e2933b6cacdb..923dec654e8c2da5da6645889cb7e34ac433e602 100644 --- a/lib/workers/repository/onboarding/pr/index.ts +++ b/lib/workers/repository/onboarding/pr/index.ts @@ -96,9 +96,7 @@ If you need any further assistance then you can also [request help here](${ if (GlobalConfig.get('dryRun')) { // TODO: types (#7154) logger.info(`DRY-RUN: Would check branch ${config.onboardingBranch!}`); - } else if ( - await scm.isBranchModified(config.onboardingBranch!, config.defaultBranch) - ) { + } else if (await scm.isBranchModified(config.onboardingBranch!)) { configDesc = emojify( `### Configuration\n\n:abcd: Renovate has detected a custom config for this PR. Feel free to ask for [help](${ config.productLinks!.help diff --git a/lib/workers/repository/update/branch/index.ts b/lib/workers/repository/update/branch/index.ts index 327e8ef57cac2dc7d6dd9afa069cc61fe1d75182..077e15eacb9b9b98ff47125f49f39c77ad2742fa 100644 --- a/lib/workers/repository/update/branch/index.ts +++ b/lib/workers/repository/update/branch/index.ts @@ -190,10 +190,7 @@ export async function processBranch( } logger.debug('Checking if PR has been edited'); - const branchIsModified = await scm.isBranchModified( - config.branchName, - config.baseBranch - ); + const branchIsModified = await scm.isBranchModified(config.branchName); if (branchPr) { logger.debug('Found existing branch PR'); if (branchPr.state !== 'open') { diff --git a/lib/workers/repository/update/branch/reuse.ts b/lib/workers/repository/update/branch/reuse.ts index 92a82eabcf3b7c759bc43d9646ddeec5df04aa48..d99be4947512210fd382cbc4082ee5fa29fcb0b5 100644 --- a/lib/workers/repository/update/branch/reuse.ts +++ b/lib/workers/repository/update/branch/reuse.ts @@ -59,7 +59,7 @@ export async function shouldReuseExistingBranch( if (await scm.isBranchBehindBase(branchName, baseBranch)) { logger.debug(`Branch is behind base branch and needs rebasing`); // We can rebase the branch only if no PR or PR can be rebased - if (await scm.isBranchModified(branchName, baseBranch)) { + if (await scm.isBranchModified(branchName)) { logger.debug('Cannot rebase branch as it has been modified'); result.reuseExistingBranch = true; result.isModified = true; @@ -80,7 +80,7 @@ export async function shouldReuseExistingBranch( if (result.isConflicted) { logger.debug('Branch is conflicted'); - if ((await scm.isBranchModified(branchName, baseBranch)) === false) { + if ((await scm.isBranchModified(branchName)) === false) { logger.debug(`Branch is not mergeable and needs rebasing`); if (config.rebaseWhen === 'never') { logger.debug('Rebasing disabled by config'); diff --git a/lib/workers/repository/update/pr/automerge.ts b/lib/workers/repository/update/pr/automerge.ts index 43853908c345c2674b333e832674e50feb13681a..66960c8f8e56af85e65e60884e9e6681326e1a11 100644 --- a/lib/workers/repository/update/pr/automerge.ts +++ b/lib/workers/repository/update/pr/automerge.ts @@ -82,7 +82,7 @@ export async function checkAutoMerge( }; } // Check if it's been touched - if (await scm.isBranchModified(branchName, config.baseBranch)) { + if (await scm.isBranchModified(branchName)) { logger.debug('PR is ready for automerge but has been modified'); return { automerged: false,