From 46806a23fcb36e0b42ebf857a9303e0e88d44c71 Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Mon, 27 Jan 2025 13:59:41 -0500 Subject: [PATCH 1/8] Refactor --- crates/oxide/src/scanner/allowed_paths.rs | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/crates/oxide/src/scanner/allowed_paths.rs b/crates/oxide/src/scanner/allowed_paths.rs index 8a584d06fb2c..8dc1e5fd74f8 100644 --- a/crates/oxide/src/scanner/allowed_paths.rs +++ b/crates/oxide/src/scanner/allowed_paths.rs @@ -33,17 +33,13 @@ pub fn resolve_allowed_paths(root: &Path) -> impl Iterator { #[tracing::instrument(skip_all)] pub fn resolve_paths(root: &Path) -> impl Iterator { - WalkBuilder::new(root) - .hidden(false) - .require_git(false) + create_walk_builder(root) .build() .filter_map(Result::ok) } pub fn read_dir(root: &Path, depth: Option) -> impl Iterator { - WalkBuilder::new(root) - .hidden(false) - .require_git(false) + create_walk_builder(root) .max_depth(depth) .filter_entry(move |entry| match entry.file_type() { Some(file_type) if file_type.is_dir() => match entry.file_name().to_str() { @@ -59,6 +55,21 @@ pub fn read_dir(root: &Path, depth: Option) -> impl Iterator WalkBuilder { + let mut builder = WalkBuilder::new(root); + + // Scan hidden files / directories + builder.hidden(false); + + // By default, allow .gitignore files to be used regardless of whether or not + // a .git directory is present. This is an optimization for when projects + // are first created and may not be in a git repo yet. + builder.require_git(false); + + builder +} + + pub fn is_allowed_content_path(path: &Path) -> bool { // Skip known ignored files if path From 4a550b26b38712515212d5877966b166e5050673 Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Mon, 27 Jan 2025 14:19:33 -0500 Subject: [PATCH 2/8] =?UTF-8?q?Don=E2=80=99t=20look=20at=20ignore=20files?= =?UTF-8?q?=20outside=20the=20repo?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/oxide/src/scanner/allowed_paths.rs | 41 +++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/crates/oxide/src/scanner/allowed_paths.rs b/crates/oxide/src/scanner/allowed_paths.rs index 8dc1e5fd74f8..5e2f022e9566 100644 --- a/crates/oxide/src/scanner/allowed_paths.rs +++ b/crates/oxide/src/scanner/allowed_paths.rs @@ -66,6 +66,47 @@ fn create_walk_builder(root: &Path) -> WalkBuilder { // are first created and may not be in a git repo yet. builder.require_git(false); + // Don't descend into .git directories inside the root folder + // This is necessary when `root` contains the `.git` dir + builder.filter_entry(|entry| entry.file_name() != ".git"); + + // If we are in a git repo then require it to ensure that only rules within + // the repo are used. For example, we don't want to consider a .gitnore file + // in the user's home folder if we're in a git repo. + // + // The alternative is using a call like `.parents(false)` but that will + // prevent looking at parent directories for .gitignore files from within + // the repo and that's not what we want. + // + // For example, in a project with this structure: + // + // home + // .gitignore + // my-project + // .gitignore + // apps + // .gitignore + // web + // {root} + // + // We do want to consider all .gitignore files listed: + // - home/.gitignore + // - my-project/.gitignore + // - my-project/apps/.gitignore + // + // However, if a repo is initalized inside my-project then only the following + // make sense for consideration: + // - my-project/.gitignore + // - my-project/apps/.gitignore + // + // Setting the require_git(true) flag conditionally allows us to do this. + for parent in root.ancestors() { + if parent.join(".git").exists() { + builder.require_git(true); + break; + } + } + builder } From e0a3027bd9312eaa0dab7cefcbe6efc320943973 Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Mon, 27 Jan 2025 14:46:03 -0500 Subject: [PATCH 3/8] Add tests --- crates/oxide/tests/scanner.rs | 105 ++++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) diff --git a/crates/oxide/tests/scanner.rs b/crates/oxide/tests/scanner.rs index cdec9a44dd26..878050e0ad93 100644 --- a/crates/oxide/tests/scanner.rs +++ b/crates/oxide/tests/scanner.rs @@ -586,4 +586,109 @@ mod scanner { ] ); } + + #[test] + fn skips_ignore_files_outside_of_a_repo() { + // Create a temporary working directory + let dir = tempdir().unwrap().into_path(); + + // Create files + create_files_in( + &dir, + &[ + // This file should always be picked up + ("home/project/apps/web/index.html", "content-['index.html']"), + + // Set up various ignore rules + ("home/.gitignore", "ignore-home.html"), + ("home/project/.gitignore", "ignore-project.html"), + ("home/project/apps/.gitignore", "ignore-apps.html"), + ("home/project/apps/web/.gitignore", "ignore-web.html"), + + // Some of these should be ignored depending on which dir is the repo root + ("home/project/apps/web/ignore-home.html", "content-['ignore-home.html']"), + ("home/project/apps/web/ignore-project.html", "content-['ignore-project.html']"), + ("home/project/apps/web/ignore-apps.html", "content-['ignore-apps.html']"), + ("home/project/apps/web/ignore-web.html", "content-['ignore-web.html']"), + ], + ); + + + let sources = vec![ + GlobEntry { + base: dir.join("home/project/apps/web").to_string_lossy().to_string(), + pattern: "**/*".to_owned(), + }, + ]; + + let candidates = Scanner::new(Some(sources.clone())).scan(); + + // All ignore files are applied because there's no git repo + assert_eq!( + candidates, + vec![ + "content-['index.html']".to_owned(), + ] + ); + + // Initialize `home` as a git repository and scan again + // The results should be the same as before + _ = Command::new("git").arg("init").current_dir(dir.join("home")).output(); + let candidates = Scanner::new(Some(sources.clone())).scan(); + + assert_eq!( + candidates, + vec![ + "content-['index.html']".to_owned(), + ] + ); + + // Drop the .git folder + fs::remove_dir_all(dir.join("home/.git")).unwrap(); + + // Initialize `home/project` as a git repository and scan again + _ = Command::new("git").arg("init").current_dir(dir.join("home/project")).output(); + let candidates = Scanner::new(Some(sources.clone())).scan(); + + assert_eq!( + candidates, + vec![ + "content-['ignore-home.html']".to_owned(), + "content-['index.html']".to_owned(), + ] + ); + + // Drop the .git folder + fs::remove_dir_all(dir.join("home/project/.git")).unwrap(); + + // Initialize `home/project/apps` as a git repository and scan again + _ = Command::new("git").arg("init").current_dir(dir.join("home/project/apps")).output(); + let candidates = Scanner::new(Some(sources.clone())).scan(); + + assert_eq!( + candidates, + vec![ + "content-['ignore-home.html']".to_owned(), + "content-['ignore-project.html']".to_owned(), + "content-['index.html']".to_owned(), + ] + ); + + // Drop the .git folder + fs::remove_dir_all(dir.join("home/project/apps/.git")).unwrap(); + + // Initialize `home/project/apps` as a git repository and scan again + _ = Command::new("git").arg("init").current_dir(dir.join("home/project/apps/web")).output(); + let candidates = Scanner::new(Some(sources.clone())).scan(); + + assert_eq!( + candidates, + vec![ + "content-['ignore-apps.html']".to_owned(), + "content-['ignore-home.html']".to_owned(), + "content-['ignore-project.html']".to_owned(), + "content-['index.html']".to_owned(), + ] + ); + } } From 53a7b3a10d520181d7693812c1df1e64168d2dfa Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Mon, 27 Jan 2025 14:48:43 -0500 Subject: [PATCH 4/8] Cleanup --- crates/oxide/src/scanner/allowed_paths.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/oxide/src/scanner/allowed_paths.rs b/crates/oxide/src/scanner/allowed_paths.rs index 5e2f022e9566..14bdb4e1282a 100644 --- a/crates/oxide/src/scanner/allowed_paths.rs +++ b/crates/oxide/src/scanner/allowed_paths.rs @@ -110,7 +110,6 @@ fn create_walk_builder(root: &Path) -> WalkBuilder { builder } - pub fn is_allowed_content_path(path: &Path) -> bool { // Skip known ignored files if path From 01313d00a32dec9f86c9e5ba424e9ea8a6b53a30 Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Tue, 28 Jan 2025 08:24:11 -0500 Subject: [PATCH 5/8] Fix typos Co-authored-by: Robin Malfait --- crates/oxide/src/scanner/allowed_paths.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/oxide/src/scanner/allowed_paths.rs b/crates/oxide/src/scanner/allowed_paths.rs index 14bdb4e1282a..dd8ef074ad7b 100644 --- a/crates/oxide/src/scanner/allowed_paths.rs +++ b/crates/oxide/src/scanner/allowed_paths.rs @@ -67,11 +67,11 @@ fn create_walk_builder(root: &Path) -> WalkBuilder { builder.require_git(false); // Don't descend into .git directories inside the root folder - // This is necessary when `root` contains the `.git` dir + // This is necessary when `root` contains the `.git` dir. builder.filter_entry(|entry| entry.file_name() != ".git"); // If we are in a git repo then require it to ensure that only rules within - // the repo are used. For example, we don't want to consider a .gitnore file + // the repo are used. For example, we don't want to consider a .gitignore file // in the user's home folder if we're in a git repo. // // The alternative is using a call like `.parents(false)` but that will @@ -94,7 +94,7 @@ fn create_walk_builder(root: &Path) -> WalkBuilder { // - my-project/.gitignore // - my-project/apps/.gitignore // - // However, if a repo is initalized inside my-project then only the following + // However, if a repo is initialized inside my-project then only the following // make sense for consideration: // - my-project/.gitignore // - my-project/apps/.gitignore From 70982618ca739d1727b4b2f01ee15cb1cc121e92 Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Tue, 28 Jan 2025 10:26:51 -0500 Subject: [PATCH 6/8] Run `cargo fmt` --- crates/oxide/src/scanner/allowed_paths.rs | 96 +++++++++++------------ crates/oxide/tests/scanner.rs | 70 ++++++++++------- 2 files changed, 88 insertions(+), 78 deletions(-) diff --git a/crates/oxide/src/scanner/allowed_paths.rs b/crates/oxide/src/scanner/allowed_paths.rs index dd8ef074ad7b..b906335f8979 100644 --- a/crates/oxide/src/scanner/allowed_paths.rs +++ b/crates/oxide/src/scanner/allowed_paths.rs @@ -33,9 +33,7 @@ pub fn resolve_allowed_paths(root: &Path) -> impl Iterator { #[tracing::instrument(skip_all)] pub fn resolve_paths(root: &Path) -> impl Iterator { - create_walk_builder(root) - .build() - .filter_map(Result::ok) + create_walk_builder(root).build().filter_map(Result::ok) } pub fn read_dir(root: &Path, depth: Option) -> impl Iterator { @@ -56,58 +54,58 @@ pub fn read_dir(root: &Path, depth: Option) -> impl Iterator WalkBuilder { - let mut builder = WalkBuilder::new(root); + let mut builder = WalkBuilder::new(root); - // Scan hidden files / directories - builder.hidden(false); + // Scan hidden files / directories + builder.hidden(false); - // By default, allow .gitignore files to be used regardless of whether or not - // a .git directory is present. This is an optimization for when projects - // are first created and may not be in a git repo yet. - builder.require_git(false); + // By default, allow .gitignore files to be used regardless of whether or not + // a .git directory is present. This is an optimization for when projects + // are first created and may not be in a git repo yet. + builder.require_git(false); - // Don't descend into .git directories inside the root folder - // This is necessary when `root` contains the `.git` dir. - builder.filter_entry(|entry| entry.file_name() != ".git"); + // Don't descend into .git directories inside the root folder + // This is necessary when `root` contains the `.git` dir. + builder.filter_entry(|entry| entry.file_name() != ".git"); - // If we are in a git repo then require it to ensure that only rules within - // the repo are used. For example, we don't want to consider a .gitignore file - // in the user's home folder if we're in a git repo. - // - // The alternative is using a call like `.parents(false)` but that will - // prevent looking at parent directories for .gitignore files from within - // the repo and that's not what we want. - // - // For example, in a project with this structure: - // - // home - // .gitignore - // my-project - // .gitignore - // apps - // .gitignore - // web - // {root} - // - // We do want to consider all .gitignore files listed: - // - home/.gitignore - // - my-project/.gitignore - // - my-project/apps/.gitignore - // - // However, if a repo is initialized inside my-project then only the following - // make sense for consideration: - // - my-project/.gitignore - // - my-project/apps/.gitignore - // - // Setting the require_git(true) flag conditionally allows us to do this. - for parent in root.ancestors() { - if parent.join(".git").exists() { - builder.require_git(true); - break; + // If we are in a git repo then require it to ensure that only rules within + // the repo are used. For example, we don't want to consider a .gitignore file + // in the user's home folder if we're in a git repo. + // + // The alternative is using a call like `.parents(false)` but that will + // prevent looking at parent directories for .gitignore files from within + // the repo and that's not what we want. + // + // For example, in a project with this structure: + // + // home + // .gitignore + // my-project + // .gitignore + // apps + // .gitignore + // web + // {root} + // + // We do want to consider all .gitignore files listed: + // - home/.gitignore + // - my-project/.gitignore + // - my-project/apps/.gitignore + // + // However, if a repo is initialized inside my-project then only the following + // make sense for consideration: + // - my-project/.gitignore + // - my-project/apps/.gitignore + // + // Setting the require_git(true) flag conditionally allows us to do this. + for parent in root.ancestors() { + if parent.join(".git").exists() { + builder.require_git(true); + break; + } } - } - builder + builder } pub fn is_allowed_content_path(path: &Path) -> bool { diff --git a/crates/oxide/tests/scanner.rs b/crates/oxide/tests/scanner.rs index 878050e0ad93..e2bfdf37e558 100644 --- a/crates/oxide/tests/scanner.rs +++ b/crates/oxide/tests/scanner.rs @@ -598,56 +598,62 @@ mod scanner { &[ // This file should always be picked up ("home/project/apps/web/index.html", "content-['index.html']"), - // Set up various ignore rules ("home/.gitignore", "ignore-home.html"), ("home/project/.gitignore", "ignore-project.html"), ("home/project/apps/.gitignore", "ignore-apps.html"), ("home/project/apps/web/.gitignore", "ignore-web.html"), - // Some of these should be ignored depending on which dir is the repo root - ("home/project/apps/web/ignore-home.html", "content-['ignore-home.html']"), - ("home/project/apps/web/ignore-project.html", "content-['ignore-project.html']"), - ("home/project/apps/web/ignore-apps.html", "content-['ignore-apps.html']"), - ("home/project/apps/web/ignore-web.html", "content-['ignore-web.html']"), + ( + "home/project/apps/web/ignore-home.html", + "content-['ignore-home.html']", + ), + ( + "home/project/apps/web/ignore-project.html", + "content-['ignore-project.html']", + ), + ( + "home/project/apps/web/ignore-apps.html", + "content-['ignore-apps.html']", + ), + ( + "home/project/apps/web/ignore-web.html", + "content-['ignore-web.html']", + ), ], ); - - let sources = vec![ - GlobEntry { - base: dir.join("home/project/apps/web").to_string_lossy().to_string(), - pattern: "**/*".to_owned(), - }, - ]; + let sources = vec![GlobEntry { + base: dir + .join("home/project/apps/web") + .to_string_lossy() + .to_string(), + pattern: "**/*".to_owned(), + }]; let candidates = Scanner::new(Some(sources.clone())).scan(); // All ignore files are applied because there's no git repo - assert_eq!( - candidates, - vec![ - "content-['index.html']".to_owned(), - ] - ); + assert_eq!(candidates, vec!["content-['index.html']".to_owned(),]); // Initialize `home` as a git repository and scan again // The results should be the same as before - _ = Command::new("git").arg("init").current_dir(dir.join("home")).output(); + _ = Command::new("git") + .arg("init") + .current_dir(dir.join("home")) + .output(); let candidates = Scanner::new(Some(sources.clone())).scan(); - assert_eq!( - candidates, - vec![ - "content-['index.html']".to_owned(), - ] - ); + assert_eq!(candidates, vec!["content-['index.html']".to_owned(),]); // Drop the .git folder fs::remove_dir_all(dir.join("home/.git")).unwrap(); // Initialize `home/project` as a git repository and scan again - _ = Command::new("git").arg("init").current_dir(dir.join("home/project")).output(); + _ = Command::new("git") + .arg("init") + .current_dir(dir.join("home/project")) + .output(); let candidates = Scanner::new(Some(sources.clone())).scan(); assert_eq!( @@ -662,7 +668,10 @@ mod scanner { fs::remove_dir_all(dir.join("home/project/.git")).unwrap(); // Initialize `home/project/apps` as a git repository and scan again - _ = Command::new("git").arg("init").current_dir(dir.join("home/project/apps")).output(); + _ = Command::new("git") + .arg("init") + .current_dir(dir.join("home/project/apps")) + .output(); let candidates = Scanner::new(Some(sources.clone())).scan(); assert_eq!( @@ -678,7 +687,10 @@ mod scanner { fs::remove_dir_all(dir.join("home/project/apps/.git")).unwrap(); // Initialize `home/project/apps` as a git repository and scan again - _ = Command::new("git").arg("init").current_dir(dir.join("home/project/apps/web")).output(); + _ = Command::new("git") + .arg("init") + .current_dir(dir.join("home/project/apps/web")) + .output(); let candidates = Scanner::new(Some(sources.clone())).scan(); assert_eq!( From ad1b13c2031de546e639895e1d314ef82dc4da44 Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Tue, 28 Jan 2025 10:57:14 -0500 Subject: [PATCH 7/8] Add integration test --- integrations/cli/index.test.ts | 77 ++++++++++++++++++++++++++++++++++ integrations/utils.ts | 10 ++++- 2 files changed, 85 insertions(+), 2 deletions(-) diff --git a/integrations/cli/index.test.ts b/integrations/cli/index.test.ts index aff5b108994f..b47659a1320e 100644 --- a/integrations/cli/index.test.ts +++ b/integrations/cli/index.test.ts @@ -556,6 +556,83 @@ describe.each([ ]) }, ) + + test( + 'git ignore files outside of a repo are not considered', + { + fs: { + // Ignore everything in the "home" directory + 'home/.gitignore': '*', + + // Only ignore files called ignore-*.html in the actual git repo + 'home/project/.gitignore': 'ignore-*.html', + + 'home/project/package.json': json` + { + "type": "module", + "dependencies": { + "tailwindcss": "workspace:^", + "@tailwindcss/cli": "workspace:^" + } + } + `, + + 'home/project/src/index.css': css` @import 'tailwindcss'; `, + 'home/project/src/index.html': html` +
+ `, + 'home/project/src/ignore-1.html': html` +
+ `, + 'home/project/src/ignore-2.html': html` +
+ `, + }, + + installDependencies: false, + }, + async ({ fs, root, exec }) => { + await exec(`pnpm install --ignore-workspace`, { + cwd: path.join(root, 'home/project'), + }) + + // No git repo = all ignore files are considered + await exec(`${command} --input src/index.css --output dist/out.css`, { + cwd: path.join(root, 'home/project'), + }) + + await fs.expectFileNotToContain('./home/project/dist/out.css', [ + candidate`content-['index.html']`, + candidate`content-['ignore-1.html']`, + candidate`content-['ignore-2.html']`, + ]) + + // Make home/project a git repo + // Only ignore files within the repo are considered + await exec(`git init`, { + cwd: path.join(root, 'home/project'), + }) + + await exec(`${command} --input src/index.css --output dist/out.css`, { + cwd: path.join(root, 'home/project'), + }) + + await fs.expectFileToContain('./home/project/dist/out.css', [ + candidate`content-['index.html']`, + ]) + + await fs.expectFileNotToContain('./home/project/dist/out.css', [ + candidate`content-['ignore-1.html']`, + candidate`content-['ignore-2.html']`, + ]) + }, + ) }) test( diff --git a/integrations/utils.ts b/integrations/utils.ts index 4644ece49ab8..20f3fb49ec9e 100644 --- a/integrations/utils.ts +++ b/integrations/utils.ts @@ -32,6 +32,8 @@ interface TestConfig { fs: { [filePath: string]: string | Uint8Array } + + installDependencies?: boolean } interface TestContext { root: string @@ -382,14 +384,18 @@ export function test( await context.fs.write(filename, content) } + let shouldInstallDependencies = config.installDependencies ?? true + try { // In debug mode, the directory is going to be inside the pnpm workspace // of the tailwindcss package. This means that `pnpm install` will run // pnpm install on the workspace instead (expect if the root dir defines // a separate workspace). We work around this by using the // `--ignore-workspace` flag. - let ignoreWorkspace = debug && !config.fs['pnpm-workspace.yaml'] - await context.exec(`pnpm install${ignoreWorkspace ? ' --ignore-workspace' : ''}`) + if (shouldInstallDependencies) { + let ignoreWorkspace = debug && !config.fs['pnpm-workspace.yaml'] + await context.exec(`pnpm install${ignoreWorkspace ? ' --ignore-workspace' : ''}`) + } } catch (error: any) { console.error(error) console.error(error.stdout?.toString()) From 8e769954ec763575571ea0306e97ce804e8ef816 Mon Sep 17 00:00:00 2001 From: Jordan Pittman Date: Tue, 28 Jan 2025 11:09:44 -0500 Subject: [PATCH 8/8] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d2170c400940..2cd3ea0ad8eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Suggest container query variants ([#15857](https://github.com/tailwindlabs/tailwindcss/pull/15857)) - Disable bare value suggestions when not using the `--spacing` variable ([#15857](https://github.com/tailwindlabs/tailwindcss/pull/15857)) - Ensure suggested classes are properly sorted ([#15857](https://github.com/tailwindlabs/tailwindcss/pull/15857)) +- Don’t look at ignore files outside initialized repos ([#15941](https://github.com/tailwindlabs/tailwindcss/pull/15941)) - _Upgrade_: Ensure JavaScript config files on different drives are correctly migrated ([#15927](https://github.com/tailwindlabs/tailwindcss/pull/15927)) ## [4.0.0] - 2025-01-21