diff mbox

(build) Patch to fix cp/cfns.gperf building issues

Message ID 1303558468.699813970@www2.webmail.us
State New
Headers show

Commit Message

Nicola Pero April 23, 2011, 11:34 a.m. UTC 
> Additionally,
>
>  contrib/gcc_update --touch
>
> can be used to to fix the time stamps until such time as someone changes the gperf rule to be under maintainer mode.

Thanks Mike,

good to know. :-)


> So, only the dependency should go away under a maintainer rules, as in the below, not the entire rule.

What is the reason to keep the rule without the dependency ?  Is it so that even with --disable-maintainer-mode
you can force the file to be recreated by manually deleting it ?

That would make sense ... it sounds like a good idea, so here is yet another refinement of the patch with
that refinement added too :-)

Ok to commit ?

Thanks

Comments

Mike Stump April 23, 2011, 12:14 p.m. UTC | #1 
On Apr 23, 2011, at 4:34 AM, Nicola Pero wrote:
> What is the reason to keep the rule without the dependency ?  Is it so that even with --disable-maintainer-mode you can force the file to be recreated by manually deleting it ?

Yes.  Think of it as a really cheap maintainer mode.  Another way to look at it, would be consistency with the existing style.

I think the patch is ready for a build person to review it.  :-)
Alexandre Oliva April 29, 2011, 5:41 a.m. UTC | #2 
On Apr 23, 2011, "Nicola Pero" <nicola.pero@meta-innovation.com> wrote:

> Ok to commit ?

Yeah, thanks.

> Index: ChangeLog
> +2011-04-22  Nicola Pero  <nicola.pero@meta-innovation.com>
> +
> +       * Makefile.in (ENABLE_MAINTAINER_RULES): New.
> +
> Index: cp/ChangeLog
> +2011-04-23  Nicola Pero  <nicola.pero@meta-innovation.com>,
> +           Mike Stump <mikestump@comcast.net>
> +
> +       * Make-lang.in ($(srcdir)/cp/cfns.h): Enable the dependency only
> +       in maintainer mode.  Use the --output-file option of gperf instead
> +       of > to prevent creating an empty cp/cfns.h when gperf is not
> +       available.
> +

-pedantic review: how about outputting to a temporary file (say
cp/cfns.hT) and only renaming to the intended name on success, so that,
if gperf crashes or we reboot part-way through it, we don't end up with
a partially-generated file that will seem to be up to date?
Nicola Pero April 29, 2011, 7:45 p.m. UTC | #3 
Alexandre

thanks for the review :-)

> -pedantic review: how about outputting to a temporary file (say
> cp/cfns.hT) and only renaming to the intended name on success, so that,
> if gperf crashes or we reboot part-way through it, we don't end up with
> a partially-generated file that will seem to be up to date?

It's a good suggestion.

On the other hand, now that the rule is maintainer-only, and considering
it's used once every few years, I wouldn't personally want to spend more
time on it. ;-)

Thanks
diff mbox

Patch

Index: ChangeLog
===================================================================
--- ChangeLog   (revision 172860)
+++ ChangeLog   (working copy)
@@ -1,3 +1,7 @@ 
+2011-04-22  Nicola Pero  <nicola.pero@meta-innovation.com>
+
+       * Makefile.in (ENABLE_MAINTAINER_RULES): New.
+
 2011-04-22  Jakub Jelinek  <jakub@redhat.com>
 
        PR c/48716
Index: cp/Make-lang.in
===================================================================
--- cp/Make-lang.in     (revision 172860)
+++ cp/Make-lang.in     (working copy)
@@ -104,10 +104,20 @@  cc1plus$(exeext): $(CXX_OBJS) cc1plus-checksum.o $
        +$(LINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \
              $(CXX_OBJS) cc1plus-checksum.o $(BACKEND) $(LIBS) $(BACKENDLIBS)
 
-# Special build rules.
+ifeq ($(ENABLE_MAINTAINER_RULES), true)
+# Special build rule.  This is a maintainer rule, that is only
+# available when GCC is configured with --enable-maintainer-mode.  In
+# other cases, it is not available to avoid triggering rebuilds if a
+# user has the source checked out with unusual timestamps.
 $(srcdir)/cp/cfns.h: $(srcdir)/cp/cfns.gperf
+else
+# We keep the rule so that you can still force a rebuild, even if you
+# didn't configure GCC with --enable-maintainer-mode, by manually
+# deleting the $(srcdir)/cp/cfns.h file.
+$(srcdir)/cp/cfns.h:
+endif
        gperf -o -C -E -k '1-6,$$' -j1 -D -N 'libc_name_p' -L ANSI-C \
-               $(srcdir)/cp/cfns.gperf > $(srcdir)/cp/cfns.h
+               $(srcdir)/cp/cfns.gperf --output-file $(srcdir)/cp/cfns.h
 
 #^L
 # Build hooks:
Index: cp/ChangeLog
===================================================================
--- cp/ChangeLog        (revision 172860)
+++ cp/ChangeLog        (working copy)
@@ -1,3 +1,11 @@ 
+2011-04-23  Nicola Pero  <nicola.pero@meta-innovation.com>,
+           Mike Stump <mikestump@comcast.net>
+
+       * Make-lang.in ($(srcdir)/cp/cfns.h): Enable the dependency only
+       in maintainer mode.  Use the --output-file option of gperf instead
+       of > to prevent creating an empty cp/cfns.h when gperf is not
+       available.
+
 2011-04-20  Jason Merrill  <jason@redhat.com>
 
        * semantics.c (finish_compound_literal): Don't put an array
Index: Makefile.in
===================================================================
--- Makefile.in (revision 172860)
+++ Makefile.in (working copy)
@@ -165,8 +165,19 @@  C_STRICT_WARN = @c_strict_warn@
 NOCOMMON_FLAG = @nocommon_flag@
 
 # This is set by --disable-maintainer-mode (default) to "#"
+# FIXME: 'MAINT' will always be set to an empty string, no matter if
+# --disable-maintainer-mode is used or not.  This is because the
+# following will expand to "MAINT := " in maintainer mode, and to
+# "MAINT := #" in non-maintainer mode, but because '#' starts a comment,
+# they mean exactly the same thing for make.
 MAINT := @MAINT@
 
+# The following provides the variable ENABLE_MAINTAINER_RULES that can
+# be used in language Make-lang.in makefile fragments to enable
+# maintainer rules.  So, ENABLE_MAINTAINER_RULES is 'true' in
+# maintainer mode, and '' otherwise.
+@MAINT@ ENABLE_MAINTAINER_RULES = true
+
 # These are set by --enable-checking=valgrind.
 RUN_GEN = @valgrind_command@
 VALGRIND_DRIVER_DEFINES = @valgrind_path_defines@