1510431 - buildhub.json should use (or at least include) display version, not (just) version

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[William Lachance (:wlach) (use needinfo!)]

For buildhub to be useful for tools like crashstop and mission control for Firefox betas, it really needs us to provide us with the displayVersion -- which includes the bXX flag next to it.

It appears that currently, buildhub.json (to be used by the next iteration of buildhub) only includes the "version" field:

https://archive.mozilla.org/pub/firefox/candidates/64.0b12-candidates/build1/win64/en-US/buildhub.json

Note that "version" here only says "64.0" even though this is really "64.0b12".

The whole displayVersion vs. version thing is IMO needlessly confusing and annoying -- in an ideal world, we would do away with it entirely. That's probably complicated and hard to do, so I won't propose it here. However, for the purposes of buildhub.json, I think we can just use the value of displayVersion as version and I think we'll be fine. buildhub as it stands now uses the displayVersion (as it should)

:calixte -- I understand you were investigating this area but wasn't sure how far along you were. I figured it couldn't hurt to just file a bug (feel free to dupe it if there's one already, I couldn't find it).

Comment 1

[William Lachance (:wlach) (use needinfo!)]

I suspect we can just do something like this here:

  'version': s['MOZ_APP_VERSION'], => 'version': s['MOZ_APP_VERSION_DISPLAY'] or ['MOZ_APP_VERSION'],

https://searchfox.org/mozilla-central/rev/0859e6b10fb901875c80de8f8fc33cbb77b2505e/toolkit/mozapps/installer/informulate.py#82

MOZ_APP_VERSION_DISPLAY seems to always be defined from what I can tell but it doesn't hurt to be double careful.

Comment 2

[Calixte Denizet (:calixte)]

:wlach, I'm fine to replace version (64.0) by displayed version (64.0b12).
But that means that buildhub v2 will have two different kind of values (version vs displayed version) in the database.
An other way to get the displayed version is to get it from the url (available in buildhub.json):
"https://archive.mozilla.org/pub/firefox/candidates/64.0b12-candidates/build1/win64/en-US/Firefox%20Setup%2064.0b12.exe"

I don't know if we can rely on the application name (Firefox...64.0b12.exe).
Anyway, maybe we should add a field 'displayed_version' to avoid to have backward compatibility issues.
:peterbe, wdyt ?

Comment 3

[William Lachance (:wlach) (use needinfo!)]

In reply to Calixte Denizet (:calixte) from comment #2)
> :wlach, I'm fine to replace version (64.0) by displayed version (64.0b12).
> But that means that buildhub v2 will have two different kind of values
> (version vs displayed version) in the database.

Whether we make version == displayVersion or just include displayVerison by itself, I think we would just have buildhub ingest the value that corresponds to the display version. No reason to track anything else in buildhub IMO (we don't for buildhub1).

> An other way to get the displayed version is to get it from the url
> (available in buildhub.json):
> "https://archive.mozilla.org/pub/firefox/candidates/64.0b12-candidates/
> build1/win64/en-US/Firefox%20Setup%2064.0b12.exe"

I think getting this data from the URL is needlessly complex compared to just including it in buildhub.json.

Comment 4

[Peter Bengtsson [:peterbe]]

(In reply to Calixte Denizet (:calixte) from comment #2)
> :wlach, I'm fine to replace version (64.0) by displayed version (64.0b12).
> But that means that buildhub v2 will have two different kind of values
> (version vs displayed version) in the database.

Either that was a typo or you're referring to a historic glitch we'd have to live with. As of today, we have prod data in Buildhub2 that says "64.0" (when, as per this bug, it should say "64.0b12") without any "64.0b12" information.

> An other way to get the displayed version is to get it from the url
> (available in buildhub.json):
> "https://archive.mozilla.org/pub/firefox/candidates/64.0b12-candidates/
> build1/win64/en-US/Firefox%20Setup%2064.0b12.exe"
> 
> I don't know if we can rely on the application name (Firefox...64.0b12.exe).
> Anyway, maybe we should add a field 'displayed_version' to avoid to have
> backward compatibility issues.
> :peterbe, wdyt ?

To sound as vague as I possibly can; I don't care. If there's *value* in the "64.0" string (to avoid having to do `plain_version = version.split('b')[0]` in the apps) then we should definitely have BOTH "version" and "display_version". 
However, the only people I know who use and care about Buildhub are you guys plus Socorro (sorta') and if you guys don't need/care about the "64.0" then we should *replace* the version string with the display version. ...in mozilla-central.

Comment 5

[William Lachance (:wlach) (use needinfo!)]

Attachment: Bug 1510431 - Use display version for version field in buildhub.json r?gps

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/75c7af226bcf
Use display version for version field in buildhub.json r=gps

Comment 7

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/75c7af226bcf

Comment 8

[Peter Bengtsson [:peterbe]]

Pardon my ignorance of release process but this landed in mozilla-central 5 days ago but https://archive.mozilla.org/pub/firefox/candidates/64.0b14-candidates/build1/mac/en-US/buildhub.json does not have the right target.version value. It's "64.0" when it should be "64.0b14" if I understand wlach's intention with the patch. 

Does this mean the patch hasn't "taken effect" yet in TaskCluster or does it mean the code ran but didn't work as intended?

Comment 9

[William Lachance (:wlach) (use needinfo!)]

(In reply to Peter Bengtsson [:peterbe] from comment #8)
> Pardon my ignorance of release process but this landed in mozilla-central 5
> days ago but
> https://archive.mozilla.org/pub/firefox/candidates/64.0b14-candidates/build1/
> mac/en-US/buildhub.json does not have the right target.version value. It's
> "64.0" when it should be "64.0b14" if I understand wlach's intention with
> the patch. 
> 
> Does this mean the patch hasn't "taken effect" yet in TaskCluster or does it
> mean the code ran but didn't work as intended?

This landed in 65, so I wouldn't expect to see this take effect before the beta 65 cycle.

Comment 10

[William Lachance (:wlach) (use needinfo!)]

65 beta 3 has version set to 65.0b3 as expected:

https://archive.mozilla.org/pub/firefox/candidates/65.0b3-candidates/build2/win64/en-US/buildhub.json