Fix @source with folders that are ignored (#20214)
This PR fixes an issue where a `@source` that's pointing to a folder that is git ignored, is also ignored by the `@source` even if it's explicitly added. Internally, we convert `@source` directives from `PublicSourceEntry`s to `SourceEntry`s where we have dedicated enum branches for `Auto`, `Pattern`, `Ignored` and `External`. The `Auto` one accepts a `base` path, and will be used for auto content detection. However, these paths will make use of all the default auto content detection rules, which includes git ignore rules. We also have `External` where we link to something that's "external" to the current repo. We can probably improve this name, but it's external in the sense that it won't show up on GitHub for example, aka ignored. We have some content dirs that we ignore by default, such as the `node_modules` folder. When you do use `@source` with `node_modules` in the path, then we will mark it as an `external` resource which does not look at the `gitignore` related rules and allowing it to be included this way. The idea with this is that, even though the folder is ignored by default, you can still include files from the folder by explicitly using the `@source` directive. The issue as seen in #19844 is using `vendor/` instead of `node_modules/` which is _not_ ignored by default. While we can add `vendor/` to this same ignored dirs list, it will result in a breaking change because this folder is often used by the Laravel community to store some resources in. This PR fixes this problem by not only looking at the content dirs we ignore by default, but also looking at the actual git ignore state of this folder. If it turns out that this is ignored, then we promote the `Auto` source to an `External` source. Fixes: #19844 Closes: #20057 ## Test plan 1. Added integration tests for this situation 2. Ran the fix on the reproduction from #19844. If we run the CLI with the `DEBUG=*` environment variable, the log file produces these results: ```diff diff --git a/./tailwindcss-29207.log b/./tailwindcss-30381.log index bc3017c..921af0e 100644 --- a/./tailwindcss-29207.log +++ b/./tailwindcss-30381.log @@ -6,8 +6,9 @@ INFO tailwindcss_oxide::scanner: Source: PublicSourceEntry { base: "/Users/robin INFO tailwindcss_oxide::scanner: Optimized sources: INFO tailwindcss_oxide::scanner: Source: Pattern { base: "/Users/robin/github.com/GrimLink/tailwind-gitignore-bug/app/design/frontend/theme", pattern: "/**/*.phtml" } INFO tailwindcss_oxide::scanner: Source: Pattern { base: "/Users/robin/github.com/GrimLink/tailwind-gitignore-bug/app/design/frontend/theme", pattern: "/**/*.xml" } -INFO tailwindcss_oxide::scanner: Source: Auto { base: "/Users/robin/github.com/GrimLink/tailwind-gitignore-bug/vendor/acme/theme" } +INFO tailwindcss_oxide::scanner: Source: External { base: "/Users/robin/github.com/GrimLink/tailwind-gitignore-bug/vendor/acme/theme" } INFO tailwindcss_oxide::scanner: Source: Ignored { base: "/Users/robin/.fnm/node-versions/v26.1.0/installation/bin", pattern: "/node" } INFO discover_sources: tailwindcss_oxide::scanner: enter -INFO discover_sources: tailwindcss_oxide::scanner: Reading "/Users/robin/github.com/GrimLink/tailwind-gitignore-bug/app/design/frontend/theme/index.phtml" +INFO tailwindcss_oxide::scanner: Reading "/Users/robin/github.com/GrimLink/tailwind-gitignore-bug/app/design/frontend/theme/index.phtml" +INFO tailwindcss_oxide::scanner: Reading "/Users/robin/github.com/GrimLink/tailwind-gitignore-bug/vendor/acme/theme/module/templates/component.phtml" INFO discover_sources: tailwindcss_oxide::scanner: exit ``` We're checking some `.gitignore` related files, so let's check on each OS [ci-all]
This commit is contained in:
Robin Malfait
GitHub
parent
1d5e15e9d8
commit
1bf4291e85
6 changed files with 256 additions and 26 deletions
|
|
@ -31,6 +31,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||
- Ensure later `@source` rules can re-include files excluded by earlier `@source not` rules ([#20203](https://github.com/tailwindlabs/tailwindcss/pull/20203))
|
||||
- Upgrade: don't migrate empty class rules to invalid `@utility` rules ([#20205](https://github.com/tailwindlabs/tailwindcss/pull/20205))
|
||||
- Ensure transitions between `inset-shadow-none` and other inset shadows work correctly ([#20208](https://github.com/tailwindlabs/tailwindcss/pull/20208))
|
||||
- Ensure explicitly referenced `@source` directories are scanned even when ignored by git ([#20214](https://github.com/tailwindlabs/tailwindcss/pull/20214))
|
||||
|
||||
### Changed
|
||||
|
||||
|
|
|
|||
|
|
@ -688,7 +688,7 @@ fn create_walker(sources: &Sources) -> Option<WalkBuilder> {
|
|||
}
|
||||
|
||||
// External sources should take precedence even over git-ignored files:
|
||||
emit(base, format!("!{}", "/**/*"));
|
||||
emit(base, "!/**/*".to_owned());
|
||||
|
||||
// External sources should still disallow binary extensions:
|
||||
emit(base, BINARY_EXTENSIONS_GLOB.clone());
|
||||
|
|
@ -780,14 +780,7 @@ fn create_walker(sources: &Sources) -> Option<WalkBuilder> {
|
|||
let pattern_sources: Vec<(PathBuf, String)> = sources
|
||||
.iter()
|
||||
.filter_map(|source| match source {
|
||||
SourceEntry::Pattern { base, pattern } => {
|
||||
let normalized = if pattern.starts_with("/") {
|
||||
pattern.to_string()
|
||||
} else {
|
||||
format!("/{pattern}")
|
||||
};
|
||||
Some((base.clone(), normalized))
|
||||
}
|
||||
SourceEntry::Pattern { base, pattern } => Some((base.into(), pattern.into())),
|
||||
_ => None,
|
||||
})
|
||||
.collect();
|
||||
|
|
|
|||
|
|
@ -1,5 +1,7 @@
|
|||
use crate::GlobEntry;
|
||||
use bexpand::Expression;
|
||||
use fxhash::FxHashMap;
|
||||
use ignore::gitignore::Gitignore;
|
||||
use std::path::{Component, Path, PathBuf};
|
||||
use tracing::{event, Level};
|
||||
|
||||
|
|
@ -48,13 +50,14 @@ pub enum SourceEntry {
|
|||
/// ```
|
||||
Ignored { base: PathBuf, pattern: String },
|
||||
|
||||
/// External sources are sources outside of your git root which should not
|
||||
/// follow gitignore rules.
|
||||
/// External sources are directories that are ignored (by us or .gitignore rules), but should be
|
||||
/// included bypassing the default ignore rules.
|
||||
///
|
||||
/// Represented by:
|
||||
///
|
||||
/// ```css
|
||||
/// @source "../node_modules/my-lib";`
|
||||
/// @source "../node_modules/my-lib/**/*";`
|
||||
/// ```
|
||||
External { base: PathBuf },
|
||||
}
|
||||
|
|
@ -311,6 +314,80 @@ mod tests {
|
|||
);
|
||||
assert_eq!(source.pattern, "/**/*.html");
|
||||
}
|
||||
|
||||
/// Run the public-to-private conversion for an auto-detected source pointing at `base` and
|
||||
/// return the resulting entry.
|
||||
fn auto_source_entry(base: &Path) -> SourceEntry {
|
||||
public_source_entries_to_private_source_entries(vec![PublicSourceEntry {
|
||||
base: base.to_string_lossy().to_string(),
|
||||
pattern: "**/*".to_string(),
|
||||
negated: false,
|
||||
}])
|
||||
.into_iter()
|
||||
.next()
|
||||
.unwrap()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn auto_detected_folders_become_auto_sources() {
|
||||
let dir = tempdir().unwrap();
|
||||
let base = dir.path().join("src");
|
||||
fs::create_dir_all(&base).unwrap();
|
||||
let base = dunce::canonicalize(&base).unwrap();
|
||||
|
||||
assert_eq!(auto_source_entry(&base), SourceEntry::Auto { base });
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn folders_ignored_by_default_become_external_sources() {
|
||||
let dir = tempdir().unwrap();
|
||||
let base = dir.path().join("node_modules").join("my-lib");
|
||||
fs::create_dir_all(&base).unwrap();
|
||||
let base = dunce::canonicalize(&base).unwrap();
|
||||
|
||||
assert_eq!(auto_source_entry(&base), SourceEntry::External { base });
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn folders_ignored_by_gitignore_become_external_sources() {
|
||||
let dir = tempdir().unwrap();
|
||||
// Pretend this is a git repository so the `.gitignore` search is bounded to it.
|
||||
fs::create_dir_all(dir.path().join(".git")).unwrap();
|
||||
fs::write(dir.path().join(".gitignore"), "dist/\n").unwrap();
|
||||
|
||||
let base = dir.path().join("dist");
|
||||
fs::create_dir_all(&base).unwrap();
|
||||
let base = dunce::canonicalize(&base).unwrap();
|
||||
|
||||
assert_eq!(auto_source_entry(&base), SourceEntry::External { base });
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn folders_ignored_by_a_parent_gitignore_become_external_sources() {
|
||||
let dir = tempdir().unwrap();
|
||||
fs::create_dir_all(dir.path().join(".git")).unwrap();
|
||||
// A `.gitignore` higher up in the tree should still apply to nested directories.
|
||||
fs::write(dir.path().join(".gitignore"), "generated/\n").unwrap();
|
||||
|
||||
let base = dir.path().join("packages").join("app").join("generated");
|
||||
fs::create_dir_all(&base).unwrap();
|
||||
let base = dunce::canonicalize(&base).unwrap();
|
||||
|
||||
assert_eq!(auto_source_entry(&base), SourceEntry::External { base });
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn folders_not_ignored_by_gitignore_stay_auto_sources() {
|
||||
let dir = tempdir().unwrap();
|
||||
fs::create_dir_all(dir.path().join(".git")).unwrap();
|
||||
fs::write(dir.path().join(".gitignore"), "dist/\n").unwrap();
|
||||
|
||||
let base = dir.path().join("src");
|
||||
fs::create_dir_all(&base).unwrap();
|
||||
let base = dunce::canonicalize(&base).unwrap();
|
||||
|
||||
assert_eq!(auto_source_entry(&base), SourceEntry::Auto { base });
|
||||
}
|
||||
}
|
||||
|
||||
/// For each public source entry:
|
||||
|
|
@ -360,19 +437,91 @@ pub fn public_source_entries_to_private_source_entries(
|
|||
})
|
||||
.collect::<Vec<_>>();
|
||||
|
||||
// Compiled `.gitignore` matchers are cached per directory so we read and parse each
|
||||
// `.gitignore` file at most once, even though entries commonly share ancestor directories
|
||||
// (e.g. the repository root). A cached `None` means the directory has no `.gitignore` file.
|
||||
let mut gitignores: FxHashMap<PathBuf, Option<Gitignore>> = FxHashMap::default();
|
||||
|
||||
// Boundary for the `.gitignore` walk when a source is not inside a git repository (see
|
||||
// below).
|
||||
let cwd = std::env::current_dir()
|
||||
.map(|cwd| dunce::canonicalize(&cwd).unwrap_or(cwd))
|
||||
.ok();
|
||||
|
||||
// Convert from public SourceEntry to private SourceEntry
|
||||
expanded_globs
|
||||
.into_iter()
|
||||
.map(Into::into)
|
||||
.collect::<Vec<_>>()
|
||||
.map(|public_source| {
|
||||
let mut source: SourceEntry = public_source.into();
|
||||
|
||||
// Promote auto-sources to external sources if they were gitignored
|
||||
if let SourceEntry::Auto { ref base } = source {
|
||||
let inside_git_repo = base.ancestors().any(|dir| dir.join(".git").exists());
|
||||
|
||||
// Walk up from the folder, applying each `.gitignore` relative to the directory
|
||||
// that contains it (matching git), and stop at the git repository root so
|
||||
// `.gitignore` files outside of the repo are not considered.
|
||||
for dir in base.ancestors() {
|
||||
let gitignore = gitignores.entry(dir.to_path_buf()).or_insert_with(|| {
|
||||
let path = dir.join(".gitignore");
|
||||
|
||||
// `Gitignore::new` roots the matcher at the directory
|
||||
// containing the file, so patterns match relative to it.
|
||||
path.is_file().then(|| Gitignore::new(&path).0)
|
||||
});
|
||||
|
||||
if let Some(gitignore) = gitignore {
|
||||
if gitignore
|
||||
.matched_path_or_any_parents(&base, true)
|
||||
.is_ignore()
|
||||
{
|
||||
source = SourceEntry::External { base: base.into() };
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
// Stop at the git repository root.
|
||||
if dir.join(".git").exists() {
|
||||
break;
|
||||
}
|
||||
|
||||
// Without a git repository there is no repository root to stop at. Stop
|
||||
// once the directory contains the current working directory instead, so
|
||||
// `.gitignore` files outside of the project (e.g. in the user's home
|
||||
// directory) can never promote a source to an external source. Note that
|
||||
// the file walker still applies those `.gitignore` files when deciding
|
||||
// which files to scan.
|
||||
if !inside_git_repo && cwd.as_ref().is_some_and(|cwd| cwd.starts_with(dir)) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
source
|
||||
})
|
||||
.collect::<Vec<SourceEntry>>()
|
||||
}
|
||||
|
||||
/// Convert a public source entry to a source entry
|
||||
impl From<PublicSourceEntry> for SourceEntry {
|
||||
fn from(value: PublicSourceEntry) -> Self {
|
||||
if value.negated {
|
||||
return SourceEntry::Ignored {
|
||||
base: value.base.into(),
|
||||
pattern: value.pattern,
|
||||
};
|
||||
}
|
||||
|
||||
let auto = value.pattern.ends_with("**/*")
|
||||
|| PathBuf::from(&value.base).join(&value.pattern).is_dir();
|
||||
|
||||
if !auto {
|
||||
return SourceEntry::Pattern {
|
||||
base: value.base.into(),
|
||||
pattern: value.pattern,
|
||||
};
|
||||
}
|
||||
|
||||
let inside_ignored_content_dir = IGNORED_CONTENT_DIRS.iter().any(|dir| {
|
||||
value.base.contains(&format!(
|
||||
"{}{}{}",
|
||||
|
|
@ -384,21 +533,13 @@ impl From<PublicSourceEntry> for SourceEntry {
|
|||
.ends_with(&format!("{}{}", std::path::MAIN_SEPARATOR, dir,))
|
||||
});
|
||||
|
||||
match (value.negated, auto, inside_ignored_content_dir) {
|
||||
(false, true, false) => SourceEntry::Auto {
|
||||
match inside_ignored_content_dir {
|
||||
false => SourceEntry::Auto {
|
||||
base: value.base.into(),
|
||||
},
|
||||
(false, true, true) => SourceEntry::External {
|
||||
true => SourceEntry::External {
|
||||
base: value.base.into(),
|
||||
},
|
||||
(false, false, _) => SourceEntry::Pattern {
|
||||
base: value.base.into(),
|
||||
pattern: value.pattern,
|
||||
},
|
||||
(true, _, _) => SourceEntry::Ignored {
|
||||
base: value.base.into(),
|
||||
pattern: value.pattern,
|
||||
},
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1514,6 +1514,30 @@ mod scanner {
|
|||
);
|
||||
}
|
||||
|
||||
// https://github.com/tailwindlabs/tailwindcss/issues/19844
|
||||
#[test]
|
||||
fn test_allow_explicit_sources_ignored_by_allow_list_gitignore() {
|
||||
let ScanResult { candidates, .. } = scan_with_globs(
|
||||
&[
|
||||
(".gitignore", "*\n!/app\n!/app/design\n!/app/design/**\n"),
|
||||
(
|
||||
"app/design/frontend/theme/templates/component.phtml",
|
||||
"content-['app/design/frontend/theme/templates/component.phtml']",
|
||||
),
|
||||
(
|
||||
"vendor/acme/theme/module/templates/component.phtml",
|
||||
"content-['vendor/acme/theme/module/templates/component.phtml']",
|
||||
),
|
||||
],
|
||||
vec!["@source 'vendor/acme/theme'"],
|
||||
);
|
||||
|
||||
assert_eq!(
|
||||
candidates,
|
||||
vec!["content-['vendor/acme/theme/module/templates/component.phtml']"]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_ignore_node_modules_without_gitignore() {
|
||||
let ScanResult {
|
||||
|
|
|
|||
|
|
@ -2083,6 +2083,76 @@ test(
|
|||
},
|
||||
)
|
||||
|
||||
// https://github.com/tailwindlabs/tailwindcss/issues/19844
|
||||
test(
|
||||
'@source scans directories ignored by allow-list .gitignore files',
|
||||
{
|
||||
fs: {
|
||||
'package.json': json`
|
||||
{
|
||||
"dependencies": {
|
||||
"tailwindcss": "workspace:^",
|
||||
"@tailwindcss/cli": "workspace:^"
|
||||
}
|
||||
}
|
||||
`,
|
||||
'.gitignore': txt`
|
||||
*
|
||||
!/app
|
||||
!/app/design
|
||||
!/app/design/**
|
||||
`,
|
||||
'src/index.css': css`
|
||||
@import 'tailwindcss/utilities' source(none);
|
||||
@source '../vendor/acme/theme';
|
||||
`,
|
||||
// 1. Ignored by the `*` in `.gitignore`
|
||||
// 2. Included by the `!` pattern in `.gitignore`
|
||||
// 3. Ignored by `source(none)`
|
||||
//
|
||||
// → Should be ignored
|
||||
'app/design/frontend/theme/templates/component.phtml': html`
|
||||
<div
|
||||
class="content-['app/design/frontend/theme/templates/component.phtml']"
|
||||
></div>
|
||||
`,
|
||||
// 1. Ignored by the `*` in `.gitignore`
|
||||
// 2. Included by the `!` pattern in `.gitignore`
|
||||
// 3. Ignored by `source(none)`
|
||||
// 4. Included by the `@source` directive
|
||||
//
|
||||
// → Should be included
|
||||
'vendor/acme/theme/module/templates/component.phtml': html`
|
||||
<div
|
||||
class="content-['vendor/acme/theme/module/templates/component.phtml']"
|
||||
></div>
|
||||
`,
|
||||
},
|
||||
},
|
||||
async ({ fs, exec }) => {
|
||||
await exec('pnpm tailwindcss --input src/index.css --output dist/out.css')
|
||||
|
||||
// 1. Ignored by the `*` in `.gitignore`
|
||||
// 2. Included by the `!` pattern in `.gitignore`
|
||||
// 3. Ignored by `source(none)`
|
||||
//
|
||||
// → Should be ignored
|
||||
await fs.expectFileNotToContain('dist/out.css', [
|
||||
candidate`content-['app/design/frontend/theme/templates/component.phtml']`,
|
||||
])
|
||||
|
||||
// 1. Ignored by the `*` in `.gitignore`
|
||||
// 2. Included by the `!` pattern in `.gitignore`
|
||||
// 3. Ignored by `source(none)`
|
||||
// 4. Included by the `@source` directive
|
||||
//
|
||||
// → Should be included
|
||||
await fs.expectFileToContain('dist/out.css', [
|
||||
candidate`content-['vendor/acme/theme/module/templates/component.phtml']`,
|
||||
])
|
||||
},
|
||||
)
|
||||
|
||||
test(
|
||||
'@source works with symlinks (referencing folder in current folder)',
|
||||
{
|
||||
|
|
|
|||
|
|
@ -59,7 +59,7 @@ interface TestContext {
|
|||
filePath: string,
|
||||
contents: string | RegExp | (string | RegExp)[],
|
||||
): Promise<void>
|
||||
expectFileNotToContain(filePath: string, contents: string | string[]): Promise<void>
|
||||
expectFileNotToContain(filePath: string, contents: string[]): Promise<void>
|
||||
}
|
||||
}
|
||||
type TestCallback = (context: TestContext) => Promise<void> | void
|
||||
|
|
@ -481,7 +481,7 @@ export function test(
|
|||
try {
|
||||
await context.exec('git init', { cwd: root })
|
||||
await context.exec('git add --all', { cwd: root })
|
||||
await context.exec('git commit -m "before migration"', { cwd: root })
|
||||
await context.exec('git commit -m "before migration" --allow-empty', { cwd: root })
|
||||
} catch (error: any) {
|
||||
console.error(error)
|
||||
console.error(error.stdout?.toString())
|
||||
|
|
@ -597,6 +597,7 @@ export async function retryAssertion<T>(
|
|||
try {
|
||||
return await fn()
|
||||
} catch (err) {
|
||||
Error.captureStackTrace(err, retryAssertion)
|
||||
error = err
|
||||
await new Promise((resolve) => setTimeout(resolve, delay))
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue