Skip to content

Conversation

@vepadulano
Copy link
Member

Fixes #8850

May still need polishing, will add test if logic is approved.

With this commit, when root receives an unrecognized options it prints an error (similar to other linux commands errors) and exits before showing the prompt.

$ root --nonexistingoption
Error in <TApplication::GetOptions>: unrecognized option '--nonexistingoption'
Try 'root --help' for more information.
$ cp --nonexistingoption
cp: unrecognized option '--nonexistingoption'
Try 'cp --help' for more information.

@phsft-bot
Copy link

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

@phsft-bot
Copy link

Build failed on ROOT-performance-centos8-multicore/default.
Running on olbdw-01.cern.ch:/data/sftnight/workspace/root-pullrequests-build
See console output.

Failing tests:

@phsft-bot
Copy link

Build failed on ROOT-ubuntu16/nortcxxmod.
Running on sft-ubuntu-1604-1.cern.ch:/build/workspace/root-pullrequests-build
See console output.

Failing tests:

@phsft-bot
Copy link

Build failed on ROOT-debian10-i386/cxx14.
Running on pcepsft11.dyndns.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

@phsft-bot
Copy link

@phsft-bot
Copy link

Build failed on mac1014/python3.
Running on macitois21.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

@phsft-bot
Copy link

@phsft-bot
Copy link

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

@vepadulano
Copy link
Member Author

Errors above were due to some options unrecognized by root were actually necessary for other callables in the test. Last commit moves the check to TRint and also now checks if there are multiple unrecognized options issued by the user:

$ root --random -z --nonexistingoption
root: unrecognized option '--random'
root: unrecognized option '-z'
root: unrecognized option '--nonexistingoption'
Try 'root --help' for more information.

Also changed to using std::cerr for simplicity. Before merging we can discuss the correct output stream

@vepadulano vepadulano requested a review from bellenot as a code owner August 31, 2021 09:17
@phsft-bot
Copy link

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

@phsft-bot
Copy link

Build failed on mac11.0/cxx17.
Running on macphsft20.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

Options passed to the `root` command are parsed in
`TApplication::GetOptions(Int_t *argc, char **argv)`. String options in `argv`
are removed from the array of options after parsing. If any string remains in
argv, it means it is not recognized by the `root` command. While these could
still be useful for other `TApplication` instances, in the specific case of
calling `root` which is based on `TRint` it simply means the remaining options
are not valid.

This commit introduces an early exit logic in the constructor of `TRint`. If
there are any remaining unrecognized options as described above, they will be
printed to `std::cerr` and then ROOT will terminate execution, similar to other
linux commands:

```
$: root -z --nonexistingoption
root: unrecognized option '-z'
root: unrecognized option '--nonexistingoption'
Try 'root --help' for more information.
```
@phsft-bot
Copy link

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

@phsft-bot
Copy link

Build failed on ROOT-performance-centos8-multicore/default.
Running on olbdw-01.cern.ch:/data/sftnight/workspace/root-pullrequests-build
See console output.

Errors:

Failing tests:

@phsft-bot
Copy link

Build failed on mac11.0/cxx17.
Running on macphsft20.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Errors:

Failing tests:

@vepadulano
Copy link
Member Author

@eguiraud @Axel-Naumann if you're ok with this PR it is ready to be merged 👍

Copy link
Contributor

@eguiraud eguiraud left a comment

Choose a reason for hiding this comment

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

LGTM although i'm not the code owner 😄

@phsft-bot
Copy link

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos8-multicore/default, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac11.0/cxx17, windows10/cxx14
How to customize builds

@phsft-bot
Copy link

Build failed on mac11.0/cxx17.
Running on macphsft23.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

@phsft-bot
Copy link

Build failed on windows10/cxx14.
Running on null:C:\build\workspace\root-pullrequests-build
See console output.

Failing tests:

@vepadulano vepadulano merged commit 0e0154b into root-project:master Sep 7, 2021
@vepadulano vepadulano deleted the root-unrec-opt branch October 25, 2021 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

root command ignores unknown options - it should complain instead

5 participants