Enable path globbing for native commands by andyleejordan · Pull Request #2325 · PowerShell/PowerShell

Andy Jordan (andyleejordan) · 2016-09-20T23:32:00Z

Resolves Implement shell globbing #954
Still need tests
Need to update known issues
Need to fix globbing to have unix semantics:
- expand hidden (.dotfile) directories
- return relative (not absolute) paths
Need to file issue about escape characters

Enable path globbing for native commands.

E.g. ls *.md expands to ls /path/to/README.md /path/to/CHANGELOG.md

If expansion fails, the argument is sent literally, so ls foo* results in /usr/bin/ls: cannot access foo*: No such file or directory.

Don't glob expand if the verbatim marker was used. So ls --% * fails with /bin/ls: cannot access '*': No such file or directory

Same caveats as #1639; but the code is enabled only on *nix platforms, and the failing test is fixed.

Microsoft Contribution License Agreements (msftclas) · 2016-09-20T23:32:03Z

Hi Andy Jordan (@andschwa), I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

It looks like you're a Microsoft contributor (Andy Schwartzmeyer). If you're full-time, we DON'T require a Contribution License Agreement. If you are a vendor, please DO sign the electronic Contribution License Agreement. It will take 2 minutes and there's no faxing! https://cla.microsoft.com.

TTYL, MSBOT;

Andy Jordan (andyleejordan) · 2016-09-20T23:44:13Z

Jeffery's great. The problem with ls .git* and other .dotfiles is that the globbing semantics by default ignore hidden directories. This isn't a problem with Get-ChildItem because it can be passed -Hidden; but for globbing on *nix, I'll add a set of semantics to the globber to include hidden files (and return relative instead of absolute paths).

Jason Shirk (lzybkr) · 2016-09-21T01:28:23Z

You can avoid a memory allocation that is thrown away in a common case by initializing with null instead.

Jason Shirk (lzybkr) · 2016-09-21T01:32:09Z

This is worth some discussion - I think we'll actually want to add escaping to globbing instead of this.

While I agree escaping is the better way to go, I think that not only can it be done separately, it really needs to be done separately as it's a much much larger change (and would affect many more systems, as this is the same globbing system used by cmdlets).

sergei (vors)

I think we should talk about the overall approach in more details

sergei (vors) · 2016-09-21T01:44:50Z

I would argue that we should provide some way to control it and not tide it to the platform. We can have different defaults on different platforms.

But the problem come with cross-platform tools (i.e. git). It would be pretty unfun to get inconsistent behavior on different platforms.

Jason Shirk (@lzybkr) wanted it tied to the platform in #1639. Do you want something in addition to --% to disable globbing?

I agree, inconsistent behavior isn't fun. But this is what I'm operating off:

At a minimum, I think we can't do globbing on Windows at all - Windows tools implement globbing themselves, so even if we did something, it might work differently than the tool we're calling.

So Windows versus Linux tools are already introducing inconsistent behavior regardless of us.

As I said, the motivation example are tools that exists on both platforms, like git, gcc and so on.

Yes, and they appear to already operate differently. Investigating:

Git

PS C:\Users\andrew\src\test> ls Directory: C:\Users\andrew\src\test Mode LastWriteTime Length Name ---- ------------- ------ ---- -a---- 9/22/2016 10:42 AM 0 foo -a---- 9/22/2016 10:42 AM 0 foobar -a---- 9/22/2016 10:42 AM 0 foobaz -a---- 9/22/2016 10:42 AM 0 foobil -a---- 9/22/2016 10:42 AM 0 foocod PS C:\Users\andrew\src\test> git add foo* PS C:\Users\andrew\src\test> git status On branch master Initial commit Changes to be committed: (use "git rm --cached <file>..." to unstage) new file: foo new file: foobar new file: foobaz new file: foobil new file: foocod

It did the same with PowerShell on Linux without these changes; so it globs itself on both platforms. That said, Bash definitely glob expands and it works just fine.

GCC

On Linux without these changes:

~/test |-/ ls foobar.c foobaz.c foobil.c foo.c foocod.c ~/test |-/ gcc *.c gcc: error: *.c: No such file or directory gcc: fatal error: no input files compilation terminated.

But with GCC through MinGW (i.e. not Cygwin) on Windows, GCC globs:

PS C:\Users\andrew\src\test> ls Directory: C:\Users\andrew\src\test Mode LastWriteTime Length Name ---- ------------- ------ ---- -a---- 9/22/2016 11:07 AM 0 foo.c -a---- 9/22/2016 11:07 AM 0 foobar.c -a---- 9/22/2016 11:07 AM 0 foobaz.c -a---- 9/22/2016 11:07 AM 0 foobil.c -a---- 9/22/2016 11:07 AM 0 foocod.c PS C:\Users\andrew\src\test> C:\MinGW\bin\gcc.exe *.c PS C:\Users\andrew\src\test> PS C:\Users\andrew\src\test> C:\MinGW\bin\gcc.exe *.blah gcc.exe: error: *.blah: Invalid argument gcc.exe: fatal error: no input files compilation terminated.

We can't test everything, but so far Jason Shirk (@lzybkr) appears right, and tools built for Windows (even cross-platform ones like Git and GCC) don't expect globbing to be done for them, and so do it themselves.

Thank you for the detailed info!

sergei (vors) · 2016-09-21T01:49:34Z

I actually don't get why globbing place any role here. There is no *, so it should not alternate the arguments that are passed.

[ is also a wildcard character.

And was previously discovered by Jason Shirk (@lzybkr), the :: is being interpreted as a filesystem provider separator.

The meaning of :: as file path separator should not anyhow trigger globbing resolving. The [ makes more sense, I think the explanation comment above needs correction.

Sure, I agree; but it's what's happening:

> & $powershell 2>&1 -noprofile -command '[Console]::Error.Write(\"foo`n`nbar`n`nbaz\");' Cannot find a provider with the name '[Console]'. At line:1 char:1 + & $powershell 2>&1 -noprofile -command '[Console]::Error.Write(\"foo` ... + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + CategoryInfo : ObjectNotFound: ([Console]:String) [], ProviderNotFoundException + FullyQualifiedErrorId : ProviderNotFound

This is where I could really use some help, is there a better globber to use inside of PowerShell than this:

context.LocationGlobber.GetGlobbedProviderPathsFromMonadPath

It sure seemed correct; but I have minimal context.

(By the way, if you do the non-sensical test of:

~ |-/ dir '[Console]::Error.Write(\"foo`n`nbar`n`nbaz\");' dir : Cannot find a provider with the name '[Console]'. At line:1 char:1 + dir '[Console]::Error.Write(\"foo`n`nbar`n`nbaz\");' + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + CategoryInfo : ObjectNotFound: ([Console]:String) [Get-ChildItem], ProviderNotFoundException + FullyQualifiedErrorId : ProviderNotFound,Microsoft.PowerShell.Commands.GetChildItemCommand

the same problem happens due to globbing. This is just a weird scenario because we're passing non-path data to a native command, but now glob arguments to native commands; and in this case we don't want to do that, so use verbatim.)

If expansion fails, the argument is sent literally, so ls foo* results in /usr/bin/ls: cannot access foo*: No such file or directory.

Same should be applied here: if expansion failed due to provider error, the argument should be passed literally.

Ah I see, yes I agree. Will fix.

Andy Jordan (andyleejordan) · 2016-09-22T17:28:45Z

I think we should talk about the overall approach in more details

sergei (@vors) Sure, please do. What ideas do you have? My approach was to plumb PowerShell's existing globbing logic into the native command parameter parser; minimum work and reuses logic to be consistent.

sergei (vors) · 2016-09-22T17:38:17Z

Andy Jordan (@andschwa) I was mostly referring to UX and scripts portability, not the implementation. My concerns are in the single-line comments above.

sergei (vors) · 2016-09-22T18:19:43Z

Hm, still getting used to GitHub reviews. I tried to remove my "change needed" mark, but the only way I found was to turn it into "approved". Now I don't see a way to recall it.

E.g. `ls *.md` expands to `ls /path/to/README.md /path/to/CHANGELOG.md` If expansion fails, the argument is sent literally, so `ls foo*` results in `/usr/bin/ls: cannot access foo*: No such file or directory`. Don't glob expand if the verbatim marker was used.

sergei (vors) · 2017-01-28T06:57:02Z

Per commitie decision we should revisit this work item.

Jason Shirk (@lzybkr) proposed an implementation that would just call glob(3) lib function. From glancing at the current version of the code, it seems that it could modified to this with minimal changes. All changes would be localized in native command binder.

Andy Jordan (andyleejordan) · 2017-01-30T18:44:19Z

sergei (@vors) just read through the decision, it looks like a solid plan to me. I think we can close this PR, but feel free to reuse any portions of course.

Microsoft Contribution License Agreements (msftclas) added the cla-not-required label Sep 20, 2016

Jason Shirk (lzybkr) reviewed Sep 21, 2016

View reviewed changes

sergei (vors) suggested changes Sep 21, 2016

View reviewed changes

sergei (vors) approved these changes Sep 22, 2016

View reviewed changes

Andy Jordan (andyleejordan) mentioned this pull request Sep 27, 2016

Add RFC 0009 to extend globbing to native tools PowerShell/PowerShell-RFC#31

Merged

Andy Jordan (andyleejordan) force-pushed the globbing branch 2 times, most recently from fa20ea1 to e072f7e Compare September 27, 2016 21:23

Andy Jordan (andyleejordan) mentioned this pull request Sep 27, 2016

RFC0009-Glob-for-Native-Tools comments PowerShell/PowerShell-RFC#33

Closed

Andy Jordan (andyleejordan) force-pushed the globbing branch from e072f7e to 92baf00 Compare September 28, 2016 19:26

Partially restore relative paths

fa50131

Andy Jordan (andyleejordan) mentioned this pull request Oct 4, 2016

Implement shell globbing #954

Closed

sergei (vors) mentioned this pull request Oct 5, 2016

Fix set-content failure on Linux for creating file in psdrive. (#2206) #2392

Merged

PowerShellTeam added the Review - Needed label Nov 10, 2016

Hemant Mahawar (HemantMahawar) removed the Review-Needed label Dec 1, 2016

Andy Jordan (andyleejordan) self-assigned this Dec 8, 2016

Andy Jordan (andyleejordan) removed their assignment Jan 25, 2017

Andy Jordan (andyleejordan) closed this Jan 30, 2017

Ilya (iSazonov) removed the Review - Needed The PR is being reviewed label Mar 27, 2017

Conversation

Andy Jordan (andyleejordan) commented Sep 20, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Microsoft Contribution License Agreements (msftclas) commented Sep 20, 2016

Uh oh!

Andy Jordan (andyleejordan) commented Sep 20, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sergei (vors) left a comment

Choose a reason for hiding this comment

Uh oh!

sergei (vors) Sep 21, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Git

GCC

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Andy Jordan (andyleejordan) Sep 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Andy Jordan (andyleejordan) Sep 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Andy Jordan (andyleejordan) commented Sep 22, 2016

Uh oh!

sergei (vors) commented Sep 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sergei (vors) commented Sep 22, 2016

Uh oh!

sergei (vors) commented Jan 28, 2017

Uh oh!

Andy Jordan (andyleejordan) commented Jan 30, 2017

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Andy Jordan (andyleejordan) commented Sep 20, 2016 •

edited

Loading

sergei (vors) Sep 21, 2016 •

edited

Loading

Andy Jordan (andyleejordan) Sep 22, 2016 •

edited

Loading

Andy Jordan (andyleejordan) Sep 22, 2016 •

edited

Loading

sergei (vors) commented Sep 22, 2016 •

edited

Loading