Bug 6439 - Let GT distro set version number of java ws core
: Let GT distro set version number of java ws core
Status: RESOLVED FIXED
: Java WS Core
globus_wsrf_core
: 4.2.1
: Macintosh All
: P3 normal
: 4.2.2
Assigned To:
:
: 4.0.x
:
:
  Show dependency treegraph
 
Reported: 2008-10-08 14:28 by
Modified: 2008-10-27 09:34 (History)


Attachments


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2008-10-08 14:28:55
These two files have GT distro version numbers in them:
wsrf/java/core/source/src/org/globus/wsrf/utils/Version
wsrf/java/build.xml

Please provide a way that I can populate them with a value.  For instance "ant
distsrc -Dgt.version=4.2.2" would work for me, though I am not tied to that
method if something else is easier for you.  Please note whatever you choose
will have to work for the org/globus/wsrf/utils/Version even if it is being
built by GPT as a package instead of by wsrf/java/build.xml.

However, I am only concerned about this when java ws core is being distributed
standalone (source and binary) or as part of a full GT toolkit distribution.  I
would prefer if it returned a version like "globus_4_2_branch" or "HEAD" when
it was not part of an official GT distribution.
------- Comment #1 From 2008-10-13 16:19:05 -------
Providing a patch with a potential solution:
http://www.mcs.anl.gov/~ranantha/bug6439.patch. This uses a build.properties
file for version number, allows it to be overridden with Ant option and at
compile time sets up the version number in Version.java. Version number is now
stored in wsrf/build.properties file only.

Draw back is that that it does a token replacement, so any accidental commit to
that file is going to cause the system to break.
------- Comment #2 From 2008-10-15 10:38:01 -------
Reworked patch to do a regular expression replace. On build, GT_VERSION
variable in Version.java is always replaced with value of property gt.version
in build.properties.

Fix merged to trunk, 4.2 branch and 4.0 branch.
------- Comment #3 From 2008-10-15 14:06:02 -------
Charles pointed out that without supporting MAJOR.MINOR.MICRO-string,
differentiating between nightly releases is an issue. Setting aside issue of
generating unique string for nightly for now, support of arbitrary string is
required.
------- Comment #4 From 2008-10-16 08:43:01 -------
A GridShib buildbot test failed last night:

http://www.globus.org/mail_archive/gridshib-dev/2008/10/msg00079.html

If you dig down into that error report, you find this:

http://buildbot.ncsa.uiuc.edu:8010/builders/gs4gt-gpt-build-HEAD/builds/28/steps/shell/logs/stdio

which is almost certainly a consequence of this version number patch.

I can't tell from the buildbot error messages exactly what the problem is.  I'm
investigating further at this time.
------- Comment #5 From 2008-10-16 09:08:45 -------
The following comment is independent of the bug report in Comment #4.

I have no proof but I believe the following method in Version.java is broken:

public static String getVersion() {
    return GT_VERSION;
}

If getVersion() is called before calling any other method in Version.java, it
returns the original value of GT_VERSION (MAJOR.MINOR.MICRO-SUFFIX).  If,
however, getVersion() is called subsequent to calling some other method in
Version.java, it returns the computed value of GT_VERSION (MAJOR.MINOR.MICRO). 
A workaround is to call parseVersion() before calling getVersion().

Now if the above behavior is intentional, then I claim that breaks backward
compatibility.  If you want to provide an interface to both version numbers
(MAJOR.MINOR.MICRO and MAJOR.MINOR.MICRO-SUFFIX), and you want to maintain
backward compatibility, then you need to provide a new method to return the
latter.

A nice by-product of adding a new method is that parseVersion() need not be
exposed in the API.  In fact, you can get rid of parseVersion() altogether by
wrapping its body in a static initializer.
------- Comment #6 From 2008-10-16 09:22:06 -------
parseVersion does not modify the value of GT_VERSION to remove the suffix. So
you should not see the suffix dropped if you use it.

I see your point that your parsing code will need to change to accommodate
getVersion method change. Will add a method to get the whole piece and have
getVersion return only MAJOR.MINOR.MICRO.
------- Comment #7 From 2008-10-16 09:26:02 -------
It is an unfortunate fact that prior to this patch, the API of Version.java
provided access to constants in two ways, both directly and by a getter method.
 For example:

public static final int MAJOR = 4;
public static int getMajor() {
    return MAJOR;
}

In the old version of Version.java, this isn't a big deal, but in the new
version of Version.java the constants are no longer constant:

public static int MAJOR = 0;
public static int getMajor() {
    parseVersion();
    return MAJOR;
}

This breaks backward compatibility since direct access to the "constant"
returns a different value than it did before.  The only solution I can see to
this problem is to remove parseVersion() from the API and wrap its body in a
static initializer (as mentioned in Comment #5).

In fact, removing the 'final' modifier from the constants in Version.java
itself breaks backward compatibility since now the "constants" are instance
variables.  This breaks the public API of Version.java.

In summary, I recommend the 'final' modifier be reinstated and that the values
of the constants be computed in a static initializer for the class.
------- Comment #8 From 2008-10-16 09:56:59 -------
Thanks Tom. Pasting below a patch for trunk where the getVersion() and version
variables are back to what they were. 

Any idea on the GridShib build issue? I'll wait until we sort out that issue,
before I commit. In your logs I do see that the GT installer piece that picks
up the version variable worked. So probably related to API usage?

Index: java/core/source/src/org/globus/wsrf/utils/Version.java
===================================================================
RCS file:
/home/globdev/CVS/globus-packages/wsrf/java/core/source/src/org/globus/wsrf/utils/Version.java,v
retrieving revision 1.9
diff -u -r1.9 Version.java
--- java/core/source/src/org/globus/wsrf/utils/Version.java    15 Oct 2008
20:32:57 -0000    1.9
+++ java/core/source/src/org/globus/wsrf/utils/Version.java    16 Oct 2008
14:52:30 -0000
@@ -20,40 +20,27 @@
  * The version number is composed as MAJOR.MINOR.MICRO.
  */
 public class Version {
-    

     /**
      * GT version. Required to be MAJOR.MINOR.MICRO-SUFFIX, with
      * format integer.integer.integer-string . If MICRO is
      * not set, it is set to 0. Suffix is optional. 
      */
-    public static String GT_VERSION = "@GT_VERSION@";
-
-    private static boolean parsed = false;
-
+    private static String GT_VERSION = "@GT_VERSION@";
+    
     /** The major release number */
-    public static int MAJOR = 0;
+    public static final int MAJOR;

     /** The minor release number */
-    public static int MINOR = 0;
+    public static final int MINOR;

     /** The micro release number */
-    public static int MICRO = 0;
-    
+    public static final int MICRO;
+
+    /** The suffix string following the micro release number */
     public static String MICRO_SUFFIX = "";

-    /** 
-     * Returns the current version as string in the form MAJOR.MINOR.MICRO.
-     */
-    public static String getVersion() {
-        return GT_VERSION;
-    }
-    
-    public static void parseVersion() {
-        
-        if (parsed) {
-            return;
-        }
+    static {

         int firstDot = GT_VERSION.indexOf(".");
         if (firstDot == -1) {
@@ -125,17 +112,29 @@
                 }
             }
         }
-        parsed = true;
-        
     }
-
+    
+    /** 
+     * Returns the current version as string in the form MAJOR.MINOR.MICRO.
+     */
+    public static String getVersion() {
+        return getMajor() + "." + getMinor() + "." + getMicro();
+    }
+    
+    /** 
+     * Returns the current version as string in the form 
+     * MAJOR.MINOR.MICRO-SUFFIX
+     */
+    public static String getFullVersion() {
+        return GT_VERSION;
+    }
+    
     /**
      * Returns the major release number.
      * 
      * @return the major release
      */
     public static int getMajor() {
-        parseVersion();
         return MAJOR;
     }

@@ -145,7 +144,6 @@
      * @return the minor release number
      */
     public static int getMinor() {
-        parseVersion();
         return MINOR;
     }

@@ -155,12 +153,10 @@
      * @return the micro level
      */
     public static int getMicro() {
-        parseVersion();
         return MICRO;
     }

     public static String getMicroSuffix() {
-        parseVersion();
         return MICRO_SUFFIX;
     }

@@ -169,6 +165,7 @@
      */
     public static void main(String [] args) {
         System.out.println("Java WS Core version: " + getVersion());
+        System.out.println("Java WS Core full version: " + getFullVersion());
     }

 }
------- Comment #9 From 2008-10-16 17:00:20 -------
I am not sure if this is the only issue, but the following needs to be fixed:

In trunk 
./fait_accompli/installer.Makefile.prelude:export GLOBUS_VERSIONNAME

In 4.2. branch
./fait_accompli/installer.Makefile.prelude:export GLOBUS_VERSION
GLOBUS_VERSIONNAME

Core uses the environment variable GLOBUS_VERSION in the build instructions.
------- Comment #10 From 2008-10-17 07:44:40 -------
(In reply to comment #4)
> A GridShib buildbot test failed last night...

FYI, there was no test failure last night AFAIK.  Was the bug in Comment #9
fixed, or is there some other explanation for the test behavior?
------- Comment #11 From 2008-10-17 09:56:49 -------
Tom, it's worth pointing out that the non-release (i.e., nightly CVS) behavior
is still going to change from your perspective of parsing the version number to
determine attributes of the release.  From a version setting of "4.0.0-branch",
you're going to get major=4, minor=0, micro=0.  Thus your pre/post-4.0.8 logic
is going to break in your nightly builds even after the removal of the suffix,
as would logic (if you ever need it) about earlier/later versions of 4.2.x.

My personal suggestion is that you not try to use the Java WS Core Version API
to determine how to build gridshib, but to look for the actual features that
are important to you; namely the WSRF spec level and presence/absence of
incompatible security jar files.
------- Comment #12 From 2008-10-17 10:15:57 -------
(In reply to comment #11)
> Tom, it's worth pointing out that the non-release (i.e., nightly CVS) behavior
> is still going to change from your perspective of parsing the version number to
> determine attributes of the release.  From a version setting of "4.0.0-branch",
> you're going to get major=4, minor=0, micro=0.  Thus your pre/post-4.0.8 logic
> is going to break in your nightly builds even after the removal of the suffix,
> as would logic (if you ever need it) about earlier/later versions of 4.2.x.

Charles, are you saying that GT 4.0.9 and later will carry a version number of
"4.0.0-branch"?  If so, then I must strongly object to this
compatibility-breaking modification to Version.java.

> My personal suggestion is that you not try to use the Java WS Core Version API
> to determine how to build gridshib, but to look for the actual features that
> are important to you; namely the WSRF spec level and presence/absence of
> incompatible security jar files.

That requires a major overhaul to the GS4GT build process that I'm not prepared
to do at this time.
------- Comment #13 From 2008-10-17 10:36:46 -------
No, please re-read what I wrote.  I am talking about the behavior in nightly
CVS.  Releases will continue to be marked with the release number.
------- Comment #14 From 2008-10-17 10:42:52 -------
Tom,

Do you need to detect that a nightly installer is post a particular release?
Like say that the nightly is post 4.0.8?

We could change the scheme from setting branch version to be 4.0.0-branch to
4.0.x-branch. This probably also preserves the behavior from before.

Rachana
------- Comment #15 From 2008-10-17 10:57:29 -------
Patch from comment 8 has been committed to 4.2.x, 4.0.x and trunk.
------- Comment #16 From 2008-10-17 12:03:28 -------
(In reply to comment #7)
> 
> In fact, removing the 'final' modifier from the constants in Version.java
> itself breaks backward compatibility since now the "constants" are instance
> variables.  This breaks the public API of Version.java.

Sorry, the first sentence above is false.  Of course removing the 'final'
modifier simply allows the user to modify the static variable.

> In summary, I recommend the 'final' modifier be reinstated... 

Thanks for reinstating the API of Version.java.  To followup, I still think
it's a good idea to reinstate the 'final' modifiers on the "constants,"
otherwise users can modify their values and leave the class in an inconsistent
state.  I think it's as simple as changing the following lines

public static int MAJOR = 0;
public static int MINOR = 0;
public static int MICRO = 0;
public static String MICRO_SUFFIX = "";

to this

final public static int MAJOR;
final public static int MINOR;
final public static int MICRO;
final public static String MICRO_SUFFIX;

You can (and should) make GT_VERSION final also, but then you'll have to modify
the static initializer to *not* assign a new value to the constant GT_VERSION,
but rather a private variable that ultimately gets returned by
getFullVersion().

Finally, I'll mention that the 'parsed' variable is dead code and the copyright
should be updated.

Thanks.
------- Comment #17 From 2008-10-17 12:08:00 -------
Tom,

What code are your comments based on? The patch does all of that: final
modifiers are back, GT_VERSION is private and cannot be modified, only has a
get method that returns it, parse has been removed. I suggest you look at
committed code now

Thanks,
Rachana
------- Comment #18 From 2008-10-17 12:30:56 -------
(In reply to comment #17)
> Tom,
> 
> What code are your comments based on?

Revision 1.6.2.5

> The patch does all of that: final
> modifiers are back, GT_VERSION is private and cannot be modified, only has a
> get method that returns it, parse has been removed.

Ah, I see Revision 1.10 has all the patches (except final MICRO_SUFFIX and
copyright patch), thanks.

> I suggest you look at committed code now

I was.  Revision 1.6.2.5 and Revision 1.7.2.4 are still not patched (but I'm
sure you already know that).  Thanks.
------- Comment #19 From 2008-10-17 12:32:40 -------
(In reply to comment #14)
> 
> Do you need to detect that a nightly installer is post a particular release?
> Like say that the nightly is post 4.0.8?

Yes.

> We could change the scheme from setting branch version to be 4.0.0-branch to
> 4.0.x-branch. This probably also preserves the behavior from before.

Sounds great, thanks.
------- Comment #20 From 2008-10-17 14:07:59 -------
All branches have been updated. 

Tom, the GT_VERSION and MICRO_SUFFIX are new variables introduced and I have
them as private. So I don't see an issue with GT_VERSION not being final. 

The version numbers now have 4.0.x-branch and 4.2.x-branch where x is the last
point release number.
------- Comment #21 From 2008-10-17 14:17:28 -------
(In reply to comment #20)
> 
> Tom, the GT_VERSION and MICRO_SUFFIX are new variables introduced and I have
> them as private. So I don't see an issue with GT_VERSION not being final. 

You're right, thanks.
------- Comment #22 From 2008-10-17 14:19:27 -------
(In reply to comment #20)
> 
> The version numbers now have 4.0.x-branch and 4.2.x-branch where x is the last
> point release number.

This is great!  I'm happy :-)  Thanks, Rachana.
------- Comment #23 From 2008-10-27 09:34:46 -------
Looks like everyone is happy with the changes.