| Summary: | [native] Don't fail install when rmdir cannot delete empty directory | ||
|---|---|---|---|
| Product: | [Eclipse Project] Equinox | Reporter: | DJ Houghton <dj.houghton> |
| Component: | p2 | Assignee: | P2 Inbox <equinox.p2-inbox> |
| Status: | RESOLVED WONTFIX | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | henrik.lindberg, john.arthorne, pascal.rapicault, pascal |
| Version: | 3.7 | ||
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | Mac OS X - Carbon (unsup.) | ||
| Whiteboard: | stalebug | ||
| Bug Depends on: | |||
| Bug Blocks: | 332148 | ||
|
Description
DJ Houghton
This might be more serious than originally thought. Since the RmDirAction returns an IStatus with ERROR level severity, the Engine will bail on the operation. Adding Henrik to the CC list since he was interested in a related problem in bug 272312. Seems to me that operations running "on top of a shared install" where certain operations are not allowed needs to be handled at a level above performing actual file/directory changes (with corresponding requests to backup or restore). Yep, that sounds right. Thanks Henrik. John and I have also talked it over and in the use case we have, we believe that the metadata is incorrect and p2 is correctly behaving to what it has been told (but does not have permission) to do. Henrik, in bug 272312 we decided to be more forgiving when performing actions which failed. Do you think we should be returning a warning here instead of an error or would that hide too many real problems? (In reply to comment #4) > Henrik, in bug 272312 we decided to be more forgiving when performing actions > which failed. Do you think we should be returning a warning here instead of an > error or would that hide too many real problems? I think the "silently ignore errors" mode of operation is problematic in general - it makes things less robust. I would prefer if a higher level knew that things are "shared" and "locked down" and therefore should not be done/removed at all, or if not possible that it is prepared to handle the errors. Is it possible to affect the code that is performing the logging? Maybe it can filter out certain types of expected errors when it knows that it is a "shared install" ? If we change the semantics of the "remove" touchpoint actions to "remove x - warn if user is not authorized to remove and threat all other problems as errors" we risk giving positive feedback to users when in fact, the installation did not succeed. There are many corner cases that could produce bad results (a delete of a directory is expected so that all of its content is removed, the directory is then recreated and new content is placed in the directory - if it was not deleted, and the addition just creates missing directories (and thus does not notice that it was not deleted) the result could be a mix of old files that should have been deleted and new (or overwritten) files, etc.) BTW - Is there some logic that "understands" that operations are running on a shared install? Or is this just a consequence of ignoring errors ? We looked at the rmdir code again today, and noticed the following current behaviour: 1) If the directory is not empty, we ignore the failure and continue 2) If the directory is actually a file, we ignore the failure and continue 3) If the directory is empty but it could not be deleted, the install/uninstall fails entirely What I suggest is changing 3) so that it logs the failure, but continues with the install. I think it *should* log this rather than completely ignoring it, because it may represent a metadata problem as it turned out to be in the case we looked at. But, failing the install entirely because an empty directory couldn't be deleted seems like overkill. For example if the user has a command prompt open on a directory in Windows it could prevent such a deletion. Changing the summary to refect the different direction. We'll continue logging all warnings regardless of what mode we're in. Failure to delete an empty directory should change from error to warning. Move all 3.8 bugs to Juno. This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie. This bug was marked as stalebug a while ago. Marking as wontfix. If this report is still relevant for the current release, please reopen and remove the stalebug whiteboard tag. |