AMO should use new version-comparison code

RESOLVED FIXED in Future

Status

addons.mozilla.org Graveyard
Developer Pages
--
major
RESOLVED FIXED
13 years ago
3 years ago

People

(Reporter: Benjamin Smedberg, Assigned: morgamic)

Tracking

unspecified
Future
x86
All
Dependency tree / graph

Details

Attachments

(2 attachments)

(Reporter)

Description

13 years ago
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.
(Reporter)

Comment 1

13 years ago
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. ***

Comment 3

13 years ago
Deer Park needs this for testing
Severity: normal → critical

Updated

13 years ago
Blocks: 255803
(Assignee)

Comment 4

13 years ago
*** Bug 308157 has been marked as a duplicate of this bug. ***
(Assignee)

Updated

13 years ago
Status: NEW → ASSIGNED
(Reporter)

Comment 5

13 years ago
Created attachment 200612 [details]
Updated with "*" syntax
Attachment #200612 - Flags: first-review?(jabradford)
(Reporter)

Updated

13 years ago
Blocks: 313605

Updated

13 years ago
Flags: blocking1.8rc1?
(Assignee)

Comment 6

13 years ago
Working on this today, this was up after the maintenance scripts were implemented, and that happened on Friday.

Comment 7

13 years ago
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+
(Assignee)

Comment 8

13 years ago
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.

Updated

13 years ago
Flags: blocking1.8rc1? → blocking1.8rc1+
(Assignee)

Comment 9

13 years ago
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?

Comment 10

13 years ago
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
(Reporter)

Comment 11

13 years ago
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.

Comment 12

13 years ago
moreinfo.php should be using some sort of comparison to show the latest version.
(Assignee)

Comment 13

13 years ago
Created attachment 200756 [details] [diff] [review]
Code to be committed, with comments.

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.
(Reporter)

Comment 14

13 years ago
*** Bug 255803 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 15

13 years ago
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.
(Assignee)

Comment 16

13 years ago
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.
(Assignee)

Updated

13 years ago
No longer blocks: 313605

Updated

13 years ago
OS: Windows XP → All
*** Bug 271269 has been marked as a duplicate of this bug. ***

Updated

12 years ago
No longer blocks: 255803
Summary: UMO1 and 2 should use new version-comparison code → AMO should use new version-comparison code

Comment 19

12 years ago
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?
(Assignee)

Comment 20

12 years ago
> 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

Comment 21

12 years ago
(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.
(Assignee)

Comment 22

12 years ago
(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
Last Resolved: 12 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.