Bugzilla – Bug 6439
Let GT distro set version number of java ws core
Last modified: 2008-10-27 09:34:46
You need to log in before you can comment on or make changes to this bug.
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.
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.
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.
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.
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.
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.
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.
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.
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()); } }
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.
(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?
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.
(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.
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.
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
Patch from comment 8 has been committed to 4.2.x, 4.0.x and trunk.
(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.
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
(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.
(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.
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.
(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.
(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.
Looks like everyone is happy with the changes.