FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Adding support for native command globbing on UNIX by BrucePay · Pull Request #3643 · PowerShell/PowerShell · GitHub

Adding support for native command globbing on UNIX - #3643

Merged
Dongbo Wang (daxian-dbw) merged 2 commits into
PowerShell:masterfrom
BrucePay:brucepay-glob
May 1, 2017
Merged

Adding support for native command globbing on UNIX#3643
Dongbo Wang (daxian-dbw) merged 2 commits into
PowerShell:masterfrom
BrucePay:brucepay-glob

Conversation

Bruce Payette (BrucePay) commented Apr 25, 2017
edited
Loading

Copy link
Copy Markdown
Collaborator

This change enables globbing (wildcard expansion) against the file system for native commands like /bin/ls. Expansion is only done in the file system. In non-filesystem drives expansion is not done and the pattern is returned unchanged.

Limitations:

Currently quoting is not honored so for a command like /bin/ls "*.txt", wildcard expansion will still be done. Adding support for bare word detection will come in a future PR. Use --% to suppress wildcard expansion e.g. git add --% *

This (partially) fixes #954

Copy link
Copy Markdown
Member

BIN test/powershell/Language/Scripting/NativeExecution/NativeUnixGlobbing.Tests.ps1
Binary file not shown.

This usually indicates the encoding of the file is not utf-8. Could you please change the encoding to utf-8?

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I thought we settled on calling the C api glob.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I have no recollection of any such thing. Why would we do that rather than using PowerShell's intrinsic globbing capability?

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

To be compatible as compatible with bash as reasonably possible. There are things glob does that we don't with character classes.

I don't recall discussing or looking closely at the inverse - if our wildcards support things that glob does not.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

PowerShell globbing supports all of the constructs described in glob(3) but is case-insensitive plus it understands powershell drives which fnmatch(3) is unaware of. Having the globbing behavior change based on the type of the command sounds like a very bad user experience.

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Are you sure? /bin/echo * should not echo files starting with ., and /bin/echo "*" should echo *, not the files in the current directory.

Maybe less interesting, but glob(7) says glob supports character classes like [:upper:] and more - we definitely don't support those. That said, it doesn't look like bash supports those.

Case-insensitive also concerns me. rm M* shouldn't remove files starting with lowercase m.

I'm not saying that glob(3) is exactly what we should use - just that there may be subtleties that we miss by not using a POSIX api.

As for PowerShell drives - I'm really curious if they'll be used on non-Windows platforms - I'm skeptical - symbolic links have served that purpose perfectly and work outside of PowerShell.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looking at the bash source, a lot of the subtleties around globbing are in bash itself, not the glob routine. For example ~ expansion is not handled by glob. Likewise quote processing is handled in bash itself, not by the glob routine. Given that PowerShell does it's quote removal (and addition) in a very different way than bash, there are going to be differences. (To make /bin/echo "*" work, we need to be able to be able to determine if a string value was parsed as a bareword literal at execution time.) Also note that Bash optionally supports extended globbing (See Bash Extended Globbing for an overview). Anyway, I've opened a new issue to track this investigation #3655

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I would rather these tests be done in $TESTDRIVE which will eliminate the need for AfterAll ($TESTDRIVE and contents are automatically deleted by pester)

    BeforeAll {
        if (-not $IsWindows )
        {
            "" > "$TESTDRIVE/abc.txt"
            "" > "$TESTDRIVE/bbb.txt"
            "" > "$TESTDRIVE/cbb.txt"
        }
    }

and the tests:

    # Test simple * expansion
    It 'The globbing pattern should match 3 files' @PesterSkip {
        (/bin/ls $TESTDRIVE/*.txt).Length | Should Be 3
    }

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Fixed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

How about using Context.EngineSessionState.CurrentLocation? Then you can check if (pwd.Provider.Name == FileSystemProvider.ProviderName) to see if it's a filesystem location, and pwd.ProviderPath to get the full path.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Fixed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

How about if (! (pbo is System.IO.FileSystemInfo))?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Fixed.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

It's recommended to use is-expression introduced in C# 7 to replace the as operator because it can help reduce nested brackets. For example, here it can be:

if (pbo is FileInfo fi)
{
    ...
}
else if (pbo is DirectoryInfo di)
{
    ...
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

With the changes to the code, this no longer applies.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Why not use SessionState.Path.NormalizeRelativePath.NormalizeRelativePath(string path, string basePath)? We use it in Resolve-Path.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Fixed.

Steve Lee (SteveL-MSFT) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks like you accidentally pulled in other commits, remove them from this PR

Bruce Payette (BrucePay) force-pushed the brucepay-glob branch 2 times, most recently from fec390e to d9955b0 Compare April 27, 2017 23:16

Copy link
Copy Markdown
Member

Since Bruce Payette (@BrucePay) is OOF for a conference, I pushed a commit to update the tests to use the recommended SKIP pattern.

Copy link
Copy Markdown

Might anyone be able to explain

Use --% to suppress wildcard expansion e.g. git add --% *

a little more, I can't get it to work in my command line. It may be that it also suppresses $variable evaluation?

$database="postgres://$(othercalculatedelements)"
/bin/echo --% -database "$database" -source file://./update up

results in $database being echoed literally, not evaluated.

Copy link
Copy Markdown
Member

Chris F Carroll (@chrisfcarroll) --% basically treats everything after it as literal, you should be able to:

$database="postgres://$(othercalculatedelements)"
/bin/echo -database "$database" --% -source file://./update up

Chris F Carroll (chrisfcarroll) commented Sep 21, 2017
edited
Loading

Copy link
Copy Markdown

Steve Lee (@SteveL-MSFT) thanks for clarification. Alas that brings back all the problems of bug #3931 which is what I'm struggling to work around.

Bruce Payette (BrucePay) deleted the brucepay-glob branch June 21, 2018 21:59
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj/PowerShell that referenced this pull request Aug 6, 2026
This change enables globbing (wildcard expansion) against the file system for native commands like '/bin/ls'. The expansion is only done in the file system. In non-filesystem drives expansion is not done and the pattern is returned unchanged.

Limitations of the fix:
Currently quoting is not honored so for a command like /bin/ls "*.txt", wildcard expansion will still be done. Adding support for bare word detection will come in a future PR. Use --% to suppress wildcard expansion e.g. git add --% *
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

Implement shell globbing

7 participants


Back | FazBrowse Home | New Git URL