public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 0/2] make using mklog a little nicer
@ 2014-04-29  2:12 tsaunders
  2014-04-29  2:13 ` [PATCH 1/2] teach mklog to get name / email from git config when available tsaunders
  2014-04-29  2:16 ` [PATCH 2/2] allow running mklog as a filter tsaunders
  0 siblings, 2 replies; 25+ messages in thread
From: tsaunders @ 2014-04-29  2:12 UTC (permalink / raw)
  To: gcc-patches; +Cc: dnovillo

Hi,

 These patches make it a little nicer to use mklog, but there not particularly  pretty.

Trev

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/2] teach mklog to get name / email from git config when available
  2014-04-29  2:12 [PATCH 0/2] make using mklog a little nicer tsaunders
@ 2014-04-29  2:13 ` tsaunders
  2014-04-29  5:12   ` Yury Gribov
                     ` (2 more replies)
  2014-04-29  2:16 ` [PATCH 2/2] allow running mklog as a filter tsaunders
  1 sibling, 3 replies; 25+ messages in thread
From: tsaunders @ 2014-04-29  2:13 UTC (permalink / raw)
  To: gcc-patches; +Cc: dnovillo, Trevor Saunders

From: Trevor Saunders <tbsaunde@mozilla.com>

Hi,

 finger gives the wrong data on my machines, and while I could fix it it seems
nicer to use what's configured for the git repo we're in if any, that way you
can use different defaults from the rest of the machine.

Trev

contrib/ChangeLog:

2014-04-28  Trevor Saunders  <tbsaunde@mozilla.com>

	* mklog: if in a git checkout try to get name and email from git.
---
 contrib/mklog | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/contrib/mklog b/contrib/mklog
index fb489b0..5f5d98e 100755
--- a/contrib/mklog
+++ b/contrib/mklog
@@ -38,6 +38,20 @@ $gcc_root = $0;
 $gcc_root =~ s/[^\\\/]+$/../;
 chdir $gcc_root;
 
+# if this is a git tree then take name and email from the git configuration
+if (-d .git) {
+  $gitname = `git config user.name`;
+  chomp($gitname);
+  if ($gitname) {
+	  $name = $gitname;
+  }
+
+  $gitaddr = `git config user.email`;
+  chomp($gitaddr);
+  if ($gitaddr) {
+	  $addr = $gitaddr;
+  }
+}
 
 #-----------------------------------------------------------------------------
 # Program starts here. You should not need to edit anything below this
-- 
2.0.0.rc0

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 2/2] allow running mklog as a filter
  2014-04-29  2:12 [PATCH 0/2] make using mklog a little nicer tsaunders
  2014-04-29  2:13 ` [PATCH 1/2] teach mklog to get name / email from git config when available tsaunders
@ 2014-04-29  2:16 ` tsaunders
  2014-04-29  8:28   ` Yury Gribov
  1 sibling, 1 reply; 25+ messages in thread
From: tsaunders @ 2014-04-29  2:16 UTC (permalink / raw)
  To: gcc-patches; +Cc: dnovillo, Trevor Saunders

From: Trevor Saunders <tbsaunde@mozilla.com>

Hi,

I'd like to be able to suggest a git prepare-committ-msg hook, that uses this
at some point to populate the commit message at some point.  This doesn't do
that, but its a step in that direction, what would remain is just writing a
shell script to pipe git diff to mklog and then put the result in the commit
template.  Until that's done this atleast makes it so you don't need to
interact with a diff file at any point.

Trev


2014-04-28  Trevor Saunders
<tbsaunde@mozilla.com>

	* mklog: if reading the patch on stdin write the ChangeLog to stdout.
---
 contrib/mklog | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/contrib/mklog b/contrib/mklog
index 5f5d98e..dfdd2a4 100755
--- a/contrib/mklog
+++ b/contrib/mklog
@@ -277,8 +277,16 @@ foreach my $clname (keys %cl_entries) {
 	print CLFILE "$clname:\n\n$hdrline\n\n$cl_entries{$clname}\n";
 }
 
-# Concatenate the ChangeLog template and the original .diff file.
-system ("cat $diff >>$temp && mv $temp $diff") == 0
-    or die "Could not add the ChangeLog entry to $diff";
+# XXX We should probably accept /dev/stdin or maybe magic autodetection of
+# being supposed to get the patch from stdin.
+#
+# In any case if we got the diff on stdin then write the ChangeLog to stdout.
+if ($diff == "-") {
+	system("cat $temp");
+} else {
+	# Concatenate the ChangeLog template and the original .diff file.
+	system ("cat $diff >>$temp && mv $temp $diff") == 0
+		or die "Could not add the ChangeLog entry to $diff";
+}
 
 exit 0;
-- 
2.0.0.rc0

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] teach mklog to get name / email from git config when available
  2014-04-29  2:13 ` [PATCH 1/2] teach mklog to get name / email from git config when available tsaunders
@ 2014-04-29  5:12   ` Yury Gribov
  2014-04-29 10:15     ` Trevor Saunders
  2014-04-29 15:47   ` segher
  2014-05-09 14:48   ` Diego Novillo
  2 siblings, 1 reply; 25+ messages in thread
From: Yury Gribov @ 2014-04-29  5:12 UTC (permalink / raw)
  To: tsaunders, gcc-patches; +Cc: dnovillo, Trevor Saunders

Hi Trevor,

I think this looks rather useful.

 > +if (-d .git) {

What about moving default name/addr (with finger, etc.) to else branch?

 > +  chomp($gitname);
 > +  chomp($gitaddr);

Missing whites before (.

-Y

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/2] allow running mklog as a filter
  2014-04-29  2:16 ` [PATCH 2/2] allow running mklog as a filter tsaunders
@ 2014-04-29  8:28   ` Yury Gribov
  2014-04-29 10:17     ` Trevor Saunders
  0 siblings, 1 reply; 25+ messages in thread
From: Yury Gribov @ 2014-04-29  8:28 UTC (permalink / raw)
  To: tsaunders, gcc-patches; +Cc: dnovillo, Trevor Saunders

> +# XXX We should probably accept /dev/stdin or maybe magic autodetection of
> +# being supposed to get the patch from stdin.
> +#

Can we just set $diff to '-' if @ARGV is empty?

> +# In any case if we got the diff on stdin then write the ChangeLog to stdout.

Hm, this is breaks semantics: you only dump CL instead of CL+diff just 
because diff comes from stdin. Perhaps we could append contents of 
@diff_lines here?

> +if ($diff == "-") {

This will work but 'eq' is preferred way to compare strings.

-Y

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] teach mklog to get name / email from git config when available
  2014-04-29  5:12   ` Yury Gribov
@ 2014-04-29 10:15     ` Trevor Saunders
  2014-04-29 12:27       ` Yury Gribov
  0 siblings, 1 reply; 25+ messages in thread
From: Trevor Saunders @ 2014-04-29 10:15 UTC (permalink / raw)
  To: Yury Gribov; +Cc: gcc-patches, dnovillo, Trevor Saunders

[-- Attachment #1: Type: text/plain, Size: 566 bytes --]

On Tue, Apr 29, 2014 at 09:08:50AM +0400, Yury Gribov wrote:
> Hi Trevor,
> 
> I think this looks rather useful.

sounds great!

> > +if (-d .git) {
> 
> What about moving default name/addr (with finger, etc.) to else branch?

Well, consider the case git doesn't know, that said I could do

if (-d .git) {
// check git
}

if (!name) {
// finger to get name
}

if (!email) {
// finger stuff to get email
}

or something like that.

Trev

> 
> > +  chomp($gitname);
> > +  chomp($gitaddr);
> 
> Missing whites before (.
> 
> -Y
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/2] allow running mklog as a filter
  2014-04-29  8:28   ` Yury Gribov
@ 2014-04-29 10:17     ` Trevor Saunders
  2014-04-29 12:17       ` Yury Gribov
  2014-05-09 15:09       ` Diego Novillo
  0 siblings, 2 replies; 25+ messages in thread
From: Trevor Saunders @ 2014-04-29 10:17 UTC (permalink / raw)
  To: Yury Gribov; +Cc: gcc-patches, dnovillo, Trevor Saunders

[-- Attachment #1: Type: text/plain, Size: 988 bytes --]

On Tue, Apr 29, 2014 at 09:18:39AM +0400, Yury Gribov wrote:
> >+# XXX We should probably accept /dev/stdin or maybe magic autodetection of
> >+# being supposed to get the patch from stdin.
> >+#
> 
> Can we just set $diff to '-' if @ARGV is empty?

sgtm

> >+# In any case if we got the diff on stdin then write the ChangeLog to stdout.
> 
> Hm, this is breaks semantics: you only dump CL instead of CL+diff just
> because diff comes from stdin. Perhaps we could append contents of
> @diff_lines here?

 I agree stdin gets different semantics than other files but I think
 that's sort of ok because it means you generated the patch somehow so
 presumably you can do that again if you need the patch.  Writing the
 diff to stdout seems possible, but atleast for my use cases its
 annoying, but I guess I'm open to adding a flag or something.

> >+if ($diff == "-") {
> 
> This will work but 'eq' is preferred way to compare strings.

oh perl

Trev

> 
> -Y

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/2] allow running mklog as a filter
  2014-04-29 10:17     ` Trevor Saunders
@ 2014-04-29 12:17       ` Yury Gribov
  2014-05-09 15:09       ` Diego Novillo
  1 sibling, 0 replies; 25+ messages in thread
From: Yury Gribov @ 2014-04-29 12:17 UTC (permalink / raw)
  To: Trevor Saunders; +Cc: gcc-patches, dnovillo, Trevor Saunders

>  I agree stdin gets different semantics than other files but I think
>  that's sort of ok because it means you generated the patch somehow so
>  presumably you can do that again if you need the patch.

True but this would be a surpising behavior for ordinary Linux user. We 
can wait for Diego's opinion.

>Writing the
>  diff to stdout seems possible, but atleast for my use cases its
>  annoying, but I guess I'm open to adding a flag or something.

You could pipe output through sed or something to extract CL-part:
$ mklog ... | sed -ne '/^diff --git/q; p'

-Y

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] teach mklog to get name / email from git config when available
  2014-04-29 10:15     ` Trevor Saunders
@ 2014-04-29 12:27       ` Yury Gribov
  0 siblings, 0 replies; 25+ messages in thread
From: Yury Gribov @ 2014-04-29 12:27 UTC (permalink / raw)
  To: Trevor Saunders; +Cc: gcc-patches, dnovillo, Trevor Saunders

On 04/29/2014 02:06 PM, Trevor Saunders wrote:
>>> +if (-d .git) {
>>
>> What about moving default name/addr (with finger, etc.) to else branch?
>
> Well, consider the case git doesn't know

I wonder whether it's ok to force user to configure git before running 
mklog...

-Y

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] teach mklog to get name / email from git config when available
  2014-04-29  2:13 ` [PATCH 1/2] teach mklog to get name / email from git config when available tsaunders
  2014-04-29  5:12   ` Yury Gribov
@ 2014-04-29 15:47   ` segher
  2014-04-29 18:03     ` Trevor Saunders
  2014-04-29 19:24     ` Peter Bergner
  2014-05-09 14:48   ` Diego Novillo
  2 siblings, 2 replies; 25+ messages in thread
From: segher @ 2014-04-29 15:47 UTC (permalink / raw)
  To: tsaunders; +Cc: gcc-patches, dnovillo, Trevor Saunders

> +# if this is a git tree then take name and email from the git configuration
> +if (-d .git) {
> +  $gitname = `git config user.name`;
> +  chomp($gitname);
> +  if ($gitname) {
> +	  $name = $gitname;
> +  }
> +
> +  $gitaddr = `git config user.email`;
> +  chomp($gitaddr);
> +  if ($gitaddr) {
> +	  $addr = $gitaddr;
> +  }
> +}

"-d .git" is, erm, not so great.

How about something like

sub get_git_config {
  my $res = `git config --get @_`;
  return undef if $?;
  chomp $res;
  return $res;
}


Segher

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] teach mklog to get name / email from git config when available
  2014-04-29 15:47   ` segher
@ 2014-04-29 18:03     ` Trevor Saunders
  2014-04-29 23:00       ` Segher Boessenkool
  2014-04-29 19:24     ` Peter Bergner
  1 sibling, 1 reply; 25+ messages in thread
From: Trevor Saunders @ 2014-04-29 18:03 UTC (permalink / raw)
  To: segher; +Cc: gcc-patches, dnovillo, Trevor Saunders

[-- Attachment #1: Type: text/plain, Size: 953 bytes --]

On Tue, Apr 29, 2014 at 10:39:00AM -0500, segher@kernel.crashing.org wrote:
> > +# if this is a git tree then take name and email from the git configuration
> > +if (-d .git) {
> > +  $gitname = `git config user.name`;
> > +  chomp($gitname);
> > +  if ($gitname) {
> > +	  $name = $gitname;
> > +  }
> > +
> > +  $gitaddr = `git config user.email`;
> > +  chomp($gitaddr);
> > +  if ($gitaddr) {
> > +	  $addr = $gitaddr;
> > +  }
> > +}
> 
> "-d .git" is, erm, not so great.

yeah, the only reason I was willing to do it is the script already
requires being run at top level, but that annoys me too.

> How about something like

It has the same issue that it'll activate ina svn checkout if you have
git configured, but I'm not sure that's a case worth caring about.

Trev

> 
> sub get_git_config {
>   my $res = `git config --get @_`;
>   return undef if $?;
>   chomp $res;
>   return $res;
> }
> 
> 
> Segher

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] teach mklog to get name / email from git config when available
  2014-04-29 15:47   ` segher
  2014-04-29 18:03     ` Trevor Saunders
@ 2014-04-29 19:24     ` Peter Bergner
  1 sibling, 0 replies; 25+ messages in thread
From: Peter Bergner @ 2014-04-29 19:24 UTC (permalink / raw)
  To: segher; +Cc: tsaunders, gcc-patches, dnovillo, Trevor Saunders

On Tue, 2014-04-29 at 10:39 -0500, segher@kernel.crashing.org wrote:
> > +# if this is a git tree then take name and email from the git configuration
> > +if (-d .git) {
> > +  $gitname = `git config user.name`;
> > +  chomp($gitname);
> > +  if ($gitname) {
> > +	  $name = $gitname;
> > +  }
> > +
> > +  $gitaddr = `git config user.email`;
> > +  chomp($gitaddr);
> > +  if ($gitaddr) {
> > +	  $addr = $gitaddr;
> > +  }
> > +}
> 
> "-d .git" is, erm, not so great.
> 
> How about something like
> 
> sub get_git_config {
>   my $res = `git config --get @_`;
>   return undef if $?;
>   chomp $res;
>   return $res;
> }

I've always used a hacked up version that reads a ~/.mklog config
file for the name and email address to use.  Ala:


[bergner@otta ~]$ cat ~/.mklog 
NAME = Peter Bergner
EMAIL = bergner@vnet.ibm.com


my $conf = "$ENV{HOME}/.mklog";
if (-f "$conf")
  {
    open (CONF, "$conf")
         or die "Could not open file '$conf' for reading: $!\n";
    while (<CONF>)
      {
        if (m/^\s*NAME\s*=\s*(.*)\s*$/)
          {
            $name = $1;
          }
        elsif (m/^\s*EMAIL\s*=\s*(.*)\s*$/)
          {
            $addr = $1;
          }
      }
  }



Peter



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] teach mklog to get name / email from git config when available
  2014-04-29 18:03     ` Trevor Saunders
@ 2014-04-29 23:00       ` Segher Boessenkool
  0 siblings, 0 replies; 25+ messages in thread
From: Segher Boessenkool @ 2014-04-29 23:00 UTC (permalink / raw)
  To: Trevor Saunders; +Cc: gcc-patches, dnovillo, Trevor Saunders

> > > +if (-d .git) {

> > "-d .git" is, erm, not so great.
> 
> yeah, the only reason I was willing to do it is the script already
> requires being run at top level, but that annoys me too.

That, but also it is equivalent to

if ((-d $_)."git") {

which is probably not what you wanted ;-)

"use warnings" complains loudly and "use strict" plain refuses to
compile this.

> > How about something like
> 
> It has the same issue that it'll activate ina svn checkout if you have
> git configured, but I'm not sure that's a case worth caring about.

If you have configured your name and email for git (for this repo or
globally) it probably is the name/email you want to use, better than
the finger output (but an override might be handy, dunno).


Segher

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] teach mklog to get name / email from git config when available
  2014-04-29  2:13 ` [PATCH 1/2] teach mklog to get name / email from git config when available tsaunders
  2014-04-29  5:12   ` Yury Gribov
  2014-04-29 15:47   ` segher
@ 2014-05-09 14:48   ` Diego Novillo
  2014-11-20 16:28     ` Tom de Vries
  2 siblings, 1 reply; 25+ messages in thread
From: Diego Novillo @ 2014-05-09 14:48 UTC (permalink / raw)
  To: Trevor Saunders; +Cc: gcc-patches, Trevor Saunders

On Mon, Apr 28, 2014 at 10:11 PM, <tsaunders@mozilla.com> wrote:

> 2014-04-28  Trevor Saunders  <tbsaunde@mozilla.com>
>
>         * mklog: if in a git checkout try to get name and email from git.
> ---
>  contrib/mklog | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/contrib/mklog b/contrib/mklog
> index fb489b0..5f5d98e 100755
> --- a/contrib/mklog
> +++ b/contrib/mklog
> @@ -38,6 +38,20 @@ $gcc_root = $0;
>  $gcc_root =~ s/[^\\\/]+$/../;
>  chdir $gcc_root;
>
> +# if this is a git tree then take name and email from the git configuration
> +if (-d .git) {

I would probably use git config directly here. It would work with both
git and svn checkouts (if you have a global .git configuration). But
testing for .git is fine with me as well.

I like Peter's idea of having a ~/.mklog file to override. This would
work for both svn and git checkouts.


Diego.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/2] allow running mklog as a filter
  2014-04-29 10:17     ` Trevor Saunders
  2014-04-29 12:17       ` Yury Gribov
@ 2014-05-09 15:09       ` Diego Novillo
  2014-07-21  8:25         ` Yury Gribov
  1 sibling, 1 reply; 25+ messages in thread
From: Diego Novillo @ 2014-05-09 15:09 UTC (permalink / raw)
  To: Trevor Saunders; +Cc: Yury Gribov, gcc-patches, Trevor Saunders

On Tue, Apr 29, 2014 at 6:16 AM, Trevor Saunders <tsaunders@mozilla.com> wrote:

>> >+# In any case if we got the diff on stdin then write the ChangeLog to stdout.
>>
>> Hm, this is breaks semantics: you only dump CL instead of CL+diff just
>> because diff comes from stdin. Perhaps we could append contents of
>> @diff_lines here?
>
>  I agree stdin gets different semantics than other files but I think
>  that's sort of ok because it means you generated the patch somehow so
>  presumably you can do that again if you need the patch.  Writing the
>  diff to stdout seems possible, but atleast for my use cases its
>  annoying, but I guess I'm open to adding a flag or something.

I slightly prefer the semantics that gets me just the ChangeLog. The
workflow I'm envisioning is:

$ cat mypatch | mklog > message.txt

Diego.

>
>> >+if ($diff == "-") {
>>
>> This will work but 'eq' is preferred way to compare strings.
>
> oh perl

Indeed. I've always regretted writing this in perl.  A python version
would be so much more pleasant to maintain.

OK with Yuri's suggestion of assuming '-' when ARGV is empty.


Diego.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/2] allow running mklog as a filter
  2014-05-09 15:09       ` Diego Novillo
@ 2014-07-21  8:25         ` Yury Gribov
  2014-07-21  9:25           ` Trevor Saunders
  0 siblings, 1 reply; 25+ messages in thread
From: Yury Gribov @ 2014-07-21  8:25 UTC (permalink / raw)
  To: Diego Novillo, Trevor Saunders; +Cc: gcc-patches, Trevor Saunders

On 05/09/2014 07:09 PM, Diego Novillo wrote:
 > I slightly prefer the semantics that gets me just the ChangeLog.
 > The workflow I'm envisioning is:

I've commited both patches in r212883 and r12884. Mklog now runs as a 
filter and prints generated ChangeLog to stdout instead of modifying the 
patchfile.

 > OK with Yuri's suggestion of assuming '-' when ARGV is empty.

I didn't change this because currently empty ARGV is used for printing 
help message (similar to some other scripts in contrib/ folder).

-Y

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/2] allow running mklog as a filter
  2014-07-21  8:25         ` Yury Gribov
@ 2014-07-21  9:25           ` Trevor Saunders
  2014-07-28  6:47             ` Yury Gribov
  0 siblings, 1 reply; 25+ messages in thread
From: Trevor Saunders @ 2014-07-21  9:25 UTC (permalink / raw)
  To: Yury Gribov; +Cc: Diego Novillo, gcc-patches, Trevor Saunders

On Mon, Jul 21, 2014 at 11:49:05AM +0400, Yury Gribov wrote:
> On 05/09/2014 07:09 PM, Diego Novillo wrote:
> > I slightly prefer the semantics that gets me just the ChangeLog.
> > The workflow I'm envisioning is:
> 
> I've commited both patches in r212883 and r12884. Mklog now runs as a filter
> and prints generated ChangeLog to stdout instead of modifying the patchfile.

thanks for taking care of it!

> > OK with Yuri's suggestion of assuming '-' when ARGV is empty.
> 
> I didn't change this because currently empty ARGV is used for printing help
> message (similar to some other scripts in contrib/ folder).

I'm not really sure which is the better UI, but I'd rather time be spent
on better automatic change log generation.  I may or may not hope we'll
eventually have a mklog that can autogenerate most ChangeLogs and then
people will have a hard time arguing they're useful.

Trev

> 
> -Y

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/2] allow running mklog as a filter
  2014-07-21  9:25           ` Trevor Saunders
@ 2014-07-28  6:47             ` Yury Gribov
  2014-07-28 11:17               ` Trevor Saunders
  0 siblings, 1 reply; 25+ messages in thread
From: Yury Gribov @ 2014-07-28  6:47 UTC (permalink / raw)
  To: Trevor Saunders; +Cc: Diego Novillo, gcc-patches, Trevor Saunders

On 07/21/2014 12:55 PM, Trevor Saunders wrote:
> I'm not really sure which is the better UI,
> but I'd rather time be spent
> on better automatic change log generation.

Yeah. Do you have some particular complaints btw?

>I may or may not hope we'll
> eventually have a mklog that can autogenerate most ChangeLogs and then
> people will have a hard time arguing they're useful.

I wonder how many GCC developers actually use the script (our team does).

-Y

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/2] allow running mklog as a filter
  2014-07-28  6:47             ` Yury Gribov
@ 2014-07-28 11:17               ` Trevor Saunders
  2014-07-28 14:36                 ` Yury Gribov
  0 siblings, 1 reply; 25+ messages in thread
From: Trevor Saunders @ 2014-07-28 11:17 UTC (permalink / raw)
  To: Yury Gribov; +Cc: Diego Novillo, gcc-patches, Trevor Saunders

On Mon, Jul 28, 2014 at 10:42:51AM +0400, Yury Gribov wrote:
> On 07/21/2014 12:55 PM, Trevor Saunders wrote:
> >I'm not really sure which is the better UI,
> >but I'd rather time be spent
> >on better automatic change log generation.
> 
> Yeah. Do you have some particular complaints btw?

I haven't actually used it in a while, but istr there's an issue where
if you change the prototype of a function mklog makes an entry for the
previous function.

Automatically inserting likewise where it would be appropriate would be
nice, but fairly hard I suspect.

> >I may or may not hope we'll
> >eventually have a mklog that can autogenerate most ChangeLogs and then
> >people will have a hard time arguing they're useful.
> 
> I wonder how many GCC developers actually use the script (our team does).

Not sure.  I actually haven't used it ina while because I've mostly been
generating patches where it was easier to do one entry for a ton of
files so it was easier to generate that with git diff and sed.

Trev

> 
> -Y

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/2] allow running mklog as a filter
  2014-07-28 11:17               ` Trevor Saunders
@ 2014-07-28 14:36                 ` Yury Gribov
  2014-07-28 14:50                   ` Trevor Saunders
  0 siblings, 1 reply; 25+ messages in thread
From: Yury Gribov @ 2014-07-28 14:36 UTC (permalink / raw)
  To: Trevor Saunders; +Cc: Diego Novillo, gcc-patches, Trevor Saunders

On 07/28/2014 03:01 PM, Trevor Saunders wrote:
>> Yeah. Do you have some particular complaints btw?
>
> I haven't actually used it in a while, but istr there's an issue where
> if you change the prototype of a function mklog makes an entry for the
> previous function.

I think this is because mklog relies on markers generated by diff -p
(like @@ -96,20 +108,22 @@ bitmap_descriptor (const char *file, int 
line, const char *function)).
Diff will indeed report old name instead of the new one.

> Automatically inserting likewise where it would be appropriate would be
> nice, but fairly hard I suspect.

This depends on the actual use-case. For tests and deleted files this
should be relatively straightforward but probably not very useful.

-Y

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/2] allow running mklog as a filter
  2014-07-28 14:36                 ` Yury Gribov
@ 2014-07-28 14:50                   ` Trevor Saunders
  0 siblings, 0 replies; 25+ messages in thread
From: Trevor Saunders @ 2014-07-28 14:50 UTC (permalink / raw)
  To: Yury Gribov; +Cc: Diego Novillo, gcc-patches, Trevor Saunders

On Mon, Jul 28, 2014 at 06:29:22PM +0400, Yury Gribov wrote:
> On 07/28/2014 03:01 PM, Trevor Saunders wrote:
> >>Yeah. Do you have some particular complaints btw?
> >
> >I haven't actually used it in a while, but istr there's an issue where
> >if you change the prototype of a function mklog makes an entry for the
> >previous function.
> 
> I think this is because mklog relies on markers generated by diff -p
> (like @@ -96,20 +108,22 @@ bitmap_descriptor (const char *file, int line,
> const char *function)).
> Diff will indeed report old name instead of the new one.

that was my guess as well, I imagine it can be fixed with enough work,
but given my limited use of mklog so far I haven't tried to fix it.

> >Automatically inserting likewise where it would be appropriate would be
> >nice, but fairly hard I suspect.
> 
> This depends on the actual use-case. For tests and deleted files this
> should be relatively straightforward but probably not very useful.

yeah, I forget exactly what mklog does now for created / removed files,
but making it take care of those on its own would be nice.  You might
also be able to deal with function renaming fairly well with some
effort.

Trev

> 
> -Y

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] teach mklog to get name / email from git config when available
  2014-05-09 14:48   ` Diego Novillo
@ 2014-11-20 16:28     ` Tom de Vries
  2014-11-20 16:48       ` Segher Boessenkool
  0 siblings, 1 reply; 25+ messages in thread
From: Tom de Vries @ 2014-11-20 16:28 UTC (permalink / raw)
  To: Diego Novillo; +Cc: Peter Bergner, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 536 bytes --]

On 09-05-14 16:47, Diego Novillo wrote:
> I would probably use git config directly here. It would work with both
> git and svn checkouts (if you have a global .git configuration). But
> testing for .git is fine with me as well.
>
> I like Peter's idea of having a ~/.mklog file to override. This would
> work for both svn and git checkouts.
>

Diego,

this patch implements both:
- it uses the ~/.mklog file proposed by Peter
- in absence of a ~/.mklog file, it uses git config, also when not in a git
   repository

OK?

Thanks,
- Tom

[-- Attachment #2: 0001-Use-.mklog-name-and-email-settings-or-git-settings.patch --]
[-- Type: text/x-patch, Size: 2208 bytes --]

2014-11-20  Tom de Vries  <tom@codesourcery.com>
	    Peter Bergner  <bergner@vnet.ibm.com>

	* mklog: Handle .mklog.  Use git setting independent of presence .git
	directory.
---
 contrib/mklog | 56 +++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 35 insertions(+), 21 deletions(-)

diff --git a/contrib/mklog b/contrib/mklog
index 840f6f8..abbf0af 100755
--- a/contrib/mklog
+++ b/contrib/mklog
@@ -29,32 +29,46 @@
 use File::Temp;
 use File::Copy qw(cp mv);
 
-# Change these settings to reflect your profile.
-$username = $ENV{'USER'};
-$name = `finger $username | grep -o 'Name: .*'`;
-@n = split(/: /, $name);
-$name = $n[1]; chop($name);
-$addr = $username . "\@my.domain.org";
 $date = `date +%Y-%m-%d`; chop ($date);
 
+$dot_mklog_format_msg =
+    "The .mklog format is:\n"
+    . "NAME = ...\n"
+    . "EMAIL = ...\n";
+
+# Create a .mklog to reflect your profile, if necessary.
+my $conf = "$ENV{HOME}/.mklog";
+if (-f "$conf") {
+    open (CONF, "$conf")
+	or die "Could not open file '$conf' for reading: $!\n";
+    while (<CONF>) {
+	if (m/^\s*NAME\s*=\s*(.*)\s*$/)	{
+	    $name = $1;
+	} elsif (m/^\s*EMAIL\s*=\s*(.*)\s*$/) {
+	    $addr = $1;
+	}
+    }
+    if (!($name && $addr)) {
+	die "Could not read .mklog settings.\n"
+	    . $dot_mklog_format_msg;
+    }
+} else {
+    $name = `git config user.name`;
+    chomp($name);
+    $addr = `git config user.email`;
+    chomp($addr);
+
+    if (!($name && $addr)) {
+	die "Could not read git user.name and user.email settings.\n"
+	    . "Please add missing git settings, or create a .mklog file in"
+	    . " $ENV{HOME}.\n"
+	    . $dot_mklog_format_msg;
+    }
+}
+
 $gcc_root = $0;
 $gcc_root =~ s/[^\\\/]+$/../;
 
-# if this is a git tree then take name and email from the git configuration
-if (-d "$gcc_root/.git") {
-  $gitname = `git config user.name`;
-  chomp($gitname);
-  if ($gitname) {
-	  $name = $gitname;
-  }
-
-  $gitaddr = `git config user.email`;
-  chomp($gitaddr);
-  if ($gitaddr) {
-	  $addr = $gitaddr;
-  }
-}
-
 #-----------------------------------------------------------------------------
 # Program starts here. You should not need to edit anything below this
 # line.
-- 
1.9.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] teach mklog to get name / email from git config when available
  2014-11-20 16:28     ` Tom de Vries
@ 2014-11-20 16:48       ` Segher Boessenkool
  2014-11-20 22:34         ` Tom de Vries
  0 siblings, 1 reply; 25+ messages in thread
From: Segher Boessenkool @ 2014-11-20 16:48 UTC (permalink / raw)
  To: Tom de Vries; +Cc: Diego Novillo, Peter Bergner, gcc-patches

On Thu, Nov 20, 2014 at 05:22:20PM +0100, Tom de Vries wrote:
> +my $conf = "$ENV{HOME}/.mklog";
> +if (-f "$conf") {
> +    open (CONF, "$conf")
> +	or die "Could not open file '$conf' for reading: $!\n";
> +    while (<CONF>) {
> +	if (m/^\s*NAME\s*=\s*(.*)\s*$/)	{

The final \s* never matches anything since the .* gobbles up everything.
Use .*? if you really want to get rid of the trailing whitespace.


Segher

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] teach mklog to get name / email from git config when available
  2014-11-20 16:48       ` Segher Boessenkool
@ 2014-11-20 22:34         ` Tom de Vries
  2014-11-25 16:09           ` Diego Novillo
  0 siblings, 1 reply; 25+ messages in thread
From: Tom de Vries @ 2014-11-20 22:34 UTC (permalink / raw)
  To: Segher Boessenkool, Diego Novillo; +Cc: Peter Bergner, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 555 bytes --]

On 20-11-14 17:43, Segher Boessenkool wrote:
> On Thu, Nov 20, 2014 at 05:22:20PM +0100, Tom de Vries wrote:
>> +my $conf = "$ENV{HOME}/.mklog";
>> +if (-f "$conf") {
>> +    open (CONF, "$conf")
>> +	or die "Could not open file '$conf' for reading: $!\n";
>> +    while (<CONF>) {
>> +	if (m/^\s*NAME\s*=\s*(.*)\s*$/)	{
>
> The final \s* never matches anything since the .* gobbles up everything.
> Use .*? if you really want to get rid of the trailing whitespace.
>

Thanks for spotting that, patch updated.

OK for trunk?

Thanks,
- Tom

>
> Segher
>


[-- Attachment #2: 0001-Use-.mklog-name-and-email-settings-or-git-settings.patch --]
[-- Type: text/x-patch, Size: 2210 bytes --]

2014-11-20  Tom de Vries  <tom@codesourcery.com>
	    Peter Bergner  <bergner@vnet.ibm.com>

	* mklog: Handle .mklog.  Use git setting independent of presence .git
	directory.
---
 contrib/mklog | 56 +++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 35 insertions(+), 21 deletions(-)

diff --git a/contrib/mklog b/contrib/mklog
index 840f6f8..f7974a7 100755
--- a/contrib/mklog
+++ b/contrib/mklog
@@ -29,32 +29,46 @@
 use File::Temp;
 use File::Copy qw(cp mv);
 
-# Change these settings to reflect your profile.
-$username = $ENV{'USER'};
-$name = `finger $username | grep -o 'Name: .*'`;
-@n = split(/: /, $name);
-$name = $n[1]; chop($name);
-$addr = $username . "\@my.domain.org";
 $date = `date +%Y-%m-%d`; chop ($date);
 
+$dot_mklog_format_msg =
+    "The .mklog format is:\n"
+    . "NAME = ...\n"
+    . "EMAIL = ...\n";
+
+# Create a .mklog to reflect your profile, if necessary.
+my $conf = "$ENV{HOME}/.mklog";
+if (-f "$conf") {
+    open (CONF, "$conf")
+	or die "Could not open file '$conf' for reading: $!\n";
+    while (<CONF>) {
+	if (m/^\s*NAME\s*=\s*(.*?)\s*$/) {
+	    $name = $1;
+	} elsif (m/^\s*EMAIL\s*=\s*(.*?)\s*$/) {
+	    $addr = $1;
+	}
+    }
+    if (!($name && $addr)) {
+	die "Could not read .mklog settings.\n"
+	    . $dot_mklog_format_msg;
+    }
+} else {
+    $name = `git config user.name`;
+    chomp($name);
+    $addr = `git config user.email`;
+    chomp($addr);
+
+    if (!($name && $addr)) {
+	die "Could not read git user.name and user.email settings.\n"
+	    . "Please add missing git settings, or create a .mklog file in"
+	    . " $ENV{HOME}.\n"
+	    . $dot_mklog_format_msg;
+    }
+}
+
 $gcc_root = $0;
 $gcc_root =~ s/[^\\\/]+$/../;
 
-# if this is a git tree then take name and email from the git configuration
-if (-d "$gcc_root/.git") {
-  $gitname = `git config user.name`;
-  chomp($gitname);
-  if ($gitname) {
-	  $name = $gitname;
-  }
-
-  $gitaddr = `git config user.email`;
-  chomp($gitaddr);
-  if ($gitaddr) {
-	  $addr = $gitaddr;
-  }
-}
-
 #-----------------------------------------------------------------------------
 # Program starts here. You should not need to edit anything below this
 # line.
-- 
1.9.1


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/2] teach mklog to get name / email from git config when available
  2014-11-20 22:34         ` Tom de Vries
@ 2014-11-25 16:09           ` Diego Novillo
  0 siblings, 0 replies; 25+ messages in thread
From: Diego Novillo @ 2014-11-25 16:09 UTC (permalink / raw)
  To: Tom de Vries, Segher Boessenkool; +Cc: Peter Bergner, gcc-patches



On 20/11/2014, 16:51 , Tom de Vries wrote:

> OK for trunk?

This is fine. Thanks.


Diego.

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2014-11-25 15:17 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-29  2:12 [PATCH 0/2] make using mklog a little nicer tsaunders
2014-04-29  2:13 ` [PATCH 1/2] teach mklog to get name / email from git config when available tsaunders
2014-04-29  5:12   ` Yury Gribov
2014-04-29 10:15     ` Trevor Saunders
2014-04-29 12:27       ` Yury Gribov
2014-04-29 15:47   ` segher
2014-04-29 18:03     ` Trevor Saunders
2014-04-29 23:00       ` Segher Boessenkool
2014-04-29 19:24     ` Peter Bergner
2014-05-09 14:48   ` Diego Novillo
2014-11-20 16:28     ` Tom de Vries
2014-11-20 16:48       ` Segher Boessenkool
2014-11-20 22:34         ` Tom de Vries
2014-11-25 16:09           ` Diego Novillo
2014-04-29  2:16 ` [PATCH 2/2] allow running mklog as a filter tsaunders
2014-04-29  8:28   ` Yury Gribov
2014-04-29 10:17     ` Trevor Saunders
2014-04-29 12:17       ` Yury Gribov
2014-05-09 15:09       ` Diego Novillo
2014-07-21  8:25         ` Yury Gribov
2014-07-21  9:25           ` Trevor Saunders
2014-07-28  6:47             ` Yury Gribov
2014-07-28 11:17               ` Trevor Saunders
2014-07-28 14:36                 ` Yury Gribov
2014-07-28 14:50                   ` Trevor Saunders

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).