| Summary: | Don't force me to refresh all the time | ||
|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Lance Eason <lance.eason> |
| Component: | Resources | Assignee: | James Blackburn <jamesblackburn+eclipse> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | P3 | CC: | angvoz.dev, antonel.pazargic, daniel_megert, d_a_carver, esteban.dugueperoux, jamesblackburn+eclipse, jeremy, kkazmierczyk+eclipse, lars.geyer-blaumeiser, loskutov, malaperle, markus.kell.r, mober.at+eclipse, pwebster, remy.suen, Szymon.Brandys, wbeckwith, win32pro, xavier.mehaut, yevshif |
| Version: | 2.0 | ||
| Target Milestone: | 3.7 M7 | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
| Bug Depends on: | 303517, 340978 | ||
| Bug Blocks: | 353025 | ||
|
Description
Lance Eason
Which build are you using? Which OS are you running on? What type is your file system? Is your workspace data on a network drive? Which VM are you using? Do you have external programs modifying the contents of your project outside of Eclipse? Do you have reproducable steps to make your resources out of sync? There are two possible scenarios here: 1) You are knowingly modifying files from outside the workbench, using some external tool. In this case, it is expected that you will be out of sync. You can try using the autorefresh plugin that is found on the core team web page: http://dev.eclipse.org/viewcvs/index.cgi/%7Echeckout%7E/platform-core-home/dev.h tml 2) There is some tool in eclipse that is misbehaving, or some error that is causing eclipse to become out of sync with the workspace. In this case, we are very interested in getting more details from you because this should not be happening. In this case, please answer as many of DJ's questions as possible so we can help you track this down. Sorry, I thought I had already answered the questions. Must not have hit commit. The main answer is that yes I'm using external tools to modify files. My project includes both Python and HTML in addition to Java, both of which I am editing with tools other than Eclipse (though launched from Eclipse). I think the rest is probably moot but: build: Eclipse 2.0, 20020416 OS: Windows 2000 Filesystem: Fat32 Workspace: local VM: Java HotSpot(TM) Client VM (build 1.3.0-C, mixed mode) Reproducible steps: edit any file with an external tool. I'll try the plugin, thanks. This sounds like a candidate for the auto-refresh plugin. Core has no intention of automatically performing an auto-refresh before each workspace resource operation, so having the back-ground task automatically check for out of sync resources seems like the right answer. Give the auto-refresh plugin a try and let us know how it goes. Closing bug for now. *** Bug 20378 has been marked as a duplicate of this bug. *** *** Bug 30049 has been marked as a duplicate of this bug. *** We should reconsider this enhancement since the preference to auto-refresh the whole workspace is not sufficient to solve the problems of our users: 1. one might not want to enable that preference 2. if enabled, the user (or some job) might touch a file before it got refreshed Over the time we got many bugs from users that don't want to see the "out of sync" messages when a file gets touched. Main areas are: - opening a file (bug 168695, bug 248068, bug 329100) - search (see comment 0, bug 268797) - team operations like commit or update (see comment 0, bug 73260) They simply expect the file to be refreshed and then read again. We should add a new preference [ ] Automatically refresh files when touched For performance reasons we don't need to refresh it unless we get the out of sync exception when reading the file. The advantage of this new preference is that it helps the user in all kinds of scenarios and he is not forced to use the workspace refresh in the background. *** Bug 329100 has been marked as a duplicate of this bug. *** Szymon, can someone look into this? Shouldn't be too much work. See also: bug 303517 for a proposal for core.resources to automatically queue out-of-sync resources with the RefreshManager. (In reply to comment #9) Gosia will look at it. (In reply to comment #7) Dani, I think you are right, in plenty of places where out of sync causes throwing an exception we should try to refresh the file before throwing an exception. The proposal James attached in bug 303517 is great if user has the autorefresh flag on and we may go a little bit further and add a flag you proposed that would work even if general autorefresh is off. However think if we refresh the file whenever it's touched, because file refresh may trigger some additional operations, like rebuild. Let's assume that we have a project with 10 files out of sync and we are performing a search on it. Now after the search is finished user gets an information the those 10 files were skipped. In future we would like to refresh those files and include them in search. We have two solutions: 1. We refresh the files one by one when search gets to them. But then if every file refresh would trigger a build or other time-consuming operation we may end up with out workspace totally blocked by 10 builds running one by one. 2. We refresh the project when first out-of-sync file is found - good but may be doing it unnecessary, what if only one file was out of sync? This also requires that the refresh would be performed on a very high level: we know the context of performed operation and this means that we won't be able to solve all the out-of-sync problems by one fix. Each of the bugs attached by you should be solved separately. Do you think that may be a way to avoid triggering multiple operations by multiple refreshes but still solving the problem on the low level? (In reply to comment #12) > Do you think that may be a way to avoid triggering multiple operations by > multiple refreshes but still solving the problem on the low level? The refresh manager refresh job already batches refreshes... (In reply to comment #13) > (In reply to comment #12) > > Do you think that may be a way to avoid triggering multiple operations by > > multiple refreshes but still solving the problem on the low level? > > The refresh manager refresh job already batches refreshes... Exactly. And currently, the user will face the same problems you list just with the difference that he has to initiate this manually. Refreshing the project in case you find an out-of-sync file is not an option. I don't want to refresh my large project just because a file is out of sync. James, Dani, if user has auto-refresh flag on he may in fact face those problems, but in theory the resources are refreshed soon after they get modified in filesystem. So if files are modified from time to time it's probable that we won't have 10 refresh requests in a row and there won't be 10 same jobs triggered by the file change queued. If this would happen users would probably keep the auto-refresh flag off. On the other hand if user has the auto-refresh flag off and performs a search he will get information that 10 files are out of sync. I doubt that he would refresh all of them one by one. User rather refreshes the parent folder or the parent project because it's faster. And I think this creates only one delta that will trigger only one build. I would like to avoid situation that before we start the operation we know that we will refresh 10 files and we generate 10 deltas instead of generating just one. (In reply to comment #15) > James, Dani, if user has auto-refresh flag on he may in fact face those > problems, but in theory the resources are refreshed soon after they get > modified in filesystem. This isn't the case on Linux (for example) or any platform uses the polling provider. > On the other hand if user has the auto-refresh flag off and performs a search > he will get information that 10 files are out of sync. I doubt that he would > refresh all of them one by one. User rather refreshes the parent folder or the > parent project because it's faster. And I think this creates only one delta > that will trigger only one build. It won't be faster to refresh the full project. And fundamentally there's no need to -- the RefreshJob already batches changes, and there's API to schedule the resource for refresh: RefreshManager#refresh(). AFAICS fixing this is just a matter of wiring. It's never useful for out-of-sync resources to persist for long periods in the workspace. > I doubt that he would > refresh all of them one by one. User rather refreshes the parent folder or the > parent project because it's faster. There might be n parents, not just one ;-) > I would like to avoid situation that before we start the operation we know > that> we will refresh 10 files and we generate 10 deltas instead of > generating just > one. Well *core* does not know while the client *might* know. However, in order to have the new option work seamlessly it would mean that *every* client has to update its code which is simply not feasible. As James pointed out, the RefreshJob should take care of your concerns. (In reply to comment #7) Good discussion, and great summary of the problem from Dani. I'd only be careful with terminology since "touching a file" is often seen as modifying it or its timestamp (like the unix touch command). I think what you meant was likely "accessing the file". Today when a client wants to read some file contents, an out-of-sync exception may be thrown; I can see how in the future the client could be allowed access to the file contents, and refresh can be performed in batch as a side-effect (thus queueing notifications of the modified contents for other clients). Essentially, the client tells other clients "by the way, I just found that X has been modified". I'm not yet sure what this would mean for containers though. Does a search that iterates over containers (folders) in the Workspace check whether those containers are still in sync? I doubt so, and this may mean that a search might miss files added on the file system. Today, as Malgorzata points out, users who are prompted to refresh manually will often refresh the project or workspace, thus detecting such added files. > I think what you meant was likely "accessing the file". Yes. > I'm not yet sure what this would mean for containers though. Does a search > that iterates over containers (folders) in the Workspace check whether those > containers are still in sync? No. >Today, as Malgorzata points out, users who >are prompted to refresh manually will often refresh the project or workspace, >thus detecting such added files. Well, only if you're lucky and search has accessed a file that was out-of-sync. If that did not happen, then you would also have no indication. I don't think we should use that argument to not implement the new option or to make it more complicated. Well, you are right that this bug is primarily about addressing the annoying "why do you prompt me to refresh when you could just refresh yourself" issue so it's not immediately about Containers. But looking at the scenarios mentioned by John in comment 2, the bigger picture in which this occurs is people who have their files modified by programs outside Eclipse. Such users are much more interested in seeing actual file system state at any one time, than just seeing cached resource system state. Looking at the code in Container#members(int), there is already now a code path in which calling members() results in performing refresh as a side-effect (when members are UNKNOWN, eg very soon after opening a project with deferred refresh). I could imagine simply extending this concept: - Have a Preference "Always refresh local files on access" - Always have queries for file state "pass through" to the file system when this is on - Only on the "local" file system (no EFS-based file systems, due to performance) This wouldn't necessarily make the solution much more complex, but would make it more consistent. There is a small risk that performance might suffer (especially for commands like ctrl+shift+R "Open Resource"), but I feel like it is worth a try in order to cater to those who use external tools. Perhaps this could be handled with a separate bug, but I do think it is very related to this discussion. > This wouldn't necessarily make the solution much more complex, but would make
> it more consistent. There is a small risk that performance might suffer
> (especially for commands like ctrl+shift+R "Open Resource"), but I feel like it
> is worth a try in order to cater to those who use external tools.
>
I'm not against that - just wanted to express that we shouldn't defer/close this bug again just in case we will not be able to solve the full issue.
Dani is dead on; there are like fifty dupes of this bug, and if you read the comments people are really annoyed by it. However, despite it being around for months nothing has been done with it (and given that the last comment on it was three months ago, it looks like still, nothing is being done with it). Please DO NOT overcomplicate this bug; all anyone wants is the simplest thing possible: when you open the file, it refreshes. I don't care how many components are involved or how Eclipse does things on the back-end; if the user can highlight the file in the package explorer and hit F5 after opening the file, so can Eclipse, and that's all that's really needed here. PLEASE just do the bare minimum; you'll make thousands of Eclipse users happier than if you overthink things and try to account for cases no one cares about. +1 for auto-refreshing the file in question as requested, and not caring about other files in the folder. I agree that a "file modified outside Eclipse" is more likely to happen that a "file added/removed outside Eclipse". And that dealing with containers is harder than dealing with files. So +1 for proceeding with the proposed approach and making the easy fix. Containers may be dealt with in a separate bug. (In reply to comment #23) > +1 for auto-refreshing the file in question as requested, and not caring about > other files in the folder. Agreed, all that's required is for someone to care enough to write some code and tests... Refresh is on my list, but it's unfortunately not yet at the top. > And that dealing with containers is > harder than dealing with files. So +1 for proceeding with the proposed approach > and making the easy fix. Containers may be dealt with in a separate bug. See also bug 334930. ChrisR may be doing some prototyping on this issue over the next couple weeks. It can be fixed quite easily in FileSystemResourceManager#read, #isSynchronized, etc. with only a few lines of code. See 5-minute prototype on: bug 303517 The patch + tests in bug 303517 will fix this issue in core.resources. Reading over the comments again, I think this bug took another turn than what I had in mind in comment 7 (and still have): in order to solve the problem of out-of-sync messages without many fixes on the client side it is not enough to schedule the refresh. The new preference would tell the current APIs to not throw an exception but refresh the resource immediately and return the correct/refreshed data (and then cause a delta to be sent). If the refresh is only batched then this would not solve the issue for clients like search, delete, etc. >>If the refresh is only batched then this would not solve the issue for clients
>> like search, delete, etc.
So, some people (eg. you, evidently), care about "search, delete, etc." Others (like myself), just want to be able to open a file without having to then click on the file in the package explorer and hit F5 every flippin' time :-)
I don't mean to in any way suggest that your concerns shouldn't also be addressed. However, if the current proposed fix will solve the case of opening a file and having to refresh it (which for most users probably occurs a lot more often than searching/deleting/etc.), then I really don't think that fix should be held up just because a similar/related concern isn't being addressed.
This bug has been around since 2002 for Pete's sake! Please let it get fixed already (even if only partially).
(In reply to comment #27) > ... The new preference would tell the current APIs to not > throw an exception but refresh the resource immediately and return the > correct/refreshed data (and then cause a delta to be sent)... This sounds clear, simple & good enough for the 70% of use cases where users are thinking "why this IDE can't just update this file?". The rest of all possible use cases should go to other bugs/enhancements. Please keep it simple enough to make it into 3.7. Just to add my $0.02, but in a lot of places eclipse gives the user the option to configure the IDE to match their env., and this is no different. Non-static access to a static member can be an error, warning or ignored but it is ultimately up to the user to decide. I understand the desire not to go preference crazy or that the proposed solution doesn't address all use cases, but it does adress a large portion of the end user grief that comes from using eclipse. (In reply to comment #27) > Reading over the comments again, I think this bug took another turn than what I > had in mind in comment 7 (and still have): in order to solve the problem of > out-of-sync messages without many fixes on the client side it is not enough to > schedule the refresh. The new preference would tell the current APIs to not > throw an exception but refresh the resource immediately and return the > correct/refreshed data (and then cause a delta to be sent). If the refresh is > only batched then this would not solve the issue for clients like search, > delete, etc. I think the best thing to do here is iterate on this as we go forward. Community is requesting for the simpliest thing that can possibly work that addresses the most seen cases. We can get it perfect as we go forward. Start with the current patch, and then iterate and improve it as you run into the other problems. Fixes for bug 340978 and bug 303517 seem to address this issue too. James could you verify that? This has been fixed by the changes in Bug 303517 and Bug 340978. The new lightweight refresh preference, when enabled, suppresses these out-of-sync errors and auomatically brings the affected resources back into sync shortly after detection. Bug 340977 exists for potentially changing the default for LIGHTWEIGHT_REFRESH -> true |