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

tar: fail early for bad options #978

Merged
merged 2 commits into from
Mar 4, 2025
Merged

Conversation

mknos
Copy link
Contributor

@mknos mknos commented Mar 4, 2025

  • Add missing getopts() return value check
  • This version of tar didn't have a usage string, so add one
  • Don't list -c in usage string for now because archive creation isn't implemented
  • Unconditionally use Compress::Zlib because it is a core library
%perl tar -x -f yo -L
Unknown option: L
usage: tar {-tx} [-mvZz] [-f archive] [file ...]

* Add missing getopts() return value check
* This version of tar didn't have a usage string, so add one
* Don't list -c in usage string for now because archive creation isn't implemented
* Unconditionally use Compress::Zlib because it is a core library
@mknos mknos temporarily deployed to housekeeping March 4, 2025 01:49 — with GitHub Actions Inactive
@github-actions github-actions bot added Type: enhancement improve a feature that already exists Priority: low get to this whenever Program: tar The tar program Status: needs verification issue needs to be verified Type: bug an existing feature does not work labels Mar 4, 2025
@mknos mknos temporarily deployed to automated_testing March 4, 2025 01:49 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 01:49 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 01:49 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 01:49 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 01:49 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 01:49 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 01:49 — with GitHub Actions Inactive
@github-actions github-actions bot requested a review from briandfoy March 4, 2025 01:49
@mknos mknos temporarily deployed to automated_testing March 4, 2025 01:50 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 01:50 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 01:50 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 01:50 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 01:50 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 01:50 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 01:50 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 01:50 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 01:50 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 01:50 — with GitHub Actions Inactive
@mknos mknos had a problem deploying to automated_testing March 4, 2025 01:50 — with GitHub Actions Error
@mknos mknos temporarily deployed to automated_testing March 4, 2025 01:50 — with GitHub Actions Inactive
@mknos mknos had a problem deploying to automated_testing March 4, 2025 01:50 — with GitHub Actions Failure
@coveralls
Copy link

coveralls commented Mar 4, 2025

Pull Request Test Coverage Report for Build 13655550118

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 4 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.9%) to 70.088%

Files with Coverage Reduction New Missed Lines %
bin/glob 1 57.14%
bin/units 3 69.91%
Totals Coverage Status
Change from base Build 13632474012: -0.9%
Covered Lines: 396
Relevant Lines: 565

💛 - Coveralls

bin/tar Outdated
@@ -13,34 +13,21 @@ License:

use strict;

use Compress::Zlib qw(gzopen);
Copy link
Owner

Choose a reason for hiding this comment

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

This is a v5.10 module, so we can't rely on it being there. The conditional check should still be there,.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback. New commit added which restores the module-exists check, without conditionally changing the personality of getopts().
As a test I created an archive with GNU tar 1.34, listed the entries with perl tar -tf x.tar, compressed the archive with gzip 1.12 and did a detailed list of compressed archive with perl tar -tvzf x.tar.gz.

@briandfoy briandfoy added the Status: changes requested adjust the pull request as noted in comments label Mar 4, 2025
@briandfoy briandfoy self-assigned this Mar 4, 2025
@mknos mknos temporarily deployed to automated_testing March 4, 2025 14:10 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 14:10 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 14:10 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 14:10 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 14:10 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 14:10 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 14:10 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 14:10 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 14:10 — with GitHub Actions Inactive
@mknos mknos had a problem deploying to automated_testing March 4, 2025 14:10 — with GitHub Actions Error
@mknos mknos temporarily deployed to automated_testing March 4, 2025 14:10 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 14:10 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 14:10 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 14:10 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 14:10 — with GitHub Actions Inactive
@mknos mknos had a problem deploying to automated_testing March 4, 2025 14:10 — with GitHub Actions Failure
@mknos mknos temporarily deployed to automated_testing March 4, 2025 14:10 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 14:10 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 14:10 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing March 4, 2025 14:10 — with GitHub Actions Inactive
@briandfoy briandfoy merged commit b30249e into briandfoy:master Mar 4, 2025
20 of 22 checks passed
@briandfoy briandfoy removed the Status: changes requested adjust the pull request as noted in comments label Mar 4, 2025
@github-actions github-actions bot added Status: accepted The fix is accepted and removed Priority: low get to this whenever labels Mar 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Program: tar The tar program Status: accepted The fix is accepted Type: bug an existing feature does not work Type: enhancement improve a feature that already exists
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants