threads / patch / 33999

v9, 4 partsNew git-related helper

Subject: [PATCH v9 0/4] New git-related helper

## tl;dr

8 messages between May 31, 2013 and Jun 3, 2013. Diffs are folded; open one to read it.

replies: 7people: 2as markdown or json

Felipe Contreras· May 31, 2013, 07:46 UTC · lore
Hi,

I changed my mind, this is the same as v9 but minus some potentially controversial changes.

Felipe Contreras (4):
  Add new git-related helper to contrib
  contrib: related: add support for multiple patches
  contrib: related: add option to parse from committish
  contrib: related: parse committish like format-patch
 contrib/related/git-related | 172 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 172 insertions(+)
 create mode 100755 contrib/related/git-related
-- 
1.8.3.358.g5a91d05
Felipe Contreras· May 31, 2013, 07:46 UTC · re: Felipe Contreras · lore

[PATCH v9 1/4] Add new git-related helper to contrib

This script find people that might be interested in a patch, by going back through the history for each single hunk modified, and finding people that reviewed, acknowledge, signed, or authored the code the patch is modifying.

It does this by running 'git blame' incrementally on each hunk, and then parsing the commit message. After gathering all the relevant people, it groups them to show what exactly was their role when the participated in the development of the relevant commit, and on how many relevant commits they participated. They are only displayed if they pass a minimum threshold of participation.

For example:
  % git related 0001-remote-hg-trivial-cleanups.patch
  Felipe Contreras <felipe.contreras@gmail.com>
  Jeff King <peff@peff.net>
  Max Horn <max@quendi.de>
  Junio C Hamano <gitster@pobox.com>
Thus it can be used for 'git send-email' as a cc-cmd.

There might be some other related functions to this script, not just to be used as a cc-cmd.

Comments-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/related/git-related | 120 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 120 insertions(+)
 create mode 100755 contrib/related/git-related
Show changes to contrib/related/git-related +120 −0
diff --git a/contrib/related/git-related b/contrib/related/git-related
new file mode 100755
index 0000000..1b9b1e7
--- /dev/null
+++ b/contrib/related/git-related
@@ -0,0 +1,120 @@
+#!/usr/bin/env ruby
+
+# This script finds people that might be interested in a patch
+# usage: git related <file>
+
+$since = '5-years-ago'
+$min_percent = 10
+
+class Commit
+
+  attr_reader :persons
+
+  def initialize(id)
+    @id = id
+    @persons = []
+  end
+
+  def parse(data)
+    msg = nil
+    data.each_line do |line|
+      if not msg
+        case line
+        when /^author ([^<>]+) <(\S+)> (.+)$/
+          @persons << '%s <%s>' % [$1, $2]
+        when /^$/
+          msg = true
+        end
+      else
+        if line =~ /^(Signed-off|Reviewed|Acked)-by: ([^<>]+) <(\S+?)>$/
+          @persons << '%s <%s>' % [$2, $3]
+        end
+      end
+    end
+    @persons.uniq!
+  end
+
+end
+
+class Commits
+
+  def initialize
+    @items = {}
+  end
+
+  def size
+    @items.size
+  end
+
+  def each(&block)
+    @items.each(&block)
+  end
+
+  def import
+    return if @items.empty?
+    File.popen(%w[git cat-file --batch], 'r+') do |p|
+      p.write(@items.keys.join("\n"))
+      p.close_write
+      p.each do |line|
+        if line =~ /^(\h{40}) commit (\d+)/
+          id, len = $1, $2
+          data = p.read($2.to_i)
+          @items[id].parse(data)
+        end
+      end
+    end
+  end
+
+  def get_blame(source, start, len, from)
+    return if len == 0
+    len ||= 1
+    File.popen(['git', 'blame', '--incremental', '-C', '-C',
+               '-L', '%u,+%u' % [start, len],
+               '--since', $since, from + '^',
+               '--', source]) do |p|
+      p.each do |line|
+        if line =~ /^\h{40}/
+          id = $&
+          @items[id] = Commit.new(id)
+        end
+      end
+    end
+  end
+
+  def from_patch(file)
+    from = source = nil
+    File.open(file) do |f|
+      f.each do |line|
+        case line
+        when /^From (\h+) (.+)$/
+          from = $1
+        when /^---\s+(\S+)/
+          source = $1 != '/dev/null' ? $1[2..-1] : nil
+        when /^@@ -(\d+)(?:,(\d+))?/
+          get_blame(source, $1, $2, from) if source and from
+        end
+      end
+    end
+  end
+
+end
+
+exit 1 if ARGV.size != 1
+
+commits = Commits.new
+commits.from_patch(ARGV[0])
+commits.import
+
+count_per_person = Hash.new(0)
+
+commits.each do |id, commit|
+  commit.persons.each do |person|
+    count_per_person[person] += 1
+  end
+end
+
+count_per_person.each do |person, count|
+  percent = count.to_f * 100 / commits.size
+  next if percent < $min_percent
+  puts person
+end
-- 
1.8.3.358.g5a91d05
Felipe Contreras· May 31, 2013, 07:46 UTC · re: Felipe Contreras · lore

[PATCH v9 2/4] contrib: related: add support for multiple patches

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/related/git-related | 35 +++++++++++++++++++----------------
 1 file changed, 19 insertions(+), 16 deletions(-)
Show changes to contrib/related/git-related +19 −16
diff --git a/contrib/related/git-related b/contrib/related/git-related
index 1b9b1e7..3379982 100755
--- a/contrib/related/git-related
+++ b/contrib/related/git-related
@@ -1,7 +1,7 @@
 #!/usr/bin/env ruby
 
 # This script finds people that might be interested in a patch
-# usage: git related <file>
+# usage: git related <files>
 
 $since = '5-years-ago'
 $min_percent = 10
@@ -40,6 +40,7 @@ class Commits
 
   def initialize
     @items = {}
+    @main_commits = {}
   end
 
   def size
@@ -75,23 +76,27 @@ class Commits
       p.each do |line|
         if line =~ /^\h{40}/
           id = $&
-          @items[id] = Commit.new(id)
+          @items[id] = Commit.new(id) if not @main_commits.include?(id)
         end
       end
     end
   end
 
-  def from_patch(file)
-    from = source = nil
-    File.open(file) do |f|
-      f.each do |line|
-        case line
-        when /^From (\h+) (.+)$/
-          from = $1
-        when /^---\s+(\S+)/
-          source = $1 != '/dev/null' ? $1[2..-1] : nil
-        when /^@@ -(\d+)(?:,(\d+))?/
-          get_blame(source, $1, $2, from) if source and from
+  def from_patches(files)
+    source = nil
+    files.each do |file|
+      from = nil
+      File.open(file) do |f|
+        f.each do |line|
+          case line
+          when /^From (\h+) (.+)$/
+            from = $1
+            @main_commits[from] = true
+          when /^---\s+(\S+)/
+            source = $1 != '/dev/null' ? $1[2..-1] : nil
+          when /^@@ -(\d+)(?:,(\d+))?/
+            get_blame(source, $1, $2, from) if source and from
+          end
         end
       end
     end
@@ -99,10 +104,8 @@ class Commits
 
 end
 
-exit 1 if ARGV.size != 1
-
 commits = Commits.new
-commits.from_patch(ARGV[0])
+commits.from_patches(ARGV)
 commits.import
 
 count_per_person = Hash.new(0)
-- 
1.8.3.358.g5a91d05
Felipe Contreras· May 31, 2013, 07:46 UTC · re: Felipe Contreras · lore

[PATCH v9 3/4] contrib: related: add option to parse from committish

For example master..feature-a.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/related/git-related | 38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)
Show changes to contrib/related/git-related +36 −2
diff --git a/contrib/related/git-related b/contrib/related/git-related
index 3379982..20eb456 100755
--- a/contrib/related/git-related
+++ b/contrib/related/git-related
@@ -1,10 +1,12 @@
 #!/usr/bin/env ruby
 
 # This script finds people that might be interested in a patch
-# usage: git related <files>
+# usage: git related <files | rev-list options>
 
 $since = '5-years-ago'
 $min_percent = 10
+$files = []
+$rev_args = []
 
 class Commit
 
@@ -102,10 +104,42 @@ class Commits
     end
   end
 
+  def from_rev_args(args)
+    source = nil
+    File.popen(%w[git rev-list --reverse] + args) do |p|
+      p.each do |e|
+        id = e.chomp
+        @main_commits[id] = true
+        File.popen(%w[git show -C --oneline] + [id]) do |p|
+          p.each do |e|
+            case e
+            when /^---\s+(\S+)/
+              source = $1 != '/dev/null' ? $1[2..-1] : nil
+            when /^@@ -(\d+)(?:,(\d+))?/
+              get_blame(source, $1, $2, id) if source
+            end
+          end
+        end
+      end
+    end
+  end
+
+end
+
+ARGV.each do |e|
+  if File.exists?(e)
+    $files << e
+  else
+    $rev_args << e
+  end
 end
 
 commits = Commits.new
-commits.from_patches(ARGV)
+if $files.empty?
+  commits.from_rev_args($rev_args)
+else
+  commits.from_patches($files)
+end
 commits.import
 
 count_per_person = Hash.new(0)
-- 
1.8.3.358.g5a91d05
Junio C Hamano· Jun 3, 2013, 19:05 UTC · re: Felipe Contreras · lore

Re: [PATCH v9 3/4] contrib: related: add option to parse from committish

Felipe Contreras <felipe.contreras@gmail.com> writes:
Show 36 quoted lines
> For example master..feature-a.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  contrib/related/git-related | 38 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/related/git-related b/contrib/related/git-related
> index 3379982..20eb456 100755
> --- a/contrib/related/git-related
> +++ b/contrib/related/git-related
> @@ -1,10 +1,12 @@
>  #!/usr/bin/env ruby
>  
>  # This script finds people that might be interested in a patch
> -# usage: git related <files>
> +# usage: git related <files | rev-list options>
>  
>  $since = '5-years-ago'
>  $min_percent = 10
> +$files = []
> +$rev_args = []
>  
>  class Commit
>  
> @@ -102,10 +104,42 @@ class Commits
>      end
>    end
>  
> +  def from_rev_args(args)
> +    source = nil
> +    File.popen(%w[git rev-list --reverse] + args) do |p|
> +      p.each do |e|
> +        id = e.chomp
> +        @main_commits[id] = true
> +        File.popen(%w[git show -C --oneline] + [id]) do |p|

Is there a solid design choice behind -C, or is it just what happens to have worked for you in practice? If the former, it may want to be explained somewhere (either in the log or in the code) so that later tweaks will not break it, especially given that the invocation of blame seems to use double-C's.

Show 33 quoted lines
> +          p.each do |e|
> +            case e
> +            when /^---\s+(\S+)/
> +              source = $1 != '/dev/null' ? $1[2..-1] : nil
> +            when /^@@ -(\d+)(?:,(\d+))?/
> +              get_blame(source, $1, $2, id) if source
> +            end
> +          end
> +        end
> +      end
> +    end
> +  end
> +
> +end
> +
> +ARGV.each do |e|
> +  if File.exists?(e)
> +    $files << e
> +  else
> +    $rev_args << e
> +  end
>  end
>  
>  commits = Commits.new
> -commits.from_patches(ARGV)
> +if $files.empty?
> +  commits.from_rev_args($rev_args)
> +else
> +  commits.from_patches($files)
> +end
>  commits.import
>  
>  count_per_person = Hash.new(0)
Felipe Contreras· Jun 3, 2013, 21:05 UTC · re: Junio C Hamano · lore

Re: [PATCH v9 3/4] contrib: related: add option to parse from committish

On Mon, Jun 3, 2013 at 2:05 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 44 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> For example master..feature-a.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>>  contrib/related/git-related | 38 ++++++++++++++++++++++++++++++++++++--
>>  1 file changed, 36 insertions(+), 2 deletions(-)
>>
>> diff --git a/contrib/related/git-related b/contrib/related/git-related
>> index 3379982..20eb456 100755
>> --- a/contrib/related/git-related
>> +++ b/contrib/related/git-related
>> @@ -1,10 +1,12 @@
>>  #!/usr/bin/env ruby
>>
>>  # This script finds people that might be interested in a patch
>> -# usage: git related <files>
>> +# usage: git related <files | rev-list options>
>>
>>  $since = '5-years-ago'
>>  $min_percent = 10
>> +$files = []
>> +$rev_args = []
>>
>>  class Commit
>>
>> @@ -102,10 +104,42 @@ class Commits
>>      end
>>    end
>>
>> +  def from_rev_args(args)
>> +    source = nil
>> +    File.popen(%w[git rev-list --reverse] + args) do |p|
>> +      p.each do |e|
>> +        id = e.chomp
>> +        @main_commits[id] = true
>> +        File.popen(%w[git show -C --oneline] + [id]) do |p|
>
> Is there a solid design choice behind -C, or is it just what happens
> to have worked for you in practice?  If the former, it may want to
> be explained somewhere (either in the log or in the code) so that
> later tweaks will not break it, especially given that the invocation
> of blame seems to use double-C's.
No reason in particular.
-- 
Felipe Contreras
Felipe Contreras· May 31, 2013, 07:46 UTC · re: Felipe Contreras · lore

[PATCH v9 4/4] contrib: related: parse committish like format-patch

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/related/git-related | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
Show changes to contrib/related/git-related +15 −0
diff --git a/contrib/related/git-related b/contrib/related/git-related
index 20eb456..bded6f6 100755
--- a/contrib/related/git-related
+++ b/contrib/related/git-related
@@ -105,6 +105,21 @@ class Commits
   end
 
   def from_rev_args(args)
+    revs = []
+
+    File.popen(%w[git rev-parse --revs-only --default HEAD --symbolic] + args).each do |rev|
+      revs << rev.chomp
+    end
+
+    case revs.size
+    when 1
+      r = revs[0]
+      r = '^' + r if r[0] != '-'
+      args = [ r, 'HEAD' ]
+    else
+      args = revs
+    end
+
     source = nil
     File.popen(%w[git rev-list --reverse] + args) do |p|
       p.each do |e|
-- 
1.8.3.358.g5a91d05
Junio C Hamano· Jun 3, 2013, 19:13 UTC · re: Felipe Contreras · lore

Re: [PATCH v9 0/4] New git-related helper

Felipe Contreras <felipe.contreras@gmail.com> writes:
Show 12 quoted lines
> I changed my mind, this is the same as v9 but minus some potentially
> controversial changes.
>
> Felipe Contreras (4):
>   Add new git-related helper to contrib
>   contrib: related: add support for multiple patches
>   contrib: related: add option to parse from committish
>   contrib: related: parse committish like format-patch
>
>  contrib/related/git-related | 172 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 172 insertions(+)
>  create mode 100755 contrib/related/git-related
Replaced what was parked on 'pu' with this.

← back to recent threads