Patchwork [ANNOUNCEMENT] GDB 8.1 release branch created!

login
register
mail settings
Submitter Yao Qi
Date Feb. 1, 2018, 3:12 p.m.
Message ID <86wozwbv68.fsf@gmail.com>
Download mbox | patch
Permalink /patch/25724/
State New
Headers show

Comments

Yao Qi - Feb. 1, 2018, 3:12 p.m.
Eli Zaretskii <eliz@gnu.org> writes:

> No further comments, so I went ahead and pushed the change, without
> the __cplusplus part, to master, and then cherry-picked to
> gdb-8.1-branch.

Hi Eli,
Is it intended to include error messages in the changelog entry?  If
not, can we remove it? in the patch below,

2018-01-27  Eli Zaretskii  <eliz@gnu.org>

	Avoid compilation errors in MinGW native builds

	The error is triggered by including python-internal.h, and the
	error message is:

	     In file included from d:\usr\lib\gcc\mingw32\6.3.0\include\c++\math.h:36:0,
		      from build-gnulib/import/math.h:27,
		      from d:/usr/Python26/include/pyport.h:235,
		      from d:/usr/Python26/include/Python.h:58,
		      from python/python-internal.h:94,
		      from python/py-arch.c:24:
	     d:\usr\lib\gcc\mingw32\6.3.0\include\c++\cmath:1157:11: error: '::hypot' has not been declared
	using ::hypot;
		^~~~~

	This happens because Python headers define 'hypot' to expand t
	'_hypot' in the Windows builds.
	* python/python-internal.h (_hypot) [__MINGW32__]: Define back to
	'hypoth'.  This avoids a compilation error.
Eli Zaretskii - Feb. 1, 2018, 4:27 p.m.
> From: Yao Qi <qiyaoltc@gmail.com>
> Cc: simon.marchi@ericsson.com,  gdb-patches@sourceware.org
> Date: Thu, 01 Feb 2018 15:12:47 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > No further comments, so I went ahead and pushed the change, without
> > the __cplusplus part, to master, and then cherry-picked to
> > gdb-8.1-branch.
> 
> Hi Eli,
> Is it intended to include error messages in the changelog entry?  If
> not, can we remove it? in the patch below,

Maybe I'm missing something, but isn't the commit log entry supposed
to have the same text as the ChangeLog for that change?

And what's wrong with having error messages in ChangeLog's, anyway?
Yao Qi - Feb. 1, 2018, 4:51 p.m.
On Thu, Feb 1, 2018 at 4:27 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> Maybe I'm missing something, but isn't the commit log entry supposed
> to have the same text as the ChangeLog for that change?

My understanding is that ChangeLog entry should be copied at the end
of commit log

https://sourceware.org/gdb/wiki/ContributionChecklist#Properly_formatted_commit_messages

>
> And what's wrong with having error messages in ChangeLog's, anyway?

Each entry in ChangeLog describes the changes to the code,
https://www.gnu.org/prep/standards/standards.html#Change-Log-Concepts
IMO, error messages are out of the scope of what ChangeLog is intended
to describe.  Also, I've never seen such ChangeLog entry (with error
messages) before.
Eli Zaretskii - Feb. 1, 2018, 5:34 p.m.
> From: Yao Qi <qiyaoltc@gmail.com>
> Date: Thu, 1 Feb 2018 16:51:36 +0000
> Cc: Simon Marchi <simon.marchi@ericsson.com>, GDB Patches <gdb-patches@sourceware.org>
> 
> On Thu, Feb 1, 2018 at 4:27 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> > Maybe I'm missing something, but isn't the commit log entry supposed
> > to have the same text as the ChangeLog for that change?
> 
> My understanding is that ChangeLog entry should be copied at the end
> of commit log

I believe I did that, except that I moved the motivation for the
change before the ChangeLog header, as it belongs to the preamble of
the Git log.  (In ChangeLog, there's no subdivision of the entry.)

> > And what's wrong with having error messages in ChangeLog's, anyway?
> 
> Each entry in ChangeLog describes the changes to the code,
> https://www.gnu.org/prep/standards/standards.html#Change-Log-Concepts
> IMO, error messages are out of the scope of what ChangeLog is intended
> to describe.

This is changing, and some GNU projects already do it differently.  I
believe the next version of standards.texi will change the
recommendations to be more lax.  And using different style for each
project is a pain.  Is GDB really so rigid as to not tolerate this?

Please note that the error message I cited is just part of the
motivation for the change, so we are really talking about including
the motivation in the ChangeLog.

> Also, I've never seen such ChangeLog entry (with error messages)
> before.

That's not necessarily a reason for rejecting such entries.

So: what is the project's take on this, if there is one?
Yao Qi - Feb. 1, 2018, 9:32 p.m.
On Thu, Feb 1, 2018 at 5:34 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Yao Qi <qiyaoltc@gmail.com>
>> Date: Thu, 1 Feb 2018 16:51:36 +0000
>> Cc: Simon Marchi <simon.marchi@ericsson.com>, GDB Patches <gdb-patches@sourceware.org>
>>
>> On Thu, Feb 1, 2018 at 4:27 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> > Maybe I'm missing something, but isn't the commit log entry supposed
>> > to have the same text as the ChangeLog for that change?
>>
>> My understanding is that ChangeLog entry should be copied at the end
>> of commit log
>
> I believe I did that, except that I moved the motivation for the
> change before the ChangeLog header, as it belongs to the preamble of
> the Git log.  (In ChangeLog, there's no subdivision of the entry.)
>

It is absolutely fine to put the reason of the change in the commit
log, but it is questionable to put commit log into ChangeLog.  At
least, nowadays, "commit log" is more than ChangeLog entry.  This
is the convention we followed for several years.

>> > And what's wrong with having error messages in ChangeLog's, anyway?
>>
>> Each entry in ChangeLog describes the changes to the code,
>> https://www.gnu.org/prep/standards/standards.html#Change-Log-Concepts
>> IMO, error messages are out of the scope of what ChangeLog is intended
>> to describe.
>
> This is changing, and some GNU projects already do it differently.  I
> believe the next version of standards.texi will change the
> recommendations to be more lax.  And using different style for each
> project is a pain.  Is GDB really so rigid as to not tolerate this?
>

GDB is evolving, so I am completely fine to changes, but you
made the change of the way of writing changelog, which is not
discussed before.

> Please note that the error message I cited is just part of the
> motivation for the change, so we are really talking about including
> the motivation in the ChangeLog.
>

Yes, do we want to change the rule of writing ChangeLog, so that
we can/should put the motivation for the change?  Is the motivation
mandatory in the changelog or optional?

>> Also, I've never seen such ChangeLog entry (with error messages)
>> before.
>
> That's not necessarily a reason for rejecting such entries.
>
> So: what is the project's take on this, if there is one?

We need a clear rule on writing changelog, otherwise how do we
write changelog or review patches?
Eli Zaretskii - Feb. 2, 2018, 8:34 a.m.
> From: Yao Qi <qiyaoltc@gmail.com>
> Date: Thu, 1 Feb 2018 21:32:13 +0000
> Cc: Simon Marchi <simon.marchi@ericsson.com>, GDB Patches <gdb-patches@sourceware.org>
> 
> > Please note that the error message I cited is just part of the
> > motivation for the change, so we are really talking about including
> > the motivation in the ChangeLog.
> >
> 
> Yes, do we want to change the rule of writing ChangeLog, so that
> we can/should put the motivation for the change?  Is the motivation
> mandatory in the changelog or optional?
> 
> >> Also, I've never seen such ChangeLog entry (with error messages)
> >> before.
> >
> > That's not necessarily a reason for rejecting such entries.
> >
> > So: what is the project's take on this, if there is one?
> 
> We need a clear rule on writing changelog, otherwise how do we
> write changelog or review patches?

I agree.  I hope Someone(TM) will answer these questions.
Joel Brobecker - Feb. 2, 2018, 3:53 p.m.
> > We need a clear rule on writing changelog, otherwise how do we
> > write changelog or review patches?
> 
> I agree.  I hope Someone(TM) will answer these questions.

Well, if it were me, I would stop using ChangeLog files with
immediate effect.

The purpose of ChangeLog files, as I understand it, is to describe
the "what changed", and is aimed at people who download a given
release and want to know what changed (because they do not have
access to the version control information).

While the above might have been essential 20 or 25 years ago,
it is trivial for anyone today to have access to the change
history of our program. Two options:
  - git clone our public repository; or...
  - ... if they don't want to use git, they can use the gitweb interface.

I think that this is an acceptable way for us to provide the log
of changes that were made.

Currently, the situation is that we all pay a price in term of time
wasted populating those files, and when we add the time collectively
spent on those, I cannot imagine the few people's convenience would
be worth that much effort.

I vote for we stop using those ChangeLog files right now. We can
still have them as part of the ChangeLog if people want -- although,
I am also open to considering the pros and cons of keeping it,
but I think they might not be worth the investiment either.
Simon Marchi - Feb. 2, 2018, 4:19 p.m.
On 2018-02-02 10:53, Joel Brobecker wrote:
> Well, if it were me, I would stop using ChangeLog files with
> immediate effect.
> 
> The purpose of ChangeLog files, as I understand it, is to describe
> the "what changed", and is aimed at people who download a given
> release and want to know what changed (because they do not have
> access to the version control information).
> 
> While the above might have been essential 20 or 25 years ago,
> it is trivial for anyone today to have access to the change
> history of our program. Two options:
>   - git clone our public repository; or...
>   - ... if they don't want to use git, they can use the gitweb 
> interface.
> 
> I think that this is an acceptable way for us to provide the log
> of changes that were made.
> 
> Currently, the situation is that we all pay a price in term of time
> wasted populating those files, and when we add the time collectively
> spent on those, I cannot imagine the few people's convenience would
> be worth that much effort.
> 
> I vote for we stop using those ChangeLog files right now. We can
> still have them as part of the ChangeLog if people want -- although,
> I am also open to considering the pros and cons of keeping it,
> but I think they might not be worth the investiment either.

I agree about the time consumed producing ChangeLog entries.  I have to 
admit that it sometimes discourages me to do structural changes that 
would affect a lot files/functions.  I also feel like I'm filling some 
administrative paperwork, which is not my favorite way of spending my 
time.

Maintaining an internal port of GDB, one use I have for the ChangeLog is 
to find what commit removed a given function.  Let's say I rebase our 
code and find out a function we use was removed, grepping for that 
function would find the ChangeLog entry, leading me to the commit, which 
helps in fixing the code.  However, I recently discovered "git blame 
--reverse", which allows to ask git about which commit removed a 
particular line.  So the ChangeLog is not really required for that use 
case.

Since with git the whole history is available offline, I think the 
usefulness of this kind of ChangeLogs is not as big as it used to be.

To be fair I'll also state a pro I see for the ChangeLog.  When I write 
the ChangeLog entry, it makes me look at my change a last time in 
details and sometimes find some additional issues I might not have found 
otherwise.

It might be worth starting a new thread to get the attention of 
everybody to discuss this issue.

Simon
Joseph Myers - Feb. 2, 2018, 5:41 p.m.
On Fri, 2 Feb 2018, Joel Brobecker wrote:

> > > We need a clear rule on writing changelog, otherwise how do we
> > > write changelog or review patches?
> > 
> > I agree.  I hope Someone(TM) will answer these questions.
> 
> Well, if it were me, I would stop using ChangeLog files with
> immediate effect.

Feel free to join the discussion on bug-standards (which has been going 
for several months) of eliminating the GNU Coding Standards requirement 
for the ChangeLog *format*.  (It's already been allowed by the GCS for 
over 20 years to generate the ChangeLog *file* at release time for the 
release tarball from version control history rather than having it checked 
in, but then of course you need to make sure that commit messages are 
appropriately formatted to derive ChangeLog entries - so I consider that 
the format, not the file, is the main issue.)

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 5c3338f..40f4008 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -144,23 +144,6 @@ 
 
 2018-01-27  Eli Zaretskii  <eliz@gnu.org>
 
-	Avoid compilation errors in MinGW native builds
-
-	The error is triggered by including python-internal.h, and the
-	error message is:
-
-	     In file included from d:\usr\lib\gcc\mingw32\6.3.0\include\c++\math.h:36:0,
-		      from build-gnulib/import/math.h:27,
-		      from d:/usr/Python26/include/pyport.h:235,
-		      from d:/usr/Python26/include/Python.h:58,
-		      from python/python-internal.h:94,
-		      from python/py-arch.c:24:
-	     d:\usr\lib\gcc\mingw32\6.3.0\include\c++\cmath:1157:11: error: '::hypot' has not been declared
-	using ::hypot;
-		^~~~~
-
-	This happens because Python headers define 'hypot' to expand t
-	'_hypot' in the Windows builds.
 	* python/python-internal.h (_hypot) [__MINGW32__]: Define back to
 	'hypoth'.  This avoids a compilation error.