This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 73042 - MFS Subversion support
Summary: MFS Subversion support
Status: RESOLVED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: Filesystems (show other bugs)
Version: 5.x
Hardware: All All
: P3 blocker (vote)
Assignee: rmatous
URL:
Keywords: API, API_REVIEW_FAST
Depends on: 74743
Blocks: 72404
  Show dependency tree
 
Reported: 2006-02-27 10:36 UTC by _ pkuzel
Modified: 2008-12-22 17:58 UTC (History)
4 users (show)

See Also:
Issue Type: ENHANCEMENT
Exception Reporter:


Attachments
Preliminary version for testing (41.33 KB, patch)
2006-03-16 15:08 UTC, rmatous
Details | Diff
Suggestion with polished InterceptionListener2 to make it more extensible, flexible for future. (3.78 KB, text/plain)
2006-03-31 09:27 UTC, rmatous
Details
Friend contact to be reviewed (3.54 KB, text/plain)
2006-03-31 14:44 UTC, rmatous
Details
issue_73042 merged into nb55 (38.49 KB, application/octet-stream)
2006-06-06 13:31 UTC, rmatous
Details
the same as above but now as a text file (38.49 KB, patch)
2006-06-06 13:34 UTC, rmatous
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description _ pkuzel 2006-02-27 10:36:43 UTC
In subversion I need to intercept rename and move.


Please extend InterceptionListener to cover:

  beforeMove
  moveSuccess
  moveFailure

Thank you.

It's incompatible change good old CVS friend fails.
Comment 1 _ pkuzel 2006-03-07 14:35:32 UTC
Could you evalute please? I'd need it in trunk in 14 days.
Comment 2 _ pkuzel 2006-03-08 13:40:47 UTC
Notes from our dicussion:

1. call all registered InterceptionListeners and AnnotationProviders (so CVS and
SVN can work at the same time).

2. define MoveInterceptor interface (and its registration method):

  /** 
   * Tests whether is "mv src destFolder/fileName" handled
   * by the moveImpl().
   */
  boolean implsMove(FileObject src, FO destFolder, String fileName);

  /**
   * Actually moves the file (e.g. svn move src destFolder/fileName).
   */
  void moveImpl(FileObject src, FO destFolder, String fileName) throws IOException;

  /** 
   * Tests whether is "mv src fileName" handled
   * by the renameImpl().
   */
  boolean implsRename(FileObject src,  String fileName);

  /**
   * Actually renames the file (e.g. svn move src  fileName).
   */
  void renameImpl(FileObject src, String fileName) throws IOException;
Comment 3 _ pkuzel 2006-03-08 13:45:55 UTC
And do not forget to declare friendship with org.netbeans.modules.subversion :-).
Comment 4 rmatous 2006-03-16 15:08:10 UTC
Created attachment 29270 [details]
Preliminary version for testing
Comment 5 rmatous 2006-03-16 15:12:09 UTC
Petr, please check if it is what you need. 

Warning: nobody should put this patch into trunk without reviewing code and new
friend contract - this patch is just first shot.
Comment 6 _ pkuzel 2006-03-27 12:57:16 UTC
Patch is not complete see issue #74030 for reason. I can not run tests.
Comment 7 _ pkuzel 2006-03-27 17:21:27 UTC
The code was moved to issue_73042 branch, please continue there.
Comment 8 _ pkuzel 2006-03-27 17:42:33 UTC
with raw:

  srcFile.renameTo(dstFile);

implementation I get rather unexpected refactoring behaviour.

Renaming A.java to B.java results into getting bunch of MDR exceptions and
having two files on my disk:

A.java that contains:
  public class B ....

and B.java that contains:
  public class A ....
  

No A class references renamed.
Comment 9 _ pkuzel 2006-03-28 14:30:37 UTC
It works now. Thank you.

It should be API_REVIEW_FAST-ed and promoted to trunk. (I'd prefer fileName
parametered signatures over name, ext couples).
Comment 10 rmatous 2006-03-31 09:27:37 UTC
Created attachment 29523 [details]
Suggestion with polished InterceptionListener2 to make it more extensible, flexible for future.
Comment 11 _ pkuzel 2006-03-31 12:32:16 UTC
You could define statefull handler with just one method:

interface IOHandler {
  handle() throws IOException;
}

and let all getXXXHandler(.........) methods return its instances.
Comment 12 rmatous 2006-03-31 13:41:11 UTC
IOHandler is fine. So, I'll rewrite my part of it to use it as suggested and put
it on branch. 
Comment 13 rmatous 2006-03-31 14:44:22 UTC
Created attachment 29531 [details]
Friend contact to be reviewed
Comment 14 rmatous 2006-03-31 14:49:45 UTC
Please review this API change reqquired for subversion implementation.

See:
http://www.netbeans.org/nonav/issues/showattachment.cgi/29531/ProvidedExtensions.java
Comment 15 _ pkuzel 2006-04-10 12:55:36 UTC
      1.47.26.2	2006/04/07 12:32:51 rmatous
#73042


Radek please provide last pre-commit tag if any. We need to temprorary build
with old issue_73042 API.

Thank you
Comment 16 _ pkuzel 2006-04-10 17:39:19 UTC
I have temporary rollbacked before_api_change:after_api_change and applied
following rename event fix:

Checking in MasterFileObject.java;
/cvs/openide/masterfs/src/org/netbeans/modules/masterfs/MasterFileObject.java,v
 <--  MasterFileObject.java
new revision: 1.47.26.5; previous revision: 1.47.26.4
done

I expect to merge before_api_change:after_api_change back after Subversion
module focus-study stabilization finishes...
Comment 17 _ pkuzel 2006-04-10 17:40:35 UTC
For review purposes use after_api_change tagget version, please,
Comment 18 _ pkuzel 2006-04-10 19:39:03 UTC
Issue #74743 dependency, reopening:

---- %% ----
with raw:

  srcFile.renameTo(dstFile);

implementation I get rather unexpected refactoring behaviour.

Renaming A.java to B.java results into getting bunch of MDR exceptions and
having two files on my disk:

A.java that contains:
  public class B ....

and B.java that contains:
  public class A ....

It works now. Thank you.
---- %% ----

It worked because I started to use preview-less refactoring :-(.

It looks like already opened Documents (InputStreams) are not switched correctly.
Comment 19 Jaroslav Tulach 2006-04-11 06:21:51 UTC
I am not quite sure what should be reviewed here. I've only found one java 
source. I do not understand what it is good for, I see no sample usages/tests. 
Imho if you have nothing more than this issue and the one attachement, it is 
not ready for integration to trunk, not even speaking about release55.
Comment 20 _ pkuzel 2006-04-11 11:08:25 UTC
Ideal SPI from my perspective:

  boolean canRename(File from, File to);
  void rename(File from, File to) throws IOException;

Radek tries to simplify the implementation assuming there is not old
VCSFileSystem (it's guaranteed by new CVS and Subversion at module enablement
level).
Comment 21 rmatous 2006-04-14 17:23:13 UTC
#74743 is fixed + API change + FileSystemHandler in subversion modified to
accept this change.  Petre, please check it. 
Comment 22 _ pkuzel 2006-04-18 10:59:23 UTC
Current Status

openide/masterfs branch issue_73042 contains functional implementation
fulfilling this RFE.

subversion trunk contains code leveraging it

javacvs/cvsmodule issue_73042 branch contains CVS changes leveraging it. 
javacvs/cvsmodule trunk contains CVS changes leveraging it.

(All can be be merged to trunks and release55 branch if needed).



Post mortem, even simpler one-method SPI:

  boolean move(File, File) throws IOException

where returned false means that it was not handled. Returned true or IOException
means it was handled. It's more atomic than two methods...
Comment 23 rmatous 2006-05-03 11:01:17 UTC
Fixed
Comment 24 rmatous 2006-06-05 16:51:44 UTC
I'm going to fix it into 5.5
Comment 25 Jaroslav Tulach 2006-06-05 17:12:11 UTC
Can you please attach diff that you are about to apply to release55 branch? I 
want to review it carefully as this is an integration to platform cluster 
which shall contain no incompatibilities with release50. Also please make sure 
you compile on JDK 1.4.
Comment 26 rmatous 2006-06-06 13:31:50 UTC
Created attachment 30827 [details]
issue_73042 merged into nb55
Comment 27 rmatous 2006-06-06 13:34:38 UTC
Created attachment 30828 [details]
the same as above but now as a text file
Comment 28 Jaroslav Tulach 2006-06-06 16:26:40 UTC
Reviewed. I went thru the code with Radek and as far as I can tell, it is 
compatible with masterfs in release50

Most of the differences are guarded by if (handler != null) and this handler 
is only provided by Subversion, e.g. they cannot cause incompatibilities. This 
is about 90% of changes, for the rest, Radek managed to explain me, why they 
are there and I agreed that it was a meaningful and compatible change.

I support the integration of this change to release55

Comment 29 rmatous 2006-06-13 13:45:13 UTC
Fixed in NB5.5 according to patch.diff.