1185930 - Accept dash suffixes in langpack versions

Status: RESOLVED FIXED

(Marketplace Graveyard :: Validation, enhancement, P4)

Description

[Staś Małolepszy :stas]

We need a way to support partner modifications in langpacks, and the solution that we'd like to go with is to overload the version of the Firefox OS to carry the information about the partner.  So 2.2 would be Mozilla's vanilla version while 2.2-x could be the verrsion shipped by partner X.

This will require small changes to the Gaia's build system and the Marketplace app validator to make both accept such version numbers.

I would also be helpful to not limit the possible Gaia versions by hardcoding them in 

https://github.com/mozilla/app-validator/blob/7762a030a2a5a01cd70173d99167efc771239304/appvalidator/specs/webapps.py#L321

For instance, 3.0 is now 2.5.  Can we use a more lax form of version checking here?

Comment 1

[Mathieu Pillard [:mat]]

We could use a regexp instead ; or just let it be free-form, but that's dangerous, a typo could be hard to detect then. Note that we still need the version string to match whatever FxOS settings page sends using the Web Activity.

Comment 2

[Staś Małolepszy :stas]

(In reply to Mathieu Pillard [:mat] from comment #1)
> We could use a regexp instead ; or just let it be free-form, but that's
> dangerous, a typo could be hard to detect then.

Right.  I don't think we need to make it easy to create a new version, so I'm fine with having a strict verification step.  OTOH,  I wonder what the best process to update this for you would be.  Maybe some kind of settings/langpack-versions.(py|josn) file that I could submit pull requests to?

> Note that we still need the
> version string to match whatever FxOS settings page sends using the Web
> Activity.

Right, this is the same version.  So the webactivity should support 2.2-x and 2.5.  Should I file a separate bug for this?

Comment 3

[Mathieu Pillard [:mat]]

We don't need another bug, as long as the version matches it should work - the frontend does not verify the version string, it just sends it to the API which does an exact match.

I feel like whitelisting is going to be a pain if we need to deal with builds from partners etc, so I'd prefer a simple regexp. Here is the thing though: should a langpack created for 2.2 work with 2.2-x ? And vice-versa ?

Comment 4

[Staś Małolepszy :stas]

(In reply to Mathieu Pillard [:mat] from comment #3)

> I feel like whitelisting is going to be a pain if we need to deal with
> builds from partners etc, so I'd prefer a simple regexp. Here is the thing
> though: should a langpack created for 2.2 work with 2.2-x ? And vice-versa ?

I think we can consider making this work in one direction:  fxos_version=2.2-x could return 2.2 langpacks, too.  If we forbid this, we'd need to make sure version numbers in partner builds only add the suffix when localizations are modified.  Zibi, what do you think?

(Ideally, partner builds should not come with changes to localizations and should then be allowed to use vanilla versions.)

Comment 5

[Zibi Braniecki [:zbraniecki][:gandalf]]

I'd prefer to start with strict matches where we don't show any matching langpacks for partner builds who use custom build versions unless they provide custom langpacks.

I'd prefer that over accidentally providing incomplete langpacks.

We could even maybe add a pref for partners to choose langpack "channel"?

Comment 6

[Staś Małolepszy :stas]

In bug 1202407 I'd like to rename moz.b2g.version to something more langpack-specific, which would effectively be the name of a langpack "channel", as you suggested.  Maybe we should even use the word "channel".

If the meaning of this settings is clear, the current implementation in the Marketplace should be sufficient:  asking for 2.5-x would only return 2.5-x langpacks.  Hopefully, this would encourage partners to stay on the official langpack channel.

My personal preference would be to go for a regexp, but I also see benefits of having strict control here.  Mat, if we stick to hardcoding strict version matches, what's the update frequency of app-validator? How much in advance would we need to know that a new version needs to be accepted?

Comment 7

[Mathieu Pillard [:mat]]

The app-validator is updated by simply bumping a requirement in mozilla/zamboni, which is pushed weekly (every Tuesday).

Comment 8

[Mathieu Pillard [:mat]]

Replied too quickly: we tag for the push on Friday (leaving a couple days for QA to verify there are no big regressions) so ideally, we'd need roughly a one week warning for validator changes.

Comment 9

[Staś Małolepszy :stas]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #5)
> I'd prefer to start with strict matches where we don't show any matching
> langpacks for partner builds who use custom build versions unless they
> provide custom langpacks.

> I'd prefer that over accidentally providing incomplete langpacks.

Agreed.  Let's start with 2.5-x only returning 2.5-x langpacks.

As far as validation goes, we still have control over who can submit new langpacks.  If a partner sets langpack.channel to something non-standard, they still need to have permissions to upload corresponding langpacks to the Marketplace.

Zibi, do you think that's enough for now?  I'm a bit afraid of the manual labor required to enable vanilla langpacks for new versions of Gaia.  I think I'd prefer the regexp approach.  What's your opinion on this?

Comment 10

[Zibi Braniecki [:zbraniecki][:gandalf]]

Good point. wfm!

Comment 11

[Staś Małolepszy :stas]

Attachment: Pull request

This regular expression will match the following versions:

  2.5
  2.10
  2.3-p
  2.10-partner
  2.5-partner_2_5_6

I'll add tests today and request reviews.

Comment 12

[Staś Małolepszy :stas]

Comment on attachment 8661110 [details] [review]
Pull request

Mat, how does this look to you?

Comment 13

[Mathieu Pillard [:mat]]

https://github.com/mozilla/app-validator/commit/fe85a97294bb9c84c10b81847fea08e7420f6481
https://github.com/mozilla/zamboni/commit/1f4aa85fcaba35a3d7e43c953017d6858d86a691

Will land on dev in a few minutes, on stage later this week, in prod next Tuesday.

Comment 14

[Mathieu Pillard [:mat]]

Wrong commit for app-validator, it's actually https://github.com/mozilla/app-validator/commit/aac7d5b252459d08f24df882b27ff5cdbc23b8d2

Comment 15

[Staś Małolepszy :stas]

Thanks for the help, closing as fixed.

Comment 16

[Victor Carciu]

Can you please add some STRs to this bug or mark it as [qa-]?

Comment 17

[Staś Małolepszy :stas]

We don't currently have partner langpacks to test and I think the unit test should be enough for now.