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.

Sign up for GitHub

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

Jump to bottom

bug: Fix install from source will lose version information #1024

Merged
merged 7 commits into from Oct 14, 2022
Merged

bug: Fix install from source will lose version information #1024

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ca7f3aa
fix
VoVAllen Oct 13, 2022
93d13a0
lint
VoVAllen Oct 13, 2022
a7a10ce
lint
VoVAllen Oct 13, 2022
f8f1dcf
add
VoVAllen Oct 14, 2022
78f3b74
fix
VoVAllen Oct 14, 2022
924175b
fix
VoVAllen Oct 14, 2022
e780da2
fix
VoVAllen Oct 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Expand Up @@ -65,5 +65,6 @@ share/python-wheels/
.installed.cfg
*.egg
MANIFEST
.GIT_TAG_INFO

test.tar
1 change: 1 addition & 0 deletions MANIFEST.in
View file
Open in desktop
Expand Up @@ -4,6 +4,7 @@ include Makefile
include .goreleaser.yaml .goreleaser/*.Dockerfile
include pkg/* cmd/*
include go.mod go.sum
include .GIT_TAG_INFO
prune examples
prune docs
prune .github
3 changes: 2 additions & 1 deletion Makefile
View file
Open in desktop
Expand Up @@ -71,7 +71,7 @@ BUILD_DIR := ./build
VERSION ?= $(shell git describe --match 'v[0-9]*' --always --tags --abbrev=0)
BUILD_DATE=$(shell date -u +'%Y-%m-%dT%H:%M:%SZ')
GIT_COMMIT=$(shell git rev-parse HEAD)
GIT_TAG=$(shell if [ -z "`git status --porcelain`" ]; then git describe --exact-match --tags HEAD 2>/dev/null; fi)
GIT_TAG ?= $(shell if [ -z "`git status --porcelain`" ]; then git describe --exact-match --tags HEAD 2>/dev/null; fi)
GIT_TREE_STATE=$(shell if [ -z "`git status --porcelain`" ]; then echo "clean" ; else echo "dirty"; fi)
GITSHA ?= $(shell git rev-parse --short HEAD)
GIT_LATEST_TAG ?= $(shell git describe --tags --abbrev=0)
Expand Down Expand Up @@ -110,6 +110,7 @@ build-release:
-X $(ROOT)/pkg/version.gitTag=$(GIT_TAG)" \
$(CMD_DIR)/$${target}; \
done
@[[ ! -z "$(GIT_TAG)" ]] && echo "$(GIT_TAG)" > .GIT_TAG_INFO
VoVAllen marked this conversation as resolved.
Show resolved Hide resolved

help: ## Display this help
@awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n make \033[36m<target>\033[0m\n"} /^[a-zA-Z0-9_-]+:.*?##/ { printf " \033[36m%-15s\033[0m %s\n", $$1, $$2 } /^##@/ { printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST)
Expand Down
10 changes: 9 additions & 1 deletion setup.py
View file
Open in desktop
Expand Up @@ -26,7 +26,15 @@
def build_envd_if_not_found():
if not os.path.exists("bin/envd"):
logging.info("envd not found. Build from scratch")
errno = subprocess.call(["make", "build-release"])
try:
with open(".GIT_TAG_INFO") as f:
Copy link
Member

Choose a reason for hiding this comment

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

Where do we write the file? Is it maintained by us?

Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/tensorchord/envd/pull/1024/files#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52R113
Seems like here. Newly added.

Copy link
Contributor

Choose a reason for hiding this comment

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

However, I didn't get the whole logic. Can you explain the design and execution order? Thanks! @VoVAllen

Copy link
Member Author

Choose a reason for hiding this comment

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

On macos arm, it built from source without git info. But the whole logic related to version depends on git tag. Missing git tag results in the bug. Thus here we manually save git tag info here

Copy link
Contributor

@dragonly dragonly Oct 14, 2022 •
edited

Choose a reason for hiding this comment

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

nit: It's better to add comments here to explain the necessity of the code block. If one day macOS pre-built binary is released, this can be removed by consulting the comment here.

logging.info("Use build_tag from envd._version")
tag = f.read().strip()
errno = subprocess.call(
["make", "build-release", "GIT_TAG={}".format(tag)]
)
except:
errno = subprocess.call(["make", "build-release"])
assert errno == 0, "Failed to build envd"


Expand Down