indygreg / PyOxidizer

A modern Python application packaging and distribution tool
Mozilla Public License 2.0
5.42k stars 234 forks source link

starlark_tugger.glob() appears to always use / as the path separator, even on Windows #421

Open durin42 opened 3 years ago

durin42 commented 3 years ago

This took a while to figure out, but as far as I can tell a glob like glob(include=["C:\foo\bar\**\*"]) will never match anything, and instead would need to be expressed with forward slashes (that is, glob(include=["C:/foo/bar/**/*"])) in order to match things.

indygreg commented 3 years ago

Fascinating.

Reading the glob crate source, it appears \ should work. However, we're wrapping this code and it is possible for us to mix forward and back slashes. I wonder if that is confusing something. But most Windows APIs should treat / and \ interchangeably. I dunno what's going on here.

I'm tempted to change the code to bail if backslashes are used at all. That looks weird for absolute paths on Windows. But it is consistent and does appear to work according to your testimony!

durin42 commented 3 years ago

It's entirely possible I got confused at some point in the glob headaches I was having the day I did this, FWIW.

On Sat, Jul 24, 2021 at 5:40 PM Gregory Szorc @.***> wrote:

Fascinating.

Reading the glob crate source, it appears \ should work. However, we're wrapping this code and it is possible for us to mix forward and back slashes. I wonder if that is confusing something. But most Windows APIs should treat / and \ interchangeably. I dunno what's going on here.

I'm tempted to change the code to bail if backslashes are used at all. That looks weird for absolute paths on Windows. But it is consistent and does appear to work according to your testimony!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/indygreg/PyOxidizer/issues/421#issuecomment-886114382, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAE6LMBW6KALMVRX46KZBLTZMXMNANCNFSM5AP243BA .