Closed Bug 1510431 Opened 2 years ago Closed 2 years ago

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

Categories

(Firefox Build System :: General, enhancement)

enhancement
Not set
normal

Tracking

(firefox65 fixed)

RESOLVED FIXED
mozilla65
Tracking Status
firefox65 --- fixed

People

(Reporter: wlach, Assigned: wlach)

Details

Attachments

(1 file)

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).
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.
: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 ?
Flags: needinfo?(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.

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.
(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.
Flags: needinfo?(peterbe)
Assignee: nobody → wlachance
Pushed by wlachance@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/75c7af226bcf
Use display version for version field in buildhub.json r=gps
https://hg.mozilla.org/mozilla-central/rev/75c7af226bcf
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 65
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?
Flags: needinfo?(wlachance)
(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.
Flags: needinfo?(wlachance)
Target Milestone: Firefox 65 → mozilla65
You need to log in before you can comment on or make changes to this bug.