Skip to content

Conversation

mknos
Copy link
Contributor

@mknos mknos commented Sep 22, 2024

  • This version of grep skips binary files by default; this behaviour is inconsistent with GNU version but the difference could be addressed later
  • If the file argument exists but cannot be opened, grep was skipping the file with a strange "binary file" message
  • I found this when testing "grep -s", which is expected to exit with 2 if a file couldn't be opened
  • I triggered open() error by removing file permissions on my Linux system
%chmod 0 a.c
%perl grep -T include a.c
grep: skipping binary file "a.c"
  • Correct the binary file condition; skip the file argument if file is binary (filetest -B) and we're not running grep -a
  • Correct the logic for exit code; $Errors should increment regardless of -s because exit code is determined from it
  • Bump version
  • With patch applied, grep (without -s) correctly prints the error from open() (Permission Denied for my test), and exits with code 2
  • Regression test: "perl grep -s pat1 bad.file.name" --> exit 2, silent

* If file argument exists but cannot be opened, grep was skipping the file with a strange "binary file" message
* I found this when testing "grep -s", which is expected to exit with 2 if a file couldn't be opened
* I triggered open() error by removing file permissions on my Linux system
%chmod 0 a.c
%perl grep -T include a.c 
grep: skipping binary file "a.c"

* Correct the binary file condition; skip the file argument if file is binary (filetest -B) and we're not running grep -a
* Correct the logic for exit code; $Errors should increment regardless of -s because exit code is determined from it
* Bump version
* Regression test: "perl grep -s pat1 bad.file.name" --> exit 2, silent
@mknos mknos temporarily deployed to automated_testing September 22, 2024 11:59 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing September 22, 2024 11:59 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing September 22, 2024 11:59 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing September 22, 2024 11:59 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing September 22, 2024 11:59 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing September 22, 2024 11:59 — with GitHub Actions Inactive
@github-actions github-actions bot added the Type: enhancement improve a feature that already exists label Sep 22, 2024
@mknos mknos temporarily deployed to automated_testing September 22, 2024 11:59 — with GitHub Actions Inactive
@github-actions github-actions bot added Priority: low get to this whenever Program: grep The grep program labels Sep 22, 2024
@mknos mknos temporarily deployed to automated_testing September 22, 2024 11:59 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing September 22, 2024 11:59 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing September 22, 2024 11:59 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing September 22, 2024 11:59 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing September 22, 2024 11:59 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing September 22, 2024 11:59 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing September 22, 2024 11:59 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing September 22, 2024 11:59 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing September 22, 2024 11:59 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing September 22, 2024 11:59 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing September 22, 2024 11:59 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing September 22, 2024 11:59 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing September 22, 2024 11:59 — with GitHub Actions Inactive
@coveralls
Copy link

coveralls commented Sep 22, 2024

Pull Request Test Coverage Report for Build 10980919074

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.7%) to 73.069%

Totals Coverage Status
Change from base Build 10973938881: -0.7%
Covered Lines: 350
Relevant Lines: 479

💛 - Coveralls

Copy link
Owner

@briandfoy briandfoy left a comment

Choose a reason for hiding this comment

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

nice catch

@briandfoy briandfoy merged commit b6a4dcf into briandfoy:master Sep 22, 2024
22 of 23 checks passed
@briandfoy briandfoy added Type: bug an existing feature does not work and removed Type: enhancement improve a feature that already exists labels Sep 22, 2024
@briandfoy briandfoy added Status: accepted The fix is accepted and removed Priority: low get to this whenever labels Sep 22, 2024
@briandfoy
Copy link
Owner

changes: -s now exits with correct value if It cannot read the file; -a works correctly if the perl thinks the file is binary and you selected -a

@briandfoy briandfoy self-assigned this Sep 24, 2024
@briandfoy briandfoy added Status: released there is a new release with this fix and removed Status: accepted The fix is accepted labels Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Program: grep The grep program Status: released there is a new release with this fix Type: bug an existing feature does not work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants