If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

AMO not sending compatiblity updates for addons when compatMode=normal

RESOLVED FIXED in 6.3.8

Status

addons.mozilla.org Graveyard
API
RESOLVED FIXED
6 years ago
2 years ago

People

(Reporter: glob, Assigned: robhudson)

Tracking

unspecified
6.3.8

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
Created attachment 585347 [details]
extensions.sqlite

"mass password reset" addon is identified as being incompatible with firefox 12.0a1 (nightly 2012-01-02), and i'm unable to enable or install it.

my extensions.log file doesn't contain anything interesting from today:

> 2011-10-01 11:07:03 ERROR addons.repository: Outstanding transaction, rolling back. at resource://gre/modules/AddonRepository.jsm:1325
> 2011-12-28 14:18:46 ERROR addons.xpi: Failed to open database (1st attempt): [Exception... "Component returned failure code: 0x80630001 (NS_ERROR_STORAGE_BUSY) [mozIStorageService.openUnsharedDatabase]"  nsresult: "0x80630001 (NS_ERROR_STORAGE_BUSY)"  location: "JS frame :: resource:///modules/XPIProvider.jsm :: XPIDB_openDatabaseFile :: line 4216"  data: no] at resource:///modules/XPIProvider.jsm:4216
> 2011-12-28 14:18:46 ERROR addons.xpi: Error processing file changes: TypeError: this.installLocations is null at resource:///modules/XPIProvider.jsm:1779
Related bugs with the same storage error: bug 671894 bug 705205.
This seems to be caused by this extension having a maxVersion of 3.6a1pre in it's install.rdf - which is less than the minimum compatible version (4.0). AMO has updated compatibility data which makes the maxVersion 11.0a1, which makes it compatible. However, it seems AMO is not sending updated compatibility data on the update ping, when compatMode=normal. Presumably, a regression from bug 703781.

The requested URL for the update ping:
https://versioncheck.addons.mozilla.org/update/VersionCheck.php?reqVersion=2&id=masspasswordreset@johnathan.nightingale&version=1.05&maxAppVersion=3.6a1pre&status=userEnabled,incompatible&appID={ec8030f7-c20a-464f-9b0e-13a3a9e97384}&appVersion=12.0a1&appOS=WINNT&appABI=x86-msvc&locale=en-US&currentAppVersion=12.0a1&updateType=49&compatMode=normal

Currently returns nothing, whereas it should be returning data on the current version, so that the Add-ons Manager gets the updated compatibility data.

A similar update ping request with compatMode=ignore correctly includes the relevant data:
https://versioncheck.addons.mozilla.org/update/VersionCheck.php?reqVersion=2&id=masspasswordreset@johnathan.nightingale&version=1.05&maxAppVersion=3.6a1pre&status=userEnabled,incompatible&appID={ec8030f7-c20a-464f-9b0e-13a3a9e97384}&appVersion=12.0a1&appOS=WINNT&appABI=x86-msvc&locale=en-US&currentAppVersion=12.0a1&updateType=49&compatMode=ignore
Component: Add-ons Manager → API
OS: Mac OS X → All
Product: Toolkit → addons.mozilla.org
QA Contact: add-ons.manager → api
Hardware: x86 → All
Summary: "mass password reset" addon is identified as being incompatible with firefox 12.0a1 (nightly) → AMO not sending compatiblity updates for addons when compatMode=normal
Version: 12 Branch → unspecified
Blocks: 703781
Assignee: nobody → chudson
Target Milestone: --- → 6.3.7
(Assignee)

Comment 3

6 years ago
Looking at the code history, I can't see that we've ever returned the latest version if no compatible versions were found.  Would Firefox look at the RDF to tell whether the version we're supplying is either (1) a compatible version or (2) not a compatible version but the latest version fallback?

Also, isn't the GUID search the place to get compatibility information about an add-on?
(In reply to Rob Hudson [:robhudson] from comment #3)
> Looking at the code history, I can't see that we've ever returned the latest
> version if no compatible versions were found.  Would Firefox look at the RDF
> to tell whether the version we're supplying is either (1) a compatible
> version or (2) not a compatible version but the latest version fallback?

When Firefox installs an add-on that appears incompatible it pings versioncheck to ask AMO to send the latest compatibility information about the add-on. In the past if the latest compatibility information didn't include the user's current Firefox then not sending it wouldn't have been a problem. Now that we have DTC in some cases it would be required to achieve the effect we want (identifying add-ons that are compatible with at least Firefox 4 even if they aren't compatible with the current Firefox).
Duplicate of this bug: 716257
For future reference:
This is testable by attempting to install Mass Password Reset v1.04. It's maxVersion according to AMO is 10.*, but according to it's install.rdf it's some version below 4.0 - which is the minimum cut-off. Note that the newer v1.05 doesn't exhibit this behaviour, because it's install.rdf was recently updated in order to work around this bug.
https://addons.mozilla.org/en-US/firefox/addon/mass-password-reset/versions/
(Assignee)

Comment 7

6 years ago
I spoke with Blair on IRC. I was misunderstanding this bug to mean send the latest version even if there are no compatible versions found. But this was incorrect.

So I'm looking at the details of the parameters passed and what we have in AMO to return the correct result in these cases. The example posted above no longer works. Is there another example I can look at?
(In reply to Rob Hudson [:robhudson] from comment #7)
> I spoke with Blair on IRC. I was misunderstanding this bug to mean send the
> latest version even if there are no compatible versions found. But this was
> incorrect.
> 
> So I'm looking at the details of the parameters passed and what we have in
> AMO to return the correct result in these cases. The example posted above no
> longer works. Is there another example I can look at?

Mass Password Reset 1.0.4 (as Blair mentioned in comment 6) still triggers this for me on the latest Nightly build.
(Assignee)

Comment 9

6 years ago
According to AMO, this add-on has binary components, which triggers version check to switch to strict compatibility mode.

Is it true this add-on has binary components?  If not, that would be a bug in our validator.

If it is true and version check is doing the wrong thing, please let me know.
(In reply to Rob Hudson [:robhudson] from comment #9)
> According to AMO, this add-on has binary components, which triggers version
> check to switch to strict compatibility mode.
> 
> Is it true this add-on has binary components?  If not, that would be a bug
> in our validator.

No, mass password reset contains no binary components
(Assignee)

Comment 11

6 years ago
In this particular case it looks like the validator is tripping up on the .sh files:
https://github.com/mozilla/amo-validator/blob/master/validator/testcases/packagelayout.py#L8

The verbose output from the validator is as follows...

Warning: Flagged file extension found

The file "build.sh" has a flagged file extension.
The extension of this file is flagged because it usually identifies binary components. Please see http://addons.mozilla.org/developers/docs/policies/reviews#section-binary for more information on the binary content review process.
	Tier:	1
	File:	build.sh

Warning: Flagged file extension found

The file "config_build.sh" has a flagged file extension.
The extension of this file is flagged because it usually identifies binary components. Please see http://addons.mozilla.org/developers/docs/policies/reviews#section-binary for more information on the binary content review process.
	Tier:	1
	File:	config_build.sh
Oh boy... then I think AMO and the Add-ons Manager may have wildly differing definitions of "binary component". For the purposes of compatible-by-default, a binary component is one that is registered via a binary-component line in the addon's chrome.manifest file. Anything else has no relevance to compatibility, as far as the Add-ons Manager is concerned.
Odd, .sh files are never binary to my knowledge. Perhaps the validator is thinking the add-on might run them and then run OS stuff perhaps? Either way AMO really should be using the same mechanism for detecting binary components as Firefox does, looking for binary-component lines in chrome.manifest.
You can embed binary content in .sh scripts.  Some installers have their logic at the top and then attach the binary content at the bottom in order to distribute their files in a single runnable blob.

I agree the binary detection mechanisms between AMO and Firefox should match.  CCing Jorge and Andrew for what the right thing to do here is.
(In reply to Wil Clouser [:clouserw] from comment #14)
> I agree the binary detection mechanisms between AMO and Firefox should
> match.  CCing Jorge and Andrew for what the right thing to do here is.

For review purposes, its best to have all possible executable files flagged in the validator so they're not missed.  For compatibility reasons it only really makes sense to flag binary XPCOM components as they're the only files that definitely need recompiling each gecko version.

IMO a good solution would be the introduction of a new validator check to do the same as Firefox and check for the binary-component* line in chrome.manifest and use that as the basis of the DTC check/fail.  Keep the other checks for review purposes though.

* I'm assuming we don't need to bother about Firefox3.6 and prior add-ons as apparently this instruction was only introduced in Firefox4.
Bug 718694 adjusts the validator for the additional binary detection
(Assignee)

Comment 17

6 years ago
https://github.com/mozilla/zamboni/commit/1fca769

This updates AMO to use the binary_components field in versioncheck. Note: Before this will work we need to have AMO scan all files for a validator check to update this flag.
Target Milestone: 6.3.7 → 6.3.8
(Assignee)

Comment 18

6 years ago
Going to close. We'll run the re-validation soon when https://bugzilla.mozilla.org/show_bug.cgi?id=719545 is fixed.
Status: NEW → RESOLVED
Last Resolved: 6 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.