Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

[Bug] Generated version codes do not match the intended format #14031

Closed
jklukas opened this issue Aug 21, 2020 · 4 comments
Closed

[Bug] Generated version codes do not match the intended format #14031

jklukas opened this issue Aug 21, 2020 · 4 comments
Labels
🐞 bug Crashes, Something isn't working, .. needs:triage Issue needs triage

Comments

@jklukas
Copy link
Contributor

jklukas commented Aug 21, 2020

Steps to reproduce

The SimpleDateFormat invocation in Config.kt is incorrect. The format string should be yyyyMMddHHmmss. The current version uses all caps, but casing is important. In particular, SimpleDateFormat docs indicate that D is associated with the day number in the year rather than the month, and that M gives month while m gives minutes. We may have builds with minute values like 58 being treated as adding 58 months.

Expected behavior

It should be possible to take a 10-digit app_build string and extract the date on which the build was produced.

Actual behavior

Following the documented logic, the extracted values are in the future.

Builds of Fenix over the past month are reporting telemetry with app_build set to a 10-digit numeric string. A recent value is "2015757667". If you parse a date out of this value using the documented format, you get a date in the future: '2021-03-17'.

Note that the current logic breaks the assumption that build numbers will always increase. They may currently be unstable according to the minute on which builds are produced. On the first of the year, we may also see the number jump down. It's unclear to me exactly how the different parts are being added together. Thankfully, the formatter is used only to parse the one constant value "20150801000000". So there's a constant offset here, and build numbers should reliably increase.

I'm interested in this problem from the perspective of analyzing telemetry data in BigQuery and being able to reliable determine the date of a given build. Here's some example BigQuery code that I used to determine the date associated with "2015757667":

with s as (select as value '2015757667')
select datetime_add(datetime '2015-08-01 00:00:00', interval (cast(s as int64) << (64 - 20) >> (64 - 20 + 3)) hour) from s
-- Returns 2021-03-17 04:00:00

It's unclear to me what the implications of this problem are for future builds. I'm happy to help from the data side, such as determining the maximum date that's been reported so far under the current scheme.

Device information

  • Android device: N/A
  • Fenix version: Builds since approximately early August

┆Issue is synchronized with this Jira Task

@jklukas jklukas added the 🐞 bug Crashes, Something isn't working, .. label Aug 21, 2020
@github-actions github-actions bot added the needs:triage Issue needs triage label Aug 21, 2020
@jklukas
Copy link
Contributor Author

jklukas commented Aug 21, 2020

The existing code:

        val format = SimpleDateFormat("YYYYMMDDHHMMSS", Locale.US)
        val cutoff = format.parse("20150801000000")

produces a cutoff value of "Sun Dec 28 00:00:00 UTC 2014", so our effective epoch is midnight beginning 2014-12-28 rather than midnight beginning 2015-08-01.

As described in the code comments:

This yields a little under 15 years worth of hourly build identifiers, since 2**17 / (366 * 24) =~ 14.92.

With the epoch set to 2014-12-28, we'll max out the 17 bits on 2029-12-10, which still seems like plenty of headroom.

Adjusting the epoch up to 2015-08-01 to match Fennec would be problematic because it would cause build id values to jump backwards.

@kbrosnan
Copy link
Contributor

There may be some behavior that was copied from bz-1137898

@jklukas
Copy link
Contributor Author

jklukas commented Aug 25, 2020

cc @pocmo

@jklukas
Copy link
Contributor Author

jklukas commented Sep 10, 2020

I chatted with @pocmo and we agreed that causing build numbers to decrease by correcting the epoch isn't an option and that sticking with the currently implemented behavior seems like the way to go. Android is evolving support for 64-bit version codes, so we even have a path forward for what to do in 2029.

I opened #14952 to update the code and commentary to explicitly document the adjusted epoch value.

@jklukas jklukas closed this as completed Sep 11, 2020
abhijitvalluri pushed a commit to abhijitvalluri/firefox2 that referenced this issue Sep 11, 2020
abhijitvalluri pushed a commit to fork-maintainers/iceraven-browser that referenced this issue Sep 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🐞 bug Crashes, Something isn't working, .. needs:triage Issue needs triage
Projects
None yet
Development

No branches or pull requests

2 participants