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.

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

Don’t look at ignore files outside initialized repos #15941

Merged
merged 8 commits into from
Jan 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
65 changes: 57 additions & 8 deletions crates/oxide/src/scanner/allowed_paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,11 @@ pub fn resolve_allowed_paths(root: &Path) -> impl Iterator<Item = DirEntry> {

#[tracing::instrument(skip_all)]
pub fn resolve_paths(root: &Path) -> impl Iterator<Item = DirEntry> {
WalkBuilder::new(root)
.hidden(false)
.require_git(false)
.build()
.filter_map(Result::ok)
create_walk_builder(root).build().filter_map(Result::ok)
}

pub fn read_dir(root: &Path, depth: Option<usize>) -> impl Iterator<Item = DirEntry> {
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() {
Expand All @@ -59,6 +53,61 @@ pub fn read_dir(root: &Path, depth: Option<usize>) -> impl Iterator<Item = DirEn
.filter_map(Result::ok)
}

fn create_walk_builder(root: &Path) -> 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);

// 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;
}
}

builder
}

pub fn is_allowed_content_path(path: &Path) -> bool {
// Skip known ignored files
if path
Expand Down
117 changes: 117 additions & 0 deletions crates/oxide/tests/scanner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -586,4 +586,121 @@ 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(),
]
);
}
}
77 changes: 77 additions & 0 deletions integrations/cli/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`
<div
class="content-['index.html']"
></div>
`,
'home/project/src/ignore-1.html': html`
<div
class="content-['ignore-1.html']"
></div>
`,
'home/project/src/ignore-2.html': html`
<div
class="content-['ignore-2.html']"
></div>
`,
},

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(
Expand Down
10 changes: 8 additions & 2 deletions integrations/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ interface TestConfig {
fs: {
[filePath: string]: string | Uint8Array
}

installDependencies?: boolean
}
interface TestContext {
root: string
Expand Down Expand Up @@ -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())
Expand Down