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

make script posix compliant #62

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

make script posix compliant #62

wants to merge 1 commit into from

Conversation

txgk
Copy link

@txgk txgk commented May 25, 2022

This makes hr work in the busybox environment in my Alpine Linux container.

@LuRsT
Copy link
Owner

LuRsT commented Nov 30, 2022

Hey @txgk, I appreciate the PR, I think I'd rather keep hr as a bash script to make it easier to work with, but I was testing your changes and it doesn't seem to work in Alpine still, here's the steps I used.

With this Dockerfile:

FROM alpine

RUN apk update && apk add gcc make
COPY . /tmp

WORKDIR /tmp

RUN make install

Then running hr inside the container, I'm getting "tput: not found" is tput perhaps not part of ash? In which case, hr as it is would need a lot more work to make it work with ash.

Let me know if I am making any mistake in my testing.

@txgk
Copy link
Author

txgk commented Nov 30, 2022

Hello!

I think I'd rather keep hr as a bash script to make it easier to work

That's fine, no problem.

Then running hr inside the container, I'm getting "tput: not found" is tput perhaps not part of ash?

The thing is that tput is not a part of bash either. In Linux distributions this utility is provided by ncurses usually.

hr as it is would need a lot more work to make it work with ash.

I don't think so. I'd say bash variant and POSIX variant aren't that different, it's just that you get portability with latter.

@LuRsT
Copy link
Owner

LuRsT commented Dec 5, 2022

Thanks for the reply @txgk!

The thing is that tput is not a part of bash either. In Linux distributions this utility is provided by ncurses usually.

Oh right, I wasn't aware, so then my test didn't work because the docker container did not have ncurses installed, I forgot/didn't realise that hr actually had that dependency, I'm going to take a second look at these changes then 👍

@txgk
Copy link
Author

txgk commented Dec 5, 2022

Take your time :D

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

Successfully merging this pull request may close these issues.

None yet

2 participants