Skip to content

Fix infinite recursion in ignore#540

Merged
Nicell merged 4 commits intoluau-lang:primaryfrom
wmccrthy:fix-ignore
Nov 7, 2025
Merged

Fix infinite recursion in ignore#540
Nicell merged 4 commits intoluau-lang:primaryfrom
wmccrthy:fix-ignore

Conversation

@wmccrthy
Copy link
Copy Markdown
Contributor

@wmccrthy wmccrthy commented Nov 7, 2025

ignore was recently ported over from a different repo, that used an adhoc path implementation. This repo's path.dirname returned a string or nil, but lute's path.dirname always returns a string.

On various occasions in ignore ( one example here), we gate continued processing behind checks that path.dirname(arbitrary) exists. These are all invalid checks now and in some of these cases, we recur leading to death by stack overflow.

This PR fixes aforementioned checks, using path.basename (returns string | nil if at root) to determine if the current path is the root.

I found this bug when encountering a stack overflow error while trying to use lute transform:

_readIgnoreRecursive
... (+19979 frames)
@cli/transform/lib/ignore.luau:227 function _readIgnoreRecursive
@cli/transform/lib/ignore.luau:227 function _readIgnoreRecursive
@cli/transform/lib/ignore.luau:227 function _readIgnoreRecursive
@cli/transform/lib/ignore.luau:227 function _readIgnoreRecursive
@cli/transform/lib/ignore.luau:227 function _readIgnoreRecursive
@cli/transform/lib/ignore.luau:214 function isIgnoredDirectory
@cli/transform/lib/files.luau:16 function traverseDirectoryRecursive
@cli/transform/lib/files.luau:52 function getSourceFiles
@cli/transform/init.luau:139 function main
@cli/transform/init.luau:153

I rebuilt lute with these changes and transform worked as expected.

@wmccrthy wmccrthy marked this pull request as ready for review November 7, 2025 01:13
Copy link
Copy Markdown
Collaborator

@Vighnesh-V Vighnesh-V left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small change, but otherwise looks good to me.

Comment thread lute/cli/commands/transform/lib/ignore.luau Outdated
@wmccrthy wmccrthy requested a review from Vighnesh-V November 7, 2025 17:06
local function parseGitignore(filepath: path.path): GitignoreData
local contents = fs.readfiletostring(path.format(filepath))
return parseGitignoreContents(assert(path.dirname(filepath)), contents)
local parentDirectory = if path.basename(filepath) then path.dirname(filepath) else nil
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have path.path objects, so I believe the following would be more performant:
if #path.parts > 0 then path.dirname(filepath) else nil

Copy link
Copy Markdown
Contributor Author

@wmccrthy wmccrthy Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If filePath is always a table here, won't path.basename mirror this exact logic (non-nil if #path.parts > 0 and nil otherwise)? Happy to change but curious what the performance bottleneck would be

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, the logic will be the same, but we avoid the table check and passing the string basename around this way

@wmccrthy wmccrthy requested a review from skberkeley November 7, 2025 21:31
@Nicell Nicell merged commit 4f98891 into luau-lang:primary Nov 7, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants