Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build: include commit hash & version #2453

Closed
wants to merge 23 commits into from
Closed

build: include commit hash & version #2453

wants to merge 23 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Mar 7, 2016

(partially) fixes #2304

New script bin/genversion generates SemVer-compatible string out of closest git tag. The script is called in Makefile to pass IPFS version and commit info as linker arguments.
Dockerfiles got optional build argument IPFS_VERSION to avoid including .git/ into the image.

License: MIT
Signed-off-by: hutenosa hutenosa@mm.st

@Kubuxu
Copy link
Member

Kubuxu commented Mar 8, 2016

IIRC there was API version comparison somewhere, IMO it should work among the same pre-release, different commit metadata. You can try it by building, starting daemon, adding commit, building, trying to use ipfs files ls /.

@ghost
Copy link
Author

ghost commented Mar 8, 2016

Travis uses quite outdated docker (1.8.2) which doesn't support build arguments (appeared in 1.9.0). I was unable to upgrade docker in Travis environment so I've just replaced ARG with ENV in both Dockerfiles. Drawback: docker builds will all have hardcoded version 0.4.0-dev unless explicitly changed.

Once Travis refreshes its docker (they promised to keep it updated), we can go back to build arguments and pass the version via cmdline: docker build --build-arg IPFS_VERSION=...

@Kubuxu I checked, client and server versions should match and it's about version tags like 0.4.0-rc2. Commit metadata can be different. Since now version is SemVer-compatible string, we may soften the condition and compare only major versions starting from 1.0.0

@ghost ghost changed the title deploy: includes commit hash & version build: include commit hash & version Mar 9, 2016
@Kubuxu
Copy link
Member

Kubuxu commented Mar 9, 2016

@ghost
Copy link
Author

ghost commented Mar 9, 2016

config.ApiVersion contains only the version number and not the current commit: https://github.com/hutenosa/go-ipfs/blob/fix/version-commit-hash/repo/config/version.go#L18

However, client and daemon with different version number / pre-release will not be able to communicate.

@Kubuxu
Copy link
Member

Kubuxu commented Mar 9, 2016

Right, sorry for trouble.

Go/Bash LGTM, but I can't say anything about docker.

@ghost
Copy link
Author

ghost commented Mar 9, 2016

@lgierth Are you fine with Dockerfile changes?

There's an important one: to build an image from source code, make docker should be called instead of mere docker build. I didn't find another way to pass the version to the container.

Docker 1.9+ is now required. A workaround for Travis is created.

atomgardner and others added 23 commits April 2, 2016 19:19
License: MIT
Signed-off-by: Thomas Gardner <tmg@fastmail.com>
License: MIT
Signed-off-by: Thomas Gardner <tmg@fastmail.com>
License: MIT
Signed-off-by: Thomas Gardner <tmg@fastmail.com>
License: MIT
Signed-off-by: David Dias <daviddias.p@gmail.com>
License: MIT
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
IPFS version is taken from git describe output.
Done for Makefile and Dockerfile

License: MIT
Signed-off-by: hutenosa <hutenosa@mm.st>
fixed bug in main Makefile
added make target 'docker'
travis docker workaround

License: MIT
Signed-off-by: hutenosa <hutenosa@mm.st>
License: MIT
Signed-off-by: dignifiedquire <dignifiedquire@gmail.com>
License: MIT
Signed-off-by: Jeromy Johnson <why@ipfs.io>
License: MIT
Signed-off-by: Jeromy Johnson <why@ipfs.io>
License: MIT
Signed-off-by: Richard Littauer <richard.littauer@gmail.com>
(not green, but at least it executes)

License: MIT
Signed-off-by: Tarnay Kálmán <kalmisoft@gmail.com>
License: MIT
Signed-off-by: Richard Littauer <richard.littauer@gmail.com>
License: MIT
Signed-off-by: Lars Gierth <larsg@systemli.org>
License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
License: MIT
Signed-off-by: Michael Muré <batolettre@gmail.com>
License: MIT
Signed-off-by: Michael Muré <batolettre@gmail.com>
License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
License: MIT
Signed-off-by: Jeromy <jeromyj@gmail.com>
This way, we don't claim to pin when unpinning.

License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
…ype`

License: MIT
Signed-off-by: Steven Allen <steven@stebalien.com>
@whyrusleeping
Copy link
Member

@hutenosa could you rebase these changes on the lastest master? I'll also poke @lgierth to have him take a look at the dockerfile changes.

@whyrusleeping whyrusleeping added the need/author-input Needs input from the original author label May 14, 2016
// CurrentCommit is the current git commit, this is set as a ldflag in the Makefile
var CurrentCommit string

// CurrentVersionNumber is the current application's version literal
const CurrentVersionNumber = "0.4.0-dev"
var CurrentVersionNumber = "0.0.0-dev"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CurrentVersionNumber should stay as it is and not carry commit information -- it's used in too many places where the assumption is a simple vN.N.N string. Maybe just use CurrentCommit?

@ghost
Copy link

ghost commented May 18, 2016

Hey @hutenosa I'm so sorry for letting this sit so long. I like this PR very much!

I think this can be simplified a bit, by not touching CurrentVersionNumber,
and instead introducing a "full version" as well as deprecating "current commit".

  • leave CurrentVersionNumber (and ApiVersion) untouched
  • FullVersion, passed as IPFS_FULL_VERSION
  • add FullVersion to /api/v0/version
  • add ipfs version --full option
  • add Full Version to :8080/version
  • deprecate/remove mentions of CurrentCommit
    • deprecate ipfs version --commit
    • deprecate Commit in /api/v0/version
    • remove Commit in :8080/version

I can help you out with a few of these any time.

Regarding git, I haven't found a way to use git describe without a full repo,
so passing the full version via docker is okay.

@ghost
Copy link
Author

ghost commented May 26, 2016

I'm closing this PR since I deleted the forked repository. Unfortunately, github has no means to rebind the PR to a new fork. I'll resubmit a new one shortly.

@ghost ghost closed this May 26, 2016
@ghost ghost mentioned this pull request May 26, 2016
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/author-input Needs input from the original author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Include commit hash or tag in version string
10 participants