[GRADLE-525] Enhancement to support debugging Created: 17/Jun/09 Updated: 31/May/13 Resolved: 28/Aug/09 |
|
Status: | Resolved |
Project: | Gradle |
Affects Version/s: | 0.6 |
Fix Version/s: | 0.8 |
Type: | Improvement | ||
Reporter: | John Murph | Assignee: | Adam Murdoch |
Resolution: | Fixed | Votes: | 0 |
Attachments: | debug.patch |
Description |
Because of Gradle's multiproject handling, there can be many scripts with the same name in a Gradle build. A Java debugger needs to know which source file goes with a given runtime class. I have discussed this with Intellij, and they agreed that adding a static field to the class that points to the source file would work. I have already implemented this, and written an IDEA plugin that allows me to debug Gradle scripts using this information. I am curious as to the best way to submit this update. Approximately 15 files were changed, so it might be best to submit this using Git. Please let me know. |
Comments |
Comment by John Murph [ 19/Jun/09 ] |
Here is a Subversion patch that contains the changes. A few notes about this patch. 1) To add a field to the generated class, I had to use the AST stuff in Groovy. To do this, I changed the transformer support from a single transformer at a time to a list of transformers. I think this makes that whole framework more flexible. 2) If the source file and caches are moved together, and the source in not modified, we still must regenerate the class (now) because otherwise the new field will not have the correct value. To do this, I've added a new properties to the cache to track the souce file location. This way, the cache can be invalidated even if it's just moved on disk. 3) I added a DebugHelper class to the org.gradle package. The idea behind this class is to isolate tools like IDEA from internal changes to Gradle. This allows Gradle to restructure code, or even reimplement this support and have debugging continue to work in such tools. |
Comment by John Murph [ 19/Jun/09 ] |
A coworker who uses Linux pointed out a problem with my patch. gradle.bat and the gradle shell script set a system property called "tools.jar" that points to the tools.jar in the JDK/lib directory. So, my change to build.gradle uses that system property to add tools.jar to the compile classpath for Gradle. It turns out, however, that the gradlew shell script does not correctly set this property (although that gradlew.bat file does). This will need to be fixed before my patch will compile. Sorry about this. |
Comment by John Murph [ 10/Jul/09 ] |
I have updated my changes to be compatible with the (nearly) most recent trunk. These changes are available via Git at git://github.com/sappling/gradle.git in the debug branch. This will hopefully make the changes easier to understand/integrate. One note, I noticed that my original changes caused tools.jar to be in the lib directory of a Gradle distribution (I added tools.jar to the compile configuration). This is not what we want, however, so this new branch now modifies the build script to not copy tools.jar. However, I did this in a hackish way via testing for a jar file with the name "tools.jar". We really need a better way to indicate libraries that will be provided by the runtime system. It would be nice if the Java plugin provided a "provided" configuration for this purpose. |
Comment by Adam Murdoch [ 20/Jul/09 ] |
I have some conerns about this patch:
I would prefer an API something like: String getScriptClassName(File scriptFile) These should be reachable from the Gradle class, I think, and non-static. For implementation, a really simple option is to encode the script file location in the script class name. |
Comment by John Murph [ 20/Jul/09 ] |
On Mon, Jul 20, 2009 at 5:06 PM, Adam Murdoch (JIRA) <jira@codehaus.org> wrote: I have some conerns about this patch:
I would prefer an API something like: String getScriptClassName(File scriptFile) I was trying to isolate the IDE plugins from knowing anything about how this is implemented in Gradle. If we use the "static field" approach that I decided to use, then the suggested APIs will not work. On the other hand, the APIs I submitted would work if we decided to implement it via a "class name mangling" approach. These should be reachable from the Gradle class, I think, and non-static. I put these in a separate class to isolate them from normal Gradle. I thought this important since these APIs are designed to be used via reflection. That's also why I made them static, simply easier to use via reflection. We need reflection so that the IDE user can decide at runtime which version of Gradle to use. That said, I think our GUI work will offer a better non-reflection (mostly) way of achieving this. So, for now I don't much care. For implementation, a really simple option is to encode the script file location in the script class name. Peter Gromov (from IntelliJ) and I discussed that option, but felt that encoding the information into a static field was a better solution. It avoids issues where the mangled path (to make a valid class name) does not unmangle to a unique place on the hard drive, but that doesn't seem very likely. I don't really know what issues Peter might have had with the idea (it was his idea), but he felt that the field was better. I don't really feel strongly about any solution, except that it cannot require that Gradle be "executed" in any normal sense. That's the other reason I made the DebugHelper methods static, so that it was clear they would be executed outside the normal runtime enviroment of Gradle (I hope that statement makes sense). – |
Comment by John Murph [ 20/Jul/09 ] |
So, the last comment was not well formatted, and was therefore confusing. Let me try it differently: >>On Mon, Jul 20, 2009 at 5:06 PM, Adam Murdoch (JIRA) <jira@codehaus.org> wrote: I was trying to isolate the IDE plugins from knowing anything about how this is implemented in Gradle. If we use the "static field" approach that I decided to use, then the suggested APIs will not work. On the other hand, the APIs I submitted would work if we decided to implement it via a "class name mangling" approach. >> These should be reachable from the Gradle class, I think, and non-static. I put these in a separate class to isolate them from normal Gradle. I thought this important since these APIs are designed to be used via reflection. That's also why I made them static, simply easier to use via reflection. We need reflection so that the IDE user can decide at runtime which version of Gradle to use. That said, I think our GUI work will offer a better non-reflection (mostly) way of achieving this. So, for now I don't much care. >> For implementation, a really simple option is to encode the script file location in the script class name. Peter Gromov (from IntelliJ) and I discussed that option, but felt that encoding the information into a static field was a better solution. It avoids issues where the mangled path (to make a valid class name) does not unmangle to a unique place on the hard drive, but that doesn't seem very likely. I don't really know what issues Peter might have had with the idea (it was his idea), but he felt that the field was better. I don't really feel strongly about any solution, except that it cannot require that Gradle be "executed" in any normal sense. That's the other reason I made the DebugHelper methods static, so that it was clear they would be executed outside the normal runtime enviroment of Gradle (I hope that statement makes sense). |
Comment by Peter Gromov [ 21/Jul/09 ] |
I'm here . Just to clarify. The issues I see with encoding path in a class name are the unmangling one (which John mentioned) and that it's not evident where to place the root from which the path starts. It can be anywhere in file system and IDE should quickly determine it and then unmangle the path further. Therefore I agree that placing some raw String in class is easier to manage. Maybe not in a static field, maybe in a runtime annotation. There could be also one special class at runtime keeping all the debug information about the all the scripts so that their classes remain untouched. |
Comment by John Murph [ 30/Jul/09 ] |
As per our offline discussions, I've reimplemented this. The new code is also available via Git at git://github.com/sappling/gradle.git in the debug_v2 branch. This implementation avoids several of the drawbacks from the first attempt. In particular, it does not use the Groovy AST stuff, the caches do not need to be maintained if the files are moved around on disk, and tools.jar is no longer needed to build Gradle. The new approach gives each build script class a unique name and stores a mapping file in the .gradle cache directory (of the project root) whenever a build script class is compiled. This mapping file allows us to find the source file for a given class name. The unique name is computed via a MD5 hash of the source location, so it should be fairly unique, and is easily recomputed. So, for a given script file it is easy to determine the unique class name that will be assigned. The DebugHelper concept was preserved, although it now needs to know the project root directory so that it can find the mapping file. Otherwise, the APIs are very similar to Adam's suggestion. The main drawback to this approach is that the mapping file may get out of sync with the execution environment somehow. I think this is unlikely unless the user interacts with the cache (deleting it, for example) while Gradle is running. In this case, it should repair itself when Gradle is next run. |
Comment by Steve Appling [ 30/Jul/09 ] |
Applied patch provided by John Murph. |
Comment by Adam Murdoch [ 20/Aug/09 ] |
Attempt number 2. |
Comment by Adam Murdoch [ 28/Aug/09 ] |
Applied. Thank you for the patches. |
Comment by Luke Daley [ 31/May/13 ] |
I can't get this to work with 1.6. I started Gradle in debug mode, connected via the remote debugger in IDEA and tried to set a break point in a script… no dice. Execution rolls on past it. I can debug classes fine though of course. Should it still work or did we intentionally remove this capability? The original description mentions an IDEA plugin. Is that required? If so, is it available anywhere? |