Re: [CrystalSpace] #965: CS-2.0_beta3: Build system not fit for system wide installation on Linux

#965: CS-2.0_beta3: Build system not fit for system wide installation on Linux
--------------------------+-------------------------------------------------
 Reporter:  yamakuzure    |       Owner:  admin           
     Type:  defect        |      Status:  new             
 Priority:  major         |   Milestone:  Version 2.0 pre1
Component:  build system  |     Version:  V2.0            
 Keywords:                |  
--------------------------+-------------------------------------------------

Comment(by sunshine):

 Replying to [comment:7 yamakuzure]:
 > First of all: Sorry for the late answer. I have no idea why, but trac
 does not send any e-mails to me. Maybe I missed some preferences
 somewhere?

 Trac will send emails if you provide your full email address for the
 "Reported by" field. If you want to give me your email address, I can
 adjust "Reported by" so that you receive emails when this ticket is
 updated. If you do so, your email address will only be visible in Trac by
 developers. Spambots will see only "`yamakuzure@...`". Alternately, you
 can subscribe to the `crystal-tracker` mailing list to which all ticket
 changes are mailed. However, `crystal-tracker` also receives SCM change
 messages, so it is not as discriminating as just providing your email
 address for "Reported by".

 > Then my patch failed its purpose. For instance: ODE is installed in
 /usr/lib, but I have a development version installed in /usr/local/lib.
 The patch was meant to make configure to not look into /usr/local/lib,
 where it'd find the development version for my private pleasure, and not
 the system wide installed version it ought to use.

 Thank you for the clarification. This is the sort of explanation which is
 helpful to include in a change log message accompanying a patch.

 Unfortunately, there isn't an obvious right or wrong answer or approach to
 this issue. Many people are in exactly the opposite situation where they
 are on platforms where the distribution-supplied libraries are very old
 (often ancient). Consequently, the user installs newer versions in
 `/usr/local` and wants the newer version discovered. This situation is
 very common on Mac OS X.

 For your particular example, you could configure CS with `--with-ode=/usr`
 to avoid the ODE in `/usr/local`, but this is not a general solution.
 Perhaps it might make sense to introduce a control (either via command-
 line switch or environment variable) which tells CS `configure` to disable
 automatic searching of `/usr/local`.

 > About the 'custom' mode: Yes, I agree the name is not very creative. It
 was the best I could come up with in a very limited amount of spare time.
 (And name generation is one of my strongest weaknesses! ;))

 Indeed, it is not always easy coming up with a name. Of the alternatives I
 listed, I somewhat like ''inherit'', ''env'', and ''user'', though they
 are not great. ''custom'' might be good enough if we can't find something
 better.

 > However I'd like to point out that a) I have no idea how autoconf and
 JamConfig are really working in deep detail, so much of this was "learning
 by doing", and b) the 'custom' mode is more an idea than a the missing of
 it a show-stopper. :)

 Having a custom mode seems valid, as does disabling "cpu-specific
 optimizations" in custom mode. That patch needed some cleanup and fixes,
 as I noted in my inital review, but it was otherwise in relatively decent
 shape. With the noted problems corrected, I think that this is a change
 which could be incorporated into the project without much hassle.

 > The next thing I have no real knowledge about is the m4 system. The
 contents of these new functions are a combination of shell programming and
 ... well ... google. Normally I can handle everything with a bunch of
 Makefiles. (At least in the job I am getting paid for, anyway.) So I
 apologize for any bugs or incomplete implementations. A pro on autoconf
 should take care of this, really.

 Except for the issue of non-atomic build flags like "-framework Foo" being
 corrupted by folding out duplicates, the problems noted in my review were
 purely issues of robustness with the shell code.

 Replying to [comment:8 yamakuzure]:
 > `./config.status: line 1408: 25322 Segmentation fault ( "$JAM"
 -sDUMPTARGETS_FILE=$tmp/jcache dumptargets 1>&2 ) 2>&5`

 Ideally, ftjam should not segfault (ever). If provided with arguments too
 long for its buffers, it should at least emit a diagnostic and exit with
 an error code. Better would be for the ftjam developers to remove these
 hard-coded limits. Aside from trackling the issue on the CS side, perhaps
 it would make sense to submit a bug report to ftjam as well.

 > So the global CFLAGS get added twice, first duplication riddle solved.
 The same goes with the CXXFLAGS ...and the LDFLAGS. For the record: If I
 manually delete the duplications in Jamconfig, everything is fine.

 I tracked this problem down to an interaction between the Autoconf macros
 `CS_PROG_CC` / `CS_PROG_CXX` / `CS_PROG_LINK` and how `configure` handles
 the result of the `CS_CHECK_CSWIN32LIBS` check. The situation seems to
 have arisen over time as the build system was generalized. Historically,
 `CFLAGS`, `CXXFLAGS`, and `LDFLAGS` were emitted once. Even after the
 introduction of `CS_CHECK_CSWIN32LIBS` they still were emitted only once.
 However, the introduction of `CS_PROG_CC` and cousins caused them to be
 emitted twice: once by `CS_PROG_CC` and cousins, and once by
 `configure.ac` itself. `CS_PROG_CC` and cousins emit those flags because
 those macros want to be self-contained, so as to be useful by other
 projects. CS's `configure.ac`, however, also needs to emit those flags for
 itself in order to ensure that the results of `CS_CHECK_CSWIN32LIBS` are
 utilized by the build.

 I think that the issue can be resolved satisfactorily with the following
 patch, but I would appreciate if someone on Windows and any other generic
 Linux could test the patch to ensure that it does not have any undesirable
 impact.

 {{{
 #!diff
 Index: configure.ac
 ===================================================================
 --- configure.ac        (revision 38350)
 +++ configure.ac        (working copy)
 @@ -141,19 +141,14 @@
  # manually one at a time.
 #------------------------------------------------------------------------------
  CS_NOTABLE([cs-winlibs], [], [windows], [$cs_cv_cslibs],
 -    [CS_CHECK_CSWIN32LIBS])
 +    [CS_CHECK_CSWIN32LIBS
 +    AS_IF([test "$cs_cv_cslibs" = yes],
 +
 [CS_EMIT_BUILD_PROPERTY([COMPILER.CFLAGS],[$cs_cv_cslibs_cflags],[+])
 +
 CS_EMIT_BUILD_PROPERTY([COMPILER.C++FLAGS],[$cs_cv_cslibs_cflags],[+])
 +
 CS_EMIT_BUILD_PROPERTY([COMPILER.LFLAGS],[$cs_cv_cslibs_lflags],[+])])])


 -#------------------------------------------------------------------------------
 -# Use the CPPFLAGS, CXXFLAGS, CFLAGS, and LDFLAGS passed to configure, as
 well
 -# as any additional flags provided by cs-win32libs.
 -#------------------------------------------------------------------------------
 -CS_EMIT_BUILD_PROPERTY([COMPILER.CFLAGS], [$CPPFLAGS $CFLAGS], [+])
 -CS_EMIT_BUILD_PROPERTY([COMPILER.C++FLAGS], [$CPPFLAGS $CXXFLAGS], [+])
 -CS_EMIT_BUILD_PROPERTY([COMPILER.LFLAGS], [$LDFLAGS], [+])

 -
 -
 #------------------------------------------------------------------------------
  # Check for typical required libraries (libc, libm, libmx, libdl,
 libnsl).
 #------------------------------------------------------------------------------
 }}}

 > But there is another one. I want -O2, but it gets overwritten by -O3.
 And further, your configure adds flags, that are already enabled. (like
 -fomit-frame-pointers) And some other options are sabotaging the
 optimization efford, like using -mtune=generic instead of -march=native.
 Or better: Leave such options out. Package maintainers compiling for a
 bianry distro will want generic options. Users compiling for their own
 machine are better off compiling for their machine only. These were the
 reasons for me to try to a) get rid of duplicate options, and b) add a
 'custom' mode where the user is solely resposible for.

 Your suggested ''custom'' mode should address these issues nicely, and
 with the mentioned cleanups and fixes, it should not be difficult to get
 it merged into the project.

 ----

 To move forward, it would be helpful if you could create a separate Trac
 ticket for each distinct and well-defined issue which you want to resolve
 rather than intermingling them into one large problem report. Doing so
 will make it easier to review the patches and carry on properly targetted
 discussion. Also be sure to link back to this ticket via the notation
 "#965" in the description of each new ticket to make it easy to refer to
 earlier context. Distinct tickets might cover:

  - ''custom'' build mode (along with disabling cpu-specific optimizations,
 etc.)
  - Duplicate `CFLAGS`, `CXXFLAGS`, `LDFLAGS` lines in `configure.ac`
 (which my above suggested patch might resolve).
  - Optional suppression of `/usr/local` in searches.
  - Backquotes in `configure.ac` comments botching your editor?

 Finally, the `crystal-develop` mailing list is a good place to discuss
 fixes to the build system. Often, a developer on the list can suggest a
 better approach to resolve a problem than a solution and patch derived in
 isolation.

-- 
Ticket URL: <http://www.crystalspace3d.org/trac/CS/ticket/965#comment:9>
CrystalSpace <http://www.crystalspace3d.org/>
Free open-source 3D SDK.
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/