Fix the include file mess on linux (solution will impact windows build system also)

[GoogleCodeExporter]

There is a really bad mix between include file on the system and the one
provided by 3rdparty. I create this issue to discuss the solution and
coordinate effort.

* bad mixing of include
By default we include 3rdparty directory, so all .h are taken in this
directory with higher priories.
If you build against systeme libraries, include are taken in 3rdparty...
Cause of people complaining with zlib version. 

* Not standard include path: I hope the path does not depend on distribution.
Example: zlib.h
External include -> /usr/include/zlib.h (gcc use /usr/include)
Internal include -> 3rdparty/zlib/zlib.h (gcc use 3rdparty)
The issue is the code source uses #include "zlib/zlib.h" so the compilation
will always uses the internal include. But it can be linked agains sytem
library.

Solutions: The idea will be to add some extra directory in 3rdparty to
allow to pick only some 3rdparty modules and not all-or-nothings.

Zlib and bzip are ok, we must fix the code to include "zlib.h" instead of
<zlib/zlib.h>. There is also an issue with portaudio
"portaudio/include/portaudio.h"
For soundtouch and liba52, we need to create the following dir (be aware of
the case)
3rdparty/<soundtouch_internal>/soundtouch/<soundtouch files>
3rdparty/<liba52_internal>/a52dec/<a52 files>

In the build system, we need to include 3rdparty/zlib, 3rdparty/bzip,
3rdparty/<soundtouch_internal>, 3rdparty/<liba52_internal>

Original issue reported on code.google.com by gregory....@gmail.com on 4 Jun 2010 at 11:51

[GoogleCodeExporter]

Just for information and see the impact. I join a patch of what I changed for
building with my system libraries.

Original comment by gregory....@gmail.com on 4 Jun 2010 at 12:05

Attachments:

• 02_fix_include_path.patch.gz

[GoogleCodeExporter]

Ok I'm still mulling over the best way to go about this.

Clearly the obvious things to fix are the Soundtouch case sensitivity (which 
sucks 
because via SVN workings we'll need to move the folder *twice* just to change 
it's 
case), and the a52dec naming consistency.  But before we do that, I have 
another idea 
to throw out (below).

The less obvious is zlib.  I actually have a beef with systems that install 
lots of 
headers in the global public namespace -- ie, without putting them in a 
subfolder 
named for the library.  So when I built the pcsx2 svn, I made sure all the 
3rdparty 
libs were fully qualified names, to avoid such inadvertent header file 
conflicts that 
can occur from having a dozen 3rdparty libs' headers in the global root-level 
search 
path.  I figured it would break Linux stuff, but I was hoping there'd be a 
makefile-
level workaround for it linux-side.

Alas, I found out later that there really isn't one, least not without some 
crazy 
amount of headaching.

Is the problem that GCC always pulls the <zlib.h> from the system-installed 
zlib?  I 
seem to recall that from my reading of the include order.  Namely that there's 
no way 
to insert new system-level includes into the top of GCC's includes list.  I had 
felt 
that <> would be preferable on win32 systems (thanks to msvc allowing pretty 
strict 
control over include dir order), but it's not really important either way.  It 
won't 
affect any standard Win32 build setup.

So feel free to remove the <> whenever it suits you.

** Regarding soundtouch and a52dec renaming **

I've been mulling over the idea of moving all of the 3rdparty libs to a folder 
*outside* /trunk.  Drawbacks: windows devs will need to checkout and update two 
svn 
folders (which means the googlecode svn tutorial sample won't even be 
sufficient).  
Accidental compilation errors can occur from updating one but forgetting to 
update 
the other.  Advantages: Linux devs won't have to check out 3rdparty at all, so 
long 
as their soundtouch and/or zlib are up-to-date; and technically speaking 
3rdparty 
really shouldn't be in the trunk and shouldn't be part of svn branches and such.

... but the idea of having our windows devs have to check out two folders is 
probably 
a deal breaker.  In any case, if I *do do that, it'd be a perfect time to 
rename 
soundtouch and a52.  Question is if I'll do it (and the answer is probably not, 
but 
we'll hold off on renaming the folders for a couple days until we can come to a 
definitive decision on it).

Original comment by Jake.Stine on 5 Jun 2010 at 3:45

[GoogleCodeExporter]

Hum,

IMHO, for the moment you can keep 3rdparty directory in trunk.

The first problem for zlib is not <> (not sure of the real impact) but the path 
in C.
We need to do include "zlib.h" instead of "zlib/zlib.h". Because there is no
/usr/include/zlib/zlib.h in the system only /usr/include/zlib.h

The only gcc tricks that I see is the -I option. With last cmake (can look
SearchForStuff.cmake) it can handle it. The idea is to not use include from 
3rdpary
but from 3rdpary sub-directory. 3rdparty vs 3rdparty/zlib,
3rdparty/soundtouch_internal etc... 

For example if you want to use zlib 3rdparty and you do not have soundtouch on 
your
system. -I flags will be in this order: -I3rdpary/zlib 
-I3rdparty/soundtouch_internal
-I/usr/include -I3rdparty

What we can do.

* zlib/bzip/(portaudio):
1/add 3rdparty/zlib in the include directory build system. 
Note: For codeblock if you want to use 3rdparty zlib by default it must be put 
before
/usr/include. In case you want to use system one put it last.
2/Fix the c code to use "zlib.h" instead of "zlib/zlib.h"

* a52/soundtouch
1/create a new subdirectory in 3rdparty. For example 3rdpary/soundtouch_internal
2a/move 3rdparty/SoundTouch to 3rdpary/soundtouch_internal/soundtouch
2b/create a symlink to keep compatibility. ln -s
3rdpary/soundtouch_internal/soundtouch 3rdparty/SoundTouch
# now it is like zlib example
3/Update the build system to include 3rdpary/soundtouch_internal
4/fix the c code to use the good include path.
5/when done, remove the symlink.

Original comment by gregory....@gmail.com on 5 Jun 2010 at 10:41

[GoogleCodeExporter]

Ugh, clearly there's going to be no nice solution to this, eh?

Step One will be removing any -I for "3rdparty" -- we're going to need to add 
includes for each and every lib independently on both Linux and Windows.

Second is apparently the need and necessity to place soundtouch and a52 in 
subfolders, which bugs the hell out of me, but there's really no other solution 
if we 
want to have per-component select-ability on Linux.

A third idea I'm honestly mulling over at this time is creating a set of header 
files 
that just "pipe" to the proper include, based on compiler define.  For example:

---> myzlib.h:
#ifdef _USE_SYSTEM_ZLIB_
#   include <zlib.h>
#else
#   include "../zlib/zlib.h"
#endif
<---

And stick those in a new /3rdparty/includes folder (one include file each for 
zlib, 
bzip2, portaudio, etc.).  Thoughts?

Original comment by Jake.Stine on 6 Jun 2010 at 6:36

[GoogleCodeExporter]

Point 1/ Just for information: if we remove 3rdparty, we will not possible to 
select
some 3rparty include (google, GL, etc...).
 a/ we move it also in a sub-folder, which 
 b/ we just do not use for linux build. But I thinks it is the best way. Not sure if
there are really used anyway (we do not link agains library).

Point 3/ Interesting idea, that move the selection from build system to c. In my
mitigate on this one. Previous solution is more standard, if you do not want to 
build
against 3rdparty include, just remove the dirs. In this situation you have 2 
layers.
Gcc must select the flags -> pipe_include select the real include. So you do not
delete gcc trick. The advantage of this one, it to avoid to change the windows 
build
system (but change a couple of directory is probably not difficult)

I think the good direction for linux build is to drop the 3rdparty in the 
future. For
the moment only soundtouch is really needed. Keep the pipe_include layer will 
not be
clean at that time. Maybe I dream, if the library is used by important software 
(aka
software that standard people use) it will probably create new packaging 
career. Just
put a big fat warning ^^ that soundtouch is not up to date and will create some 
sound
crack. 

Original comment by gregory....@gmail.com on 6 Jun 2010 at 9:40

[GoogleCodeExporter]

Yeah, maybe that really is the best way to go.

Ok, let's work from the angle that 3rdparty isn't used on Linux; *except* for 
Soundtouch.

Thought #1:  Symlinks work on Linux, and can be added to the svn repo.  Create 
a 
soundtouch_internal folder and inside it have *only* a symlink to 
/3rdparty/SoundTouch.  The symlink won't work on windows, but it's not needed 
here 
anyway, since it's case-insensitive.

Will that work?

(could optionally do the same for A52, though there's no compelling reason to 
even 
have an option to use the pcsx2 included version at this time).

Also, removing /3rdparty from the -I list is still a good idea, I think.  I 
never 
actually intended for the linux build of pcsx2 to use the packaged GLEW and GL 
headers (note: don't think GLEW is even used anymore -- it was only used by 
GSdx 
iirc).  Linux distros have their own packages of that stuff available anyway, 
as 
usual.

Better if we just add -I's for the specific selected packages only.

I'll add /3rdparty/zlib and /3rdparty/bzip and /3rdparty/SoundTouch/include to 
the 
MSVC project include lists; that will allow VC to compile everything error-free 
with 
the linux-friendly include names.

#include "zlib.h"

Original comment by Jake.Stine on 7 Jun 2010 at 1:01

[GoogleCodeExporter]

An others possibility for soundtouch (a52) is to include 3rdparty/SoundTouch/ 
and in
c only do include "SoundTouch.h" ?

If I sum up the idea:
3rdparty/sound_touch_internal/soundtouch (link to 3rdparty/SoundTouch)
in c we have include "soundtouch/the.h", for linux is normally ok (I will do a 
quick
test to be sure). Windows I do not know.

/3rdparty/SoundTouch/include < you want to say 3rdparty/portaudio/include

Original comment by gregory....@gmail.com on 7 Jun 2010 at 8:33

[GoogleCodeExporter]

Glew's still used; it was a dependency of ZeroGS and ZZOgl before GSdx. On 
Linux, it 
uses the system version, though. It's possible someone might want to use it 
while 
compiling ZZOgl for Windows, I suppose.

With A52, is there a compelling reason to use it at all? The only place it's 
used is 
in Decoder.cpp in spu2-x, and I got the impression from gigaherz a while ago 
that 
that code using it never actually worked...

And we can use the system libraries for zlib, but it looks like we can't depend 
on 
anything past 1.2.3 for the moment.

And I'm good with only using SoundTouch internally in Linux, and having 
everything 
else as dependencies.

Original comment by arcum42@gmail.com on 7 Jun 2010 at 8:39

[GoogleCodeExporter]

For information, I do a search of soundtouch include files in all debian 
packages. There is no collision of include file. So I think it is reasonable to 
use -I 3rdparty/SoundTouch/ and so keep the exact svn structure ^^ Any opinion.

Original comment by gregory....@gmail.com on 8 Jun 2010 at 9:39

[GoogleCodeExporter]

Ok, let's go with that for now.  If it proves to be a problem, it'll be easy 
enough to do a full a roper relocation of soundtouch at some point later on.

Original comment by Jake.Stine on 8 Jun 2010 at 10:59

[GoogleCodeExporter]

Yes. Unfortunetaly we can not do the same for liba52 in case it is really 
useful.

Original comment by gregory....@gmail.com on 8 Jun 2010 at 11:37

[GoogleCodeExporter]

I tweaked the include patch slightly for the inclusion of liba52, since the 
version in the patch won't compile in windows (we lack inttypes.h).

Apply the patch whenever you're ready; since I think the patch also needs to be 
paired with the soundtouch shortcut creation, and with some cmake changes?  In 
any case, I committed necessary changes to the VisualStudio projects to allow 
the patch to compile error free; so we should be good to go. :)

Original comment by Jake.Stine on 10 Jun 2010 at 7:49

Attachments:

• fix_include_paths.v2.patch.gz

[GoogleCodeExporter]

Yes I was really not sure about inttypes.h.

Soundtouch case,
1/ With the last patch, a symlink is necessary.
2/ The others method was to include directly 3rdparty/SoundTouch and to include 
only the file ie (#include "SoundTouch.h")

A52 case, I will only support the a52 system library. There is no reason to use 
the pcsx2 one for linux. So no need to change svn for this library.

Others library are already fine on cmake.

Original comment by gregory....@gmail.com on 11 Jun 2010 at 8:15

[GoogleCodeExporter]

For the moment, I commit the method 1. In case you change your mind, it will be 
easy to update to case 2.

Original comment by gregory....@gmail.com on 11 Jun 2010 at 1:46

[GoogleCodeExporter]

time to close it.

Original comment by gregory....@gmail.com on 21 Jun 2010 at 2:23

• Changed state: Fixed