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

Update Makefile #1066

Closed

Conversation

Danilo-Araujo-Silva
Copy link

When building the Dockerfile pointing to the latest Ubuntu LTS release, the property KHOST is not filled properly, and we cannot configure it from outside since we are using := instead of ?=. Proposing a change for more flexibility.

When building the Dockerfile pointing to the latest Ubuntu LTS release, the property KHOST is not filled properly, and we cannot configure it from outside since we are using `:=` instead of `?=`. Proposing a change for more flexibility.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 38.084% when pulling 0032901 on Danilo-Araujo-Silva:patch-1 into 8614acc on ctubio:master.

@ctubio
Copy link
Owner

ctubio commented Feb 23, 2021

morniiing'''
im sorry KHOST is not supposed to be changed, the values are mandatory as-is related to the existent filenames of the releases for each platform and also are related to the existent build folders (inside each zip)

maybe you want to change CHOST?

🐨 oor maybe can you show with some logs what error are you trying to fix? thank you'¡

@Danilo-Araujo-Silva
Copy link
Author

Danilo-Araujo-Silva commented Feb 23, 2021

Sure, explaining a bit more.

I was checking the docker file available here:

FROM overlordq/8-buster

RUN apt-get update \
 && apt-get install --no-install-recommends -y git sudo

# Feel free to choose the branch you want to build:
RUN git clone -b master https://github.com/ctubio/Krypto-trading-bot.git /K

WORKDIR /K

RUN make docker && rm -rf /var/lib/apt/lists/

EXPOSE 3000 5000

# See examples and descriptions of the
# following variables at etc/K.sh.dist.

ENV OPTIONAL_ARGUMENTS --colors --port 3000

ENV API_EXCHANGE NULL
ENV API_CURRENCY BTC/USD
ENV API_KEY NULL
ENV API_SECRET NULL
ENV API_PASSPHRASE NULL

ENV K_BINARY_FILE K-trading-bot

CMD ["./K.sh", "--naked", "--without-ssl"]

But I've seen that if we change overlordq/8-buster to ubuntu:latest the build crashes. The possibility to change this would be useful if we want to keep the things up to date and also do some other modifications.

So, after playing with some configurations and inspecting the errors, I can confirm the following docker file will work (if we set the KHOST to linux-x86_64 and have that modification on Makefile):

FROM ubuntu:latest

# Example configuration if we want to change the timezone from UTC:
#ARG DEBIAN_FRONTEND=noninteractive
#ENV TZ=America/Sao_Paulo
#RUN ln -snf /usr/share/zoneinfo/$TZ /etc/localtime && echo $TZ > /etc/timezone

RUN apt-get update \
 && apt-get install --no-install-recommends -y tzdata sudo ca-certificates make curl git

RUN git clone -b master git clone -b master https://github.com/ctubio/Krypto-trading-bot.git /K

WORKDIR /K

ENV KHOST=linux-x86_64
RUN make docker && rm -rf /var/lib/apt/lists/

EXPOSE 3000

# See examples and descriptions of the
# following variables at etc/K.sh.dist.

ENV OPTIONAL_ARGUMENTS --colors --port 3000

ENV API_EXCHANGE NULL
ENV API_CURRENCY BTC/USD
ENV API_KEY NULL
ENV API_SECRET NULL
ENV API_PASSPHRASE NULL

ENV K_BINARY_FILE K-trading-bot

CMD ["./K.sh", "--naked", "--without-ssl"]

Please kindly let me know what do you think

@ctubio
Copy link
Owner

ctubio commented Feb 23, 2021

i think you want to use CHOST=x86_64-linux-gnu because KHOST is not suposed to be changed (cos is not suposed to be nothing by itself, is suposed to be an edited CHOST value)

the supported CHOST values are at:

CARCH = x86_64-linux-gnu \
arm-linux-gnueabihf \
aarch64-linux-gnu \
x86_64-apple-darwin17 \
x86_64-w64-mingw32

@Danilo-Araujo-Silva
Copy link
Author

Hum, I see now,

it is because g++was missing and the CHOST comes from g++ -dumpmachine

With this:

apt-get install --no-install-recommends -y tzdata sudo ca-certificates make curl git g++

the Dockerfile works, without any modication.

I think we can close this issue then

If you don't mind, in the future I'd like to propose some changes to the docker files, like an automated way to create a robot party that I'm working on

@ctubio
Copy link
Owner

ctubio commented Feb 23, 2021

🐨 sure you are welcome if makes sense (im not using docker thats why i didnt did much about)
thank you'¡

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants