resurrecting-open-source-projects / scrot

SCReenshOT - command line screen capture utility
Other
511 stars 51 forks source link

Decouple the source code from autoconf #308

Closed N-R-K closed 1 year ago

N-R-K commented 1 year ago

The main purpose of this PR is to reduce the source code's dependency on autoconf, so that someone can simply run the following and have scrot compile without issue:

$ c99 -o scrot src/*.c $(pkg-config --cflags --libs ./deps.pc)

(Note: there's no intention to remove autoconf, the goal instead is to make it easy to port scrot to other build systems - including a "bare" cli build shown above.)

In addition, there are some other autoconf related cleanups being done here, such as:

N-R-K commented 1 year ago

One annoying thing right now is that the AC_CHECK_{FUNCS,HEADERS} create some -DHAVE_*=1 cli arguments - which are entirely unused by scrot and thus redundant.

The non S version of these macros (e.g AC_CHECK_FUNC) doesn't define HAVE_* automatically, but it only checks one function/header at a time, not multiple.

guijan commented 1 year ago

I just noticed PACKAGE and VERSION don't seem to be documented: https://www.gnu.org/software/autoconf/manual/autoconf-2.71/html_node/Initializing-configure.html Maybe switch to the documented PACKAGE_NAME and PACKAGE_VERSION?

N-R-K commented 1 year ago

Maybe switch to the documented PACKAGE_NAME and PACKAGE_VERSION?

Sure. But we might want to figure out when exactly these got added - if these are too new, we might face issues like #225.

guijan commented 1 year ago

GNU's tarball archive at https://ftp.gnu.org/gnu/autoconf/ says 2.60 was released in 2006 and PACKAGE_NAME and PACKAGE_VERSION are documented https://www.gnu.org/software/autoconf/manual/autoconf-2.60/html_node/Initializing-configure.html The documentation of older versions doesn't seem to be in the GNU website.

We should also figure out what systems we're willing to support. I'm happy to work on supporting any libre system that is currently maintained. Even Termux inside DivestOS running Linux 3.0, that's still actively maintained, even though Linux 3.0 is abandoned by kernel.org. I'll also work on any actively maintained proprietary system that we can have CI access to.

N-R-K commented 1 year ago

Added autoconf-archive back, doesn't seem like there's any easy way to check compiler flag with bare autoconf.

Also switched to PACKAGE_{NAME,VERSION}.

guijan commented 1 year ago

To be honest, I don't like this PR, I've rewritten the build system in Meson https://github.com/resurrecting-open-source-projects/scrot/pull/92 and it only took a few hours as someone who was much less proficient in this source tree, Meson, and Autotools. I'd keep only the fixes, such as removing src/scrotconfig.h from .gitignore and using PACKAGE{NAME,VERSION}. We won't need our source code decoupled from autoconf unless one day we decide to change the build system, then we can do it on the spot.

N-R-K commented 1 year ago

To be honest, I don't like this PR

Yeah, I kinda figured that. But I don't think there's any harm being done here, autoconf still remains (and will remain) as the recommended build method - and so if we really need to do some autoconf hackery, we can do so. But allowing for easy cli builds have a number of benefits, at least for me.

I was going to write these in more details but it quickly became obvious it'd get too long. So here's the short version:

I'm pretty much fully aware that some of the things above goes against "established/good practices" (which IMO is too often applied blindly, without considering the context. E.g something that's good for the linux kernel, doesn't mean it's good for your project too, because your project probably isn't the same size and scope as linux).

But even if none of the above convince you - this is still very much undocumented and experimental. So if it doesn't work out, we can just drop it, no issues.

N-R-K commented 1 year ago

it just becomes a massive annoyance

On this topic, there are many thing about our current autoconf setup that can definitely be improved, and I'm open towards accepting PRs for it.

I'll open a separate issue with a list of these annoyances that can (hopefully) be fixed.