Skip to content

🏗 Generate a consistent version number when building/disting#16631

Merged
erwinmombay merged 2 commits intoampproject:masterfrom
danielrozenberg:consistent-version-numbers
Aug 30, 2018
Merged

🏗 Generate a consistent version number when building/disting#16631
erwinmombay merged 2 commits intoampproject:masterfrom
danielrozenberg:consistent-version-numbers

Conversation

@danielrozenberg
Copy link
Copy Markdown
Member

@choumx and I discussed the idea of changing the way we generate version numbers from relying on the timestamp of when the user hit enter on gulp [build|dist] to have a consistent version number, so that running gulp [build|dist] on the exact same commit would create the exact same output.

Our current version numbers are 13 digits long, where the 13 digits represent a timestamp in milliseconds.

My proposal is this formula:

`${ten-digits-commit-timestamp-in-seconds}${unique-identifier-derived-from-commit-hash}`

For the first 10 digits we continue to use the timestamp, which we take from the commit time (not author time.) Git only stores the seconds, not milliseconds, so we cannot rely on a 13 digits timestamp as the version number.

For the last 3 digits we take the short commit hash (e.g., a2a6e4ad5) and convert it back into a number, then modulo 1,000 to get 3 digits, then left-pad with 0's. This is to avoid the (highly unlikely) case in which two commits were committed at the exact same second.

If there are any uncommitted files in the working directory (staged or otherwise,) the last 3 digits will be replaced with 000, to indicate that.

This PR is a proof-of-concept and a place to start a discussion around this. What do you all think?

@cramforce
Copy link
Copy Markdown
Member

I like it. This maintains the property that later releases have higher "timestamps", right?

@danielrozenberg
Copy link
Copy Markdown
Member Author

@cramforce other than the fact that this PoC doesn't work (gotta work out some async kinks,) the answer is yes-modulo-1-sec. The module-1-sec is extremely unlikely in how we create releases and release branches.

e.g., if we this is the tree:

[square brackets = branch name]
(parentheses = commit hash + timestamp]

[release-1]                                                       [master]
(aaaa + 1000) ------- (bbbb + 2000) ------- (cccc + 3000) ------- (dddd + 4000)
          \
           \
            \        [release-2]
             ------- (cccc* + 3500)

cccc* is a cherry pick of the cccc, it has a different timestamp because it was committed
again on a different branch.

The release created from release-1 will have a release number of 1000???, and release-2 will have 3500???.

The only case where the release number won't be higher is if, say, we were to cherry pick cccc onto release-2 after timestamp 4000, and then create a release starting at commit dddd, e.g.,:

                                                                  [master] + [release-3]
(aaaa + 1000) ------- (bbbb + 2000) ------- (cccc + 3000) ------- (dddd + 4000)
          \
           \
            \        [release-2]
             ------- (cccc* + 4500)

In this case release-2 will have release number of 4500??? and release-3 will have release number of 4000???.

Of course right now if you were to run gulp dist on release-3 at timestamp 4000, and gulp dist on release-2 at timestamp 4500, you would achieve the same thing.

As for the "modulo-1-sec" issue, if someone were to cherry-pick a commit onto a release branch at the exact same second that someone else committed a commit to another release branch, there's no guarantee of order between the two, e.g.:

                                                                  [master] + [release-3]
(aaaa + 1000) ------- (bbbb + 2000) ------- (cccc + 3000) ------- (dddd + 4000)
          \
           \
            \        [release-2]
             ------- (cccc* + 4000)

In this case release-2 and release-3 will both have a release number of 4000???, where the ??? part is derived from the commit hash. I don't think we should be worried at all about this case.

@danielrozenberg danielrozenberg force-pushed the consistent-version-numbers branch from 34d9485 to cc44e6d Compare July 11, 2018 20:42
@codecov-io
Copy link
Copy Markdown

codecov-io commented Jul 11, 2018

Codecov Report

Merging #16631 into master will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #16631      +/-   ##
==========================================
+ Coverage   77.82%   77.83%   +0.01%     
==========================================
  Files         562      562              
  Lines       41170    41170              
==========================================
+ Hits        32039    32046       +7     
+ Misses       9131     9124       -7
Flag Coverage Δ
#integration_tests 36.15% <ø> (-0.04%) ⬇️
#unit_tests 76.9% <ø> (+0.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a7ca569...c254306. Read the comment docs.

@dreamofabear
Copy link
Copy Markdown

Using commit time should work for cherry-picks across multiple release branches and custom amphtml forks. Schedule for next design review?

@danielrozenberg
Copy link
Copy Markdown
Member Author

From design review discussion: @cramforce wants to change the version numbers to be human readable, instead of seconds-since-epoch - e.g., 1808011146120 would be the version built from a commit with time 2018-08-01 11:46:12. The last digit, to maintain the 13 digits version number, could be a binary indicator for tree cleanliness as above (0 = clean tree; 1 = uncommited changes)

As we're making this change now, in the year 2018, version numbers will start with 18, which is higher than the current version prefix of 15. This versioning scheme will continue to have increasing numbers until the end of the year 2099 :)

/cc commentators in the design review meeting: @cramforce @jridgewell @erwinmombay @rsimha @choumx (apologies if I forgot someone)

@danielrozenberg danielrozenberg force-pushed the consistent-version-numbers branch from cc44e6d to ccc558a Compare August 1, 2018 18:39
@danielrozenberg danielrozenberg changed the title [DO NOT MERGE] 🏗 Generate a consistent version number when building/disting 🏗 Generate a consistent version number when building/disting Aug 1, 2018
@danielrozenberg
Copy link
Copy Markdown
Member Author

PTAL

@danielrozenberg danielrozenberg force-pushed the consistent-version-numbers branch from c254306 to 7030137 Compare August 29, 2018 18:43
@danielrozenberg
Copy link
Copy Markdown
Member Author

Rebased on master, PTAL

@erwinmombay erwinmombay merged commit f2af186 into ampproject:master Aug 30, 2018
@danielrozenberg danielrozenberg deleted the consistent-version-numbers branch August 30, 2018 15:45
@rsimha
Copy link
Copy Markdown
Contributor

rsimha commented Sep 4, 2018

Delayed "yay!" :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes INTENT TO IMPLEMENT Proposes implementation of a significant new feature. https://bit.ly/amp-contribute-code Type: Feature Request WG: infra

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants