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

Merge my 'autoupdate' branch #289

Open
wants to merge 55 commits into
base: master
Choose a base branch
from

Conversation

cooljeanius
Copy link
Contributor

@cooljeanius cooljeanius commented Sep 26, 2023

This isn't actually ready to merge yet (see cooljeanius#1), but I thought I'd open this as a draft anyways just so I can have something to look at while I fix the merge conflicts...

OK, so, basically what I did here was to:

  • Run autoupdate
  • Notice that it broke some things, so fix the things it broke
  • Get a testsuite started, to try to catch the sorts of things that autoupdate might break
  • Fix the remaining breakages that the testsuite caught
  • Do a few other misc. style tweaks that I noticed while I was at it

…ravis is using

also check that variable for gnulib-tool is empty before setting it;
this allows the user to override it by setting it to the path to their
own gnulib-tool

also print more bootstrap messages

the gitignore files got updating by my re-running the bootstrap script
while testing it
also change the test for empty variable in the bootstrap script to
accommodate Travis

and add comment about bug in doc/Makefile.am

also deal with issue with `echo -n`
…or a C compiler

and remove the manual call to AC_PROG_CC in configure.ac (gl_EARLY
still seems to call it, though...)

Also enable maintainer mode, and some other things to get the bootstrap
script to work on Travis.
also try to fix docs build failure
… are parse-able

still need to try actually calling them all...
…his branch for in the first place

that is, run `autoupdate` on all the m4 files, and see what the fallout
is.

(note: this will probably cause a lot of errors)
there is still plenty more remaining to clean up though
also bump up various serial numbers
and replace instances of configure.in with configure.ac
and misc. other edits noticed while doing the aforementioned cleanup

(I fully expect this build to fail btw)
try marking TODO as org-mode
Check for path to texi2dvi, to make it easier to override.
I was trying to work around the following error:
```
/usr/bin/texi2dvi: Can't use option `--output' with more than one argument.
```
...but it seems that doing `make distcheck AM_DISTCHECK_DVI_TARGET=html` works better.
serial bump caught by CI
@cooljeanius
Copy link
Contributor Author

ok merge conflicts fixed; next step is getting CI passing again...

good grief, how many serial bumps is this going to take?!
not complete yet, though; failure expected
still need to do everything after that...
still got the rest to go...
(locally, at least)
serial bump caught on CI
bump serial some more
GitHub Actions needs lzip now, too
[travis skip]
try to skip the dvi step
[travis skip]
I *say* I actually like autotools, but sometimes the haters can actually be right: autotools can actually be really annoying sometimes!
minor testsuite tweak
let's fix Travis next
maybe the difference between CIs is their distro version?
@cooljeanius cooljeanius marked this pull request as ready for review September 28, 2023 23:12
@cooljeanius
Copy link
Contributor Author

OK it now passes both CIs

@cooljeanius
Copy link
Contributor Author

@peti so I realize that this might be a bit too much to review all at once; do you have any tips on how I could make this easier to review?


dist_doc_DATA = AUTHORS COPYING COPYING.EXCEPTION README
dist_doc_DATA = AUTHORS COPYING COPYING.EXCEPTION README.md
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a file called README in the release tar all or Automake will complain. Can you please revert this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peti the plain-text README already gets included by default without needing to be specified explicitly because it's one of the files that automake puts under its am__DIST_COMMON variable. If I change README.md to just README, then make distcheck fails with:

Making all in test
/Applications/Xcode.app/Contents/Developer/usr/bin/make  all-am
make[3]: Nothing to be done for `all-am'.
make[2]: *** No rule to make target `../../README.md', needed by `../../README'.  Stop.
make[1]: *** [all-recursive] Error 1
make: *** [distcheck] Error 1

If I change it back to README.md, however, it gets past that step successfully (it then proceeds to fail with an error from texi2dvi making autoconf-archive.dvi in doc, but that's probably why the CI runs distcheck with AM_DISTCHECK_DVI_TARGET="", and in any case, it's unrelated to the issue of what the README is named)

bootstrap.sh Outdated Show resolved Hide resolved

set -eu
# Wait until after we finish testing a possibly empty variable to do `set -u`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure whether I understand this change. What problem does this solve?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peti: The -u flag causes errors whenever the script encounters an undefined variable. This defeats the point of testing to see if certain variables are empty or not. For example, the test on line 11 of:

if [ "x${gnulibtool}" = "x" ]; then

...will cause the entire script to error out if the script is run with set -u, like this:

$ sh ./bootstrap.sh
./bootstrap.sh: line 11: gnulibtool: unbound variable

revert change to shebang line as requested in PR
replace some tabs with spaces, and disable some shellcheck warnings
@cooljeanius
Copy link
Contributor Author

@peti I changed bootstrap.sh's shebang back to #!/bin/sh in deddd9a, and silenced a few additional shellcheck warnings for bootstrap.sh in 300ae70.

@cooljeanius cooljeanius requested a review from peti November 8, 2023 17:33
@cooljeanius
Copy link
Contributor Author

Hi @peti, may I please gently ping this? I think the recent xz backdoor has shown that testing to ensure that autoconf macros actually do what they say they do can be important for security, so in that light, I think that makes the beginnings of a testsuite that I've started here to be a bit more important now... Please let me know if there's anything I can do to help move this along.

bump suggested by CI run of prev. commit
there's gotta be some sort of bug with the serial-bumping check; applying the exact diff that it suggests still fails to get it to pass...
how many serial bumps is this going to take?!
```
--- m4/ax_check_compile_flag.m4	2024-06-07 04:05:57.348392358 +0000
+++ stage/ax_check_compile_flag.m4	2024-06-07 04:06:46.236406376 +0000
@@ -34,7 +34,7 @@
 #   and this notice are preserved.  This file is offered as-is, without any
 #   warranty.
 
-#serial 14
+#serial 15
 
 AC_DEFUN([AX_CHECK_COMPILE_FLAG],
 [AC_PREREQ([2.64])dnl for _AC_LANG_PREFIX and AS_VAR_IF
make: *** [cfg.mk:36: stage/ax_check_compile_flag.m4] Error 1
Error: Process completed with exit code 2.
```
--- m4/ax_check_compile_flag.m4	2024-06-07 04:09:26.127066767 +0000
+++ stage/ax_check_compile_flag.m4	2024-06-07 04:10:15.695371330 +0000
@@ -34,7 +34,7 @@
 #   and this notice are preserved.  This file is offered as-is, without any
 #   warranty.
 
-#serial 15
+#serial 16
 
 AC_DEFUN([AX_CHECK_COMPILE_FLAG],
 [AC_PREREQ([2.64])dnl for _AC_LANG_PREFIX and AS_VAR_IF
make: *** [cfg.mk:36: stage/ax_check_compile_flag.m4] Error 1
@cooljeanius
Copy link
Contributor Author

OK CI is passing again; I'd recommend squashing when merging, to compress all these intermediary commits...

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