After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 136953 - Can we have support for popt?
Can we have support for popt?
Status: RESOLVED FIXED
Product: gnome-python
Classification: Deprecated
Component: gnome
2.1.x
Other All
: Normal normal
: ---
Assigned To: rubensr
Python bindings maintainers
Depends on:
Blocks:
 
 
Reported: 2004-03-12 02:36 UTC by rubensr
Modified: 2004-12-22 21:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to add popt support (8.72 KB, patch)
2004-03-28 10:24 UTC, rubensr
none Details | Review
Patch to add popt support (text only now) (38.19 KB, patch)
2004-03-28 10:26 UTC, rubensr
none Details | Review
Patch now using pypopt. (45.02 KB, patch)
2004-03-31 10:04 UTC, rubensr
none Details | Review
Yet another popt patch, for 2.0.2. (20.03 KB, patch)
2004-04-08 11:46 UTC, rubensr
none Details | Review
And now - a bit tab/space problem cleaner and with sigaction (19.83 KB, patch)
2004-04-09 01:25 UTC, rubensr
none Details | Review
Final patch that went into CVS (23.57 KB, patch)
2004-04-09 17:14 UTC, Gustavo Carneiro
none Details | Review

Description rubensr 2004-03-12 02:36:03 UTC
Hi guys,

 In order to be able to write applications that can be gnome-session
compliant, it would be nice to have support for popt...

Thanks!
Comment 1 Gustavo Carneiro 2004-03-20 13:07:16 UTC
  I hear you!  I've been wanting this for a long time too.  I've been
thinking, perhaps we can use/integrate with pypopt: 
        http://www.agrajag.net/projects/pypopt/
  Unfortunately, I don't have enough time at the moment to effectively
tackle this problem.
Comment 2 rubensr 2004-03-21 03:37:43 UTC
Yes, understand - well, here is my proposal:
I could have a look at pypopt and see if I can 
start the integration, and then send patches to the responsible
pygtk person, how's that sound?
But I do have to be honest as well:
1. I may end up not having enough time to do it (but I am
   willing to try it)
2. I am stuck with Redhat 8.0 and cannot upgrade, so my patches 
   would be based on gnome-python2-1.99.11-8;
So, if no one else has the bandwidth to do this, and if you 
think I could help, then I'll try.
If this is ok, there is also the possibility of using 
python-popt (was active until redhat-7.3 I think). What do
you think?

Cheers
Rubens
Comment 3 Gustavo Carneiro 2004-03-21 11:41:59 UTC
  Well, if you proceed, just attach patches to this bug report, and
we'll take a look.  Patches against gnome-python2-1.99.11-8 should be ok.
  Regarding python-popt, I can't find it anywhere (homepage, source
tarball) on google or freshmeat, only RPMs.  Therefore, I'm a bit
reluctant to use this code.
  Finally, I would acknowledge that we have two approaches for this:
    1. Add a pypopt/python-popt dependency to gnome-python;
    2. Copy and distribute the pypopt/python-popt code inside
gnome-python.
  I'm still not sure which is the best solution.  Can anyone else comment?
Comment 4 Johan (not receiving bugmail) Dahlin 2004-03-21 11:48:06 UTC
I know that pypopt is used heavily by Red Hat, so maybe you have to
look in their packages, probably anaconda.

And since it's not widely distributed yet we might just be able to
included it in the gnome-python distribution since if I remember
correctly it's not a huge module. It also makes sense if it's not
maintained or easily accessible

Anyway, I like the idea to have good popt integration in gnome-python.
Comment 5 rubensr 2004-03-21 21:46:38 UTC
Fair enough - I'll have a look at pypopt and anaconda,
and will try something along the lines of Gustavo's option (2).
I'll pop in #pygtk in case I need some directions :)
Comment 6 Christian Reis (not reading bugmail) 2004-03-25 13:29:43 UTC
Heya Rubens,

Just a remark on your reservation #2: you should always code patches against
gnome-python CVS HEAD. You can install it into a local (~rubens/local) directory
and test it using a specified PYTHONPATH without touching your local installation.  

Looking forward to working with you again!
Comment 7 rubensr 2004-03-28 10:23:39 UTC
Ok - so after getting lots of help from jdahlin and kiko (thanks guys :)
I am posting an initial patch here, which adds support for popt (standalone),
popt (with gnome) and (hopefully) GnomeModuleInfo.
I added some notes to the patch, so these may clarify/explain the decisions
I had to take.
This is a patch against gnome-python-2.0.1-RC2.
Comment 8 rubensr 2004-03-28 10:24:59 UTC
Created attachment 26018 [details] [review]
Patch to add popt support

Now the patch :)
Comment 9 rubensr 2004-03-28 10:26:50 UTC
Created attachment 26019 [details] [review]
Patch to add popt support (text only now)
Comment 10 Gustavo Carneiro 2004-03-28 16:06:37 UTC
Ok, I tested it and it seems to work.  Some comments:

  1. You ask this in the readme file; I really think gnome-python's popt should
be placed in the gnome namespace, i.e. gnome.popt.

  2. This doesn't feel very elegant to me:
    if (rc > 95) and (rc < 100):
        arg = ctx.getOptArg()
        print "rc: %s; valuetype: %s; %s" % (rc, type(arg), arg)
It forces you to use a number range to identify the options.  I wish you had
chosen to integrate pypopt instead.  With pyopt you can do this instead:
(...)
ctx = pypopt.getcontext("testpopt", sys.argv, options, 1, 0)
options.no_error_detection = ctx.argdict['no-error-detection']
options.disable_delays     = ctx.argdict['disable-delays']
options.interactive        = ctx.argdict['interactive']
options.gui                = ctx.argdict['enable-gui']
options.sloppy             = ctx.argdict['sloppy']
options.max_rate_packets   = ctx.argdict['max-rate-packets']
(...)

  3. ctx = p._get_popt_context()
     Names starting with underscore are supposed to be private, but
_get_popt_context is clearly public.  We should remove the leading underscore,
if you don't mind.

  4. Another pyopt advantage is that it automatically parses the argument string
values to int or float as appropriate, looking at the type indicated in the
options table.  This code always presents argument values as strings.

That's all I can think of.  Other than this, nice work! ;-)
Comment 11 Christian Reis (not reading bugmail) 2004-03-28 20:09:04 UTC
>  3. ctx = p._get_popt_context()
>      Names starting with underscore are supposed to be private, but
> _get_popt_context is clearly public.  We should remove the leading underscore,
> if you don't mind.

Well, I suggested he used an underscore for a reason: avoiding potential clashes
with future GnomeProgram API (given that it's an API contrived in gnome-python
that doesn't match a GnomeProgram call). Do you have a suggested alternative?
get_popt_context_() ? Should we investigate the likelihood of a crash?

> I wish you had chosen to integrate pypopt instead.

Hmmm -- is that not an option, Rubens, Gustavo?

http://www.agrajag.net/projects/pypopt/

Comment 12 Gustavo Carneiro 2004-03-28 21:00:30 UTC
Well, just ask yourself this:  How likely is it that a function called
'gnome_program_get_popt_context' would be added that _doesn't_ do exactly the
same thing as our own '_get_popt_context'?
In case the answer is 'not likely at all', then we have no problem, because if
gnome_program_get_popt_context is added to the C library, it does exactly the
same thing as our own, therefore shadowing the C library function doesn't matter.
Comment 13 Christian Reis (not reading bugmail) 2004-03-28 22:11:48 UTC
Hmmm, I don't have such high expectations for sanity in the GnomeProgram API,
but I suspect we should just trust you on this and get on with life.
Comment 14 rubensr 2004-03-28 22:36:28 UTC
About gjc comments:

1. Ok - not a problem, I will do this.
2. Agreed - shouldnt be hard to integrate pypopt - just that the code
   itself seemed to be more convoluted to me, but thats ok. I will do this.
3. This one is easy ;-)
4. There is a method in python-popt to do this as well, if using pypopt
   doesnt work, I will reuse that.

Will post the patch when I am finished.
Comment 15 rubensr 2004-03-29 11:51:07 UTC
Ok - couldnt find you in #pygtk, so here are my new comments:

1. Did this, and just realised that doing "import gnome.popt" may be a
   bit weird if you just want to use popt without the gnome stuff (which
   I guess is unlikely, but anyway).
3. Did this.

2 and 4: Now I remember why I decided to not use the "getValue" method from
   python-popt, and why using the funky features of pypopt could cause problems:

   Both python-popt (I had to change this when integrating) and pypopt
   create a poptContext object which hold some additional data to just
   the context id in order to provide the features you like.
   The problem is, these niceties are initialised when you call
   popt_get_context(), which is NOT called by the application when you
   are using gnome! All you do is grab the poptContext id once gnome is
   finished with the parameters, and then reuse it. But then, all of the
   internal PypoptContext object members are not initialised.

   Soooooo - if you really want to have all of these nice features,
   all I can think of is that the user would not be able to call
   gnome.program.get_popt_context() (I would remove it), and he/she
   would have to do the (kind-of) nonsense call to popt.get_context() with
   command line parameters and the popt table all over again once
   gnome.init() is finished.

   If you are happy with this, then we can have all of the features from
   these libraries, no problemo - unless you can think of a different 
   solution of course :)

Cheers
Comment 16 rubensr 2004-03-31 10:04:17 UTC
Created attachment 26143 [details] [review]
Patch now using pypopt.

Here are the changes.
Comment 17 Christian Reis (not reading bugmail) 2004-04-02 00:43:08 UTC
Usability? What does this have to do with usability? :-)
Comment 18 Gustavo Carneiro 2004-04-03 13:07:42 UTC
Just avoid conflicts/race conditions, I'll be reviewing/debugging this code this
afternoon...
Comment 19 Gustavo Carneiro 2004-04-03 13:44:53 UTC
Instead of pypopt.error_noarg, pypopt.error_badopt, etc., we should have
pypopt.NoArgError, pypopt.BadOptError, etc.  In addition, module constants
(popt.arg_int, etc.) should be in ALL_CAPS style.  Does everyone agree?
Comment 20 Johan (not receiving bugmail) Dahlin 2004-04-03 13:49:11 UTC
Yes, and the constants should just point to the builtins str, int, double and None.
Comment 21 Gustavo Carneiro 2004-04-03 14:13:40 UTC
The problem is, I'm not sure how to handle POP_ARG_VAL in that case.  From popt(3):
       "POPT_ARG_VAL causes arg to be set to the  (integer)  value
       of  val  when  the  argument is found.  This is most often
       useful for mutually&#8208;exclusive arguments in cases where  it
       is  not an error for multiple arguments to occur and where
       you want the last argument specified to win; for  example,
       "rm  &#8208;i &#8208;f".  POPT_ARG_VAL causes the parsing function not
       to return a value, since the value of val has already been
       used."
Comment 22 Johan (not receiving bugmail) Dahlin 2004-04-03 14:42:00 UTC
Okay, let's do it the other way around then, so str, int, double and None maps
to POPT_*
Comment 23 Gustavo Carneiro 2004-04-03 15:38:45 UTC
I'm worried about two issues: 1. the first string option bug already mentioned,
but still can't find the bug; 2. passing standard gnome options makes parsing
application options fail; example: python gnome_popt.py --enable-sound --aaa asd
Comment 24 Gustavo Carneiro 2004-04-03 15:50:27 UTC
Found this comment in libgnome:
    /* Make a copy of argv, the thing is that while we know the
     * original argv will live until the end of the program,
     * there are those evil people out there that modify it.
     * Also, this may be some other argv, think 'fake argv' here */
This means that libgnome doesn't modify argv to remove the arguments that it
processes.  Which probably means we have a dead end. :|
I'm beginning to think this is an impossible task, or at least so "hackish" that
is not worth the trouble...
Comment 25 rubensr 2004-04-04 04:02:12 UTC
What about trying to use the callback functionality from popt?  
  
We could have a dummy callback implemented in poptmodule.c,   
that would call a python callback in the python popt options table...  
 
In this way, the poptOptions table would only allow callbacks... 
 
Does that sound feasible (or too hackish)?  
  
Comment 26 Gustavo Carneiro 2004-04-04 09:35:20 UTC
Hm.. I did not know of this callback functionality.  It is probably a good idea!...
Comment 27 Gustavo Carneiro 2004-04-04 11:45:13 UTC
OK, looking at the man page, the callback functionality is exactly what we need!
See this: 
"This is especially usefull when included  option tables  are  being used, as
the program which provides the top&#8208;level option table doesn&#8217;t need to  be  aware
 of  the other  options  which  are provided by the included table."

I'm also thinking we could use a different approach.  The general idea is that
we don't need a full-blown popt module, we just need "popt-light", we minimal
functionality.  Let me explain what I'd like to have with an example:
----BEGIN EXAMPLE----
#     (longName, shortName, type , default, flags, descrip , argDescrip)
table=[("foo"  , 'f'      , int  , 1      , 0    , "Foobar", "Level"),
       ("aaa"  , 'a'      , str  , 'a'    , 0    , 'aaa'   , "AAA"),
       ("bbb"  , 'b'      , None , None   , 0    , 'bbb'   , "BBB"),
       ("ccc"  , 'c'      , float, 2.5    , 0    , 'ccc'   , "BBB")]
prog = gnome.init ("myprog", "1.0", gnome.libgnome_module_info_get(),
                   sys.argv, table)
leftover_args, argdict = prog.get_popt_args()
----END EXAMPLE----

Now, this is what I'd call elegant.  Moreover, it would be a lot more
maintainable, because I'm thinking we don't need to integrate the
pypopt/popt-python module at all.  We only need to be able to create a popt
option table to pass into gnome_program_init.  The POPT_ARG_XXX values are not
needed, we can just use python types (int, str, float, bool, etc.).  I think we
should sacrifice POP_ARG_VAL.  We do need the POPT_ARGFLAG_XXX constants;  these
can be defined directly in the gnome module, preserving the POPT_ prefix.

Some hints on how to perform all this:
  1- The option dictionary (argdict) is created first;
  2- The table is constructed, all elements are of type POPT_ARG_CALLBACK,
callback is our C static func callback, descrip is the callback data, which
points to a structure containting (optiontype, argdict); While the table is
constructed, the argdict is initialized to the default values;
  3- We call gnome_program_init with the table.  Then callback is called for
each option:
    a) Callback parses option value, and stores result in argdict;
  4- Before returning from the gnome_progam_init wrapper, we store a reference
to the argdict in the GnomeProgram object, with g_object_set_data;
  5. New method get_popt_args() returns (argdic, leftoverargs) this way:
     a) argdic is obtained with g_object_get_data from the GnomeProgram;
     b) we can obtain the popt context from GnomeProgram, and from the
poptContext we obtain the leftoverargs

Rubens, are you still up for this?  I could do it too, but it will happen much
faster if you do it instead of me. :)
Comment 28 rubensr 2004-04-04 13:00:34 UTC
I like your example - let's try that, it is (theoretically) cleaner  
and simpler than having gnome.popt.  
(I was playing with popt in C - the callback has to be defined once in the  
table, everything after that row is handled via the callback)  
Just some clarifications: 
On point 1: so do you want to have keys in the dict for all options, even 
  for the ones that are not passed in the command line and have no defaults? 
On point 2: already commented on this :) 
On point 3: excellent! 
On points 4 and 5: The argdict could still be stored in a "poptContext" 
            python object, what do you think? This could remove the need 
            for the g_object_get_data()? Because of 5.b we would still need 
            a "99% fat free gnome.popt" any way, isnt it? (or shall we 
            call it the mini-me.popt) 
 
Anyway, I dont mind doing this. Can take a few days, if you're ok with this. 
 
Comment 29 rubensr 2004-04-08 11:46:15 UTC
Created attachment 26465 [details] [review]
Yet another popt patch, for 2.0.2.

So here is the new patch for popt - a couple of remarks:
* please someone check that the Python reference decs/incs are correct,
  as I am not that experienced with that;
* There is a known leak there - it is marked in the code, argv cannot
  be deleted after gnome_program_init() is called, as it is used
  by get_popt_args().
Comment 30 Christian Reis (not reading bugmail) 2004-04-08 12:56:33 UTC
Rubens, indentation in that patch looks funky; are you mixing tabs and spaces?
It appears the relevant file uses 4-space indents in the area you're editing
(and yes, mixed tabs and spaces are horrific).
Comment 31 rubensr 2004-04-09 01:25:19 UTC
Created attachment 26492 [details] [review]
And now - a bit tab/space problem cleaner and with sigaction

One interesting note - I managed to get hold of a Fedora Core 1 distro to test
this, and 2.0.2 did not build after './configure', 'make'. It stopped
in the bonobo directory asking me to run 'aclocal; autoconf; automake'...
Comment 32 Gustavo Carneiro 2004-04-09 17:14:28 UTC
Created attachment 26535 [details] [review]
Final patch that went into CVS

Committed to CVS HEAD, with some code simplification, and memory leak fixes.
I am a bit afraid to commit to the 2.0 branch, because that is a stable branch,
and only bug fixes should be committed.
Perhaps someday we'll backport, after extensive testing in HEAD.
Comment 33 Christian Reis (not reading bugmail) 2004-04-09 18:32:41 UTC
Rubens, give it some testing and if you're happy, resolve as FIXED. What's your
next bug? :-)

Thanks to everybody that chipped in to make this happen.
Comment 34 Gustavo Carneiro 2004-04-10 18:24:38 UTC
I found a bug, most likely introduced by my code simplifications :-P  It
segfaults when no popt table is given.  The following command shows the fix:
  cvs diff -r 1.9 -r 1.10 gnome/gnome.override
Comment 35 Christian Reis (not reading bugmail) 2004-04-13 13:23:27 UTC
Looks fixed to me. File new bugs for regressions and other missing bits.
Comment 36 Gustavo Carneiro 2004-04-13 21:43:00 UTC
Actually, let me reopen with a reminder that I need to implement standalone popt
parsing, not integrated with (independent of) gnome.init().
  I have this strange use case involving the new bonobo.Application API, where
sometimes a program is required to parse popt options multiple times, but you
can only gnome.init() once.
  This should be fairly trivial to implement.  All code is already there; I just
need to reuse the existing functions.
Comment 37 Christian Reis (not reading bugmail) 2004-04-14 20:20:29 UTC
How about opening a new bug for that? Better than leaving bugs with code checked
in dangling..
Comment 38 Gustavo Carneiro 2004-04-17 17:41:19 UTC
Added to CVS such function:

leftover_args, argdict = gnome.popt_parse(argv, popt_table, flags=0)

Closing. I hope everyone agrees with the name, otherwise I can change it.
Comment 39 Christian Reis (not reading bugmail) 2004-05-28 01:08:13 UTC
Unless there's risk that it collides with a "real" popt_parse method coming from
C GNOME popt itself, I think that's fine.
Comment 40 Gustavo Carneiro 2004-06-20 16:45:42 UTC
The email in pygtk list about parsing repeated options makes me think we can
easily support this.  Also, looking at the code, I discovered a bug.
    switch (opt->argInfo & POPT_ARG_MASK) {
    case POPT_ARG_INT:
        item = PyInt_FromLong(*(int *)(opt->arg));
        break;
(...)
  If the same option is given more than once, we leak a python object.  We could
fix this bug and support multiple options at the same time if we automatically
append multiple option values to a list, and return the list instead of a single
value.  We could invent a new flag that indicates whether we accept multiple
values for an option, and if so we always return a list even when there is only
one value.  This simplifies the application programming downstream, I think..
Comment 41 Gustavo Carneiro 2004-06-24 17:30:11 UTC
Oops, sorry there is no leak after all :-P
Comment 42 Gustavo Carneiro 2004-06-24 18:34:18 UTC
Committed to CVS multiple value per option support.  I ended up doing a bit
differently.  I couldn't find any "safe" flag value: there's always some risk
that popt might decide to use such value in the future.
    For details on the code, see:
http://cvs.gnome.org/viewcvs/gnome-python/gnome-python/gnome/gnome.override?r1=1.13&r2=1.14
http://cvs.gnome.org/viewcvs/gnome-python/gnome-python/examples/popt/popt.py?r1=1.2&r2=1.3