[ET Trac] [Einstein Toolkit] #241: Thorns should be able to specify implementation versions, and other thorns should be able to depend on those

[Einstein Toolkit]

#241: Thorns should be able to specify implementation versions, and other thorns
should be able to depend on those
--------------------------+-------------------------------------------------
  Reporter:  knarf        |       Owner:            
      Type:  enhancement  |      Status:  reopened  
  Priority:  minor        |   Milestone:  ET_2016_05
 Component:  Cactus       |     Version:            
Resolution:               |    Keywords:            
--------------------------+-------------------------------------------------
Changes (by rhaas):

  * status:  review => reopened


Comment:

 * this patch contains commented out debugging code. Please remove.
 * please add an explicit {{{return 0}}} at the end of
 CompareVersionStrings if the the loop is actually expected to ever end
 * CompareVersionStrings contains an infinite loop if none of the if
 statements in it trigger (ie would would seem to happen if the two version
 strings are exactly identical?). If there is not infinite loop, it is
 confusing logic and requires are comment.
 * please provide patches from git via "git format-patch" rather than "git
 diff"
 * this patch does not include a documentation update
 * the patch removes a space " " from the allowed capabilities names. While
 I am fine with this, this should be mentioned and stated that this does
 not cause any problems with currently existing thorns or (better) that
 other parts of the infrastructure currently would have failed if spaces
 had been used so that there are guaranteed to be no thorns with spaces in
 capability names out in the wild.
 * rather than the "last_req" logic I would try and use perl's pattern
 matching capabilities like this
 {{{
 $a = "a b c (1.0) d (2.0) e f";
 while($a =~ m/ *([a-z]+) +(\([^)]+\))?/g) {
   $cap = $1;
   $ver = ($2 or "all versions");
   print "cap: $cap in version $ver\n";
 }
 }}}
 * the version number that is parsed by ParseProvideBlock seems to be
 required to start with a number. This is strange and different from what
 CompareVersionStrings help text implies "Compares two version strings:
 first non-numeric prefix lexically, next numeric prefix of remainder
 numerically, and so on."

 I think the patch is mostly fine. There are some minor points where I am
 asking for clarification and suggesting if possible a different approach.
 This patch needs to add documentation of this feature to the User guide
 though before it can be approved.

 I notice we are still lacking a category "reviewed and not approved".

-- 
Ticket URL: <https://trac.einsteintoolkit.org/ticket/241#comment:10>
Einstein Toolkit <http://einsteintoolkit.org>
The Einstein Toolkit