[petsc-dev] downloadname usage inconsistency in config.package.Package?

[Barry Smith]

   Dmitry,

    Look at BlasLapack.py and MPI.py, though they don't use Package completely correctly they are examples where downloadname is different then package and why downloadname exists.

    Possible changes, eliminate downloadname and put parts of MPI.py into MPICH.py and OpenMPI.py and similar with BlasLapack.py; the reason this hasn't been done is that MPI.py and BlasLapack.py are ugly
beasts and no one has the courage to properly refactor them for fear of breaking lots of stuff. I think this is the right approach but who will volunteer for such an ugly task?


    Barry


On Oct 19, 2010, at 10:43 AM, Dmitry Karpeev wrote:

> On Mon, Oct 18, 2010 at 8:26 PM, Barry Smith <bsmith at mcs.anl.gov> wrote:
>> 
>> On Oct 18, 2010, at 7:46 PM, Dmitry Karpeev wrote:
>> 
>>> In config/BuildSystem/config/package.py we have this:
>>> 
>>> class Package:
>>>  <snip>
>>> def setupHelp(self,help):
>>>    '''Prints help messages for the package'''
>>>    <snip>
>>>    if self.download and not self.download[0] == 'redefine':
>>>      help.addArgument(self.PACKAGE,
>>> '-download-'+self.package+'=<no,yes,filename>',
>>> nargs.ArgDownload(None, 0, 'Download and install '+self.name))
>>>    return
>>> 
>>> Then:
>>> 
>>>  def checkDownload(self, requireDownload = 1):
>>>    '''Check if we should download the package, returning the install
>>> directory or the empty string indicating installation'''
>>>    if not self.download:
>>>      return ''
>>>    downloadPackage = 0
>>>    downloadPackageVal =
>>> self.framework.argDB['download-'+self.downloadname.lower()]
>>>    <snip>
>>> 
>>> An apparent inconsistency if not self.downloadname.tolower() == self.package.
>>> Is this a bug?
>> 
>>   I think it just means the help message would be wrong if downloadname was not package.
> 
> Yes, but let's say self.downloadname.tolower() is not self.package, then
> 1) in setupHelp 'download-'+<package> will be inserted into argDB,
> 2) the user might then say '--download-<package>=yes' on the command line,
> 3) checkDownload will try to extract
> 'download-'+self.downloadname.tolower() from argDB,
>    will not find it and determine that no download is necessary.
> 
> If we assume that we always have self.downloadname == self.package,
> then why not eliminate self.download?
> All versions of the package name, self.package, self.PACKAGE and
> self.downloadname are set in
> config.config.Package.__init__ and are available at the time of the
> setupHelp() call.
> At the very least, since self.downloadname appears always to be used
> in lower case, why
> not ensure that self.downloadname.tolower() == self.downloadname?
> 
> I'm in favor of eliminating self.downloadname in favor of
> self.package, since otherwise the code is
> confusing (at least it tends to confuse me).
> 
> Dmitry.
> 
>> 
>>    The problem I think is that downloadname might not be defined at the time of setupHelp() so cannot be used in printing the help message.
>> 
>> 
>>> Also, why ".tolower()" all over the place?
>> 
>>   So that you only need to type smalls in your -download-xxxx
>> 
>> 
>>> 
>>> Dmitry.
>> 
>>