Closed
Bug 292138
Opened 20 years ago
Closed 19 years ago
VersionCheck does not correctly sort versions
Categories
(addons.mozilla.org Graveyard :: Public Pages, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
1.1
People
(Reporter: Bugzilla-alanjstrBugs, Assigned: morgamic)
References
Details
We lost the versioncompare function
http://lxr.mozilla.org/mozilla/source/webtools/update/update/VersionCheck.php#94
It is used because in the mozilla world, 0.10 > 0.9
Per IRC discussion, here is a suggested approach in case one is needed. =)
At about http://lxr.mozilla.org/update1.0/source/update/VersionCheck.php#159
take out checks for version.minappversion and version.version and also the limit 1
Then drop in the procedure from
http://lxr.mozilla.org/mozilla/source/webtools/update/update/VersionCheck.php#268
to line 294 (with possible optimizations) and also the vercmp function at lines
94-106
|
Assignee | |
Comment 2•20 years ago
|
||
I think the correct way to fix this would be to take a look at how .10 > .9 in
the first place.
The source of this problem comes from a lack of constraints placed on internal
versioning. Because of this, an arithmetic impossibility occurs in versioning
across the board.
A more proper way to fix this might be:
Separate display versions from application logic.
Properly use and validate internal versions via a key or index that is
mathematically acceptable.
Advantages:
Not using PHP to simulate what MySQL should be doing with a properly ordered
result set.
Reduce load caused by unnecessary data transfer and subsequent string parsing.
Reduce time complexity from N to 1...
Would be fairly easy to implement in the database, and would be a one-time
fix to the workflow.
Disadvantages:
Would require some manipulation of current production database for ".10"
versions.
Possible negative impacts on some extensions ... ?
We could use a user-defined mysql function like ORDER BY parseInts(version_strings)
parseInts would understand A.B.C.D and convert it into something that natively
sorts correctly like fixed width strings 0000A.0000B.0000C.0000D.
Using a user-defined function, you'd still have to parse every version number.
Even if it's compiled in C, it may not be much faster than the PHP code and will
add possibly unneeded complexity. Another thing, you'd have to pad enough so
that no subversion number is longer than the padding. I guess you could safely
do 255 if the versions are being stored as VARCHARs in the first place.
|
||
Comment 5•19 years ago
|
||
*** Bug 300679 has been marked as a duplicate of this bug. ***
|
||
Comment 6•19 years ago
|
||
What if we convert all versions to be in the form x.x.x.x (by adding 0's as
necessary), then use MySQL's INET_ATON and INET_NTOA functions? The INET_ATON
function converts it to a 4 byte number, and the INET_NTOA converts it back.
This is a good way to handle IP addresses in MySQL, and as long as all the
versions are in the x.x.x.x format, it'll be sorted correctly. Another
limitation would be that you can't have a version number greater than 255.
*** Bug 312133 has been marked as a duplicate of this bug. ***
*** Bug 322069 has been marked as a duplicate of this bug. ***
|
||
Comment 9•19 years ago
|
||
Here is a possible working PHP solution. I'm sure this could be converted into a MySQL user function:
-----
<html>
<body>
<pre>
<?php
function versionCompare($a, $b)
{
if ($a{0} == '.')
$a = '0'.$a;
if ($b{0} == '.')
$b = '0'.$b;
$aa = explode('.', $a);
$ba = explode('.', $b);
while (count($aa) < count($ba))
$aa[] = 0;
while (count($ba) < count($aa))
$ba[] = 0;
for ($i = 0; $i < count($aa); $i++) {
$diff = intval($aa[$i]) - intval($ba[$i]);
if ($diff != 0)
break;
}
return $diff;
}
function test($a, $b, $expect)
{
$rc = versionCompare($a, $b);
printf(
"%s a=%s b=%s expect=%d result=%d\n",
$rc == $expect ? 'OK' : 'NO',
$a, $b, $expect, $rc
);
}
// Compare strings of equal length.
test('1', '1', 0);
test('1', '2', -1);
test('2', '1', 1);
test('1.1', '1.1', 0);
test('1.1', '1.2', -1);
test('1.2', '1.1', 1);
test('1.2.2', '1.2.2', 0);
test('1.2.2', '1.2.3', -1);
test('1.2.3', '1.2.2', 1);
// Compare more unusual cases.
test('1', '1.1', -1);
test('2.001', '2.1', 0);
test('3.1', '1.2.2', 2);
test('.10', '0.1', 9);
?>
</pre>
</body>
</html>
----
Alternatively, simply compare the versions by-submission-date instead of by-version. This assumes that previous versions cannot be replaced once submitted. Using by-submission-date should allow for numerical testing in MySQL IF the submission date is a timestamp.
|
|
Assignee | |
Comment 10•19 years ago
|
||
What about going based on vID, which an auto-incrementing primary key? This might solve the problem. What do you think?
Status: NEW → ASSIGNED
|
Reporter | |
Comment 11•19 years ago
|
||
(In reply to comment #10)
> What about going based on vID, which an auto-incrementing primary key? This
> might solve the problem. What do you think?
It would be faster than doing it by date.
The only problem would be edge cases, where someone maintains an older version. For example, what if their 0.9.x branch is for Firefox 1.0.x and their 0.10.x branch is for Firefox 1.5. If they made any update for the former, it would sort at the top.
But that's an edge case and worth the risk I say.
|
|
Reporter | |
Comment 12•19 years ago
|
||
*** Bug 333578 has been marked as a duplicate of this bug. ***
|
|
Assignee | |
Comment 13•19 years ago
|
||
This was eventually fixed by sorting by version.vID. See bug 334747.
Granted, the C++ alrorithm isn't used (and probably shouldn't be, because of performance, IMO).
An eventual fix would be generating all possible results as static RDFs and removing the need for a dynamic script here. That would be a win-win because it would remove the scalability requirement and let us mimic the vercomp stuff in the browser.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
|
||
Updated•9 years ago
|
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•