From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: merge-commits policy (was: bug#8675: lisp_string_width and strings wider than INT_MAX) Date: Tue, 17 May 2011 06:30:50 -0400 Message-ID: References: <4DD0B118.1040205@cs.ucla.edu> <83pqnjdxga.fsf@gnu.org> <4DD0B70F.7090801@cs.ucla.edu> <83mxindqmp.fsf@gnu.org> Reply-To: Eli Zaretskii NNTP-Posting-Host: lo.gmane.org X-Trace: dough.gmane.org 1305628263 15841 80.91.229.12 (17 May 2011 10:31:03 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Tue, 17 May 2011 10:31:03 +0000 (UTC) Cc: eggert@cs.ucla.edu, emacs-devel@gnu.org To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Tue May 17 12:30:58 2011 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([140.186.70.17]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1QMHYH-0005ch-Uq for ged-emacs-devel@m.gmane.org; Tue, 17 May 2011 12:30:58 +0200 Original-Received: from localhost ([::1]:56658 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QMHYG-0002hZ-JE for ged-emacs-devel@m.gmane.org; Tue, 17 May 2011 06:30:56 -0400 Original-Received: from eggs.gnu.org ([140.186.70.92]:45176) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QMHYD-0002hQ-8E for emacs-devel@gnu.org; Tue, 17 May 2011 06:30:54 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QMHYB-0008Di-Kz for emacs-devel@gnu.org; Tue, 17 May 2011 06:30:53 -0400 Original-Received: from fencepost.gnu.org ([140.186.70.10]:57367) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QMHYB-0008De-H1 for emacs-devel@gnu.org; Tue, 17 May 2011 06:30:51 -0400 Original-Received: from eliz by fencepost.gnu.org with local (Exim 4.71) (envelope-from ) id 1QMHYA-0006Ht-E2; Tue, 17 May 2011 06:30:50 -0400 In-reply-to: (message from Stefan Monnier on Mon, 16 May 2011 13:48:33 -0300) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-Received-From: 140.186.70.10 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:139459 Archived-At: This issue came up as a spin-off of discussing the proposed patch for bug #8675. I moved the discussion here because it is related to the procedures contributors are supposed to use when committing to the master repo. Here's the beginning, for those who don't read bug-gnu-emacs: > > Date: Sun, 15 May 2011 22:33:03 -0700 > > From: Paul Eggert cs.ucla.edu> > > CC: 8675 debbugs.gnu.org > > > > On 05/15/11 22:30, Eli Zaretskii wrote: > > >> Date: Sun, 15 May 2011 22:07:36 -0700 > > >> From: Paul Eggert cs.ucla.edu> > > >> > > >> PATCH 3 depends on two obvious patches: PATCH 2 introduces a > > >> helper > > >> no-return function string_overflow, and PATCH 1 updates to the > > >> latest > > >> version of gnulib. > > > > > > Thanks, but why do these patches come with unrelated changes in > > > texinfo.tex? > > > > Because that's part of PATCH 1, which updates to the latest version > > of gnulib. Gnulib contains texinfo.tex. PATCH 1 was generated > > entirely automatically by "make sync-from-gnulib". > > When you merge, could you please make the texinfo.tex update a > separate commit on the trunk, then? No one will ever expect to find > that file in a commit logged as "sync from gnulib", which will make > forensics more difficult than it needs to (since your commits are > always merge-commits). And here's how Stefan replied: > From: Stefan Monnier > Cc: Paul Eggert , 8675@debbugs.gnu.org > Date: Mon, 16 May 2011 13:48:33 -0300 > > > When you merge, could you please make the texinfo.tex update a > > separate commit on the trunk, then? > > Why? I already explained the reasons in my original message (above), i.e. that forensics is more difficult. > > No one will ever expect to find that file in a commit logged as "sync > > from gnulib", > > I, for one, do. texinfo.tex is not a file we maintain, so I fully > expect to find its changes in some kind of "sync from outside" commit. Well, then maybe replace "no one will ever" with "many people will not". However, I think even you will not expect to find that texinfo.tex was updated in commits such as these two: revno: 103360 [merge] committer: Paul Eggert branch nick: trunk timestamp: Sun 2011-02-20 00:48:52 -0800 message: Merge: Import crypto/md5 and stdint modules from gnulib. -------------------------------------------------------- revno: 103104 [merge] committer: Paul Eggert branch nick: trunk timestamp: Thu 2011-02-03 11:30:24 -0800 message: merge: allow C code to suppress warnings about ignored return values ------------------------------------------------------------ Yes, invoking "bzr log" with the -n0 or --include-merges switch would have shown that the second one of these two mentions texinfo.tex in its branch commit. But these switches makes "bzr log" much slower. Any bzr command that uses revision specs need special care (e.g., using the `before:' keyword) to ensure branch commits aren't missed. And to find out which branch commit modified texinfo.tex as part of revision 103360, you need "bzr diff", or other similar tricks. It's not rocket science, but it makes the job of finding the relevant changes harder and more error prone. Plus, it requires one to have good control of relatively obscure options. Thus my request. More generally, I thought commits to mainline from local branches are supposed to be one feature at a time. That is, each changeset committed to mainline is supposed to be self-contained and not mix different features and unrelated bugfixes. If you regard any "sync from gnulib" as a single self-contained changeset, then at least it should be committed to mainline separately from other changes. admin/notes/commits has these instructions, which I thought was supposed to be followed: (0) Each commit should correspond to a single change (whether spread over multiple files or not). Do not mix different changes in the same commit (eg adding a feature in one file, fixing a bug in another should be two commits, not one). (1) Commit all changed files at once with a single log message (which in CVS will result in an identical log message for all committed files), not one-by-one. This is pretty easy using vc-dir now. (2) Make the log message describe the entire changeset, perhaps including relevant changelog entiries (I often don't bother with the latter if it's a trivial sort of change). If that's not the policy, then what _is_ the policy? As an aside, it looks like the version of texinfo.tex we merge is from ftp://tug.org/tex/. But this message recently posted by Karl Berry, the maintainer of that file: http://lists.gnu.org/archive/html/bug-texinfo/2011-04/msg00038.html indicates that the right place is ftp://ftp.gnu.org/gnu/texinfo/.