[Distutils] patch for build_ext and a suggestion

[Greg Ward]

On 12 March 2000, Bastian Kleineidam said:
> I have a patch for distutils-20000204.tar.gz.

The code has changed a lot in the last month.  I've just changed the
Distutils "Implementation status" web page to discourage people from
using the Feb 4 snapshot:

    Update! (Mar 12, 2000) No new code snapshot is available, but the
    code ha changed a lot since the last one (Feb 4). If you want to
    keep on top of changes since 0.1.3, and especially if you want to
    contribute patches, please follow the CVS tree.

So, if you're interested in following the code, you're better off
following the CVS tree.  If you want to submit a patch, *please* follow
the CVS tree and make the patch relative to a fairly recent update.
Thanks.

Oh yeah: please *always* submit either context (diff -c) or unified
(diff -u) diffs.  Absolute diffs are obsolete as soon as I add or remove
one line from the file.

> patch for command/build_ext.py:
> 1. use correct name 'libs' instead of 'libraries' in build_ext options

Hmmm, I'm not sure which is correct here -- I've been inconsistent,
using "libs" as the command option and "libraries" as the build info
dictionary key.  I prefer "libraries" myself, so I'll probably fix it
that way.  Thanks for spotting this.

> 2. adjust a String argument in BuildExt.libs to a list

Good catch -- and I think your analysis of this error is pretty good.

> So there are two restrictions for options:
> 1) An option is allowed only once and 
> 2) an option value type has to be a string.
> Both restrictions are not necessary. What I want to have is that if I 
> supply mulitple --libs options, each option value gets added to a list.

Right, I made a definite choice that fancy_getopt would *not* be an
all-singing, all-dancing Python command-line parser with typed,
aggregate options and careful type-checking.  Every time I set about
writing one of those, it takes ages to get it right and I go way
overboard.  (IOW, I've never met a type system I didn't like so much
that I immediately set about reinventing it; so far, I've managed to
restrain myself from reinventing Python's type system in the Distutils.)

> Here is the code snippet from fancy_getopt.py thats responsible:
>         attr = attr_name[opt]
>         if takes_arg[opt]:
>             setattr (object, attr, val)
> 
> Solution: instead of setattr, you should call the "set_option" function
> of the object with "val" as argument.
> So in the example above, this is
>         attr = attr_name[opt]
>         if takes_arg[opt]:
>             object.set_option(attr, val)

Interesting idea.  It sort of violates one of the design principles
behind fancy_getopt, which was that you could pass in *any* object
(well, any object that setattr works on, namely any class instance), not 
just Distutils command objects.

However, the more I think about it, the more I think your proposal just
bends the rules rather than breaking them.  First, we could say, "call
set_option if the object has it, otherwise setattr".  Second, we
document the use of set_option, and make this the way to impose a type
system on the Distutils command line.

I like this.  That leaves decisions about how strongly to type, whether
to support aggregate types, etc. etc. up to the application that uses
fancy_getopt (Distutils in this case) -- and lets fancy_getopt stay
relatively simple, as it currently is.

I think the right way to do this would be something like this:

  * tweak fancy_getopt to use 'set_option()' if it's there, setattr
    otherwise
  * add some type annotations to each command class' 'user_options'
    dictionary (eg. a fourth element of each tuple)
  * tweak fancy_getopt so it doesn't barf on those type annotations
    -- it might already accept > 3 element tuples, so this might
    not be necessary
  * change the 'set_option' method provided by Command (the base class)
    to pay attention to these type annotations

If you're interested in hacking on this, be my guest: sounds like a
useful addition.  But please do it on the current CVS tree!

Thanks!

        Greg
-- 
Greg Ward - Unix bigot                                  gward@python.net
http://starship.python.net/~gward/
No man is an island, but some of us are long peninsulas.