patchcontrib/hooks/post-receive-email: get description from repo.git/config

Hello,

The included patch attempts to improve post-receive-email. It's a trivial change, but one that helps us Gitolite users. Comments are welcome, and this is my first attempt at contributing to the Git project. Please keep me on CC as I'm not on the list.

From: Trond Hasle Amundsen <t.h.amundsen@usit.uio.no>
Date: Mon, 6 May 2013 15:41:25 +0200
Subject: [PATCH] contrib/hooks/post-receive-email: get description from repo.git/config

When getting the project description, we first try gitweb.description entry in repo.git/config, but repo.git/description takes precedence if it exists. This behaviour mimics that of Gitweb, and is what we want when using Gitolite, which deletes repo.git/description upon repo creation and rather maintains a gitweb.description entry in repo.git/config if a description is configured.

Signed-off-by: Trond Hasle Amundsen <t.h.amundsen@usit.uio.no>
---
 contrib/hooks/post-receive-email |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

Trond Hasle Amundsen <t.h.amundsen@usit.uio.no> writes:

Bah.. It seems I attached this as MIME, sorry about that. Here it is, properly inlined this time:

From: Trond Hasle Amundsen <t.h.amundsen@usit.uio.no>
Date: Mon, 6 May 2013 15:41:25 +0200
Subject: [PATCH] contrib/hooks/post-receive-email: get description from repo.git/config

When getting the project description, we first try gitweb.description entry in repo.git/config, but repo.git/description takes precedence if it exists. This behaviour mimics that of Gitweb, and is what we want when using Gitolite, which deletes repo.git/description upon repo creation and rather maintains a gitweb.description entry in repo.git/config if a description is configured.

Signed-off-by: Trond Hasle Amundsen <t.h.amundsen@usit.uio.no>
---
 contrib/hooks/post-receive-email |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

Trond Hasle Amundsen <t.h.amundsen@usit.uio.no> writes:

Please don't ;-)

Eh, actually, thanks for the patch.

But when you send a patch the next time around, please have the above and the next three lines (i.e. "introductory text") _below_ the three-dash line.

And remove these five lines above. We will read the authorship and subject from the e-mail header of your message.

If description file takes precedence, then the right order to do this would be

    projectdesc=$(sed -ne 1p "$GIT_DIR/description" 2>/dev/null)
    if expr "$projectdesc" : "Unnamed repository" >/dev/null
    then
            : use it as is
    elif projectdesc=$(git config gitweb.description)
    then
            : use it as is
    else
            projectdesc="UNNAMED PROJECT"
    fi

to avoid calling "git config" when it is not even necessary.

We can obviously shorten it by making it less readable, e.g.        

    projectdesc=$(sed -ne 1p "$GIT_DIR/description" 2>/dev/null)

    ! expr "$projectdesc" : "Unnamed repository" >/dev/null ||
    projectdesc=$(git config gitweb.description) ||
    projectdesc="UNNAMED PROJECT"

but I do not think we want to go in that direction ;-)

Junio C Hamano <gitster@pobox.com> writes:

More precisely: you should have a look at git-multimail (in directory contrib/, in branch for now pu/, or from https://github.com/mhagger/git-multimail) before spending time on post-receive-email.

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

Thanks for the tip, I'll check it out. I used post-receive-email out of convenience, as it was already there.

Regards,

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

Oh, by the way, is this a vote of confidence in that topic, hinting that we would want to advance it to 'next'?

As it is only adding a new script to contrib/, it could be argued that we could even push it to 1.8.3-rc1, but until I hear from the original author (who will be the champion for that corner of the contrib/ area), I wouldn't do so.

On 05/07/2013 08:36 AM, Junio C Hamano wrote:

My understanding is that we are waiting on two things:

1. Consensus from the community.  I would characterize the feedback on
the mailing list as limited in quantity but strongly positive [1-4] and
I think that most/all of the wishes for post-receive-email features that
were originally omitted from git-multimail have been implemented in the
current version.  Some of the mailing list feedback was about earlier
versions.  Do you want people to give feedback specifically about the
current version?

2. For me to figure out what part of the git-multimail history I think
should be included in the Git project, do any necessary repository
rewriting, and submit a pull request to you.  The fact that I haven't
gotten to this is due to the fact that I've been busy getting git-imerge
[5] ready to present at GitMerge.

Michael

[1] http://article.gmane.org/gmane.comp.version-control.git/209168 (Branchaud) [2] http://article.gmane.org/gmane.comp.version-control.git/214941 (Bjarmason) [3] http://article.gmane.org/gmane.comp.version-control.git/214987 (Hiestand) [4] http://article.gmane.org/gmane.comp.version-control.git/216705 (Moy) [5] https://github.com/mhagger/git-imerge

Michael Haggerty <mhagger@alum.mit.edu> writes:

Both are intertwined.

I was looking at the history of your project at GitHub. I got an impression that it is better to evolve as a standalone project with its own rich history, and the longer I looked at it, the more convinced I got that I shouldn't pull history from you.

As "batteries included" service for the end users, it may be convenient to have a copy of a stable release of the script in the contrib/ area, but I do not think if it is the best for the script to further develop it in my tree. I'd be just an unnecessary bottleneck.

I have a mildly strong suspicion that a better approach might be to:

 - Copy the current stable snapshot to the contrib/ area, but mark
   it clearly that the copy is merely for convenience, meant for end
   users who choose not to pull from your authoritative GitHub
   repository, and the real development happens elsewhere;

 - Keep the development at your GitHub repository, with you as the
   project lead.  People who are interested in evolving it gather
   and work there; and

 - Update what is in contrib/ in my tree with a stable snapshot,
   every once in a while (close to the release points of Git project
   or of MultiMail project).

Junio C Hamano <gitster@pobox.com> writes:

I think this is not very different from

- merge the current version in the contrib/ area

- keep the development at the GitHub repository

- merge the GitHub repository into the git.git once in a while

(which is essentially what happens with gitk and git-gui as far as I understood)

I tend to prefer the "merge" approach to the "copy files from one repo to another", even if git_multimail is never edited directly from git.git. I find it conceptually cleaner, and it gives a bit more flexibility (e.g. if someone accidentally commits in git.git's repository, the commit would also be valid wrt git_multimail's GitHub repo, and can serve as a pull request).

Michael Haggerty <mhagger@alum.mit.edu> writes:

Ping, now GitMerge is over?

No need to hurry, but just to make sure this didn't disappear from everybody's radar.

Thanks.

On 07/01/2013 11:58 PM, Junio C Hamano wrote:

Yes, and its reverberations are slowly getting under control, too :-)

I definitely haven't forgotten it. I wasn't so happy with the script's Python API, and was holding off on the final submission to avoid casting the old way in stone. But I finally had some time over the last two weekends to convert to what I think is a more sensible system [1]. (I also improved the test coverage considerably.)

Feedback would be very welcome, especially from people who have tried out the old Python API. I am even willing to personally help people adapt to the new API, as it would help me verify that real-life customizations are now indeed easier. (Ævar, any news?)

This week I want to convert $WORK to the new version; after that I would feel comfortable submitting to Git contrib.

(By the way, the changes don't affect how the script can be configured via "git config" settings, or its backwards compatibility with post-receive-email. The improvements are all to the Python API.)

I have a logistical question: git-multimail doesn't have its own mailing list, and GitHub doesn't offer one. I was thinking about setting up a Google group, but a few people at GitMerge suggested that I instead direct discussion of git-multimail to the main Git mailing list. I would slightly prefer that, but I would first like to make sure that the extra traffic (probably not much) would be welcome on the Git mailing list.

Michael

[1] "master" branch at https://github.com/mhagger/git-multimail

Michael Haggerty <mhagger@alum.mit.edu> writes:

I think we shouldn't be worried about the "cluttering" too much before we start. If the volume is too small, the messages relating to multimail may be drawfed by other traffic, but that is the same for messages relating to any other parts of the system.

It only becomes a problem if the volume on multimail is too large but most of the list readers are not interested in them. I somehow find it hard to imagine that will happen soon.

Junio C Hamano <gitster@pobox.com> writes:

I definitely would like to see git-multimail in contrib/, and to have it considered as "the recommended way to send emails from a hook". It does all the old script did, and much more.

post-receive-email can stay for people who don't have Python on their servers, or for existing users who don't want to migrate.

My preference would go for a subtree merge to keep the existing history (that would mean extracting a subdirectory of git-multimail's tree before merging it to git.git).

OTOH, it's not urgent, people can already use git-multimail by taking it from GitHub.

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

Yes. There are less and less reason to rush things into contrib/ these days, which is a very good thing from ecosystem's point of view. It is a sign that Git has matured that my tree no longer has to be the promotion place for third-party add-ons.

Junio C Hamano <gitster@pobox.com> writes:

Allright, noted.

So many rules.. ;) Also noted.

That doesn't work, you'll always call "git config" unless the string matches "Unnamed repository". If you negate the expr line it still doesn't work. To avoid calling "git config" I'd rather suggest something like this:

  projectdesc=$(sed -ne 1p "$GIT_DIR/description" 2>/dev/null)
  if [ -z "$projectdesc" ]; then
          projectdesc=$(git config gitweb.description)
  fi

And let this block remain intact:

  if expr "$projectdesc" : "Unnamed repository.*$" >/dev/null
  then
          projectdesc="UNNAMED PROJECT"
  fi

The only change would then be the three added lines containing the if block that calls "git config" if the "projectdesc" variable is empty. The idea is just to get the description from config if the description file doesn't exist.

Just curious.. why would we avoid calling "git config" unless necessary?

Regards,