Bugzilla – Full Text Bug Listing
| Summary: | NPE when trying to create a patch | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Technology] EGit | Reporter: | Kaloyan Raev <kaloyan> | ||||
| Component: | Core | Assignee: | Matthias Sohn <matthias.sohn> | ||||
| Status: | VERIFIED FIXED | QA Contact: | |||||
| Severity: | critical | ||||||
| Priority: | P3 | CC: | alex.blewitt, angvoz.dev, caniszczyk, cdtdoug, daniel_megert, ed, henrich.kraemer, jamesblackburn+eclipse, marc.khouzam, matthias.sohn, mober.at+eclipse, nicolas.bros, pwebster, remy.suen | ||||
| Version: | 0.12 | ||||||
| Target Milestone: | --- | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Kaloyan Raev
Were all the files selected currently in your Eclipse workspace? Edwin, sorry for the delayed reply. I found out that I don't have the habit to call Pull after I push changes into Git. In this case I called Create Patch after several such commits. This tries to generate quite a big patch involving all the files that are part from these commits. However, I remember I removed some of the projects that were involved in those commits before I called Create Patch. I see two issues: 1) NPE is always bad - it does not give a clue to users what's wrong. 2) I expect that Create Patch will create a diff only for the change of the commit under selection. Instead it creates a diff for all commit from the selected one to HEAD, which may involve more commits and hence create larger patch than desired. Kevin, sorry for confusing your name. It's quite late in my time zone. May be it's time to go to bed. By "removed some of the projects" I meant I removed them from the workspace, but they were still in the local Git repo. I'm seeing this also but on Linux with Egit 1.0.0201106071213 *** Bug 347031 has been marked as a duplicate of this bug. *** http://wiki.eclipse.org/Development_Resources/Handling_Git_Contributions suggests that we'll just pull commits from a user's fork of the repo on github. However given we won't have access to gerrit, contributors will be going through bugzilla and likely submitting patches for review (commits on github may be transient, so patches are important for code review and persistence). => I guess this is pretty blocker for project's adopting git. I can't reproduce this using a git patch. Removing blocking to CDT's move. I also just ran into this -- creating the patch in non-git format threw an NPE, creating in git format worked but placed a lot of junk into the patch which was totally unrelated to my contribution.
For a contributor, creating a patch for a git-based project seems to be a major obstacle. Here's the hurdles I ran into:
1.) Didn't find "Team > Create patch" in the first place. From CVS, I was used
to creating a patch between my workspace changes and the repo -- this isn't
possible in egit as per the Indigo version.
2.) Found "Create Patch" in the history view, but it creates a patch for the
selected commit rather than my workspace, so I need to commit first.
3.) Committed my change into my local repo, but "Create patch" generated TONS
of junk in addition to my change, although I had just pulled and thought
that my local repo as well as the workspace should be in sync.
4.) Created a local branch for my changes and committed the change into my own
branch. Now "Create Patch" finally produced something useful, although again
the patch included an additional unrelated change which wasn't part of my
branch nor my commit.
Part of the reason for moving to git was making it easier for contributors to contribute. Today this isn't the case. I'd really appreciate introducing a "Create Patch" again which creates the patch from the Synchronize View or from selection of workspace changes. That's good enough for most simple changes and hides a lot of unnecessary complexity.
(In reply to comment #8) A couple of comments after I've been using EGit for a little over a month. > 1.) Didn't find "Team > Create patch" in the first place. From CVS, I was used > to creating a patch between my workspace changes and the repo -- this isn't > possible in egit as per the Indigo version. I miss that too. But I've started asking myself if maybe we should not be using patches as much as we used to and focus on using local commits. > 2.) Found "Create Patch" in the history view, but it creates a patch for the > selected commit rather than my workspace, so I need to commit first. What I do is create a branch for that work and then I don't mind committing before creating the patch since it is restricted to that branch. I think this pays off in the end because commits are harder to loose than patches; they can be rebased into other branches, or cherry-picked into them. Another way to mimic the old CVS workflow is to do the commit, create the patch, then "reset soft" on the branch to undo the commit but leave the code changes uncommitted in the workspace. This is a hack to me. > 3.) Committed my change into my local repo, but "Create patch" generated TONS > of junk in addition to my change, although I had just pulled and thought > that my local repo as well as the workspace should be in sync. I've never had this problem. I don't use Pull myself. I use fetch then rebase. I was told it was cleaner. But maybe pull does the same in Egit? > 4.) Created a local branch for my changes and committed the change into my own > branch. Now "Create Patch" finally produced something useful, although > again > the patch included an additional unrelated change which wasn't part of my > branch nor my commit. Weird. You may want to check if the patch matches the "Compare with each other" result when you select your commit and the one preceding it on the same branch. (In reply to comment #9) > What I do is create a branch for that work and then I don't mind committing Sounds like a workable workaround in the short term, is this already mentioned in the CDT Contributor Guide ? Sure it makes sense to educate contributors, and perhaps some / many may appreciate the git way of tracking changes eventually. Perhaps we'll all move to Gerrit eventually... But even then, for simple quick patches (eg just fix a typo somewhere), creating a patch for a selection out out of the workspace would seem easiest to me, especially combined with support for Eclipse-SourceReferences to import a released version into my workspace. I'm seeing the same stack trace while creating a patch in GIT format. *** Bug 354495 has been marked as a duplicate of this bug. *** This NPE occurs when you try to create a patch for a change that affects files not in the Eclipse workspace. What do you think about this solution? * Wizard warns that files selected outside the Eclipse workspace will not be included unless "Export in Git patch format" is selected * Files outside Eclipse workspace are ignored when creating Eclipse-style patch instead of NPE Matthias: I'm willing to work on this unless you have already started on it. (In reply to comment #14) > * Files outside Eclipse workspace are ignored when creating Eclipse-style > patch instead of NPE This would be a no-go in my opinion as that would imply the patch does not actually reflect the contents of the commit in question. So do you think the Finish button should just be disabled with a wizard page error shown? (In reply to comment #16) > So do you think the Finish button should just be disabled with a wizard page > error shown? The wizard either a) needs to support this use case or b) precheck the 'Export in Git patch format' checkbox (which should then be moved to the first page). Actually, I am curious as to why it cannot create such a diff. Right now all the paths in the patch are relative to the project. If the commit affects multiple projects, the patch will not apply correctly. What is the reason behind the algorithm not choosing to use a repository-relative path for creating the diff? I think it is because it is using the Eclipse diff format which requires Eclipse workspace-relative paths which fails for paths in the commit that cannot be resolved to an IResource handle. (In reply to comment #11) > I'm seeing the same stack trace while creating a patch in GIT format. I am surprised to hear this, Edward. The Git formatted patch should not go down this code path. (In reply to comment #18) > I think it is because it is using the Eclipse diff format which requires > Eclipse workspace-relative paths which fails for > paths in the commit that cannot be resolved to an IResource handle. I think the wizard should always generate repository-relative paths unless the user specifies that it that the patch's root should be the workspace (one of the radio button options in the second page of the CVS wizard). If there's an issue with applying the patch because the projects are a few folders deep in the repository, they can be filtered from the patch wizard by asking it to ignore a few leading path segments. The most important thing is for the patch to reflect the content of the selected commit. Users can afford to click a bit and check some paths and possibly apply the patch manually from the command line or by copying and pasting. They cannot however afford to lose data. If anyone on the CC list has an opinion on what should happen, please speak up. I don't want to lead Kevin down a path that only makes me happy. :) I am seeing this issue as well. Learning more about the git way has been interesting to me. But as an occasional contributor there is quite a learning curve to understand what is needed. As of now I have not been able to create a patch for a planned contribution to ECF bug 297742. For sporadic contributors a simple and foolproof (patch) wizard appears very much needed to me. (In reply to comment #19) > (In reply to comment #11) > > I'm seeing the same stack trace while creating a patch in GIT format. > > I am surprised to hear this, Edward. The Git formatted patch should not go down > this code path. I seem to have a habit of causing trouble. I have yet to succeed in creating a patch that can subsequently be applied, despite four or five attempts. It may work, but not in the way 'CVS' users expect. (In reply to comment #20) > I am seeing this issue as well. > Learning more about the git way has been interesting to me. But as an > occasional contributor there is quite a learning curve to understand what is > needed. I started using EGit and concluded that it just wasn't ready. I've persevered and am now coming to mostly like EGit. You may find http://wiki.eclipse.org/MDT/OCL/Dev/EGit useful; it tries to present things from an Eclipse committer's point of view. The crunch is that until you really understand the History View, you cannot hope to use EGit successfully. (In reply to comment #21) > I have yet to succeed in creating a patch that can subsequently be applied, > despite four or five attempts. It may work, but not in the way 'CVS' users > expect. I can create patches (that are not of the Git format), it's just that they are project-relative so I have to select the right project for the wizard to apply it to (unlike the CVS patches that can be made to work at the workspace level). If the patch spans N projects, then you have to apply it N times because they are all project-relative and as such, all believe the files are src/x/y/z/. So you first have to apply it to one project, while filtering out files for the second project that was affected. Then you apply it a second time, filtering out files from the first project that was affected. For another patch-related request, see bug 354800. (In reply to comment #8) > 4.) Created a local branch for my changes and committed the change into my own > branch. Now "Create Patch" finally produced something useful, although > again > the patch included an additional unrelated change which wasn't part of my > branch nor my commit. > This worked for me as well to recover from the NPE. I did not see extra stuff in the patch. This is a critical bug which is 4 months old. Any plans to fix this? (In reply to comment #24) > This is a critical bug which is 4 months old. Any plans to fix this? I think Benny's recent changes has made this obsolete. I don't have problems anymore with the original scenario I reported for bug 354495 anyway with 1.1.0.201108241714. (In reply to comment #25) > (In reply to comment #24) > > This is a critical bug which is 4 months old. Any plans to fix this? > > I think Benny's recent changes has made this obsolete. > > I don't have problems anymore with the original scenario I reported for bug > 354495 anyway with 1.1.0.201108241714. I can confirm that it works for me using 1.1.0.201108251914. Unfortunately the latest N-builds have bug 355691, which is a blocker for me. (In reply to comment #26) > (In reply to comment #25) > > (In reply to comment #24) > > > This is a critical bug which is 4 months old. Any plans to fix this? > > > > I think Benny's recent changes has made this obsolete. > > > > I don't have problems anymore with the original scenario I reported for bug > > 354495 anyway with 1.1.0.201108241714. > > I can confirm that it works for me using 1.1.0.201108251914. Unfortunately the > latest N-builds have bug 355691, which is a blocker for me. Could you retry as bug 355691 has been closed ? > Could you retry as bug 355691 has been closed ?
Works for me using Verified in 1.1.0.201108281957.
(In reply to comment #28) > > Could you retry as bug 355691 has been closed ? > > Works for me using Verified in 1.1.0.201108281957. In better words: I verified in EGit 1.1.0.201108281957 that it works. Dani verified that this is fixed *** Bug 360529 has been marked as a duplicate of this bug. *** |