Rework build system v2

[N-R-K]

As discussed in #66 (comment) , this PR takes care of any remaining cleanups on the makefile and cleans version.h upon make clean.

Also revert back to using mkdir -> cp -> chmod since install is not posix.

We may want to reconsider reverting ifeq as well, see: xyb3rt/sxiv@5155d52

[N-R-K]

The initial reasoning for having auto library detection was that minimalist distros might benefit from it, but now as it turns out this might've actually harmed those distro users who are using non-gnu system.

I think this should be a good lesson for us moving forward, while certain things might seem "smart" or "convenient" it actually does more harm by bringing unnecessary dependencies.

[N-R-K]

I don't believe -include is standardized either. And using HAVE_LIBX ?= instead of ifeq can cause potential confusion as described in #19 (comment)

Since the entire rational behind auto-detection was that it might benefit minimalist distros, but in reality these non standard Makefile might end up harming these minimalist distros, I think we may just want to revert the auto-detection all together given the fact that it will make our build more portable and running make HAVE_LIBX=0 isn't hard at all. @TAAPArthur @bakkeby @XPhyro

[TAAPArthur]

I don't believe -include is standardized either. And using HAVE_LIBX ?= instead of ifeq can cause potential confusion as described in #19 (comment)

Since we auto create the config.mk in the file version, I believe we can drop the - since it should always exist/be autocreated. Didn't realize it wasn't portable, but don't think there is anything else non-standard that would warrant reverting the issue.

I think we may just want to revert the auto-detection all together given the fact that it will make our build more portable and running make HAVE_LIBX=0 isn't hard at all.

I don't want to special case any single package manager/distro, but the reason that this is helpful for KISS Linux, is that the packager generally provides the bare minimum build script/dependencies needed to build the package. So they either have to forcibly list libgif and friends as mandatory or not use them. Something like make AUTODETECT=1 would fulfill the purpose too if we just didn't want to make it a default.

[N-R-K]

is that the packager generally provides the bare minimum build script/dependencies needed to build the package.

AFIAK Kiss packages are just shell/build scripts, so adding HAVE_LIBX should be absolutely trivial. Also consider this, imlib2 already pulls in giflib if built with gif support. And imlib on the k1ss community repo seems to advise manually un-commenting it if needed. Is there any reason why nsxiv can't be the same?

Something like make AUTODETECT=1 would fulfill the purpose too if we just didn't want to make it a default.

If we can make it portable without using anything non-standard then, I'm okay with that.

[TAAPArthur]

AFIAK Kiss packages are just shell/build scripts, so adding HAVE_LIBX should be absolutely trivial.
And imlib on the k1ss community repo seems to advise manually un-commenting it if needed.

Yes it is trivial but requires everyone who wants a different set of depends to fork which is also trivial but extra, unneeded work for when I claim we know the correct behavior. It isn't that I want to choose between portability and convenience but think we can get both.
Also the comments are just someone being nice and listing the optional dependencies. There is no standard for doing so but also nothing preventing nsxiv from copying.

If we can make it portable without using anything non-standard then, I'm okay with that.

So we agree to just drop the "-". It is leftover from the draft where there was an optional configure script. Is there any other portability concerns.

Should we also include .POSIX target to avoid accidentally being non-POSIX

EDIT:
As far as I'm concerned, making nsxiv build with minimal dependencies by default would be sufficient. Thought auto-detecting would be more user friendly but I personally don't use any of the optional libs so not completely against removing the auto detection

[N-R-K]

Is there any other portability concerns.

I couldn't find any info about ifeq being standard. And given that xyb3rt/sxiv@5155d52 is removing ifeqs I am assuming it's non-standard.

[TAAPArthur]

Is there any other portability concerns.

I couldn't find any info about ifeq being standard. And given that muennich/sxiv@5155d52 is removing ifeqs I am assuming it's non-standard.

Well I found mention that ifdef and include themselves are an extension so

Historically, the make utility has been an especially fertile ground for vendor and research organization-specific syntax modifications and extensions. Examples include:
...
Adding C preprocessor-style "include" and "ifdef" constructs (System V, GNU, BSD, and others)

So you are probably right. If we want to be as portable as possible on principle, then I don't object to removing the auto detection. Still think the we should build with minimal dependencies by default.

[TAAPArthur]

Wait how would optional libs even work then. We need ifs to actually add the libs.

[N-R-K]

Wait how would optional libs even work then. We need ifs to actually add the libs.

We don't. We can add HAVE_LIBX = 1 as it was before. User's would directly edit the makefile, or pass in make HAVE_LIBX=0.

So since we can't do any auto-detection without using extensions, are you still opposed to dropping it?
Looks like you answered it here, I glanced over it.

then I don't object to removing the auto detection.

[N-R-K]

Since you don't oppose it any longer, I've pushed the change.

There's now two more things we need to address,

• ?=
• and $(RM)

Are they standard?

It seems like ?= was accepted (?) https://austingroupbugs.net/view.php?id=330
I'm not sure about $(RM)

[eylles]

Adding C preprocessor-style "include" and "ifdef" constructs (System V, GNU, BSD, and others)

wait, doesn't that mean that idef and include are present in bsd?

[N-R-K]

wait, doesn't that mean that idef and include are present in bsd?

Yes, as non standard extensions. The sxiv makefile previously used to use them, but dropped it in favor of portability and sticking to the standard. See the linked commit in OP.

[eylles]

Hmm i would preffer to not give up on the auto detection tho, ah btw you can assign variables to the result of shell commands with the syntax:
VAR = `command`
And that is a posix macro, is there any posibility we can use that to still have autodetection but in a more portable way?.
also if gnu make and bsd make have the extensions to use autoreload is not like nsxiv would be compiled in many more systems, would need to check what extensions do other popular make utils have (except windows make) before deciding harshly as iirc there's no make implementation out there that is truly posix make as all have extensions.

Edit:
I will be looking around and asking about this to find out what extensions of the current makefile are not supported or have no equivalents in ither systems, also what can be done to convert the library auto detection to a more portable format.

[N-R-K]

making nsxiv build with minimal dependencies by default would be sufficient.

Taap, I see where you're coming from, and I honestly agree that user's should have to turn optional dependancies on manually. However consider this, if someone is coming over from sxiv and he's used to having gif playback by default, simply running make will no longer have multiframe gif for him.

One of our project goals is to not break backwards compatibility, I'm not sure if this case qualifies but it's still something we should consider more deeply.

[eylles]

looking around include is actually POSIX, on the regard of dealing with libraries we could use pkg-config which is supported by all unix like OSes from mac to bsd and the syntax seems easy enough to use.

DEP1_CFLAGS = `pkg-config --cflags dep1` -D HAVE_DEP1
DEP1_LDFLAGS = `pkg-config --libs dep1`

MYCFLAGS = $(CFLAGS) $(DEP1_CFLAGS)
MYLDFLAGS = $(LDFLAGS) $(DEP1_LDFLAGS)

.c.o:
        $(CC) -c $(MYCFLAGS) $<

[N-R-K]

I don't believe pkg-config is posix. man 1p pkg-config does not bring up anything.

looking around include is actually POSIX

Can you link the source?

[TAAPArthur]

making nsxiv build with minimal dependencies by default would be sufficient.

Taap, I see where you're coming from, and I honestly agree that user's should have to turn optional dependancies on manually. However consider this, if someone is coming over from sxiv and he's used to having gif playback by default, simply running make will no longer have multiframe gif for him.

One of our project goals is to not break backwards compatibility, I'm not sure if this case qualifies but it's still something we should consider more deeply.

I suspected the backwards compatibility argument would be made to an attempt to change the defaults which is why I went with auto-detection. But auto-detection was just a means to achieve the goal of not having to explicitly disable the growing list of optional dependencies.

[N-R-K]

If include is posix, then I believe we can have a make autoconfig rule, which you said should take care of your use case.

We should change the output to something else, maybe autolibs.mk, though. Since config.mk is typically used for configuring makefile variables in suckless projects, meant to be edited by the users.

[TAAPArthur]

I don't believe pkg-config is posix. man 1p pkg-config does not bring up anything.

Not POSIX and isn't available on all systems (but probably is on most). Not saying the program itself sucks, but suckless.com promotes a less sucky alternative to pkg-config. I went with using the compiler because we could guarantee it was there. But it really is the same end result either way assuming things are installed in a standard location

[TAAPArthur]

If include is posix, then I believe we can have a make autoconfig rule, which you said should take care of your use case.

Does this imply we are reverting to enabling all optional deps by default?
Also the source I linked stated include was an extension (that basically everyone implemented). And if include/ifeq are accetable, why not just keep the auto-detection as default as that seems the most friendly behavior.

[eylles]

so apparently include is indeed posix but -include isn't

[N-R-K]

And if include/ifeq are accetable, why not just keep the auto-detection as default as that seems the most friendly behavior.

Entirely depends on if include is posix or not. ifeq i don't think is posix, but we can use ?= instead to get around that.

[N-R-K]

Welp, that gets me back to an earlier point about ?= being a potential source of confusion #19 (comment)

[N-R-K]

@TAAPArthur does this work for you? I think the comments should take care of the confusion.

Also I think you misunderstood the manpage, it's talking about options which used to be extensions but are now included in posix. man 1p make has include defined. So include should be posix, but ifeq is not defined.

---

Finally now the only remaining concern is $(RM).

[N-R-K]

Related: https://github.com/nsxiv/nsxiv/issues/70#issuecomment-922286267

I have habit to just run sudo make clean install for all suckles software and sxiv. That worked for sxiv but not here.

[eylles]

Regardless number of commits we cannot take this as a personal pet project, we always have to have users in mind, a balance between what users want and need and hwta the developer wants is the space where a project thrives, projects with toxic devs out there are a dime the dozen.

[TAAPArthur]

Regardless number of commits we cannot take this as a personal pet project, we always have to have users in mind, a balance between what users want and need and hwta the developer wants is the space where a project thrives, projects with toxic devs out there are a dime the dozen.

Was this statement made in response to something I said? Or was it just a general comment?

[eylles]

in general really, i do think every decision that affects the codebase in any significant way a consideration should be made of how benefitial it is to the userbase not just for ourselves.

[N-R-K]

Having an optional config.mk file seems the most favorable to extension.

Imo that should be avoided as it just fractures the build time configuration. If we do want to go for some config.mk it shouldn't just be libraries, we should move all Makefile vars onto it, similar to other suckless projects. See: https://git.suckless.org/dwm/file/config.mk.html

---

Have a var like HAVE_LIB_DEFAULT ?= 1 that controls the default value of optional libs.

Can this be done inside the Makefile POSIX-ly ? If so then I'm on board.

[TAAPArthur]

Imo that should be avoided as it just fractures the build time configuration. If we do want to go for some config.mk it shouldn't just be libraries, we should move all Makefile vars onto it, similar to other suckless projects. See: https://git.suckless.org/dwm/file/config.mk.html

I don't understand what you mean by "fractures". It optionally allows users to use a config file. I don't see the problem with giving people extra options that can opt-in to. Feel like a similar argument can be made for Xresource and some cli args along with the same counterargument that they don't hurt people who don't care about them. Sticking -include config.mk at the top of the makefile seems harmless and simple enough along with having some targets auto generate it. The problem I see with including it as part of the repo is that it prevents us from overriding it via make and it being a tad harder for users to bring their own config.mk like they do for config.h. Also don't see a point in hard coding it if we can generate it.

I don't object to including more things than just libs. Users could add whatever they wanted into it as stated. We could create it with all the default args that are in the Makefile to better show what users can override I guess.

Can this be done inside the Makefile POSIX-ly ? If so then I'm on board.

Wouldn't it be something like:

HAVE_LIB_DEFAULT ?= 1
HAVE_LIB_WEBP ?= $(HAVE_LIB_DEFAULT)
HAVE_LIB_GIF ?= $(HAVE_LIB_DEFAULT)
...

And this is just my personal opinion here, but with this, I don't think we'd need a make minimal. I'd be content with just dropping -include.mk at the top and if someone wanted they could just echo "HAVE_LIB_DEFAULT ?= 1" > config.mk.

[N-R-K]

Wouldn't it be something like:

HAVE_LIB_DEFAULT ?= 1
HAVE_LIB_WEBP ?= $(HAVE_LIB_DEFAULT)
HAVE_LIB_GIF ?= $(HAVE_LIB_DEFAULT)
...

I was thinking more in terms of something like NSXIV_MINIMAL_BUILD and have that disable inotify as well. But I guess this works too.

But if we allow a var to disable all deps, then I don't see much reason to also add include whatever.mk since that env var can just be exported.

[TAAPArthur]

But if we allow a var to disable all deps, then I don't see much reason to also add include whatever.mk since that env var can just be exported.

By that logic shouldn't every config.mk not exist since the any var could also be exported? Exporting everything either risks polluting the user's env namespace and passing vars on the command line is more key strokes when building. Nothing fatal, but at least that's why I see benefit in this extra line. You just drop the settings in and don't have to keep thinking about them if you don't want to but are free to specify them every time too. Also helps if you want something inbetween all off or all on like if you want just the set that sxiv needed or something. It isn't specific to dependencies. You could also set PREFIX or AUTORELOAD with the same change.

What's the downside of adding the extra line?

[N-R-K]

What's the downside of adding the extra line?

It's including a file that won't exist. Why not just go full suckless mode with a config.mk and put all configurable vars in there?

[TAAPArthur]

What's the downside of adding the extra line?

It's including a file that won't exist. Why not just go full suckless mode with a config.mk and put all configurable vars in there?

Including a file that may not exists. It is a harmless if it doesn't exist and has merit if it does. Also opens the downs for a more sane make minimal/make auto. You haven't listed a downside.

As for why this may be more convient than including a config.mk in the repo... same reason config.def.h is more convenient than having config.h directly in the repo. So people can just drop their own settings in.

[N-R-K]

Including a file that may not exists. It is a harmless if it doesn't exist and has merit if it does. Also opens the downs for a more sane make minimal/make auto. You haven't listed a downside.

Potential confusion as this file doesn't exist nor will it be generated by anything. I guess we'll just have to deal with it via adding a comment.

---

So are we just going with HAVE_LIB_DEFAULT which only disables optional libs or do we try to disable inotify as well?

[N-R-K]

Still not fully sold on having -include custom.mk ... but anyways what should the comment be @TAAPArthur ?

Other than this, if there are no other objections then I'll proceed to squash everything and then rebase on top of master.

[0mp]

Still not fully sold on having -include custom.mk ... but anyways what should the comment be @TAAPArthur ?

You may avoid -include by instructing the users that if customization are necessary they can create a file called makefile where they

1. put their custom variables
2. add include Makefile

Since makefile has a precedence over Makefile, running make is going to read makefile instead of Makefile.

[N-R-K]

Still not fully sold on having -include custom.mk ... but anyways what should the comment be @TAAPArthur ?

You may avoid -include by instructing the users that if customization are necessary they can create a file called makefile where they

1. put their custom variables

2. add `include Makefile`

Since makefile has a precedence over Makefile, running make is going to read makefile instead of Makefile.

That works nicely, thanks. I guess we're not doing any ghost includes in the Makefile then.

[N-R-K]

Squashed, rebased and ready for review.

If there's any remaining concerns, please raise them now. After this, the build system should not receive any major changes. We can't keep changing our build system and making life harder for package maintainers.

[TAAPArthur]

You may avoid -include by instructing the users that if customization are necessary they can create a file called makefile where they

This is a great solution. I completely forgot make checks other things besides Makefile Wounder how much of that is historical vs for this specific usecase. That satisfies me and is much better than the ghost include