Bug 2563

Summary: pybindgen version check does not allow for multiple versions of NS-3 on the same system
Product: ns-3 Reporter: Robert Ammon <ammo6818>
Component: python bindingsAssignee: Gustavo J. A. M. Carneiro <gjcarneiro>
Status: RESOLVED FIXED    
Severity: normal CC: ns-bugs, tomh
Priority: P3    
Version: ns-3-dev   
Hardware: All   
OS: All   

Description Robert Ammon 2016-11-24 10:50:54 UTC 
The WAF CONFIGURE function checks the version of pybindgen on the system for a minimum version.  The implementation of the check function in bindings/python/wscript uses a '==' comparison to the minimum required version string of pybindgen.

When a newer version of NS-3 is installed that requires a newer version of pybindgen, the minimum version check for pybindgen in the other version of NS-3 fails because only one version of pybindgen exists on the system and the version string check fails because of the newer version of pybindgen.

Need to modify the pybindgen version check to require a minimum version of pybindgen rather than a specific version.
Comment 1 Robert Ammon 2016-11-24 11:01:51 UTC 
Changes required uploaded as http://codereview.appspot.com/318840043
Comment 2 Tom Henderson 2016-11-24 13:06:40 UTC 
This seems reasonable and should work based on how pybindgen version strings are currently being written.  Let's wait for Gustavo's input (who originally coded the test for strict equality-- maybe there is another reason for that?)
Comment 3 Gustavo J. A. M. Carneiro 2016-11-24 18:26:33 UTC 
So right now pybindgen versions are something like "0.17.0.post58+ngcf00cc0".  While ngcf00cc0 is just a git short hash, we can at least rely on the 'post58' part to be monotonically increasing.  "0.17.0.post59+whatever" >= "0.17.0.post58+ngcf00cc0" should hold true.

The problem will come when we jump from post99 to post100: 

  >>> "0.17.0.post100" > "0.17.0.post99"
  False

Basically string comparison here is not the best way.  I mean, it will work for the foreseeable future, but there will come a time when this will stop working.

A better approach would be along these lines:

>>> def split_version(version):
...     ver = re.split('[.+]', version)[:4]
...     return (int(ver[0]), int(ver[1]), int(ver[2]), int(ver[3].split('post')[1]))
... 
>>> split_version('0.17.0.post100+ng1feb387')
(0, 17, 0, 100)
>>> split_version('0.17.0.post100+ng1feb387') > split_version('0.17.0.post99+ng1feb387')
True

On the whole "allowing pybindgen version to be greater than the ns-3 requested version" issue, I don't mind at all.  It was like that in the beginning, but then there was an incident when a new pybindgen version broke ns-3 build and then Mathieu Lacage pushed for freezing the PBG version.  I don't mind either way.  Not doing much development in PBG lately, so it should remain stable.
Comment 4 Robert Ammon 2016-11-24 20:11:25 UTC 
Updated change set uploaded to address review comments and make implementation more generic.
Comment 5 Gustavo J. A. M. Carneiro 2016-11-25 10:36:21 UTC 
(In reply to Robert Ammon from comment #4)
> Updated change set uploaded to address review comments and make
> implementation more generic.

LGTM
Comment 6 Tom Henderson 2016-11-30 23:17:08 UTC 
pushed in changeset 12438:db0538b1b326