Indexing source files when multiple toolchains are used. #157
Conversation
|
About the code cleanliness check: I ran the standard Source > Format menu on the code, using the Eclipse built-in formatter profile. |
|
@ewaterlander : You need to run |
|
The JDT's Save Actions should have generated the correct format for the code. However over time as JDT bugs are fixed we need to update the JDT on the build machine to make sure they match as there are slight formatting changes. Also, some refactorings skip the save actions, if that refactoring changes the length of an identifier it leaves badly formatted code behind. I am more than happy to accept improvements in this area. One idea that I had was to simply commit the change back to the PR rather than fail the code cleanliness check. This is easier now that we are on GitHub. |
PS - the CDT project does not use the built-in formatter profile, there is a CDT profile. But on closer inspection I see that there is also trailing whitespace on some lines. The save actions are supposed to clean that up, but sometimes they don't as there seems to be a conflict between JDT removing trailing spaces and the formatter adding trailing spaces in javadoc comments. |
|
Anyway, don't worry about the formatting, I can fix that up before merge this time once I complete the rest of the review. |
There was a problem hiding this comment.
I need more time to mull this over. It is much better than the #130 as you have solved the problem of leaving things in a bad state, so I am inclined to merge it. Leave it with me for a day or two.
|
I cannot push my formatting updated because (I think) you have disabled "Allow edits and access to secrets by maintainers" - which is fine as there are security considerations around it - but it means you'll have to do the formatting fix before I can merge. The checkbox is on the right side column: |
This change solves the indexing of C/C++ files when multiple toolchains are used in a single Makefile. This is for the use case in which one (Linux) gcc compiler plus one or more custom embedded C compilers (all producing ELF format binaries) are used. To get proper indexing we need to know for each resource which toolchain was used. The sub build configuration (via extension point org.eclipse.cdt.core.buildConfigProvider) extends StandardBuildConfiguration.java, and overrides method IToolChain (List<String> commandgetToolChain). tcMap is filled with a map of toolchains per resource. The primary toolchain keeps pointing to the gcc toolchain.
ewaterlander
force-pushed
the
core_build_make_toolchains
branch
from
bfb1e83
to
a28dfce
Compare
|
Indeed, I did not check that box. I have fixed the formatting now. |
|
OT:
This is ugly stuff. Especially as you probably want to run it using |
I also allow maintainers edits now. |
No, my plan would be to add a new commit automatically, in the same way I would have done manually if "Allow edits..." was enabled. Let's continue this discussion in #166 |
There was a problem hiding this comment.
I need more time to mull this over. It is much better than the #130 as you have solved the problem of leaving things in a bad state, so I am inclined to merge it. Leave it with me for a day or two.
Approved. I am still a little unsure about this, but it does address your use case and it doesn't have the problems I was concerned about in #130. So once build gives it +1 I can merge this.
|
Do not merge it yet. |
The hang was not caused by my patch. I had it too in the unmodified version. After rebuilding my stuff I did not see it anymore. |
This change can be merged. |
If you are able to, please provide a bug report on this. Ideally including the output of jstack which will hopefully show any deadlocks |
OK, I will. I do not have experience with jstack. Do you have some hints? |
|
jstack should be included with your JDK, it is a command line program that you pass the PID to to get the stack traces of all running threads, and it is able to detect some kinds of deadlocks. |
This change solves the indexing of C/C++ files when multiple toolchains are used in a single Makefile. This is for the use case in which one (Linux) gcc compiler plus one or more custom embedded C compilers (all producing ELF format binaries) are used.
To get proper indexing we need to know for each resource which toolchain was used. The sub build configuration (via extension point org.eclipse.cdt.core.buildConfigProvider) extends
StandardBuildConfiguration.java, and overrides method IToolChain (List commandgetToolChain). tcMap is filled with a map of toolchains per resource. The primary toolchain keeps pointing to the gcc toolchain.