Community
Participate
Working Groups
Currently, you can specify a patch as a File path or Clipboard contents. All of the patches I apply come from Bugzilla links. For exanmple: http://issues.apache.org/bugzilla/showattachment.cgi?attach_id=12697 It would save a manual download step and a copy step if I could paste the URL in the dialog. Thanks. Gary
We're looking into better integration between Eclipse and Bugzilla. I can't say if we'll provide this exact support but we definitely want to do something that simplifies applying patches. We will update this bug with any approaches we are considering.
The Bugzilla plugin is not developed further so this should go into the Apply Patch dialog.
Retagging as 3.2 since it will not make it into 3.1.
Yes please add this since it makes life much easier: 1. copy bugzilla patch URL 2. paste into Apply Patch wizard Instead of saving and selecting the file again using a dialog.
I agree that it would be useful but we do not plan on addressing this in 3.3. Patches will be accepted.
Created attachment 55350 [details] applypatch_handle_url.diff Here is the first version of a patch to support applying patches directly from URLs. * new URL combo with history (like file) * validation of the url * automatic recognation of urls in clipboard comment on the last point: the behavior at the moment doesn't make much sense to me. it's like this: if(workbench selection == file) // check file related patch stuff // if file is not a patch, break else // check clipboard In my eyes, we should check the clipboard too when the selected file in the workbench is not a patch. This is also done with this patch. Additionally, if you have a URL in your clipboard, the Apply Patch wizard will recognize it, will select the URL-option for you and paste it into the combo. Nothing to do for you :) As I mentioned, this is the first version of the patch and i hope to get some feedback how we could improve it. Patch is against current HEAD.
Thanks for the patch. I took a quick look at the patch and it looks consistent with the code that already exists in the file. I agree with your suggestion that the else should just happen if the target file isn't a patch (i.e. you can take out the "if (1 = 1)". I'm curious why you took out the null check on the Control in that same method. Is it 100% guaranteed that the control won;t be null when that method is called. From my standpoint, it seems the safest to leave it in (i.e. because the cost is low and I don't really want to go through all the possible scenarios for the method to see if the null check was actually required. There is one really tricky issue that you have not addressed. When connecting to remote systems, it is very easy to get caught in a situation where the connection blocks. I haven't done a lot with URLs but with Sockets, you need to do a bunch of things to ensure that the connection does not block the users: 1) You need to make the connection and fetch the contents in a background thread. You can use the getContainer#run(true, true, runnable) method for this. 2) You need to support cancelation. We have special stream in Team/Core that do this for Sockets but, unfortunately, Compare does not depend on Team/Core. It is possible that it could but we would need to determine whether this would be a problem for clients that may use Compare in an RCP app. The easiest thing to begin with would be to set the socket timeout to a reasonable value (say 60 seconds) so that the read won;t block indefinitely. 3) I don't know about URLs, but with Sockets there is no connection timeout so we needed to make sure that the connection attempt didn't block indefinitely. You can see what the code looks like by looking in the Util#createSocket method of the org.eclipse.team.cvs.ui method. This is pretty messy stuff but it (or something similar) needs to be done since any failure in the connection that causes an indefinite block will make the user very unhappy. I've copied John because he mentioned recently that he may be adding support to the Jobs framework that would help in this situation. We should probably wait for his reply since tis may simplify the situation greatly.
FYI, for those using Mylar (http://www.eclipse.org/mylar), the latest version supports applying patches directly from bug reports. See: http://www.jroller.com/page/eu?entry=no_more_excuses_to_not
What work is needed to champion this patch and get it progressed. I use eclipse as part of my workflow for patch/review approval on the WordPress project and at the moment I find myself switching back to the commandline where I can use curl + patch to apply a patch directly to a svn checkout from a url. For bigger patches I really want to work in eclipse but having to download the patch first is a real pain - i have tried using the clipboard method but it seems to choke on the svn generated patches which have revision info in them.
(In reply to comment #9) > What work is needed to champion this patch and get it progressed. I belive that Michael clearly explained (comment 7) what need to be done to make Benjamin's patch fully operational. Currently, we don't have the manpower to address it, but patches will be accepted. > i have tried using the clipboard method but it > seems to choke on the svn generated patches which have revision info in them. Why don't you file a bug for this particular issue?
(In reply to comment #7) > 1) You need to make the connection and fetch the contents in a background > thread. (...) > 2) You need to support cancelation. (...) > 3) (...) make sure that the connection attempt didn't block indefinitely. (...) The first two are rather straightforward. The third one seems to be a hard nut to crack with the EE limitation we have. The simplest solution would be to call setReadTimeout/setConnectTimeout on URLConnection but given the fact that both methods are available since J2SE-1.5[1] and o.e.compare requires J2SE-1.4 this won't happen :/ . Here are our options: a) set J2SE-1.5 as EE for o.e.compare b) wrap the connection in a timer thread c) set system-wide properties: sun.net.client.defaultConnectTimeout and sun.net.client.defaultReadTimeout[2] (global settings and don't work for HTTPS afaik) d) write an HTTP handler to set a timeout value, something similar to what Mike Reiche suggested here[3] e) use HTTPClient[4] from the Apache Foundation f) check if 1.5 API exist and call via reflection I think I will try combination of f) and e) marking Apache packages optional. This way I'm sure the o.e.compare plug-in will run even if the package dependency is missing. On the other hand, if neither 1.5 API nor HTTPClient lib is available the option to apply patch using an URL won't show up. [1] http://javadiff.sourceforge.net/jdiff/reports/j2se142_j2se150b1/changes/java.net.URLConnection.html [2] http://java.sun.com/j2se/1.4.2/docs/guide/net/properties.html [3] http://objectmix.com/weblogic/568262-weblogic-net-httpurlconnection-settimeout-int-i-not-there.html [4] http://jakarta.apache.org/commons/httpclient/
>a) set J2SE-1.5 as EE for o.e.compare This seems like the best. People are really going to require running E on top of Java 1.4? This feels like bending over backwards to support a dead platform. I say we welcome them to the 21st century and Java 5! :)
Switching to 1.5 is not an option - at least not for 3.6 as 1.4 us one of the officially supported target environments for 1.4.
Created attachment 154650 [details] Patch v02 Here is Benjamin's updated patch. I added support for cancelation and made the connection/fetching run in a background. Regarding the timeout issue I decided to go only with f) (see comment 11) for now. As a user I would like to be able to use an URL to apply a patch even though it can hang the app, so the option is there even if you don't run on 1.5 API. I will open a separate bug for those who think this should be more robust (turn it off for 1.4 and/or use httpclient).
Created attachment 154651 [details] mylyn/context/zip
(In reply to comment #14) > I will open a separate bug for those who > think this should be more robust (turn it off for 1.4 and/or use httpclient). It's bug 298460.
Latest patch applied to HEAD (with minor changes made on the fly). Available in builds >N20091217-2000. Benjamin, thanks for the initial version of the patch.
