git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Alexandre Julliard <julliard@winehq.org>,
	Dorab Patel <dorabpatel@gmail.com>,
	Eric Sunshine <sunshine@sunshineco.com>,
	David Kågedal <davidk@lysator.liu.se>,
	Kyle Meyer <kyle@kyleam.com>,
	Martin Ågren <martin.agren@gmail.com>,
	Ami Fischman <fischman@google.com>,
	Jonathan Nieder <jrnieder@gmail.com>,
	Todd Zullinger <tmz@pobox.com>
Subject: Re: [PATCH v4] git{,-blame}.el: remove old bitrotting Emacs code
Date: Thu, 12 Apr 2018 08:52:13 +0200
Message-ID: <87fu40c3xe.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <xmqqk1td2ml2.fsf@gitster-ct.c.googlers.com>


On Thu, Apr 12 2018, Junio C. Hamano wrote:

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> 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.
>> ...
>>
>>  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
>
> 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.

  reply index

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <EKzyQbDEJGG4Lm5YboF8xg@mail.gmail.com>
2018-03-10 18:45 ` [PATCH v3] " Ævar Arnfjörð Bjarmason
2018-03-13 18:53   ` [PATCH v2] " Ævar Arnfjörð Bjarmason
2018-03-13 22:14     ` Junio C Hamano
2018-03-27 16:57   ` [PATCH v3] " Jonathan Nieder
2018-04-11 20:42     ` [PATCH v4] " Ævar Arnfjörð Bjarmason
2018-04-12  2:19       ` Junio C Hamano
2018-04-12  6:52         ` Ævar Arnfjörð Bjarmason [this message]
2018-04-12  9:23           ` Junio C Hamano
2018-04-18 20:16             ` Todd Zullinger
2018-03-03  3:48 [PATCH] git.el: handle default excludesfile properly Dorab Patel
2018-03-03  8:42 ` Eric Sunshine
2018-03-04  1:36   ` Dorab Patel
2018-03-04  2:12     ` Eric Sunshine
2018-03-04  2:57       ` Dorab Patel
2018-03-04  4:34         ` Eric Sunshine
2018-03-05  2:36     ` Junio C Hamano
2018-03-06 11:54       ` Alexandre Julliard
2018-03-07 21:52         ` Dorab Patel
2018-03-08  9:41           ` Ævar Arnfjörð Bjarmason
2018-03-08  9:45         ` [PATCH] git{,-blame}.el: remove old bitrotting Emacs code Ævar Arnfjörð Bjarmason
2018-03-08 17:27           ` Junio C Hamano
2018-03-10 12:30             ` [PATCH v2] " Ævar Arnfjörð Bjarmason
2018-03-10 16:50               ` Martin Ågren
2018-03-13 18:40                 ` Junio C Hamano
2018-03-08 17:55           ` [PATCH] " Kyle Meyer
2018-03-06  4:38 ` [PATCH v2] git.el: handle default excludesfile properly Dorab Patel

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87fu40c3xe.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=davidk@lysator.liu.se \
    --cc=dorabpatel@gmail.com \
    --cc=fischman@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=julliard@winehq.org \
    --cc=kyle@kyleam.com \
    --cc=martin.agren@gmail.com \
    --cc=sunshine@sunshineco.com \
    --cc=tmz@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox