dolfin team mailing list archive

[Johan Hake <hake@xxxxxxxxx>]

On Tuesday 30 September 2008 17:28:19 Garth N. Wells wrote:
> Ilmar Wilbers wrote:
> > Anders Logg wrote:
> >> On Tue, Sep 30, 2008 at 02:35:21PM +0200, Johannes Ring wrote:
> >>> On Tue, September 30, 2008 13:24, Anders Logg wrote:
> >>>> On Tue, Sep 30, 2008 at 09:38:07AM +0200, Johannes Ring wrote:
> >>>>> On Tue, September 30, 2008 09:15, Anders Logg wrote:
> >>>>>> On Tue, Sep 30, 2008 at 08:39:41AM +0200, Johannes Ring wrote:
> >>>>>>> On Sat, September 27, 2008 20:05, Garth N. Wells wrote:
> >>>>>>>> Johan Hake wrote:
> >>>>>>>>> On Saturday 27 September 2008 18:17:12 Harish Narayanan wrote:
> >>>>>>>>>> In order to get the swig-generated dolfin_wrap.cc to build with
> >>>>>
> >>>>> my
> >>>>>
> >>>>>>> gcc,
> >>>>>>>
> >>>>>>>>>> I need to perform some simple substitutions on the file which
> >>>>>
> >>>>> I've
> >>>>>
> >>>>>>>>>> expressed as a perl script:
> >>>>>>>>>>
> >>>>>>>>>> perl -pi -e 's/Type v;/Type v = 0;/g' dolfin/swig/dolfin_wrap.cc
> >>>>>>>>>
> >>>>>>>>> Maybee you could make a runnable script called 'swig' and put it
> >>>>>>>>> somwhere in
> >>>>>>>>> your path, and make sure it executes before the system swig. Then
> >>>>>
> >>>>> in
> >>>>>
> >>>>>>> the
> >>>>>>>
> >>>>>>>>> script you call the system swig with the handed sysargs. After
> >>>>>
> >>>>> that
> >>>>>
> >>>>>>> line
> >>>>>>>
> >>>>>>>>> you
> >>>>>>>>> execute the command above. Its a hack, but I do not think scons
> >>>>>>>
> >>>>>>> support
> >>>>>>>
> >>>>>>>>> this
> >>>>>>>>> in a better way.
> >>>>>>>>
> >>>>>>>> I don't really like this approach. I would prefer to enable
> >>>>>
> >>>>> different
> >>>>>
> >>>>>>>> compiler flags for dolfin_wrap.cc, i.e. do not use -Werror, if
> >>>>>>>> that
> >>>>>>>
> >>>>>>> does
> >>>>>>>
> >>>>>>>> the trick. dolfin_wrap.cc won't compile under Ubuntu Intrepid with
> >>>>>>>> -Werror, so we'll need to do something.
> >>>>>>>
> >>>>>>> What if we removed the -Werror flag from CXXFLAGS as default and
> >>>>>
> >>>>> instead
> >>>>>
> >>>>>>> let the user decide whether to add it through the customCxxFlags
> >>>>>
> >>>>> option?
> >>>>>
> >>>>>>> This option is cached in scons/options.cache so you only have to
> >>>>>>> set
> >>>>>
> >>>>> it
> >>>>>
> >>>>>>> once.
> >>>>>>>
> >>>>>>> Currently, the only way a user can remove the -Werror flag when
> >>>>>
> >>>>> running
> >>>>>
> >>>>>>> scons is by turning off debugging with enableDebug=no. I don't
> >>>>>>> think this
> >>>>>>> is very nice since -Werror isn't really a debugging flag.
> >>>>>>>
> >>>>>>> Johannes
> >>>>>>
> >>>>>> I don't like that. It would increase the risk of someone pushing
> >>>>>> code that compiles with warnings. DOLFIN has been free of warnings
> >>>>>> for a few years now and it was quite a bit of work to remove them to
> >>>>>> begin with.
> >>>>>
> >>>>> Okay, I see.
> >>>>>
> >>>>> Is it okay then if I move the -Werror flag from the debugging flags
> >>>>> to the
> >>>>> default CXXFLAGS and if the user defines CXXFLAGS the default
> >>>>> CXXFLAGS is
> >>>>> overwritten? Like this:
> >>>>>
> >>>>> diff -r cfbe8dbbf3c9 SConstruct
> >>>>> --- a/SConstruct	Mon Sep 29 15:23:53 2008 +0200
> >>>>> +++ b/SConstruct	Tue Sep 30 09:36:49 2008 +0200
> >>>>> @@ -149,7 +149,10 @@
> >>>>>      # FIXME: should we also remove the file scons/options.cache?
> >>>>>
> >>>>>  # Default CXX and FORTRAN flags
> >>>>> -env["CXXFLAGS"] = "-Wall -pipe -ansi" # -Werror"
> >>>>> +if os.environ.has_key("CXXFLAGS"):
> >>>>> +  env['CXXFLAGS'] = os.environ["CXXFLAGS"]
> >>>>> +else:
> >>>>> +  env["CXXFLAGS"] = "-Wall -pipe -ansi -Werror"
> >>>>>  #env["SHFORTRANFLAGS"] = "-Wall -pipe -fPIC"
> >>>>>
> >>>>>  # Default link flags
> >>>>> @@ -157,7 +160,7 @@
> >>>>>
> >>>>>  # If Debug is enabled, add -g:
> >>>>>  if env["enableDebug"]:
> >>>>> -  env.Append(CXXFLAGS=" -DDEBUG -g -Werror")
> >>>>> +  env.Append(CXXFLAGS=" -DDEBUG -g")
> >>>>>
> >>>>>  if not env["enableDebugUblas"]:
> >>>>>    env.Append(CXXFLAGS=" -DNDEBUG")
> >>>>>
> >>>>>
> >>>>> Johannes
> >>>>
> >>>> Sounds good.
> >>>
> >>> Done.
> >>>
> >>> To build without -Werror, define CXXFLAGS before running scons, e.g.,
> >>>
> >>>   CXXFLAGS="" scons
> >>>
> >>> or
> >>>
> >>>   CXXFLAGS="-Wall -pipe -ansi" scons
> >>>
> >>> Johannes
> >>
> >> Nice. Could you add a note on it to the FAQ for people with SWIG
> >> problems?
> >>
> >> As I understand it, we will all have this problem next month when
> >> everyone upgrades Ubuntu... :-)
> >
> > Correct, this is due to the new gcc version. Really, someone should
> > check with the SWIG people and get this fixed...
>
> Looks like it might be fixed in Swig 1.3.36, but Intrepid is shipping
> with 1.3.35. It also looks like 1.3.35 may have some issues with
> shared_ptr which could affect us.
>
> The change still doesn't really help much - I would like to use -Werror
> on the dolfin code, and to not use it on the swig generated code.

Attached is a patch that should fixe this. Now the swig version is extracted 
to a variable "swig_version". This is used in a check for equal or larger 
than 1.3.36. If so use " -Werror" for wrapper code. If not do not use the 
option.

Can someone with the problem comfirm that it works?

Johan

> Garth
>
> > ilmar
> > _______________________________________________
> > DOLFIN-dev mailing list
> > DOLFIN-dev@xxxxxxxxxx
> > http://www.fenics.org/mailman/listinfo/dolfin-dev
>
> _______________________________________________
> DOLFIN-dev mailing list
> DOLFIN-dev@xxxxxxxxxx
> http://www.fenics.org/mailman/listinfo/dolfin-dev


# HG changeset patch
# User "Johan Hake <hake@xxxxxxxxx>"
# Date 1222800262 -7200
# Node ID dde1ee5710f84cc26d6c3c85c33bfaf83dbf2570
# Parent  ab198cf7754b38c71909523ed9746280342e9710
Removing "-Werror" when compiling wrapper code for swig versions
previous to 1.3.36. Could include a check for gcc version too.

diff -r ab198cf7754b -r dde1ee5710f8 dolfin/SConscript
--- a/dolfin/SConscript	Sat Sep 27 23:28:16 2008 +0200
+++ b/dolfin/SConscript	Tue Sep 30 20:44:22 2008 +0200
@@ -105,11 +105,14 @@
     swigFlags = "-python -c++ -shadow -I%s" % os.path.join("include","swig")
     swigFlags = swigFlags.split()
     out, err = scons.runCommand("swig", "-version")
-    # Determine swig version, v 1.3.28 and later supports -O flag
-    m = re.match(r"SWIG Version (.+)", out, re.M)
-    #if scons.checkVersion(m.group(1), "1.3.28"):
+
+    # Determine swig version
+    swig_version = re.match(r"SWIG Version (.+)", out, re.M).group(1)
+    
+    #if scons.checkVersion(swig_version, "1.3.28"):
     #    swigFlags.append("-O")
 
+    # Ths swig environment
     swigEnv = env.Copy()
     swigEnv["SWIGFLAGS"] = swigFlags
     swigEnv["SHLIBPREFIX"] = "_"
@@ -203,8 +206,11 @@
         modEnv = swigEnv.Copy(CXXFLAGS=mod.cxxFlags, LINKFLAGS=mod.linkFlags + linkOpts)
         modEnv.Append(SWIGFLAGS=mod.swigFlags + ["-I%s" % i for i in swigcppPath])
         
-        ### uncomment the following line to remove -Werror from the CXXFLAGS:
-        #modEnv["CXXFLAGS"] = re.sub("-Werror","",modEnv["CXXFLAGS"])
+        # Version previous to 1.3.36 together with gcc version 4.3 and later
+        # produce warnings that are treated as error using -werror flag
+        # FIXME: Is this correct? If so maybee we should insert a chek for gcc version?
+        if not scons.checkVersion(swig_version, "1.3.36"):
+            modEnv["CXXFLAGS"] = modEnv["CXXFLAGS"].replace(" -Werror","")
             
         # Add python dependency (always required in python wrappers)
         pyPkg = configuredPackages[python_version]