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

Unfriendly error message printed when user just intend to check usage #548

Closed
yunchih opened this Issue Nov 14, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@yunchih

yunchih commented Nov 14, 2017

Run poudriere first time without root will produce:

[yunchih@FreeBSD ~]$ poudriere image -h
mkdir: /usr/local/poudriere: Permission denied
/usr/local/share/poudriere/common.sh: parallel_shutdown: not found

Obviously, poudriere needs privilege to create that directory, but the above parallel_shutdown: not found error might be confusing for new users. Why not source the necessary files in exit_handler:

diff --git a/src/share/poudriere/common.sh b/src/share/poudriere/common.sh
index f2df1da..006f45f 100755
--- a/src/share/poudriere/common.sh
+++ b/src/share/poudriere/common.sh
@@ -1091,6 +1091,8 @@ exit_handler() {
        # Ignore SIGINT while cleaning up
        trap '' SIGINT
 
+    . ${SCRIPTPREFIX}/include/parallel.sh
+
        if was_a_bulk_run; then
                log_stop
                # build_queue may have done cd MASTERMNT/.p/pool,

Interestingly, the ports.sh script can run all the way to printing "usage" before executing exit handler, while the image.sh script jump directly to exit handler before it can source include/*.sh.
poudriere image -h: http://ix.io/Cfl
poudriere ports -h: http://ix.io/Cfm

In addition, maybe it is better to run . ${SCRIPTPREFIX}/common.sh after getopt, so user can avoid running common.sh if they just want to check usage?

@bdrewery

This comment has been minimized.

Show comment
Hide comment
@bdrewery

bdrewery Nov 14, 2017

Member

The mkdir isn't even needed anymore, it's a leftover from other refactoring. Will remove it.

Member

bdrewery commented Nov 14, 2017

The mkdir isn't even needed anymore, it's a leftover from other refactoring. Will remove it.

@bdrewery bdrewery added this to the 3.2.1 milestone Nov 14, 2017

@bdrewery bdrewery closed this in a3916a9 Nov 14, 2017

@bdrewery

This comment has been minimized.

Show comment
Hide comment
@bdrewery

bdrewery Nov 14, 2017

Member

In addition, maybe it is better to run . ${SCRIPTPREFIX}/common.sh after getopt, so user can avoid running common.sh if they just want to check usage?

The getopts line may depend on defaults set by common.sh so it must come after.

The difference with ports vs image is that image runs with set -e and exits immediately on the mkdir error.

Member

bdrewery commented Nov 14, 2017

In addition, maybe it is better to run . ${SCRIPTPREFIX}/common.sh after getopt, so user can avoid running common.sh if they just want to check usage?

The getopts line may depend on defaults set by common.sh so it must come after.

The difference with ports vs image is that image runs with set -e and exits immediately on the mkdir error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment