Community
Participate
Working Groups
This looks the result of fix made in bug 416911. http://download.eclipse.org/eclipse/downloads/drops4/N20131016-2000/testresults/html/org.eclipse.jdt.compiler.apt.tests_win32.win32.x86_7.0.html And here: http://download.eclipse.org/eclipse/downloads/drops4/N20131016-2000/testresults/html/org.eclipse.jdt.apt.tests_win32.win32.x86_7.0.html
Michael, can you look at the failures please? Thanks!
Ok, it seems like those test do actually require the lib/apttestprocessors.jar to work. As I didn't remove the file in the last commit, it was still working in PDE and with Surefire. I'll restore creation of this jar during build. As it doesn't contain test to run and is only a resource, it shouldn't prevent surefire from running.
This patch should help: https://git.eclipse.org/r/17525
The apttestprocessors needs to be present under org.eclipse.jdt.compiler.apt.tests/lib/ as it's not created during a regular build. All tests are(In reply to Mickael Istria from comment #3) > This patch should help: https://git.eclipse.org/r/17525 Tests are still failing. Are you sure the jar needs to be removed? The tests expects the JAR to be present already. I don't think they get created on the fly.
(In reply to Jayaprakash Arthanareeswaran from comment #4) > The apttestprocessors needs to be present under > org.eclipse.jdt.compiler.apt.tests/lib/ as it's not created during a regular > build. All tests are(In reply to Mickael Istria from comment #3) > > This patch should help: https://git.eclipse.org/r/17525 > > Tests are still failing. Are you sure the jar needs to be removed? The tests > expects the JAR to be present already. I don't think they get created on the > fly. Ignore the garbled comments. Just the last two lines matter.
I just found it cleaner as I thought PDE/Build was able to honor the "jars.compile.order" property in IDE so it would recreate the jar when necessary. Apparently it's not, you should open an enhancement request to make IDE more consistent with build. I pushed a new patch set on the same Gerrit contributions, which keeps this jar in repo.
It seems like maybe there is some confusion about the purpose of this jar? IIRC, the functionality being tested is that in a running IDE, it is possible to load annotation processors from a jar file. So, the apttestprocessors jar is input to the test IDE, and it is discovered at runtime of the test IDE. But it is important that it is actually a jar file per se. If you simply replaced it with class files you would be testing different functionality. The jar file becomes, effectively, a (non-OSGi) plug-in to the compiler. Note that this means it cannot necessarily be unloaded, on some platforms, until the IDE terminates. This is the source of much of the seeming complexity of the APT tests.
(In reply to Mickael Istria from comment #6) > I just found it cleaner as I thought PDE/Build was able to honor the > "jars.compile.order" property in IDE so it would recreate the jar when > necessary. Apparently it's not, you should open an enhancement request to > make IDE more consistent with build. > > I pushed a new patch set on the same Gerrit contributions, which keeps this > jar in repo. I think our maven/Tycho build might honor some form of "output.<library>=" (See PDE help). But, jar order just specifies order to build jars in, after something else says to build them ... I believe. But, just "having in repo" isn't all bad, if it is just "data" to be used by the test ... but would be good to have a file there with it, say 'description.txt', that describes how to recreate it if/when that is ever necessary.
(In reply to David Williams from comment #8) > But, just "having in repo" isn't all bad, if it is just "data" to be used by > the test ... but would be good to have a file there with it, say > 'description.txt', that describes how to recreate it if/when that is ever > necessary. There is this file: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/tree/org.eclipse.jdt.compiler.apt.tests/apttestprocessors.jardesc > But, jar order just specifies order to build jars in, after something else > says to build them ... I believe. Yes, that's what I think too. So jar order only works for PDE plugin export, but not in incremental build. It seems a bit inconsistent to me. Do you agree supporting this in incremental build would make a good enhancement request for PDE ?
(In reply to Mickael Istria from comment #9) > (In reply to David Williams from comment #8) > > But, just "having in repo" isn't all bad, if it is just "data" to be used by > > the test ... but would be good to have a file there with it, say > > 'description.txt', that describes how to recreate it if/when that is ever > > necessary. > > There is this file: > http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/tree/org.eclipse.jdt. > compiler.apt.tests/apttestprocessors.jardesc > > > But, jar order just specifies order to build jars in, after something else > > says to build them ... I believe. > > Yes, that's what I think too. So jar order only works for PDE plugin export, > but not in incremental build. It seems a bit inconsistent to me. Do you > agree supporting this in incremental build would make a good enhancement > request for PDE ? I think I see what you mean now. It wouldn't hurt to enter an enhancement ... though, realistically, PDE isn't getting a lot of priority right at this point in time ... but ... perhaps someone will give it priority, and time, in future, so doesn't hurt to have it logged (though, fix might be in JDT ... I'm not sure who is "in charge" of incremental builds).
(In reply to David Williams from comment #10) > I think I see what you mean now. It wouldn't hurt to enter an enhancement > ... though, realistically, PDE isn't getting a lot of priority right at this > point in time ... but ... perhaps someone will give it priority, and time, > in future, so doesn't hurt to have it logged (though, fix might be in JDT > ... I'm not sure who is "in charge" of incremental builds). I guess this is the build script in PDE in this case. The generic (incremental) build infrastructure comes form Platform (not JDT).
In any case, I am planning to release the change to retain the JAR in the repo/project for tonight's I build. Any objections?
(In reply to Jayaprakash Arthanareeswaran from comment #12) > In any case, I am planning to release the change to retain the JAR in the > repo/project for tonight's I build. Any objections? No objections ... but ... I think that /lib was left in the .gitignore file, so if we do "git clean -f -d -x" (as we do, during builds) the "ignored" lib folder is removed which is not what we want in this case. Right?
(In reply to David Williams from comment #13) > (In reply to Jayaprakash Arthanareeswaran from comment #12) > > In any case, I am planning to release the change to retain the JAR in the > > repo/project for tonight's I build. Any objections? > > No objections ... but ... I think that /lib was left in the .gitignore file, > so if we do "git clean -f -d -x" (as we do, during builds) the "ignored" lib > folder is removed which is not what we want in this case. Right? Or, if I'm confused about which jar you want left in repo, and you really do want to ignore 'lib' folder, then you need to delete files and lib folder first, from git repo, and then add to .gitignore.
(In reply to David Williams from comment #13) > No objections ... but ... I think that /lib was left in the .gitignore file, > so if we do "git clean -f -d -x" (as we do, during builds) the "ignored" lib > folder is removed which is not what we want in this case. Right? David, can you please confirm this is why the build failed? Do you recommend a revert (for the time being) or fixing the .gitignore?
Have you tried to run "mvn clean verify -Pbuild-individual-bundles" locally? If it fails, then it will fail on CI. Also, Hudson did vote -1 on the patch for the same reason. The cause is the change I made to the maven-clean-plugin configuration, which alters the tree before running other Tycho plugins. A solution is either to remove this specific configuration from pom.xml.
(In reply to Mickael Istria from comment #16) > Have you tried to run "mvn clean verify -Pbuild-individual-bundles" locally? > If it fails, then it will fail on CI. Also, Hudson did vote -1 on the patch > for the same reason. > The cause is the change I made to the maven-clean-plugin configuration, > which alters the tree before running other Tycho plugins. A solution is > either to remove this specific configuration from pom.xml. If you didn't notice, I did wonder about the hudson's vote. But wasn't sure. Anyway, not being a build expert and not able to weigh in the options we have, I am beginning to think we should just revert changes made here as well as bug 416911. I.e. until we come up with a proper fix to get rid of nested jars. Dani, what do you think?
(In reply to Jayaprakash Arthanareeswaran from comment #17) > (In reply to Mickael Istria from comment #16) > > Have you tried to run "mvn clean verify -Pbuild-individual-bundles" locally? > > If it fails, then it will fail on CI. Also, Hudson did vote -1 on the patch > > for the same reason. > > The cause is the change I made to the maven-clean-plugin configuration, > > which alters the tree before running other Tycho plugins. A solution is > > either to remove this specific configuration from pom.xml. > > If you didn't notice, I did wonder about the hudson's vote. But wasn't sure. > > Anyway, not being a build expert and not able to weigh in the options we > have, I am beginning to think we should just revert changes made here as > well as bug 416911. I.e. until we come up with a proper fix to get rid of > nested jars. > > Dani, what do you think? I'm OK with that. Obviously the patch wasn't complete (yet).
See http://dev.eclipse.org/mhonarc/lists/platform-releng-dev/msg21628.html to understand what an Hudson -1 mean. Hudson -1 mean "don't push that change, it breaks build". You don't have to be a build expert to check a contribution "mvn clean verify -Pbuild-individual-bundles" before a push, it's just a matter of habit. In comment #16, I give a piece of solution to fix that. It's more useful to fix rather to revert a change. I turned it into a Gerrit patch: https://git.eclipse.org/r/17652
Instead of reverting, just wait for Hudson to vote +1 on the patch I submitted, and then try to run test locally from to check whether it works or not.
(In reply to Mickael Istria from comment #19) > See http://dev.eclipse.org/mhonarc/lists/platform-releng-dev/msg21628.html > to understand what an Hudson -1 mean. Hudson -1 mean "don't push that > change, it breaks build". Thanks for that, but a couple of days ago wouldn't have been too bad either. However, in future if you insist on using Gerrit, you need to respond to comments (assuming you did see my comment) on a contribution in a better way. By the way, how much time does the Hudson job take? We don't want to block other teams from having a good I build.
(In reply to Mickael Istria from comment #19) > In comment #16, I give a piece of solution to fix that. It's more useful to > fix rather to revert a change. I turned it into a Gerrit patch: > https://git.eclipse.org/r/17652 David, may I request you to take a look at the posted fix and see if you think it's safe? TIA!
(In reply to Jayaprakash Arthanareeswaran from comment #22) > (In reply to Mickael Istria from comment #19) > > In comment #16, I give a piece of solution to fix that. It's more useful to > > fix rather to revert a change. I turned it into a Gerrit patch: > > https://git.eclipse.org/r/17652 > > David, may I request you to take a look at the posted fix and see if you > think it's safe? TIA! It is hard for me to tell what it is trying to accomplish ... but appears to be trying to clean the working tree? Not sure I understand why that'd be necessary (or, a good idea). [Apologies if this was explained elsewhere ... but not at my main computer and trying to give a quick response.]
I have reverted the changes for the time being, i.e. until we can be sure that the fix is enough. I have reopened bug 416911 to address the original fix.
Another way to get rid of this jar would be to move all the apt processors into their own test bundle. In that case, they would still be inside a jar file and that bundle can be referenced from the apt test bundles as being the location to retrieve the apt processors. Just my 2 cents.
(In reply to Olivier Thomann from comment #25) > Another way to get rid of this jar would be to move all the apt processors > into their own test bundle. > In that case, they would still be inside a jar file and that bundle can be > referenced from the apt test bundles as being the location to retrieve the > apt processors. > Just my 2 cents. I think we'd concluded there's no need to get rid of the jar. That is is "pre-built" and is actually "part of the test data" (i.e. does not run the tests, but the tests make sure it (still) works correctly) ... I think just have it check into to git (and remove /lib from .gitignore) is all the fix that's needed. I think part of the confusion was whether or not this jar should be re-built "during a build". While its nice to have the ability to do so (for the few times it might be needed) ... I don't think that's normally desired, since the test (I assume) is to make sure APT processor still works with those provided classes, even if something else about "apt processing" changes. Or ... am I confusing this bug with another? Just my 1 cent (compared to Olivier's knowledge :)
The only thing that needs to be done from the dev point of view is to rebuild (manually) the jar each time an apt processor is modified or added. There is a file to do that in that project. So it takes no time to do it. And yes, as far as I know, the jar doesn't need to be rebuilt at build time.
(In reply to Jayaprakash Arthanareeswaran from comment #21) > By the way, how much time does the Hudson job take? We don't want to block > other teams from having a good I build. Hudson jobs are running separately from current "main" builds of platform, so there is no way a Gerrit change can block an I-Build.
(In reply to Mickael Istria from comment #28) > Hudson jobs are running separately from current "main" builds of platform, > so there is no way a Gerrit change can block an I-Build. What I really meant was I was going to wait for the Hudson job before releasing into master. Anyway, finally, the tests ran fine with the latest I build. Verified for 4.4 M3 with buil I20131029-2000.
