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 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?

 Replying to [comment:4 sunshine]:
 > Replying to [ticket:965 yamakuzure]:
 > > [...] configure allows to set specific paths for the parts of the
 installation and to add custom `CFLAGS`, `CXXFLAGS` and `LDFLAGS`. The
 current build system does not take care of those custom flags, which ends
 in `ftjam` segfaulting if the command lines become too large (over ~1,000
 characters). The first thing that crashes is `config.status` when trying
 to build the `Makefile` emulation.
 >
 > It would be helpful to present the actual command-line composed by Jam
 which is greater than 1000 characters so that we can determine if the
 generated command is bogus. (This has happened in the past when some
 change was made to the CS `Jamfiles` resulting in unnecessarily long
 commands.) Without seeing the actual problem command(s), it also is
 difficult to judge the involvement of `CFLAGS`, `CXXFLAGS`, and `LDFLAGS`.

 Well, I do not have the logs any more, as they where built using debug
 level 6 or 7, meaning it was huge. But believe me, it contained the same
 flags over and over again, which seems to be understandable seeing that
 the configure script simply hammers everything together it finds, being
 superfluous or not.

 >
 > Also have you tried building with stock `jam`, rather than `ftjam` which
 we do not support explicitly? You might also have better luck with the
 `jam` bundled with CS. If I recall correctly, neither stock `jam` nor the
 one bundled with CS suffer from the 1000-character limit. The `jam`
 bundled with CS should be available automatically as `./jam` in the CS
 build directory if `jam` was not found on your system.

 Other versions of jam are not available on gentoo linux. Therefore it
 would be futile to use a different brand. FTJam is a 100% compatible
 enhanced Jam implementation.
 Further, using the bundled one would mean that developers build CS with a
 different Jam implementation than they'd build their apps. (But the latter
 is quite philosophical, yes.

 >
 > > '''1'''.: `configure.ac` forces `/usr/local/lib` (and
 `/usr/local/include` if present) upon users, which is a bad thing to do.
 Patch in a check to not do this if the `--prefix`, `--libdir` and/or
 `--includedir` options lead to default paths.
 >
 > It is not at all clear what the intention of this patch is, nor what you
 mean with ''is a bad thing to do''. Please provide more context explaining
 what actually goes wrong in your case, such as error messages or a
 concrete argument if the issue is a philosophical one.
 >
 > With this patch, it appears that you are conflating locations which CS
 searches for 3rd-party libraries with locations into which CS gets
 installed. The code which this patch touches has nothing to do with
 locations into which CS gets installed. This code is merely a convenience
 to help `configure` locate 3rd-party libraries on platforms on which the
 development tools do not search `/usr/local/{include,lib}` by default.
 Options `--prefix`, `--libdir`, `--includedir`, on the other hand, are all
 installation-time options which the CS build system does respect.

 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.

 >
 > Minor comment: The patch also makes meaningless whitespace changes
 (adding blank line and converting tab to spaces) which make it more
 difficult to review.

 Sorry, forgot -w option on diff.

 (...snip...)
 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! ;))

 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. :)

 (...snip...)
 > > '''3'''.: Before the new custom mode can be set into action some new
 functions to `mk/autoconf/trim.m4` have to be introduced. This patch does
 that.
 >
 > Unfortunately, these additional functions both lack robustness and will
 break the build for other platforms. Problems with
 `CS_{APPEND,PREPEND}_TRIMMED` include:
 (...snip...)

 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.

 However, I really appreciate all your hints and tips!
 > The hunk which unconditionally clears `CFLAGS` and its cousins just
 seems wrong. These flags are not only emitted into `Jamconfig`, but they
 are used by `configure` itself when performing checks, and the user may
 very well have assigned specific values to these flags before running
 `configure` in order to alter how the checks are performed or to make
 checks succeed which might otherwise fail. Therefore, clearing them
 unconditionally is the wrong approach; and it's not even clear what
 benefit you gain by doing so.

 They are saved and restored after getting rid of duplicates. At this point
 the *FLAGS might be composed from several sources.


 > All of the files in `CS/mk/autoconf` are intended to be project-
 agnostic, which means that they can be used in projects other than CS, so
 the modifications to `compiler.m4`, in particular, are very problematic.
 Any project external to CS which is based upon the CS build system (via
 `CS/scripts/jamtemplate`, for instance) will be broken by this change.

 Sorry, I did not know this. :-(

 >
 > > '''8'''.: `cs-config` doesn't look into `/usr/lib[32|64]`, yet.
 >
 > Unfortunately, this patch performs a wholesale conversion of tabs to
 spaces, which makes it appear as if the entire file was rewritten, thus
 completely obscuring from the person reviewing the patch any ''real''
 changes made to support `/usr/lib{32,64}`. Even with the whitespace
 changes removed, the patch is plagued by several instances of churn, such
 as splitting or joining of lines of code, unrelated to the fundamental
 `/usr/lib{32,64}` change, once again making the patch more difficult to
 review. (Churn which makes code formatting more consistent may be okay,
 but should be submitted as a separate patch so as to not obscure ''real''
 changes in a more fundamental patch.)

 I have no idea how this happened. Normally I do not route a diff into a
 patch file before having controlled the outcome. They all looked ok and
 applied perfectly. But I'll take more care next time.

-- 
Ticket URL: <http://www.crystalspace3d.org/trac/CS/ticket/965#comment:7>
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/