[1/2]tools/winemaker: add project-file support(try3)

[Francois Gouget]

Hi,

I had a look at this patch and here are some comments.

The case of the filenames in the dsp and dsw files may not match the 
case of the filenames on the filesystem. So you need to ajsut the names 
before you put them in the makefile. I believe search_from() can be used 
for that.

I see that you're translating the Visual C++ compiler flags to gcc 
compiler flags. That's good. But I'm not sure about the following 
conversion from /GX to -fexceptions.
Exception handling is pretty different between windows and Unix. Do you 
know if /GX is about structured exception handing? I really couldn't 
tell, but maybe some exception expert will know.

I see you're converting the /Zp1 option which is very nice (packing 
being one of the disagreements between Visual C++ and gcc). But you 
don't do it for Zp(2|4|8|16). I believe you could do it by simply 
appending the packing size to the '-fpack-struct=' option. (But maybe I 
missed something)

A bigger issue is that you're putting all the compiler options in 
CXXEXTRA which only applies to C++ code. But in the dsp file it looks 
like this applies to both C and C++ code. So it probably should be put 
in both (except -fexceptions which seems to be on by default in C++, 
but that's minor).

About the commented out call to fix_file_and_directory_names(), I don't 
think winemaker should go and modify files outside of '.'. So I think 
it's safe to remove this call entirely.


Also I have some questions about a couple of heuristics:
  if ($has_headers) {
    push @{@$project_settings[$T_INCLUDE_PATH]},"-I.";
  }

Is this still needed? In source_scan_directory() it was needed because 
we had no idea what the include path was supposed to look like. But the 
project file should define it clearly enough for us. However I have a 
dsp file in a directory with a header file and I don't see a '/I.'. So 
maybe it is implied?

Also this part seems wrong:
  # Finally if we are building both libraries and programs in
  # this project, then the programs should be linked with all
  # the libraries

The project file should be explicit enough on this part for us not to 
have to do any guessing.

Also do we really need to worry much about matching source file with a 
given target? Won't we always have a single target per dsp file? In 
which case the matching can probably be simplified.

The end of source_scan_project_file() looks pretty similar to the end of 
source_scan_directory(). Maybe some code can be shared.


To make the patch smaller you can first submit the dsp parser, then the 
dsw one, the vcproj one, and finally the sln one.

And once that's done, it would be nice to integrate this all with the 
regular source_scan() function, so that if it detects one of these files 
in the subdirectories, it invokes the appropriate project parser for 
that directory and stops recursing. That can be done as a further 
separate patch.


Style issues:

The style issues are at least partly my fault as I used two-space 
indentation and braces on the same line when I first wrote winemaker. 
Nowadays I'd prefer 4-space indentation with braces on their own line 
but that's a bit tricky when maintaining a consistent indentation.

In any case, in places your code is too incoherent. For instance:
      } elsif(/^# TARGTYPE/) {
          if (/[[:space:]]0x0101$/) {
            # Win32 (x86) Application
            $prj_target_type=1;

This snippet has both 2 and 4 space indentation and it's all new code. 
There's also an indentation problem there:

  }
      }
      foreach my $source (@sources_misc) {
  if ($source =~ /^$basename/i) {

Personnally I'd recommend to maintain a consistent indentation scheme at 
least per function (though the policy is more to be consistent per file 
normally :-/ ).

Also, I'd prefer to have a space between 'if' and the prentheseses and 
braces.

You can also simplify a some string concatenations. Replace:

      $prj_target_cflags=$prj_target_cflags."-D".$1." ";
with
      $prj_target_cflags.="-D$1 ";


In any case, thanks for taking this on. I think you're pretty close to 
the end (even if you don't feel that way after reading the above<g>).


-- 
Francois Gouget <fgouget at free.fr>              http://fgouget.free.fr/
               If you think the whole world revolves around you,
                 quit staring at the GPS display while driving.