diff --git a/lib/modules/platform/default-scm.spec.ts b/lib/modules/platform/default-scm.spec.ts index 023948fa3dacaec6f2e22f0474dd94533029ba16..28826974dcd88ad45f4c571e2ca895a7f59d870a 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 d21f3cff0b832eb826322c473558475dc2309f1a..cf30e1e6bbd674ba6b2b11c59a20e21708984d4f 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<boolean> { - return git.isBranchModified(branchName); + isBranchModified(branchName: string, baseBranch?: string): Promise<boolean> { + return git.isBranchModified(branchName, baseBranch); } } diff --git a/lib/modules/platform/types.ts b/lib/modules/platform/types.ts index d37eae8db6c50a30f114545e4728619c17b62243..1cd330290f8141331c92f32f8c0a1ea1761a06d3 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): Promise<boolean>; + isBranchModified(branchName: string, baseBranch?: 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 1f205f85bd489361f379d7e9d95ffff6b0dca90a..7cdbf5e507fa1684935c046629ef3504a7920052 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 2df34dfdcd43b9dfc305fbb759f4fd340929b38f..8aea27da4d2c0690b17b64a4c93d13cc07c6be44 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<boolean> { +export async function isBranchModified( + branchName: string, + baseBranch?: string +): Promise<boolean> { if (!branchExists(branchName)) { logger.debug('branch.isModified(): no cache'); return false; @@ -623,21 +626,48 @@ export async function isBranchModified(branchName: string): Promise<boolean> { 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<boolean> { ); 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 cb932c114842442c1e3e5b3738723e06a0851387..041a6cf48539bcf9feef311b1604b5d8af0c116b 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)) { + 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 4cb1d27345db4d68bf4434b38b7144da67d3f88d..04f28a4c01b81176e1b07ba17ad631c094794165 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 923dec654e8c2da5da6645889cb7e34ac433e602..04aa5e753a83c4de9cf66206bdc4e2933b6cacdb 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 077e15eacb9b9b98ff47125f49f39c77ad2742fa..327e8ef57cac2dc7d6dd9afa069cc61fe1d75182 100644 --- a/lib/workers/repository/update/branch/index.ts +++ b/lib/workers/repository/update/branch/index.ts @@ -190,7 +190,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 d99be4947512210fd382cbc4082ee5fa29fcb0b5..92a82eabcf3b7c759bc43d9646ddeec5df04aa48 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 66960c8f8e56af85e65e60884e9e6681326e1a11..43853908c345c2674b333e832674e50feb13681a 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,