Skip to content

Commit

Permalink
fix: codeowners was consuming too much memory (#107)
Browse files Browse the repository at this point in the history
* fix

* fix
  • Loading branch information
fwuensche authored Sep 27, 2024
1 parent 3aaf4f4 commit c7a3811
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 21 deletions.
4 changes: 4 additions & 0 deletions CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,7 @@
# In this example, the user owns any files in the src directory
# at the root of the repository and any of its subdirectories.
/src/ @fwuensche @rchoquet

# There's a plugin folder inside /src/ that we'll be using to test
# that the pattern matching is working correctly. See codeowners.test.ts
plugins/ @fwuensche
25 changes: 13 additions & 12 deletions src/codeowners.test.ts
Original file line number Diff line number Diff line change
@@ -1,29 +1,30 @@
import { describe, expect, it } from 'vitest'
import { describe, expect, test } from 'vitest'

import Codeowners from './codeowners.js'

describe('getOwners', () => {
// Assuming src folder is owned by @fwuensche and @rchoquet
it('recognizes folder patterns', async () => {
test('/src/ pattern matches files inside src folder', async () => {
const codeowners = new Codeowners()
expect(codeowners.getOwners('src/codeowners.test.ts')).toEqual(['@fwuensche', '@rchoquet'])
})

// Assuming bin folder has no defined owners, but @fwuensche is the default owner
it('recognizes default owners', async () => {
test('plugins/ pattern should match any folder named plugins', async () => {
const codeowners = new Codeowners()
expect(codeowners.getOwners('bin/commands/run.ts')).toEqual(['@fwuensche'])
expect(codeowners.getOwners('src/plugins/eslint.js')).toEqual(['@fwuensche'])
})

// Assuming js files are owned by @rchoquet
it('recognizes file extension patterns', async () => {
test('defaults to the root owner', async () => {
const codeowners = new Codeowners()
expect(codeowners.getOwners('bin/codeowners.js')).toEqual(['@rchoquet'])
expect(codeowners.getOwners('bin/commands/push.ts')).toEqual(['@fwuensche'])
})

// Assuming the file does not exist, but matches an existing pattern
it('returns who would theoretically own the file even tho it does not exist', async () => {
test('*.js also matches files from subfolders', async () => {
const codeowners = new Codeowners()
expect(codeowners.getOwners('bin/non-existing-file')).toEqual(['@fwuensche'])
expect(codeowners.getOwners('bin/commands/diff.js')).toEqual(['@rchoquet'])
})

test('non existing files return an empty list of owners', async () => {
const codeowners = new Codeowners()
expect(codeowners.getOwners('bin/non-existing-file')).toEqual([])
})
})
101 changes: 92 additions & 9 deletions src/codeowners.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,97 @@
import Codeowners from 'codeowners'
import { findUpSync } from 'find-up'
import fs from 'fs'
import glob from 'glob'
import intersection from 'lodash/intersection.js'
import { isDirectory } from './file.js'
import path from 'path'
import trueCasePath from 'true-case-path'
import uniq from 'lodash/uniq.js'

// Create a subclass of Codeowners
class ExtendedCodeowners extends Codeowners {
getOwners: typeof this.getOwner
// TODO: This should be dynamically generated from the .gitignore file
const IGNORES = ['**/node_modules/**', '**/.git/**', '**/.github/**', '**/.gitlab/**', '**/docs/**']

constructor(...args: ConstructorParameters<typeof Codeowners>) {
super(...args)
// Point getOwners to the getOwner method (for backwards compatibility)
this.getOwners = this.getOwner
const { trueCasePathSync } = trueCasePath

class Codeowners {
ownersByFile: Record<string, string[]>

constructor(currentPath?: string) {
this.ownersByFile = {}
this.init(currentPath)
}

init(currentPath?: string) {
// Find the CODEOWNERS file or use the one provided as an argument
const pathOrCwd = currentPath || process.cwd()
const codeownersPath = findUpSync(['.github/CODEOWNERS', '.gitlab/CODEOWNERS', 'docs/CODEOWNERS', 'CODEOWNERS'], {
cwd: pathOrCwd,
})

if (!codeownersPath) return

const codeownersFilePath = trueCasePathSync(codeownersPath)
let codeownersDirectory = path.dirname(codeownersFilePath)

// We might have found a bare codeowners file or one inside the three supported subdirectories.
// In the latter case the project root is up another level.
if (codeownersDirectory.match(/\/(.github|.gitlab|docs)$/i)) codeownersDirectory = path.dirname(codeownersDirectory)

const codeownersFile = path.basename(codeownersFilePath)

if (codeownersFile !== 'CODEOWNERS')
throw new Error(`Found a CODEOWNERS file but it was lower-cased: ${codeownersFilePath}`)

if (isDirectory(codeownersFilePath))
throw new Error(`Found a CODEOWNERS but it's a directory: ${codeownersFilePath}`)

const codeownersLines = fs
.readFileSync(codeownersFilePath)
.toString()
.split(/\r\n|\r|\n/) // Split by line breaks
.filter(Boolean) // Remove empty lines
.map((line) => line.trim()) // Remove leading and trailing whitespace

for (const line of codeownersLines) {
// Remove comments
if (line.startsWith('#')) continue

// Split the line into path and owners
const [codeownersPattern, ...owners] = line.split(/\s+/)

// We do it in the order of the file, so that the last one wins

for (const file of this.#globPatterns(codeownersPattern)) {
this.ownersByFile[file] = uniq(owners)
}
}
}

/** Returns the files from the codebase mathing the given pattern */
#globPatterns(pattern: string) {
if (pattern.includes('*')) return glob.sync(pattern.replace('*', '**/*'), { nodir: true, ignore: IGNORES })

if (pattern.endsWith('/')) {
if (pattern.startsWith('/')) {
return glob.sync(path.join(pattern.substring(1), '**', '*'), { nodir: true, ignore: IGNORES })
} else {
return glob.sync(path.join('**', pattern, '*'), { nodir: true, ignore: IGNORES })
}
}
return [pattern]
}

getOwners(file: string) {
return this.ownersByFile[file] || []
}

getFiles(owners: string[]) {
return uniq(
Object.entries(this.ownersByFile)
.filter(([, fileOwners]) => intersection(owners, fileOwners).length > 0)
.map(([file]) => file)
.flat()
)
}
}

export default ExtendedCodeowners
export default Codeowners

0 comments on commit c7a3811

Please sign in to comment.