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

Add an initial implementation of xargs #121

Merged
merged 1 commit into from
Jan 23, 2022
Merged

Conversation

refi64
Copy link
Contributor

@refi64 refi64 commented Dec 27, 2021

This includes much of the core xargs functionality, with the following
notable exceptions:

  • Parallel execution (-P): This option currently just does
    nothing, that way anything that passes -P can at least run without a
    notable behavior shift (other than simply being slower).
  • Replacement strings (-I): This can easily be worked around via an
    intermediate shell invocation (e.g. xargs -L1 sh -c 'do-things-with $@' --).
  • EOF strings (-E): I've honestly never seen this actually used,
    though it would not be particularly difficult to implement given the
    current architecture.

Closes #37

Signed-off-by: Ryan Gonzalez ryan.gonzalez@collabora.com

This has not been tested with Windows CI yet and also depends on #120, so I've marked it as a draft.

@refi64 refi64 force-pushed the xargs branch 2 times, most recently from 6483075 to 49c962f Compare December 27, 2021 19:50
@refi64
Copy link
Contributor Author

refi64 commented Dec 27, 2021

Looks like the failures from #120 were affecting here too, should be working here now as well (also fixed the clippy warnings)

@codecov
Copy link

codecov bot commented Jan 10, 2022

Codecov Report

Merging #121 (3a1c421) into main (8fe924c) will increase coverage by 4.60%.
The diff coverage is 82.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #121      +/-   ##
==========================================
+ Coverage   47.97%   52.58%   +4.60%     
==========================================
  Files          20       23       +3     
  Lines        4106     4650     +544     
  Branches     1431     1529      +98     
==========================================
+ Hits         1970     2445     +475     
- Misses       1650     1660      +10     
- Partials      486      545      +59     
Impacted Files Coverage Δ
src/lib.rs 35.94% <ø> (+0.94%) ⬆️
src/xargs/mod.rs 77.46% <77.46%> (ø)
src/testing/commandline/main.rs 71.42% <78.37%> (+1.15%) ⬆️
src/xargs/main.rs 100.00% <100.00%> (ø)
tests/xargs_tests.rs 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8fe924c...3a1c421. Read the comment docs.

@sylvestre
Copy link
Sponsor Contributor

looks like it is working well
did you try to run the other testsuites?

@refi64 refi64 marked this pull request as ready for review January 10, 2022 14:57
@refi64
Copy link
Contributor Author

refi64 commented Jan 10, 2022

did you try to run the other testsuites?

Well all of them work locally, so 🤞

@sylvestre
Copy link
Sponsor Contributor

@sylvestre
Copy link
Sponsor Contributor

could you please rebase this one too?

This includes much of the core xargs functionality, with the following
notable exceptions:

- Parallel execution (`-P`): This option currently just does
  nothing, that way anything that passes -P can at least run without a
  notable behavior shift (other than simply being slower).
- Replacement strings (`-I`): This can easily be worked around via an
  intermediate shell invocation (e.g. `xargs -L1 sh -c 'do-things-with
  $@' --`).
- EOF strings (`-E`): I've honestly never seen this actually used,
  though it would not be particularly difficult to implement given the
  current architecture.

Closes uutils#37

Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>
@refi64
Copy link
Contributor Author

refi64 commented Jan 23, 2022

Just rebased, the GNU test suite is now running on this xargs so a few new tests will fail (since this doesn't support stuff like -i/-I like I mentioned previously) but hey, that means the test suite is working!

@sylvestre sylvestre merged commit 1e67813 into uutils:main Jan 23, 2022
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.

xargs please!
2 participants