bug#52420: admin/release-branch.txt: move NEWS in single commit

bug#52420: admin/release-branch.txt: move NEWS in single commit

[Stefan Kangas]

Severity: wishlist

In admin/release-branch.txt, step 4 says:

    Set the version on the master branch to the next major release:

     M-x set-version RET XY+1.0.50 RET

    This creates a new file etc/NEWS.XY.  "git add" it.

I think the instructions should be changed so that the NEWS file is
moved like this:

    git mv etc/NEWS etc/NEWS.NN

And then committed in a single commit.  This way, git will be able to
track it as a rename and we preserve the ability to use git blame, etc.

I'm not sure if any changes will have to be made to set-version in
admin/admin.el.

bug#52420: admin/release-branch.txt: move NEWS in single commit

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Sat, 11 Dec 2021 01:30:01 +0000
> 
> In admin/release-branch.txt, step 4 says:
> 
>     Set the version on the master branch to the next major release:
> 
>      M-x set-version RET XY+1.0.50 RET
> 
>     This creates a new file etc/NEWS.XY.  "git add" it.
> 
> I think the instructions should be changed so that the NEWS file is
> moved like this:
> 
>     git mv etc/NEWS etc/NEWS.NN
> 
> And then committed in a single commit.  This way, git will be able to
> track it as a rename and we preserve the ability to use git blame, etc.
> 
> I'm not sure if any changes will have to be made to set-version in
> admin/admin.el.

I think we should modify admin.el to use "git mv" and commit that
(after prompting), instead of calling rename-file etc. that it does
today.  IOW, there should be no need to do the above manually.

bug#52420: admin/release-branch.txt: move NEWS in single commit

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> I think we should modify admin.el to use "git mv" and commit that
> (after prompting), instead of calling rename-file etc. that it does
> today.  IOW, there should be no need to do the above manually.

Yes, that sounds better.

bug#52420: admin/release-branch.txt: move NEWS in single commit

[Stefan Kangas]

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

tags 52420 + patch
thanks

Eli Zaretskii <eliz@gnu.org> writes:

> I think we should modify admin.el to use "git mv" and commit that
> (after prompting), instead of calling rename-file etc. that it does
> today.  IOW, there should be no need to do the above manually.

How does the attached patch look?

[-- Attachment #2: 0001-admin.el-Move-etc-NEWS-to-etc-NEWS.NN-in-one-commit.patch --]
[-- Type: text/x-diff, Size: 2195 bytes --]

From 153060766abc96e4c77873f9bf3305e5d44955bc Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Mon, 27 Dec 2021 02:44:27 +0100
Subject: [PATCH] admin.el: Move etc/NEWS to etc/NEWS.NN in one commit

* admin/admin.el (admin-git-command): New variable.
(set-version): Move etc/NEWS to etc/NEWS.NN and prompt to commit
it.  (Bug#52420)
---
 admin/admin.el | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/admin/admin.el b/admin/admin.el
index ad4208beef..55c3694f3c 100644
--- a/admin/admin.el
+++ b/admin/admin.el
@@ -88,6 +88,9 @@ set-version-in-file
     (kill-buffer)
     (message "No need to update `%s'" file)))
 
+(defvar admin-git-command (executable-find "git")
+  "The `git' program to use.")
+
 (defun set-version (root version)
   "Set Emacs version to VERSION in relevant files under ROOT.
 Root must be the root of an Emacs source tree."
@@ -96,6 +99,8 @@ set-version
 		(read-string "Version number: " emacs-version)))
   (unless (file-exists-p (expand-file-name "src/emacs.c" root))
     (user-error "%s doesn't seem to be the root of an Emacs source tree" root))
+  (unless admin-git-command
+    (user-error "Could not find git in `exec-path'"))
   (message "Setting version numbers...")
   ;; There's also a "version 3" (standing for GPLv3) at the end of
   ;; `README', but since `set-version-in-file' only replaces the first
@@ -157,7 +162,13 @@ set-version
 Documentation changes might not have been completed!"))))
     (when (and majorbump
                (not (file-exists-p oldnewsfile)))
-      (rename-file newsfile oldnewsfile)
+      (call-process admin-git-command nil nil nil
+                    "mv" newsfile oldnewsfile)
+      (when (y-or-n-p "Commit move of NEWS file?")
+        (call-process admin-git-command nil nil nil
+                      "commit" "-m" (format "; Move etc/%s to etc/%s"
+                                            (file-name-nondirectory newsfile)
+                                            (file-name-nondirectory oldnewsfile))))
       (find-file oldnewsfile)           ; to prompt you to commit it
       (copy-file oldnewsfile newsfile)
       (with-temp-buffer
-- 
2.30.2

bug#52420: admin/release-branch.txt: move NEWS in single commit

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Sun, 26 Dec 2021 17:52:28 -0800
> Cc: 52420@debbugs.gnu.org
> 
> +  (unless admin-git-command
> +    (user-error "Could not find git in `exec-path'"))

When this happens, I think admin.el should show instructions for how
to do that manually, instead of just displaying an error message.

Other than that, LGTM, thanks.

bug#52420: admin/release-branch.txt: move NEWS in single commit

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> +  (unless admin-git-command
>> +    (user-error "Could not find git in `exec-path'"))
>
> When this happens, I think admin.el should show instructions for how
> to do that manually, instead of just displaying an error message.

I'm not sure I understand what you mean here.  Could you elaborate?  For
example, what instructions do you think should we show here?

bug#52420: admin/release-branch.txt: move NEWS in single commit

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Mon, 27 Dec 2021 08:11:12 -0800
> Cc: 52420@debbugs.gnu.org
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> +  (unless admin-git-command
> >> +    (user-error "Could not find git in `exec-path'"))
> >
> > When this happens, I think admin.el should show instructions for how
> > to do that manually, instead of just displaying an error message.
> 
> I'm not sure I understand what you mean here.  Could you elaborate?  For
> example, what instructions do you think should we show here?

Something like

  Git was not found, perform "git mv ...." by hand

bug#52420: admin/release-branch.txt: move NEWS in single commit

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> Something like
>
>   Git was not found, perform "git mv ...." by hand

Could we assume the user even has git installed in that situation?

IOW, how about:

    "Could not find git in `exec-path'; please install git"

bug#52420: admin/release-branch.txt: move NEWS in single commit

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Mon, 27 Dec 2021 09:32:59 -0800
> Cc: 52420@debbugs.gnu.org
> 
> IOW, how about:
> 
>     "Could not find git in `exec-path'; please install git"

  Could not find git; please install git and move NEWS manually

bug#52420: admin/release-branch.txt: move NEWS in single commit

[Stefan Kangas]

close 52420 29.1
thanks

Eli Zaretskii <eliz@gnu.org> writes:

>   Could not find git; please install git and move NEWS manually

Thanks, pushed to master with that change (commit 0fb55c8776).