You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2021/04/11 08:31:54 UTC
[GitHub] [apisix-docker] iamayushdas opened a new pull request #161: docs: Replace docker build command with make
iamayushdas opened a new pull request #161:
URL: https://github.com/apache/apisix-docker/pull/161
Fixed #147
@Yiyiyimu have a look
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-docker] tokers commented on a change in pull request #161: docs: Replace `docker build` command with `make`
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #161:
URL: https://github.com/apache/apisix-docker/pull/161#discussion_r612866389
##########
File path: README.md
##########
@@ -11,27 +11,27 @@
# Assign Apache release version number to variable `APISIX_VERSION`, for example: 2.4. The latest version can be find at `https://github.com/apache/apisix/releases`
export APISIX_VERSION=2.4
-docker build -t apisix:${APISIX_VERSION}-alpine --build-arg APISIX_VERSION=${APISIX_VERSION} -f alpine/Dockerfile alpine
+make -t apisix:${APISIX_VERSION}-alpine --build-arg APISIX_VERSION=${APISIX_VERSION} -f alpine/Dockerfile alpine
Review comment:
```suggestion
# alpine
make build-on-alpine
# centos
make build-on-centos
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-docker] iamayushdas commented on pull request #161: docs: Replace `docker build` command with `make`
Posted by GitBox <gi...@apache.org>.
iamayushdas commented on pull request #161:
URL: https://github.com/apache/apisix-docker/pull/161#issuecomment-820984200
@Yiyiyimu @tokers have a look
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-docker] Yiyiyimu commented on a change in pull request #161: docs: Replace `docker build` command with `make`
Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on a change in pull request #161:
URL: https://github.com/apache/apisix-docker/pull/161#discussion_r614629880
##########
File path: README.md
##########
@@ -30,15 +30,18 @@ $ make build-on-centos
```
# To copy apisix into image, we need to include it in build context
$ cd <APISIX-PATH>
-# alpine-local
-$ make build-on-alpine-local
+
+# alpine
+
+$ APISIX_PATH=/path/to/apisix make build-on-alpine-local
+
# Might need root privilege if encounter "error checking context: 'can't stat'"
```
**Note:** For Chinese, the following command is always recommended. The additional build argument `ENABLE_PROXY=true` will enable proxy to definitely accelerate the progress.
```sh
-$ make build-on-alpine-cn
+$ docker build -t apisix:${APISIX_VERSION}-alpine --build-arg APISIX_VERSION=${APISIX_VERSION} --build-arg ENABLE_PROXY=true -f alpine/Dockerfile alpine
Review comment:
```suggestion
$ make build-on-alpine-cn
```
##########
File path: Makefile
##########
@@ -34,8 +34,8 @@ build-on-alpine:
build-on-alpine-local:
docker build -t $(IMAGE_NAME):$(APISIX_VERSION)-alpine-local --build-arg APISIX_PATH=${APISIX_PATH} -f ./alpine-local/Dockerfile .
-### build-on-alpine-cn: Build apache/apisix:xx-alpine image (for chinese)
- docker build -t $(IMAGE_NAME):${APISIX_VERSION}-alpine --build-arg APISIX_VERSION=${APISIX_VERSION} --build-arg ENABLE_PROXY=true -f alpine/Dockerfile alpine
+# build-on-alpine-cn: Build apache/apisix:xx-alpine image (for chinese)
+# docker build -t $(IMAGE_NAME):${APISIX_VERSION}-alpine --build-arg APISIX_VERSION=${APISIX_VERSION} --build-arg ENABLE_PROXY=true -f alpine/Dockerfile alpine
Review comment:
```suggestion
build-on-alpine-cn:
docker build -t $(IMAGE_NAME):${APISIX_VERSION}-alpine --build-arg APISIX_VERSION=${APISIX_VERSION} --build-arg ENABLE_PROXY=true -f alpine/Dockerfile alpine
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-docker] iamayushdas commented on pull request #161: docs: Replace `docker build` command with `make`
Posted by GitBox <gi...@apache.org>.
iamayushdas commented on pull request #161:
URL: https://github.com/apache/apisix-docker/pull/161#issuecomment-818727985
> @iamayushdas In fact, changing to `make` will not work. You need to know the operating mechanism of the makefile and make sure that CI can pass
It would really be great if you help me in learning mechanism makefile.
Or else you give a PR to this issue and i will be learning by your PR for now.
Thanks for review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-docker] iamayushdas commented on pull request #161: docs: Replace `docker build` command with `make`
Posted by GitBox <gi...@apache.org>.
iamayushdas commented on pull request #161:
URL: https://github.com/apache/apisix-docker/pull/161#issuecomment-821756102
@Yiyiyimu review please
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-docker] iamayushdas commented on a change in pull request #161: docs: Replace `docker build` command with `make`
Posted by GitBox <gi...@apache.org>.
iamayushdas commented on a change in pull request #161:
URL: https://github.com/apache/apisix-docker/pull/161#discussion_r613277522
##########
File path: README.md
##########
@@ -12,29 +12,33 @@
export APISIX_VERSION=2.4
# alpine
-make build-on-alpine
+$ make build-on-alpine
# centos
-make build-on-centos
+$ make build-on-centos
```
2. Build from master branch version, which has latest code(ONLY for the developer's convenience):
```
export APISIX_VERSION=master
-make -t apisix:${APISIX_VERSION}-alpine --build-arg APISIX_VERSION=${APISIX_VERSION} -f alpine/Dockerfile alpine
+# alpine
+$ make build-on-alpine
+# centos
+$ make build-on-centos
```
3. Build from local code:
```
# To copy apisix into image, we need to include it in build context
-cd <APISIX-PATH>
-make -t apisix:alpine-local --build-arg APISIX_PATH=. -f <APISIX-DOCKER-PATH>/alpine-local/Dockerfile alpine
+$ cd <APISIX-PATH>
+# alpine-local
Review comment:
okay got you
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-docker] Yiyiyimu commented on a change in pull request #161: docs: Replace `docker build` command with `make`
Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on a change in pull request #161:
URL: https://github.com/apache/apisix-docker/pull/161#discussion_r612964805
##########
File path: README.md
##########
@@ -11,27 +11,30 @@
# Assign Apache release version number to variable `APISIX_VERSION`, for example: 2.4. The latest version can be find at `https://github.com/apache/apisix/releases`
export APISIX_VERSION=2.4
-docker build -t apisix:${APISIX_VERSION}-alpine --build-arg APISIX_VERSION=${APISIX_VERSION} -f alpine/Dockerfile alpine
+# alpine
+make build-on-alpine
+# centos
+make build-on-centos
```
2. Build from master branch version, which has latest code(ONLY for the developer's convenience):
```
export APISIX_VERSION=master
-docker build -t apisix:${APISIX_VERSION}-alpine --build-arg APISIX_VERSION=${APISIX_VERSION} -f alpine/Dockerfile alpine
+make -t apisix:${APISIX_VERSION}-alpine --build-arg APISIX_VERSION=${APISIX_VERSION} -f alpine/Dockerfile alpine
Review comment:
more places need to be fixed @iamayushdas
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-docker] Yiyiyimu commented on pull request #161: docs: Replace `docker build` command with `make`
Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on pull request #161:
URL: https://github.com/apache/apisix-docker/pull/161#issuecomment-822135401
Thanks for the fix!! @iamayushdas
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-docker] Yiyiyimu merged pull request #161: docs: Replace `docker build` command with `make`
Posted by GitBox <gi...@apache.org>.
Yiyiyimu merged pull request #161:
URL: https://github.com/apache/apisix-docker/pull/161
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-docker] iamayushdas commented on pull request #161: docs: Replace `docker build` command with `make`
Posted by GitBox <gi...@apache.org>.
iamayushdas commented on pull request #161:
URL: https://github.com/apache/apisix-docker/pull/161#issuecomment-817271951
@Yiyiyimu @juzhiyuan there's an issue :disappointed:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-docker] iamayushdas commented on pull request #161: docs: Replace `docker build` command with `make`
Posted by GitBox <gi...@apache.org>.
iamayushdas commented on pull request #161:
URL: https://github.com/apache/apisix-docker/pull/161#issuecomment-819507190
@tokers @Yiyiyimu can i have a review please
thank you
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-docker] iamayushdas commented on a change in pull request #161: docs: Replace `docker build` command with `make`
Posted by GitBox <gi...@apache.org>.
iamayushdas commented on a change in pull request #161:
URL: https://github.com/apache/apisix-docker/pull/161#discussion_r613268091
##########
File path: README.md
##########
@@ -12,29 +12,33 @@
export APISIX_VERSION=2.4
# alpine
-make build-on-alpine
+$ make build-on-alpine
# centos
-make build-on-centos
+$ make build-on-centos
```
2. Build from master branch version, which has latest code(ONLY for the developer's convenience):
```
export APISIX_VERSION=master
-make -t apisix:${APISIX_VERSION}-alpine --build-arg APISIX_VERSION=${APISIX_VERSION} -f alpine/Dockerfile alpine
+# alpine
+$ make build-on-alpine
+# centos
+$ make build-on-centos
```
3. Build from local code:
```
# To copy apisix into image, we need to include it in build context
-cd <APISIX-PATH>
-make -t apisix:alpine-local --build-arg APISIX_PATH=. -f <APISIX-DOCKER-PATH>/alpine-local/Dockerfile alpine
+$ cd <APISIX-PATH>
+# alpine-local
Review comment:
i used it just to make clear that it is locally existing alpine not remotely. :thinking:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-docker] Yiyiyimu commented on a change in pull request #161: docs: Replace `docker build` command with `make`
Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on a change in pull request #161:
URL: https://github.com/apache/apisix-docker/pull/161#discussion_r613260603
##########
File path: README.md
##########
@@ -12,29 +12,33 @@
export APISIX_VERSION=2.4
# alpine
-make build-on-alpine
+$ make build-on-alpine
# centos
-make build-on-centos
+$ make build-on-centos
```
2. Build from master branch version, which has latest code(ONLY for the developer's convenience):
```
export APISIX_VERSION=master
-make -t apisix:${APISIX_VERSION}-alpine --build-arg APISIX_VERSION=${APISIX_VERSION} -f alpine/Dockerfile alpine
+# alpine
+$ make build-on-alpine
+# centos
+$ make build-on-centos
```
3. Build from local code:
```
# To copy apisix into image, we need to include it in build context
-cd <APISIX-PATH>
-make -t apisix:alpine-local --build-arg APISIX_PATH=. -f <APISIX-DOCKER-PATH>/alpine-local/Dockerfile alpine
+$ cd <APISIX-PATH>
+# alpine-local
+$ make build-on-alpine-local
Review comment:
We need to set environment ${APISIX_PATH} first here, see Makefile
##########
File path: README.md
##########
@@ -12,29 +12,33 @@
export APISIX_VERSION=2.4
# alpine
-make build-on-alpine
+$ make build-on-alpine
# centos
-make build-on-centos
+$ make build-on-centos
```
2. Build from master branch version, which has latest code(ONLY for the developer's convenience):
```
export APISIX_VERSION=master
-make -t apisix:${APISIX_VERSION}-alpine --build-arg APISIX_VERSION=${APISIX_VERSION} -f alpine/Dockerfile alpine
+# alpine
+$ make build-on-alpine
+# centos
+$ make build-on-centos
```
3. Build from local code:
```
# To copy apisix into image, we need to include it in build context
-cd <APISIX-PATH>
-make -t apisix:alpine-local --build-arg APISIX_PATH=. -f <APISIX-DOCKER-PATH>/alpine-local/Dockerfile alpine
+$ cd <APISIX-PATH>
+# alpine-local
Review comment:
we could remove this line alpine-local is just a random name 🤣
##########
File path: Makefile
##########
@@ -34,6 +34,12 @@ build-on-alpine:
build-on-alpine-local:
docker build -t $(IMAGE_NAME):$(APISIX_VERSION)-alpine-local --build-arg APISIX_PATH=${APISIX_PATH} -f ./alpine-local/Dockerfile .
+### build-on-alpine-cn: Build apache/apisix:xx-alpine image (for chinese)
Review comment:
need to add another line of function name, now you comment it out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-docker] iamayushdas commented on a change in pull request #161: docs: Replace `docker build` command with `make`
Posted by GitBox <gi...@apache.org>.
iamayushdas commented on a change in pull request #161:
URL: https://github.com/apache/apisix-docker/pull/161#discussion_r611295719
##########
File path: README.md
##########
@@ -11,27 +11,27 @@
# Assign Apache release version number to variable `APISIX_VERSION`, for example: 2.4. The latest version can be find at `https://github.com/apache/apisix/releases`
export APISIX_VERSION=2.4
-docker build -t apisix:${APISIX_VERSION}-alpine --build-arg APISIX_VERSION=${APISIX_VERSION} -f alpine/Dockerfile alpine
+make -t apisix:${APISIX_VERSION}-alpine --build-arg APISIX_VERSION=${APISIX_VERSION} -f alpine/Dockerfile alpine
Review comment:
@tokers @Yiyiyimu could you be elaborative here for what to do here now?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-docker] tokers commented on a change in pull request #161: docs: Replace `docker build` command with `make`
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #161:
URL: https://github.com/apache/apisix-docker/pull/161#discussion_r611276033
##########
File path: README.md
##########
@@ -11,27 +11,27 @@
# Assign Apache release version number to variable `APISIX_VERSION`, for example: 2.4. The latest version can be find at `https://github.com/apache/apisix/releases`
export APISIX_VERSION=2.4
-docker build -t apisix:${APISIX_VERSION}-alpine --build-arg APISIX_VERSION=${APISIX_VERSION} -f alpine/Dockerfile alpine
+make -t apisix:${APISIX_VERSION}-alpine --build-arg APISIX_VERSION=${APISIX_VERSION} -f alpine/Dockerfile alpine
Review comment:
Well, all the following options are used for `docker`, not for `make`. I bet this command will fail when running. All the options should be encapsulated into the make directive, please seeing https://github.com/apache/apisix-docker/blob/master/Makefile for more details.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-docker] gxthrj commented on pull request #161: docs: Replace `docker build` command with `make`
Posted by GitBox <gi...@apache.org>.
gxthrj commented on pull request #161:
URL: https://github.com/apache/apisix-docker/pull/161#issuecomment-818723540
@iamayushdas In fact, changing to `make` will not work. You need to know the operating mechanism of the makefile and make sure that CI can pass
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-docker] iamayushdas commented on a change in pull request #161: docs: Replace `docker build` command with `make`
Posted by GitBox <gi...@apache.org>.
iamayushdas commented on a change in pull request #161:
URL: https://github.com/apache/apisix-docker/pull/161#discussion_r612965249
##########
File path: README.md
##########
@@ -11,27 +11,30 @@
# Assign Apache release version number to variable `APISIX_VERSION`, for example: 2.4. The latest version can be find at `https://github.com/apache/apisix/releases`
export APISIX_VERSION=2.4
-docker build -t apisix:${APISIX_VERSION}-alpine --build-arg APISIX_VERSION=${APISIX_VERSION} -f alpine/Dockerfile alpine
+# alpine
+make build-on-alpine
+# centos
+make build-on-centos
```
2. Build from master branch version, which has latest code(ONLY for the developer's convenience):
```
export APISIX_VERSION=master
-docker build -t apisix:${APISIX_VERSION}-alpine --build-arg APISIX_VERSION=${APISIX_VERSION} -f alpine/Dockerfile alpine
+make -t apisix:${APISIX_VERSION}-alpine --build-arg APISIX_VERSION=${APISIX_VERSION} -f alpine/Dockerfile alpine
Review comment:
Ok
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-docker] Yiyiyimu commented on a change in pull request #161: docs: Replace `docker build` command with `make`
Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on a change in pull request #161:
URL: https://github.com/apache/apisix-docker/pull/161#discussion_r613276156
##########
File path: README.md
##########
@@ -12,29 +12,33 @@
export APISIX_VERSION=2.4
# alpine
-make build-on-alpine
+$ make build-on-alpine
# centos
-make build-on-centos
+$ make build-on-centos
```
2. Build from master branch version, which has latest code(ONLY for the developer's convenience):
```
export APISIX_VERSION=master
-make -t apisix:${APISIX_VERSION}-alpine --build-arg APISIX_VERSION=${APISIX_VERSION} -f alpine/Dockerfile alpine
+# alpine
+$ make build-on-alpine
+# centos
+$ make build-on-centos
```
3. Build from local code:
```
# To copy apisix into image, we need to include it in build context
-cd <APISIX-PATH>
-make -t apisix:alpine-local --build-arg APISIX_PATH=. -f <APISIX-DOCKER-PATH>/alpine-local/Dockerfile alpine
+$ cd <APISIX-PATH>
+# alpine-local
Review comment:
But if you refer to other places that use hash prefixes, you could find it would be `alpine` or `centos` which are OS. Apparently here it is not. At least it confused me
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-docker] iamayushdas commented on a change in pull request #161: docs: Replace `docker build` command with `make`
Posted by GitBox <gi...@apache.org>.
iamayushdas commented on a change in pull request #161:
URL: https://github.com/apache/apisix-docker/pull/161#discussion_r614631574
##########
File path: README.md
##########
@@ -30,15 +30,18 @@ $ make build-on-centos
```
# To copy apisix into image, we need to include it in build context
$ cd <APISIX-PATH>
-# alpine-local
-$ make build-on-alpine-local
+
+# alpine
+
+$ APISIX_PATH=/path/to/apisix make build-on-alpine-local
+
# Might need root privilege if encounter "error checking context: 'can't stat'"
```
**Note:** For Chinese, the following command is always recommended. The additional build argument `ENABLE_PROXY=true` will enable proxy to definitely accelerate the progress.
```sh
-$ make build-on-alpine-cn
+$ docker build -t apisix:${APISIX_VERSION}-alpine --build-arg APISIX_VERSION=${APISIX_VERSION} --build-arg ENABLE_PROXY=true -f alpine/Dockerfile alpine
Review comment:
but as previously you said i have commented out the makefile
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-docker] juzhiyuan commented on pull request #161: docs: Replace `docker build` command with `make`
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on pull request #161:
URL: https://github.com/apache/apisix-docker/pull/161#issuecomment-817274141
cc @gxthrj @tokers to take a look
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix-docker] Yiyiyimu commented on a change in pull request #161: docs: Replace `docker build` command with `make`
Posted by GitBox <gi...@apache.org>.
Yiyiyimu commented on a change in pull request #161:
URL: https://github.com/apache/apisix-docker/pull/161#discussion_r615512813
##########
File path: README.md
##########
@@ -30,15 +30,18 @@ $ make build-on-centos
```
# To copy apisix into image, we need to include it in build context
$ cd <APISIX-PATH>
-# alpine-local
-$ make build-on-alpine-local
+
+# alpine
+
+$ APISIX_PATH=/path/to/apisix make build-on-alpine-local
+
# Might need root privilege if encounter "error checking context: 'can't stat'"
```
**Note:** For Chinese, the following command is always recommended. The additional build argument `ENABLE_PROXY=true` will enable proxy to definitely accelerate the progress.
```sh
-$ make build-on-alpine-cn
+$ docker build -t apisix:${APISIX_VERSION}-alpine --build-arg APISIX_VERSION=${APISIX_VERSION} --build-arg ENABLE_PROXY=true -f alpine/Dockerfile alpine
Review comment:
Yes but it seems you de-comment out the wrong way 🤣
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org