Skip to content
This repository has been archived by the owner on Dec 1, 2022. It is now read-only.

Fix version issue of storage and meta binary #273

Merged
merged 10 commits into from
Dec 31, 2020
Merged

Fix version issue of storage and meta binary #273

merged 10 commits into from
Dec 31, 2020

Conversation

yixinglu
Copy link
Contributor

@yixinglu yixinglu commented Dec 29, 2020 •
edited
Loading

panda-sheep
panda-sheep previously approved these changes Dec 30, 2020
View reviewed changes
@critical27
Copy link
Contributor

critical27 commented Dec 30, 2020 •
edited
Loading

show hosts meta/graph/storage previously will print the git info sha, so it won't work if we don't package?
--version? BTW, there is another http will show the sha.

@yixinglu
Copy link
Contributor Author

show hosts meta/graph/storage previously will print the git info sha, so it won't work if we don't package?
--version? BTW, there is another http will show the sha.

previous git sha is common commit hash, not the storage repo commit info, since the cmake target defined in common repo. And if you want to embed the git sha into storage binary locally, you can modify the cmake configure options like cmake -DGIT_INFO_SHA=$(git rev-parse --short HEAD) ... to do that.

@yixinglu yixinglu added the ready-for-testing PR: ready for the CI test label Dec 30, 2020
@yixinglu
Copy link
Contributor Author

Hold on this PR, I will check whether NEBULA_STRINGFY(GIT_INFO_SHA) is expected when not define GIT_INFO_SHA macro.

@critical27
Copy link
Contributor

Hold on this PR, I will check whether NEBULA_STRINGFY(GIT_INFO_SHA) is expected when not define GIT_INFO_SHA macro.

👌

@critical27
Copy link
Contributor

LGTM, we'd better add DGIT_INFO_SHA when package. Otherwise, storage and services will only print the NEBULA_BUILD_VERSION, which is set as graph SHA when packaging.

critical27
critical27 previously approved these changes Dec 30, 2020
View reviewed changes
@yixinglu yixinglu force-pushed the update-pkg branch 2 times, most recently from b5dd432 to 0636a24 Compare December 30, 2020 14:10
laura-ding
laura-ding reviewed Dec 31, 2020
View reviewed changes
src/version/CMakeLists.txt Outdated Show resolved Hide resolved
src/version/Version.cpp.in Outdated Show resolved Hide resolved
src/CMakeLists.txt Outdated Show resolved Hide resolved
@yixinglu yixinglu changed the title Set GIT_INFO_SHA only when packaging Fix version issue of storage and meta binary Dec 31, 2020
@yixinglu yixinglu requested a review from laura-ding December 31, 2020 08:42
critical27
critical27 reviewed Dec 31, 2020
View reviewed changes
src/version/Version.cpp.in
namespace storage {

std::string gitInfoSha() {
return "@GIT_INFO_SHA@";
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

critical27
critical27 approved these changes Dec 31, 2020
View reviewed changes
Copy link
Contributor

@critical27 critical27 left a comment

Choose a reason for hiding this comment

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

Good job, thx!

@critical27 critical27 merged commit 09270f5 into vesoft-inc:master Dec 31, 2020
@yixinglu yixinglu deleted the update-pkg branch December 31, 2020 09:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@laura-ding laura-ding laura-ding approved these changes

@critical27 critical27 critical27 approved these changes

@panda-sheep panda-sheep Awaiting requested review from panda-sheep

Assignees
No one assigned
Labels
ready-for-testing PR: ready for the CI test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@yixinglu @critical27 @laura-ding @panda-sheep