Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PS 7.4.0: "-Tail 0" ignores "-Wait" and immediately stops Get-Content #20731

Closed
5 tasks done
6e6e58c4 opened this issue Nov 20, 2023 · 8 comments · Fixed by #20734
Closed
5 tasks done

PS 7.4.0: "-Tail 0" ignores "-Wait" and immediately stops Get-Content #20731

6e6e58c4 opened this issue Nov 20, 2023 · 8 comments · Fixed by #20734
Labels
In-PR Indicates that a PR is out for the issue

Comments

@6e6e58c4
Copy link

Prerequisites

Steps to reproduce

Get-Content -Path "<any file>" -Wait -Tail 0

Expected behavior

PS> Get-Content -Path "<any file>" -Wait -Tail 0
<no file content, waiting for new content>

Actual behavior

PS> Get-Content -Path "<any file>" -Wait -Tail 0
PS> <Get-Content immediately stops>

Error details

No response

Environment data

PSVersion: 7.4.0
PSEdition: Core
GitCommitId: 7.4.0
OS: Microsoft Windows 10.0.19044
Platform: Win32NT
PSCompatibleVersions: {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion: 2.3
SerializationVersion: 1.1.0.1
WSManStackVersion: 3.0

Visuals

gc-tail-broken

@6e6e58c4 6e6e58c4 added the Needs-Triage The issue is new and needs to be triaged by a work group. label Nov 20, 2023
@6e6e58c4
Copy link
Author

6e6e58c4 commented Nov 20, 2023

Might be caused by #19715 and that || Tail == 0

if (TotalCount == 0 || Tail == 0)
            {
                // Don't read anything
                return;

https://github.com/PowerShell/PowerShell/pull/19715/files#diff-0cf19feadd29d19fb01dcfe57f5fd9bd24be0d5fed7c1241f0cfa05da14c2fc1R90

GitHub
PR Summary

Disallow negative values for -Head and -Tail
Add fast return if -Tail == 0
Cleanup code

TODO:

Add tests

PR Context
#19710 @jhoneill @mklement0

PR Checklist

PR has a meaningful ti...

@mklement0
Copy link
Contributor

mklement0 commented Nov 20, 2023

Yes, it looks like the - meaningful and previously supported - combination of -Tail 0 with -Wait wasn't considered in the linked PR.
Correct, @CarloToso?

@NoMoreFood
Copy link
Contributor

In my opinion, -Tail 0 -Wait is a super common use case.

@CarloToso
Copy link
Contributor

CarloToso commented Nov 20, 2023

Yes, it looks like the - meaningful and previously supported - combination of -Tail 0 with -Wait wasn't considered in the linked PR. Correct, @CarloToso?

It was an oversight on my part, I'll open a PR to fix it later. Should we also support -Head 0 with -Wait ?

@mklement0
Copy link
Contributor

mklement0 commented Nov 20, 2023

Thanks, @CarloToso.

Yes, I'd say there's no reason to not also support -Head 0 (though probably an uncommon use case)

However, note that combining -Head with -Wait is currently fundamentally broken, which predates your PR (same behavior in WinPS).

The docs don't mention any mutual exclusion for -Wait (other than, sensibly, -Raw).

At least hypothetically, there could be - conceptually flawed - code out there that relies on -Wait being ignored in the presence of -Head / -TotalCount, though that strikes me as a bucket 3 change still worth making.
@SteveL-MSFT, any thoughts?

@mklement0
Copy link
Contributor

mklement0 commented Nov 20, 2023

@CarloToso, a simple way forward is to fix the regression bug with respect to -Tail only.

The -Head case (if you will) can then be dealt with separately.

Update: See

@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR Indicates that a PR is out for the issue label Nov 21, 2023
@echalone
Copy link

Well, good thing I looked in the open bugs, was about to report the same issue :D

Copy link
Contributor

microsoft-github-policy-service bot commented Jan 11, 2024

📣 Hey @6e6e58c4, how did we do? We would love to hear your feedback with the link below! 🗣️

🔗 https://aka.ms/PSRepoFeedback

@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs-Triage The issue is new and needs to be triaged by a work group. label Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
In-PR Indicates that a PR is out for the issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants