diff --git a/lib/modules/platform/default-scm.spec.ts b/lib/modules/platform/default-scm.spec.ts index 023948fa3dacae..28826974dcd88a 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'); + await defaultGitScm.isBranchModified('branchName', 'main'); expect(git.isBranchModified).toHaveBeenCalledTimes(1); }); }); diff --git a/lib/modules/platform/default-scm.ts b/lib/modules/platform/default-scm.ts index d21f3cff0b832e..cf30e1e6bbd674 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): Promise { - return git.isBranchModified(branchName); + isBranchModified(branchName: string, baseBranch?: string): Promise { + return git.isBranchModified(branchName, baseBranch); } } diff --git a/lib/modules/platform/types.ts b/lib/modules/platform/types.ts index 06b9fc1896db75..5e0aca4ffa30ce 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; - isBranchModified(branchName: string): Promise; + isBranchModified(branchName: string, baseBranch?: string): Promise; isBranchConflicted(baseBranch: string, branch: string): Promise; branchExists(branchName: string): Promise; getBranchCommit(branchName: string): Promise; diff --git a/lib/util/git/index.spec.ts b/lib/util/git/index.spec.ts index 1f205f85bd4893..7cdbf5e507fa16 100644 --- a/lib/util/git/index.spec.ts +++ b/lib/util/git/index.spec.ts @@ -77,6 +77,20 @@ 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'); @@ -275,11 +289,16 @@ 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')).toBeFalse(); + expect( + await git.isBranchModified('renovate/future_branch', defaultBranch) + ).toBeFalse(); }); it('should return false when author is ignored', async () => { @@ -287,15 +306,42 @@ 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 2df34dfdcd43b9..8aea27da4d2c06 100644 --- a/lib/util/git/index.ts +++ b/lib/util/git/index.ts @@ -603,7 +603,10 @@ export async function isBranchBehindBase( } } -export async function isBranchModified(branchName: string): Promise { +export async function isBranchModified( + branchName: string, + baseBranch?: string +): Promise { if (!branchExists(branchName)) { logger.debug('branch.isModified(): no cache'); return false; @@ -623,21 +626,48 @@ export async function isBranchModified(branchName: string): Promise { return isModified; } - logger.debug('branch.isModified(): using git to calculate'); - await syncGit(); - // Retrieve the author of the most recent commit - let lastAuthor: string | undefined; + // Retrieve the commit authors + let branchAuthors: string[] = []; try { - lastAuthor = ( - await git.raw([ - 'log', - '-1', - '--pretty=format:%ae', - `origin/${branchName}`, - '--', - ]) - ).trim(); + 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') + ), + ]; + } } catch (err) /* istanbul ignore next */ { if (err.message?.includes('fatal: bad revision')) { logger.debug( @@ -646,26 +676,19 @@ export async function isBranchModified(branchName: string): Promise { ); throw new Error(REPOSITORY_CHANGED); } - logger.warn({ err }, 'Error checking last author for isBranchModified'); + logger.warn({ err }, 'Error retrieving git authors for isBranchModified'); } const { gitAuthorEmail } = config; - 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; + let branchIsModified = false; + for (const author of branchAuthors) { + if (author !== gitAuthorEmail && !config.ignoredAuthors.includes(author)) { + branchIsModified = true; + } } - logger.debug( - { branchName, lastAuthor, gitAuthorEmail }, - 'branch.isModified() = true' - ); - config.branchIsModified[branchName] = true; - setCachedModifiedResult(branchName, true); - return true; + logger.debug(`branch.isModified() = ${branchIsModified}`); + config.branchIsModified[branchName] = branchIsModified; + setCachedModifiedResult(branchName, branchIsModified); + return branchIsModified; } 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 cb932c11484244..041a6cf48539bc 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 { logger.debug('Checking if migration branch needs rebasing'); const branchName = getMigrationBranchName(config); - if (await scm.isBranchModified(branchName)) { + if (await scm.isBranchModified(branchName, config.baseBranch)) { 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 4cb1d27345db4d..04f28a4c01b811 100644 --- a/lib/workers/repository/onboarding/branch/rebase.ts +++ b/lib/workers/repository/onboarding/branch/rebase.ts @@ -25,7 +25,9 @@ export async function rebaseOnboardingBranch( } // TODO #7154 - if (await scm.isBranchModified(config.onboardingBranch!)) { + if ( + await scm.isBranchModified(config.onboardingBranch!, config.defaultBranch) + ) { 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 923dec654e8c2d..04aa5e753a83c4 100644 --- a/lib/workers/repository/onboarding/pr/index.ts +++ b/lib/workers/repository/onboarding/pr/index.ts @@ -96,7 +96,9 @@ 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!)) { + } else if ( + await scm.isBranchModified(config.onboardingBranch!, config.defaultBranch) + ) { 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 188e5f31a1a666..101706360c2063 100644 --- a/lib/workers/repository/update/branch/index.ts +++ b/lib/workers/repository/update/branch/index.ts @@ -189,7 +189,10 @@ export async function processBranch( } logger.debug('Checking if PR has been edited'); - const branchIsModified = await scm.isBranchModified(config.branchName); + const branchIsModified = await scm.isBranchModified( + config.branchName, + config.baseBranch + ); 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 d99be494751221..92a82eabcf3b7c 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)) { + if (await scm.isBranchModified(branchName, baseBranch)) { 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)) === false) { + if ((await scm.isBranchModified(branchName, baseBranch)) === 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 66960c8f8e56af..43853908c345c2 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)) { + if (await scm.isBranchModified(branchName, config.baseBranch)) { logger.debug('PR is ready for automerge but has been modified'); return { automerged: false,