API review

Proposer: Tully Foote

Present at review:

• List reviewers
  • kwc
  • Brian
  • Josh
  • Tully

Question / concerns / comments

kwc

• We need to clarify if we are using "version" or "codename"

• Debian should extend a base API class for clarify

• OSAbstraction

  • OSAbstraction is a little odd. How about OSDetect?. Same goes for OSAbstractionException

  • Not sure I understand the Override() example

  • get_os_specific_class should probably be renamed to whatever the base class of Debian becomes

• determine_buildability
  • the contract is not clear to me from the brief snippet. Does it return True/False? Return some enumerated state? Personally, I would just want to call:

    roslib.packages.builds_on('rospy', 'ubuntu', '9.10')
    roslib.packages.builds_on_current('rospy')

    • The reason I have separated the two calls instead of using the keyword style is that os/version are not independently specifiable, i.e. they don't have independent default values.

Adding roslib submodule roslib.os_detect

Example Class:

   1 class Debian:
   2     def check_presence(self):
   3         if "Debian" == lsb_get_os():
   4             return True
   5         return False
   6 
   7     def get_version(self):
   8         return lsb_get_codename()
   9     def get_name(self):
  10         return "debian"

OSAbstraction Class:

   1 class OSAbstraction:
   2     """ This class will iterate over registered classes to lookup the
   3     active OS and Version of that OS for lookup in rosdep.yaml"""
   4     def __init__(self, os_list = [Debian(), Ubuntu(), Mint(), Macports(), Arch(), Fedora(), Rhel()]):
   5         self._os_list = [ Override()]
   6         self._os_list.extend(os_list)
   7 
   8         self.detect_os()
   9 
  10     def add_os(self, class_ref):
  11         self._os_list.append(class_ref)
  12 
  13         # \TODO look at throwing here
  14     def detect_os(self):
  15         for os_class in self._os_list:
  16             if os_class.check_presence():
  17                 self._os_name = os_class.get_name()
  18                 self._os_version = os_class.get_version()
  19                 self.os_class = os_class
  20                 return True
  21         return False
  22 
  23     def get_os_specific_class(self):
  24         if not self.os_class:
  25             if not self.detect_os():
  26                 raise OSAbstractionException("No OS detected")
  27         else:
  28             return self._os_class
  29 
  30     def get_name(self):
  31         if not self._os_name:
  32             os_class = self.get_os()
  33             if os_class:
  34                 return os_class.get_name()
  35             else:
  36                 return False
  37         return self._os_name
  38 
  39     def get_version(self):
  40         if not self._os_version:
  41             if self.os_class:
  42                 self._os_version = self.os_class.get_version()
  43         return self._os_version

determine buildability (OS specific)

This will obey the whitelist(OS dependent) if requested, otherwise it will simply check for the blacklist(OS independent)

method:

roslib.packages.packages.do_not_build(package, use_whitelist = False, os=None, version=None)

Returns reason("not on whitelist" or "blacklisted with content'foo'") or None

First cut implementation

   1 def can_build(pkg, use_whitelist = False, use_blacklist = False, os_name = None, os_version = None):
   2     """
   3     Return (buildable, "reason why not")
   4     """
   5     output_str = ""
   6     output_state = True
   7 
   8     if use_whitelist:
   9         pass
  10 
  11     if use_blacklist:
  12         blacklist_packages = roslib.rospack.rospack_depends_on(pkg)
  13         blacklist_packages.append(pkg)
  14         for p in blacklist_packages:
  15             blacklist_path = os.path.join(get_pkg_dir(p), "ROS_BUILD_BLACKLIST")
  16             if os.path.exists(blacklist_path):
  17                 output_state = False
  18                 output_str += "ROS_BUILD_BLACKLIST found in %s"%p
  19                 with open(blacklist_path) as f:
  20                     output_str += f.read() + "\n"
  21 
  22     if os.path.exists(os.path.join(get_pkg_dir(pkg), "ROS_NOBUILD")):
  23         output_state = False
  24         output_str += "ROS_NOBUILD in package\n"
  25 
  26 
  27     if not os.path.exists(os.path.join(get_pkg_dir(pkg), "Makefile")):
  28         output_state = False
  29         output_str += " No Makefile in package\n"
  30 
  31 
  32 
  33     return (output_state, output_str)

manifest extension

Os specific whitelist of tested OSs. This should be the set of OSs which are build tested for this package. (it will be used by hudson and deb builders)

Tully's Format below:

<tested-build>
 <os ubuntu>
   <version hardy >
   <version jaunty>
   <version karmic>

Issues: precedence?

Ken's original probably better since simpler with codenames instead:

<platform name="ubuntu" version="hardy" />

If in exports (eliminate nomenclature mismatch about version vs. codename):

<export>
<platform name="ubuntu-hardy" />
<platform_status ubuntu-jaunty="build_tested"
                 ubuntu-hardy="build_tested"/>
<build_tested platform="ubuntu-jaunty ubuntu-hardy ubuntu-karmic"/>
</export>

Meeting agenda

To be filled out by proposer based on comments gathered during API review period

Conclusion

Package status change mark change manifest)

• Action items that need to be taken.

• Major issues that need to be resolved

• add to roslib manifest parsing of top level "platform", os="ubuntu" version="8.04", notes="human readable notes about this status"
  • version required, on rolling release use latest
  • notes optional
• make os detection of numbers work, look at a way override pyyaml parsers of floats
• use versions, strip to major.minor for ubuntu
• platform os=osx version=10.6(string match even if a number)
• buildability determined by platform_supported("package", "os", "version")
  • quick version current_platform_supported("package")
  • add a recursive version
  • only do whitelist, write whitelist blacklist makefile nobuild in rosmake
  • rosdep won't skip blacklisted packages