Skip to content

u-root command: add checkArgs function#2642

Merged
rminnich merged 1 commit intou-root:mainfrom
rminnich:failonemptyfilescommand
Mar 23, 2023
Merged

u-root command: add checkArgs function#2642
rminnich merged 1 commit intou-root:mainfrom
rminnich:failonemptyfilescommand

Conversation

@rminnich
Copy link
Copy Markdown
Member

There are errors which have confusing diagnostics.

A typical one is this
u-root -files which ethtool -files which bash all

if ethtool is not installed, this expands to
u-root -files -files which bash all
and a warning that is very confusing:
Skipping /usr/bin/bash because it is not a directory

This is not even remotely related to the problem.

Still worse, this command will create a badly broken initrd!

This problem has been known to hold up interns for weeks at a time :-)

Add checkArgs, which looks for common problems in os.Args, and is easily expanded. Add a test.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 23, 2023

Codecov Report

Patch coverage has no change and project coverage change: +2.67 🎉

Comparison is base (107f179) 72.25% compared to head (23e0637) 74.93%.

❗ Current head 23e0637 differs from pull request most recent head f45e308. Consider uploading reports for the commit f45e308 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2642      +/-   ##
==========================================
+ Coverage   72.25%   74.93%   +2.67%     
==========================================
  Files         354      413      +59     
  Lines       37332    41661    +4329     
==========================================
+ Hits        26974    31217    +4243     
- Misses      10358    10444      +86     

see 119 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@rminnich rminnich force-pushed the failonemptyfilescommand branch from 0f7fa98 to 23e0637 Compare March 23, 2023 18:29
@rminnich
Copy link
Copy Markdown
Member Author

if anyone can tell me why the lint step is failing, I'd be happy to hear it

@rminnich rminnich force-pushed the failonemptyfilescommand branch from 23e0637 to 134d448 Compare March 23, 2023 19:11
There are errors which have confusing diagnostics.

A typical one is this
u-root -files `which ethtool` -files `which bash` all

if ethtool is not installed, this expands to
u-root -files -files `which bash` all
and a warning that is very confusing:
Skipping /usr/bin/bash because it is not a directory

This is not even remotely related to the problem.

Still worse, this command will create a badly broken initrd!

This problem has been known to hold up interns for weeks at a time :-)

Add checkArgs, which looks for common problems in os.Args,
and is easily expanded. Add a test.

Signed-off-by: Ronald G. Minnich <[email protected]>
@rminnich rminnich force-pushed the failonemptyfilescommand branch from 134d448 to f45e308 Compare March 23, 2023 19:12
@rminnich
Copy link
Copy Markdown
Member Author

ah it's just go versions. bleah.

Copy link
Copy Markdown
Contributor

@rafaelcn rafaelcn left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍🏻

@rminnich rminnich merged commit 5203ef2 into u-root:main Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Awaiting reviewer Waiting for a reviewer.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants