tenox7 / ttyplot

a realtime plotting utility for terminal/console with data input from stdin
Apache License 2.0
956 stars 42 forks source link

ttyplot doesn't build (in general and on Void Linux) because of issues in the Makefile #159

Closed MIvanchev closed 6 months ago

MIvanchev commented 6 months ago

I'm updating the Void Linux package and found several problems with the Makefile for which I provide a PR. Back quotes are not used for shell expansion in Make because a Makefile is not a shell script. The proper way is through $(shell pkg-config ...). Also if CFLAGS are passed through make CFLAGS=.... (like Void Linux does) the += statements in Makefile have no effect. So I added override where it's needed.

MIvanchev commented 6 months ago

Seems to apply to GNU make, PR needs some more polishing.

MIvanchev commented 6 months ago

For the time being I'll just add a patch to Void Linux until we find the best way to proceed.

hartwork commented 6 months ago

Hi @MIvanchev please note that we cover non-GNU make flavors bsdmake and bmake in CI and all three of these were made to work fine. What flavor of make does Void have and/or use by default?

hartwork commented 6 months ago

@MIvanchev I only now saw the make CFLAGS=.... up there — must be CFLAGS=.... make instead and should then work just fine. That is known and works as expected.

hartwork commented 6 months ago

Update: I found line…

build_style=gnu-makefile

at https://github.com/void-linux/void-packages/blob/03dea5d36774312ad42c3f4b02bbdbfd4f9fb9c5/srcpkgs/ttyplot/template#L5C1-L5C12 now, so it's GNU make and the issue is in the way it's used and not in the makefile. Closing as not a bug, works as expected, see https://github.com/tenox7/ttyplot/issues/159#issuecomment-1874219330 above for details.

MIvanchev commented 6 months ago

Yeah, it uses GNU make. OK, so no GNU make support. Anyhow I got it to build but the behavior is very weird. Let me open some issues.

edgar-bonet commented 6 months ago

@MIvanchev wrote:

no GNU make support

The intent is that ttyplot’s Makefile works with GNU make (for Linux) as well as with the standard make implementations of *BSD and macOS.

On Linux, you can install bmake in order to have a BSD-flavored make to test against.

MIvanchev commented 6 months ago

But it doesn't work with GNU make because the backtick is not a legal command substitution.

edgar-bonet commented 6 months ago

@MIvanchev: It does at least work with GNU Make 4.3, as shipped with Ubuntu 22.04.

because the backtick is not a legal command substitution

The backticks are not interpreted by make. They are interpreted by the shell that runs the recipes.

MIvanchev commented 6 months ago

They are interpreted by the shell that runs the recipes.

OK, so the build also requires a shell with backtick substitution. Maybe the commands could be modified to invoke sh directly instead of assuming it's the parent process.

edgar-bonet commented 6 months ago

@MIvanchev: Backtick substitution is a very standard shell feature. Do you have a use case for running make with a $SHELL that does not support it?

MIvanchev commented 6 months ago

I mean I've been compiling stuff on the fish shell for years and so far ttyplot's Makefile is the only one that makes an assumption regarding a shell feature. IMO it's a valid one, it just should be more obvious than a lengthy discussion or just straight out automatic or baked by some kind of a test 😅.

edgar-bonet commented 6 months ago

@MIvanchev: It should be possible to tell make what shell to use, albeit the way to do so depends on the implementation. You may want to try adding this to the top of the Makefile, then calling make and bmake from fish:

# Make sure we use a standard shell that supports backtick expansion.
SHELL =      /bin/sh  # for GNU make
.SHELL: path=/bin/sh  # for BSD make
hartwork commented 6 months ago

@MIvanchev backticks are POSIX shell, which we require. It could hardly be a more standard shell feature than than. The current makefile works just as fine when invoked with CFLAGS=... LDFLAGS=... make with 3+ different flavors of make and POSIX shell. Supporting make CFLAGS=... LDFLAGS=... could be a bonus but is not a bug. I doubt it's worth making that work for all three flavors of make together.

MIvanchev commented 6 months ago

I'm not convinced by embedding shell stuff into a Makefile without an explicit call to the shell but as long as I know how to build it without N surprises I'm happy.