Update nightly builds to use Golang pseudo-version by thaJeztah · Pull Request #125 · docker/docker-ce-packaging

thaJeztah · 2018-07-19T22:12:43Z

based on golang's pseudo-version: https://groups.google.com/forum/#!topic/golang-dev/a5PqQuBljF4

using a "pseudo-version" of the form v0.0.0-yyyymmddhhmmss-abcdefabcdef,
where the time is the commit time in UTC and the final suffix is the prefix
of the commit hash. The time portion ensures that two pseudo-versions can
be compared to determine which happened later, the commit hash identifes
the underlying commit, and the v0.0.0- prefix identifies the pseudo-version
as a pre-release before version v0.0.0, so that the go command prefers any
tagged release over any pseudo-version.

thaJeztah · 2018-07-19T22:14:01Z

deb/gen-deb-ver


-DATE_COMMAND="date"
-if [[ $(uname) -eq "Darwin" ]]; then
-    DATE_COMMAND="docker run --rm alpine date"


Looks like this was because macOS date doesn't have --date="string"?

I replaced this for echo "datestring" | xargs date -d ....

Argh; looks like that doesn't work on macOS after all 😕

thaJeztah · 2018-07-19T22:17:39Z

deb/gen-deb-ver

+    # the underlying commit, and the v0.0.0- prefix identifies the pseudo-version
+    # as a pre-release before version v0.0.0, so that the go command prefers any
+    # tagged release over any pseudo-version.
+    gitUnix="$($GIT_COMMAND log -1 --pretty='%ct')"


note: changed %at (author date) to %ct (committer date) - author date can be days in the past, i.e., not matching the date that the commit was merged

thaJeztah · 2018-07-19T22:18:19Z

deb/gen-deb-ver

+    gitDate="$(echo @${gitUnix} | xargs date +'%Y%m%d%H%M%S' -u -d)"
    gitCommit="$($GIT_COMMAND log -1 --pretty='%h')"
-    debVersion="0.${gitDate}-${gitCommit}"
+    debVersion="0.0.0-${gitDate}-${gitCommit}"


IIUC; both deb and rpm don't expect the v prefix

thaJeztah · 2018-07-19T22:18:59Z

deb/gen-deb-ver

+    # $ dpkg --compare-versions 18.06.0-ce-rc3 gt 18.06.0-ce-rc2  && echo true || echo false
+    # true
+    # $ dpkg --compare-versions 18.06.0-ce gt 18.06.0-ce-rc2  && echo true || echo false
+    # false


Noticed this one; curious how we resolved that

We add some magic sorting numbers at around line 30

I honestly think we should just remove these comments in general, they haven't really been relevant for a while.

Yeah, I ran them to verify the new version did what we expected

We should put this all in a function (looks like it's basically the same in deb, rpm and static) .. but that's for a follow-up

I guess the comments are misleading since they don't accurately reflect the final outcome, but I like the idea of being able to see the functionality.

thaJeztah · 2018-07-19T22:20:29Z

ping @seemethere @andrewhsu PTAL (please double-check; I tested it out-of-band in a shell)

based on golang's pseudo-version: https://groups.google.com/forum/#!topic/golang-dev/a5PqQuBljF4 > using a "pseudo-version" of the form v0.0.0-yyyymmddhhmmss-abcdefabcdef, > where the time is the commit time in UTC and the final suffix is the prefix > of the commit hash. The time portion ensures that two pseudo-versions can > be compared to determine which happened later, the commit hash identifes > the underlying commit, and the v0.0.0- prefix identifies the pseudo-version > as a pre-release before version v0.0.0, so that the go command prefers any > tagged release over any pseudo-version. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

seemethere

LGTM, I'll need to check with our nightly build reaper to see if it works correctly with that but as far as this PR goes I think it should work.

seemethere · 2018-07-19T22:38:09Z

P.S. I love your feature branch name @thaJeztah ❤️

thaJeztah · 2018-07-19T22:46:27Z

P.S. I love your feature branch name @thaJeztah ❤️

LOL, should see some of my other ones 😂

jose-bigio

I am wondering what benefit we will have from using a pseudo-version?

From my understanding the benefit of a pseudo-version is that it would allow people to go get modules in a better fashion. However, this is changing deb and rpm packages to use pseudo-version.

Does this somehow make it easier for people to consume the nightly packages?

jose-bigio · 2018-07-20T17:20:31Z

In response to my own comment catching up on:
https://github.com/docker/release-packaging/issues/181#issuecomment-406405743

to better understand the need for this change.

I was even cced on the discussion 🤦‍♂️

jose-bigio · 2018-07-20T17:29:04Z

After reading the discussion my understanding is that we want to make this change because it could become a standard.

I haven't really given much thought to whether one versioning scheme (the original) is better than the other, but am fine either way.
I understand the timeliness of this change, but want to point out that it seems one of the motivations for the change seems to be based on the assumption that this could become the standard.

My apologies for discussing this a little late, but wanted to talk about it before we brought it in.

thaJeztah · 2018-07-20T17:52:17Z

I think the big advantage is that it's valid semver and provides all the information that's needed (time committed, git sha); in addition to something becoming more of a standard (instead of a self-defined format)

thaJeztah · 2018-07-20T17:53:51Z

If we would not take this change; we should still merge the fix for the right commit date; currently I think it's possible we go back in time (ie a PR merged that was rebased, but authored a long time ago)

jose-bigio

LGTM

thaJeztah · 2018-07-24T12:13:18Z

documentation updates in docker/docs#7090

thaJeztah commented Jul 19, 2018

View reviewed changes

thaJeztah force-pushed the a_nightly_to_remember branch from ee54823 to d2baf8f Compare July 19, 2018 22:28

seemethere requested review from dhiltgen and seemethere July 19, 2018 22:32

seemethere approved these changes Jul 19, 2018

View reviewed changes

seemethere requested a review from a team July 20, 2018 16:09

jose-bigio reviewed Jul 20, 2018

View reviewed changes

jose-bigio approved these changes Jul 20, 2018

View reviewed changes

docker-unir bot merged commit a5aa7e5 into docker:master Jul 20, 2018

thaJeztah deleted the a_nightly_to_remember branch July 20, 2018 18:12

thaJeztah mentioned this pull request Jul 24, 2018

Update version-format for nightly builds docker/docs#7090

Merged

thaJeztah mentioned this pull request Oct 1, 2019

Release containerd for multiple architectures containerd/containerd#3702

Closed

thaJeztah mentioned this pull request Dec 23, 2019

Remove build time in binary docker-archive-public/docker.app#787

Merged

Conversation

thaJeztah commented Jul 19, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seemethere Jul 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Jul 19, 2018

Uh oh!

seemethere left a comment

Choose a reason for hiding this comment

Uh oh!

seemethere commented Jul 19, 2018

Uh oh!

thaJeztah commented Jul 19, 2018

Uh oh!

jose-bigio left a comment

Choose a reason for hiding this comment

Uh oh!

jose-bigio commented Jul 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jose-bigio commented Jul 20, 2018

Uh oh!

thaJeztah commented Jul 20, 2018

Uh oh!

thaJeztah commented Jul 20, 2018

Uh oh!

jose-bigio left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Jul 24, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

seemethere Jul 19, 2018 •

edited

Loading

jose-bigio commented Jul 20, 2018 •

edited

Loading