diff --git a/lib/modules/platform/default-scm.spec.ts b/lib/modules/platform/default-scm.spec.ts index 2f97a3c4ef..d7a7995bc9 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 92ceef5421..c105a0bd16 100644 --- a/lib/modules/platform/default-scm.ts +++ b/lib/modules/platform/default-scm.ts @@ -27,8 +27,8 @@ 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); } getFileList(): Promise { diff --git a/lib/modules/platform/types.ts b/lib/modules/platform/types.ts index ca98ae3337..fb3ac95387 100644 --- a/lib/modules/platform/types.ts +++ b/lib/modules/platform/types.ts @@ -233,7 +233,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 3902d862a4..a8d8e426db 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,33 @@ 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 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 c919e559ef..587eef39d4 100644 --- a/lib/util/git/index.ts +++ b/lib/util/git/index.ts @@ -616,7 +616,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; @@ -636,21 +639,49 @@ 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(newlineRegex) + ), + ]; + logger.debug(`Git authors in branch: ${branchAuthors.join(', ')}`); + } 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(newlineRegex) + ), + ]; + } } catch (err) /* istanbul ignore next */ { if (err.message?.includes('fatal: bad revision')) { logger.debug( @@ -659,26 +690,20 @@ 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)) { + logger.debug(`branch.isModified=true due to this git author: ${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 eb082c893a..4872685cb7 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/update/branch/index.ts b/lib/workers/repository/update/branch/index.ts index a78d2f1fca..c3c381f883 100644 --- a/lib/workers/repository/update/branch/index.ts +++ b/lib/workers/repository/update/branch/index.ts @@ -242,7 +242,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 d2b3a7b1ac..4abfbccd2a 100644 --- a/lib/workers/repository/update/branch/reuse.ts +++ b/lib/workers/repository/update/branch/reuse.ts @@ -29,7 +29,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; @@ -50,7 +50,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 2e9465ea97..b9baebcf28 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,