Overhaul GNU Autotools usage

[alerque]

I first noticed originally packaging this for Arch that the Autotools stuff was a bit wonky but let it go and moved on. With v4.4.0 breakage (#335) I had a closer look and there are several things that are a bit ... sideways. The version stuff isn't handled right, the VCS tracked bits don't work properly between releases, the tooling like make distcheck that should catch packaging issues early just doesn't work, there are some shell scripts that echo some of their commands but not others making debugging a bit of a goose chase, and likely other bugaboos.

I don't expect everybody to be an Autotools expert (and only much pain has made me even half-way competent with it). This issue is actually specifically so if the author would like to see these issues fixed they can assign the issue to me and I'll get to it sometime in my free time. No promises about when that is, just that it will be on my list.

[genivia-inc]

An update 4.5 will be released soon with some additions to the configure and makefile scripts.

See #185 for 7z archive search, which required adding an lzma Makefile to build a small part of the LZMA SDK as a static library to link with ugrep.

I also made some minor changes in 4.5 to configure.ac so that when zlib is not available or not wanted then we don't link all the other compression libraries which we don't use, since option -z is disabled.

[genivia-inc]

Update 4.5 is released. The project autotool files won't be updated for a while now (by me).

[genivia-inc]

@alerque Do you have updates to share? If not, I'll close this issue until revisited with an update. Or I'll streamline a few things in autotools usage for upcoming releases. Nothing appears to be broken at this point.

[drok]

@genivia-inc - kindly don't close this issue, even if @alerque has not had a chance to make progress.

I am working on a vscode extension integration autotools into the editor, and one of the features is diagnostics for autotools build configurations. I've been using this project along with a few others in my testbed, as a source of common errors to test the diagnosis code.

There are real problems to be solved in the ugrep configuration, and closing this issue would only sweep them under the rug.

I plan on submitting a more detailed account of the issues I have found, and proposed fixes (PR) once the upcoming pre-release version 0.2 of my extension is published. I am currently testing this release. Here is a preview of one issue and suggested fixes as produced by my extension:

Kindly allow me about a week or so to release my extension and then give you my feedback here.

[genivia-inc]

@drok Sure, no problem. Looking forward to it.

[alerque]

I'd also like it to stay on my list. I actually started some work locally but it's easier to remember to come back to until it's done if there is an open and assigned issue. There really is some breakage lurking in here that could be fixed with proper usage of autotools.

[genivia-inc]

"Proper usage of autotools"? Please don't (overdo it). You will get into trouble, guaranteed. There are many pitfalls. I've only used autotools for the past 25 years with programming experience starting in the early 80s writing BASIC, Pascal and Z80 assembly. But got bored with that, not fully understanding the power of algorithms and theory, so pursued an academic career in CS to beef that up a bit, eventually becoming department chair, which was even more "boring" (in a way), especially when a lot of "smart" people are wrong most of the time. Holding back not to criticize them or correct them was my best political move, even though some felt compelled to try to correct me when they are provably wrong or misunderstood the problem altogether. Learn and move on. Don't let ego get in the way of being productive and helpful.

I wrote and improved a bunch of config and m4 scripts that work fine for other projects over the years. The problem is not to do it perfectly, but to deal with autotool versions and OS compatibility issues. For example, libtool is not installed on MacOS, so this needs to be installed/copied from elsewhere (not something ugrep needs though). And this stupid subdir warning crap cannot be fixed on MacOS, because Implementing the suggested changes always breaks the scripts on that OS. I've wasted enough time in the past to try to make the scripts perfect. Now I just make it work to do its business while avoiding pitfalls.

The most recent dumb issue was to discover that bash does not use the executable name for argv[0] when installed as a symlink, but rather uses the target executable name #315. That's why ug is installed as a copy, not as a symlink. Symlink to a local executable works to get the proper argv[0] to return ug, but installing as a symlink on a bin path fails and args[0] returns ugrep (the symlink target). Perhaps a hard link works. But copy it is, for now!

It's easy to criticize, but if it is an easy fix then just do it. Just make sure it works on all *nix OS not just Linux, e.g. Cygwin, MacOS, Solaris, HP-UX etc. and also with all shells e.g. bash, dash, fish, zsh, csh, tcsh.

EDIT: clarify first sentence. "Learn and move on. Don't let ego get in the way of being productive and helpful." which I had meant to convey.

[drok]

With all respect for lessons learnt through navigation of the political currents of office life, sometimes there are technical reasons why prevailing consensus ought to be set aside in favour of a clean, maintainable, implementation, especially when the merit is proven. Sometimes the authority of bosses is not derived from expertise, but from rank, and a good boss will appreciate a technically superior solution, even at the cost of a bruised ego.

The early releases of autoconf were an M4 spaghetti mess compared to modern (v2.65+). In the 'old days' developers had to figure out from scratch how to detect various features. Many/most inconsistencies have been addressed over time. A modern configure.ac script should be very readable to someone with no m4 experience, and also easy to write without m4 experience. autoconf-archive is a gold mine for abstracting nitty-gritty portability issues away, and also for avoiding the reinvention of the wheel.

I have noticed in the git history (1c131c3, e0aa524, 6be7f9d) how the cp2bin target was initially defective, and it has gone through some unconvincing re-iterations. That was an example of rolling-your-own rather than use long-documented install-hooks, and it remains broken in ways you've not yet discovered. The lesson to be learned there was, to avoid reinventing the installation wheel. Automake's creators have also spent many years specifically understanding and abstracting compatibility issues like this.

My proposed changes will also amount to a complete overhaul of the build automation, but I think you will love it, Dr. van Engelen. I find it difficult to imagine the scripts getting into more trouble than they are now, although I've taken your guarantee under advisement.

Are there any plans to implement a test suite (ie, make check)? I see there are some tests in tests/verify.sh, awkwardly hooked via a non-standard test target in the top directory, but they eschew use of the built-in Parallel Test Harness - perhaps another reinvention that can be avoided. My vscode extension automatically discovers and exposes tests in the Parallel Test Harness, but of course has no hope of handling home-grown test infrastructure. The point of the extension is to expose the typical capabilities of a software project (edit, build, test, debug, package), as long as the project does adhere to autotools documention.

@alerque it seems we are headed for needless duplication of effort - can we coordinate? I have pushed my first (tiny, compared to what I have planned) change to Mergesium:autotools and will create a PR when I'm ready. I'm now looking into lib/Makefile.am which I intend to remove. Would you push what you have done so far so I can build on top of it?

[genivia-inc]

@drok sounds good! @alerque please coordinate efforts, thanks!

Indeed, my past experiences to work with (or around) legacy tools has made me more cautious and a bit of a do-it-yourself kind of guy.

[genivia-inc]

Adding to the list of pitfalls: do not remove SIMD_FLAGS="-march=native -DHAVE_NEON", because NEON builds (ARM6) fail, e.g. RPi zero. Using -march=native is ugly and should not be done, but we need this flag until this is fixed in GCC (good luck). This is the only way to get it to build. I spent over a day searching and trying alternatives back when I wrote this. Something like this is sideways, but necessary at the time.

[drok]

Re SIMD_FLAGS I assume you are referring to this change: https://github.com/Genivia/ugrep/compare/master...Mergesium:ugrep:autotools#diff-4cb884d03ebb901069e4ee5de5d02538c40dd9b39919c615d8eaa9d364bbbd77L2-R13

I am removing SIMD_FLAGS from the CPPFLAGS variable because previously/currently, as you've shown, it was used as both a pre-processor variable and a compiler option variable; a mixed-purpose unclean practice. SIMD_FLAGS is used in the portable detection script AX_EXT strictly as a compilation flag as described here; preprocessor defines like HAVE_{extension} will be set in config.h which is used at pre-processing time via the libreflex_a_CPPFLAGS = -include $(CONFIG_HEADER) snippet.

I am separating compiler flags from preprocessor flags to bring the scripts to a standard, boring, canonical alignment with automake flag variables documentation - Casually assigning *FLAGS variables is one root cause for the current mess, so naturally it's one of the main things I'm cleaning up. I apologize if I sound critical, but note I'm working on a solution rather than merely pointing out the problems.

The AX_EXT.m4 script as distributed with autoconf-archive does not detect ARM flags, and I haven't found an equivalent script that supports ARM; I will likely amend the existing AX_EXT.m4 to add ARM detections. It would help if your continuous integration scripts would test building on ARM and other platforms you support, like various CPUs or OS's.

A plausible way you can claim to support other OS's without specifically testing them is if your build system follows the autotools documentation and best practices to the letter or deviates only with the greatest care. Presumably that's why you're using autotools, for the wide OS/compiler compatibility it brings.

I'm no ARM expert, so I hesitate to add support to AX_EXT.m4 - maybe that's something you can do. It would need to detect CPU capabilities, possibly using special purpose feature registers, even though your project doesn't need them. Detecting target features with the -march=native option definitely breaks cross-compilation, which is a popular way to screw up autotools automation, and would not be acceptable in AX_EXT.m4. That flag needs to go, GCC bug or not.

Whether there are GCC bugs related to feature detection should not be your concern as a software developer. It is the purpose of the automake maintainers to work out how to support buggy software, and track when or if that software is fixed and when and if the workarounds should be removed. Normally workarounds should never be removed, so it is possible at any point in the future to build the software on a system where only buggy compilers are available. That's one of the core values of using autoconf in the first place. Don't try to get around it with hacks, it's trying to help you focus on big-brain algorithms, instead of the grunt work of build automation scriptology.

Do you have notes on what the "GCC bug" is, or what issue you were solving and your findings? I am not familiar with the intimate history of this project. I can read the (terse) git commit messages, linked issues, and extrapolate here and there, but I'm not aware of what bug you were trying to work around.

[genivia-inc]

RPi and other SBC use older compiler versions. So even if GCC has addressed this, it won't be something to count on for a while.

Just try to test build on RPi zero with GCC (default) with and without -march=native and -mfpu options. You'll see what I mean. It built fine on RPi3, but not on RPi zero with GCC. I don't remember the exact details from about two/three years ago and it probably was logged with the RE/flex project that ugrep uses. I sure want to build on RPi SBC like the zero. This is the only way I could get it to work to detect NEON and compile for NEON. There are some posts online that share some similarities with the problems I saw, although not 100% the same, like this one for example https://stackoverflow.com/questions/37050532/detect-arm-neon-availability-in-the-preprocessor so there is certainly some funny stuff going on that cannot be cast aside as irrelevant.

Whether there are GCC bugs related to feature detection should not be your concern as a software developer.

When tooling bugs (GCC including) cause failed builds, issues will be opened to fix. Why not fix them up front? I'm not talking about feature detection as the main problem. Failed builds are a problem. Wrong feature detection is of course also bad when builds fail or when binaries target the wrong CPU.

There is a reason why configure.ac is structured the way it is with explicit code to detect CPUs and enable the necessary compile-time flags. I had tried other "accepted" solutions posted online, but those did not work (ergo the NEON problem was the Achilles heel for me). We must use both compile time and runtime AVX2/AVX512 detection for runtime versioning with have_HW_AVX2() and have_HW_AVX512BW() to run the properly compiled code. Messing with these will break ugrep's binary portability on SSE2/AVX2/AVX512 systems (at least SSE2 support is expected when compiled on a SSE2/AVX2/AVX512 capable machine). When built on a non-SSE2 machine, ugrep simply never assumes SSE2, AVX2 or AVX512 is available nor use that code. The only missing part right now in configure.ac is to always compile AVX512 also on an AVX2 machine, so the performance is there when binary ported from an AVX2 machine to an AVX512 machine. Right now, when build on an AVX2 machine, the AVX512 capabilities are disabled (not compiled).

EDIT: fix typo

[drok]

Thank you, that explains the wonkiness, and lead to the relevant SO answer on How to check the existence of NEON on arm

[genivia-inc]

Back when I wrote the configure.ac code I made sure it detects NEON/AArch64 and doesn't fail detection and doesn't fail to build ugrep and RE/flex. I verified this with RPi3, RPi zero, Android device with termux, and Mac M1. The SSE2/AVX2/AVX512 detection is easy and can be done with another existing solution, but it has to align with the NEON/AArch64 detection and compilation that I verified to work. So I thought it would be better to do this explicitly with AC_COMPILE_IFELSE and AC_LANG_PROGRAM and not rely on another (third-party) m4 solution to detect SSE/AVX and try to adapt that to make it work somehow.

I agree that AC_COMPILE_IFELSE and AC_LANG_PROGRAM and awkward use of -march=native is something you don't typically see in configure.ac scripts. But ugrep and RE/flex aren't your typical projects either.

I also found over time that there aren't any m4 scripts for PCRE2 and some of the compression libraries. So I wrote these and sent the pcre2 one to the GNU org: https://www.gnu.org/software/autoconf-archive/ax_check_pcre2.html

These are other m4s I wrote that are not posted on the GNU org: ax_check_brotlilib.m4 ax_check_bz2lib.m4 ax_check_bzip3lib.m4 ax_check_lz4lib.m4 ax_check_lzmalib.m4 ax_check_zstdlib.m4

I also wrote a new ax_boost_regex.m4 because the one that GNU org posts does not appear to work as documented.

Perhaps other folks may find them useful? If so, let's send them to the GNU org.

[drok]

The reason pcre2 m4 scripts don't exist is because they are not needed. pcre2 ships pkg-config .pc files, and thousands of projects that depend on pcre2 had no problem with them (see Mergesium@6978c50). Your ax_check_pcre2.m4 file is a disservice to uninformed devs, who may believe it the proper way to integrate pcre2 in their projects, and makes it look like autotools requires all these elaborate hacks to accomplish the most basic of tasks. 👎🏻

As for the other compression detection m4's, I propose in #370 you remove them for the same reason. There is no reason to reinvent the wheel when a proper way is documented in the automake manual.

ax_check_brotlilib.m4 -> see b596048 ax_check_bz2lib.m4 -> see 8a47971 ax_check_bzip3lib.m4 -> see ebe6400 ax_check_lz4lib.m4 -> see a27012a ax_check_lzmalib.m4 -> see 9423c6b ax_check_zstdlib.m4 -> see 4d2b26d

You say "ugrep and RE/flex aren't your typical projects" - I disagree. They are straight-forward projects that don't require any special consideration outside documented autotools features that have existed for many, many years. In the PR #370 I removed all of special scripts, because they added complication and broke cross-compiling, portability and readability. All hooks are removed, except for "install-exec-hook", which is implemented as per automake manual to symlink binaries.

[genivia-inc]

Nice work.

So to verify a few things, I tested the PKG_CHECK_MODULES approach for PCRE2 instead of the m4 file on a MacOS M1 and I've regenerated configure. More specifically, this proposed change:

https://github.com/Genivia/ugrep/pull/370/commits/6978c50c0d60226a076ea1ff78ea4d34ed1be1c4

fails to link pcre2:

Undefined symbols for architecture arm64:
  "_pcre2_code_copy_with_tables_8", referenced from:
      reflex::PCRE2Matcher::pattern(reflex::PCRE2Matcher const&) in ugrep-ugrep.o
      reflex::PCRE2Matcher::PCRE2Matcher(reflex::PCRE2Matcher const&) in ugrep-ugrep.o
...

Part of the problem might be the installers on MacOS such as Homebrew place libs in their specific /opt/homebrew/ locations that are tested in my m4. Where, besides the PKG_CHECK_MODULES in configure.ac is this change applicable to link against pcre2?

A second thing I tried is something that annoyed me relentlessly about autotools, because of the subdir warnings. So I was hoping for a miracle. But the _INIT_AUTOMAKE([foreign subdir-objects]) won't work either, as I suspected, because this never reliably worked on MacOS for other projects. Indeed, trying https://github.com/Genivia/ugrep/pull/370/commits/83b91d1b1d315df46824dc6478fc3e32899c1dfc gives:

$ aclocal
$ autoheader
$ automake --add-missing --foreign
$ autoconf
$ automake
configure.ac:3: error: AM_INIT_AUTOMAKE expanded multiple times
/opt/local/share/aclocal-1.16/init.m4:29: AM_INIT_AUTOMAKE is expanded from...
configure.ac:2: the top level
/opt/local/share/aclocal-1.16/init.m4:29: AM_INIT_AUTOMAKE is expanded from...
configure.ac:3: the top level
autom4te: error: /opt/local/bin/gm4 failed with exit status: 1
aclocal: error: autom4te failed with exit status: 1

EDIT: aha, I made the changes and AM_INIT_AUTOMAKE works. I was skeptical, because a couple of years back the same AM_INIT_AUTOMAKE did not work on MacOS for another project and I am sure we made the exact recommended changes, probably because a few year back we still were forced to use older autotools. This looks better now.

[genivia-inc]

As for the other compression detection m4's, I propose in #370 you remove them for the same reason. There is no reason to reinvent the wheel when a proper way is documented in the automake manual.

ax_check_brotlilib.m4 -> see b596048 ax_check_bz2lib.m4 -> see 8a47971 ax_check_bzip3lib.m4 -> see ebe6400 ax_check_lz4lib.m4 -> see a27012a ax_check_lzmalib.m4 -> see 9423c6b ax_check_zstdlib.m4 -> see 4d2b26d

You say "ugrep and RE/flex aren't your typical projects" - I disagree. They are straight-forward projects that don't require any special consideration outside documented autotools features that have existed for many, many years. In the PR #370 I removed all of special scripts, because they added complication and broke cross-compiling, portability and readability. All hooks are removed, except for "install-exec-hook", which is implemented as per automake manual to symlink binaries.

Sigh. These suggested changes do not work on MacOS, probably for the same reason PCRE2 is detected but does not link. I will also try a Mac Intel Monterey which are still frequently used, but that's likely to throw the same errors. When I said that this project that isn't typical, it means that I've tested portability to many platforms, not just Linux or BSD Unix that have autotools installed and made sure SIMD is detected and compiled, which wasn't working for ARM6 with the usual stuff back then.

After adding PKG_CHECK_MODULES([ZSTD], [libzstd], to replace AX_CHECK_ZSTDLIB:

$ aclocal
$ autoheader
$ automake --add-missing --foreign
$ autoconf
$ automake
$ ./build.sh
...
checking for libzstd... yes
...
g++ -std=gnu++11  -Wall -Wextra -Wunused -O2  -L/opt/homebrew/lib -L/opt/homebrew/lib -L/opt/local/lib -L/opt/local/lib -L/opt/local/lib -L/opt/local/lib -L/opt/homebrew/lib -L/opt/homebrew/lib -L/opt/homebrew/lib -L/opt/homebrew/lib -L/opt/homebrew/lib -L/opt/homebrew/lib -L../lzma/C -o ugrep ugrep-ugrep.o ugrep-cnf.o ugrep-glob.o ugrep-output.o ugrep-query.o ugrep-screen.o ugrep-stats.o ugrep-vkey.o ugrep-zopen.o -lpthread ../lib/libreflex.a -lviiz -lbrotlidec -lbrotlienc -llz4 -llzma -lbz2 -lz -lpcre2-8 
...
Undefined symbols for architecture arm64:
  "_ZSTD_DStreamInSize", referenced from:
      zstreambuf::open(char const*, __sFILE*) in ugrep-ugrep.o
      zstreambuf::next(unsigned char*, unsigned long) in ugrep-ugrep.o
...

[drok]

Part of the problem might be the installers on MacOS such as Homebrew place libs in their specific /opt/homebrew/ locations that are tested in my m4. Where, besides the PKG_CHECK_MODULES in configure.ac is this change applicable to link against pcre2?

The PCRE2_LIBS and PCRE_CFLAGS variables calculated by PKG_CHECK_MODULES are used in src/Makefile.am in the ugrep_LDADD and ugrep_CXXFLAGS variables respectively: https://github.com/Genivia/ugrep/commit/6978c50c0d60226a076ea1ff78ea4d34ed1be1c4#diff-4cb884d03ebb901069e4ee5de5d02538c40dd9b39919c615d8eaa9d364bbbd77

I also noted in the commit message for 6978c50 that it is WIP. Specifically I got undefined references when compiling with -O2 but not with -O0 - something is amiss, but I don't know what, yet. It's possible that your undefined reference to _pcre2_code_copy_with_tables_8 is a clue.

[drok]

-O2

Would you please try with -O0 to confirm whether you are also seeing a sensitivity to optimization? I have a feeling that the problem is not with PKG_CHECK_MODULES at all, but some other missed flag, or possibly the order the libs are enumerated on the link cmd line.

In any case, a hardcoded path list /usr/local /usr /opt/homebrew /opt/local /sw may very well work for you, but it breaks cross compilation, and it won't work for distros that have another filesystem layout convention. To be portable, you cannot rely on hardcoded paths at all, even those that you tested and work for you.

When cross-compiling, for instance, pkg-config searches a list of paths that include sysroots but does not include the system's running paths. In that case, all the paths you list are wrong places to look for the libraries; those places hold libs for the wrong architecture, not the target architecture on the --host= configuration option.

[drok]

SIMD is detected and compiled, which wasn't working for ARM6

ARMv6 does not have hardware FPU and thus would never support SIMD extensions if I understand correctly. Is there a reason to expect SIMD to be supported when targeting ARMv6 ?

ARMv7 has optional NEON support (it's up to the CPU manufacturer) ARMv8 NEON support is mandatory, and there is no -mfpu=neon flag because it's always on.

I have no hands-on experience with ARM's; this is my understanding from reading documentation.

[genivia-inc]

SIMD is detected and compiled, which wasn't working for ARM6

ARMv6 does not have hardware FPU and thus would never support SIMD extensions if I understand correctly. Is there a reason to expect SIMD to be supported when targeting ARMv6 ?

ARMv7 has optional NEON support (it's up to the CPU manufacturer) ARMv8 NEON support is mandatory, and there is no -mfpu=neon flag because it's always on.

I have no hands-on experience with ARM's; this is my understanding from reading documentation.

I may have confused ARM6 with ARM7 in my wording. In this case I didn't keep notes with details, because I got it working fine to detect NEON properly. I'm running an RPi zero again this morning to check, and to set it up for testing the suggested changes. The problem was either that NEON was not detected or detected incorrectly. Incorrect detection ran into a crash, obviously. This is what I remember battling with a few years back. Giving up NEON was not an option for me.

Got it with the PCRE2_LIBS part and the other library dependencies, which test OK now. However, I find this required change to the Makefile.am ugly and suspect it increases maintenance fragility. Why not accumulate the library dependencies in LIBS and LDFLAGS in configure? That approach will never lead to a problem when a Makefile.am omits a library-dependent XYZ_LIBS, for example when a new optional dependency is introduced and thus more scripts need to be changed. Perhaps I'm ignoring best practices by accumulating library dependences in LIBS, but not doing so looks wrong to me as more fragile. And yes, for cross-platform library paths it is not great to have the paths in the ax check m4 scripts in the way folks used to rely on in the past for decades.

The completions compgen scripts I wrote to generate fish and zsh completion files save me time from hand-coding the completion files when options change. But the scripts only need to be run when options have changed. The man.sh script must be run for each release to update the auto-generated man page version stamp and contents, mostly from the ugrep --help output of options modified with sed. I still run man.sh by hand, because I have to copy the generated man.txt in the README. All these scripts use sed extensively. Sed has differences across platforms, so that can be tricky to run universally if not cross-platform tested carefully beforehand. So I do not auto-generate the man page with a makefile from a template to avoid potential sed or other tool failures. Using sed is great, as long as it works for the more complex regex I have to use.

Why #!/bin/bash instead of #!/bin/sh? If the script is generic, non-bash, then it shouldn't require bash. If a script is bash-specific, then #!/bin/dash is a better choice for efficiency. Either way, if bash is not installed the script won't run so I prefer #!/bin/sh and keep things simple.

The extensive use of @ variables such as @PACKAGE_NAME@ in Makefile.am is overdoing it IMHO. The project name won't change and using these variables makes scripts less readable to those who aren't skilled in autotools. I have the same concern with PACKAGE_VERSION. The version string should be kept literally in ugrep.hpp. Remember that the project should also be compiled without autotools, such as with MSVC++. Sure, MSVC++ nowadays supports makefiles etc. but doing all that adds more baggage than it is worth. For RE/flex I used sed in a makemake.sh script to substitute the version number into the source code, which means the source compiles on MSVC++ without worry.

The --disable-simd makes sense, to update from --disable-sse2. I used to have only SSE2/AVX2 support in ugrep and RE/flex, not NEON/AArch64, hence this option. Why did I not change it back then? Because there might be scripts out there that use --disable-sse2 and those scripts will now fail, unless this option can also be used as an "legacy" alternative to --disable-simd.

Other suggested changes look in principle alright to me. But I need to spend more time reading the changes to check them and to run tests.

[genivia-inc]

To follow up on ARM6: RPi zero build will not work with the current configure script. It detects NEON when it's not supported. I don't remember the details when I worked on this, but I may have given up on ARM6 non-NEON detection in favor of making sure NEON is detected on other ARM that supports it.

If we can make detection work properly on all ARM with and without NEON, then that is a win 👍

[drok]

Why #!/bin/bash instead of #!/bin/sh? If the script is generic, non-bash, then it shouldn't require bash. If a script is bash-specific, then #!/bin/dash is a better choice for efficiency. Either way, if bash is not installed the script won't run so I prefer #!/bin/sh and keep things simple.

You are referring to ee4bf15 - the commit comment explains the reasoning for the change. I will quote the relevant paragraph below for your convenience:

The compgen file uses bash syntax ($'...') so I changed the shebang to
require bash instead of sh.

[drok]

To follow up on ARM6: RPi zero build will not work with the current configure script. It detects NEON when it's not supported.

If the compiler installed on your device is capable of generating NEON code, then NEON code should be generated. The presence of NEON features should be done at runtime, and the NEON code paths used only the the feature is available, at runtime. That is the point of b4ba7e5 - it's very possible I implemented the gating or detection wrong - please review.

So either the HWCAP_NEON detection is wrong, or the feature gating on HW.neon is wrong. You'll have to review b4ba7e5 in detail - I don't have arm hardware.

[drok]

I still run man.sh by hand,

I had not even noticed your man.sh script, since you also keep the generated file in git.

Having a look at it, it's not clear to me that this script does anything useful that autotools doesn't support out-of-the-box. Generating files from templates is literally what autoconf's main purpose is. Why reinvent it, poorly?

You have a hand-written manpage file wrapped in a shell script that prepends a header with the current date and an arbitrary version string given as argument on the command line. Autoconf has logic built in, and accessible as (0767373):

# Allow the manpage be accessed as either ug or ugrep
AC_CONFIG_FILES([man/ugrep.1])
AC_CONFIG_LINKS([man/ug.1:man/ugrep.1])

[drok]

The extensive use of @ variables such as @PACKAGE_NAME@ in Makefile.am is overdoing it IMHO. The project name won't change

@PACKAGE_NAME@ and @PACKAGE_VERSION@ are placeholders in the template file man/ugrep.1.in that the configure script replaces with the correct values at configuration time.

You may well not change the project name yourself, but being open source, someone may fork and build it for their purposes with a different name, and with customizations of their choosing. Of course they would call it something else, as to not confuse themselves (eg, mygrep is not the same as ugrep).

Some user may well decide to replace GNU grep with your program, on one or all their systems, or maybe even use it as the default grep in their distro, so that it is used in all scripts and every place where GNU grep was used previously. Ie, they make your program a drop-in replacement for GNU grep. They would naturally want it installed in /bin/grep and mark it in their package repo as an alternative to GNU grep. The command 'man grep' should then bring up your manpage, and refer to the program as 'grep' and not 'ugrep'.

I have kept the name 'ugrep' in the copyright notice and issues link at the bottom of the manpage, as there it should really refer to your project ("ugrep", immutable), even if the program name is "grep" (or @PACKAGE_NAME@)

I would not worry about the computer literacy of users that might come in contact with your source code. If they don't know that the "@word@" notation denotes a variable name, they won't know how to run configure or make, and they won't be editing anything. They would more likely be confused by the funny slashes and dots and junk that are manpage syntax. What's the real concern here? You're not worried about confusing them with manpage syntax, but worry about using a variable name instead of hardcoded name for the program name? Your reasoning doesn't add up.

[drok]

The --disable-simd makes sense, to update from --disable-sse2. Why did I not change it back then? Because there might be scripts out there that use --disable-sse2 and those scripts will now fail

IMHO it is appropriate to make those scripts fail. That failure is how to signal to the maintainers of those scripts that the configuration options have changed and they need to review the changelog/release notes.

I think you may have missed that I also replaced the --with-zlib package configuration option with --enable-zlib and --disable-zlib which is how you normally enable/disable features of a program. --with is used to request an alternate library be used such as --with-zlib=/home/blah/my-special-zlib (as opposed to the default zlib package installed on the system). The same --enable/--disable convention is now used for all compressions.

distros that package ugrep will have to review configure --help and fix up their package configurations.

Did you know that debian builds with --disable-avx on x86 and --disable-neon on arm? It seems related to https://bugs.debian.org/964957 and appears that the package maintainers at debian have to jump through some hoops to distribute ugrep

I think you should document, when you release an overhauled version, that simd detection is done at runtime, and no longer does SIMD need to be disabled in order to make the binary portable to the target architectures (ie, add the standard INSTALL file to explain unusual expectations). Of course, NEON detection should be fixed to work correctly on all CPUs

[genivia-inc]

So, if I understand this correctly, there is no clear advantage to using @ project variables in scripts for project maintenance, besides creating an assumed dependence on autotools when using these as macros in source code, right?

Overdoing it only adds to fragility, i.e. if something goes wrong or is no longer available, then the problem is no longer isolated. When we don't use autotools for example, or when autotools is replaced with something else or autotools v10.0 comes out and we can't build legacy projects with it any longer. Similar things happened decades ago with projects that required specialized software to run to create and maintain it, when this specialized software is no longer available, and worse, the binaries run on obsolete CPUs.

[genivia-inc]

This is my short list viz. battle plan:

1. refactor makemake.sh to run to auto-update the source code tree, man page, completions and configure.ac when bumping a version for release
2. only use makefiles when appropriate, i.e. to track source code dependencies to compile/link code, no makefile abuse as if these are shell scripts
3. ergo, keep man/ugrep.1 updating and completions updating out of makefiles, use scripts, no makefile abuse
4. ergo, keep the ugrep/patterns/ out of makefiles, these are static data to be installed in datadir with a hook; creating makefiles adds fragility to maintenance
5. stay clean of required dependence on autotools and/or cmake, i.e. source code must be compilable and linkable without autotools or cmake, recall that autotools/cmake are just tools/hacks to make compiling and linking independent of a platform, it was never meant to be a monopolistic (take it or leave it) system for compilation and linkage, which can always be done "by hand" or with a makefile or with a custom script or with MSVC++ project properties
6. fix detection and linkage of libraries such as https://github.com/Genivia/ugrep/pull/370/commits/2fd7710e88caf164acb99f4ed119236bae87b74c
7. fix SIMD detection https://github.com/Genivia/ugrep/pull/370/commits/201c50840ab4d65cfbd36d2587748cccc1cce2c6 in particular NEON https://github.com/Genivia/ugrep/pull/370/commits/b4ba7e5ec0cace0c40bc7437f447ff56a0ec6366

Point 7 is important to address for both accurate compile-time and runtime use of SSE2/AVX2/AVX512BW and compile-time NEON/AArch64 use (no runtime check).

[drok]

After adding PKG_CHECK_MODULES([ZSTD], [libzstd], to replace AX_CHECK_ZSTDLIB:

$ ./build.sh
...
checking for libzstd... yes
...
g++ -std=gnu++11  -Wall -Wextra -Wunused -O2  -L/opt/homebrew/lib -L/opt/homebrew/lib -L/opt/local/lib -L/opt/local/lib -L/opt/local/lib -L/opt/local/lib -L/opt/homebrew/lib -L/opt/homebrew/lib -L/opt/homebrew/lib -L/opt/homebrew/lib -L/opt/homebrew/lib -L/opt/homebrew/lib -L../lzma/C -o ugrep ugrep-ugrep.o ugrep-cnf.o ugrep-glob.o ugrep-output.o ugrep-query.o ugrep-screen.o ugrep-stats.o ugrep-vkey.o ugrep-zopen.o -lpthread ../lib/libreflex.a -lviiz -lbrotlidec -lbrotlienc -llz4 -llzma -lbz2 -lz -lpcre2-8 
...
Undefined symbols for architecture arm64:
  "_ZSTD_DStreamInSize", referenced from:
      zstreambuf::open(char const*, __sFILE*) in ugrep-ugrep.o
      zstreambuf::next(unsigned char*, unsigned long) in ugrep-ugrep.o
...

Whatever gymnastics the build.sh script does, it is wrong. The linker command you showed does not include -lzstd, which is the reason for the "Undefined symbol _ZSTD_DStreamInSize".

The commit that introduces PKG_CHECK_MODULES([ZSTD], [libzstd] (4d2b26d) adds $(ZSTD_LIBS) to the program's LDADD flags, which you might have omitted.

I propose deleting build.sh - the correct way to build, which correctly handles dependencies is simply 'make' - nothing more is needed.

[genivia-inc]

After adding PKG_CHECK_MODULES([ZSTD], [libzstd], to replace AX_CHECK_ZSTDLIB:

$ ./build.sh
...
checking for libzstd... yes
...
g++ -std=gnu++11  -Wall -Wextra -Wunused -O2  -L/opt/homebrew/lib -L/opt/homebrew/lib -L/opt/local/lib -L/opt/local/lib -L/opt/local/lib -L/opt/local/lib -L/opt/homebrew/lib -L/opt/homebrew/lib -L/opt/homebrew/lib -L/opt/homebrew/lib -L/opt/homebrew/lib -L/opt/homebrew/lib -L../lzma/C -o ugrep ugrep-ugrep.o ugrep-cnf.o ugrep-glob.o ugrep-output.o ugrep-query.o ugrep-screen.o ugrep-stats.o ugrep-vkey.o ugrep-zopen.o -lpthread ../lib/libreflex.a -lviiz -lbrotlidec -lbrotlienc -llz4 -llzma -lbz2 -lz -lpcre2-8 
...
Undefined symbols for architecture arm64:
  "_ZSTD_DStreamInSize", referenced from:
      zstreambuf::open(char const*, __sFILE*) in ugrep-ugrep.o
      zstreambuf::next(unsigned char*, unsigned long) in ugrep-ugrep.o
...

Whatever gymnastics the build.sh script does, it is wrong. The linker command you showed does not include -lzstd, which is the reason for the "Undefined symbol _ZSTD_DStreamInSize".

The commit that introduces PKG_CHECK_MODULES([ZSTD], [libzstd] (4d2b26d) adds $(ZSTD_LIBS) to the program's LDADD flags, which you might have omitted.

I propose deleting build.sh - the correct way to build, which correctly handles dependencies is simply 'make' - nothing more is needed.

Seems you make an attempt to deflect.

The problem above has nothing to do with ./build.sh, which just runs configure, make, and make test. Nothing "magical" happening there.

Read my comment why this failed on my end, because the Makefile.am now requires extra $(ZSTD_LIBS). It links with that, but this approach adds maintenance fragility as I've explained. If all optional libraries need a $(XYZ_LIBS) in makefiles, then I object in principle, because not all libs are required but optional.

Why not add $(ZSTD_LIBS) to LIBS when configuring, so we don't have to mess with lib/Makefile.am? After al, LIBS is intended to collect library dependencies.

[genivia-inc]

Instead of focussing on criticizing my decision to use scripts makemake.sh and build.sh to prep and build with commands that folks typically run by hand from the command line, please read my seven points that actually address the reasonable parts of your concerns in the PR, but without overdoing the overhaul such as by adding a lot things that do not matter and/or that makes things worse in terms of maintenance and project evolution.

These seven points support you PR partly with 6 and 7, and 1 to update AC_INIT that you argue is important, but rejects all notions that makefiles are good for everything, including for static data content such as patterns. It also rejects an encroachment of "autotools or die" into the project.

[genivia-inc]

When reviewing the PR, I see several code changes that break binary portability. I left a comment and explained why that is the case. For example, when compiled on a AVX512 machine, the run-time test for AVX512 and also AVX2 must BOTH be there. If the first fails at runtime, the code may run on an AVX2-capable CPU. You are making a mistake here to remove that second possibility.

If you do not understand the logic, do not change the code!

[genivia-inc]

https://github.com/Genivia/ugrep/pull/370/commits/10e0f69dbd0ecc965c93d5cc20ca5f17ac0c858b

This is wrong. Bash completion requires this bash completion installation to be present. If you don't believe me, you can try on MacOS which does not have this third-party installation and therefore the bash completions I wrote will not work.

[alerque]

Speaking with my distro packager hat on (thousands of packages on Arch, Nix, PLD etc.), you don't want to depend on shells being installed or having completion stuff being around, you just want to provide completions and if folks have the tooling that uses them then they get the benefit. Some software does try to detect shells or require them or parts of them as dependencies but this is something we have to patch out to get it packaged.

@grok I have some m4 macro files that handle shell completions, see this bit and more here. The AX_SHELL_COMPLETION_DIRS obviously goes in configure.ac and the other bit gets included into Makefile.am plus a few other bits. I've used it for a few projects and it's pretty robust across distros. That implementation is setup for Rust builds but I can adapt it to this. I should get it submitted to autoconf-archive. If you want I'll PR that against yours too to keep the configure.ac as clean and minimal as possible with macros for handling the dirty work across platforms.

[genivia-inc]

@alerque I agree. I don't include these compgen.sh shell scripts to be run by others to create completions. Completions are created up front on my end once and for all and then included with the repo. The scripts I've used to create them are also included for safe keeping and future maintenance. But nobody needs to run these to install the package. Creating makefiles to run compgen.sh and man.sh and for patterns makes no sense. These files are static.

[alerque]

Okay we're talking about two different things here ....

But switching to your topic, if they are generated then the tooling to generate them should be part of the Makefile.am and used when make dist is run. The generated files also shouldn't be tracked. People using the tarball source releases will not need the tooling to generate them because fresh ones will come pre-packaged, but that doesn't mean the tooling doesn't get tracked. Same for man page tooling.

[genivia-inc]

https://github.com/Genivia/ugrep/pull/370/commits/ebe6400c7b19e92176539f0ba93f1a58397b5b64

This is wrong, because bzip3 is not linked by default. It is only linked with --with-bzip3 as a third-tier optional library. Recommended are 1st tier, optional are 2nd tier included by default, 3rd tier are optional not included by default.

[genivia-inc]

Okay we're talking about two different things here ....

But switching to your topic, if they are generated then the tooling to generate them should be part of the Makefile.am and used when make dist is run. The generated files also shouldn't be tracked. People using the tarball source releases will not need the tooling to generate them because fresh ones will come pre-packaged, but that doesn't mean the tooling doesn't get tracked. Same for man page tooling.

There are indeed two related threads: A) no dependence on shells means no scripts to run, which I never required installers to do. I don't know where this inclination came from. The build.sh is only there for convenience when folks download and install. The compgen.sh and man.sh are mine to run, just included for safe keeping and future maintenance (by others perhaps). B) with respect to completions, the bash completions do need some completion support that configure checks. That's all.

I should add that creating makefiles for the man page, completions, and patterns makes no sense. It's static data. It would also require running the shell scripts, which you both agree/disagree on? What is it? I don't require any shells to be run to install the repo or prepare it, except on my end when I make code changes. It is my way of saving time to not to have to edit files and to assure all is in sync.

[genivia-inc]

You say "ugrep and RE/flex aren't your typical projects" - I disagree. They are straight-forward projects that don't require any special consideration outside documented autotools features that have existed for many, many years. In the PR #370 I removed all of special scripts, because they added complication and broke cross-compiling, portability and readability. All hooks are removed, except for "install-exec-hook", which is implemented as per automake manual to symlink binaries.

Well, that was an epic fail. Almost nothing worked in the PR #370 when I ran tests on several platforms and I explained why I ran into trouble and why the suggested logic is incorrect in several parts. So, yeah, this is not a typical project. Also some of the PR #370 suggestions are not aligned with the project. They appear to be based on a knowledge base of general recommendations, I assume. Correct me if I am wrong.

What I have verified and done so far:

• Using PKG_CHECK_MODULES for compression libraries is OK, but it was poorly executed because some compression libraries are recommended, others are optional when present or optional on demand only. This makes them a bit different in the way they should be checked and reported.
• Cross-compilation in PR #370 failed. See my latest comments #370 why ax_ext.m4 is trash to check SIMD CPU extensions. Parts of it also use CPUID, which is on the build machine, not the host machine.
• Removing ./build.sh is out of the question. Removing configure and other scripts breaks manual installation after download, because folks can no longer build with ./configure && make like folks typically do after downloading a project repo.
• Creating makefiles for static content (man page, completion files, pattern files) is unnecessary and complicates things, having to juggle more moving parts. This increases maintenance fragility.
• Removing the windows binaries is fine, but where should these go? The release artifact is always one zip file that Winget package releaser workflow uses, so we cannot use that without creating some conflicts. Perhaps can be fixed in the future, it is not a high priority.

I rewrote configure.ac and added cross-compilation checking and removed the dreaded -march-native. I also use PKG_CHECK_MODULES when applicable to check libraries and smoothed some things out. Verified with several platforms so far to work fine, in particular ARM6, ARM7, AArch64, MacOS M1, MacOS intel, Cygwin, Android termux.

My draft configure.ac is work in progress, nearing completion. Feel free to use it. Comments and suggestions are welcome.

[drok]

I have some m4 macro files that handle shell completions, see this bit and more here.

@alerque - In these m4's, would you clarify the thought process:

Bash-completions

These scripts ultimately lead to a package's completions going to $datadir/bash-completion, which is a directory owned by the the third party program "bash-completion". This program is not associated with bash, or the package seeking to install its completions into bash.

bash-completion seems to be a third-party library of useful completions, a collection/archive of sorts. They seem to provide completions for programs that didn't care to provide their own.

If a user were to install a program that has completions, like ugrep here, and removed the bash-completion package as unnecessary, they would not be able to use their ugrep completions, because the 'hook' /etc/profile.d/bash_completion.sh which is provided by the bash-completion is gone.

This means the bash-completion package is a middle man.

Q: Does installing completions into /usr/share/bash-completion rather than /etc/bash.completion.d (which is bash's documented repo of completions for installed programs), not create a problem instead of solving a problem that doesn't exist to begin with?

What does relying on bash-completion (created by Debian, I think, not the bash people) gain that makes it useful?

I understand that Debian made this collection of completions to make their distro more user friendly.

Fish completions

Your m4 script would have fish completions installed in /usr/share/fish... instead of /etc/fish/completions

The /usr/share/fish... dir belongs to fish. So if you uninstall fish, completions installed there are 'orphan' - do distro's delete them when fish is gone? Are they kept around?

Q: Why isn't the more obvious directory (/etc/fish/completions) the default destination for installed programs' completions? What is the argument in favour of packages installing their files into /usr/share/fish/..., an unnatural, possibly unconventional location ?

Zsh completions

The problem I see is very similar to Fish, except there is no pkg-config for zsh to point to the /usr/share/zsh directory, this is preferred for some reason to /etc/zsh/... Thanks

[genivia-inc]

So it appears that CI does not install autoconf-archive because AX_CHECK_COMPILE_FLAG is rejected: https://github.com/Genivia/ugrep/actions/runs/8309397279

Perhaps regress to using AC_COMPILE_IFELSE or install autoconf-archive as a pre step to CI.

I prefer AC_COMPILE_IFELSE. So I'll make a few changes to retry.

@drok the completion parts that I got configured this way is from their own instructions and from projects that use it. My understanding is that it retrieves the completion directories configured when shell and completions were installed.

[genivia-inc]

With AC_COMPILE_IFELSE cross-compilation succeeds. The updated configure.ac with history: https://github.com/Genivia/ugrep/blob/master/configure.ac

I left the AX_CHECK_COMPILE_FLAG old part comment out. Strangely, I noticed that AX_CHECK_COMPILE_FLAG is defined in ac_local.m4 when AX_CHECK_COMPILE_FLAG was used. So it is not really missing. We could also add ax_check_compile_flag.m4.

I'm not too happy about this. A more thorough review of this cross-compilation part in configure.ac is needed.

[genivia-inc]

So autoconf 2.71 appears broken. Autoconf 2.69 works.

With autoconf 2.71 to generate configure from configure.ac and run it on ARM7 that requires -mfpu=neon:

checking whether g++ -std=gnu++11 supports NEON/AArch64 intrinsics... no

With autoconf 2.69 to generate configure from configure.ac and run it on the same ARM7 machine that requires -mfpu=neon:

checking whether g++ -std=gnu++11 supports NEON/AArch64 intrinsics... yes

In the latter case we get g++ -std=gnu++ ... -mfpu=neon -DHAVE_NEON, but in the first case we cannot detect NEON when -mpfu=neon is required.

This impacts the important parts that use AC_PREPROC_IFELSE and AC_RUN_IFELSE (i.e. not cross-compiling):

if test -z "$SIMD_FLAGS"; then
    if ! test "x$with_no_neon" = "xyes"; then
      AC_MSG_CHECKING([whether ${CXX} supports NEON/AArch64 intrinsics])
      AC_RUN_IFELSE([AC_LANG_PROGRAM([[#include <arm_neon.h>]],[[uint64x2_t n; uint64_t m = vgetq_lane_u64(n, 0);]])],
                    [mneon_ok=yes],
                    [mneon_ok=no])
      if test "x$mneon_ok" = "xyes"; then
        SIMD_FLAGS="-DHAVE_NEON"
      else
        save_CXXFLAGS=$CXXFLAGS
        CXXFLAGS="-mfpu=neon"
        AC_PREPROC_IFELSE([AC_LANG_PROGRAM([[#include <arm_neon.h>]],[[uint64x2_t n; uint64_t m = vgetq_lane_u64(n, 0);]])],
                          [mneon_ok=yes],
                          [mneon_ok=no])
        if test "x$mneon_ok" = "xyes"; then
          AC_RUN_IFELSE([AC_LANG_PROGRAM([[#include <arm_neon.h>]],[[uint64x2_t n; uint64_t m = vgetq_lane_u64(n, 0);]])],
                        [mneon_ok=yes],
                        [mneon_ok=no])
          if test "x$mneon_ok" = "xyes"; then
            SIMD_FLAGS="-mfpu=neon -DHAVE_NEON"
          else
            CXXFLAGS="-march=native -mfpu=neon"
            AC_RUN_IFELSE([AC_LANG_PROGRAM([[#include <arm_neon.h>]],[[uint64x2_t n; uint64_t m = vgetq_lane_u64(n, 0);]])],
                          [mneon_ok=yes],
                          [mneon_ok=no])
            if test "x$mneon_ok" = "xyes"; then
              SIMD_FLAGS="-march=native -mfpu=neon -DHAVE_NEON"
            fi
          fi
        fi
        CXXFLAGS=$save_CXXFLAGS
      fi
      AC_MSG_RESULT($mneon_ok)
    fi
  fi

EDIT: if I leave out AC_PREPROC_IFELSE and the test branch, then it works fine.

[genivia-inc]

@drok @alerque asking for help to figure out why make distcheck still fails with the latest committed configure.ac and makefiles.

It runs fine for the most part without warnings or errors, until this point where the cwd to the include files (g++ option -I) fails to locate the header files in the project's src dir:

$ make distcheck
...
... [lib/ sources compiling]
... [lzma/C sources compiling]
gcc -DHAVE_CONFIG_H -I. -I../../../../lzma/C -I../..  -DZ7_PPMD_SUPPORT -DZ7_EXTRACT_ONLY -DNDEBUG -D_REENTRANT -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -I/opt/homebrew/Cellar/pcre2/10.43/include  -I/opt/homebrew/Cellar/xz/5.6.1/include -I/opt/homebrew/Cellar/lz4/1.9.4/include -I/opt/homebrew/Cellar/zstd/1.5.5/include -I/opt/homebrew/Cellar/brotli/1.1.0/include -I../lzma/C  -Wall -Wextra -Wunused -O2 -MT libviiz_a-Ppmd7Dec.o -MD -MP -MF .deps/libviiz_a-Ppmd7Dec.Tpo -c -o libviiz_a-Ppmd7Dec.o `test -f 'Ppmd7Dec.c' || echo '../../../../lzma/C/'`Ppmd7Dec.c
mv -f .deps/libviiz_a-Ppmd7Dec.Tpo .deps/libviiz_a-Ppmd7Dec.Po
rm -f libviiz.a
ar cru libviiz.a libviiz_a-viizip.o libviiz_a-7zAlloc.o libviiz_a-7zArcIn.o libviiz_a-7zBuf.o libviiz_a-7zBuf2.o libviiz_a-7zCrc.o libviiz_a-7zCrcOpt.o libviiz_a-7zDec.o libviiz_a-7zFile.o libviiz_a-7zStream.o libviiz_a-Bcj2.o libviiz_a-Bra.o libviiz_a-Bra86.o libviiz_a-BraIA64.o libviiz_a-CpuArch.o libviiz_a-Delta.o libviiz_a-Lzma2Dec.o libviiz_a-LzmaDec.o libviiz_a-Ppmd7.o libviiz_a-Ppmd7Dec.o 
ranlib libviiz.a
Making all in src
g++ -std=gnu++11 -DHAVE_CONFIG_H -I. -I../../../src -I..  -I../../../include -DWITH_COLOR  -DHAVE_NEON -pthread -DPLATFORM=\"aarch64-apple-darwin21.6.0\" -DGREP_PATH=\"/Users/engelen/Projects/ugrep/ugrep-5.1.1/_inst/share/ugrep/patterns\" -DWITH_NO_INDENT -I/opt/homebrew/Cellar/pcre2/10.43/include  -I/opt/homebrew/Cellar/xz/5.6.1/include -I/opt/homebrew/Cellar/lz4/1.9.4/include -I/opt/homebrew/Cellar/zstd/1.5.5/include -I/opt/homebrew/Cellar/brotli/1.1.0/include -I../lzma/C  -Wall -Wextra -Wunused -O2 -MT ugrep-ugrep.o -MD -MP -MF .deps/ugrep-ugrep.Tpo -c -o ugrep-ugrep.o `test -f 'ugrep.cpp' || echo '../../../src/'`ugrep.cpp
../../../src/ugrep.cpp:77:10: fatal error: 'ugrep.hpp' file not found
#include "ugrep.hpp"
         ^~~~~~~~~~~
1 error generated.

I'm stuck on this one.

[alerque]

The autoconf-archive package should not need to be installed on any systems in order to build and test this. It's a handy package to have around while doing development locally, but as soon as you decide to use something from it in a project, a copy of whatever macro you want to use from it should be vendored in the macros folder (I use build-aux but last I looked at the PR m4 was being used, either is fine just be consistent). That way autoconf-archive is never and end user dependency. In fact not just autoconf-archive but also autoconf ends up not being an end user dependency if they use the source tarball releases generated by make dist since it vendors all the necessary generated scripts. For building from Git clones, having autoconf (but not the macros archive) is expected build tooling.There are still a few generated autoconf things being tracked in Git in the PR that should not be since they will cause version mismatches if run without being regenerated anyway.

The error with make distcheck look like an issue with just that. Regenerating the autoconf stuff on my system (autoconf --install --force) completes a distcheck just fine. I think something is getting cross wired between versions of autoconf and where variables like top_srcdir are getting set. I suggest pulling all the autoconf generated tooling out of the root directory and only tracking the vendored macros from the macros archive, not any of the main autoconf ones. Then target everything in configure.ac to whatever version of autoconf you'll be using on whatever system is going to be running make dist (whether that is local or CI) and don't worry about what versions might be on target systems.

[drok]

There are still a few generated autoconf things being tracked in Git in the PR

Thanks @alerque for the note, I removed ar-lib compile config.sub config.guess depcomp install-sh missing in 4d5589e in #370 - I think these are the files you're referring to.

[genivia-inc]

There are still a few generated autoconf things being tracked in Git in the PR

Thanks @alerque for the note, I removed ar-lib compile config.sub config.guess depcomp install-sh missing in 4d5589e in #370 - I think these are the files you're referring to.

That makes downloading the repo zip file and manually installing fail, which is not great. For example, when we remove compile we get:

$ ./configure && make
configure: error: cannot find required auxiliary files: compile

Same error when any one of these are removed: ar-lib, config.sub, config.guess, depcomp (make fails), install-sh, and missing. All of these are necessary to be able to run ./configure && make.

[alerque]

It isn't just those files, if you git clone or download the git archive generated snapshots you don't gave a configure script either. That and all the other files we're mentioning are part of the distribution tooling that is either expected to be used as generated without messing with it (as in using the make dist generated source tarballs) or generated on the build system using autotools.

The source dist generated with make dist will have the tooling batteries included those for people that want to build without host system tooling. For the Git clones or git archive generated snapshopts of a commit without the pre-generated tooling then the user should be expected to have autotools installed and run autoreconf --install before running ./configure.

It is common place but not required to include a bootstrap script to make it more obvious how to get started, e.g.g bootstrap.sh:

#!/usr/bin/env sh
autoreconf --install

This file does not get listed in EXTRA_DIST so it is not present in source distributions where configure and friends will exist out of the box.

[genivia-inc]

I know autoreconf will fix this, as I've included autoreconf -fi as a suggestion to users to use that when stuck when running ./build.sh.

Pretty much every software download that uses autotools to build the software asks users to simply run ./configure && make. It almost always works right away, even when downloading Git clones etc. It's the way I've done this for decades. I don't recall having to ever use autoreconf.

It's also true that Git messes up timestamps, so a autoreconf -fi will fix that as I've added this in the ./build.sh output to inform folks not to panic and use this command and then try again.

Otherwise, most folks that get stuck here will not think when and how to use autoreconf -fi.

They will just give up.

That's my concern.

EDIT: look at the number of web posts "autoreconf not found" when autoconf is not installed. I had a number of people complain about this in the past, so I took counter measures in ./build.sh as you can see.