Hello,
I have a patch for distutils-20000204.tar.gz.
patch for command/build_ext.py:
- use correct name 'libs' instead of 'libraries' in build_ext options
- adjust a String argument in BuildExt.libs to a list
77d76
<
88d86
<
105a104,106
# because fancy_getopts assigns strings if type(self.libs)==StringType: self.libs = [self.libs]235c236 < libraries = build_info.get ('libraries')
libraries = build_info.get ('libs')
The second thing is a suggestion. Bug number 2 occurs on several more places in the distutils code. Everytime the option type is list, the fancy_getopt method stores a string instead. 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.
In the current implementation, subsequent option calls override the old value. If I call
# ./setup.py build_ext --libs=ssl --libs=crypto
the ssl library value gets overridden. 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)
Lets look now at the set_option function. The initial implementation is given in class Command: def set_option (self, option, value): """Set the value of a single option for this command. Raise DistutilsOptionError if 'option' is not known.""" if not hasattr (self, option): raise DistutilsOptionError, \ "command '%s': no such option '%s'" % \ (self.get_command_name(), option) if value is not None: setattr (self, option, value)
This is okay if I am happy with the above mentioned restrictions. To extend this function, you override it in a subclass, for example in the BuildExt class:
# somewhere in the init function is this statement:
self.list_value_options = ["libs", "include_dirs"] # and some more
# now the new set_option
def set_option(self, option, value):
if option in self.list_value_options:
getattr(self, option).append(value)
else:
Command.set_option(self, option, value)
Of course the above changes need some further refinement: you have to initialize the list_value_options with the empty list, but then a check "self.libs is None" will fail. Use "not self.libs" instead. Then you have to ensure that every object we supply in fancy_getopt has this set_option method. And so on.
What do you think of my suggestion?
Greetings,
Bastian Kleineidam
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:
- 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.
- 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.
Hi, some further additions from me: :) > 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. I discovered the Command.finalize_options() function. There is implemented a way to supply a list of values, separated by os.pathsep. if type (self.include_dirs) is StringType: self.include_dirs = string.split (self.include_dirs, os.pathsep)
So you could type "python setup.py build_ext -I/usr/include:/usr/local/include" Well, its not beautiful (espacially because os.pathsep has different values on different platforms), but it works.
:) 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! I try to get distutils to work for my project. With the above os.pathsep thingy, the extension of the fancy_getopt module is not the most important thing for me right now.
What I need more and try to implement are "clean" and "uninstall" commands. The clean command was discussed before as I see from the mailing list archives. The most easy way seems to be to "rm -rf" the build/ directory. The "uninstall" command is a bit more difficult. The Perl guys (and girls) seem to write an uninstall script for each install command.
If you have any ideas to these two commands, let me know them.
Bastian Kleineidam
-
Bastian Kleineidam -
Greg Ward