Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exclude .git directory content when calculating package file count #525

Merged
merged 4 commits into from
Mar 18, 2024
Merged

Exclude .git directory content when calculating package file count #525

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6eead81
Exclude .git directory content for package file count
qtomlinson Sep 28, 2023
ca3901a
Refactor and add unit test
qtomlinson Jan 11, 2024
d2e08a0
Merge branch 'master' into qt/exclude-git-dir
qtomlinson Mar 15, 2024
740967c
Address review comments
qtomlinson Mar 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions lib/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,10 @@ function trimAllParents(paths, parents) {
return paths.map(path => trimParents(path, parents))
}

function isValidExcludingGit(file) {
const exclusions = ['.git']
function isGitFile(file) {
if (!file) return false
const segments = file.split(/[\\/]/g)
return !intersection(segments, exclusions).length
return intersection(segments, ['.git']).length > 0
}

function extractDate(dateAndTime, formats = dateTimeFormats) {
Expand Down Expand Up @@ -83,4 +82,4 @@ function spawnPromisified(command, args, options) {
})
}

module.exports = { normalizePath, normalizePaths, trimParents, trimAllParents, isValidExcludingGit, extractDate, spawnPromisified }
module.exports = { normalizePath, normalizePaths, trimParents, trimAllParents, isGitFile, extractDate, spawnPromisified }
4 changes: 2 additions & 2 deletions providers/process/abstractClearlyDefinedProcessor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const throat = require('throat')
const path = require('path')
const { pick, merge } = require('lodash')
const du = require('du')
const { trimParents, isValidExcludingGit } = require('../../lib/utils')
const { trimParents, isGitFile } = require('../../lib/utils')

class AbstractClearlyDefinedProcessor extends AbstractProcessor {
get toolVersion() {
Expand Down Expand Up @@ -75,7 +75,7 @@ class AbstractClearlyDefinedProcessor extends AbstractProcessor {
let count = 0
const bytes = await du(location, {
filter: file => {
if (!isValidExcludingGit(file)) return false
if (isGitFile(file)) return false
count++
return true
}
Expand Down
4 changes: 2 additions & 2 deletions providers/process/abstractProcessor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const { clone, flatten, pick, set } = require('lodash')
const { promisify } = require('util')
const readdir = promisify(fs.readdir)
const lstat = promisify(fs.lstat)
const { trimAllParents, isValidExcludingGit } = require('../../lib/utils')
const { trimAllParents, isGitFile } = require('../../lib/utils')

class AbstractProcessor extends BaseHandler {
constructor(options) {
Expand Down Expand Up @@ -150,7 +150,7 @@ class AbstractProcessor extends BaseHandler {
*/
async filterFiles(location) {
const fullList = await this.getFiles(location)
const filteredList = fullList.filter(file => isValidExcludingGit(file))
const filteredList = fullList.filter(file => file && !isGitFile(file))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't necessarily need to check if file is undefined here since isGitFile already does: if (!file) return false

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm... I am afraid that the check for undefined file is still necessary. In case of falsy file, isGitFile(file) returns false, and !isGitFile returns true. Using !isGitFile alone would not exclude falsy file values.

return trimAllParents(filteredList, location).filter(x => x)
}

Expand Down
28 changes: 14 additions & 14 deletions test/unit/lib/utilsTests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

const chai = require('chai')
const chaiAsPromised = require('chai-as-promised')
const { normalizePath, normalizePaths, trimParents, trimAllParents, extractDate, spawnPromisified, isValidExcludingGit } = require('../../../lib/utils')
const { normalizePath, normalizePaths, trimParents, trimAllParents, extractDate, spawnPromisified, isGitFile } = require('../../../lib/utils')
const { promisify } = require('util')
const execFile = promisify(require('child_process').execFile)
chai.use(chaiAsPromised)
Expand Down Expand Up @@ -56,20 +56,20 @@ describe('Utils path functions', () => {
})
})

describe('Util isValidExcludingGit', () => {
it('should exclude .git and its contents', () => {
const data = new Map([
[null, false],
['/', true],
['/tmp/tempX/package/src', true],
['.git', false],
['/tmp/tempX/package/.git', false],
['/tmp/tempX/package/.git/hooks/pre-merge-commit.sample', false]
])
data.forEach((expected, input) => {
expect(isValidExcludingGit(input)).to.eq(expected)
})
describe('Util isGitFile', () => {
const entries = new Map([
[null, false],
['/', false],
['/tmp/tempX/package/src', false],
['.git', true],
['/tmp/tempX/package/.git', true],
['/tmp/tempX/package/.git/hooks/pre-merge-commit.sample', true]
])

entries.forEach((expected, file) => {
it(`should return ${expected} for isGitFile given '${file}'`, () => expect(isGitFile(file)).to.eq(expected))
})

})

describe('Util extractDate', () => {
Expand Down