Bugzilla – Full Text Bug Listing
| Summary: | Apply patch needs to accept standard git format | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Technology] EGit | Reporter: | bram goffings <bramgoffings> | ||||
| Component: | Core | Assignee: | Project Inbox <egit.core-inbox> | ||||
| Status: | CLOSED INVALID | QA Contact: | |||||
| Severity: | enhancement | ||||||
| Priority: | P3 | CC: | angvoz.dev, caniszczyk, dj.houghton, matthias.sohn, mn, pwebster, randy, remy.suen, robin.rosenberg, stefan.lay, tomasz.zarna | ||||
| Version: | 0.12 | ||||||
| Target Milestone: | --- | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
bram goffings
We'll look at it for the 0.12 release. It's nice to see that the Drupal team is adopting EGit :) (In reply to comment #1) > We'll look at it for the 0.12 release. > > It's nice to see that the Drupal team is adopting EGit :) Yes this will be fantastic for Drupal developers. We decided to go with native git patches (patch -p1 patches) because it seemed to be the right thing when adopting git. Thanks very much for your help on this (and it is urgent for our egit users - thanks!) 'Apply Patch' is a command coming from the platform, not from EGit. What is the best way to include the Apply Git Patch functionality? Should we create a new menu entry 'Apply Git Patch' or should we replace 'Apply Patch'? In the replaced 'Apply Patch' we could check the patch format and either delegate to the platform handler or to the new Git-Patch handler. The first solution is less error prone and we would not have nasty dependencies to the platform code. However, then the user would have to know if the patch is of git patch format or not. Another approach would be to let the user specify the -p level of the patch to be applied, which would be more general. Users often end up with patches that are not -p0 (might be -p1 *OR* -p8 or something because the diff was done at the wrong level of the tree. That said, the Drupal project the automated testing is (currently) supporting both -p1 and -p0. The strategy is that -p1 is tried first. If it succeeds, it goes. If it fails, -p0 is tried. This isn't really user friendly, most egit users will not know the difference between p1 and p0 (as it it command line language) that is the reason they use Gui tools. (In reply to comment #4) > Another approach would be to let the user specify the -p level of the patch to > be applied, which would be more general. Users often end up with patches that > are not -p0 (might be -p1 *OR* -p8 or something because the diff was done at > the wrong level of the tree. > > That said, the Drupal project the automated testing is (currently) supporting > both -p1 and -p0. The strategy is that -p1 is tried first. If it succeeds, it > goes. If it fails, -p0 is tried. (In reply to comment #2) > (In reply to comment #1) > > We'll look at it for the 0.12 release. > > > > It's nice to see that the Drupal team is adopting EGit :) > > Yes this will be fantastic for Drupal developers. We decided to go with native > git patches (patch -p1 patches) because it seemed to be the right thing when > adopting git. Thanks very much for your help on this (and it is urgent for our > egit users - thanks!) I guess you have a review process based on these patches, maybe similar to what linux kernel does ? Then I recommend you should maybe have a look at Gerrit code review [1]. Watch it in action used by EGit itself here [2]. Alex' screencast [3] shows the essentials of its code review workflow and also showcases its integration with jenkins using the gerrit-trigger plugin [4] to get automated verification of all changes uploaded for review. Latest EGit also has some integration with Gerrit, more will come from the Mylyn Reviews Gerrit Connector which currently is still in a very early state [5]. [1] http://code.google.com/p/gerrit/ [2] http://egit.eclipse.org/ [3] http://alblue.bandlem.com/2011/02/gerrit-git-review-with-jenkins-ci.html [4] http://wiki.jenkins-ci.org/display/JENKINS/Gerrit+Trigger [5] http://www.eclipse.org/reviews/ Is the issue with a/ b/ still an issue. When I tested, I could apply a git patch just fine. (In reply to comment #7) > Is the issue with a/ b/ still an issue. When I tested, I could apply a git > patch just fine. I've been dealing with git patches in Eclipse for some time now and the "Apply Patch" wizard works fine for me as well. You need to be aware how some of its options work (e.g. apply to workspace root, ignore leading segments), which is not always obvious indeed. Bug 358527 in Platform/Compare is about to improve it. Side note, I was even able to apply git patches in "email friendly git format" as long as they came with CRLF line endings (I'm on Windows). Created attachment 203914 [details]
Setting for git patches
This shows the setting for applying a git patch.
The patch wizard looks great to me. You have to set "ignore leading path name segments" to 1 for a git patch, but that's fine. Thanks for the great work on this! See attachment https://bugs.eclipse.org/bugs/attachment.cgi?id=203914 Is this bug referring to the format which requires "git am" to apply or should I open a new one about that? Those types of patches will not apply without first editing the files by hand. (In reply to comment #11) > Is this bug referring to the format which requires "git am" to apply or should > I open a new one about that? Based on the comments, I don't get the impression that this patch is talking about the patches with comments embedded in them. Ok thanks. For interested parties, I've opened Bug 368358 to track that problem. [Batch change] Remove passed Target Milestones If anyone on CC list is going to fix/implement this, feel free to assign a new, post-1.3/2.0, target milestone. The initial assertion that Eclipse does not accept git patches at all is wrong. |