Please *bootstrap* patches

Please *bootstrap* patches

[Steven Bosscher]

Hi,

For the second time in three days, my daily tested failed to
complete the nightly bootstrap because of a silly warning that is
an error because we bootstrap with -Werror:

cc1: warnings being treated as errors
../../mainline/gcc/cp/decl.c: In function 'reshape_init':
../../mainline/gcc/cp/decl.c:4162: warning: 'max_index_cst' may be used
uninitialized in this function
make[2]: *** [cp/decl.o] Error 1
make[2]: *** Waiting for unfinished jobs....

This is probably Giovanni's patch.  Two days ago bootstrap failed
because Jeff decided to write fun () instead of fun (void).

Apparently people don't bootstrap patches, or these mistakes would
have been detected before patches got commited.

If the testers fail every other day, or many days in a row, it is
very hard to track the performance of the compiler, or to do things
like hunting regressions later on, etc.
So, *please*, can people *bootstrap* their patches, and make sure
this kind of mistake does not happen again?

Gr.
Steven

RE: Please *bootstrap* patches

[Andreas Tobler]

>-- Original Message --

>cc1: warnings being treated as errors
>../../mainline/gcc/cp/decl.c: In function 'reshape_init':
>../../mainline/gcc/cp/decl.c:4162: warning: 'max_index_cst' may be used
>uninitialized in this function
>make[2]: *** [cp/decl.o] Error 1
>make[2]: *** Waiting for unfinished jobs....

I took the freedom to apply the fix as obvious. I hope it is ok.

http://gcc.gnu.org/ml/gcc-patches/2004-09/msg02045.html

Andreas

Re: Please *bootstrap* patches

[Giovanni Bajo]

[resent to gcc@ instead of gcc-patches@, sorry about that]

Steven Bosscher wrote:

> cc1: warnings being treated as errors
> ../../mainline/gcc/cp/decl.c: In function 'reshape_init':
> ../../mainline/gcc/cp/decl.c:4162: warning: 'max_index_cst' may be
> used uninitialized in this function
> make[2]: *** [cp/decl.o] Error 1
> make[2]: *** Waiting for unfinished jobs....
>
> This is probably Giovanni's patch.  Two days ago bootstrap failed
> because Jeff decided to write fun () instead of fun (void).
>
> Apparently people don't bootstrap patches, or these mistakes would
> have been detected before patches got commited.

Yes, this was my patch. While I sympathize with your sentiment, I must notice
that there is no official need to bootstrap a C++ only patch (a patch affecting
only files in cp/). In fact, our official rules at
http://gcc.gnu.org/contribute.html say:

---------- QUOTE ----------
If your change is to the C++ front end, you should rebuild the compiler,
libstdc++, libjava and run the C++ testsuite. If you already did a complete
C,C++,Java bootstrap from your build directory before, you can use the
following:

make clean-target-libstdc++-v3                    # clean libstdc++ and ...
test -d */libjava && make -C */libjava clean-nat  # ... parts of libjava
make all-target-libstdc++-v3 all-target-libjava   # rebuild compiler and
libraries
make -k check-c++                                 # run C++/libstdc++ testsuite
---------- UNQUOTE ----------

This was suggested by Jason Merrill. If this procedure does not
activate -Werror, then it is a problem of our contribution rules, which I
follow literally. When I invoke "make all-gcc" on my patched decl.c, I see the
following command lines:

gcc -c    -DIN_GCC   -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-pro
totypes   -fno-common   -DHAVE_CONFIG_H    -I. -Icp -I../../gcc/gcc -I../../gcc
/gcc/cp -I../../gcc/gcc/../include -I../../gcc/gcc/../libcpp/include
../../gcc/gcc/cp/decl.c -o cp/decl.o
gcc    -DIN_GCC   -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-protot
ypes  -fno-common   -DHAVE_CONFIG_H  -o cc1plus \
      cp/cp-lang.o stub-objc.o cp/call.o cp/decl.o cp/expr.o cp/pt.o
cp/typeck2.o cp/class.o cp/decl2.o cp/error.o cp/lex.o cp/parser.o cp/ptree.o
cp/rtti.o cp/typeck.o cp/cvt.o cp/except.o cp/friend.o cp/init.o cp/method.o
cp/search.o cp/semantics.o cp/tree.o cp/repo.o cp/dump.o cp/optimize.o
cp/mangle.o cp/cp-objcp-common.o cp/name-lookup.o cp/cxx-pretty-print.o
cp/cp-gimplify.o tree-mudflap.o attribs.o c-common.o c-format.o c-pragma.o
c-semantics.o c-lex.o c-dump.o  c-pretty-print.o c-opts.o c-pch.o c-incpath.o
cppdefault.o c-ppoutput.o c-cppbuiltin.o prefix.o c-gimplify.o tree-inline.o
main.o tree-browser.o libbackend.a ../libcpp/libcpp.a ../libcpp/libcpp.a
../libiberty/libiberty.a

So there are two problems here: First, there is no -Werror. Second, there is no
warning at all. This is probably caused by the fact that "make all-gcc" uses
the default compiler and not the bootstrapped one. Maybe the make target should
be changed to something like "make stage3", I surely don't know the details.
Can some makefile guru comment on this?

> So, please, can people bootstrap their patches, and make sure
> this kind of mistake does not happen again?

I would like to add that I rarely contribute anything outside the C++ FE for
several reasons, including expertice (of course) but also very limited testing
resources. So deleting the above quoted lines from contribute.html, thus
forcing a full bootstrap to test C++-only changes would be a good way to
prevent me from doing any work on GCC.

Giovanni Bajo

Re: Please *bootstrap* patches

[Daniel Jacobowitz]

On Tue, Sep 21, 2004 at 03:26:46PM +0200, Giovanni Bajo wrote:
> Yes, this was my patch. While I sympathize with your sentiment, I must notice
> that there is no official need to bootstrap a C++ only patch (a patch affecting
> only files in cp/). In fact, our official rules at
> http://gcc.gnu.org/contribute.html say:
> 
> ---------- QUOTE ----------
> If your change is to the C++ front end, you should rebuild the compiler,
> libstdc++, libjava and run the C++ testsuite. If you already did a complete
> C,C++,Java bootstrap from your build directory before, you can use the
> following:
> 
> make clean-target-libstdc++-v3                    # clean libstdc++ and ...
> test -d */libjava && make -C */libjava clean-nat  # ... parts of libjava
> make all-target-libstdc++-v3 all-target-libjava   # rebuild compiler and
> libraries
> make -k check-c++                                 # run C++/libstdc++ testsuite
> ---------- UNQUOTE ----------
> 
> This was suggested by Jason Merrill. If this procedure does not
> activate -Werror, then it is a problem of our contribution rules, which I
> follow literally. When I invoke "make all-gcc" on my patched decl.c, I see the
> following command lines:

This should, instead, use bubblestrap.  That will not take
substantially longer, but will have the benefit of catching -Werror
problems.


-- 
Daniel Jacobowitz

Re: Please *bootstrap* patches

[Robert Dewar]

Steven Bosscher wrote:

> So, *please*, can people *bootstrap* their patches, and make sure
> this kind of mistake does not happen again?
> 
> Gr.
> Steven

Of course I agree with Steven that all patches should be bootstrapped
before being submitted, but I just wanted to mention an additional
working practice that we use in the GNAT world that provides an added
precaution, which is that all developers use by default the switches
that the compiler uses for bootstrapping, so all our internal
development of compiler components is always done with the
warnings-are-on-and-fatal flags set. That seems a good working
practice to adopt more generally.

Re: Please *bootstrap* patches

[Giovanni Bajo]

Daniel Jacobowitz wrote:

> This should, instead, use bubblestrap.  That will not take
> substantially longer, but will have the benefit of catching -Werror
> problems.

Other people in IRC suggested quickstrap instead. What is the correct one?
AFAICT what we want to do is just rebuilding stage3 cc1plus with the stage 2
compiler.

Giovanni Bajo

Re: Please *bootstrap* patches

[Andreas Schwab]

"Giovanni Bajo" <giovannibajo@libero.it> writes:

> Other people in IRC suggested quickstrap instead. What is the correct one?
> AFAICT what we want to do is just rebuilding stage3 cc1plus with the stage 2
> compiler.

If you run make all-gcc in a previously bootstrapped build directory this
should happen automatically.  At least it worked the last time I tried.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Please *bootstrap* patches

[Frank Ch. Eigler]

stevenb wrote:

> For the second time in three days, my daily tested failed to
> complete the nightly bootstrap because of a silly warning that is
> an error because we bootstrap with -Werror:
> [...]
> Apparently people don't bootstrap patches, or these mistakes would
> have been detected before patches got commited.
> [...]
> So, *please*, can people *bootstrap* their patches, and make sure
> this kind of mistake does not happen again?

In some cases, considering the rate of change on the source tree, this
is not physically possible.  To an extent, the bootstrap process
benefits us exclusively for its enforcement of absence of certain
warnings.  I'd love to see a change to the configury so that the
stage-0 gcc build was likewise warning-checked, if the host gcc was
capable.

- FChE

Re: Please *bootstrap* patches

[Matt Austern]

On Sep 21, 2004, at 2:18 AM, Steven Bosscher wrote:

> Hi,
>
> For the second time in three days, my daily tested failed to
> complete the nightly bootstrap because of a silly warning that is
> an error because we bootstrap with -Werror:
>
> cc1: warnings being treated as errors
> ../../mainline/gcc/cp/decl.c: In function 'reshape_init':
> ../../mainline/gcc/cp/decl.c:4162: warning: 'max_index_cst' may be used
> uninitialized in this function
> make[2]: *** [cp/decl.o] Error 1
> make[2]: *** Waiting for unfinished jobs....

And there's another bootstrap failure today in Java that's related to 
-Werror:
stage1/xgcc -Bstage1/ -B/work/root.u/powerpc-apple-darwin7.5.0/bin/ -c  
  -g -O2 -mdynamic-no-pic -DIN_GCC   -W -Wall -Wwrite-strings 
-Wstrict-prototypes -Wmissing-prototypes -Werror -fno-common   
-DHAVE_CONFIG_H    -I. -Ijava -I../../fsf.u/gcc -I../../fsf.u/gcc/java 
-I../../fsf.u/gcc/../include -I./../intl 
-I../../fsf.u/gcc/../libcpp/include  ../../fsf.u/gcc/java/gjavah.c -o 
java/gjavah.o
cc1: warnings being treated as errors
../../fsf.u/gcc/java/gjavah.c:642: warning: function declaration isn't 
a prototype
make[2]: *** [java/gjavah.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [stage2_build] Error 2
make: *** [bootstrap] Error 2


Anyone mind if I check in the patch below as obvious?

			--Matt


Index: gcc/java/gjavah.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/java/gjavah.c,v
retrieving revision 1.119
diff -p -r1.119 gjavah.c
*** gcc/java/gjavah.c   21 Sep 2004 16:28:40 -0000      1.119
--- gcc/java/gjavah.c   21 Sep 2004 18:14:10 -0000
*************** name_is_method_p (const unsigned char *n
*** 638,644 ****

   /* Free the method name list.  */
   static void
! free_method_name_list ()
   {
     struct method_name *p = method_name_list;
     while (p != NULL)
--- 638,644 ----

   /* Free the method name list.  */
   static void
! free_method_name_list (void)
   {
     struct method_name *p = method_name_list;
     while (p != NULL)

Re: Please *bootstrap* patches

[Zack Weinberg]

Matt Austern <austern@apple.com> writes:

> ../../fsf.u/gcc/java/gjavah.c:642: warning: function declaration isn't 
> a prototype
...
> Anyone mind if I check in the patch below as obvious?

I'll officially approve that, even. :-)

zw

>
> Index: gcc/java/gjavah.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/java/gjavah.c,v
> retrieving revision 1.119
> diff -p -r1.119 gjavah.c
> *** gcc/java/gjavah.c   21 Sep 2004 16:28:40 -0000      1.119
> --- gcc/java/gjavah.c   21 Sep 2004 18:14:10 -0000
> *************** name_is_method_p (const unsigned char *n
> *** 638,644 ****
>
>   /* Free the method name list.  */
>   static void
> ! free_method_name_list ()
>   {
>     struct method_name *p = method_name_list;
>     while (p != NULL)
> --- 638,644 ----
>
>   /* Free the method name list.  */
>   static void
> ! free_method_name_list (void)
>   {
>     struct method_name *p = method_name_list;
>     while (p != NULL)

Re: Please *bootstrap* patches

[Ranjit Mathew]

Giovanni Bajo wrote:
>
>>This should, instead, use bubblestrap.  That will not take
>>substantially longer, but will have the benefit of catching -Werror
>>problems.
> 
> 
> Other people in IRC suggested quickstrap instead. What is the correct one?
> AFAICT what we want to do is just rebuilding stage3 cc1plus with the stage 2
> compiler.

Assuming dependencies in the non-C front-end are set up
correctly, the following should suffice IMHO:

  1. rm -fr $BUILD_DIR/$TARGET_TRIPLET/$LANG_RUNTIME
  2. make bubblestrap
  3. cd $BUILD_DIR/$TARGET_TRIPLET/$LANG_RUNTIME
  4. make -k check

For added safety and just to make sure, you can probably
delete the front-end's files from $BUILD_DIR/gcc/$LANG
and $BUILD_DIR/gcc/stage2/$LANG before executing the above.

This seems to work well for me for the Java front-end
at least. Not so sure about C++ and others though.

Ranjit.

-- 
Ranjit Mathew          Email: rmathew AT gmail DOT com

Bangalore, INDIA.      Web: http://ranjitmathew.tripod.com/

Re: Please *bootstrap* patches

[Paolo Bonzini]

> If you run make all-gcc in a previously bootstrapped build directory this
> should happen automatically.  At least it worked the last time I tried.

This is a quickstrap, but a bubblestrap (i.e. rebuild the affected files 
for all the stages) would be a bit longer but safer.  It could even be 
changed in the toplevel Makefile.def.

Paolo

Re: Please *bootstrap* patches

[Geoffrey Keating]

Ranjit Mathew <rmathew@gmail.com> writes:

> Giovanni Bajo wrote:
> >
> >>This should, instead, use bubblestrap.  That will not take
> >>substantially longer, but will have the benefit of catching -Werror
> >>problems.
> > 
> > 
> > Other people in IRC suggested quickstrap instead. What is the correct one?
> > AFAICT what we want to do is just rebuilding stage3 cc1plus with the stage 2
> > compiler.
> 
> Assuming dependencies in the non-C front-end are set up
> correctly, the following should suffice IMHO:
> 
>   1. rm -fr $BUILD_DIR/$TARGET_TRIPLET/$LANG_RUNTIME
>   2. make bubblestrap
>   3. cd $BUILD_DIR/$TARGET_TRIPLET/$LANG_RUNTIME
>   4. make -k check
> 
> For added safety and just to make sure, you can probably
> delete the front-end's files from $BUILD_DIR/gcc/$LANG
> and $BUILD_DIR/gcc/stage2/$LANG before executing the above.
> 
> This seems to work well for me for the Java front-end
> at least. Not so sure about C++ and others though.

If you'd like to propose this as a patch to the web page, I'll approve
it.