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

chore: unite the bin cache dir for all projects in go.work #609

Merged
merged 9 commits into from
Jul 2, 2024
Merged

chore: unite the bin cache dir for all projects in go.work #609

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9c6b3ba
chore: ignore goland .idea dir
Chever-John Jun 25, 2024
7131b29
chore: finish the unite the bin cache dir for controller project
Chever-John Jun 25, 2024
8a1bbb3
chore: finish the unite the bin cache dir for e2e project
Chever-John Jun 25, 2024
f6be174
chore: update test.yml in .github
Chever-John Jun 26, 2024
3edbe29
chore(makefile): use realpath to replace the $LOCALBIN var
Chever-John Jun 30, 2024
53e855c
chore: update all makefile which use th e LOCALBIN var
Chever-John Jul 1, 2024
3c91675
chore: update istio localbin var
Chever-John Jul 2, 2024
6d5c547
chore: update site && Makefile
Chever-John Jul 2, 2024
76d4771
chore: remove duplicate SHELL var
Chever-John Jul 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,4 @@ external/*
e2e/log
# helm charts
manifests/charts/**/*.tgz
.idea
2 changes: 1 addition & 1 deletion controller/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ KIND ?= kind
##@ Build Dependencies

## Location to install dependencies to
LOCALBIN ?= $(shell pwd)/bin
LOCALBIN ?= $(shell cd .. && pwd)/bin
Copy link
Member

Choose a reason for hiding this comment

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

What about using

LOCALBIN ?= $(dir $(lastword $(MAKEFILE_LIST)))bin

in the common.mk?

Copy link
Member

Choose a reason for hiding this comment

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

@Chever-John
Would you give this a try? Thanks!

Copy link
Contributor Author

@Chever-John Chever-John Jun 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Sry for late~

Yes, I have tried it.

I added the following code (A) into the common.mk file and commented out the corresponding code in the controller/Makefile as shown below (B):

A. code

LOCALBIN ?= $(dir $(lastword $(MAKEFILE_LIST)))
$(LOCALBIN):
	@mkdir -p $(LOCALBIN)

B. code

comment out the LOCALBIN ?= $(shell cd .. && pwd)/bin

## Location to install dependencies to
#LOCALBIN ?= $(shell cd .. && pwd)/bin
# Remember to remove tools downloaded into bin directory manually before updating them.
# If they need to be updated frequently, we can consider to store them in the `Dockerfile.dev`.
$(LOCALBIN):
	mkdir -p $(LOCALBIN)

And I tried to run the command make sth in the htnn/controller dir as follows:

╭─cheverjohn at Jonathan’s MacBook Pro in ~/Workspace/OpenSource/github.com/Chever-John/htnngit:(chore/unite-the-bin-cache-dir*)
╰─± cd controller 
╭─cheverjohn at Jonathan’s MacBook Pro in ~/Workspace/OpenSource/github.com/Chever-John/htnn/controllergit:(chore/unite-the-bin-cache-dir*)
╰─± make controller-gen
test -s ..//controller-gen && ..//controller-gen --version | grep -q v0.13.0 || \
        GOBIN=../ go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.13.0
cannot install, GOBIN must be an absolute path
make: *** [..//controller-gen] Error 1

Then errors came out:)

Copy link
Contributor Author

@Chever-John Chever-John Jun 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

How about use the following code:

# common.mk
ROOT_DIR := $(shell dirname $(realpath $(firstword $(MAKEFILE_LIST))))
LOCALBIN := $(ROOT_DIR)/bin
$(LOCALBIN):
	@mkdir -p $(LOCALBIN)

@spacewander

Copy link
Member

Choose a reason for hiding this comment

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

@Chever-John
LGTM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update!!!

from ROOT_DIR := $(shell dirname $(realpath $(firstword $(MAKEFILE_LIST)))) to ROOT_DIR := $(shell dirname $(realpath $(lastword $(MAKEFILE_LIST))))

# Remember to remove tools downloaded into bin directory manually before updating them.
# If they need to be updated frequently, we can consider to store them in the `Dockerfile.dev`.
$(LOCALBIN):
Expand Down
2 changes: 1 addition & 1 deletion e2e/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

include ../common.mk

LOCALBIN ?= $(shell pwd)/bin
LOCALBIN ?= $(shell cd .. && pwd)/bin
$(LOCALBIN):
@mkdir -p $(LOCALBIN)

Expand Down
4 changes: 3 additions & 1 deletion e2e/istio.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
set -eo pipefail
set -x

HELM="$(pwd)/bin/helm"
PARENT_DIR="$(cd .. && pwd)"
Copy link
Member

Choose a reason for hiding this comment

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

We can pass LOCALBIN to the script, like

htnn/Makefile

Line 44 in e95743b

LOCALBIN=$(LOCALBIN) tools/gen-crd-code.sh

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sry, I missed it here.

Please wait for me for a few minutes.


HELM="${PARENT_DIR}/bin/helm"
E2E_DIR="$(pwd)"

install() {
Expand Down
Loading