Skip to content

Conversation

@tdejager
Copy link
Contributor

@tdejager tdejager commented Sep 19, 2025

Use the ** globs instead of */**. The latter does not match files in the current directory for ignore and other glob implementations, wax was incorrect in that regard. We need this when switching to: prefix-dev/pixi#4578. (This PR is needed to fix build errors there).

In draft until I verify the behavior completely.

@tdejager tdejager marked this pull request as draft September 19, 2025 13:30
@baszalmstra
Copy link
Contributor

ah yeah makes sense!

@tdejager
Copy link
Contributor Author

I verfied that it works with the wax implementation as well.

@tdejager
Copy link
Contributor Author

@baszalmstra yeah fyi unix glob normally matches */** as:

* → first matches exactly one directory in the current directory.
/** → then matches everything inside it, at any recursive depth.

So:

  • It will not match things in the current directory.
  • It will match all files and directories inside each immediate subdirectory, at any depth.

But for wax it is the same as **

@tdejager tdejager marked this pull request as ready for review September 22, 2025 07:17
@lucascolley
Copy link
Collaborator

out of interest, is **/* the same as **? Or does that exclude top-level files too?

@tdejager
Copy link
Contributor Author

tdejager commented Sep 22, 2025

I think it should be yes. In our case at least.

@tdejager
Copy link
Contributor Author

Test in suite this 'proves' (as far as those tests prove) backward compatibility.
prefix-dev/pixi-build-testsuite#69

Copy link
Contributor

@Hofer-Julian Hofer-Julian left a comment

Choose a reason for hiding this comment

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

Looks good, also great that you added comments to the existing logic!

@Hofer-Julian Hofer-Julian merged commit f61be52 into prefix-dev:main Sep 22, 2025
13 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