public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Christophe Lyon <christophe.lyon@linaro.org>
To: Alexandre Oliva <oliva@gnu.org>
Cc: gcc-patches@gcc.gnu.org, josmyers@redhat.com
Subject: Re: [PATCH] gcc/Makefile.in: Fix install-info target if BUILD_INFO is empty
Date: Sat, 10 Feb 2024 23:06:53 +0100	[thread overview]
Message-ID: <CAPS5khYSEeVJ6yJeHqGbDU_gL+L8uJ8E2H9YUu0dcK_jRhHFvw@mail.gmail.com> (raw)
In-Reply-To: <orjzni6tm6.fsf@lxoliva.fsfla.org>

[-- Attachment #1: Type: text/plain, Size: 3618 bytes --]

Hi!

On Tue, 6 Feb 2024 at 06:37, Alexandre Oliva <oliva@gnu.org> wrote:
>
> Hello, Christophe,
>
> Thanks for the patch.
>
> On Feb  5, 2024, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>
> > In order to save build time, our CI overrides BUILD_INFO="", which
> > works when invoking 'make all' but not for 'make install' in case some
> > info files need an update.
>
> Hmm, I don't think this would be desirable.  We ship updated info files
> in release tarballs, and it would be desirable to install them even if
> makeinfo is not available in the build environment.
>
> > I noticed this when testing a patch posted on the gcc-patches list,
> > leading to an error at 'make install' time after updating tm.texi (the
> > build reported 'new text' in tm.texi and stopped).  This is because
> > 'install' depends on 'install-info', which depends on
> > $(DESTDIR)$(infodir)/gccint.info (among others).
>
> Ideally, we'd detect and report info files that are out-of-date WRT
> their ultimate sources, especially to catch tm.texi.in changes, but
> doing so only at install time is clearly suboptimal.
>
> I mean, if we don't have the tools to build info files, it's fine if we
> skip their building, and even refrain from installing info files that
> are missing or outdated, but we should install prebuilt ones if they're
> available, and we should probably *not* refrain from trying to satisfy
> the dependencies for info files at build time, even if it turns out that
> we can't build the info files themselves.
>
> This suggests to me that, rather than setting BUILD_INFO to the empty
> string, we should set it to e.g. no-info, so that $(MAKEINFO) will not
> be run because x$(BUILD_INFO) != xinfo, but so that we still get the
> dependencies resolved, e.g. by making no-info depend on info.  Or maybe
> make it info-check-deps, and insert that between info and its current
> deps.  WDYT?

I've just spent quite a bit of time looking at your suggestion, and well...

I hadn't considered the case of makeinfo missing/too old, in our use
case makeinfo is present and recent enough but we want to save a few
minutes of build time during the CI loop.

As I mentioned, we (tried to) do this by doing BUILD_INFO="" when
invoking 'make', and it took me ages to realize it is not working as
expected, because GCC's top-level Makefile does not propagate
BUILD_INFO recursively, and this conflicted with the setting of
BUILD_INFO=no-info (I wanted to try to support several values for
BUILD_INFO: info, no-info and "", where "" would disable more things,
but it seems to be too much hassle)

So, the attached small patch implements your suggestion, and works as
expected: it makeinfo is not available, we now detect problems with
tm.texi.in at build time rather than install time. OK?

Looking deeper, I realized that texi2dvi and texi2pdf belong to the
texinfo package, like makeinfo, so the dvi and pdf rules should
probably also depend on BUILD_INFO? To generate html, we call makeinfo
--html so the html rules should also depend on BUILD_INFO.  However,
unlike install-man, the install-html, install-dvi and install-pdf are
not part of the plain 'install' target, so maybe we can argue that if
someone runs 'make install-pdf' without texinfo, then too bad for him?

Thanks,

Christophe




>
> --
> Alexandre Oliva, happy hacker                    https://FSFLA.org/blogs/lxo/
>    Free Software Activist                           GNU Toolchain Engineer
> Disinformation flourishes because many people care deeply about injustice but
> very few check the facts.  Think Assange & Stallman.  The empires strike back

[-- Attachment #2: v3-0001-gcc-Makefile.in-Always-check-info-dependencies.patch --]
[-- Type: text/x-patch, Size: 2972 bytes --]

From 7ec6ff8ead24eb7c07d011371347cf12db11faf5 Mon Sep 17 00:00:00 2001
From: Christophe Lyon <christophe.lyon@linaro.org>
Date: Sat, 10 Feb 2024 21:17:08 +0000
Subject: [PATCH v3] gcc/Makefile.in: Always check info dependencies

BUILD_INFO is currently a byproduct of checking makeinfo
presence/version.  INSTALL_INFO used to be defined similarly, but was
removed in 2000 (!) by commit 17db658241d18cf6db59d31bc2d6eac96e9257df
(svn r38141).

In order to save build time, our CI overrides MAKEINFO=echo, which
works when invoking 'make all' but not for 'make install' in case some
info files need an update.

I noticed this while testing a patch posted on the gcc-patches list,
leading to an error at 'make install' time after updating tm.texi (the
build reported 'new text' in tm.texi and stopped).  This is because
'install' depends on 'install-info', which depends on
$(DESTDIR)$(infodir)/gccint.info (among others).

As discussed, it is better to detect this problem during 'make all'
rather than 'make install', and we still want to detect it even if
makeinfo is not available.

This patch makes configure set BUILD_INFO=no-info in case makeinfo is
missing/too old, which effectively makes the build rules no-ops
(x$(BUILD_INFO) != xinfo), and updates Makefile.in so that 'info'
dependencies are still checked.

2024-02-10  Christophe Lyon  <christophe.lyon@linaro.org>

	gcc/
	* Makefile.in: Add no-info dependency.
	* configure.ac: Set BUILD_INFO=no-info if makeinfo is not
	available.
	* configure: Regenerate.
---
 gcc/Makefile.in  | 7 +++++++
 gcc/configure    | 2 +-
 gcc/configure.ac | 2 +-
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 95caa54a52b..a74761b7ab3 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -3480,6 +3480,13 @@ install-no-fixedincludes:
 
 doc: $(BUILD_INFO) $(GENERATED_MANPAGES)
 
+# If BUILD_INFO is set to no-info by configure, we still want to check
+# 'info' dependencies even the build rules are no-ops because
+# BUILD_INFO != info (see %.info rule)
+ifeq ($(BUILD_INFO),no-info)
+no-info: info
+endif
+
 INFOFILES = doc/cpp.info doc/gcc.info doc/gccint.info \
             doc/gccinstall.info doc/cppinternals.info
 
diff --git a/gcc/configure b/gcc/configure
index 4acb254d830..771b93380bc 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -8835,7 +8835,7 @@ if test $gcc_cv_prog_makeinfo_modern = no; then
 $as_echo "$as_me: WARNING:
 *** Makeinfo is missing or too old.
 *** Info documentation will not be built." >&2;}
-  BUILD_INFO=
+  BUILD_INFO=no-info
 else
   BUILD_INFO=info
 fi
diff --git a/gcc/configure.ac b/gcc/configure.ac
index d2ed14496c1..2855545aaf0 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -1245,7 +1245,7 @@ if test $gcc_cv_prog_makeinfo_modern = no; then
   AC_MSG_WARN([
 *** Makeinfo is missing or too old.
 *** Info documentation will not be built.])
-  BUILD_INFO=
+  BUILD_INFO=no-info
 else
   BUILD_INFO=info
 fi
-- 
2.34.1


  reply	other threads:[~2024-02-10 22:07 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-01 17:15 [PATCH] gcc/configure: Re-introduce INSTALL_INFO Christophe Lyon
2024-02-02 10:10 ` rep.dot.nop
2024-02-02 10:40   ` Christophe Lyon
2024-02-05 11:26     ` [PATCH] gcc/Makefile.in: Fix install-info target if BUILD_INFO is empty Christophe Lyon
2024-02-06  5:37       ` Alexandre Oliva
2024-02-10 22:06         ` Christophe Lyon [this message]
2024-02-11  5:56           ` Alexandre Oliva
2024-02-12 10:13             ` Christophe Lyon
2024-02-12 10:27               ` Jakub Jelinek
2024-02-12 15:48                 ` Christophe Lyon
2024-02-05 11:30     ` [PATCH] gcc/configure: Re-introduce INSTALL_INFO Christophe Lyon
2024-02-05 12:25       ` rep.dot.nop

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAPS5khYSEeVJ6yJeHqGbDU_gL+L8uJ8E2H9YUu0dcK_jRhHFvw@mail.gmail.com \
    --to=christophe.lyon@linaro.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=josmyers@redhat.com \
    --cc=oliva@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).