Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 312835

Summary:

CDT build settings which are set at the folder level are ignored in certain situations

Product:

[Tools] CDT

Reporter:

MJ <jensem2>

Component:

cdt-build-managed

Assignee:

Andrew Gvozdev <angvoz.dev>

Status:

RESOLVED FIXED

QA Contact:

Chris Recoskie <recoskie>

Severity:

normal

Priority:

CC:

elaskavaia.cdt

Version:

8.0

Target Milestone:

8.0.1

Hardware:

OS:

Linux

Whiteboard:

Attachments:

Description	Flags
Example project showing bug	none
GnuMakefileGenerate.java with fix and comments	angvoz.dev: iplog-
Patch for org.eclipse.cdt.managedbuilder.makegen.gnu.GnuMakefileGenerator.java	angvoz.dev: iplog-
Patch for CDT 8.0	angvoz.dev: iplog+
modified patch	angvoz.dev: iplog-
Additional patch	angvoz.dev: iplog-

Description MJ

2010-05-13 15:29:50 EDT

Build Identifier: 20100218-1602

When generating makefiles on linux for GCC:

The autogenerated makefile includes various subdirectory-level makefiles
i.e. 
# All of the sources participating in the build are defined here
-include sources.mk
-include subdir.mk
-include src/subdir.mk
-include src/foo/subdir.mk
-include objects.mk

(From my investigating, this occurs in the GnuMakefileGenerator class)

Within each subdir.mk makefile, build rules appear for either (a) specific files in the subdirector and/or (b) all files (%.o) in the directory.

i.e. src/subdir.mk includes
---------------
src/%.o: ../src/%.cpp
	@echo 'Building file: $<'
	@echo 'Invoking: GCC C++ Compiler'
	g++ -O0 -g3 -Wall -c -fmessage-length=0 -MMD -MP -MF"$(@:%.o=%.d)" -MT"$(@:%.o=%.d)" -o"$@" "$<"
	@echo 'Finished building: $<'
	@echo ' '
---------------


i.e. src/foo/subdir.mk includes
---------------
src/foo/%.o: ../src/foo/%.cpp
	@echo 'Building file: $<'
	@echo 'Invoking: GCC C++ Compiler'
	g++ -DSHOULD_APPEAR -O0 -g3 -Wall -c -fmessage-length=0 -MMD -MP -MF"$(@:%.o=%.d)" -MT"$(@:%.o=%.d)" -o"$@" "$<"
	@echo 'Finished building: $<'
	@echo ' '
---------------


The problem arrises due to the order in which items appear in the include list in the main makefile.  This order appears to be the result of an alphabetical sort on the actual source files' full paths.

This can lead to the condition shown above where a sub-directory's subdir.mk gets included AFTER its parent.  When this happens, make will use the wrong build rule for the subdirectories %.o target.  In the example above, when make looks for a rule to build src/foo/bar.cpp, the first match it encounters is src/%.o: ../src/%.cpp.  Thus, SHOULD_APPEAR does not get defined for bar.cpp.
This problem will occur for any build settings that are applied in this manner.

This problem can be solved by sorting the include list reverse-alphabetically before appending the "subdir.mk".  This will ensure that subdirectories are listed before parent directories.

I will attach a "fix" that I have tested and seems to work for my specific situation.  note: I'm not usually Java programmer, haven't done any development on Eclipse and suspect that a better/more elegant solution can be found.

Mark


Reproducible: Always

Steps to Reproduce:
1. Create new C++ project / Hello World project (Project Name: thebug)
2. Within the src folder, create a new folder called foo.
3. Within the folder foo, create a new source file called bar.cpp
4. Within project explorer, right-click on the folder foo and select properties
5. Under C/C++ Build --> Settings, Tool Settings, GCC C++ Compiler, Preprocessor: Define the symbol SHOULD_APPEAR.
6. Build the project

from make output:
Building file: ../src/foo/bar.cpp
Invoking: GCC C++ Compiler
g++ -O0 -g3 -Wall -c -fmessage-length=0 -MMD -MP -MF"src/foo/bar.d" -MT"src/foo/bar.d" -o"src/foo/bar.o" "../src/foo/bar.cpp"
Finished building: ../src/foo/bar.cpp

The expected output is:
Building file: ../src/foo/bar.cpp
Invoking: GCC C++ Compiler
g++ -DSHOULD_APPEAR -O0 -g3 -Wall -c -fmessage-length=0 -MMD -MP -MF"src/foo/bar.d" -MT"src/foo/bar.d" -o"src/foo/bar.o" "../src/foo/bar.cpp"
Finished building: ../src/foo/bar.cpp

Comment 1 MJ

2010-05-13 15:34:32 EDT

Created attachment 168458 [details]
Example project showing bug

Comment 2 MJ

2010-05-13 15:39:05 EDT

Created attachment 168459 [details]
GnuMakefileGenerate.java with fix and comments

My "Fix" starts at line 1242.

Comment 3 Elena Laskavaia

2010-05-13 15:44:34 EDT

(In reply to comment #2)
> Created an attachment (id=168459) [details]
> GnuMakefileGenerate.java with fix and comments
> 
> My "Fix" starts at line 1242.

It is suppose to be a patch? Is so please attach a patch,
see http://wiki.eclipse.org/CDT/contributing

Comment 4 MJ

2010-05-13 17:49:18 EDT

> It is suppose to be a patch? Is so please attach a patch,
> see http://wiki.eclipse.org/CDT/contributing

Its not really supposed to be a patch.  The attachment is more my attempt at working around the issue so that my dev team could continue to do work w/o renaming key files in our project.

I'm not really that familiar with Eclipse plug-in development (or Java, for that matter), but I did manage to cobble together a solution a little while back when we first ran into the problem (circa CDT 6.0.0).  I figured that my "fix" could be used more as a guide by someone that knew what they were doing.

That said, I did just attempt to follow the instructions to generate a patch.  I'm getting a lot of timeout errors trying to connect to the CVS repository, so I can't really get the full source in order to generate a patch.  At first I couldn't connect at all, either via pserver or proxy.

I can provide a diff of the individual file, if that would be more helpful.  Sorry that I'm not more familiar with the whole process.

Mark

Comment 5 Elena Laskavaia

2010-05-13 21:08:49 EDT

Patch is a diff in a special patch form. You can generate it from eclipse, see
wiki page I mentioned above

Comment 6 MJ

2010-05-13 22:33:22 EDT

I understand what a patch file is... I simply don't have the full CDT project to generate a patch from - thats why I offered to send a diff.  I don't have the source for CDT and have had trouble connecting to the repository.

I created my fix based on code for GnuMakefileGenerator.java that I found on some other site (don't remember where) that I found while searching for information on the problem.  I downloaded, modified the Java file, played around with an eclipse Java project until I got a it to generate a class file... then took the class, and put it in the appropriate jar (involved decompressing the jar from the plugins folder and then re-compressing it).

Like I said, I'm not familiar with Eclipse plug-in development and not familiar with Java.  I knew enough about coding in general to put together a hack to fix a specific problem.  My "fix" is very small and not really worth spending a few hours trying to set myself up with the tools necessary to check out all of CDT. I agree a patch file would be ideal, but I'm just not the right person to generate it. Anyways, here is the <10 line change.

I replaced this section of code:
---------------- ~line 1242.. If the file has changed, just search for "-include"
while(subDirIterator.hasNext())
{
	IContainer subDir = (IContainer)subDirIterator.next();
	IPath projectRelativePath = subDir.getProjectRelativePath();
			
	if(!projectRelativePath.toString().equals("")) //$NON-NLS-1$
		buffer.append("-include " + escapeWhitespaces(projectRelativePath.toString()) + SEPARATOR + "subdir.mk"+ NEWLINE); //$NON-NLS-1$ //$NON-NLS-2$
}
----------------


with this:
----------------
Vector codeDirs = new Vector();
while(subDirIterator.hasNext())
{
	IContainer subDir = (IContainer)subDirIterator.next();
	IPath projectRelativePath = subDir.getProjectRelativePath();
	if(!projectRelativePath.toString().equals("")) //$NON-NLS-1$
		codeDirs.add(projectRelativePath.toString());
}
Comparator comparator = Collections.reverseOrder();
Collections.sort(codeDirs,comparator);

Iterator subDirStringIterator = codeDirs.iterator();
while(subDirStringIterator.hasNext())
{
	buffer.append("-include " + escapeWhitespaces( (String) subDirStringIterator.next() ) + SEPARATOR + "subdir.mk"+ NEWLINE); //$NON-NLS-1$ //$NON-NLS-2$
}
----------------

I couldn't have written this code from scratch. It seemed reasonable to assume that the problem would be solved by sorting the subdirs prior to appending the "include" line to the "buffer". I simply rearranged the existing code and added a few lines to allow that to happen.  Because I did it all with local vars, I can't see how this would affect anything else... However, because I'm not familiar with this code, I can't make any warranties other than saying that it worked for me.

I would really appreciate it if you (or someone else) could generate the patch file (if it is needed as part of the bug-fix process) as I'm sure that someone could do that in less time than it took me to type all this up.

Thanks,
Mark

Comment 7 MJ

2010-08-25 21:59:23 EDT

Created attachment 177488 [details]
Patch for org.eclipse.cdt.managedbuilder.makegen.gnu.GnuMakefileGenerator.java

Code change creates a reverse-alphabetical vector of the subdirectory text.  The vector is used only to create the subdir.mk include list.  Having the list in reverse alphabetical order guarantees that parent directories are listed after child directories.  This, in turn, allows the rules applied to subdirectories to be evaluated for a match prior to rules applied to parent directories.

I tested this patch with Helios.

Comment 8 MJ

2011-04-17 20:50:02 EDT

This bug is still present in CDT 8.0 nightly build I201104150807.  I would appreciate it if someone on the dev team could apply the patch I attached last August... or at least provide some feedback if the patch is no longer applicable.  

Thank you,

Mark

Comment 9 MJ

2011-07-01 18:12:19 EDT

Created attachment 198988 [details]
Patch for CDT 8.0

Updated patch to latest CDT 8.0.

Comment 10 Andrew Gvozdev

2011-07-02 00:54:36 EDT

Created attachment 198991 [details]
modified patch

You made a patch against your own version in git, not from CDT version, it does not apply. However I looked at the changes and they look good to me. I slightly modified the code, please doublecheck.

Committed on master and CDT 8.0. Thanks for the patch.

Comment 11 Andrew Gvozdev

2011-07-02 00:57:07 EDT

Well not 8.0 but CDT 8.0.1. Closing the bug.

Comment 12 Andrew Gvozdev

2011-07-02 02:34:20 EDT

Created attachment 198993 [details]
Additional patch

Restored reverse sorting plus unit tests adjusted

Comment 13 MJ

2011-07-05 13:00:31 EDT

The patch update looks good. I tested out the version in CDT master and the bug is fixed. Thanks so much!