[cdt-dev] 3 bugs/problems with the class GnuMakefileGenerator

[Jan Baeyens <jan@xxxxxxxxxx>]

Hi
I succeeded to make a toolchain that allows me to have a library and my 
project code in the same project.
The difficulty in doing so is that the datamodel supported in plugin.xml 
is not supported by the default class GnuMakefileGenerator.

Technically I need to "split" the code to send part of it to the 
archiver and part of it to the linker.
As the plugin.xml allows for multiple outputtypes for 1 tool and each 
output type has its own "buildVariable" I thought it would be possible 
to get this to work with the standard toolchain.

The solution I came up with is as follows:
Using 2 "primary output" "outputtypes" with different "build variable" 
(AR_OBJ and LINK_OBJ) values and different (complementary) "name 
providers".
AR_OBJ is used as input for the archiver and LINK_OBJ is used as input 
by the linker.
Using 2 complementary "name provider"s I filter the files that need to 
go to the linker or the archiver. I can do this easily because the files 
to archive are in a subfolder.
Both of the outputtypes must be primary output (at least as far as I 
understood)
So there are 3 key attributes. I found a bug in all 3 of them in 
GnuMakefileGenerator.

I copied the code of GnuMakefileGenerator to create my own 
MakefileGenerator to make it work and I found that only some minor 
changes are needed to get it to work. Moreover the changes look more 
like bug fixes than enhancments.

Problem 1 : When using more than 1 output type with the flag "Primary 
output" only the first one is found. All others are ignored. I consider 
this a bug as the data model allows to have multiple "primary output"s. 
If there can only be one it should be defined in the "tool" object and 
not in the "output type"

Problem 2: Buildvariable is always the buildvariable defined in the 
first "outputtype" taged with "primary output type true" and if none 
tagged the first output type is used. I consider this to be a bug as the 
data model allows to specify a build variable for each outputtype but 
the code ignores this.

Problem 3: Name provider can not return null. As such it is impossible 
to completely ignore a input file. All cases -except one- cater for 
null. The fact that the default generated code returns null does not 
prove the code should support null but ....

Fixing problem 1 and 2 may introduce downwards incompatibility. Fixing 
problem 3 will not introduce downwards incompatibility. I think it would 
be a good idea to fix this in the CDT code.

How I fixed it.
Problem 1:
on line 3113 it states :
boolean primaryOutput = (type == tool.getPrimaryOutputType());
Replacing with
boolean primaryOutput = type.getPrimaryOutput();
Fixes the bug.

Problem 3: (I do 3 first as the solution of 2 is easier to explain when 
3 is done)
on line 3256/7 it states:
IPath[] outPaths = nameProvider.getOutputNames(tool, inPaths);
for (int j=0; j<outPaths.length; j++) {
...
}
This fails if nameProvider.getOutputNames(tool, inPaths); returns a null.
as outPaths is no longer used after the for loop the code below fixes 
the problem
IPath[] outPaths = nameProvider.getOutputNames(tool, inPaths);
if (outPaths!=null){
  for (int j=0; j<outPaths.length; j++) {
  ...
  }
}

Problem 2: (this one needs changes at several locations)
The bug is at line 2168
IOutputType outType = tool.getPrimaryOutputType();
because later (2148) it states
buildVariable = outType.getBuildVariable();
This way the build variable is always the build variable of the 
PrimaryOutputType or the first outputtype if none is provided.
To fix this I removed the line and changed outType to a field (I 
refactored/rename to usedOutType)
The code changed in Probem 3 knows the outputType so I changed that to
IPath[] outPaths = nameProvider.getOutputNames(tool, inPaths);
if (outPaths!=null){
  usedOutType=type;
  for (int j=0; j<outPaths.length; j++) {
    ...
  }
}
To make sure the code doesn't use old "output type" values I changed 
line 2108 from
addRuleForSource(relativePath, ruleBuffer, resource, sourceLocation, 
rcInfo, generatedSource, generatedDepFiles, generatedOutputs);
to
usedOutType = null;
addRuleForSource(relativePath, ruleBuffer, resource, sourceLocation, 
rcInfo, generatedSource, generatedDepFiles, generatedOutputs);

This is all that is needed to fix it.
My question now is: Do I need to bother to create defects? And if so do 
I create 1 for the 3 changes or 3 defects?

Best regards
Jantje