Community
Participate
Working Groups
Assume that you have project configured like this: +src1 -> output to src1 + A.java +src2 -> output to src1 + B.java + c.txt c.txt is not copied into the src1. If you configure output to bin, you will receive inside A.class, B.class and c.txt. In the first example, c.txt should be also copied (or it should be impossible to allow for mentioned output path configuration).
This issue is very important if you use Eclipse based software which generates java & (helper) ser files from sqlj. If such files are generated to separate source folder, ser files are not copied and are missing from classpath.
This bug has been reported before (see bug 114778) but got marked as WORKSFORME. A fix would have to throw and exception or create an error marker in case of collisions.
As mentioned in bug 114770, this is not a setup we can support IF extra resources need to be copied from one source folder to another. Will look into creating a problem marker on the project that will inform the user that a separate output folder is required.
>Will look into creating a problem marker on the project that will inform the >user that a separate output folder is required. That's not what I have in mind. We should allow this scenario and only when there's a conflict when copying the resources we create the marker (or as another option we fail during build). If I understood correctly the suggested workaround does not work for the reporter of the problem.
Dani, think about what you're asking. before 1st build: src1(is also bin) X.java src2 Y.java x.txt after 1st build: src1(is also bin) X.java X.class Y.class x.txt src2 Y.java x.txt Now change src2/x.txt How are we to tell that src1/x.txt was a copy and not created by the user ? The 2nd build will ALWAYS report a collision case.
>How are we to tell that src1/x.txt was a copy and not created by the user ? Mark it as derived.
I am not sure if this bug should be fixed by copying those non-java files. This may affect all people who rely on existing behavior. I'd rather disallow for such configuration.
>I'd rather disallow for such configuration. Doesn't that contradict your comment 1: "This issue is very important if you use Eclipse based software"?
Yes, It does. "Eclipse based software" developers agree that fixing this bug by copying reasorces is dangerous, and therefore original issue will be documented and workaround from bug 114770 applied. But even in this situation the issue suprises the user and therefore should be solved in one or another way.
>Yes, It does. "Eclipse based software" developers agree that fixing this bug by >copying reasorces is dangerous Who are they? Where is this documented? Don't get me wrong: I'm OK with just dropping this and warn about the wrong setup instead but if we can offer a working solution based on the derived state then that would even be better.
Created attachment 146612 [details] NonJavaCopied_v1.0 This simple patch ensures that all non-java files will be properly copied, cleaned and rebuilt during all operation on both: independent and same as source output folder. Copied non-java files are marked as "derived".
Created attachment 146654 [details] NonJavaCopiedWithTests_v1.0 New, better patch added. I've analyzed conflicts problem in details and made some new changes. Patch includes also two test cases for OutputFolderTests class. If resource of the same name as copied resource already exists in the output folder, and is not derived, then the source file will not be copied and will be given a warning about not being copied into output folder.
After some more thinking about this, I conclude that we should not change the existing copy behavior as it could break clients that rely on the existing behavior. Using the derived state can also be problematic because the user (or a tool) can change that state at will. Instead of flagging such setups with a problem on the project we should report the problem on the file(s) that aren't copied. That way we still tolerate such setups if the source folders don't contain any non-Java resources.
(In reply to comment #3) > Will look into creating a problem marker on the project that will inform the > user that a separate output folder is required. (In reply to comment #13) > Instead of flagging such setups with a problem on the project we should report > the problem on the file(s) that aren't copied. That way we still tolerate such > setups if the source folders don't contain any non-Java resources. If there are hundreds of non-Java resources in a single project, then doesn't it seem to be excessive to add a separate marker for each file? The problem seems to be more in project setup not in file type, so one additional problem marker sounds like a good trade off to me. Either way, would it be possible to add this bug to JDT 3.8 plan?
(In reply to comment #14) > (In reply to comment #3) > > Will look into creating a problem marker on the project that will inform the > > user that a separate output folder is required. > > (In reply to comment #13) > > Instead of flagging such setups with a problem on the project we should report > > the problem on the file(s) that aren't copied. That way we still tolerate such > > setups if the source folders don't contain any non-Java resources. > > If there are hundreds of non-Java resources in a single project, then doesn't > it seem to be excessive to add a separate marker for each file? The problem > seems to be more in project setup not in file type, so one additional problem > marker sounds like a good trade off to me. I agree, we don't need markers on each resource and we should not even try to be smart i.e. don't try to allow the setup if there are no resource. Interestingly, we allow the invalid setup by setting the default output folder to a source folder but if one tries to set this up by enabling 'Allow output folders for source folders' then we report an error and don't allow it. This means we already have all the logic. Shouldn't be too hard to fix this.
(In reply to comment #15) > Interestingly, we allow the invalid setup by setting the default output folder > to a source folder but if one tries to set this up by enabling 'Allow output > folders for source folders' then we report an error and don't allow it. This > means we already have all the logic. Shouldn't be too hard to fix this. That is true. It seems that to fix the problem we simply need to toggle line comment in lines 1791 and 1792 of org.eclipse.jdt.internal.core.ClasspathEntry. However, there is also bug 36465 mentioned there that gives some background why it works like that. I do not know what "2.0 backward compatibility" means in this context and how we can fix this not breaking it.
Jay, please follow up. Thanks!
Between bug 33207 and bug 36465, looks like we went back and forth on whether a source entry can be used as the output. We have to make up our mind on this one.
The current behavior makes no sense to me i.e. that via default output folder one can create the dangerous scenario but not when having setting up custom output folders. If at any rate, the dangerous scenario should only be possible when doing the custom setup. Now, it looks like fixing this would break existing clients (see bug 36465) and in fact the setup can be valid if no resource files are present. I suggest we allow the setup via dialog for both scenarios but warn in the build path dialog and after that we put a build path error on the project. This error should be configurable on the 'Building' preference/property page so that those who want/need such a setup can get rid of the error.
(In reply to comment #19) > I suggest we allow the setup via dialog for both scenarios but warn in the > build path dialog and after that we put a build path error on the project. This > error should be configurable on the 'Building' preference/property page so that > those who want/need such a setup can get rid of the error. You meant we warn the user in the dialog regardless of the preference for the build path error, didn't you? Otherwise, that's what I think we should do. I have a patch. Will post it soon with a test.
(In reply to comment #20) > (In reply to comment #19) > > I suggest we allow the setup via dialog for both scenarios but warn in the > > build path dialog and after that we put a build path error on the project. This > > error should be configurable on the 'Building' preference/property page so that > > those who want/need such a setup can get rid of the error. > > You meant we warn the user in the dialog regardless of the preference for the > build path error, didn't you? Otherwise, that's what I think we should do. No, the reporting must match the severity, i.e. - 'Error' ==> show an error at the top of the properties page, 'OK' disabled - 'Warning' ==> show a warning at the top of the properties page, 'OK' enabled - 'Ignore' ==> show an info at the top of the properties page, 'OK' enabled Note that there is a little inconsistency: the dialog which allows to enter the output folder currently shows a warning at the bottom, but when pressing 'OK', the page shows an error. This needs to be kept in sync.
(In reply to comment #21) > Note that there is a little inconsistency: the dialog which allows to enter the > output folder currently shows a warning at the bottom, but when pressing 'OK', > the page shows an error. This needs to be kept in sync. My concern was just warning the user of the consequences somewhere, even if he deliberately chooses to allow such a set-up.
> My concern was just warning the user of the consequences somewhere, even if he > deliberately chooses to allow such a set-up. In that case he will get an info.
Created attachment 202303 [details] Proposed patch This patch introduces a new JavaCore preference and checks for the same before throwing the error/warning. All model tests pass and I shall update the bug once I run all the tests. I don't know if the JDT/UI requires anything else specific for the UI part. Dani, please let me know if there any such requirement.
(In reply to comment #24) > Created attachment 202303 [details] [diff] > Proposed patch > > This patch introduces a new JavaCore preference and checks for the same before > throwing the error/warning. All model tests pass and I shall update the bug > once I run all the tests. > > I don't know if the JDT/UI requires anything else specific for the UI part. > Dani, please let me know if there any such requirement. The option needs to be shown on the Java Compiler > Building page. NOTE: In case we backport this bug, we would not backport the UI portion.
(In reply to comment #24) > I don't know if the JDT/UI requires anything else specific for the UI part. > Dani, please let me know if there any such requirement. Sorry, it wasn't clear. What I wanted to ask was whether the patch is good from a JDT/UI standpoint or if anything is missing.
The patch does not work: it now prevents to setup the project as outlined in comment 0 instead of showing a warning. Also, this bug is not just about specific (per source folder) output locations, hence the constant name and the Javadoc should not use that term. Typo in: CORE_SPECIFIC_OUTPUT_LOCATION_OVERLAPNG_ANOTHER_SOURCE ==> OVERLAPPING Besides that, for easier testing it would be nice to also see the UI portion of this work ;-).
Created attachment 205106 [details] Updated JDT/Core patch This patch addresses issues raised by Dani. One pending problem, though is the user doesn't get the warning message on the buildpath dialog. This is because, on an overlapping output error, I create a problem marker, but do not return the error status back to JDT/UI. Initially I thought I should show some info to the user on the dialog and tried returning the error status and in the BuildPathsBlock#updateBuildPathStatus() tried setting an info/warning status. But for some reason the changes made to the build path are not getting saved. I am going to attach a patch for JDT/UI as well for the new preferences. If we don't think what I mentioned above is not a problem, then these two patches should do. Otherwise, we will have to find a way to let the dialog save the changes despite receiving an error status.
Created attachment 205107 [details] JDT/UI patch This patch to be applied with the previous one
Satyam, would you be able to spare some time to review the attached patches?
Created attachment 205307 [details] Updated patch Sorry, there were issues with the previous patch as it was not sending the right status that JDT/UI was expecting. This patch addresses that issue.
Created attachment 205312 [details] Updated patch Alright, here is another patch. The previous one had some weird issues applying. I generated this one with eGit and works good. Dani, please use this.
(In reply to comment #32) > Created attachment 205312 [details] [diff] > Updated patch > > Alright, here is another patch. The previous one had some weird issues > applying. I generated this one with eGit and works good. Dani, please use this. The patch is missing a change in JavaModelOperation.runOperation(IProgressMonitor): IJavaModelStatus status= verify(); if (status.getSeverity() == IStatus.ERROR) { throw new JavaModelException(status); } Without that one cannot create a project as outlined in comment 0: it fails with an exception in the .log.
Created attachment 205313 [details] JDT UI Fix
There is another problem with the latest JDT Core patch: when I open the Java Build Path properties page the problem marker just disappears. This is wrong.
(In reply to comment #35) > There is another problem with the latest JDT Core patch: when I open the Java > Build Path properties page the problem marker just disappears. This is wrong. Yet another issue: Clean + Full build does not create the marker.
The error in the build path dialog was not allowing me to proceed. I will use the JDT/UI fix to test and fix the reported issues.
Please also fix the typo in the preference constant: "overlapping" is written with "pp".
Created attachment 205417 [details] Upated patch This patch addresses all concerns raised. Dani, the UI fix doesn't work when output location is changed from the source folder's context menu. The JavaConventions.validateClasspath is invoked from ClasspathModifier from two places and the second one should be fixed too. Another point Satyam brought up is whether there would be any side-effects of allowing a build path error as a warning. Currently the validation ends when we come across a overlapping output location (even when the option is set for warning), which means we could potentially ignore other severe errors. I think this needs some thinking too. And that's probably why we had that code in JavaModelOperation. What do you think?
(In reply to comment #39) > Another point Satyam brought up is whether there would be any side-effects of > allowing a build path error as a warning. Currently the validation ends when we > come across a overlapping output location (even when the option is set for > warning), which means we could potentially ignore other severe errors. I think > this needs some thinking too. And that's probably why we had that code in > JavaModelOperation. What do you think? To add more, validateClasspath() returns only one status for all classpath entries. We could make the validation to not return warnings immediately, but if there are multiple warnings, only one of them will show up. As of now, this is the only one warning and hence is not a problem.. However, Jay is pointing out that validateClasspath() is probably not designed to return warnings and hence probably should be addressed differently. Here is another observation - Javadoc for API org.eclipse.jdt.core.JavaConventions.validateClasspath(IJavaProject, IClasspathEntry[], IPath) mentions that it returns OK and doesn't talk about warnings. Essentially it means there is a slight change in the API behaviour. Dani, Do you think this change in API is OK? If this change in API is OK and we change it, at a latter point of time if we have more warnings, we might have to introduce one more API and then this change in API will look bad. What is your advice?
(In reply to comment #40) > Here is another observation - Javadoc for API > org.eclipse.jdt.core.JavaConventions.validateClasspath(IJavaProject, > IClasspathEntry[], IPath) mentions that it returns OK and doesn't talk about > warnings. Essentially it means there is a slight change in the API behaviour. Jay, If we are sticking to this approach, don't forget to change the Javadoc for this API.
> Dani, the UI fix doesn't work when output location is changed from the source > folder's context menu. The JavaConventions.validateClasspath is invoked from > ClasspathModifier from two places and the second one should be fixed too. My bad. Will be fixed with the next UI patch, coming shortly. > Another point Satyam brought up is whether there would be any side-effects of > allowing a build path error as a warning. Currently the validation ends when we > come across a overlapping output location (even when the option is set for > warning), which means we could potentially ignore other severe errors. You need to make sure that you only return this as a warning (or info) when all other checks don't return a more severe status (from the same validate call) - like it's currently the case. > However, Jay is pointing > out that validateClasspath() is probably not designed to return warnings and > hence probably should be addressed differently. What should be addressed differently? > org.eclipse.jdt.core.JavaConventions.validateClasspath(IJavaProject, > IClasspathEntry[], IPath) mentions that it returns OK and doesn't talk about > warnings. Essentially it means there is a slight change in the API behaviour. > Dani, Do you think this change in API is OK? I don't see a change here. It would be a change if the API said that ERROR is returned if not OK - but that's not the case.
Created attachment 205426 [details] JDT UI Fix
The JDT Core patch works for me but I did not review the code. Srikanth, please review the patch, given it is quite bug and gets backported.
(In reply to comment #42) > You need to make sure that you only return this as a warning (or info) when all > other checks don't return a more severe status (from the same validate call) - > like it's currently the case. Now, I see that JavaModelStatus can have children, we could even return all. > > > However, Jay is pointing > > out that validateClasspath() is probably not designed to return warnings and > > hence probably should be addressed differently. > What should be addressed differently? I meant, we could use a complete different approach. Now, you think this approach should be fine, we don't have to go here.
Created attachment 205470 [details] Updated JDT/Core patch Essentially same as the previous patch with a comment added in ClasspathEntry#validateClasspath() so all ERROR status can be checked before this potential WARNING status. Srikanth, Satyam has already reviewed the previous patch. So, I am marking him for review.
Another bit of inconsistency I found: When using the source folder's context menu to change the specific output location (let's say src2 is having src1 as output) from another source entry to let's say /P/bin, the user is asked, "Do you want to delete the old output folder at location..." and on saying yes, src1 is deleted. However, the alert doesn't appear when the same is done from the Java Build Path dialog. Is it alright to have it this way?
Created attachment 205481 [details] Patch with a change in Doc I have modified the Javadoc of JavaConventions#validateClasspath(IJavaProject, IClasspathEntry[], IPath) to mention about the new option in JavaCore.
The changes look good to me.
(In reply to comment #48) > Created attachment 205481 [details] > Patch with a change in Doc The same patch works well with R3_7_maintenance. Of course, this will go with part of the JDT/UI fix.
(In reply to comment #47) > Another bit of inconsistency I found: When using the source folder's context > menu to change the specific output location (let's say src2 is having src1 as > output) from another source entry to let's say /P/bin, the user is asked, > > "Do you want to delete the old output folder at location..." and on saying yes, > src1 is deleted. > > However, the alert doesn't appear when the same is done from the Java Build > Path dialog. Is it alright to have it this way? This has nothing to do with this bug. You can file a feature request if you think you need this from the Java Build Path dialog.
Hi Jay, Here are my review comments: Overall it looks promising, we need to resolve the folowing: ClasspathTests.java: -------------------- (1) testClasspathValidation22: The save of options introduced at the top i.e. Hashtable options = JavaCore.getOptions(); and the related restoration inside the finally block: are these needed at all, since we are only changing a project specific setting in the block in between and the project itself is being deleted in the finally block ? (2) I would have liked to see a single test cloned cloned over and modified to test the cases CORE_OUTPUT_LOCATION_OVERLAPPING_ANOTHER_SOURCE being set to error, ignore and warning explicitly and once with the default setting to prove the behavior is as expected. JavaCore.java: -------------- (3) Terminate the sentence "Reporting an output location ..." with a full stop. JavaConventions.java: --------------------- (4) In the javadoc for validateClasspath, fix the (pre-existing) odd usage of the word "coincidate" replacing all usages with "coincide". I see the (non) word coincidate used in the code too. I would leave these as they are and fix only the client visible documentation. IJavaModelMarker.java ---------------------- (5) The javadoc for OUTPUT_OVERLAPPING_SOURCE is badly formatted, if you hover over it, you can see the problem, part of the javadoc is eaten up and doesn't show up. ClasspathEntry.java (6) The comment says: // NOTE: The above code that checks for IJavaModelStatusConstants.OUTPUT_LOCATION_OVERLAPPING_ANOTHER_SOURCE, can be configured to return // a WARNING status and hence should be at the end of this validation method. Any other code that might return a more severe ERROR should be // inserted before the mentioned code. but I am not sure the code would actually do that. For instance in the for loop that iterates over the class path entries (line 1792), if we encounter a classpath_cannotUseDistinctSourceFolderAsOutput problem ahead of classpath_cannotUseLibraryAsOutput, then we would return a warning if CORE_OUTPUT_LOCATION_OVERLAPPING_ANOTHER_SOURCE is set to warning and will return an error of type INVALID_CLASSPATH if CORE_OUTPUT_LOCATION_OVERLAPPING_ANOTHER_SOURCE is set to ignore. I think we should not immediately return but rather capture the status and return it instead of VERIFIED_OK. ClasspathValidation.java (7) Could you annotate the new parameter to flushClasspathProblemMarkers just the way the old parameters are annotated ? (8) I am not entirely sure about the correctness/point behind the successive calls to flushClasspathProblemMarkers being made from validate() Can you explain what is going on here ?
Thanks for the detailed feedback Srikanth. After sleeping over this and running into more trouble with the warning status being returned (a test failure and several other places where we would fail over the status.isOK() pattern) I come to the conclusion that we cannot do this. Every client that follows that pattern will be broken when it encounters a project which such a state. I also discussed this with Markus and he agrees as well that we shouldn't do this (not even in 'master'). Good news is that we can fix this with a non-breaking fix that won't even require any UI code for the backport and only needs 3 changes to the current JDT Core patch: 1. The ClassPathEntry.validate... method will not set the WARNING status when the option is set to 'Warning' but use the OK status. That way we can transfer the warning message without breaking the clients. 2. We need to add a partial fix for bug 361263. To reduce the risk we will only special case the isOK() method for OUTPUT_LOCATION_OVERLAPPING_ANOTHER_SOURCE. 3. In ClasspathValidation.validate() we map the OK status to WARNING. Another good side-effect is that with this we also don't need the change in JavaModelOperation.runOperation(IProgressMonitor). Note that for 'master' we should set the default to 'Error' since that setup is really bad.
Created attachment 205529 [details] Modified JDT Core Patch (needs work) Here's the previous patch with the changes mentioned in the previous comment. Two things are missing: - Javadoc updates - Test updates This patch fixes the reported problem and does not need the UI changes for the backport. Filed bug 361403 for the UI addition to 3.8 M3.
Created attachment 205572 [details] Updated patch This patch contains changes from Dani's last patch + corrections as pointed out by Srikanth.
(In reply to comment #55) > Created attachment 205572 [details] > Updated patch > > This patch contains changes from Dani's last patch + corrections as pointed out > by Srikanth. Jay, please see if the following changes make it a tad bit cleaner. (1) Eliminate the change to JavaModelStatus.isOK() (2) Rather than returning a JavaModelStatus object with severity set to OK and code set to OUTPUT_LOCATION_OVERLAPPING_ANOTHER_SOURCE, return an object of anonymous subtype of JavaModelStatus with severity set to OK and code set to OUTPUT_LOCATION_OVERLAPPING_ANOTHER_SOURCE. This anonymous type will override the isOK method and always return true. (3) Change if (!status.isOK() || status.getCode() == IJavaModelStatusConstants.OUTPUT_LOCATION_OVERLAPPING_ANOTHER_SOURCE) this.project.createClasspathProblemMarker(status); into if (status.getCode() != OK) this.project.createClasspathProblemMarker(status); To me this looks relatively cleaner while maintaining the semantics agreed upon. Let me now if I have overlooked something.
Created attachment 205586 [details] Updated patch The patch contains the the suggestions given by Srikanth. I have also made some changes to the ClasspathTests in the form of new tests, esp. the case where status with more severe status should not be overlooked when there is a warning.
The latest patch looks good to me.
I was testing the last patch and found one major problem: the fix only works for newly created Java projects or when I touch the build path. Even a clean + full build does not show the problem marker. The only thing which does the trick is close + reopen. 1. Use latest build to create the setup from comment 0 2. Exit the workspace 3. Launch the workspace from your development workspace which has the patch ==> no warning shows up [ok could live with that] 4. Clean + full build ==> BAD: still no warning shows up Maybe related to bug 361267?
(In reply to comment #59) > I was testing the last patch and found one major problem: the fix only works > for newly created Java projects or when I touch the build path. Even a clean + > full build does not show the problem marker. The only thing which does the > trick is close + reopen. > > 1. Use latest build to create the setup from comment 0 > 2. Exit the workspace > 3. Launch the workspace from your development workspace which has the patch > ==> no warning shows up [ok could live with that] > 4. Clean + full build > ==> BAD: still no warning shows up Looking at the call hierarchy view of org.eclipse.jdt.internal.core.ClasspathEntry.validateClasspath(IJavaProject, IClasspathEntry[], IPath), I conclude that class path validation is run and the related markers are created on a need to do basis, i.e only when there is a perceived change to class path (i.e set class path, set container, set variable ...) A clean + rebuild does not involve a class path change and hence doesn't trigger a revalidation. This in itself does not sound like a bug to me. Though I agree that with the current fix deployed on a existing project, we should strive to report a diagnostic. Will continue investigation,
> Though I agree that with the current fix deployed on a existing project, > we should strive to report a diagnostic. > > Will continue investigation, We already have means to rebuild when the compiler version increases. Maybe we could add a one time hook to detect and add the new problem marker if needed.
(In reply to comment #61) > > Though I agree that with the current fix deployed on a existing project, > > we should strive to report a diagnostic. > > > > Will continue investigation, > > We already have means to rebuild when the compiler version increases. Maybe we > could add a one time hook to detect and add the new problem marker if needed. That is what I am looking at, will see what comes out of it.
Created attachment 205619 [details] Improved patch This patch implements the following changes: (1) We now arrange for class path validation to be triggered every time an upgraded version of eclipse with a revised build state generation version is launched on pre-existing projects. (2) Default for the new diagnostic is set to error. (so this patch is for 3.8 stream) (3) Simplified some tests. This passes ClasspathTests and manual testing against pre-existing projects with overlapping source folder set up. In terms of code change it is only a couple of new lines. One in State.java revising build state generation version and another in JavaCore.initialzeAfterLoad to trigger class path validation. Running all tests now.
Manual testing went well. The UI tests are running...
(In reply to comment #63) > Running all tests now. All JDT/Core tests are green.
Created attachment 205633 [details] Patch to adjust JDT UI tests for 3.6.2+
Released in HEAD via commit d790a2ca8d6b4d448bd19ba6c16b13d5ab5db43f.
(In reply to comment #64) > Manual testing went well. The UI tests are running... Dani confirmed that all UI tests are green.
(In reply to comment #67) > Released in HEAD via commit d790a2ca8d6b4d448bd19ba6c16b13d5ab5db43f. I've pushed the UI changes for 3.8, see bug 361403.
The 3 new APIs need to be recorded in buildnotes and doc needs to be updated. Raised bug 361563 for that
(In reply to comment #70) > The 3 new APIs need to be recorded in buildnotes Done.
Created attachment 205652 [details] Patch for R3_6_maintenance_Java7 This patch passes all JDT model tests.
Created attachment 205653 [details] patch for R_3_7_maintenance In this patch, the default setting has been changed to "warning" instead of "error"
Released in R_3_7_maintenance via commit 1b6fcd7c5e9a33e5ad2935ce848b28193c98f15d
(In reply to comment #72) > Created attachment 205652 [details] > Patch for R3_6_maintenance_Java7 > > This patch passes all JDT model tests. This seems to have been released into R3_6_maintenance_Java7 branch via commit id: ebbe9538adae7ae47732045e7c9e9fe4b72657d4 and tagged with vB79_R36x_J7. Though at this backport commit I see the new API's introduced having a @since tag of 3.6.4, not sure if this is the right thing to do. Still chekcing.
(In reply to comment #75) > Though at this backport commit I see the new API's introduced having > a @since tag of 3.6.4, not sure if this is the right thing to do. > Still chekcing. I believe, this was a considered decision arrived at by Markus and Jay. So all seems to be well. Patch has been released so far into 3.8 M3, 3.7.x maintenance stream and 3.6.2+Java7 branches. Will get this released into 3.6.x maintenance stream tomorrow.
Released the JDT UI patch from comment 66 to R3_6_maintenance_Java7. commit 3b0a8d9bfff48f65977cc45966bac476e21a1263 The @since 3.6.4 was chosen because that's the version of jdt.core in R3_6_maintenance (the earliest branch into which this API will be released).
Released the JDT/Core patch to R3_6_maintenance commit 6d651feda919acb88d1c3c62d26bbd7e7d7732f3
Thanks everybody.
All branches now include this fix in JDT Core: 3.6.2+, 3.6.2+J7, 3.7.2, and 3.8. The JDT UI changes also have been released everywhere (maintenance branches just contain the test updates from comment 66).
Dani has verified the fix on 3.8 using build id N20111020-2000 and for 3.6.2+java7 using build id M20111020-1539
Sanity tested the fresh build id: I20111025-1800 and things look good via a vis this fix.
Jay/Dani, While testing this I found two anomalous behaviors: I am not sure whether any follow up action is needed, I'll let you analyze this case and comment on it. 1. 3.7.1 vs 3.7. RC2 ----------------- -- Launch each on a clean work space, -- open the create java project wizard, populate some project name, next -- Check allow output folder for source folders. -- Create a new source folder src2 + finish. -- Right click on src2, configure output folder, specific folder, type in src here. Say OK. In 3.7.1 you get an error right away which is reported in the dialog while on RC2, you get a warning in the problems view. This warning disappears as soon as you finish with the project creation wizard. 2. If I use 3.7.2 RC2 to create a new project with just name specified and then do the following steps: -- Right click on project + properties + Java build path + source tab -- Check allow output folder for source folders. -- Add folder + create folder src2 -- Edit output folder + type in src and OK, Now the warning that appears persists. Even after a clean & rebuild, it is emitted again. Now I am not sure how much of what is observed is due to the UI work not being back ported to 3.7.2, but it appears that: - In (1) above, we have relaxed relative to 3.7.1 - (2) is confusing too. Am I overlooking some nuance here ? Is this the right scenario to test ?
(In reply to comment #83) > 2. If I use 3.7.2 RC2 to create a new project with just name specified > Now the warning that appears persists. Even after a clean & rebuild, > it is emitted again. > - (2) is confusing too. Sorry, (2) itself is not confusing. It is the right behavior. What is confusing is that diagnostic triggered by one chain of actions is fleeting while the same diagnostic triggered by a different chain persists.
What you see is disturbing but I think you see bug bug 361267. Also, we allow more setups (with later build path warning) on purpose since we unified the code paths. See also comment 19. However, I found two strange issues when going through your steps: - The output folder label is missing when creating the project but not when editing an existing project's build path (filed bug 369076). - The output folder dialog issues an error when creating the project, however, when editing the build path after creating the project, one gets a warning which then reports an error in the build path dialog. (filed bug 369081). Both of those issues are there since at least 3.4. I think we're good with this bug unless I missed something.
(In reply to comment #83) > - In (1) above, we have relaxed relative to 3.7.1 There are two ways one can make a specific output folder point to another source folder: 1) by explicitly setting the output folder to point to another source folder 2) make default output point to a source folder and enable the "Allow output folder..." option. In 3.7.1, case 2 was allowed without any warning/errors and use case 1 was completely blocked by a UI error. So, the behavior in 3.7.2 is somewhere in between.
(In reply to comment #83) > Am I overlooking some nuance here ? Is this the right scenario to test ? My bad, I was testing this from memory without reading through the background information. Specifically testing the scenario reported in comment#0, I do see the expected behavior (with the possible manifestation of bug 361267). Other issues are covered either by existing bugs or new ones. Verified for 3.7.2RC2 using build M20120118-0800
