Skip to content

Commit 3f58e52

Browse files
authored
Ensure @source globs with symlinks are preserved (#20203)
This PR fixes an issue when working with `@source` and `@source not` that involves symlinks. Internally our sources are mapped to a source entry where we have a `base` path and a `pattern`. You can think about this where we insert a `.gitignore` file in the `base` path for the given pattern. However, we optimize these entries to move as many "static" parts into the base path. For example: ```css @source "./some/folder/here/*.html"; ``` Is mapped to something like: ```ts { base: "/projects/my-project", pattern: "./some/folder/here/*.html" } ``` We then optimize it by turning it into: ```ts { base: "/projects/my-project/some/folder/here", pattern: "*.html" } ``` While doing this, we also use `dunce::canonicalize` to resolve the actual paths on disk. This means that a symlink is resolved to their real paths. This can cause issues because the "real" path is not what you wrote in the `@source` directives. So before, it could be that you have this: ```css @source "./some/symlinked-folder/here/*.html"; ``` Which was mapped to this on the Rust side: ```ts { base: "/projects/my-project", pattern: "./some/symlinked-folder/here/*.html" } ``` But was then optimized to: ```ts { base: "/projects/my-project/some/actual-folder/here", pattern: "*.html" } ``` ...and we lost the `symlinked-folder` information. This causes issues as seen in #17985. With this PR, we keep the symlinked information in those globs since that's what you wrote in those `@source` directives. While setting up integration tests, I stumbled upon an issue because I wanted to test that ignoring a symlinked folder, but including a single particular file of that ignored folder resulted in that file being ignored as well. Let's look at an example: ```css @source '../lib'; @source not '../lib/ignored'; @source '../lib/ignored/except.html'; ``` Earlier I mentioned that we create `.gitignore` files based on these `@source` directives. In this case, when we're dealing with a folder, we use `**/*` as the contents. Looking at the example above, we should essentially have something like this: ```gitignore # lib/.gitignore # @source '../lib' !**/* # lib/ignored/.gitignore # @source '../lib/ignored' **/* # @source '../lib/ignored/except.html' !except.html ``` Since it's a `.gitignore` file, we have to invert the globs. But the bug I noticed is that in reality the result of those gitignores didn't look like the above, it looked like: ```gitignore # lib/.gitignore # @source '../lib' !**/* # lib/ignored/.gitignore # @source '../lib/ignored/except.html' !except.html # @source '../lib/ignored' **/* ``` Notice how the `!except.html` and `**/*` are flipped. When dealing with `.gitignore` files, the order is important. This was caused because internally we kept a `BTreeMap` of `BTreeSet`s where the map was the base path and a set of patterns. The patterns were sorted because of the `BTreeSet`... which is not what we want. Fixes: #17985 Closes: #20091 ## Test plan 1. Added new tests in the scanner tests (on the Rust side) 2. Added integration tests with a symlink to another folder, outside of the current folder 2. Added integration tests with a symlink to another folder, inside of the current folder 2. Added integration tests to ensure that the order of `@source` files with a folder + file is sorted correctly. 3. Since we're dealing with symlinks in these tests, let's test all OSes [ci-all]
1 parent ad66939 commit 3f58e52

6 files changed

Lines changed: 560 additions & 94 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2727
- Ensure class candidates are extracted from Twig `addClass(…)` and `removeClass(…)` calls ([#20198](https://github.com/tailwindlabs/tailwindcss/pull/20198))
2828
- Don't crash in the Ruby or Vue preprocessors when scanning files containing invalid UTF-8 bytes ([#19588](https://github.com/tailwindlabs/tailwindcss/pull/19588))
2929
- Allow `@variant` to be used inside `addBase` ([#19480](https://github.com/tailwindlabs/tailwindcss/pull/19480))
30+
- Ensure `@source` globs with symlinks are preserved ([#20203](https://github.com/tailwindlabs/tailwindcss/pull/20203))
31+
- Ensure later `@source` rules can re-include files excluded by earlier `@source not` rules ([#20203](https://github.com/tailwindlabs/tailwindcss/pull/20203))
3032

3133
### Changed
3234

crates/oxide/src/scanner/mod.rs

Lines changed: 16 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ use fxhash::{FxHashMap, FxHashSet};
1717
use ignore::{gitignore::GitignoreBuilder, WalkBuilder};
1818
use init_tracing::{init_tracing, SHOULD_TRACE};
1919
use rayon::prelude::*;
20-
use std::collections::{BTreeMap, BTreeSet};
2120
use std::path::{Path, PathBuf};
2221
use std::sync::{Arc, Mutex};
2322
use std::time::SystemTime;
@@ -637,7 +636,16 @@ fn walk_parallel(walker: &mut WalkBuilder) -> Vec<WalkEntry> {
637636
fn create_walker(sources: &Sources) -> Option<WalkBuilder> {
638637
let mut other_roots: FxHashSet<&PathBuf> = FxHashSet::default();
639638
let mut first_root: Option<&PathBuf> = None;
640-
let mut ignores: BTreeMap<&PathBuf, BTreeSet<String>> = Default::default();
639+
640+
let mut ignores: Vec<(&PathBuf, Vec<String>)> = Default::default();
641+
let mut emit = |base, pattern| match ignores.last_mut() {
642+
Some((prev_base, patterns)) if *prev_base == base => {
643+
patterns.push(pattern);
644+
}
645+
_ => {
646+
ignores.push((base, vec![pattern]));
647+
}
648+
};
641649

642650
for source in sources.iter() {
643651
match source {
@@ -649,7 +657,7 @@ fn create_walker(sources: &Sources) -> Option<WalkBuilder> {
649657
}
650658
}
651659
SourceEntry::Pattern { base, pattern } => {
652-
let mut pattern = pattern.to_string();
660+
let pattern = pattern.to_owned();
653661

654662
if first_root.is_none() {
655663
first_root = Some(base);
@@ -658,35 +666,19 @@ fn create_walker(sources: &Sources) -> Option<WalkBuilder> {
658666
}
659667

660668
if !pattern.contains("**") {
661-
// Ensure that the pattern is pinned to the base path.
662-
if !pattern.starts_with("/") {
663-
pattern = format!("/{pattern}");
664-
}
665-
666669
// Specific patterns should take precedence even over git-ignored files:
667-
ignores
668-
.entry(base)
669-
.or_default()
670-
.insert(format!("!{}", pattern));
670+
emit(base, format!("!{}", pattern));
671671
} else {
672672
// Assumption: the pattern we receive will already be brace expanded. So
673673
// `*.{html,jsx}` will result in two separate patterns: `*.html` and `*.jsx`.
674674
if let Some(extension) = Path::new(&pattern).extension() {
675675
// Extend auto source detection to include the extension
676-
ignores
677-
.entry(base)
678-
.or_default()
679-
.insert(format!("!*.{}", extension.to_string_lossy()));
676+
emit(base, format!("!*.{}", extension.to_string_lossy()));
680677
}
681678
}
682679
}
683680
SourceEntry::Ignored { base, pattern } => {
684-
let mut pattern = pattern.to_string();
685-
// Ensure that the pattern is pinned to the base path.
686-
if !pattern.starts_with("/") {
687-
pattern = format!("/{pattern}");
688-
}
689-
ignores.entry(base).or_default().insert(pattern);
681+
emit(base, pattern.to_owned());
690682
}
691683
SourceEntry::External { base } => {
692684
if first_root.is_none() {
@@ -696,16 +688,10 @@ fn create_walker(sources: &Sources) -> Option<WalkBuilder> {
696688
}
697689

698690
// External sources should take precedence even over git-ignored files:
699-
ignores
700-
.entry(base)
701-
.or_default()
702-
.insert(format!("!{}", "/**/*"));
691+
emit(base, format!("!{}", "/**/*"));
703692

704693
// External sources should still disallow binary extensions:
705-
ignores
706-
.entry(base)
707-
.or_default()
708-
.insert(BINARY_EXTENSIONS_GLOB.clone());
694+
emit(base, BINARY_EXTENSIONS_GLOB.clone());
709695
}
710696
}
711697
}

crates/oxide/src/scanner/sources.rs

Lines changed: 194 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
1-
use crate::glob::split_pattern;
21
use crate::GlobEntry;
32
use bexpand::Expression;
4-
use std::path::PathBuf;
3+
use std::path::{Component, Path, PathBuf};
54
use tracing::{event, Level};
65

76
use super::auto_source_detection::IGNORED_CONTENT_DIRS;
@@ -102,72 +101,215 @@ impl PublicSourceEntry {
102101
/// resolved path.
103102
pub fn optimize(&mut self) {
104103
// Resolve base path immediately
105-
let Ok(base) = dunce::canonicalize(&self.base) else {
104+
let Ok(mut base) = dunce::canonicalize(&self.base) else {
106105
event!(Level::ERROR, "Failed to resolve base: {:?}", self.base);
107106
return;
108107
};
109-
self.base = base.to_string_lossy().to_string();
110108

111-
// No dynamic part, figure out if we are dealing with a file or a directory.
112-
if !self.pattern.contains('*') {
113-
let combined_path = if self.pattern.starts_with("/") {
114-
PathBuf::from(&self.pattern)
115-
} else {
116-
PathBuf::from(&self.base).join(&self.pattern)
117-
};
109+
let mut new_pattern = PathBuf::new();
110+
enum ComponentStage {
111+
Base,
112+
Pattern,
113+
}
114+
let mut stage = ComponentStage::Base;
115+
116+
let mut components = Path::new(&self.pattern).components().peekable();
117+
while let Some(component) = components.next() {
118+
match stage {
119+
ComponentStage::Base => {
120+
match component {
121+
// Ignore the current dir, e.g. `.`
122+
Component::CurDir => {}
123+
124+
// Go up a directory, e.g. `..`
125+
Component::ParentDir => {
126+
base.pop();
127+
}
128+
129+
// Once we hit a component that contains a wildcard character, then we
130+
// can't change the base anymore and we must move to the pattern part.
131+
Component::Normal(part) if part.to_string_lossy().contains("*") => {
132+
new_pattern.push(component);
133+
stage = ComponentStage::Pattern;
134+
}
135+
136+
// File or folder, but not the last component
137+
Component::Normal(part) if components.peek().is_some() => {
138+
base.push(part);
139+
}
118140

119-
match dunce::canonicalize(combined_path) {
120-
Ok(resolved_path) if resolved_path.is_dir() => {
121-
self.base = resolved_path.to_string_lossy().to_string();
122-
self.pattern = "**/*".to_owned();
141+
// Last file or folder. If it's a folder, we move it to the base,
142+
// otherwise we move it to the pattern.
143+
Component::Normal(part) => {
144+
let full_path = base.join(part);
145+
if full_path.is_dir() {
146+
base.push(part);
147+
} else {
148+
new_pattern.push(part);
149+
}
150+
}
151+
152+
// When we're dealing with an absolute path, then we have to bypass the
153+
// `base` entirely.
154+
Component::Prefix(_) => {
155+
base.clear();
156+
base.push(component);
157+
}
158+
Component::RootDir => {
159+
#[cfg(not(windows))]
160+
base.clear();
161+
base.push(component);
162+
}
163+
}
123164
}
124-
Ok(resolved_path) if resolved_path.is_file() => {
125-
self.base = resolved_path
126-
.parent()
127-
.unwrap()
128-
.to_string_lossy()
129-
.to_string();
130-
// Ensure leading slash, otherwise it will match against all files in all folders/
131-
self.pattern =
132-
format!("/{}", resolved_path.file_name().unwrap().to_string_lossy());
165+
ComponentStage::Pattern => {
166+
new_pattern.push(component);
133167
}
134-
_ => {}
135168
}
136-
return;
137169
}
138170

139-
// Contains dynamic part
140-
let (static_part, dynamic_part) = split_pattern(&self.pattern);
141-
142-
let base: PathBuf = self.base.clone().into();
143-
let base = match static_part {
144-
Some(static_part) => {
145-
// TODO: If the base does not exist on disk, try removing the last slash and try
146-
// again.
147-
match dunce::canonicalize(base.join(static_part)) {
148-
Ok(base) => base,
149-
Err(err) => {
150-
event!(tracing::Level::ERROR, "Failed to resolve glob: {:?}", err);
151-
return;
152-
}
171+
self.base = base.to_string_lossy().to_string();
172+
self.pattern = path_to_posix_string(&new_pattern);
173+
174+
// Ensure we have `**/*` when the base is a folder and we don't have a pattern at all
175+
if self.pattern == "" {
176+
self.pattern = "/**/*".to_owned();
177+
}
178+
// Ensure that the pattern is pinned to the base path.
179+
else if !self.pattern.starts_with("/") {
180+
self.pattern = format!("/{}", self.pattern);
181+
}
182+
}
183+
}
184+
185+
fn path_to_posix_string(path: &Path) -> String {
186+
let mut parts = Vec::new();
187+
let mut is_rooted = false;
188+
189+
for component in path.components() {
190+
match component {
191+
Component::Prefix(prefix) => {
192+
parts.push(prefix.as_os_str().to_string_lossy().to_string());
193+
}
194+
Component::RootDir => {
195+
is_rooted = true;
196+
if parts.is_empty() {
197+
parts.push(String::new());
153198
}
154199
}
155-
None => base,
200+
Component::CurDir => {
201+
parts.push(".".to_string());
202+
}
203+
Component::ParentDir => {
204+
parts.push("..".to_string());
205+
}
206+
Component::Normal(part) => {
207+
parts.push(part.to_string_lossy().to_string());
208+
}
209+
}
210+
}
211+
212+
let result = parts.join("/");
213+
if result.is_empty() && is_rooted {
214+
"/".to_string()
215+
} else {
216+
result
217+
}
218+
}
219+
220+
#[cfg(test)]
221+
mod tests {
222+
use super::*;
223+
use pretty_assertions::assert_eq;
224+
use std::fs;
225+
use tempfile::tempdir;
226+
227+
#[test]
228+
fn path_to_posix_string_serializes_relative_paths() {
229+
let path = PathBuf::from("src").join("**").join("*.html");
230+
231+
assert_eq!(path_to_posix_string(&path), "src/**/*.html");
232+
}
233+
234+
#[test]
235+
fn path_to_posix_string_serializes_rooted_paths() {
236+
let path = PathBuf::from(std::path::MAIN_SEPARATOR.to_string())
237+
.join("src")
238+
.join("**")
239+
.join("*.html");
240+
241+
assert_eq!(path_to_posix_string(&path), "/src/**/*.html");
242+
}
243+
244+
#[test]
245+
fn path_to_posix_string_serializes_empty_paths() {
246+
assert_eq!(path_to_posix_string(&PathBuf::new()), "");
247+
}
248+
249+
#[test]
250+
fn optimize_hoists_static_directories_and_keeps_files_in_the_pattern() {
251+
let dir = tempdir().unwrap();
252+
fs::create_dir_all(dir.path().join("src").join("examples")).unwrap();
253+
254+
let mut source = PublicSourceEntry {
255+
base: dir.path().to_string_lossy().to_string(),
256+
pattern: "src/examples/index.html".to_string(),
257+
negated: false,
156258
};
157259

158-
let pattern = match dynamic_part {
159-
Some(dynamic_part) => dynamic_part,
160-
None => {
161-
if base.is_dir() {
162-
"**/*".to_owned()
163-
} else {
164-
"".to_owned()
165-
}
166-
}
260+
source.optimize();
261+
262+
assert_eq!(
263+
source.base,
264+
dunce::canonicalize(dir.path().join("src").join("examples"))
265+
.unwrap()
266+
.to_string_lossy()
267+
);
268+
assert_eq!(source.pattern, "/index.html");
269+
}
270+
271+
#[test]
272+
fn optimize_hoists_folder_patterns() {
273+
let dir = tempdir().unwrap();
274+
fs::create_dir_all(dir.path().join("src").join("examples")).unwrap();
275+
276+
let mut source = PublicSourceEntry {
277+
base: dir.path().to_string_lossy().to_string(),
278+
pattern: "src/examples".to_string(),
279+
negated: false,
167280
};
168281

169-
self.base = base.to_string_lossy().to_string();
170-
self.pattern = pattern;
282+
source.optimize();
283+
284+
assert_eq!(
285+
source.base,
286+
dunce::canonicalize(dir.path().join("src").join("examples"))
287+
.unwrap()
288+
.to_string_lossy()
289+
);
290+
assert_eq!(source.pattern, "/**/*");
291+
}
292+
293+
#[test]
294+
fn optimize_keeps_wildcards_in_the_pattern() {
295+
let dir = tempdir().unwrap();
296+
fs::create_dir_all(dir.path().join("src")).unwrap();
297+
298+
let mut source = PublicSourceEntry {
299+
base: dir.path().to_string_lossy().to_string(),
300+
pattern: "src/**/*.html".to_string(),
301+
negated: false,
302+
};
303+
304+
source.optimize();
305+
306+
assert_eq!(
307+
source.base,
308+
dunce::canonicalize(dir.path().join("src"))
309+
.unwrap()
310+
.to_string_lossy()
311+
);
312+
assert_eq!(source.pattern, "/**/*.html");
171313
}
172314
}
173315

0 commit comments

Comments
 (0)