Closed Bug 304857 Opened 19 years ago Closed 18 years ago

AMO should use new version-comparison code

Categories

(addons.mozilla.org Graveyard :: Developer Pages, defect)

x86
All
defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED
Future

People

(Reporter: benjamin, Assigned: morgamic)

References

Details

Attachments

(2 files)

Bug 300731 introduced a new form of extension version number which allows our old "1.8a1" numbering scheme. AMO 1 should support this scheme, as well as AMO 2. In attachment 192830 [details] I have PHP code which parses these version numbers in an equivalent manner to the C++ code. I don't really know the AMO code, but I'm hoping that this function can simply be plugged in where the current version-comparison code lives.
https://bugzilla.mozilla.org/attachment.cgi?id=193014 has an updated PHP function which handles a negative-number edge case.
*** Bug 305162 has been marked as a duplicate of this bug. ***
Deer Park needs this for testing
Severity: normal → critical
Blocks: 255803
*** Bug 308157 has been marked as a duplicate of this bug. ***
Status: NEW → ASSIGNED
Attached file Updated with "*" syntax (deleted) —
Attachment #200612 - Flags: first-review?(jabradford)
Blocks: 313605
Flags: blocking1.8rc1?
Working on this today, this was up after the maintenance scripts were implemented, and that happened on Friday.
Comment on attachment 200612 [details] Updated with "*" syntax Looks good. You might consider adding a "*" test to the list at the bottom. If nothing else, it helps to document the version format. Michael: This will obviously need to be slightly modified for the AMO/UMO code. What do you need specifically?
Attachment #200612 - Flags: first-review?(jabradford) → first-review+
I think I can fly with this. I'll give it a shot and ask questions if I have any. Thanks for your help, guys.
Flags: blocking1.8rc1? → blocking1.8rc1+
Could someone please elaborate on which specific pieces of UMO are breaking because of improper/incorrect version comparisons? I'm guessing additem.php (developer RDF submission script) and VersionCheck.php (extensions update script). Is that correct?
I'm not sure why additem would have a problem with version-comparison, per se. However, additem.php and appmanager.php need to be able to handle versions properly.
Blocks: 284025
I don't know how UMO works at all: I'm concerned that UMO should be able to recognize and accept extensions with maxVersion="1.5.0.*" and "1.5.0.999" and other variants that were unrecognized under the old extension version numbering scheme.
moreinfo.php should be using some sort of comparison to show the latest version.
I'll add the PHP file for version comparisons to /core, then I will figure out for myself where it needs to go. I'll start with the application manager and its corresponding DB changes, since it's most critical, then work my way towards VersionCheck.php. Sorting, etc. in public listings is last in priority. Honestly, I don't see why this is spread out over 5 different bugs. The other bugs relating to verison numbers should probably be marked as duplicates.
*** Bug 255803 has been marked as a duplicate of this bug. ***
bug 313605 is about using the administration interface to add Firefox 1.5 and allowing extensions with maxVersion="1.5.0.*" to be added. Bug 284025 is unrelated, AFAICT, and is not relevant to Firefox 1.5 where the app version will always be the same as the extension version.
I am removing blocking status from this bug, because the major problem was found to be bug 313605. This bug will be fixed by completely revamping how application versions are stored in the database and then making subsequent changes in all version comparisons throughout the application. Once a correct data structure exists along with proper validation (in admin tool) implementing attachment 200756 [details] [diff] [review] will be fairly simple. Please see my comments on bug 313605 for more info: https://bugzilla.mozilla.org/show_bug.cgi?id=313605#c7 Please let me know if this is not accurate -- it's open for discussion, but either way I don't see it as a blocker.
Flags: blocking1.8rc1+
Then this is a 1.5.1 blocker and is not a 1.5.0.1 blocker, right? Mozilla.org might be going to release 1.5.0.1 the next day of 1.5 release, if someone found a serious security issue. If a user with a 1.5.0.1 agent can download a 1.5.0.* app from u.m.o without any problem, I agree that this bug is not a blocker. All addon authors need would be the 1.5.0.* approval between 1.5rc - 1.5 period.
No longer blocks: 313605
OS: Windows XP → All
*** Bug 271269 has been marked as a duplicate of this bug. ***
No longer blocks: 255803
Summary: UMO1 and 2 should use new version-comparison code → AMO should use new version-comparison code
I think this should not only be used for version comparisons, but also for verifying version numbers in extension submissions. It doesn't really make sense to have an enumerated list of 'allowed' versions (given in the addons.m.o FAQ) as you're bound to miss some, this makes more sense. Maybe you could limit it to not being higher than 3.0 until a 4.0 branch appears or something so version numbers don't get far out. What say you?
> Please see my comments on bug 313605 for more info: > https://bugzilla.mozilla.org/show_bug.cgi?id=313605#c7 I think finite versions are mostly useful for min versions (as a cutoff) and max versions that are too specific (down to a particular subversion) are not very helpful. The problem here is more a matter of storage and sorting of versions in MySQL and how that scales. Using a proper validation regexp combined with the use of auto_increment primary keys to sort and select app versions (grouped by GUID) might be an easier way to handle things. We have to take a serious look at the `applications` table and how it ties into the `version` table -- not only for version sorting in public listings and add-on submission but also for updates needed for add-on blacklisting (bug 290759). So - short version - the C++ comparison code is great, but this will start with the database. Take a look at https://update-staging.mozilla.org/~colin/dev/ to see what I'm talking about... (wear a helmet). :\
Severity: critical → major
Status: ASSIGNED → NEW
Target Milestone: 1.0 → Future
(In reply to comment #20) > I think finite versions are mostly useful for min versions (as a cutoff) and > max versions that are too specific (down to a particular subversion) are not > very helpful. > I disagree, and so does the Toolkit Version Standard. When comparing 2 versions, you should perform a bytewise comparison on any string part of a section.
(In reply to comment #21) > I disagree, and so does the Toolkit Version Standard. When comparing 2 > versions, you should perform a bytewise comparison on any string part of a > section. > Fair enough, but it doesn't sound like you've really read my previous comment. I'm not negating the standard at all. I was attempting to explain context differences between what you'd see in the Toolkit vs. what AMO was created with. :\ Anyway, the goal for upcoming db changes for storage of app and add-on versions would be to allow for us to adhere more closely to the standard and achieve consistency without sacrificing performance. It should be possible if we store these version strings correctly.
This was apparently committed a while back (http://lxr.mozilla.org/mozilla/source/webtools/addons/shared/lib/version_comparison.inc.php) and carried over to Remora (http://svn.mozilla.org/addons/trunk/site/app/controllers/components/versioncompare.php). Resolving... please re-open if there is still an issue.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: