Community
Participate
Working Groups
Juno M2. Now that we switch(ed) to Git more patches will be in Git format. Applying such a patch is currently painful as the user has to manually set the leading segment ignore count and also manually select 'Apply patch to the workspace root' on the first page. And he has to know those tricks in the first place. Given that the Git patch can be recognized (has "diff --git " in it) we should use smarter defaults: - Apply the patch to workspace root if the patch crosses more than one project. NOTE: In case we can't detect this, we should always choose the root. - Set ignore leading segment count to 1 if workspace root is selected 2 for the other case Changing those defaults does not prevent any other (Git) patches to be applicable since the user can still change the initial values.
Another issues related to the recent switch to Git.
(In reply to comment #0) > - Apply the patch to workspace root if the patch crosses more than one project. > NOTE: In case we can't detect this, we should always choose the root. Currently we cannot detect this. The patching mechanism is not aware of projects unless it's dealing with a "workspace patch" (indicated with "### Eclipse Workspace Patch 1.0" in the first line). For CVS this is added by the "Create Patch" action, which makes the patch "Apply Patch" wizard specific. Having a similar thing in EGit would greatly improve handling patches that span multiple projects. > - Set ignore leading segment count to > 1 if workspace root is selected > 2 for the other case I'm not sure about this part. Skipping a/ b/ prefixes would be great, but what if a git patch comes without them? Given these patches are now generated in the command line (no "Create Patch" in EGit), there is little we can assume about the format or options used. Also note, that Eclipse projects can be located at any level of a git repo, so it's hard to decide how many segments to skip. Again, I think that making EGit produce "workspace patches" would help here a lot. > Changing those defaults does not prevent any other (Git) patches to be > applicable since the user can still change the initial values. Right, to be honest I consider this bugzilla more as an enhancement request since you are able to apply git patches with the current "Apply Patch" wizard. You need to be aware how some of its options work, which is not always obvious indeed.
> I'm not sure about this part. Skipping a/ b/ prefixes would be great, but what > if a git patch comes without them? Given these patches are now generated in the > command line (no "Create Patch" in EGit), there is little we can assume about > the format or options used. Also note, that Eclipse projects can be located at > any level of a git repo, so it's hard to decide how many segments to skip. > Again, I think that making EGit produce "workspace patches" would help here a > lot. > > > Changing those defaults does not prevent any other (Git) patches to be > > applicable since the user can still change the initial values. > > Right, to be honest I consider this bugzilla more as an enhancement request > since you are able to apply git patches with the current "Apply Patch" wizard. > You need to be aware how some of its options work, which is not always obvious > indeed. If you tell me that not having a/ b/ is the 80% case then I agree. If not, it's a bug and a pain when using such patches.
I would wait for the fix for bug 341036. Then we won't need any workarounds or defaults that depend on how the patch was generated. I hope that EGit guys understand the problem and will fix it ASAP. If not, we will handle it in Compare as Dani suggested.
(In reply to comment #4) > I would wait for the fix for bug 341036. Then we won't need any workarounds or > defaults that depend on how the patch was generated. > > I hope that EGit guys understand the problem and will fix it ASAP. If not, we > will handle it in Compare as Dani suggested. This does not solve the problem. Even with the fix for said bug, I can still create a simple Git patch via History view and then want to be able to easily apply it.
I think EGit could use the same wizard to generate a "workspace" patch (if possible) in both History view and Team menu. That would make life much easier for the Apply Patch wizard
> I think EGit could use the same wizard to generate a "workspace" patch (if > possible) in both History view and Team menu. That would make life much easier > for the Apply Patch wizard Sure, but as said it won't help me to apply all the patches that are already attached to some bugs. The problem with the current Git patches is that one has to try by hand how much segments one need to remove and this differs from patch to patch. E.g. in case where the real bundles are nested inside some folder, the count needs to be bigger.
> - Set ignore leading segment count to > 1 if workspace root is selected The patch wizard should be smarter and try to find the best value. See e.g. bug 361821 where a segment of 2 was needed.
(In reply to comment #4) > I hope that EGit guys understand the problem and will fix it ASAP. If not, we > will handle it in Compare as Dani suggested. Moving to M5. It seems that we have to apply the Dani's suggestion
Ping! Every time I apply a patch I have to do additional manually work.
There is a patch with pending review in EGit. It's about making EGit produce "workspace" patches. See the blocking bug or go directly to https://git.eclipse.org/r/#/c/5046/
(In reply to comment #11) > There is a patch with pending review in EGit. It's about making EGit produce > "workspace" patches. See the blocking bug or go directly to > https://git.eclipse.org/r/#/c/5046/ I'm sorry, I might not be smart enough, but how does that help with existing patches attached to bugzilla?
It doesn't. The patch for EGit fixes the root cause, which is that EGit diffing mechanism can produce only plain unified diffs and is not aware of the workspace whatsoever.
(In reply to comment #13) > It doesn't. I knew that :-) What would help as a quick remedy is if the wizard would remember the previous choices i.e. root vs. selected file and the leading segments.
(In reply to comment #14) > remember the previous choices i.e. root vs. selected file and the leading segments. The former is a result of your selection when opening the wizard ie. if the selection is empty the workspace root option is suggested, otherwise a project you had selected is used as the patch target. I wouldn't change that. The latter sounds like a good idea as long as you don't mind manually setting the combo to '0' from time to time. Or did you want it to be smart enough to check if the patch can be applied with no ignored segments and then do the same with the saved value? Or the other way round, check the saved value first, and when failed fallback to the default 0. Maybe a better solution would be having EGit provide their own Apply Patch wizard instead of tweaking the one in Compare? The first step has been made, JGit ApplyCommand is on its way (see bug 361548).
(In reply to comment #15) > (In reply to comment #14) > > remember the previous choices i.e. root vs. selected file and the leading > > segments. > > The former is a result of your selection when opening the wizard ie. if the > selection is empty the workspace root option is suggested, otherwise a project > you had selected is used as the patch target. I wouldn't change that. I see your point and you're right. Yes, for the segment it would be best if it would try to compute the correct segment option value based on the patch, but that will only work if workspace root was chosen or the correct projects selected (which is a pain). > Maybe a better solution would be having EGit provide their own Apply Patch > wizard -1 for that. There are also other features like pasting a patch in the Package Explorer, which would then have to be adapted. Let's look again at the original idea from comment 0: why can't we check whether the first segment in a git patch that contains "--- a" and/or "+++ b" are present in the workspace and if so, default to workspace root and 1 leading segment? In all other cases we behave as we currently do.
(In reply to comment #16) > Let's look again at the original idea from comment 0: why can't we check whether > the first segment in a git patch that contains "--- a" and/or "+++ b" are > present in the workspace and if so, default to workspace root and 1 leading > segment? In all other cases we behave as we currently do. Implemented in c16d33cb372c32c25cb422662c7f86d9761572b3. Marking as fixed and no longer dependent on bug 367735.
The auto-detection seems to have a problem with Git patches in e-mail format. Test Case: 1. clone the 'eclipse.platform.text' repository 2. import 'org.eclipse.jface.text' 3. open https://bugs.eclipse.org/bugs/attachment.cgi?id=212117 4. copy the patch but without the last two lines (see bug 373975 why) 5. start to apply the patch ==> no auto-detection 6. copy the patch starting at "diff --git " but without the last two lines 7. start to apply the patch ==> auto-detection works
http://git.eclipse.org/c/platform/eclipse.platform.team.git/commit/?id=1607e13426ba2abc67ae4017efab88bcb93d9733 plus parent commit. Also rebased 'integration'.
Verified in I20120314-1800.
Just thought I would share that I was just stumped by this for about an hour. The 'Apply Patch' was complaining the 'file does not exist' - the solution was to of course Ignore one leading path segment. However I see that it was set as fixed - should the 'Apply Patch' logic auto ignored the first leading path segment for me? Also please note that Help docs state you actually can not do it which is misleading: https://wiki.eclipse.org/EGit/User_Guide#Applying_Patches Thanks awesome Eclipsonauts!
(In reply to Daniel Sokolowski from comment #21) > Just thought I would share that I was just stumped by this for about an > hour. > > The 'Apply Patch' was complaining the 'file does not exist' - the solution > was to of course Ignore one leading path segment. However I see that it was > set as fixed - should the 'Apply Patch' logic auto ignored the first leading > path segment for me? I suggest to file a new bug if you can provide a reproducible test case. > Also please note that Help docs state you actually can not do it which is > misleading: https://wiki.eclipse.org/EGit/User_Guide#Applying_Patches This is still true for general Git patches. If the patch doesn't include special stuff (e.g. extensions), then it works.
