Ticket [5d65e65036]. My fix. Do not skip the second check for stable versions even when the main check fails. Avoided radical changes to the structure (kept single search loop, with selection after).

1 files changed, 58 insertions, 33 deletions

diff --git a/generic/tclPkg.c b/generic/tclPkg.c
index 52f33c3..d3dd584 100644
--- a/generic/tclPkg.c
+++ b/generic/tclPkg.c
@@ -349,7 +349,7 @@ PkgRequireCore(
     Interp *iPtr = (Interp *) interp;
     Package *pkgPtr;
     PkgAvail *availPtr, *bestPtr, *bestStablePtr;
-    char *availVersion, *bestVersion;
+    char *availVersion, *bestVersion, *bestStableVersion;
 				/* Internal rep. of versions */
     int availStable, code, satisfies, pass;
     char *script, *pkgVersionI;
@@ -395,6 +395,7 @@ PkgRequireCore(
 	bestPtr = NULL;
 	bestStablePtr = NULL;
 	bestVersion = NULL;
+	bestStableVersion = NULL;
 
 	for (availPtr = pkgPtr->availPtr; availPtr != NULL;
 		availPtr = availPtr->nextPtr) {
@@ -408,58 +409,82 @@ PkgRequireCore(
 
 		continue;
 	    }
-
+	    
+	    /* Check satisfaction of requirements before considering the current version further. */
+	    if (reqc > 0) {
+		satisfies = SomeRequirementSatisfied(availVersion, reqc, reqv);
+		if (!satisfies) {
+		    ckfree(availVersion);
+		    availVersion = NULL;
+		    continue;
+		}
+	    }
+	    
 	    if (bestPtr != NULL) {
 		int res = CompareVersions(availVersion, bestVersion, NULL);
 
 		/*
-		 * Note: Use internal reps!
+		 * Note: Used internal reps in the comparison!
 		 */
 
-		if (res <= 0) {
+		if (res > 0) {
 		    /*
-		     * The version of the package sought is not as good as the
-		     * currently selected version. Ignore it.
+		     * The version of the package sought is better than the
+		     * currently selected version.
 		     */
-
-		    ckfree(availVersion);
-		    availVersion = NULL;
-		    continue;
+		    goto newbest;
 		}
-	    }
-
-	    /* We have found a version which is better than our max. */
-
-	    if (reqc > 0) {
-		/* Check satisfaction of requirements. */
+	    } else {
+	    newbest:
+		/* We have found a version which is better than our max. */
 
-		satisfies = SomeRequirementSatisfied(availVersion, reqc, reqv);
-		if (!satisfies) {
-		    ckfree(availVersion);
-		    availVersion = NULL;
-		    continue;
-		}
+		bestPtr = availPtr;
+		CheckVersionAndConvert(interp, bestPtr->version, &bestVersion, NULL);
 	    }
 
-	    bestPtr = availPtr;
-
-	    if (bestVersion != NULL) {
-		ckfree(bestVersion);
+	    if (!availStable) {
+		ckfree(availVersion);
+		availVersion = NULL;
+		continue;
 	    }
-	    bestVersion = availVersion;
 
-	    /*
-	     * If this new best version is stable then it also has to be
-	     * better than the max stable version found so far.
-	     */
+	    if (bestStablePtr != NULL) {
+		int res = CompareVersions(availVersion, bestStableVersion, NULL);
+
+		/*
+		 * Note: Used internal reps in the comparison!
+		 */
 
-	    if (availStable) {
+		if (res > 0) {
+		    /*
+		     * This stable version of the package sought is better
+		     * than the currently selected stable version.
+		     */
+		    goto newstable;
+		}
+	    } else {
+	    newstable:
+		/* We have found a stable version which is better than our max stable. */
 		bestStablePtr = availPtr;
+		CheckVersionAndConvert(interp, bestStablePtr->version, &bestStableVersion, NULL);
 	    }
-	}
 
+	    ckfree(availVersion);
+	    availVersion = NULL;
+	} /* end for */
+
+	/*
+	 * Clean up memorized internal reps, if any.
+	 */
+	
 	if (bestVersion != NULL) {
 	    ckfree(bestVersion);
+	    bestVersion = NULL;
+	}
+
+	if (bestStableVersion != NULL) {
+	    ckfree(bestStableVersion);
+	    bestStableVersion = NULL;
 	}
 
 	/*