Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 312835 - CDT build settings which are set at the folder level are ignored in certain situations
Summary: CDT build settings which are set at the folder level are ignored in certain s...
Status: RESOLVED FIXED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-build-managed (show other bugs)
Version: 8.0   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 8.0.1   Edit
Assignee: Andrew Gvozdev CLA
QA Contact: Chris Recoskie CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-13 15:29 EDT by MJ CLA
Modified: 2011-07-05 13:00 EDT (History)
1 user (show)

See Also:


Attachments
Example project showing bug (68.19 KB, application/zip)
2010-05-13 15:34 EDT, MJ CLA
no flags Details
GnuMakefileGenerate.java with fix and comments (173.01 KB, text/x-java)
2010-05-13 15:39 EDT, MJ CLA
angvoz.dev: iplog-
Details
Patch for org.eclipse.cdt.managedbuilder.makegen.gnu.GnuMakefileGenerator.java (1.77 KB, patch)
2010-08-25 21:59 EDT, MJ CLA
angvoz.dev: iplog-
Details | Diff
Patch for CDT 8.0 (1.45 KB, patch)
2011-07-01 18:12 EDT, MJ CLA
angvoz.dev: iplog+
Details | Diff
modified patch (2.03 KB, patch)
2011-07-02 00:54 EDT, Andrew Gvozdev CLA
angvoz.dev: iplog-
Details | Diff
Additional patch (11.43 KB, patch)
2011-07-02 02:34 EDT, Andrew Gvozdev CLA
angvoz.dev: iplog-
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description MJ CLA 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 CLA 2010-05-13 15:34:32 EDT
Created attachment 168458 [details]
Example project showing bug
Comment 2 MJ CLA 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 CLA 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 CLA 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 CLA 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 CLA 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 CLA 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 CLA 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 CLA 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 CLA 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 CLA 2011-07-02 00:57:07 EDT
Well not 8.0 but CDT 8.0.1. Closing the bug.
Comment 12 Andrew Gvozdev CLA 2011-07-02 02:34:20 EDT
Created attachment 198993 [details]
Additional patch

Restored reverse sorting plus unit tests adjusted
Comment 13 MJ CLA 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!