Uploaded image for project: 'Maven SCM'
  1. Maven SCM
  2. SCM-606

UnsupportedOperationException on blame GIT

Details

    • Wish
    • Status: Closed
    • Minor
    • Resolution: Fixed
    • 1.4
    • 1.5
    • None
    • Patch

    Description

      Sometimes, running the blame command for GIT return an error for exit code. In this case, Maven SCM throw an
      UnsupportedOperationException :

      Caused by: java.lang.UnsupportedOperationException
      at org.apache.maven.scm.provider.git.gitexe.command.blame.GitBlameCommand.executeBlameCommand(GitBlameCommand.java:46)
      etc...
      

      Is it possible to have the same way that others providers (SVN or TFS for example) which is to return a
      result (with a success to false) ?

      In other word replace :

        throw new UnsupportedOperationException();
      

      by :

      return new BlameScmResult(cl.toString(), "The git command failed.", stderr.getOutput(), false);
      

      Another reason is that we do not know that this UnsupportedOperationException may be raised because is
      a RuntimeException.

      A similar problem : http://jira.codehaus.org/browse/SONARPLUGINS-618

      Do you agree with that ?

      Attachments

        1. GitBlame_UnsupportedOperationException.patch
          1.0 kB
          Fabien Bousquet
        2. SCM606_bis.patch
          4 kB
          Fabien Bousquet

        Issue Links

          Activity

            struberg Mark Struberg added a comment -

            the way to go is to implement this feature for the maven-scm-provider-gitexe also.
            Will do this in the next week.

            struberg Mark Struberg added a comment - the way to go is to implement this feature for the maven-scm-provider-gitexe also. Will do this in the next week.

            This feature (blame for gitexe) is already implemented (See

            org.apache.maven.scm.provider.git.gitexe.command.blame.GitBlameCommand

            ) . I use it already and it is operational.
            Is what I'm wrong?

            fafanoulele Fabien Bousquet added a comment - This feature (blame for gitexe) is already implemented (See org.apache.maven.scm.provider.git.gitexe.command.blame.GitBlameCommand ) . I use it already and it is operational. Is what I'm wrong?
            struberg Mark Struberg added a comment -

            I c.

            Did more research and it looks like we currently only split the output of git-blame on tabulator characters('\t'). But here on my installation (Fedora14, kernel-2.6.38, git-1.7.4) I get spaces as separator.

            struberg Mark Struberg added a comment - I c. Did more research and it looks like we currently only split the output of git-blame on tabulator characters('\t'). But here on my installation (Fedora14, kernel-2.6.38, git-1.7.4) I get spaces as separator.

            My configurations tested :

            • git-1.7.3 on Windows Vista
            • git-1.7.4 on Windows Vista
            • git-1.7.0.4 on a Linux

            For these configurations, blame output is split on tabulator character.

            I test on GitHub projects.

            fafanoulele Fabien Bousquet added a comment - My configurations tested : git-1.7.3 on Windows Vista git-1.7.4 on Windows Vista git-1.7.0.4 on a Linux For these configurations, blame output is split on tabulator character. I test on GitHub projects.
            struberg Mark Struberg added a comment -

            maybe it's even depending on the Locale, ...

            A few of my findings so far indicate that I'll need to rewrite the whole GitLogConsumer to use regexp (will commit test cases + fix later):

            git-blame outputs spaces over here:

            ^e670863 (Mark Struberg 2007-11-22 22:16:03 +0100 17) * KIND, either express or implied. See the License for the
            ^e670863 (Mark Struberg 2007-11-22 22:16:03 +0100 18) * specific language governing permissions and limitations

            ------------------

            git-blame with user.name="Mark (work)" (bracelets in the user.name)

            66e621ed (Mark (work) 2011-02-11 19:01:15 +0100 1) testcontent

            ------------------

            git-blame on a fresh file (not committed yet...)

            00000000 (Not Committed Yet 2011-02-11 18:21:25 +0100 1) [core]
            00000000 (Not Committed Yet 2011-02-11 18:21:25 +0100 2) repositoryformatversion = 0

            ------------------

            will of course also test a git-blame on an empty file and things.

            If you have ideas of additional things to check, then please post

            LieGrue,
            strub

            struberg Mark Struberg added a comment - maybe it's even depending on the Locale, ... A few of my findings so far indicate that I'll need to rewrite the whole GitLogConsumer to use regexp (will commit test cases + fix later): git-blame outputs spaces over here: ^e670863 (Mark Struberg 2007-11-22 22:16:03 +0100 17) * KIND, either express or implied. See the License for the ^e670863 (Mark Struberg 2007-11-22 22:16:03 +0100 18) * specific language governing permissions and limitations ------------------ git-blame with user.name="Mark (work)" (bracelets in the user.name) 66e621ed (Mark (work) 2011-02-11 19:01:15 +0100 1) testcontent ------------------ git-blame on a fresh file (not committed yet...) 00000000 (Not Committed Yet 2011-02-11 18:21:25 +0100 1) [core] 00000000 (Not Committed Yet 2011-02-11 18:21:25 +0100 2) repositoryformatversion = 0 ------------------ will of course also test a git-blame on an empty file and things. If you have ideas of additional things to check, then please post LieGrue, strub
            struberg Mark Struberg added a comment -

            I think we should redo the parser and switch to the porcelain option finally
            $> git blame -p

            But that will take some time. So I will probably implement a quick fix for the current parser upfront and later re-implement it with -p.

            struberg Mark Struberg added a comment - I think we should redo the parser and switch to the porcelain option finally $> git blame -p But that will take some time. So I will probably implement a quick fix for the current parser upfront and later re-implement it with -p.
            struberg Mark Struberg added a comment -

            Fabien, could you please attach a
            $> git blame --porcelain > git-blame2.out
            ?
            Please use a repo/file which is ideally under ALv2. But at least not under some problematic license and content, need it for inclusion in our test suite

            struberg Mark Struberg added a comment - Fabien, could you please attach a $> git blame --porcelain > git-blame2.out ? Please use a repo/file which is ideally under ALv2. But at least not under some problematic license and content, need it for inclusion in our test suite
            struberg Mark Struberg added a comment -

            btw, not sure if this was intentionally, but what should scm:blame report? the author or the committer? Remember that this might be different in git!

            struberg Mark Struberg added a comment - btw, not sure if this was intentionally, but what should scm:blame report? the author or the committer? Remember that this might be different in git!
            struberg Mark Struberg added a comment -

            committed, review would be appreciated

            struberg Mark Struberg added a comment - committed, review would be appreciated

            Thanks !!!

            I can't review now. So I will review and test tomorrow.

            For difference between author and commiter, I don't understand. What is this difference ?

            For my case, I want the last developer which has modified the line (for each line).

            fafanoulele Fabien Bousquet added a comment - Thanks !!! I can't review now. So I will review and test tomorrow. For difference between author and commiter, I don't understand. What is this difference ? For my case, I want the last developer which has modified the line (for each line).
            struberg Mark Struberg added a comment -

            In git author and committer can be different. If I submit a linux patch and Linus takes and git-applys it to the kernel tree and push it upstream, then you will find Linus as committer and me as author...

            struberg Mark Struberg added a comment - In git author and committer can be different. If I submit a linux patch and Linus takes and git-applys it to the kernel tree and push it upstream, then you will find Linus as committer and me as author...

            I have a problem with date and author which may be falses.

            The header line (author, commitor etc...) is followed by the following information at least once for each commit (See http://www.kernel.org/pub/software/scm/git/docs/git-blame.html).
            So when a release have already listed in blame output, date and author can be falses.
            I propose a patch which stores release informations (author and date) and uses it when possible.

            Mark, Could you review ?

            fafanoulele Fabien Bousquet added a comment - I have a problem with date and author which may be falses. The header line (author, commitor etc...) is followed by the following information at least once for each commit (See http://www.kernel.org/pub/software/scm/git/docs/git-blame.html ). So when a release have already listed in blame output, date and author can be falses. I propose a patch which stores release informations (author and date) and uses it when possible. Mark, Could you review ?
            struberg Mark Struberg added a comment -

            Fabien,
            did you really look at the porcelain description of git-blame?
            Please see the section "THE PORCELAIN FORMAT"

            according to this (and from looking at the C sources) the committer and committer-time must always exist.

            "This header line is followed by the following information at least once for each commit:
            he author name ("author"), email ("author-mail"), time ("author-time"), and timezone "author-tz"); similarly for committer."

            Can you please provide a sample with both
            $> git blame -p
            $> git blame -c -l
            So we can extend our unit tests!
            Which git version do you use and which OS version are you running on?

            txs and LieGrue,
            strub

            struberg Mark Struberg added a comment - Fabien, did you really look at the porcelain description of git-blame? Please see the section "THE PORCELAIN FORMAT" according to this (and from looking at the C sources) the committer and committer-time must always exist. "This header line is followed by the following information at least once for each commit: he author name ("author"), email ("author-mail"), time ("author-time"), and timezone "author-tz"); similarly for committer." Can you please provide a sample with both $> git blame -p $> git blame -c -l So we can extend our unit tests! Which git version do you use and which OS version are you running on? txs and LieGrue, strub
            struberg Mark Struberg added a comment -

            I guess I c what you mean:

            "followed by the following information at least once for each commit"
            That's exactly the reason why we cannot clean the author and date information!
            If two or more following lines (right behind each other) got changed in the same commit, then the author+committer info will NOT be contained in the blame info!

            E.g. the first line looks like this

            342345234aef32323214 2 2 3
            author
            author-time
            ...
            committer-time
            ..
            filename myfile.txt
            summary thesummaryIentered
            \t[tab] changed content of this line

            but the following 2 lines (summary=3 = last number of the original sha-1 line)
            342345234aef32323214 3 3
            \t[tab] changed content of the 2nd line
            342345234aef32323214 4 4
            \t[tab] changed content of the 3nd line

            Means they will NOT contain the author info again, but 'reuse' the info above.

            struberg Mark Struberg added a comment - I guess I c what you mean: "followed by the following information at least once for each commit" That's exactly the reason why we cannot clean the author and date information! If two or more following lines (right behind each other) got changed in the same commit, then the author+committer info will NOT be contained in the blame info! E.g. the first line looks like this 342345234aef32323214 2 2 3 author author-time ... committer-time .. filename myfile.txt summary thesummaryIentered \t [tab] changed content of this line but the following 2 lines (summary=3 = last number of the original sha-1 line) 342345234aef32323214 3 3 \t [tab] changed content of the 2nd line 342345234aef32323214 4 4 \t [tab] changed content of the 3nd line Means they will NOT contain the author info again, but 'reuse' the info above.

            I added an assert in test in SCM606_bis.patch for show the bug.
            For test on git-blame.out, on line 27 (of source file), it's :

            e670863b2b03e158c59f34af1fee20f91b2bd852 27 26 4
            	import org.apache.maven.scm.ScmVersion;
            

            The author+committer infos are NOT repeated because of "followed by the following information at least once for each commit".

            So, I agree with you when 'reuse' the info above, but when this information is not repeated, these informations (commiter and release date) must be retrieve in the previous blames lines.

            fafanoulele Fabien Bousquet added a comment - I added an assert in test in SCM606_bis.patch for show the bug. For test on git-blame.out, on line 27 (of source file), it's : e670863b2b03e158c59f34af1fee20f91b2bd852 27 26 4 import org.apache.maven.scm.ScmVersion; The author+committer infos are NOT repeated because of "followed by the following information at least once for each commit". So, I agree with you when 'reuse' the info above, but when this information is not repeated, these informations (commiter and release date) must be retrieve in the previous blames lines.
            struberg Mark Struberg added a comment -

            oki I now see what you mean. It only gets written once for each commit. But if n non-consequent lines get transfered, then the committer/author/date information will not get repeated again (but only for the first time a commit sha-1 gets used in a file)
            Currently working on a fix.

            struberg Mark Struberg added a comment - oki I now see what you mean. It only gets written once for each commit. But if n non-consequent lines get transfered, then the committer/author/date information will not get repeated again (but only for the first time a commit sha-1 gets used in a file) Currently working on a fix.
            struberg Mark Struberg added a comment -

            should now finally be fixed!

            struberg Mark Struberg added a comment - should now finally be fixed!

            Thanks Mark for this fix !

            fafanoulele Fabien Bousquet added a comment - Thanks Mark for this fix !

            People

              struberg Mark Struberg
              fafanoulele Fabien Bousquet
              Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: