forgot to add the o/p from the modified pkgsend :

v...@opensolaris:~# export PKG_REPO=http://localhost:11000
v...@opensolaris:~# eval `pkgsend open [email protected]`
v...@opensolaris:~# pkgsend generate hello/file0.tar
pkgsend: generate: 'hello/file0.tar' file not found
v...@opensolaris:~# pkgsend close
PUBLISHED
pkg://opensolaris.org/[email protected],5.11:20100428T062325Z

Saurabh Vyas wrote:
Thanks Brock,

I have modified the trans_generate() to call error function.

New webrev : http://cr.opensolaris.org/~saurabhv/fix-15646-rev-2/

Also I have added the test case, but I am not too sure that I have written the test case correctly, so please advice me if I need to write that in diff way (I have used test case similar to 'pkgsend add file' test case with imaginary file)

Thanks,
~Saurabh

Brock Pytlik wrote:
On 04/23/10 02:22 AM, Saurabh Vyas wrote:
Tim Foster wrote:
Hi Saurabh,

On Thu, 2010-04-22 at 16:06 +0530, Saurabh Vyas wrote:
Bug-id : pkgsend generate traceback when bundle doesn't exist
webrev : http://cr.opensolaris.org/~saurabhv/fix-15646/

Rather than just printing an error message inside gen_actions() and
continuing, you should find a way to have trans_generate() detect the
error and return a non-zero exit code back to main_func()

I'd suggest raising an exception from gen_actions for this case,
handling it in trans_generate(), printing the exception message, then
returning a non-zero exit code.
Thanks for the inputs Tim.

So now I am raising pkg.actions.ActionDataError from gen_actions() and handling this in trans_generate() (with non zero exit) Is pkg.actions.ActionDataError right error to use or should another error type needs to be defined ?

Just for ref : http://cr.opensolaris.org/~saurabhv/fix-15646-rev-1/


I think it's probably worth using the error function on line 473 instead of just printing the error, see line 445 for an example. It'd be nice if you could attach an example of what the output looks like, though depending on how you write your test, that could be sufficient.

Question : Can't we directly handle this error in gen_actions() itself? (as we know that we cannot recover from this error & should quit with zero zero code).

We try to keep the user interaction bits of code all confined to one layer. For example, suppose we decided to create a GUI version of pkgsend. We'd want it to use gen_actions so that we didn't duplicate code. However, having gen_actions print something to a terminal (which might not exist) then call sys.exit(1) wouldn't allow a particularly useful interface to the user.

Brock


You also need to include a test case for this bug
in ./src/tests/cli/t_pkgsend.py
I will add the test case and send the final webrev soon.

Thanks,
~Saurabh
    cheers,
            tim



_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to