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

dd: implement iseek + oseek flags #3256

Merged
merged 3 commits into from
Mar 22, 2022
Merged

Conversation

chordtoll
Copy link
Contributor

These are the first half of changes needed to pass the dd/bytes.sh tests:

  • Add iseek and oseek options (additive with skip and seek options)
  • Implement tests for the new flags, matching those from dd/bytes.sh

@chordtoll
Copy link
Contributor Author

--iseek and --oseek are fairly new dd features, added in coreutils commit 4439ef3ec4 this February.

@sylvestre sylvestre requested a review from jfinkels March 14, 2022 09:01
Copy link
Collaborator

@jfinkels jfinkels left a comment

Choose a reason for hiding this comment

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

The code itself is fine, I'm requesting some changes to the tests and eliminating some duplicate code.

src/uu/dd/src/parseargs.rs Outdated Show resolved Hide resolved
tests/by-util/test_dd.rs Outdated Show resolved Hide resolved
tests/by-util/test_dd.rs Outdated Show resolved Hide resolved
tests/by-util/test_dd.rs Show resolved Hide resolved
tests/by-util/test_dd.rs Show resolved Hide resolved
tests/by-util/test_dd.rs Show resolved Hide resolved
tests/by-util/test_dd.rs Show resolved Hide resolved
@chordtoll chordtoll requested a review from jfinkels March 18, 2022 01:40
These are the first half of changes needed to pass the dd/bytes.sh tests:
- Add iseek and oseek options (additive with skip and seek options)
- Implement tests for the new flags, matching those from dd/bytes.sh
@chordtoll
Copy link
Contributor Author

I'm a bit confused by the failing GnuTests CI run- tests/misc/seq-long-double test passes both in a local build, and in GitHub CI in my fork. Is it possible the failure is an intermittent issue?

@sylvestre
Copy link
Contributor

yeah, likely not your fault :)

@jfinkels
Copy link
Collaborator

I'm a bit confused by the failing GnuTests CI run- tests/misc/seq-long-double test passes both in a local build, and in GitHub CI in my fork. Is it possible the failure is an intermittent issue?

Yes, it is an intermittent issue #2924

@sylvestre sylvestre merged commit 291b889 into uutils:main Mar 22, 2022
@chordtoll chordtoll deleted the iseek-oseek branch April 4, 2022 21:25
@tertsdiepraam
Copy link
Member

Hi! I'm doing a refactor of the argument handling of dd and ran into something I didn't quite understand. In this PR, you added tests to check that iseek and oseek are added on skip and seek, but I cannot reproduce that behaviour. Could you explain in what case they should be added?

This is with dd from GNU coreutils 9.1 and the current main branch:

❯ echo "0123456789abcdefghijklm" | dd iseek=4 skip=4 iflag=skip_bytes bs=2
456789abcdefghijklm
10+0 records in
10+0 records out
20 bytes copied, 0,000209508 s, 95,5 kB/s
❯ echo "0123456789abcdefghijklm" | cargo run --quiet -- dd iseek=4 skip=4 iflag=skip_bytes bs=2
89abcdefghijklm
8+0 records in
8+0 records out
16 bytes copied, 0.0 s, 16.0 KB/s

@chordtoll
Copy link
Contributor Author

I likewise haven't been able to reproduce the additive behavior- I probably misinterpreted the results of my tests. I've checked out the pull request, and the behavior looks to be correct now.

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.

4 participants