git.el: handle default excludesfile properly

The previous version only looked at core.excludesfile for locating the excludesfile. So, when core.excludesfile was not defined, it did not use the possible default locations.

The current version uses either $XDG_CONFIG_HOME/git/ignore or $HOME/.config/git/ignore for the default excludesfile location depending on whether XDG_CONFIG_HOME is defined or not. As per the documentation of gitignore.

---
 contrib/emacs/git.el | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

index 97919f2d7..16c6b4c05 100644
@@ -757,8 +757,12 @@ Return the list of files that haven't been handled."
 
 (defun git-get-exclude-files ()
   "Get the list of exclude files to pass to git-ls-files."
-  (let (files
-        (config (git-config "core.excludesfile")))
+  (let* (files
+	 (defaultconfig (if (getenv "XDG_CONFIG_HOME")
+			    (concat (file-name-as-directory (getenv "XDG_CONFIG_HOME")) "git/ignore")
+			  (concat (file-name-as-directory (getenv "HOME")) ".config/git/ignore")))
+	 (coreexcludes (git-config "core.excludesfile"))
+	 (config (if coreexcludes coreexcludes defaultconfig)))
     (when (file-readable-p ".git/info/exclude")
       (push ".git/info/exclude" files))
     (when (and config (file-readable-p config))

2.16.2

Perhaps take $HOME/.gitignore into account too?

[[Pardon the duplicate. I forgot to send via text/plain and the mailing list bounced it, so resending.]]

Thanks for the feedback.

Correct me if I'm wrong, but my understanding, from
https://git-scm.com/docs/gitignore, is that $HOME/.gitignore is used only if it is specified as the value of core.excludesfile in ~/.gitconfig. It is not used by default. If that is so, then the proposed (and original) code works. The changes I am proposing handle the default case, when core.excludesfile is not specified.

Looking deeper into how the function git-get-exclude-files is used, I see that it is only being called from git-run-ls-files-with-excludes. So, perhaps, a better (or additional) fix might be to add the parameter "--exclude-standard" in the call to git-run-ls-files from within git-run-ls-files-with-excludes. And remove the need for get-get-exclude-files altogether. Presumably, "--exclude-standard" handles the default case with/without XDG_CONFIG_HOME correctly. The question I'd have then is: why didn't the original author use that option? Either I'm missing something? Or the option was added later, after the original code was written? Or something else?

Thoughts?

You're right. I must have set core.excludesfile so long ago that I forgot about it and assumed $HOME/.gitignore was consulted by default.

Using --exclude-standard rather than --exclude-from and retiring git-get-exclude-files() makes sense to me.

As for why the original author didn't use --exclude-standard, project history tells us that. In particular, git-get-exclude-files() was implemented by 274e13e0e9 (git.el: Take into account the core.excludesfile config option., 2007-07-31), whereas --exclude-standard was introduced by 8e7b07c8a7 (git-ls-files: add --exclude-standard, 2007-11-15), three and a half months later.

If you do re-roll to use --exclude-standard, then it would be good for your commit message to explain this history, citing the relevant commits.

Thanks.

Thanks for reviewing and locating the commits.

OK, I'll re-roll and add the relevant commits. It may take some time.

Should I just send the revised patch as a separate thread (with the relevant commits and history)?

That would work. You can use "git format-patch -v2 ..." to mark the patch as "[PATCH v2]", and "git send-email [email protected] ..." to tie it back to this thread when you send it.

As a an aid to reviewers, it's a good idea to add commentary explaining what changed since v1 and provide a link back to v1, like this[1]. Place the commentary below the "---" line following your sign-off.

(One more minor comment: etiquette on this list is to avoid top-posting [2].)

Thanks.

[1]: https://public-inbox.org/git/[email protected]/
[2]: https://lkml.org/lkml/2005/1/11/111

It is absolutely the right thing to depend on --exclude-standard, I would think, so that we do not have to worry about details like XDG paths and such. Thanks for working that out between both of you.

Having said that, I am sorry to say that I am not sure if the copy we have is the one to be patched, so I would appreciate if Alexandre (cc'ed) can clarify the situation. The last change done to our copy of the script is from 2012, and I do not know if Alexandre is still taking care of it here but the script is so perfect that there was no need to update it for the past 5 years and we haven't seen an update, or the caninical copy is now being maintained elsewhere and we only have a stale copy, or what.

Thanks.

This is the canonical version, and I guess in theory I'm still taking care of it. However, the need that git.el was originally addressing is now fulfilled by better tools. As such, I feel that it has outlived its usefulness, and I'm no longer actively developing it.

I'd recommend that anybody still using it switch to Magit, which is being actively maintained, and IMO superior to git.el in all respects.

Alexandre Julliard
[email protected]

Thanks for updating us with the status of git.el.

The last time I looked at magit, it was too heavyweight for my needs. I like the simplicity of git.el. But perhaps it's time for me to take another look at magit.

On Wed, Mar 07 2018, Dorab Patel jotted:

You can also check out the VC mode that ships with Emacs itself. I prefer Magit, but the VC mode is more lightweight.

The git-blame.el mode has been superseded by Emacs's own vc-annotate (invoked by C-x v g). Users of the git.el mode are now much better off using either Magit or the Git backend for Emacs's own VC mode.

These modes were added over 10 years ago when Emacs's own Git support was much less mature, and there weren't other mature modes in the wild or shipped with Emacs itself.

These days these modes have very few if users, and users of git aren't well served by us shipping these (some OS's install them alongside git by default, which is confusing and leads users astray).

So let's remove these per Alexandre Julliard's message to the ML[1]. If someone still wants these for some reason they're better served by hosting these elsewhere (e.g. on ELPA), instead of us distributing them with git.

1. "Re: [PATCH] git.el: handle default excludesfile properly" ([email protected]) --
https://public-inbox.org/git/[email protected]/

---

On Tue, Mar 06 2018, Alexandre Julliard jotted:

I think at this point it's best to remove both of these modes from being distributed with Git, per this patch.

 contrib/emacs/.gitignore   |    1 -
 contrib/emacs/Makefile     |   21 -
 contrib/emacs/README       |   39 -
 contrib/emacs/git-blame.el |  483 -------------
 contrib/emacs/git.el       | 1704 --------------------------------------------
 5 files changed, 2248 deletions(-)
 delete mode 100644 contrib/emacs/.gitignore
 delete mode 100644 contrib/emacs/Makefile
 delete mode 100644 contrib/emacs/README
 delete mode 100644 contrib/emacs/git-blame.el
 delete mode 100644 contrib/emacs/git.el

deleted file mode 100644
index c531d9867f..0000000000
@@ -1 +0,0 @@
-*.elc

deleted file mode 100644
index 24d9312941..0000000000
@@ -1,21 +0,0 @@
-## Build and install stuff
-
-EMACS = emacs
-
-ELC = git.elc git-blame.elc
-INSTALL ?= install
-INSTALL_ELC = $(INSTALL) -m 644
-prefix ?= $(HOME)
-emacsdir = $(prefix)/share/emacs/site-lisp
-RM ?= rm -f
-
-all: $(ELC)
-
-install: all
-	$(INSTALL) -d $(DESTDIR)$(emacsdir)
-	$(INSTALL_ELC) $(ELC:.elc=.el) $(ELC) $(DESTDIR)$(emacsdir)
-
-%.elc: %.el
-	$(EMACS) -batch -f batch-byte-compile $<
-
-clean:; $(RM) $(ELC)

deleted file mode 100644
index 82368bdbff..0000000000
@@ -1,39 +0,0 @@
-This directory contains various modules for Emacs support.
-
-To make the modules available to Emacs, you should add this directory
-to your load-path, and then require the modules you want. This can be
-done by adding to your .emacs something like this:
-
-  (add-to-list 'load-path ".../git/contrib/emacs")
-  (require 'git)
-  (require 'git-blame)
-
-
-The following modules are available:
-
-* git.el:
-
-  Status manager that displays the state of all the files of the
-  project, and provides easy access to the most frequently used git
-  commands. The user interface is as far as possible compatible with
-  the pcl-cvs mode. It can be started with `M-x git-status'.
-
-* git-blame.el:
-
-  Emacs implementation of incremental git-blame.  When you turn it on
-  while viewing a file, the editor buffer will be updated by setting
-  the background of individual lines to a color that reflects which
-  commit it comes from.  And when you move around the buffer, a
-  one-line summary will be shown in the echo area.
-
-* vc-git.el:
-
-  This file used to contain the VC-mode backend for git, but it is no
-  longer distributed with git. It is now maintained as part of Emacs
-  and included in standard Emacs distributions starting from version
-  22.2.
-
-  If you have an earlier Emacs version, upgrading to Emacs 22 is
-  recommended, since the VC mode in older Emacs is not generic enough
-  to be able to support git in a reasonable manner, and no attempt has
-  been made to backport vc-git.el.

2.15.1.424.g9478a66081

I agree with the overall direction. The only difference between this and what I had in mind was if we want to leave README that says what you said in the log message. That way, those who just got a new version of tarball and then wonder what happened to these scripts would save one trip to the Internet.

The git-blame.el mode has been superseded by Emacs's own vc-annotate (invoked by C-x v g). Users of the git.el mode are now much better off using either Magit or the Git backend for Emacs's own VC mode.

These modes were added over 10 years ago when Emacs's own Git support was much less mature, and there weren't other mature modes in the wild or shipped with Emacs itself.

These days these modes have very few if users, and users of git aren't well served by us shipping these (some OS's install them alongside git by default, which is confusing and leads users astray).

So let's remove these per Alexandre Julliard's message to the ML[1]. If someone still wants these for some reason they're better served by hosting these elsewhere (e.g. on ELPA), instead of us distributing them with git.

1. "Re: [PATCH] git.el: handle default excludesfile properly" ([email protected]) --
https://public-inbox.org/git/[email protected]/

---

On Thu, Mar 08 2018, Junio C. Hamano jotted:

Here's a version where I amend the contrib/emacs/README to say that we used to have some code here, but no longer do, with a brief overview of the dead code and replacements for its functionality. Hopefully that's more informative than just copy/pasting what I had in the commit message.

 contrib/emacs/.gitignore   |    1 -
 contrib/emacs/Makefile     |   21 -
 contrib/emacs/README       |   32 +-
 contrib/emacs/git-blame.el |  483 -------------
 contrib/emacs/git.el       | 1704 --------------------------------------------
 5 files changed, 13 insertions(+), 2228 deletions(-)
 delete mode 100644 contrib/emacs/.gitignore
 delete mode 100644 contrib/emacs/Makefile
 delete mode 100644 contrib/emacs/git-blame.el
 delete mode 100644 contrib/emacs/git.el

deleted file mode 100644
index c531d9867f..0000000000
@@ -1 +0,0 @@
-*.elc

deleted file mode 100644
index 24d9312941..0000000000
@@ -1,21 +0,0 @@
-## Build and install stuff
-
-EMACS = emacs
-
-ELC = git.elc git-blame.elc
-INSTALL ?= install
-INSTALL_ELC = $(INSTALL) -m 644
-prefix ?= $(HOME)
-emacsdir = $(prefix)/share/emacs/site-lisp
-RM ?= rm -f
-
-all: $(ELC)
-
-install: all
-	$(INSTALL) -d $(DESTDIR)$(emacsdir)
-	$(INSTALL_ELC) $(ELC:.elc=.el) $(ELC) $(DESTDIR)$(emacsdir)
-
-%.elc: %.el
-	$(EMACS) -batch -f batch-byte-compile $<
-
-clean:; $(RM) $(ELC)

index 82368bdbff..5a63109458 100644
@@ -1,30 +1,24 @@
-This directory contains various modules for Emacs support.
+This directory used to contain various modules for Emacs support.
 
-To make the modules available to Emacs, you should add this directory
-to your load-path, and then require the modules you want. This can be
-done by adding to your .emacs something like this:
+These were added shortly after Git was first released, since then
+Emacs's own support for Git got better than what was offered by these
+modules, or was superseded by popular 3rd-party Git modes such as
+Magit.
 
-  (add-to-list 'load-path ".../git/contrib/emacs")
-  (require 'git)
-  (require 'git-blame)
-
-
-The following modules are available:
+The following modules were available, and can be dug up from the Git
+history:
 
 * git.el:
 
-  Status manager that displays the state of all the files of the
-  project, and provides easy access to the most frequently used git
-  commands. The user interface is as far as possible compatible with
-  the pcl-cvs mode. It can be started with `M-x git-status'.
+  Wrapper for "git status" that provided access to other git commands.
+
+  Modern alternatives to this are Magit, or the VC mode that ships
+  with Emacs.
 
 * git-blame.el:
 
-  Emacs implementation of incremental git-blame.  When you turn it on
-  while viewing a file, the editor buffer will be updated by setting
-  the background of individual lines to a color that reflects which
-  commit it comes from.  And when you move around the buffer, a
-  one-line summary will be shown in the echo area.
+  A wrapper for "git blame" written before Emacs's own vc-annotate
+  mode learned to invoke git-blame, which can be done via C-x v g.
 
 * vc-git.el:
 

2.15.1.424.g9478a66081

I think "There are also" is way better. If we really want to sy superseded, perhaps "The features these modules offered were superseded by Emacs's own support in VC mode, and popular third-party Git modes e.g. Magit" is what we can say.

On Tue, Mar 13 2018, Junio C. Hamano jotted:

I sent a v3 a few days ago which addressed this, it's at [email protected]
(https://public-inbox.org/git/[email protected]/)
but I see that I screwd up the In-Reply-To (I still don't have this fully automated) so that wasn't part of this thread, sorry.

That changes this wording, among other fixes (noted in the tbdiff).

Thanks, I did look at that version but did not act on it immediately and then forgot about it.

Queued.

Ævar Arnfjörð Bjarmason <[email protected]> writes:

[...]

s/if// or s/if/if any/ ?

Kyle

The previous version only looked at core.excludesfile for locating the excludesfile. So, when core.excludesfile was not defined, it did not use the relevant default locations for the global excludes file.

The issue is in git-get-exclude-files(). Investigation shows that git-get-exclude-files() is only called from git-run-ls-files-with-excludes(). Modifying git-run-ls-files-with-excludes() to use the "--exclude-standard" option to git-run-ls-files() obviates the need for git-get-exclude-files() altogether, which is now removed. In addition, the "--exclude-per-directory" option to git-run-ls-files() is used only when git-per-dir-ignore-file is not the default (.gitignore), since the default case is handled by the "--exclude-standard" option.

Looking at the history shows that git-get-exclude-files() was implemented by commit 274e13e0e9 (git.el: Take into account the core.excludesfile config option., 2007-07-31), whereas the "--exclude-standard" option was introduced by commit 8e7b07c8a7 (git-ls-files: add --exclude-standard, 2007-11-15), three and a half months later. This explains why the "--exclude-standard" option was not used in the original code.

---

Notes:
The original patch[1] V1 attempted to add code to git-get-exclude-files() to handle the case when core.excludesfile was not defined. This involved code to check for the env variable XDG_CONFIG_HOME and related processing to find the value of the default excludesfile. Further investigation showed that using the "--exclude-standard" option to git-run-ls-files-with-excludes() already does similar processing. Hence the V2 patch uses the "--exclude-standard" option and does away with git-get-exclude-files().

[1] https://public-inbox.org/git/[email protected]/

 contrib/emacs/git.el | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

index 97919f2d7..cef42f1de 100644
@@ -755,22 +755,13 @@ Return the list of files that haven't been handled."
       (setq unmerged-files (nreverse unmerged-files))  ;; assume it is sorted already
       (git-set-filenames-state status unmerged-files 'unmerged))))
 
-(defun git-get-exclude-files ()
-  "Get the list of exclude files to pass to git-ls-files."
-  (let (files
-        (config (git-config "core.excludesfile")))
-    (when (file-readable-p ".git/info/exclude")
-      (push ".git/info/exclude" files))
-    (when (and config (file-readable-p config))
-      (push config files))
-    files))
-
 (defun git-run-ls-files-with-excludes (status files default-state &rest options)
-  "Run git-ls-files on FILES with appropriate --exclude-from options."
-  (let ((exclude-files (git-get-exclude-files)))
-    (apply #'git-run-ls-files status files default-state "--directory" "--no-empty-directory"
-           (concat "--exclude-per-directory=" git-per-dir-ignore-file)
-           (append options (mapcar (lambda (f) (concat "--exclude-from=" f)) exclude-files)))))
+  "Run git-ls-files on FILES with appropriate exclude options."
+  (apply #'git-run-ls-files status files default-state
+	 "--directory" "--no-empty-directory" "--exclude-standard"
+	 (append (unless (string-equal git-per-dir-ignore-file ".gitignore") ; handled by --exclude-standard
+		   (list (concat "--exclude-per-directory=" git-per-dir-ignore-file)))
+		 options)))
 
 (defun git-update-status-files (&optional files mark-files)
   "Update the status of FILES from the index.

2.16.2

The git-blame.el mode has been superseded by Emacs's own vc-annotate (invoked by C-x v g). Users of the git.el mode are now much better off using either Magit or the Git backend for Emacs's own VC mode.

These modes were added over 10 years ago when Emacs's own Git support was much less mature, and there weren't other mature modes in the wild or shipped with Emacs itself.

These days these modes have few if any users, and users of git aren't well served by us shipping these (some OS's install them alongside git by default, which is confusing and leads users astray).

So let's remove these per Alexandre Julliard's message to the ML[1]. If someone still wants these for some reason they're better served by hosting these elsewhere (e.g. on ELPA), instead of us distributing them with git.

1. "Re: [PATCH] git.el: handle default excludesfile properly" ([email protected]) --
https://public-inbox.org/git/[email protected]/

---

On Sat, Mar 10 2018, Martin Ågren jotted:

Thanks. Here's a v3 which incorporates these suggestions, and also Kyle's suggestion upthread which I somehow missed when re-rolling this. tbdiff to v2 below:

1: ad00297143 ! 1: 5bc3d3848d git{,-blame}.el: remove old bitrotting Emacs code
@@ -11,7 +11,7 @@
was much less mature, and there weren't other mature modes in the wild or shipped with Emacs itself.

- These days these modes have very few if users, and users of git aren't
+ These days these modes have few if any users, and users of git aren't
well served by us shipping these (some OS's install them alongside git by default, which is confusing and leads users astray).

@@ -70,10 +70,10 @@
-To make the modules available to Emacs, you should add this directory -to your load-path, and then require the modules you want. This can be -done by adding to your .emacs something like this:
-+These were added shortly after Git was first released, since then ++These were added shortly after Git was first released. Since then
+Emacs's own support for Git got better than what was offered by these
-+modules, or was superseded by popular 3rd-party Git modes such as -+Magit.
++modes. There are also popular 3rd-party Git modes such as Magit which ++offer replacements for these.

- (add-to-list 'load-path ".../git/contrib/emacs")
- (require 'git)
@@ -92,7 +92,7 @@
- the pcl-cvs mode. It can be started with `M-x git-status'.
+ Wrapper for "git status" that provided access to other git commands. +
-+ Modern alternatives to this are Magit, or the VC mode that ships ++ Modern alternatives to this include Magit, and VC mode that ships
+ with Emacs.

* git-blame.el:

 contrib/emacs/.gitignore   |    1 -
 contrib/emacs/Makefile     |   21 -
 contrib/emacs/README       |   32 +-
 contrib/emacs/git-blame.el |  483 -------------
 contrib/emacs/git.el       | 1704 --------------------------------------------
 5 files changed, 13 insertions(+), 2228 deletions(-)
 delete mode 100644 contrib/emacs/.gitignore
 delete mode 100644 contrib/emacs/Makefile
 delete mode 100644 contrib/emacs/git-blame.el
 delete mode 100644 contrib/emacs/git.el

deleted file mode 100644
index c531d9867f..0000000000
@@ -1 +0,0 @@
-*.elc

deleted file mode 100644
index 24d9312941..0000000000
@@ -1,21 +0,0 @@
-## Build and install stuff
-
-EMACS = emacs
-
-ELC = git.elc git-blame.elc
-INSTALL ?= install
-INSTALL_ELC = $(INSTALL) -m 644
-prefix ?= $(HOME)
-emacsdir = $(prefix)/share/emacs/site-lisp
-RM ?= rm -f
-
-all: $(ELC)
-
-install: all
-	$(INSTALL) -d $(DESTDIR)$(emacsdir)
-	$(INSTALL_ELC) $(ELC:.elc=.el) $(ELC) $(DESTDIR)$(emacsdir)
-
-%.elc: %.el
-	$(EMACS) -batch -f batch-byte-compile $<
-
-clean:; $(RM) $(ELC)

index 82368bdbff..977a16f1e3 100644
@@ -1,30 +1,24 @@
-This directory contains various modules for Emacs support.
+This directory used to contain various modules for Emacs support.
 
-To make the modules available to Emacs, you should add this directory
-to your load-path, and then require the modules you want. This can be
-done by adding to your .emacs something like this:
+These were added shortly after Git was first released. Since then
+Emacs's own support for Git got better than what was offered by these
+modes. There are also popular 3rd-party Git modes such as Magit which
+offer replacements for these.
 
-  (add-to-list 'load-path ".../git/contrib/emacs")
-  (require 'git)
-  (require 'git-blame)
-
-
-The following modules are available:
+The following modules were available, and can be dug up from the Git
+history:
 
 * git.el:
 
-  Status manager that displays the state of all the files of the
-  project, and provides easy access to the most frequently used git
-  commands. The user interface is as far as possible compatible with
-  the pcl-cvs mode. It can be started with `M-x git-status'.
+  Wrapper for "git status" that provided access to other git commands.
+
+  Modern alternatives to this include Magit, and VC mode that ships
+  with Emacs.
 
 * git-blame.el:
 
-  Emacs implementation of incremental git-blame.  When you turn it on
-  while viewing a file, the editor buffer will be updated by setting
-  the background of individual lines to a color that reflects which
-  commit it comes from.  And when you move around the buffer, a
-  one-line summary will be shown in the echo area.
+  A wrapper for "git blame" written before Emacs's own vc-annotate
+  mode learned to invoke git-blame, which can be done via C-x v g.
 
 * vc-git.el:
 

2.15.1.424.g9478a66081

Hi,

Ævar Arnfjörð Bjarmason wrote[1]:

The trouble with removing these so abruptly is that it makes for a bad user experience.

Warning (initialization): An error occurred while loading ‘/home/jrn/.emacs’:

File error: Cannot open load file, No such file or directory, git

In some sense that is the distributor's fault: just because Git upstream stops removing the git.el file doesn't mean that the distributor needs to. But the same thing would happen if the user symlinked git.el into a place that emacs could find when using upstream Git directly. And we are putting the distributor in a bad place.

Ami Fischman (cc-ed) writes:

| IMO a placeholder git.el that did something like:
|
|   (error "git.el is no more; replace (require 'git) with (require 'magit) or
|   simply delete the former in your initialization file(s)")
|
| ideally with a pointer to a short URL explaining the rationale would have
| been fine.
| (note that though I've seen
| https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=893734 I'm _still_ unclear
| as to why the change was made; you might want to clarify in that bug and
| point to it from this, or something else)

What do you think? Would adding such a placeholder during a transitional period work well for you?

Thanks,
Jonathan

[1] https://public-inbox.org/git/[email protected]/

The git-blame.el mode has been superseded by Emacs's own vc-annotate (invoked by C-x v g). Users of the git.el mode are now much better off using either Magit or the Git backend for Emacs's own VC mode.

These modes were added over 10 years ago when Emacs's own Git support was much less mature, and there weren't other mature modes in the wild or shipped with Emacs itself.

These days these modes have few if any users, and users of git aren't well served by us shipping these (some OS's install them alongside git by default, which is confusing and leads users astray).

So let's remove these per Alexandre Julliard's message to the ML[1]. If someone still wants these for some reason they're better served by hosting these elsewhere (e.g. on ELPA), instead of us distributing them with git.

However, since downstream packagers such as Debian are packaging this as git-el it's less disruptive to still carry these files as Elisp code that'll error out with a message suggesting alternatives, rather than drop the files entirely[2].

Then rather than receive a cryptic load error when they upgrade existing users will get an error directing them to the README file, or to just stop requiring these modes. I think it makes sense to link to GitHub's hosting of contrib/emacs/README (which'll be updated by the time users see this) so they don't have to hunt down the packaged README on their local system.

1. "Re: [PATCH] git.el: handle default excludesfile properly" ([email protected]) --
https://public-inbox.org/git/[email protected]/

2. "Re: [PATCH v3] git{,-blame}.el: remove old bitrotting Emacs code" ([email protected]) --
https://public-inbox.org/git/[email protected]/

---

Indeed. Sorry for taking so long to act on this. Haven't had much time for git recently. Now we still ship these Elisp files, but they just error out with a message as suggested. Junio: This should address your note about this series in the last What's Cooking.

tbdiff with v3:

1: 5bc3d3848d ! 1: 7556844084 git{,-blame}.el: remove old bitrotting Emacs code
@@ -20,9 +20,25 @@
served by hosting these elsewhere (e.g. on ELPA), instead of us distributing them with git.

+ However, since downstream packagers such as Debian are packaging this
+ as git-el it's less disruptive to still carry these files as Elisp
+ code that'll error out with a message suggesting alternatives, rather
+ than drop the files entirely[2].
+
+ Then rather than receive a cryptic load error when they upgrade
+ existing users will get an error directing them to the README file, or
+ to just stop requiring these modes. I think it makes sense to link to
+ GitHub's hosting of contrib/emacs/README (which'll be updated by the
+ time users see this) so they don't have to hunt down the packaged
+ README on their local system.
+
1. "Re: [PATCH] git.el: handle default excludesfile properly" ([email protected]) --
https://public-inbox.org/git/[email protected]/
+
+ 2. "Re: [PATCH v3] git{,-blame}.el: remove old bitrotting Emacs
+ code" ([email protected]) --
+ https://public-inbox.org/git/[email protected]/

@@ -109,9 +125,8 @@

diff --git a/contrib/emacs/git-blame.el b/contrib/emacs/git-blame.el
-deleted file mode 100644
--- a/contrib/emacs/git-blame.el
-+++ /dev/null
++++ b/contrib/emacs/git-blame.el
@@
-;;; git-blame.el --- Minor mode for incremental blame for Git -*- coding: utf-8 -*- -;;
@@ -596,11 +611,16 @@
-(provide 'git-blame)
-
-;;; git-blame.el ends here
++(error "git-blame.el no longer ships with git. It's recommended
++to replace its use with Emacs's own vc-annotate. See ++contrib/emacs/README in git's
++sources (https://github.com/git/git/blob/master/contrib/emacs/README)
++for more info on suggested alternatives and for why this ++happened.")

diff --git a/contrib/emacs/git.el b/contrib/emacs/git.el
-deleted file mode 100644
--- a/contrib/emacs/git.el
-+++ /dev/null
++++ b/contrib/emacs/git.el
@@
-;;; git.el --- A user interface for git
-
@@ -2306,3 +2326,9 @@
-
-(provide 'git)
-;;; git.el ends here
++(error "git.el no longer ships with git. It's recommended to ++replace its use with Magit, or simply delete references to git.el ++in your initialization file(s). See contrib/emacs/README in git's ++sources (https://github.com/git/git/blob/master/contrib/emacs/README)
++for suggested alternatives and for why this happened. Emacs's own ++VC mode and Magit are viable alternatives.")

 contrib/emacs/.gitignore   |    1 -
 contrib/emacs/Makefile     |   21 -
 contrib/emacs/README       |   32 +-
 contrib/emacs/git-blame.el |  489 +----------
 contrib/emacs/git.el       | 1710 +-----------------------------------
 5 files changed, 25 insertions(+), 2228 deletions(-)
 delete mode 100644 contrib/emacs/.gitignore
 delete mode 100644 contrib/emacs/Makefile

deleted file mode 100644
index c531d9867f..0000000000
@@ -1 +0,0 @@
-*.elc

deleted file mode 100644
index 24d9312941..0000000000
@@ -1,21 +0,0 @@
-## Build and install stuff
-
-EMACS = emacs
-
-ELC = git.elc git-blame.elc
-INSTALL ?= install
-INSTALL_ELC = $(INSTALL) -m 644
-prefix ?= $(HOME)
-emacsdir = $(prefix)/share/emacs/site-lisp
-RM ?= rm -f
-
-all: $(ELC)
-
-install: all
-	$(INSTALL) -d $(DESTDIR)$(emacsdir)
-	$(INSTALL_ELC) $(ELC:.elc=.el) $(ELC) $(DESTDIR)$(emacsdir)
-
-%.elc: %.el
-	$(EMACS) -batch -f batch-byte-compile $<
-
-clean:; $(RM) $(ELC)

index 82368bdbff..977a16f1e3 100644
@@ -1,30 +1,24 @@
-This directory contains various modules for Emacs support.
+This directory used to contain various modules for Emacs support.
 
-To make the modules available to Emacs, you should add this directory
-to your load-path, and then require the modules you want. This can be
-done by adding to your .emacs something like this:
+These were added shortly after Git was first released. Since then
+Emacs's own support for Git got better than what was offered by these
+modes. There are also popular 3rd-party Git modes such as Magit which
+offer replacements for these.
 
-  (add-to-list 'load-path ".../git/contrib/emacs")
-  (require 'git)
-  (require 'git-blame)
-
-
-The following modules are available:
+The following modules were available, and can be dug up from the Git
+history:
 
 * git.el:
 
-  Status manager that displays the state of all the files of the
-  project, and provides easy access to the most frequently used git
-  commands. The user interface is as far as possible compatible with
-  the pcl-cvs mode. It can be started with `M-x git-status'.
+  Wrapper for "git status" that provided access to other git commands.
+
+  Modern alternatives to this include Magit, and VC mode that ships
+  with Emacs.
 
 * git-blame.el:
 
-  Emacs implementation of incremental git-blame.  When you turn it on
-  while viewing a file, the editor buffer will be updated by setting
-  the background of individual lines to a color that reflects which
-  commit it comes from.  And when you move around the buffer, a
-  one-line summary will be shown in the echo area.
+  A wrapper for "git blame" written before Emacs's own vc-annotate
+  mode learned to invoke git-blame, which can be done via C-x v g.
 
 * vc-git.el:
 

2.17.0.rc1.321.gba9d0f2565

I know I am to blame for prodding you to reopen this topic, but I am wondering if removal of Makefile is sensible. Is the assumption that the distro packagers won't be using this Makefile at all and have their own (e.g. debian/rules for Debian) build procedure, hence *.el files are all they need to have?

The reason why I am wondering is because I do not know what distro folks would do when they find that their build procedure does not work---I suspect the would punt, and end users of the distro would find that git-el package is no longer with them. These end users are whom this discussion is trying to help, but then to these packagers, the reason why their build procedure no longer works does not really matter, whether git.el is not shipped, or Makefile that their debian/rules-equivalent depends on is not there, for them to decide dropping the git-el package.

On the other hand, the 6-lines of e-lisp you wrote for git.el replacement is something the packagers could have written for their users, so (1) if we really want to go extra mile without trusting that distro packagers are less competent than us in helping their users, we'd be better off to leave Makefile in, or (2) if we trust packagers and leave possible end-user confusion as their problem (not ours), then we can just remove as your previous round did.

And from that point of view, I find this round slightly odd.

I think the way it is makes sense. In Debian debian/git-el.install just does:

contrib/emacs/git-blame.el usr/share/git-core/emacs contrib/emacs/git.el usr/share/git-core/emacs

Which means on installation they don't even use our contrib/emacs/Makefile, but instead on installation do:

Setting up git-el (1:2.16.3-1) ...
Install git for emacs
Install git for emacs24
install/git: Handling install of emacsen flavor emacs24 install/git: Byte-compiling for emacs24
+ emacs24 -batch -q -no-site-file -f batch-byte-compile git.el git-blame.el Wrote /usr/share/emacs24/site-lisp/git/git.elc
Wrote /usr/share/emacs24/site-lisp/git/git-blame.elc
Install git for emacs25
install/git: Handling install of emacsen flavor emacs25 install/git: Byte-compiling for emacs25
+ emacs25 -batch -q -no-site-file -f batch-byte-compile git.el git-blame.el

RedHat does use contrib/emacs/Makefile:

https://src.fedoraproject.org/cgit/rpms/git.git/tree/git.spec#n493

But they can either just do their own byte compilation as they surely do for other elisp packages, or just do this:

     git-init.el | 5 -----
     git.spec    | 9 ++-------
     2 files changed, 2 insertions(+), 12 deletions(-)

    diff --git a/git-init.el b/git-init.el
    deleted file mode 100644
    index d2a96a7..0000000
    --- a/git-init.el
    +++ /dev/null
    @@ -1,5 +0,0 @@
    -;; Git VC backend
    -(add-to-list 'vc-handled-backends 'GIT t)
    -(autoload 'git-status "git" "GIT mode." t)
    -(autoload 'git-blame-mode "git-blame"
    -       "Minor mode for incremental blame for Git." t)
    diff --git a/git.spec b/git.spec
    index ee60d3e..a82c1aa 100644
    --- a/git.spec
    +++ b/git.spec
    @@ -87,7 +87,6 @@ Source1:        https://www.kernel.org/pub/software/scm/git/%{?rcrev:testing/}%{
     Source9:        gpgkey-junio.asc

     # Local sources begin at 10 to allow for additional future upstream sources
    -Source10:       git-init.el
     Source11:       git.xinetd.in
     Source12:       git-gui.desktop
     Source13:       gitweb-httpd.conf
    @@ -491,14 +490,10 @@ for i in git git-shell git-upload-pack; do
     done

     %global elispdir %{_emacs_sitelispdir}/git
    -make -C contrib/emacs install \
    -    emacsdir=%{buildroot}%{elispdir}
    -for elc in %{buildroot}%{elispdir}/*.elc ; do
    -    install -pm 644 contrib/emacs/$(basename $elc .elc).el \
    +for el in %{buildroot}%{elispdir}/*.el ; do
    +    install -pm 644 contrib/emacs/$elc \
         %{buildroot}%{elispdir}
     done
    -install -Dpm 644 %{SOURCE10} \
    -    %{buildroot}%{_emacs_sitestartdir}/git-init.el

     %if %{libsecret}
     install -pm 755 contrib/credential/libsecret/git-credential-libsecret \

I.e. there's no rule that there *must* be an *.elc file, it's just a generally good idea, but if your file is purely a one-line (error) there's no point in pre-compiling it.

In short, Debian happens to be OK, but RedHat folks need to do work and cannot use what we ship out of the box, *IF* they care about end user experience.

That was exactly why I felt it was "odd" (iow, "uneven"). We bother to give a stub git.el; we do not bother to make sure it would keep being installed if the packagers do not bother to update their procedure.

If we do not change anything other than making *.el into stubs, then it is a lot more likely that end user experience on *any* distro that have been shipping contrib/emacs/ stuff will by default (i.e. if the packagers do not do anything to adjust) be what we design here---upon loading they'd see (error) triggering that nudge them towards modern and maintained alternatives. If we do more than that, e.g. remove Makefile, then some distros need to adjust, or their build would be broken.

I suspect that the set of people Cc'ed on the thread are a lot more familiar than I am with how distro packagers prefer us to deliber, so I'll stop speculating at this point.

Thanks.

Thanks for working on this cruft cleanup Ævar and to Jonathan & Junio for asking questions about how to improve this transition for packagers & users.

I don't think it's a big deal for the Fedora/Red Hat packages to adjust and manually install the stub git-el.

Anyone doing automated rebuilds from the current Fedora git.spec will notice the make failure and can then check the relese notes to find out about the change, I imagine.

I wonder if leaving the Makefile in place would prevent packages from even noticing the change? It might still be a good plan to help ease the transition. I don't know if that's as important for something in contrib/ or not.

I should note that I'm not an emacs user, so I likely lack a good understanding of how people use the current git{,-blame}.el files. I could simply be doing it wrong in the steps I took to test this.

With the fedora packaging, we've shipped a git-init.el that adds autoload entries for git-status and git-blame-mode (since adding the emacs files in 2007). That allows users to make use of those features without adding anything to their local emacs configuration.

If I open emacs with a current fedora packaging, I can issue M-x git-status or M-x git-blame-mode. After applying this patch and removing the git-init.el, that no longer works but rather than the detailed warning message, I just get a transient "no matches" in the emacs status line.

However, if I add "(require 'git)" to ~/.emacs, then I get the "error: git.el no longer ships with git" message in the warnings buffer.

Does this mean that only users who have manually loaded git.el will see the warning? If so, is there a preferred method to have the warning appear when calling the commands previously provided, to give a better user experience?

Todd
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Faith, n. Belief without evidence in what is told by one who speaks
without knowledge, of things without parallel.
    -- Ambrose Bierce, "The Devil's Dictionary"