Skip to content

Commit 44781ef

Browse files
authored
fix(core): negated gitignore patterns inside subdirectories are not including matched files (#35511)
### Issue # (if applicable) Closes #9146 ### Reason for this change When bundling assets with `IgnoreMode.GIT`, files that should be included via negation patterns (like `!*.html`) are incorrectly excluded if they're inside directories that are also re-included by negation patterns. Here's an example - consider the following tree: ``` - index.html - app/ - component.js - index.html - home.html ``` If we use a `exclude` pattern of `['*', '!*.html', '!*/']`, the `index.html` in the root and both HTML files from the `app` folder should be included - the `app` folder shouldn't be excluded, due to the `!*/` pattern. Right now, only the `index.html` file in the root folder is included. ### Description of changes Similar to #22002, I've updated the `completelyIgnores` logic that's used to check if we need to skip a directory tree to include a trailing slash before verifying whether it's an ignored pattern. This makes it properly match directory-specific negation patterns like `!*/` by ensuring we're checking directory paths (with trailing slashes) against directory patterns. I've also updated the logic in `fingerprint` to check if we're ignoring directories or files, and call `completelyIgnores` or `ignores` accordingly. ### Describe any new or updated permissions being added None. ### Description of how you validated changes Added unit tests covering the described scenarios. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent cf7ca17 commit 44781ef

File tree

4 files changed

+136
-16
lines changed

4 files changed

+136
-16
lines changed

packages/aws-cdk-lib/core/lib/fs/fingerprint.ts

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,12 @@ export function fingerprint(fileOrDirectory: string, options: FingerprintOptions
6666
return hash.digest('hex');
6767

6868
function _processFileOrDirectory(symbolicPath: string, isRootDir: boolean = false, realPath = symbolicPath) {
69-
if (!isRootDir && ignoreStrategy.ignores(symbolicPath)) {
69+
const stat = fs.lstatSync(realPath);
70+
71+
if (_shouldIgnore(isRootDir, symbolicPath, realPath, stat)) {
7072
return;
7173
}
7274

73-
const stat = fs.lstatSync(realPath);
74-
7575
// Use relative path as hash component. Normalize it with forward slashes to ensure
7676
// same hash on Windows and Linux.
7777
const hashComponent = path.relative(fileOrDirectory, symbolicPath).replace(/\\/g, '/');
@@ -94,6 +94,32 @@ export function fingerprint(fileOrDirectory: string, options: FingerprintOptions
9494
throw new UnscopedValidationError(`Unable to hash ${symbolicPath}: it is neither a file nor a directory`);
9595
}
9696
}
97+
98+
function _shouldIgnore(isRootDir: boolean, symbolicPath: string, realPath: string, stat: fs.Stats) {
99+
if (isRootDir) {
100+
return false;
101+
}
102+
103+
if (stat.isDirectory()) {
104+
return ignoreStrategy.completelyIgnores(symbolicPath);
105+
}
106+
107+
if (stat.isSymbolicLink()) {
108+
const linkTarget = fs.readlinkSync(symbolicPath);
109+
const resolvedLinkTarget = path.resolve(path.dirname(realPath), linkTarget);
110+
111+
if (shouldFollow(follow, rootDirectory, resolvedLinkTarget)) {
112+
const targetStat = fs.statSync(resolvedLinkTarget);
113+
114+
// If we are following a directory symlink, we should use `completelyIgnores`.
115+
if (targetStat.isDirectory()) {
116+
return ignoreStrategy.completelyIgnores(symbolicPath);
117+
}
118+
}
119+
}
120+
121+
return ignoreStrategy.ignores(symbolicPath);
122+
}
97123
}
98124

99125
export function contentFingerprint(file: string): string {

packages/aws-cdk-lib/core/lib/fs/ignore.ts

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -80,14 +80,13 @@ export abstract class IgnoreStrategy {
8080
public abstract ignores(absoluteFilePath: string): boolean;
8181

8282
/**
83-
* Determines whether a given file path should be ignored and have all of its children ignored
84-
* if its a directory.
83+
* Determines whether a given directory path should be ignored and have all of its children ignored.
8584
*
86-
* @param absoluteFilePath absolute file path to be assessed against the pattern
87-
* @returns `true` if the file should be ignored
85+
* @param absoluteDirectoryPath absolute directory path to be assessed against the pattern
86+
* @returns `true` if the directory and all of its children should be ignored
8887
*/
89-
public completelyIgnores(absoluteFilePath: string): boolean {
90-
return this.ignores(absoluteFilePath);
88+
public completelyIgnores(absoluteDirectoryPath: string): boolean {
89+
return this.ignores(absoluteDirectoryPath);
9190
}
9291
}
9392

@@ -189,6 +188,23 @@ export class GitIgnoreStrategy extends IgnoreStrategy {
189188

190189
return this.ignore.ignores(relativePath);
191190
}
191+
192+
/**
193+
* Determines whether a given directory path should be ignored and have all of its children ignored.
194+
*
195+
* @param absoluteDirectoryPath absolute directory path to be assessed against the pattern
196+
* @returns `true` if the directory and all of its children should be ignored
197+
*/
198+
public completelyIgnores(absoluteDirectoryPath: string): boolean {
199+
if (!path.isAbsolute(absoluteDirectoryPath)) {
200+
throw new UnscopedValidationError('GitIgnoreStrategy.completelyIgnores() expects an absolute path');
201+
}
202+
203+
const relativePath = path.relative(this.absoluteRootPath, absoluteDirectoryPath);
204+
const relativePathWithSep = relativePath.endsWith(path.sep) ? relativePath : relativePath + path.sep;
205+
206+
return this.ignore.ignores(relativePathWithSep);
207+
}
192208
}
193209

194210
/**
@@ -268,14 +284,13 @@ export class DockerIgnoreStrategy extends IgnoreStrategy {
268284
}
269285

270286
/**
271-
* Determines whether a given file path should be ignored and have all of its children ignored
272-
* if its a directory.
287+
* Determines whether a given directory path should be ignored and have all of its children ignored.
273288
*
274-
* @param absoluteFilePath absolute file path to be assessed against the pattern
275-
* @returns `true` if the file should be ignored
289+
* @param absoluteDirectoryPath absolute directory path to be assessed against the pattern
290+
* @returns `true` if the directory and all of its children should be ignored
276291
*/
277-
public completelyIgnores(absoluteFilePath: string): boolean {
278-
const relativePath = this.getRelativePath(absoluteFilePath);
292+
public completelyIgnores(absoluteDirectoryPath: string): boolean {
293+
const relativePath = this.getRelativePath(absoluteDirectoryPath);
279294
return this.ignore.ignores(relativePath) && this.completeIgnore.ignores(relativePath);
280295
}
281296
}

packages/aws-cdk-lib/core/test/fs/fs-copy.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,28 @@ describe('fs copy', () => {
142142
' file3.txt',
143143
]);
144144
});
145+
146+
test('negated pattern inside subdirectory with git ignore mode', () => {
147+
// GIVEN
148+
const outdir = fs.mkdtempSync(path.join(os.tmpdir(), 'copy-tests'));
149+
150+
// WHEN
151+
FileSystem.copyDirectory(path.join(__dirname, 'fixtures', 'test1'), outdir, {
152+
exclude: [
153+
'*',
154+
'!.hidden',
155+
'!*/',
156+
],
157+
ignoreMode: IgnoreMode.GIT,
158+
});
159+
160+
// THEN
161+
expect(tree(outdir)).toEqual([
162+
'subdir2 (D)',
163+
' empty-subdir (D)',
164+
' .hidden',
165+
]);
166+
});
145167
});
146168

147169
function tree(dir: string, depth = ''): string[] {

packages/aws-cdk-lib/core/test/fs/fs-fingerprint.test.ts

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import * as fs from 'fs';
22
import * as os from 'os';
33
import * as path from 'path';
4-
import { FileSystem, SymlinkFollowMode } from '../../lib/fs';
4+
import { FileSystem, IgnoreMode, SymlinkFollowMode } from '../../lib/fs';
55
import { contentFingerprint } from '../../lib/fs/fingerprint';
66

77
describe('fs fingerprint', () => {
@@ -91,6 +91,25 @@ describe('fs fingerprint', () => {
9191
// THEN
9292
expect(hashSrc).not.toEqual(hashCopy);
9393
});
94+
95+
test('changes with negated gitignore patterns inside ignored directories', () => {
96+
// GIVEN
97+
const srcdir = path.join(__dirname, 'fixtures', 'symlinks');
98+
const cpydir = fs.mkdtempSync(path.join(os.tmpdir(), 'fingerprint-tests'));
99+
FileSystem.copyDirectory(srcdir, cpydir);
100+
101+
// Add a new file that is inside an ignored directory, but has a negated pattern that includes it.
102+
const newFile = path.join(cpydir, 'ignored-dir', 'not-ignored-anymore.html');
103+
fs.mkdirSync(path.dirname(newFile), { recursive: true });
104+
fs.writeFileSync(newFile, '<h1>Do not ignore me!</h1>');
105+
106+
// WHEN
107+
const hashSrc = FileSystem.fingerprint(srcdir, { exclude: ['*', '!*.html', '!*/'], ignoreMode: IgnoreMode.GIT });
108+
const hashCopy = FileSystem.fingerprint(cpydir, { exclude: ['*', '!*.html', '!*/'], ignoreMode: IgnoreMode.GIT });
109+
110+
// THEN
111+
expect(hashSrc).not.toEqual(hashCopy);
112+
});
94113
});
95114

96115
describe('symlinks', () => {
@@ -147,6 +166,44 @@ describe('fs fingerprint', () => {
147166
expect(original).toEqual(afterChange);
148167
expect(afterRevert).toEqual(original);
149168
});
169+
170+
test('changes when following directory symlinks with negated gitignore patterns', () => {
171+
// GIVEN
172+
const srcdir = fs.mkdtempSync(path.join(os.tmpdir(), 'fingerprint-tests'));
173+
fs.mkdirSync(path.join(srcdir, 'subdir'), { recursive: true });
174+
fs.writeFileSync(path.join(srcdir, 'subdir', 'page.html'), '<h1>Hello, world!</h1>');
175+
176+
const cpydir = fs.mkdtempSync(path.join(os.tmpdir(), 'fingerprint-tests'));
177+
FileSystem.copyDirectory(srcdir, cpydir);
178+
fs.symlinkSync(path.join(cpydir, 'subdir'), path.join(cpydir, 'link-to-dir'));
179+
180+
// WHEN
181+
const options = { exclude: ['*', '!*.html', '!*/'], ignoreMode: IgnoreMode.GIT, follow: SymlinkFollowMode.ALWAYS };
182+
const hashSrc = FileSystem.fingerprint(srcdir, options);
183+
const hashCpy = FileSystem.fingerprint(cpydir, options);
184+
185+
// THEN
186+
expect(hashSrc).not.toEqual(hashCpy);
187+
});
188+
189+
test('does not change when not following directory symlinks with negated gitignore patterns', () => {
190+
// GIVEN
191+
const srcdir = fs.mkdtempSync(path.join(os.tmpdir(), 'fingerprint-tests'));
192+
fs.mkdirSync(path.join(srcdir, 'subdir'), { recursive: true });
193+
fs.writeFileSync(path.join(srcdir, 'subdir', 'page.html'), '<h1>Hello, world!</h1>');
194+
195+
const cpydir = fs.mkdtempSync(path.join(os.tmpdir(), 'fingerprint-tests'));
196+
FileSystem.copyDirectory(srcdir, cpydir);
197+
fs.symlinkSync(path.join(cpydir, 'subdir'), path.join(cpydir, 'link-to-dir'));
198+
199+
// WHEN
200+
const options = { exclude: ['*', '!*.html', '!*/'], ignoreMode: IgnoreMode.GIT, follow: SymlinkFollowMode.NEVER };
201+
const hashSrc = FileSystem.fingerprint(srcdir, options);
202+
const hashCpy = FileSystem.fingerprint(cpydir, options);
203+
204+
// THEN
205+
expect(hashSrc).toEqual(hashCpy);
206+
});
150207
});
151208

152209
describe('eol', () => {

0 commit comments

Comments
 (0)