Git development
 help / color / mirror / Atom feed
* [PATCH v4 12/12] Add Python support library for remote helpers
From: Sverre Rabbelier @ 2009-11-18  1:42 UTC (permalink / raw)
  To: Git List, Johannes Schindelin, Daniel Barkalow, Johan Herland
  Cc: Sverre Rabbelier, David Aguilar, Johan Herland
In-Reply-To: <1258508552-20752-12-git-send-email-srabbelier@gmail.com>

This patch introduces parts of a Python package called
"git_remote_helpers" containing the building blocks for
remote helpers written in Python.

No actual remote helpers are part of this patch, this patch only
includes the common basics needed to start writing such helpers.

The patch includes the necessary Makefile additions to build and
install the git_remote_helpers Python package along with the rest of
Git.

This patch is based on Johan Herland's git_remote_cvs patch and
has been improved by the following contributions:
- David Aguilar: Lots of Python coding style fixes
- David Aguilar: DESTDIR support in Makefile

Cc: David Aguilar <davvid@gmail.com>
Cc: Johan Herland <johan@herland.net>
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---

	Unchanged.

 Makefile                           |   38 ++
 git_remote_helpers/.gitignore      |    2 +
 git_remote_helpers/Makefile        |   35 ++
 git_remote_helpers/__init__.py     |   16 +
 git_remote_helpers/git/git.py      |  678 ++++++++++++++++++++++++++++++++++++
 git_remote_helpers/setup.py        |   17 +
 git_remote_helpers/util.py         |  194 ++++++++++
 t/test-lib.sh                      |    9 +
 8 files changed, 989 insertions(+), 0 deletions(-)
 create mode 100644 git_remote_helpers/.gitignore
 create mode 100644 git_remote_helpers/Makefile
 create mode 100644 git_remote_helpers/__init__.py
 create mode 100644 git_remote_helpers/git/__init__.py
 create mode 100644 git_remote_helpers/git/git.py
 create mode 100644 git_remote_helpers/setup.py
 create mode 100644 git_remote_helpers/util.py

diff --git a/Makefile b/Makefile
index ed027df..9c8fa67 100644
--- a/Makefile
+++ b/Makefile
@@ -1406,6 +1406,9 @@ endif
 ifndef NO_PERL
 	$(QUIET_SUBDIR0)perl $(QUIET_SUBDIR1) PERL_PATH='$(PERL_PATH_SQ)' prefix='$(prefix_SQ)' all
 endif
+ifndef NO_PYTHON
+	$(QUIET_SUBDIR0)git_remote_helpers $(QUIET_SUBDIR1) PYTHON_PATH='$(PYTHON_PATH_SQ)' prefix='$(prefix_SQ)' all
+endif
 	$(QUIET_SUBDIR0)templates $(QUIET_SUBDIR1)
 
 please_set_SHELL_PATH_to_a_more_modern_shell:
@@ -1524,6 +1527,35 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)) git-instaweb: % : unimplemented.sh
 	mv $@+ $@
 endif # NO_PERL
 
+ifndef NO_PYTHON
+$(patsubst %.py,%,$(SCRIPT_PYTHON)): GIT-CFLAGS
+$(patsubst %.py,%,$(SCRIPT_PYTHON)): % : %.py
+	$(QUIET_GEN)$(RM) $@ $@+ && \
+	INSTLIBDIR=`MAKEFLAGS= $(MAKE) -C git_remote_helpers -s \
+		--no-print-directory prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' \
+		instlibdir` && \
+	sed -e '1{' \
+	    -e '	s|#!.*python|#!$(PYTHON_PATH_SQ)|' \
+	    -e '}' \
+	    -e 's|^import sys.*|&; \\\
+	           import os; \\\
+	           sys.path[0] = os.environ.has_key("GITPYTHONLIB") and \\\
+	                         os.environ["GITPYTHONLIB"] or \\\
+	                         "@@INSTLIBDIR@@"|' \
+	    -e 's|@@INSTLIBDIR@@|'"$$INSTLIBDIR"'|g' \
+	    $@.py >$@+ && \
+	chmod +x $@+ && \
+	mv $@+ $@
+else # NO_PYTHON
+$(patsubst %.py,%,$(SCRIPT_PYTHON)): % : unimplemented.sh
+	$(QUIET_GEN)$(RM) $@ $@+ && \
+	sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
+	    -e 's|@@REASON@@|NO_PYTHON=$(NO_PYTHON)|g' \
+	    unimplemented.sh >$@+ && \
+	chmod +x $@+ && \
+	mv $@+ $@
+endif # NO_PYTHON
+
 configure: configure.ac
 	$(QUIET_GEN)$(RM) $@ $<+ && \
 	sed -e 's/@@GIT_VERSION@@/$(GIT_VERSION)/g' \
@@ -1748,6 +1780,9 @@ install: all
 ifndef NO_PERL
 	$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install
 endif
+ifndef NO_PYTHON
+	$(MAKE) -C git_remote_helpers prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install
+endif
 ifndef NO_TCLTK
 	$(MAKE) -C gitk-git install
 	$(MAKE) -C git-gui gitexecdir='$(gitexec_instdir_SQ)' install
@@ -1865,6 +1900,9 @@ ifndef NO_PERL
 	$(RM) gitweb/gitweb.cgi
 	$(MAKE) -C perl clean
 endif
+ifndef NO_PYTHON
+	$(MAKE) -C git_remote_helpers clean
+endif
 	$(MAKE) -C templates/ clean
 	$(MAKE) -C t/ clean
 ifndef NO_TCLTK
diff --git a/git_remote_helpers/.gitignore b/git_remote_helpers/.gitignore
new file mode 100644
index 0000000..2247d5f
--- /dev/null
+++ b/git_remote_helpers/.gitignore
@@ -0,0 +1,2 @@
+/build
+/dist
diff --git a/git_remote_helpers/Makefile b/git_remote_helpers/Makefile
new file mode 100644
index 0000000..c62dfd0
--- /dev/null
+++ b/git_remote_helpers/Makefile
@@ -0,0 +1,35 @@
+#
+# Makefile for the git_remote_helpers python support modules
+#
+pysetupfile:=setup.py
+
+# Shell quote (do not use $(call) to accommodate ancient setups);
+DESTDIR_SQ = $(subst ','\'',$(DESTDIR))
+
+ifndef PYTHON_PATH
+	PYTHON_PATH = /usr/bin/python
+endif
+ifndef prefix
+	prefix = $(HOME)
+endif
+ifndef V
+	QUIET = @
+	QUIETSETUP = --quiet
+endif
+
+PYLIBDIR=$(shell $(PYTHON_PATH) -c \
+	 "import sys; \
+	 print 'lib/python%i.%i/site-packages' % sys.version_info[:2]")
+
+all: $(pysetupfile)
+	$(QUIET)$(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) build
+
+install: $(pysetupfile)
+	$(PYTHON_PATH) $(pysetupfile) install --prefix $(DESTDIR_SQ)$(prefix)
+
+instlibdir: $(pysetupfile)
+	@echo "$(DESTDIR_SQ)$(prefix)/$(PYLIBDIR)"
+
+clean:
+	$(QUIET)$(PYTHON_PATH) $(pysetupfile) $(QUIETSETUP) clean -a
+	$(RM) *.pyo *.pyc
diff --git a/git_remote_helpers/__init__.py b/git_remote_helpers/__init__.py
new file mode 100644
index 0000000..00f69cb
--- /dev/null
+++ b/git_remote_helpers/__init__.py
@@ -0,0 +1,16 @@
+#!/usr/bin/env python
+
+"""Support library package for git remote helpers.
+
+Git remote helpers are helper commands that interfaces with a non-git
+repository to provide automatic import of non-git history into a Git
+repository.
+
+This package provides the support library needed by these helpers..
+The following modules are included:
+
+- git.git - Interaction with Git repositories
+
+- util - General utility functionality use by the other modules in
+         this package, and also used directly by the helpers.
+"""
diff --git a/git_remote_helpers/git/__init__.py b/git_remote_helpers/git/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/git_remote_helpers/git/git.py b/git_remote_helpers/git/git.py
new file mode 100644
index 0000000..a383e6c
--- /dev/null
+++ b/git_remote_helpers/git/git.py
@@ -0,0 +1,678 @@
+#!/usr/bin/env python
+
+"""Functionality for interacting with Git repositories.
+
+This module provides classes for interfacing with a Git repository.
+"""
+
+import os
+import re
+import time
+from binascii import hexlify
+from cStringIO import StringIO
+import unittest
+
+from git_remote_helpers.util import debug, error, die, start_command, run_command
+
+
+def get_git_dir ():
+    """Return the path to the GIT_DIR for this repo."""
+    args = ("git", "rev-parse", "--git-dir")
+    exit_code, output, errors = run_command(args)
+    if exit_code:
+        die("Failed to retrieve git dir")
+    assert not errors
+    return output.strip()
+
+
+def parse_git_config ():
+    """Return a dict containing the parsed version of 'git config -l'."""
+    exit_code, output, errors = run_command(("git", "config", "-z", "-l"))
+    if exit_code:
+        die("Failed to retrieve git configuration")
+    assert not errors
+    return dict([e.split('\n', 1) for e in output.split("\0") if e])
+
+
+def git_config_bool (value):
+    """Convert the given git config string value to True or False.
+
+    Raise ValueError if the given string was not recognized as a
+    boolean value.
+
+    """
+    norm_value = str(value).strip().lower()
+    if norm_value in ("true", "1", "yes", "on", ""):
+        return True
+    if norm_value in ("false", "0", "no", "off", "none"):
+        return False
+    raise ValueError("Failed to parse '%s' into a boolean value" % (value))
+
+
+def valid_git_ref (ref_name):
+    """Return True iff the given ref name is a valid git ref name."""
+    # The following is a reimplementation of the git check-ref-format
+    # command.  The rules were derived from the git check-ref-format(1)
+    # manual page.  This code should be replaced by a call to
+    # check_ref_format() in the git library, when such is available.
+    if ref_name.endswith('/') or \
+       ref_name.startswith('.') or \
+       ref_name.count('/.') or \
+       ref_name.count('..') or \
+       ref_name.endswith('.lock'):
+        return False
+    for c in ref_name:
+        if ord(c) < 0x20 or ord(c) == 0x7f or c in " ~^:?*[":
+            return False
+    return True
+
+
+class GitObjectFetcher(object):
+
+    """Provide parsed access to 'git cat-file --batch'.
+
+    This provides a read-only interface to the Git object database.
+
+    """
+
+    def __init__ (self):
+        """Initiate a 'git cat-file --batch' session."""
+        self.queue = []  # List of object names to be submitted
+        self.in_transit = None  # Object name currently in transit
+
+        # 'git cat-file --batch' produces binary output which is likely
+        # to be corrupted by the default "rU"-mode pipe opened by
+        # start_command.  (Mode == "rU" does universal new-line
+        # conversion, which mangles carriage returns.) Therefore, we
+        # open an explicitly binary-safe pipe for transferring the
+        # output from 'git cat-file --batch'.
+        pipe_r_fd, pipe_w_fd = os.pipe()
+        pipe_r = os.fdopen(pipe_r_fd, "rb")
+        pipe_w = os.fdopen(pipe_w_fd, "wb")
+        self.proc = start_command(("git", "cat-file", "--batch"),
+                                  stdout = pipe_w)
+        self.f = pipe_r
+
+    def __del__ (self):
+        """Verify completed communication with 'git cat-file --batch'."""
+        assert not self.queue
+        assert self.in_transit is None
+        self.proc.stdin.close()
+        assert self.proc.wait() == 0  # Zero exit code
+        assert self.f.read() == ""  # No remaining output
+
+    def _submit_next_object (self):
+        """Submit queue items to the 'git cat-file --batch' process.
+
+        If there are items in the queue, and there is currently no item
+        currently in 'transit', then pop the first item off the queue,
+        and submit it.
+
+        """
+        if self.queue and self.in_transit is None:
+            self.in_transit = self.queue.pop(0)
+            print >> self.proc.stdin, self.in_transit[0]
+
+    def push (self, obj, callback):
+        """Push the given object name onto the queue.
+
+        The given callback function will at some point in the future
+        be called exactly once with the following arguments:
+        - self - this GitObjectFetcher instance
+        - obj  - the object name provided to push()
+        - sha1 - the SHA1 of the object, if 'None' obj is missing
+        - t    - the type of the object (tag/commit/tree/blob)
+        - size - the size of the object in bytes
+        - data - the object contents
+
+        """
+        self.queue.append((obj, callback))
+        self._submit_next_object()  # (Re)start queue processing
+
+    def process_next_entry (self):
+        """Read the next entry off the queue and invoke callback."""
+        obj, cb = self.in_transit
+        self.in_transit = None
+        header = self.f.readline()
+        if header == "%s missing\n" % (obj):
+            cb(self, obj, None, None, None, None)
+            return
+        sha1, t, size = header.split(" ")
+        assert len(sha1) == 40
+        assert t in ("tag", "commit", "tree", "blob")
+        assert size.endswith("\n")
+        size = int(size.strip())
+        data = self.f.read(size)
+        assert self.f.read(1) == "\n"
+        cb(self, obj, sha1, t, size, data)
+        self._submit_next_object()
+
+    def process (self):
+        """Process the current queue until empty."""
+        while self.in_transit is not None:
+            self.process_next_entry()
+
+    # High-level convenience methods:
+
+    def get_sha1 (self, objspec):
+        """Return the SHA1 of the object specified by 'objspec'.
+
+        Return None if 'objspec' does not specify an existing object.
+
+        """
+        class _ObjHandler(object):
+            """Helper class for getting the returned SHA1."""
+            def __init__ (self, parser):
+                self.parser = parser
+                self.sha1 = None
+
+            def __call__ (self, parser, obj, sha1, t, size, data):
+                # FIXME: Many unused arguments. Could this be cheaper?
+                assert parser == self.parser
+                self.sha1 = sha1
+
+        handler = _ObjHandler(self)
+        self.push(objspec, handler)
+        self.process()
+        return handler.sha1
+
+    def open_obj (self, objspec):
+        """Return a file object wrapping the contents of a named object.
+
+        The caller is responsible for calling .close() on the returned
+        file object.
+
+        Raise KeyError if 'objspec' does not exist in the repo.
+
+        """
+        class _ObjHandler(object):
+            """Helper class for parsing the returned git object."""
+            def __init__ (self, parser):
+                """Set up helper."""
+                self.parser = parser
+                self.contents = StringIO()
+                self.err = None
+
+            def __call__ (self, parser, obj, sha1, t, size, data):
+                """Git object callback (see GitObjectFetcher documentation)."""
+                assert parser == self.parser
+                if not sha1:  # Missing object
+                    self.err = "Missing object '%s'" % obj
+                else:
+                    assert size == len(data)
+                    self.contents.write(data)
+
+        handler = _ObjHandler(self)
+        self.push(objspec, handler)
+        self.process()
+        if handler.err:
+            raise KeyError(handler.err)
+        handler.contents.seek(0)
+        return handler.contents
+
+    def walk_tree (self, tree_objspec, callback, prefix = ""):
+        """Recursively walk the given Git tree object.
+
+        Recursively walk all subtrees of the given tree object, and
+        invoke the given callback passing three arguments:
+        (path, mode, data) with the path, permission bits, and contents
+        of all the blobs found in the entire tree structure.
+
+        """
+        class _ObjHandler(object):
+            """Helper class for walking a git tree structure."""
+            def __init__ (self, parser, cb, path, mode = None):
+                """Set up helper."""
+                self.parser = parser
+                self.cb = cb
+                self.path = path
+                self.mode = mode
+                self.err = None
+
+            def parse_tree (self, treedata):
+                """Parse tree object data, yield tree entries.
+
+                Each tree entry is a 3-tuple (mode, sha1, path)
+
+                self.path is prepended to all paths yielded
+                from this method.
+
+                """
+                while treedata:
+                    mode = int(treedata[:6], 10)
+                    # Turn 100xxx into xxx
+                    if mode > 100000:
+                        mode -= 100000
+                    assert treedata[6] == " "
+                    i = treedata.find("\0", 7)
+                    assert i > 0
+                    path = treedata[7:i]
+                    sha1 = hexlify(treedata[i + 1: i + 21])
+                    yield (mode, sha1, self.path + path)
+                    treedata = treedata[i + 21:]
+
+            def __call__ (self, parser, obj, sha1, t, size, data):
+                """Git object callback (see GitObjectFetcher documentation)."""
+                assert parser == self.parser
+                if not sha1:  # Missing object
+                    self.err = "Missing object '%s'" % (obj)
+                    return
+                assert size == len(data)
+                if t == "tree":
+                    if self.path:
+                        self.path += "/"
+                    # Recurse into all blobs and subtrees
+                    for m, s, p in self.parse_tree(data):
+                        parser.push(s,
+                                    self.__class__(self.parser, self.cb, p, m))
+                elif t == "blob":
+                    self.cb(self.path, self.mode, data)
+                else:
+                    raise ValueError("Unknown object type '%s'" % (t))
+
+        self.push(tree_objspec, _ObjHandler(self, callback, prefix))
+        self.process()
+
+
+class GitRefMap(object):
+
+    """Map Git ref names to the Git object names they currently point to.
+
+    Behaves like a dictionary of Git ref names -> Git object names.
+
+    """
+
+    def __init__ (self, obj_fetcher):
+        """Create a new Git ref -> object map."""
+        self.obj_fetcher = obj_fetcher
+        self._cache = {}  # dict: refname -> objname
+
+    def _load (self, ref):
+        """Retrieve the object currently bound to the given ref.
+
+        The name of the object pointed to by the given ref is stored
+        into this mapping, and also returned.
+
+        """
+        if ref not in self._cache:
+            self._cache[ref] = self.obj_fetcher.get_sha1(ref)
+        return self._cache[ref]
+
+    def __contains__ (self, refname):
+        """Return True if the given refname is present in this cache."""
+        return bool(self._load(refname))
+
+    def __getitem__ (self, refname):
+        """Return the git object name pointed to by the given refname."""
+        commit = self._load(refname)
+        if commit is None:
+            raise KeyError("Unknown ref '%s'" % (refname))
+        return commit
+
+    def get (self, refname, default = None):
+        """Return the git object name pointed to by the given refname."""
+        commit = self._load(refname)
+        if commit is None:
+            return default
+        return commit
+
+
+class GitFICommit(object):
+
+    """Encapsulate the data in a Git fast-import commit command."""
+
+    SHA1RE = re.compile(r'^[0-9a-f]{40}$')
+
+    @classmethod
+    def parse_mode (cls, mode):
+        """Verify the given git file mode, and return it as a string."""
+        assert mode in (644, 755, 100644, 100755, 120000)
+        return "%i" % (mode)
+
+    @classmethod
+    def parse_objname (cls, objname):
+        """Return the given object name (or mark number) as a string."""
+        if isinstance(objname, int):  # Object name is a mark number
+            assert objname > 0
+            return ":%i" % (objname)
+
+        # No existence check is done, only checks for valid format
+        assert cls.SHA1RE.match(objname)  # Object name is valid SHA1
+        return objname
+
+    @classmethod
+    def quote_path (cls, path):
+        """Return a quoted version of the given path."""
+        path = path.replace("\\", "\\\\")
+        path = path.replace("\n", "\\n")
+        path = path.replace('"', '\\"')
+        return '"%s"' % (path)
+
+    @classmethod
+    def parse_path (cls, path):
+        """Verify that the given path is valid, and quote it, if needed."""
+        assert not isinstance(path, int)  # Cannot be a mark number
+
+        # These checks verify the rules on the fast-import man page
+        assert not path.count("//")
+        assert not path.endswith("/")
+        assert not path.startswith("/")
+        assert not path.count("/./")
+        assert not path.count("/../")
+        assert not path.endswith("/.")
+        assert not path.endswith("/..")
+        assert not path.startswith("./")
+        assert not path.startswith("../")
+
+        if path.count('"') + path.count('\n') + path.count('\\'):
+            return cls.quote_path(path)
+        return path
+
+    def __init__ (self, name, email, timestamp, timezone, message):
+        """Create a new Git fast-import commit, with the given metadata."""
+        self.name = name
+        self.email = email
+        self.timestamp = timestamp
+        self.timezone = timezone
+        self.message = message
+        self.pathops = []  # List of path operations in this commit
+
+    def modify (self, mode, blobname, path):
+        """Add a file modification to this Git fast-import commit."""
+        self.pathops.append(("M",
+                             self.parse_mode(mode),
+                             self.parse_objname(blobname),
+                             self.parse_path(path)))
+
+    def delete (self, path):
+        """Add a file deletion to this Git fast-import commit."""
+        self.pathops.append(("D", self.parse_path(path)))
+
+    def copy (self, path, newpath):
+        """Add a file copy to this Git fast-import commit."""
+        self.pathops.append(("C",
+                             self.parse_path(path),
+                             self.parse_path(newpath)))
+
+    def rename (self, path, newpath):
+        """Add a file rename to this Git fast-import commit."""
+        self.pathops.append(("R",
+                             self.parse_path(path),
+                             self.parse_path(newpath)))
+
+    def note (self, blobname, commit):
+        """Add a note object to this Git fast-import commit."""
+        self.pathops.append(("N",
+                             self.parse_objname(blobname),
+                             self.parse_objname(commit)))
+
+    def deleteall (self):
+        """Delete all files in this Git fast-import commit."""
+        self.pathops.append("deleteall")
+
+
+class TestGitFICommit(unittest.TestCase):
+
+    """GitFICommit selftests."""
+
+    def test_basic (self):
+        """GitFICommit basic selftests."""
+
+        def expect_fail (method, data):
+            """Verify that the method(data) raises an AssertionError."""
+            try:
+                method(data)
+            except AssertionError:
+                return
+            raise AssertionError("Failed test for invalid data '%s(%s)'" %
+                                 (method.__name__, repr(data)))
+
+    def test_parse_mode (self):
+        """GitFICommit.parse_mode() selftests."""
+        self.assertEqual(GitFICommit.parse_mode(644), "644")
+        self.assertEqual(GitFICommit.parse_mode(755), "755")
+        self.assertEqual(GitFICommit.parse_mode(100644), "100644")
+        self.assertEqual(GitFICommit.parse_mode(100755), "100755")
+        self.assertEqual(GitFICommit.parse_mode(120000), "120000")
+        self.assertRaises(AssertionError, GitFICommit.parse_mode, 0)
+        self.assertRaises(AssertionError, GitFICommit.parse_mode, 123)
+        self.assertRaises(AssertionError, GitFICommit.parse_mode, 600)
+        self.assertRaises(AssertionError, GitFICommit.parse_mode, "644")
+        self.assertRaises(AssertionError, GitFICommit.parse_mode, "abc")
+
+    def test_parse_objname (self):
+        """GitFICommit.parse_objname() selftests."""
+        self.assertEqual(GitFICommit.parse_objname(1), ":1")
+        self.assertRaises(AssertionError, GitFICommit.parse_objname, 0)
+        self.assertRaises(AssertionError, GitFICommit.parse_objname, -1)
+        self.assertEqual(GitFICommit.parse_objname("0123456789" * 4),
+                         "0123456789" * 4)
+        self.assertEqual(GitFICommit.parse_objname("2468abcdef" * 4),
+                         "2468abcdef" * 4)
+        self.assertRaises(AssertionError, GitFICommit.parse_objname,
+                          "abcdefghij" * 4)
+
+    def test_parse_path (self):
+        """GitFICommit.parse_path() selftests."""
+        self.assertEqual(GitFICommit.parse_path("foo/bar"), "foo/bar")
+        self.assertEqual(GitFICommit.parse_path("path/with\n and \" in it"),
+                         '"path/with\\n and \\" in it"')
+        self.assertRaises(AssertionError, GitFICommit.parse_path, 1)
+        self.assertRaises(AssertionError, GitFICommit.parse_path, 0)
+        self.assertRaises(AssertionError, GitFICommit.parse_path, -1)
+        self.assertRaises(AssertionError, GitFICommit.parse_path, "foo//bar")
+        self.assertRaises(AssertionError, GitFICommit.parse_path, "foo/bar/")
+        self.assertRaises(AssertionError, GitFICommit.parse_path, "/foo/bar")
+        self.assertRaises(AssertionError, GitFICommit.parse_path, "foo/./bar")
+        self.assertRaises(AssertionError, GitFICommit.parse_path, "foo/../bar")
+        self.assertRaises(AssertionError, GitFICommit.parse_path, "foo/bar/.")
+        self.assertRaises(AssertionError, GitFICommit.parse_path, "foo/bar/..")
+        self.assertRaises(AssertionError, GitFICommit.parse_path, "./foo/bar")
+        self.assertRaises(AssertionError, GitFICommit.parse_path, "../foo/bar")
+
+
+class GitFastImport(object):
+
+    """Encapsulate communication with git fast-import."""
+
+    def __init__ (self, f, obj_fetcher, last_mark = 0):
+        """Set up self to communicate with a fast-import process through f."""
+        self.f = f  # File object where fast-import stream is written
+        self.obj_fetcher = obj_fetcher  # GitObjectFetcher instance
+        self.next_mark = last_mark + 1  # Next mark number
+        self.refs = set()  # Keep track of the refnames we've seen
+
+    def comment (self, s):
+        """Write the given comment in the fast-import stream."""
+        assert "\n" not in s, "Malformed comment: '%s'" % (s)
+        self.f.write("# %s\n" % (s))
+
+    def commit (self, ref, commitdata):
+        """Make a commit on the given ref, with the given GitFICommit.
+
+        Return the mark number identifying this commit.
+
+        """
+        self.f.write("""\
+commit %(ref)s
+mark :%(mark)i
+committer %(name)s <%(email)s> %(timestamp)i %(timezone)s
+data %(msgLength)i
+%(msg)s
+""" % {
+    'ref': ref,
+    'mark': self.next_mark,
+    'name': commitdata.name,
+    'email': commitdata.email,
+    'timestamp': commitdata.timestamp,
+    'timezone': commitdata.timezone,
+    'msgLength': len(commitdata.message),
+    'msg': commitdata.message,
+})
+
+        if ref not in self.refs:
+            self.refs.add(ref)
+            parent = ref + "^0"
+            if self.obj_fetcher.get_sha1(parent):
+                self.f.write("from %s\n" % (parent))
+
+        for op in commitdata.pathops:
+            self.f.write(" ".join(op))
+            self.f.write("\n")
+        self.f.write("\n")
+        retval = self.next_mark
+        self.next_mark += 1
+        return retval
+
+    def blob (self, data):
+        """Import the given blob.
+
+        Return the mark number identifying this blob.
+
+        """
+        self.f.write("blob\nmark :%i\ndata %i\n%s\n" %
+                     (self.next_mark, len(data), data))
+        retval = self.next_mark
+        self.next_mark += 1
+        return retval
+
+    def reset (self, ref, objname):
+        """Reset the given ref to point at the given Git object."""
+        self.f.write("reset %s\nfrom %s\n\n" %
+                     (ref, GitFICommit.parse_objname(objname)))
+        if ref not in self.refs:
+            self.refs.add(ref)
+
+
+class GitNotes(object):
+
+    """Encapsulate access to Git notes.
+
+    Simulates a dictionary of object name (SHA1) -> Git note mappings.
+
+    """
+
+    def __init__ (self, notes_ref, obj_fetcher):
+        """Create a new Git notes interface, bound to the given notes ref."""
+        self.notes_ref = notes_ref
+        self.obj_fetcher = obj_fetcher  # Used to get objects from repo
+        self.imports = []  # list: (objname, note data blob name) tuples
+
+    def __del__ (self):
+        """Verify that self.commit_notes() was called before destruction."""
+        if self.imports:
+            error("Missing call to self.commit_notes().")
+            error("%i notes are not committed!", len(self.imports))
+
+    def _load (self, objname):
+        """Return the note data associated with the given git object.
+
+        The note data is returned in string form. If no note is found
+        for the given object, None is returned.
+
+        """
+        try:
+            f = self.obj_fetcher.open_obj("%s:%s" % (self.notes_ref, objname))
+            ret = f.read()
+            f.close()
+        except KeyError:
+            ret = None
+        return ret
+
+    def __getitem__ (self, objname):
+        """Return the note contents associated with the given object.
+
+        Raise KeyError if given object has no associated note.
+
+        """
+        blobdata = self._load(objname)
+        if blobdata is None:
+            raise KeyError("Object '%s' has no note" % (objname))
+        return blobdata
+
+    def get (self, objname, default = None):
+        """Return the note contents associated with the given object.
+
+        Return given default if given object has no associated note.
+
+        """
+        blobdata = self._load(objname)
+        if blobdata is None:
+            return default
+        return blobdata
+
+    def import_note (self, objname, data, gfi):
+        """Tell git fast-import to store data as a note for objname.
+
+        This method uses the given GitFastImport object to create a
+        blob containing the given note data.  Also an entry mapping the
+        given object name to the created blob is stored until
+        commit_notes() is called.
+
+        Note that this method only works if it is later followed by a
+        call to self.commit_notes() (which produces the note commit
+        that refers to the blob produced here).
+
+        """
+        if not data.endswith("\n"):
+            data += "\n"
+        gfi.comment("Importing note for object %s" % (objname))
+        mark = gfi.blob(data)
+        self.imports.append((objname, mark))
+
+    def commit_notes (self, gfi, author, message):
+        """Produce a git fast-import note commit for the imported notes.
+
+        This method uses the given GitFastImport object to create a
+        commit on the notes ref, introducing the notes previously
+        submitted to import_note().
+
+        """
+        if not self.imports:
+            return
+        commitdata = GitFICommit(author[0], author[1],
+                                 time.time(), "0000", message)
+        for objname, blobname in self.imports:
+            assert isinstance(objname, int) and objname > 0
+            assert isinstance(blobname, int) and blobname > 0
+            commitdata.note(blobname, objname)
+        gfi.commit(self.notes_ref, commitdata)
+        self.imports = []
+
+
+class GitCachedNotes(GitNotes):
+
+    """Encapsulate access to Git notes (cached version).
+
+    Only use this class if no caching is done at a higher level.
+
+    Simulates a dictionary of object name (SHA1) -> Git note mappings.
+
+    """
+
+    def __init__ (self, notes_ref, obj_fetcher):
+        """Set up a caching wrapper around GitNotes."""
+        GitNotes.__init__(self, notes_ref, obj_fetcher)
+        self._cache = {}  # Cache: object name -> note data
+
+    def __del__ (self):
+        """Verify that GitNotes' destructor is called."""
+        GitNotes.__del__(self)
+
+    def _load (self, objname):
+        """Extend GitNotes._load() with a local objname -> note cache."""
+        if objname not in self._cache:
+            self._cache[objname] = GitNotes._load(self, objname)
+        return self._cache[objname]
+
+    def import_note (self, objname, data, gfi):
+        """Extend GitNotes.import_note() with a local objname -> note cache."""
+        if not data.endswith("\n"):
+            data += "\n"
+        assert objname not in self._cache
+        self._cache[objname] = data
+        GitNotes.import_note(self, objname, data, gfi)
+
+
+if __name__ == '__main__':
+    unittest.main()
diff --git a/git_remote_helpers/setup.py b/git_remote_helpers/setup.py
new file mode 100644
index 0000000..4d434b6
--- /dev/null
+++ b/git_remote_helpers/setup.py
@@ -0,0 +1,17 @@
+#!/usr/bin/env python
+
+"""Distutils build/install script for the git_remote_helpers package."""
+
+from distutils.core import setup
+
+setup(
+    name = 'git_remote_helpers',
+    version = '0.1.0',
+    description = 'Git remote helper program for non-git repositories',
+    license = 'GPLv2',
+    author = 'The Git Community',
+    author_email = 'git@vger.kernel.org',
+    url = 'http://www.git-scm.com/',
+    package_dir = {'git_remote_helpers': ''},
+    packages = ['git_remote_helpers', 'git_remote_helpers.git'],
+)
diff --git a/git_remote_helpers/util.py b/git_remote_helpers/util.py
new file mode 100644
index 0000000..dce83e6
--- /dev/null
+++ b/git_remote_helpers/util.py
@@ -0,0 +1,194 @@
+#!/usr/bin/env python
+
+"""Misc. useful functionality used by the rest of this package.
+
+This module provides common functionality used by the other modules in
+this package.
+
+"""
+
+import sys
+import os
+import subprocess
+
+
+# Whether or not to show debug messages
+DEBUG = False
+
+def notify(msg, *args):
+    """Print a message to stderr."""
+    print >> sys.stderr, msg % args
+
+def debug (msg, *args):
+    """Print a debug message to stderr when DEBUG is enabled."""
+    if DEBUG:
+        print >> sys.stderr, msg % args
+
+def error (msg, *args):
+    """Print an error message to stderr."""
+    print >> sys.stderr, "ERROR:", msg % args
+
+def warn(msg, *args):
+    """Print a warning message to stderr."""
+    print >> sys.stderr, "warning:", msg % args
+
+def die (msg, *args):
+    """Print as error message to stderr and exit the program."""
+    error(msg, *args)
+    sys.exit(1)
+
+
+class ProgressIndicator(object):
+
+    """Simple progress indicator.
+
+    Displayed as a spinning character by default, but can be customized
+    by passing custom messages that overrides the spinning character.
+
+    """
+
+    States = ("|", "/", "-", "\\")
+
+    def __init__ (self, prefix = "", f = sys.stdout):
+        """Create a new ProgressIndicator, bound to the given file object."""
+        self.n = 0  # Simple progress counter
+        self.f = f  # Progress is written to this file object
+        self.prev_len = 0  # Length of previous msg (to be overwritten)
+        self.prefix = prefix  # Prefix prepended to each progress message
+        self.prefix_lens = [] # Stack of prefix string lengths
+
+    def pushprefix (self, prefix):
+        """Append the given prefix onto the prefix stack."""
+        self.prefix_lens.append(len(self.prefix))
+        self.prefix += prefix
+
+    def popprefix (self):
+        """Remove the last prefix from the prefix stack."""
+        prev_len = self.prefix_lens.pop()
+        self.prefix = self.prefix[:prev_len]
+
+    def __call__ (self, msg = None, lf = False):
+        """Indicate progress, possibly with a custom message."""
+        if msg is None:
+            msg = self.States[self.n % len(self.States)]
+        msg = self.prefix + msg
+        print >> self.f, "\r%-*s" % (self.prev_len, msg),
+        self.prev_len = len(msg.expandtabs())
+        if lf:
+            print >> self.f
+            self.prev_len = 0
+        self.n += 1
+
+    def finish (self, msg = "done", noprefix = False):
+        """Finalize progress indication with the given message."""
+        if noprefix:
+            self.prefix = ""
+        self(msg, True)
+
+
+def start_command (args, cwd = None, shell = False, add_env = None,
+                   stdin = subprocess.PIPE, stdout = subprocess.PIPE,
+                   stderr = subprocess.PIPE):
+    """Start the given command, and return a subprocess object.
+
+    This provides a simpler interface to the subprocess module.
+
+    """
+    env = None
+    if add_env is not None:
+        env = os.environ.copy()
+        env.update(add_env)
+    return subprocess.Popen(args, bufsize = 1, stdin = stdin, stdout = stdout,
+                            stderr = stderr, cwd = cwd, shell = shell,
+                            env = env, universal_newlines = True)
+
+
+def run_command (args, cwd = None, shell = False, add_env = None,
+                 flag_error = True):
+    """Run the given command to completion, and return its results.
+
+    This provides a simpler interface to the subprocess module.
+
+    The results are formatted as a 3-tuple: (exit_code, output, errors)
+
+    If flag_error is enabled, Error messages will be produced if the
+    subprocess terminated with a non-zero exit code and/or stderr
+    output.
+
+    The other arguments are passed on to start_command().
+
+    """
+    process = start_command(args, cwd, shell, add_env)
+    (output, errors) = process.communicate()
+    exit_code = process.returncode
+    if flag_error and errors:
+        error("'%s' returned errors:\n---\n%s---", " ".join(args), errors)
+    if flag_error and exit_code:
+        error("'%s' returned exit code %i", " ".join(args), exit_code)
+    return (exit_code, output, errors)
+
+
+def file_reader_method (missing_ok = False):
+    """Decorator for simplifying reading of files.
+
+    If missing_ok is True, a failure to open a file for reading will
+    not raise the usual IOError, but instead the wrapped method will be
+    called with f == None.  The method must in this case properly
+    handle f == None.
+
+    """
+    def _wrap (method):
+        """Teach given method to handle both filenames and file objects.
+
+        The given method must take a file object as its second argument
+        (the first argument being 'self', of course).  This decorator
+        will take a filename given as the second argument and promote
+        it to a file object.
+
+        """
+        def _wrapped_method (self, filename, *args, **kwargs):
+            if isinstance(filename, file):
+                f = filename
+            else:
+                try:
+                    f = open(filename, 'r')
+                except IOError:
+                    if missing_ok:
+                        f = None
+                    else:
+                        raise
+            try:
+                return method(self, f, *args, **kwargs)
+            finally:
+                if not isinstance(filename, file) and f:
+                    f.close()
+        return _wrapped_method
+    return _wrap
+
+
+def file_writer_method (method):
+    """Decorator for simplifying writing of files.
+
+    Enables the given method to handle both filenames and file objects.
+
+    The given method must take a file object as its second argument
+    (the first argument being 'self', of course).  This decorator will
+    take a filename given as the second argument and promote it to a
+    file object.
+
+    """
+    def _new_method (self, filename, *args, **kwargs):
+        if isinstance(filename, file):
+            f = filename
+        else:
+            # Make sure the containing directory exists
+            parent_dir = os.path.dirname(filename)
+            if not os.path.isdir(parent_dir):
+                os.makedirs(parent_dir)
+            f = open(filename, 'w')
+        try:
+            return method(self, f, *args, **kwargs)
+        finally:
+            if not isinstance(filename, file):
+                f.close()
+    return _new_method
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 0b991db..6ed5b28 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -638,6 +638,15 @@ test -d ../templates/blt || {
 	error "You haven't built things yet, have you?"
 }
 
+if test -z "$GIT_TEST_INSTALLED"
+then
+	GITPYTHONLIB="$(pwd)/../git_remote_helpers/build/lib"
+	export GITPYTHONLIB
+	test -d ../git_remote_helpers/build || {
+		error "You haven't built git_remote_helpers yet, have you?"
+	}
+fi
+
 if ! test -x ../test-chmtime; then
 	echo >&2 'You need to build test-chmtime:'
 	echo >&2 'Run "make test-chmtime" in the source (toplevel) directory'
-- 
1.6.5.3.164.g07b0c

^ permalink raw reply related

* [PATCH v4 08/12] Allow helper to map private ref names into normal names
From: Sverre Rabbelier @ 2009-11-18  1:42 UTC (permalink / raw)
  To: Git List, Johannes Schindelin, Daniel Barkalow, Johan Herland
  Cc: Daniel Barkalow
In-Reply-To: <1258508552-20752-8-git-send-email-srabbelier@gmail.com>

From: Daniel Barkalow <barkalow@iabervon.org>

This allows a helper to say that, when it handles "import
refs/heads/topic", the script it outputs will actually write to
refs/svn/origin/branches/topic; therefore, transport-helper should
read it from the latter location after git-fast-import completes.

Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
---

	This has Daniel's documentation and leak fixes squashed in.

 Documentation/git-remote-helpers.txt |   16 +++++++++++++++-
 remote.c                             |   27 +++++++++++++++++++++++++++
 remote.h                             |    5 +++++
 transport-helper.c                   |   34 +++++++++++++++++++++++++++++++++-
 4 files changed, 80 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index e9aa67e..d6c5268 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -46,7 +46,11 @@ Supported if the helper has the "fetch" capability.
 'import' <name>::
 	Produces a fast-import stream which imports the current value
 	of the named ref. It may additionally import other refs as
-	needed to construct the history efficiently.
+	needed to construct the history efficiently. The script writes
+	to a helper-specific private namespace. The value of the named
+	ref should be written to a location in this namespace derived
+	by applying the refspecs from the "refspec" capability to the
+	name of the ref.
 +
 Supported if the helper has the "import" capability.
 
@@ -67,6 +71,16 @@ CAPABILITIES
 'import'::
 	This helper supports the 'import' command.
 
+'refspec' 'spec'::
+	When using the import command, expect the source ref to have
+	been written to the destination ref. The earliest applicable
+	refspec takes precedence. For example
+	"refs/heads/*:refs/svn/origin/branches/*" means that, after an
+	"import refs/heads/name", the script has written to
+	refs/svn/origin/branches/name. If this capability is used at
+	all, it must cover all refs reported by the list command; if
+	it is not used, it is effectively "*:*"
+
 REF LIST ATTRIBUTES
 -------------------
 
diff --git a/remote.c b/remote.c
index 09bb79c..1f7870d 100644
--- a/remote.c
+++ b/remote.c
@@ -673,6 +673,16 @@ static struct refspec *parse_push_refspec(int nr_refspec, const char **refspec)
 	return parse_refspec_internal(nr_refspec, refspec, 0, 0);
 }
 
+void free_refspec(int nr_refspec, struct refspec *refspec)
+{
+	int i;
+	for (i = 0; i < nr_refspec; i++) {
+		free(refspec[i].src);
+		free(refspec[i].dst);
+	}
+	free(refspec);
+}
+
 static int valid_remote_nick(const char *name)
 {
 	if (!name[0] || is_dot_or_dotdot(name))
@@ -811,6 +821,23 @@ static int match_name_with_pattern(const char *key, const char *name,
 	return ret;
 }
 
+char *apply_refspecs(struct refspec *refspecs, int nr_refspec,
+		     const char *name)
+{
+	int i;
+	char *ret = NULL;
+	for (i = 0; i < nr_refspec; i++) {
+		struct refspec *refspec = refspecs + i;
+		if (refspec->pattern) {
+			if (match_name_with_pattern(refspec->src, name,
+						    refspec->dst, &ret))
+				return ret;
+		} else if (!strcmp(refspec->src, name))
+			return strdup(refspec->dst);
+	}
+	return NULL;
+}
+
 int remote_find_tracking(struct remote *remote, struct refspec *refspec)
 {
 	int find_src = refspec->src == NULL;
diff --git a/remote.h b/remote.h
index ac0ce2f..cdc3b5b 100644
--- a/remote.h
+++ b/remote.h
@@ -91,6 +91,11 @@ void ref_remove_duplicates(struct ref *ref_map);
 int valid_fetch_refspec(const char *refspec);
 struct refspec *parse_fetch_refspec(int nr_refspec, const char **refspec);
 
+void free_refspec(int nr_refspec, struct refspec *refspec);
+
+char *apply_refspecs(struct refspec *refspecs, int nr_refspec,
+		     const char *name);
+
 int match_refs(struct ref *src, struct ref **dst,
 	       int nr_refspec, const char **refspec, int all);
 
diff --git a/transport-helper.c b/transport-helper.c
index 82caaae..da8185a 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -5,6 +5,7 @@
 #include "commit.h"
 #include "diff.h"
 #include "revision.h"
+#include "remote.h"
 
 struct helper_data
 {
@@ -12,6 +13,9 @@ struct helper_data
 	struct child_process *helper;
 	unsigned fetch : 1;
 	unsigned import : 1;
+	/* These go from remote name (as in "list") to private name */
+	struct refspec *refspecs;
+	int refspec_nr;
 };
 
 static struct child_process *get_helper(struct transport *transport)
@@ -20,6 +24,9 @@ static struct child_process *get_helper(struct transport *transport)
 	struct strbuf buf = STRBUF_INIT;
 	struct child_process *helper;
 	FILE *file;
+	const char **refspecs = NULL;
+	int refspec_nr = 0;
+	int refspec_alloc = 0;
 
 	if (data->helper)
 		return data->helper;
@@ -51,6 +58,21 @@ static struct child_process *get_helper(struct transport *transport)
 			data->fetch = 1;
 		if (!strcmp(buf.buf, "import"))
 			data->import = 1;
+		if (!data->refspecs && !prefixcmp(buf.buf, "refspec ")) {
+			ALLOC_GROW(refspecs,
+				   refspec_nr + 1,
+				   refspec_alloc);
+			refspecs[refspec_nr++] = strdup(buf.buf + strlen("refspec "));
+		}
+	}
+	if (refspecs) {
+		int i;
+		data->refspec_nr = refspec_nr;
+		data->refspecs = parse_fetch_refspec(refspec_nr, refspecs);
+		for (i = 0; i < refspec_nr; i++) {
+			free((char *)refspecs[i]);
+		}
+		free(refspecs);
 	}
 	return data->helper;
 }
@@ -72,6 +94,9 @@ static int disconnect_helper(struct transport *transport)
 
 static int release_helper(struct transport *transport)
 {
+	struct helper_data *data = transport->data;
+	free_refspec(data->refspec_nr, data->refspecs);
+	data->refspecs = NULL;
 	disconnect_helper(transport);
 	free(transport->data);
 	return 0;
@@ -119,6 +144,7 @@ static int fetch_with_import(struct transport *transport,
 {
 	struct child_process fastimport;
 	struct child_process *helper = get_helper(transport);
+	struct helper_data *data = transport->data;
 	int i;
 	struct ref *posn;
 	struct strbuf buf = STRBUF_INIT;
@@ -139,10 +165,16 @@ static int fetch_with_import(struct transport *transport,
 	finish_command(&fastimport);
 
 	for (i = 0; i < nr_heads; i++) {
+		char *private;
 		posn = to_fetch[i];
 		if (posn->status & REF_STATUS_UPTODATE)
 			continue;
-		read_ref(posn->name, posn->old_sha1);
+		if (data->refspecs)
+			private = apply_refspecs(data->refspecs, data->refspec_nr, posn->name);
+		else
+			private = strdup(posn->name);
+		read_ref(private, posn->old_sha1);
+		free(private);
 	}
 	return 0;
 }
-- 
1.6.5.3.164.g07b0c

^ permalink raw reply related

* Re: [PATCH] (to improve gitosis support) CVS Server: Support reading  base and roots from environment
From: Phil Miller @ 2009-11-18  4:11 UTC (permalink / raw)
  To: Git Mailing List
In-Reply-To: <81f018ac0911120925w848594dt276735955681e8f@mail.gmail.com>

I've seen no response or comment on this since sending it in. Does it
need revision of some sort? Did it just get lost in the shuffle? I'll
be happy to do the legwork to get this integrated, so that I don't
have to carry it in a private git tree.

Phil

On Thu, Nov 12, 2009 at 11:25, Phil Miller <mille121@illinois.edu> wrote:
> The Gitosis single-account Git/ssh hosting system runs git commands
> through git-shell after confirming that the connecting user is
> authorized to access the requested repository. This works well for
> upload-pack and receive-pack, which take a repository argument through
> git-shell. This doesn't work so well for `cvs server', which is passed
> through literally, with no arguments. Allowing arguments risks
> sneaking in `--export-all', so that restriction should be maintained.
>
> Despite that, passing a list of repository roots is necessary for
> per-user access control by the hosting software, and passing a base
> path improves usability without weakening security. Thus,
> git-cvsserver needs to come up with these values at runtime by some
> other means. Since git-shell preserves the environment for other
> purposes, the environment can carry these arguments as well.
>
> Thus, modify git-cvsserver to read $GIT_CVSSERVER_{BASE_PATH,ROOTS} in
> the absence of equivalent command line arguments.
>
> Signed-off-by: Phil Miller <mille121@illinois.edu>
>

^ permalink raw reply

* Re:
From: Anna @ 2009-11-18  5:03 UTC (permalink / raw)


Piedalies pirmaja Latvijas BEZMAKSAS pokera TV show, vinne celojumu uz Las Vegasu kur galvena balva ir $8.000.000!
http://latvijastvshovs1.co.nr

^ permalink raw reply

* [RFC PATCH 5/6] gitweb: Allow per-repository definition of new committags
From: Marcel M. Cary @ 2009-11-18  6:22 UTC (permalink / raw)
  To: Jakub Narebski, git
  Cc: Petr Baudis, Giuseppe Bilotta, Francis Galiegue, Marcel M. Cary
In-Reply-To: <1258525350-5528-5-git-send-email-marcel@oak.homeunix.org>

Committags are limited to the functionality configured by the site
administrator.

Provide two more general purpose committag subroutines that replace
text by feeding the capturing groups of a pattern to a sprintf format.
One additionally escapes the parameters of the capturing groups for
producing HTML snippets, the other does not.

Then, if permitted by the site administrator, allow the 'sub' key to
be overridden in an existing committag and allow a new committag to be
defined completely from within the repository configuration.

Signed-off-by: Marcel M. Cary <marcel@oak.homeunix.org>
---
 gitweb/gitweb.perl           |  135 ++++++++++++++++++++++++++++--------------
 t/t9502-gitweb-committags.sh |   50 +++++++++++++++
 2 files changed, 141 insertions(+), 44 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 7f7d3a3..d413f22 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -214,8 +214,7 @@ our %avatar_size = (
 );
 
 # In general, the site admin can enable/disable per-project
-# configuration of each committag.  Only the 'options' part of the
-# committag is configurable per-project.
+# configuration of each committag.
 #
 # The site admin can of course add new tags to this hash or override
 # the 'sub' key if necessary.  But such changes may be fragile; this
@@ -241,12 +240,18 @@ our %avatar_size = (
 # on the implementation of the 'sub' key.  The hyperlink_committag
 # value appends the first captured group to the 'url' option, for example.
 #
+# The project configuration can define new committags.  Although the
+# project configuration cannot supply code defining a new 'sub' key,
+# the project configuration can choose from a list of pre-approved
+# subroutines named in the 'allowed_committag_subs' feature.  Both a
+# 'sub' key and 'pattern' key must be defined for the committag to be
+# used.  If the 'allowed_committag_subs' feature is empty, no new
+# committags can be defined in the project config.
+#
 our %committags = (
 	# Link Git-style hashes to this gitweb
 	'sha1' => {
-		'options' => {
-			'pattern' => qr/\b([0-9a-fA-F]{8,40})\b/,
-		},
+		'pattern' => qr/\b([0-9a-fA-F]{8,40})\b/,
 		'override' => 0,
 		'sub' => sub {
 			my ($opts, @match) = @_;
@@ -258,10 +263,8 @@ our %committags = (
 	# Facilitate styling of common header/footer lines, suppressing
 	# any trailing newlines
 	'signoff' => {
-		'options' => {
-			'pattern' =>
-				qr/^( *(?:signed[ \-]off[ \-]by|acked[ \-]by|cc)[ :].*)\n*$/mi,
-		},
+		'pattern' =>
+			qr/^( *(?:signed[ \-]off[ \-]by|acked[ \-]by|cc)[ :].*)\n*$/mi,
 		'override' => 0,
 		'sub' => sub {
 			my ($opts, @match) = @_;
@@ -273,23 +276,19 @@ our %committags = (
 	# Link bug/features to Mantis bug tracker using Mantis-style
 	# contextual cues
 	'mantis' => {
-		'options' => {
-			'pattern' => qr/(?:BUG|FEATURE)\((\d+)\)/,
-			'url' => 'http://www.example.com/mantisbt/view.php?id=',
-		},
+		'pattern' => qr/(?:BUG|FEATURE)\((\d+)\)/,
+		'url' => 'http://www.example.com/mantisbt/view.php?id=',
 		'override' => 0,
 		'sub' => \&hyperlink_committag,
 	},
 	# Link mentions of bugs to bugzilla, allowing for separate outer
 	# and inner regexes (see unit test for example)
 	'bugzilla' => {
-		'options' => {
-			'pattern' => qr/(?i:bugs?):?\s+
-			                [#]?\d+(?:(?:,\s*|,?\sand\s|,?\sn?or\s|\s+)
-			                          [#]?\d+\b)*/x,
-			'innerpattern' => qr/#?(\d+)/,
-			'url' => 'http://bugzilla.example.com/show_bug.cgi?id=',
-		},
+		'pattern' => qr/(?i:bugs?):?\s+
+		                [#]?\d+(?:(?:,\s*|,?\sand\s|,?\sn?or\s|\s+)
+		                          [#]?\d+\b)*/x,
+		'innerpattern' => qr/#?(\d+)/,
+		'url' => 'http://bugzilla.example.com/show_bug.cgi?id=',
 		'override' => 0,
 		'sub' => sub {
 			my ($opts, @match) = @_;
@@ -308,15 +307,13 @@ our %committags = (
 	},
 	# Link URLs
 	'url' => {
-		'options' => {
-			# Avoid matching punctuation that might immediately follow
-			# a url, is not part of the url, and is allowed in urls,
-			# like a full-stop ('.').
-			'pattern' => qr!(https?|ftps?|git|ssh|ssh+git|sftp|smb|webdavs?|
-			                 nfs|irc|nntp|rsync)
-			                ://[-_a-zA-Z0-9\@/&=+~#<>;%:.?]+
-			                   [-_a-zA-Z0-9\@/&=+~#<>]!x,
-		},
+		# Avoid matching punctuation that might immediately follow
+		# a url, is not part of the url, and is allowed in urls,
+		# like a full-stop ('.').
+		'pattern' => qr!(https?|ftps?|git|ssh|ssh+git|sftp|smb|webdavs?|
+		                 nfs|irc|nntp|rsync)
+		                ://[-_a-zA-Z0-9\@/&=+~#<>;%:.?]+
+		                   [-_a-zA-Z0-9\@/&=+~#<>]!x,
 		'override' => 0,
 		'sub' => sub {
 			my ($opts, @match) = @_;
@@ -327,10 +324,8 @@ our %committags = (
 	},
 	# Link Message-Id to mailing list archive
 	'messageid' => {
-		'options' => {
-			'pattern' => qr!(?:message|msg)-?id:?\s+(<[^>]+>)!i,
-			'url' => 'http://mid.gmane.org/',
-		},
+		'pattern' => qr!(?:message|msg)-?id:?\s+(<[^>]+>)!i,
+		'url' => 'http://mid.gmane.org/',
 		'override' => 0,
 		# Includes the "msg-id" text in the link text.
 		# Since we don't support linking multiple msg-ids in one match, we
@@ -558,6 +553,22 @@ our %feature = (
 		'sub' => sub { feature_list('committags', @_) },
 		'override' => 0,
 		'default' => ['signoff', 'sha1']},
+
+	# The list of committag callbacks that are permitted to be used
+	# from within a repository configuration.  These are interpretted
+	# as perl subrouting names.  If none are listed, no new committags
+	# can be defined in the project config, which is the default.
+
+	# To enable system wide have in $GITWEB_CONFIG
+	# $feature{'allowed_committag_subs'}{'default'} = [
+	#		'hyperlink_committag',
+	#		'markup_committag',
+	#		'transform_committag',
+	#		];
+	# It would not make sense to allow per-project overrides of this.
+	'allowed_committag_subs' => {
+		'override' => 0,
+		'default' => []},
 );
 
 sub gitweb_get_feature {
@@ -1030,6 +1041,9 @@ if ($git_avatar eq 'gravatar') {
 # ordering of committags
 our @committags = gitweb_get_feature('committags');
 
+# ordering of committags
+our @allowed_committag_subs = gitweb_get_feature('allowed_committag_subs');
+
 # whether we've loaded committags for the project yet
 our $loaded_project_committags = 0;
 
@@ -1046,9 +1060,18 @@ sub gitweb_load_project_committags {
 			split(/\./, $key, 4);
 		$project_config{$ctname}{$option} = $raw_config{$key};
 	}
-	foreach my $ctname (keys(%committags)) {
-		my $override = $committags{$ctname}{'override'};
+
+	my %allowed_subs = ();
+	foreach my $sub (@allowed_committag_subs) {
+		$allowed_subs{$sub} = 1;
+	}
+
+	foreach my $ctname (keys(%project_config)) {
+		my $override = $committags{$ctname}
+			? $committags{$ctname}{'override'}
+			: 1;
 		next if (!$override);
+
 		my $override_keys = undef;
 		if (ref($override) eq "ARRAY") {
 			$override_keys = {};
@@ -1056,12 +1079,19 @@ sub gitweb_load_project_committags {
 				$override_keys->{$optname} = 1;
 			}
 		}
+
 		foreach my $optname (keys %{$project_config{$ctname}}) {
 			next if ($override_keys && !$override_keys->{$optname});
-			$committags{$ctname}{'options'}{$optname} =
-				$project_config{$ctname}{$optname};
+			my $value = $project_config{$ctname}{$optname};
+			if ($optname eq 'sub') {
+				if (!$allowed_subs{$value}) {
+					next;
+				}
+			}
+			$committags{$ctname}{$optname} = $value;
 		}
 	}
+
 	$loaded_project_committags = 1;
 }
 
@@ -1654,11 +1684,8 @@ COMMITTAG:
 		next COMMITTAG unless exists $committag->{'sub'};
 		my $sub = $committag->{'sub'};
 
-		next COMMITTAG unless exists $committag->{'options'};
-		my $opts = $committag->{'options'};
-
-		next COMMITTAG unless exists $opts->{'pattern'};
-		my $pattern = $opts->{'pattern'};
+		next COMMITTAG unless exists $committag->{'pattern'};
+		my $pattern = $committag->{'pattern'};
 
 		my @new_message_fragments = ();
 
@@ -1672,7 +1699,8 @@ COMMITTAG:
 
 			push_or_append_replacements(\@new_message_fragments,
 			                            $pattern, $fragment, sub {
-					$sub->($opts, @_);
+					no strict "refs"; # for custome committags
+					$sub->($committag, @_);
 				});
 
 		} # end foreach (@message_fragments)
@@ -1705,6 +1733,25 @@ sub hyperlink_committag {
 	                esc_html($match[0], -nbsp=>1));
 }
 
+# Returns a ref to an HTML snippet formed from the 'replacement'
+# option and match data.  The match data is HTML-escaped, and the
+# 'replacement' option is used as a sprintf format.  This is a helper
+# function used in %committags.
+sub markup_committag {
+	my ($opts, @match) = @_;
+	return \sprintf($opts->{'replacement'},
+	                map { esc_html($_, -nbsp=>1) if defined } @match);
+}
+
+# Returns a text snippet formed from the 'replacement' option and
+# match data.  The 'replacement' option is used as a sprintf format.
+# Because the result is text, it can be re-processed by subsequent
+# committags.  This is a helper function used in %committags.
+sub transform_committag {
+	my ($opts, @match) = @_;
+	return sprintf($opts->{'replacement'}, @match);
+}
+
 # Find $pattern in string $fragment, and push_or_append the parts
 # between matches and the result of calling $sub with matched text to
 # $new_fragments.
@@ -1717,7 +1764,7 @@ MATCH:
 	while ($fragment =~ m/$pattern/gc) {
 		my ($prepos, $postpos) = ($-[0], $+[0]);
 
-		my @repl = $sub->($&, $1);
+		my @repl = $sub->($&, $1, $2);
 
 		my $pre = substr($fragment, $oldpos, $prepos - $oldpos);
 		push_or_append($new_fragments, $pre);
diff --git a/t/t9502-gitweb-committags.sh b/t/t9502-gitweb-committags.sh
index 0753630..cbe607b 100755
--- a/t/t9502-gitweb-committags.sh
+++ b/t/t9502-gitweb-committags.sh
@@ -226,5 +226,55 @@ test_expect_success 'msgid link: linked when enabled' '
 test_debug 'cat gitweb.log'
 test_debug 'grep -F "y.z" gitweb.output'
 
+# ----------------------------------------------------------------------
+# custom committags
+#
+echo custom_test > file.txt
+git add file.txt
+git commit -q -F - file.txt <<END
+Something for <foo&bar@bar.com>
+END
+echo '$feature{"allowed_committag_subs"}{"default"} = [
+	"hyperlink_committag",
+	"markup_committag",
+	"transform_committag",
+	];' >> gitweb_config.perl
+git config gitweb.committags 'sha1, obfuscate'
+git config gitweb.committag.obfuscate.pattern '([a-z&]+@)[a-z]+(.com)'
+git config gitweb.committag.obfuscate.sub 'transform_committag'
+git config gitweb.committag.obfuscate.replacement '%2$sXXX%3$s'
+test_expect_success 'custom committags: transform_committag' '
+	gitweb_run "p=.git;a=commit;h=HEAD" &&
+	grep -q -F \
+		"foo&amp;bar@XXX.com" \
+		gitweb.output
+'
+test_debug 'cat gitweb.log'
+test_debug 'grep -F "foo" gitweb.output'
+
+git config gitweb.committags 'sha1, linkemail'
+git config gitweb.committag.linkemail.pattern '<([a-z&]+@[a-z]+.com)>'
+git config gitweb.committag.linkemail.sub 'markup_committag'
+git config gitweb.committag.linkemail.replacement '<a href="mailto:%2$s">%1$s</a>'
+test_expect_success 'custom committags: markup_committag' '
+	gitweb_run "p=.git;a=commit;h=HEAD" &&
+	grep -q -F \
+		"<a href=\"mailto:foo&amp;bar@bar.com\">&lt;foo&amp;bar@bar.com&gt;</a>" \
+		gitweb.output
+'
+test_debug 'cat gitweb.log'
+test_debug 'grep -F "foo" gitweb.output'
+
+echo '$feature{"allowed_committag_subs"}{"default"} = [
+	];' >> gitweb_config.perl
+test_expect_success 'custom committags: ignored when disabled' '
+	gitweb_run "p=.git;a=commit;h=HEAD" &&
+	grep -q -F \
+		"Something&nbsp;for&nbsp;&lt;foo&amp;bar@bar.com&gt;" \
+		gitweb.output
+'
+test_debug 'cat gitweb.log'
+test_debug 'grep -F "foo" gitweb.output'
+
 
 test_done
-- 
1.6.4.4

^ permalink raw reply related

* [RFC PATCH 3/6] gitweb: Allow finer-grained override controls for committags
From: Marcel M. Cary @ 2009-11-18  6:22 UTC (permalink / raw)
  To: Jakub Narebski, git
  Cc: Petr Baudis, Giuseppe Bilotta, Francis Galiegue, Marcel M. Cary
In-Reply-To: <1258525350-5528-3-git-send-email-marcel@oak.homeunix.org>

Currently, a site administrator must choose between allowing all or
none of a committag's options to be overridden in the project config.
However, a site admin may wish to permit specifying a bugzilla URL
without risking a maliciously resource hungry regular expression.

Allow the site admin to specify which committag parameters may be
overridden.  Preserve the behavior of the original 0 and 1 override
specifications.

Signed-off-by: Marcel M. Cary <marcel@oak.homeunix.org>
---
 gitweb/INSTALL               |    8 +++++++-
 gitweb/gitweb.perl           |   24 ++++++++++++++++++------
 t/t9502-gitweb-committags.sh |   13 +++++++++++++
 3 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/gitweb/INSTALL b/gitweb/INSTALL
index 9081ed8..15c0128 100644
--- a/gitweb/INSTALL
+++ b/gitweb/INSTALL
@@ -133,9 +133,15 @@ adding the following lines to your $GITWEB_CONFIG:
 	$known_snapshot_formats{'tgz'}{'compressor'} = ['gzip','-6'];
 
 To add a committag to the default list of commit tags, for example to
-enable hyperlinking of bug numbers to a bug tracker for all projects:
+enable hyperlinking of bug numbers to a bug tracker for all projects, while
+allowing each project to choose only the base URL for its bug tracker:
 
 	push @{$feature{'committags'}{'default'}}, 'bugzilla';
+	$committags{"bugzilla"}{"override"} = ["url"];
+
+And then let each project configure its bug tracker URL:
+
+	git config gitweb.committag.bugzilla.url 'http://bts.example.com?bug='
 
 
 Gitweb repositories
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 032b1c5..8f4480e 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -225,11 +225,13 @@ our %avatar_size = (
 # will not be processed further.
 #
 # For any committag, set the 'override' key to 1 to allow individual
-# projects to override entries in the 'options' hash for that tag.
-# For example, to match only commit hashes given in lowercase in one
-# project, add this to the $GITWEB_CONFIG:
+# projects to override any entry in the 'options' hash for that tag.
+# Leave 'override' as 0 to disallow all overriding of all entries.
+# Set 'override' to an array of 'option' key names to allow overriding
+# specific keys.  For example, to match only commit hashes given in
+# lowercase in one project, add this to the $GITWEB_CONFIG:
 #
-#     $committags{'sha1'}{'override'} = 1;
+#     $committags{'sha1'}{'override'} = 1;   # or ["pattern"]
 #
 # And in the project's config:
 #
@@ -237,7 +239,8 @@ our %avatar_size = (
 #
 # Some committags have additional options whose interpretation depends
 # on the implementation of the 'sub' key.  The hyperlink_committag
-# value appends the first captured group to the 'url' option.
+# value appends the first captured group to the 'url' option, for example.
+#
 our %committags = (
 	# Link Git-style hashes to this gitweb
 	'sha1' => {
@@ -1029,8 +1032,17 @@ sub gitweb_load_project_committags {
 		$project_config{$ctname}{$option} = $raw_config{$key};
 	}
 	foreach my $ctname (keys(%committags)) {
-		next if (!$committags{$ctname}{'override'});
+		my $override = $committags{$ctname}{'override'};
+		next if (!$override);
+		my $override_keys = undef;
+		if (ref($override) eq "ARRAY") {
+			$override_keys = {};
+			foreach my $optname (@$override) {
+				$override_keys->{$optname} = 1;
+			}
+		}
 		foreach my $optname (keys %{$project_config{$ctname}}) {
+			next if ($override_keys && !$override_keys->{$optname});
 			$committags{$ctname}{'options'}{$optname} =
 				$project_config{$ctname}{$optname};
 		}
diff --git a/t/t9502-gitweb-committags.sh b/t/t9502-gitweb-committags.sh
index 718e763..e13ac47 100755
--- a/t/t9502-gitweb-committags.sh
+++ b/t/t9502-gitweb-committags.sh
@@ -68,6 +68,19 @@ test_expect_success 'bugzilla: url overridden but not permitted' '
 test_debug 'cat gitweb.log'
 test_debug 'grep 1234 gitweb.output'
 
+echo '$committags{"bugzilla"}{"override"} = ["url"];' >> gitweb_config.perl
+git config gitweb.committag.bugzilla.url 'http://bts.example.com?bug='
+git config gitweb.committag.bugzilla.pattern 'slow DoS regex'
+test_expect_success 'bugzilla: url overridden but regex not permitted' '
+	gitweb_run "p=.git;a=commit;h=HEAD" &&
+	grep -F -q \
+		"Fixes&nbsp;bug&nbsp;<a class=\"text\" href=\"http://bts.example.com?bug=1234\">1234</a>&nbsp;involving" \
+		gitweb.output
+'
+test_debug 'cat gitweb.log'
+test_debug 'grep 1234 gitweb.output'
+git config --unset gitweb.committag.bugzilla.pattern
+
 echo '$committags{"bugzilla"}{"override"} = 1;' >> gitweb_config.perl
 test_expect_success 'bugzilla: url overridden' '
 	gitweb_run "p=.git;a=commit;h=HEAD" &&
-- 
1.6.4.4

^ permalink raw reply related

* [RFC PATCH 0/6] Second round of committag series
From: Marcel M. Cary @ 2009-11-18  6:22 UTC (permalink / raw)
  To: Jakub Narebski, git
  Cc: Petr Baudis, Giuseppe Bilotta, Francis Galiegue, Marcel M. Cary
In-Reply-To: <200906221318.19598.jnareb@gmail.com>

Thanks for the feedback.  I've added four more patches to the end of
the series and updated the first two.  My replies are below.

On Mon, 22 Jun 2009, Jakub Narebski wrote:
> On Fri, 19 June 2009, Marcel M. Cary wrote:
> 
> Thanks for diligently working on this issue.  Good work!
> 
> I see that it is an RFC, and not final submission, but just in case
> I'd like to remind you that some of information below should go into
> commit message, but some of it should I think go to comments (between
> "---" and diffstat).
> 
> > I want gitweb to hyperlink commits to my bug tracking system so that
> > information regarding the current status of a commit can be easily
> > cross-referenced.  For example, the QA and release status of a commit
> > cannot be inserted into the comment.  Maybe someday a "git notes"
> > feature will help with this, but for now, my organization has a
> > separate bug tracking system.  Other repository browsers such as
> > unfuddle and websvn support similar features.
> 
> The paragraph above should, I think, be made more clear.  You don't
> need to mention what you don't do; the comment about "git notes" should
> be not in commit message but in comments section.
> 
> What you want to have is to have some markers in commit message 
> (committags) hyperlinked; namely you want notifications about bug/issue
> numbers in the commit message hyperlinked to appropriate bugtracker/issue
> tracker URL.  Do I understand this correctly?

I'm not sure I would call the context of the bug numbers
"notifications".  A good example of what I'm looking for is to
hyperlink "Resolves-bug: 1234" to
http://bugzilla.example.com/show_bug.cgi?bug_id=1234.  I suppose this
could be seen as a notification of bug 1234's resolution, except that
I expect the more complex bugs (bugs, issues, and features) to require
several commits to resolve, each of which I'd like tagged with that
bug number, and so the bug would not really be resolved after the
first such commit.

> I tried here to reword what you said, to come up with better commit
> message for a future final submission.

I've rewritten the commit message paragraph as you suggested.  Is it
now more inline with what you'd like to see?

> > Since the bug hyperlinking feature was previously discussed as part of
> > "committags," a more general mechanism to embellish commit messages,
> > implement the more general mechanism instead, including the following
> > capabilities:
> 
> Well, I think that the fact that it would be not much harder to create
> general mechanism for commit message transformation, than to add 
> suitably generic and well customizable support for bugtracker 
> 'committags'.

So far, the biggest difficulty in generalizing the bugzilla hyperlink
feature has been the interaction between two or more commit tag
transformations -- how to structure them so they are composable but
decoupled.

> > * Hyperlinking mentions of bug IDs to Bugzilla
> > * Hyperlinking URLs
> > * Hyperlinking Message-Ids to a mailing list archive
> > * Hyperlinking commit hashes as before by default, now with a
> >   configurable regex
> > * Defining new committags per gitweb installation
> 
> Well, there is one _implicit_ (but important) commit message 
> transformation (filter) for display, which has to be always present[1],
> namely HTML escaping.  We make use of the fact that you can do
> HTML escaping before doing the only currently supported committag, 
> namely hyperlinking (shortened) SHA-1 to 'object' gitweb URL, but for
> other committags like mentioned "Message-Id to mail archive" committag
> (filter) it would make them more difficult.

The original patch still did the HTML escaping.  I don't see much
value in implementing this transformation as a committag since it
wouldn't be useful to configure it, and I don't really see it making
the code more clear or concise.  Did you have any particular reasons?

> Also one might consider vertical whitespace simplification (removing
> leading empty lines, compacting empty lines to single empty line 
> between paragraphs), and syntax highlighting signoff lines to be
> a kind of commit message filter like mentioned above committags
> (see git_print_log() subroutine).
> 
> Although probably vertical whitespace simplification should be not
> made into commit message filter, as it is used not for all views.
> 
> > 
> > Since different repositories may use different bug tracking systems or
> > mailing list archives, the URL parameter may be configured
> > per-repository without reiterating the regexes.  To accomodate
> > different conventions, regexes may also be configured per-project.
> 
> Also list of supported committags is separated from the list of 
> committags used[1]; just like it is done for snapshot formats.
> This could be mentioned in final commit message.
> 
> [1] well, sequence rather than list in this case, as here
>     ordering does matter a bit
> 
> > 
> > This patch is heavily based on discussions and code samples from the
> > Git list:
> > 
> > 	[RFC/PATCH] gitweb: Add committags support, Sep 2006
> > 	http://thread.gmane.org/gmane.comp.version-control.git/27504
> > 
> > 	[RFC] gitweb: Add committags support (take 2), Dec 2006
> > 	http://thread.gmane.org/gmane.comp.version-control.git/33150
> > 
> > 	[RFC] Configuring (future) committags support in gitweb, Nov 2008
> > 	http://thread.gmane.org/gmane.comp.version-control.git/100415
> 
> Hmmm... should this be put in final commit message, or only in comment
> to the patch (should this be in commit history of git repository)?

I'll exclude the mailing list references from the commit message since
they describe how I arrived at this set of changes rather than
describing the changes themselves.

> > Some issues I considered but punted:
> > 
> > * Should this configuration try to follow the bugtraq spec?
> > 
> >   As far as I know, only subversion implements it.  Separation of
> >   regexes by a newline would be a little awkward in the git config.
> >   And it is broader than just hyperlinking bugs: it also encompasses
> >   GUI bug ID form fields.  So gitweb would only implement a subset.
> 
> I didn't even know that there is such spec.  Were you talking about
> http://tortoisesvn.net/issuetracker_integration or do you have different
> URL in mind?

That is the Bugtraq spec, although I had been looking at a text-only
version of it at the time.  I can't seem to find the text-only
document, but I think that one explains it just as well.

> >   The gitweb configuration mechanism currently only reads
> >   keys starting with "gitweb.", but these parameters would be more
> >   broadly applicable, potentially to git-gui, for example.
> 
> Actually the fact that gitweb reads only keys in the 'gitweb' section
> from config is just a convention.  There were (are) no config variables
> in other places (other sections) which would be of interest to gitweb.
> 
> > 
> >   However, it *would* be useful for Git tools to standardize on
> >   config keys and interpretations of regexes and url formats.  For
> >   example, git-gui might be able to hyperlink the same text as gitweb,
> >   and even show a separate bugID field when composing a commit
> >   message.
> 
> This is I think a very good idea... but I think idea which 
> implementation can be left for later.

Very well then, I'll leave everything related to the other git tools
for later.

> > 
> > * I would prefer the regex match against the whole commit message.
> > 
> >   This would allow the regex to insist that a bug reference occur
> >   on the first line or non-first line of the commit message.  However,
> >   even if we concatenated the log lines for the first committag,
> >   subsequent committags would see the text broken up.
> > 
> >   Also, it would allow the regex to match a phrase split across a
> >   line boundary, as dicussed at some length in the first thread,
> >   but again, only if no prior committags had interfered.
> > 
> >   This could happen in a later patch.
> 
> Well, the change should be fairly easy: just concatenate lines before
> passing them as single element list to commit message filters.  OTOH
> you would have to take care of end of line characters in committags
> regexps.

Yes, it seems manageable to process the whole commit message at once
rather than line by line.  I've made this change to support patch 4 of
this series.

> > * I would prefer the site admin have a way to let a repository
> >   owner define new committags, which means having a way to specify
> >   the 'sub' key from the repo config or having a flexible default.
> 
> Perhaps, following your earlier suggestion to make committags supported
> also by other tools, gitweb (and e.g. git-gui / gitk) use config 
> variable committag.<name>.<key> (where <key> can be 'pattern' or 'url';

This is a great example of the kind of decision I'd like to make
up-front to avoid breaking existing configs later:
gitweb.committag.<name>.<key> vs. committag.<name>.<key>.  I'm not
very interested in adding the git-gui / gitk feature myself.  Is
anyone actually interested in this feature?  If not, perhaps it should
stay under gitweb.committag.<name>.<key>.  And even if there is
interest in the git-gui / gitk features, perhaps it would make sense
to start with the gitweb-specific version of the config variable names
and once there is cross-tool support, keep those configs as overrides
for gitweb only?

> although I wonder if we can allow 'pattern' as malicious user can do
> a DoS attack against gitweb / server using badly behaved regexp).

Yes, I imagine there is a risk of abuse in allowing patterns to be
configurable.  That's one reason why the site config can disallow
per-project configuration of regexes.  Are you suggesting there be a
separate flag to allow override of regexes than to allow override of
other parameters?  For example, with a separate flag for regexes,
repo.or.cz could allow you to configure the bug tracker URL but not
the bug regex.  I've added very fine grain support for allowing only
some options to be overridden in patch 3 of this series.

> > The bugtraq and some of the regex questions must be decided now to
> > avoid breaking gitweb configs later.
> 
> True.
> 
> > 
> > Signed-off-by: Marcel M. Cary <marcel@oak.homeunix.org>
> > ---
> >  gitweb/INSTALL                         |    4 +
> >  gitweb/gitweb.perl                     |  221 +++++++++++++++++++++++++++++++-
> >  t/t9500-gitweb-standalone-no-errors.sh |  150 +++++++++++++++++++++-
> >  3 files changed, 367 insertions(+), 8 deletions(-)
> > 
> > diff --git a/gitweb/INSTALL b/gitweb/INSTALL
> > index 18c9ce3..223e39e 100644
> > --- a/gitweb/INSTALL
> > +++ b/gitweb/INSTALL
> > @@ -123,6 +123,10 @@ GITWEB_CONFIG file:
> >  	$feature{'snapshot'}{'default'} = ['zip', 'tgz'];
> >  	$feature{'snapshot'}{'override'} = 1;
> >  
> > +	$feature{'committags'}{'default'} = ['sha1', 'url', 'bugzilla'];
> > +	$feature{'committags'}{'override'} = 1;
> > +
> > +
> >  
> >  Gitweb repositories
> >  -------------------
> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> > index 1e7e2d8..c66fdf3 100755
> > --- a/gitweb/gitweb.perl
> > +++ b/gitweb/gitweb.perl
> > @@ -195,6 +195,81 @@ our %known_snapshot_format_aliases = (
> >  	'x-zip' => undef, '' => undef,
> >  );
> >  
> 
> I understand that comments such as one below would be not present in
> a final submission, and they are here to provide running commentary for
> code, isn't it?

Yes.  I've removed the running commentary, since it seems to be in the
way.

> > +# Could call these something else besides committags... embellishments,
> > +# patterns, rewrite rules, ?
> 
> They are "commit filters", or "commit message filters" (or 'formatters',
> or 'processors'; they are not 'parsers').  
> 
> The name 'committag' was first introduced as far as I remember in xmms2
> fork of gitweb (in old times when gitweb was separate project, and not
> part of git repository).

Sounds as though there is no interest in changing the name to
something that more clearly describes the feature (asside from my own,
but I'd like a little more validation...).  Let's stick with
committag.

> > +#
> > +# In general, the site admin can enable/disable per-project configuration
> > +# of each committag.  Only the 'options' part of the committag is configurable
> > +# per-project.
> 
> See above caveat about allowing to customize 'regexp'/'pattern' part
> in untrusted environment; you can construct regexp which has exponential
> behavior.
> 
> > +#
> > +# The site admin can of course add new tags to this hash or override the
> > +# 'sub' key if necessary.  But such changes may be fragile; this is not
> > +# designed as a full-blown plugin architecture.
> > +our %committags = (
> 
> 
> You should put the comments here about supported keys, similar to the
> one for %known_snapshot_formats and %feature hashes.

I believe I've addressed your request by adding a comment that
applies to all tags.  Because of the similarity of the tags, I don't
think it would be very useful to do this for each tag.  Does the 'url'
option used on two of the tags need further explanation?

> > +	# Link Git-style hashes to this gitweb
> > +	'sha1' => {
> > +		'options' => {
> > +			'pattern' => qr/\b([0-9a-fA-F]{8,40})\b/,
> > +		},
> > +		'override' => 0,
> 
> Shouldn't 'override' key be better last?

I liked 'override' close to the 'options' hash because that is what it
controls.  The 'sub' key was not overridable per-project no matter how
you configure gitweb.  Now it is, so maybe now 'override' could go
last.  Or first.  But really, I don't think the order matters much.

> > +		'sub' => sub {
> > +			my ($opts, @match) = @_;
> > +			\$cgi->a({-href => href(action=>"object", hash=>$match[1]),
> > +			          -class => "text"}, esc_html($match[0], -nbsp=>1));
> > +		},
> 
> Style: although there is commonly used idiom to use 'sub { <expr>; }'
> for a wrapper subroutines (e.g. 'sub { [] }' in Moose examples), one
> should use explicit "return" statement instead of relying on Perl 
> behavior of returning last statement in a block.
> 
> See Perl::Critic::Policy::Subroutines::RequireFinalReturn policy in
> Perl::Critic (perlcritic.com).  "Perl Best Practices" says:
> 
>   Subroutines without explicit 'return' statements at their ends can be
>   confusing. It can be challenging to deduce what the return value will be.
> 
> > +	},
> > +	# Link bug/features to Mantis bug tracker using Mantis-style contextual cues
> > +	'mantis' => {
> > +		'options' => {
> > +			'pattern' => qr/(?:BUG|FEATURE)\((\d+)\)/,
> > +			'url' => 'http://bugs.xmms2.xmms.se/view.php?id=',
> 
> I don't think we want to put such URL here.  Please check if Mantis 
> documentation uses some specific links, or follow RFC conventions and
> use 'example.com' as hostname (e.g. 'bugs.example.com').

Ok, I'll use a neutral URL for the mantis default.

> By the way the bugtraq proposal you mentioned uses placeholder in URL
> for putting issue number (%BUGID%).  Perhaps gitweb should do the same
> here.

I'd rather use a regex-style or sprintf-style syntax that could be
used by all committags.  I started with just an opaque string to
prepend to the bug ID because it was the simplest way to fulfill my
requirements, but it's certainly plausible that a BTS would want urls
like "example.com/bug/1234/detail" in which case the prepending
strategy isn't sufficient.  Again, ideally we'd decide now whether to
use '$1' or '%s' so we don't break configs later.  If the bugtraq
feature is implemented at some point, those config parameters can
still use the %BUGID% placeholder.

> > +		},
> > +		'override' => 0,
> > +		'sub' => \&hyperlink_committag,
> > +	},
> > +	# Link mentions of bug IDs to bugzilla
> > +	'bugzilla' => {
> > +		'options' => {
> > +			'pattern' => qr/bug\s+(\d+)/,
> > +			'url' => 'http://bugzilla.kernel.org/show_bug.cgi?id=',
> 
> The same comment as above.
> 
> > +		},
> > +		'override' => 0,
> > +		'sub' => \&hyperlink_committag,
> > +	},
> > +	# Link URLs
> > +	'url' => {
> > +		'options' => {
> > +			# Avoid matching punctuation that might immediately follow
> > +			# a url, is not part of the url, and is allowed in urls,
> > +			# like a full-stop ('.').
> > +			'pattern' => qr!(http|ftp)s?://[-_a-zA-Z0-9\@/&=+~#<>;%:.?]+
> > +			                               [-_a-zA-Z0-9\@/&=+~#<>]!x,
> 
> If you took this regexp from some place (like blog), it would be good
> to mention URL here, to be able to check more detailed explanation of
> construction of this URL-catching regexp.

Actually, I think I started with a URL regex mentioned on-list and
then tweaked it until I thought it worked well enough.

Most of the regexes I found on the web had issues.  They tended to be
too long because they were trying to precisely validate the URL or
because they were trying to parse all the different parts of the url,
or they did not deal with the trailing punctuation problem: I don't
want the period (".") to be highlighted in this context: "See
http://example.com/foo."  I think it's a valid URL even with the
period, but typically the period will not be part of the URL.  On the
other hand, "http://example.com/foo.html" should be completely
highlighted in spite of the period.

> Should we also support irc://, nntp:// (pseudo)protocols? What about
> git:// ?

I'va added more schemes, but it's also possible to leave those as a
customization.  In my organization, it would be very uncommon for
someone to follow a URL with any of those schemes.  I'd be willing to
allow any scheme that matches "[a-z]+://".  I don't care about stuff
like "data:literal+text", and "mailto:" is also less interesting to
me.  (I'd rather add a committag to match the email address without a
"mailto:".)  And then there's those pesky windows file sharing
thingies like '\\server\directory' that work like URLs in Internet
Explorer.  I'd rather not include them in the URL committag... but a
site admin could certainly configure a committag for it.  Here's a
list schemes for which KDE allegedly supports retreival of metadata:

bluetooth, fish, ftp, imap(s), invitation, iso, ldap(s), mac, mdns,
nfs, nntp(s), nxfish, obex, pop3(s), print, printdb, sdp, service,
sftp, slp, smb, smtp(s), webdav(s)

I wouldn't want to enumerate all of those, but, in addition to the
three you suggest, these look most useful to me: sftp, smb, webdav(s),
nfs.  So really I think it comes down to whether we want to enumerate
them or just match any scheme, and risk matching something that was
not intended as a URL.  And if we enumerate, how many do we want to
list.

What do you think of the new list of schemes in patch 1?  I've
included several more than before.

> > +		},
> > +		'override' => 0,
> > +		'sub' => sub {
> > +			my ($opts, @match) = @_;
> > +			return
> > +				\$cgi->a({-href => $match[0],
> > +				          -class => "text"},
> > +				         esc_html($match[0], -nbsp=>1));
> > +		},
> 
> Here you use explicit return.
> 
> > +	},
> > +	# Link Message-Id to mailing list archive
> > +	'messageid' => {
> > +		'options' => {
> > +			# The original pattern, which I don't really understand
> > +			#'pattern' => qr!(?:message|msg)-id:?\s+<([^>]+)>;!i,
> > +			'pattern' => qr!(?:message|msg)-?id:?\s+(<[^>]+>)!i,
> 
> Errr... how original patter is different from the one used?  Also above
> comment should be removed in final submission.

The most important change is that the semicolon is removed.  I
also made the dash optional.  It wasn't clear to me whether this was
intended to match a header-style reference:

    Message-Id: <asdf@example.com>

Or a casual mention:

    In msgid <asdf@example.com>, John Doe says...

But I like the latter, and would like the former to still be
supported.

> > +			'url' => 'http://news.gmane.org/find-root.php?message_id=',
> 
> Same comment about generic URL... although on the other hand perhaps
> having a few examples of mail archive sites which support finding 
> messages by Message-Id could be a good idea.
> 
> BTW. you can write 'http://mid.gmane.org/' instead...

Thanks for the tip.  Is there another common mail archive site
that allows looking up emails by message-id?  If so, I'd love to
mention the url in the comment for that committag.  I think we need to
strike a balance between having things work with minimal configuration
and avoiding the promotion of specific web sites that won't make sense
for most sites using a particular committag.  So, unless there is a
whole slew of web sites that provide this feature, I'd suggest leaving
the specific URL in.

> > +		},
> > +		'override' => 0,
> > +		# The original version didn't include the "msg-id" text in the
> > +		# link text, but this does.  In general, I think a little more
> > +		# context makes for better link text.
> 
> I guess that is the result of using generic hyperlink_committag() 
> subroutine here.  (This comment should be removed or reworded in final
> submitted version, I think.)

Yes, although if we decided it was sub-optimal link text, I'd be
happy to use a less generic mechanism.

> BTW. it would be much easier with Perl6-ish (or Perl 5.10.x) named
> captures (named groups):
> 
> 	'pattern' => qr!(?:message|msg)-?id:?\s+(?P<query><[^>]+>)!i,
> 
> or something like that.

I like the notion of names rather than numberic indices, but I'm
hesitant to require site admins to use unfamiliar Perl regex syntax to
configure committags.  Of two admins I asked, neither could make sense
of the sample regex.  If wouldn't want a site admin to *have* to learn
new syntax to configure regexes.

> > +		'sub' => \&hyperlink_committag,
> > +	},
> > +);
> > +
> >  # You define site-wide feature defaults here; override them with
> >  # $GITWEB_CONFIG as necessary.
> >  our %feature = (
> > @@ -365,6 +440,21 @@ our %feature = (
> >  		'sub' => \&feature_patches,
> >  		'override' => 0,
> >  		'default' => [16]},
> > +
> > +	# The selection and ordering of committags that are enabled.
> > +	# Committag transformations will be applied to commit log messages
> > +	# in this order if listed here.
> 
> /this/given/
> 
> You need to mention somewhere that committag subroutines return a list
> of mixed scalar and reference to scalar elements, where using reference
> to scalar removes value from the chain of filters (including implicit
> final esc_html filter).

I chose to add that explanation in the section that describes the
configuration of committags rather than in this section that defines
the list of active committags.  Sound ok?

> > +
> > +	# To disable system wide have in $GITWEB_CONFIG
> > +	# $feature{'committags'}{'default'} = [];
> > +	# To have project specific config enable override in $GITWEB_CONFIG
> > +	# $feature{'committags'}{'override'} = 1;
> > +	# and in project config gitweb.committags = sha1, url, bugzilla
> > +	# to enable those three committags for that project
> 
> Just a thought: perhaps we should provide support for 'default' in
> config (which would currently be "sha1" or "sha1, url").

I didn't understand how this would be very useful until I added
the signoff committag.  The use case I see is that it allows the
gitweb distribution to adjust the base functionality, in this case by
pushing some functionality into a committag, without project owners
needing to reconfigure their repositories.  Site admins don't need
this because they can use "push" or "unshift" to preserve the
distributed committags, right?  The project owner should get the
distribution default, not the site default, when they write "default",
right?  Are there other use cases you had in mind?  A potential
implementation is in patch 6.

> See also comment text for 'snapshot' feature, which says:
> 
>   and in project config, a comma-separated list of [...] or 
>   "none" to disable.
> 
> > +	'committags' => {
> > +		'sub' => \&feature_committags,
> > +		'override' => 0,
> > +		'default' => ['sha1']},
> >  );
> >  
> >  sub gitweb_get_feature {
> > @@ -433,6 +523,18 @@ sub feature_patches {
> >  	return ($_[0]);
> >  }
> >  
> > +sub feature_committags {
> > +	my (@defaults) = @_;
> > +
> > +	my ($cfg) = git_get_project_config('committags');
> > +
> > +	if ($cfg) {
> > +		return ($cfg eq 'none' ? () : split(/\s*[,\s]\s*/, $cfg));
> > +	}
> > +
> > +	return @defaults;
> > +}
> 
> As this would be second feature which uses comma-separated (or for
> backward compatibility space separated) list of options, perhaps
> we should factor out this part into common helper subroutine named
> for example 'feature_list' or 'feature_multi' (like 'feature_bool').

Thanks for pointing out the opportunity to fold the feature list
functions together.

> > +
> >  # checking HEAD file with -e is fragile if the repository was
> >  # initialized long time ago (i.e. symlink HEAD) and was pack-ref'ed
> >  # and then pruned.
> > @@ -814,6 +916,34 @@ $git_dir = "$projectroot/$project" if $project;
> >  our @snapshot_fmts = gitweb_get_feature('snapshot');
> >  @snapshot_fmts = filter_snapshot_fmts(@snapshot_fmts);
> >  
> > +# ordering of committags
> > +our @committags = gitweb_get_feature('committags');
> > +
> > +# Merge project configs with default committag definitions
> > +gitweb_load_project_committags();
> 
> Good idea... although gitweb first defines and then uses subroutine,
> see evaluate_path_info().

Sounds like you're asking me to move the function call after the
function definition.  I've made the change, but I'm also curious to
know why you have that preference.  I sometimes find it less readable,
as in this case.

> > +
> > +# Load committag configs from the repository config file and and
> > +# incorporate them into the gitweb defaults where permitted by the
> > +# site administrator.
> > +sub gitweb_load_project_committags {
> > +	return if (!$git_dir);
> > +	my %project_config = ();
> > +	my %raw_config = git_parse_project_config('gitweb\.committag');
> 
> Why not do lazy-loading of a whole config here?  We use committag
> info only for project-specific actions in gitweb.

I thought the check for $git_dir would prevent loading it for
non-project-specific pages.  Even so, I suppose we might load the
config on a page like the shortlog page that doesn't need it.

> > +	foreach my $key (keys(%raw_config)) {
> > +		next if ($key !~ /gitweb\.committag\.[^.]+\.[^.]/);
> > +		my ($gitweb_prefix, $committag_prefix, $ctname, $option) =
> > +			split(/\./, $key, 4);
> > +		$project_config{$ctname}{$option} = $raw_config{$key};
> > +	}
> 
> And use created subroutines to handle config?

Are you saying I should be using other subroutines that already
exist in gitweb rather than implementing my own?  Are you thinking of
git_get_project_config?  If I understand correctly, it requires me to
enumerate all the config keys I want.  I'd rather not require the
committags to have a predetermined set of possible config keys.  I see
now that I could still call git_get_project_config to load data into
%config and access that directly... I didn't do it before because it
seems to violate some kind of abstraction.

> > +	foreach my $ctname (keys(%committags)) {
> > +		next if (!$committags{$ctname}{'override'});
> > +		foreach my $optname (keys %{$project_config{$ctname}}) {
> > +			$committags{$ctname}{'options'}{$optname} =
> > +				$project_config{$ctname}{$optname};
> > +		}
> > +	}
> > +}
> > +
> >  # dispatch
> >  if (!defined $action) {
> >  	if (defined $hash) {
> > @@ -1384,13 +1514,92 @@ sub file_type_long {
> >  sub format_log_line_html {
> >  	my $line = shift;
> >  
> > -	$line = esc_html($line, -nbsp=>1);
> > -	$line =~ s{\b([0-9a-fA-F]{8,40})\b}{
> > -		$cgi->a({-href => href(action=>"object", hash=>$1),
> > -					-class => "text"}, $1);
> > -	}eg;
> > +	# In this list of log message fragments, a string ref indicates HTML,
> > +	# and a string indicates plain text
> > +	my @list = ( $line );
> 
> Well, to be more exact string ref means that the string referenced is
> not to be processed by later filters, including final implicit esc_html.

Well... it means both not processing and HTML escaping.  The string
refs will also not be escaped in the final processing step.

> Perhaps it would be better to use less generic name than @list herem
> e.g. @process or something?

Sure.  I chose @message_fragments as the less generic name for @list.

> > -	return $line;
> > +COMMITTAG:
> > +	foreach my $ctname (@committags) {
> > +		next COMMITTAG unless exists $committags{$ctname};
> > +		my $committag = $committags{$ctname};
> > +
> > +		next COMMITTAG unless exists $committag->{'options'};
> > +		my $opts = $committag->{'options'};
> > +
> > +		next COMMITTAG unless exists $opts->{'pattern'};
> > +		my $pattern = $opts->{'pattern'};
> > +
> > +		my @newlist = ();
> > +
> > +	PART:
> > +		foreach my $part (@list) {
> > +			next PART if $part eq "";
> > +			if (ref($part)) {
> > +				push @newlist, $part;
> > +				next PART;
> > +			}
> > +
> > +			my $oldpos = 0;
> > +
> > +		MATCH:
> > +			while ($part =~ m/$pattern/gc) {
> > +				my ($prepos, $postpos) = ($-[0], $+[0]);
> > +				my $repl = $committag->{'sub'}->($opts, $&, $1);
> > +				$repl = "" if (!defined $repl);
> > +
> > +				my $pre = substr($part, $oldpos, $prepos - $oldpos);
> > +				push_or_append(\@newlist, $pre);
> > +				push_or_append(\@newlist, $repl);
> > +
> > +				$oldpos = $postpos;
> > +			} # end while [regexp matches]
> > +
> > +			my $rest = substr($part, $oldpos);
> > +			push_or_append(\@newlist, $rest);
> > +
> > +		} # end foreach (@list)
> > +
> > +		@list = @newlist;
> > +	} # end foreach (@committags)
> > +
> > +	# Escape any remaining plain text and concatenate
> > +	my $html = '';
> > +	for my $part (@list) {
> > +		if (ref($part)) {
> > +			$html .= $$part;
> > +		} else {
> > +			$html .= esc_html($part, -nbsp=>1);
> > +		}
> > +	}
> > +
> > +	return $html;
> > +}
> 
> Nice.

Hehe, that bit of code is mostly yours, which you posted on this list
ages ago. (:  I suppose we should try to get your signoff into the
first patch of the series?

> > +
> > +# Returns a ref to an HTML snippet that links the second
> > +# parameter to a URL formed from the first and last parameters.
> > +# This is a helper function used in %committags.
> > +sub hyperlink_committag {
> > +	my ($opts, @match) = @_;
> > +	return
> > +		\$cgi->a({-href => $opts->{url} . CGI::escape($match[1]),
> 
> $opts->{'url'} not $opts->{url}
> 
> '$cgi->escapeHTML' I think, not 'CGI::escape' (but I am not sure here).
> besides, we can always import 'escape'.
> 
> > +				  -class => "text"},
> > +				 esc_html($match[0], -nbsp=>1));
> > +}
> > +
> > +
> > +sub push_or_append (\@@) {
> 
> Hmmm... this would be first use of Perl subroutine prototypes in gitweb.
> But this is made to imitate 'push' built-in, so I think it is O.K.

Ok, I guess I'll leave the subroutine prototype in then.  I don't
understand all the implications, so if you think it'll work well
without the prototype, I'm happy to remove it.  This is code
that I think you posted to the list.

> > +	my $list = shift;
> > +
> > +	if (ref $_[0] || ! @$list || ref $list->[-1]) {
> > +		push @$list, @_;
> > +	} else {
> > +		my $a = pop @$list;
> > +		my $b = shift @_;
> > +
> > +		push @$list, $a . $b, @_;
> > +	}
> > +	# imitate push
> > +	return scalar @$list;
> >  }
> >  
> >  # format marker of refs pointing to given object
> > diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
> > index d539619..37a127c 100755
> > --- a/t/t9500-gitweb-standalone-no-errors.sh
> > +++ b/t/t9500-gitweb-standalone-no-errors.sh
> > @@ -55,9 +55,9 @@ gitweb_run () {
> >  	# some of git commands write to STDERR on error, but this is not
> >  	# written to web server logs, so we are not interested in that:
> >  	# we are interested only in properly formatted errors/warnings
> > -	rm -f gitweb.log &&
> > +	rm -f resp.http gitweb.log &&
> >  	perl -- "$SCRIPT_NAME" \
> > -		>/dev/null 2>gitweb.log &&
> > +		> resp.http 2>gitweb.log &&
> >  	if grep "^[[]" gitweb.log >/dev/null 2>&1; then false; else true; fi
> >  
> 
> Well, if you begin to check _output_ of gitweb, then it should be put 
> in separate test, not t/t9500-gitweb-standalone-no-errors.sh which is
> only about no-errors... or change name of gitweb test.

I see there is some precedent for adding lib-foo.sh files.  Perhaps it
would be appropriate to move parts of the no-errors test into a
lib-gitweb.sh?  Like gitweb_init, most of gitweb_run, and maybe the
perl checks (which are that way for lib-git-svn.sh)?  I'm all for
separating the tests.  I don't like having to keep telling the test to
skip the first 85 cases when all I want to run is the committag stuff.

Actually, looks like Mark Rada already made a gitweb-lib.sh which was
exactly the same as the one I'd made except the name of the lib (vs.
lib-gitweb.sh) and the name of the file holding gitweb output.  I've
rebased onto those changes.

> I think I'm gonna leave this and the other feedback
> >  	# gitweb.log is left for debugging
> > @@ -702,4 +702,150 @@ test_expect_success \
> >  	 gitweb_run "p=.git;a=summary"'
> >  test_debug 'cat gitweb.log'
> >  
> > +# ----------------------------------------------------------------------
> > +# sha1 linking
> > +#
> > +echo hi > file.txt
> > +git add file.txt
> > +git commit -q -F - file.txt <<END
> > +Summary
> > +
> > +See also commit 567890ab
> > +	h=$(git rev-parse --verify HEAD) &&
> > +	gitweb_run "p=.git;a=commit;h=$h" &&
> 
> Actually you can just use "h=HEAD" or use query without 'h' parameter
> (which defaults to "HEAD") here.
 
Thanks for the tip, I'll use h=HEAD.
 
> > +	grep -q \
> > +		"commit&nbsp;<a class=\"text\" href=\".*\">567890ab</a>" \
> > +		resp.http
> > +'
> > +test_debug 'cat gitweb.log'
> > +test_debug 'grep 567890ab resp.http'
> 
> I'd rather use Test::* (e.g. Test::WWW::Mechanize::CGI) for that...
> but having some output test for gitweb, even in such simple form would
> certainly  be nice.

Would this mean people need to install some Mechanize stuff before
they can run the tests?  I think I'd rather avoid the dependency as
long as grep seems to do the trick.

> > +
> > +# ----------------------------------------------------------------------
> > +# bugzilla commit tag
> > +#
> > +
> > +echo foo > file.txt
> > +git add file.txt
> > +git commit -q -F - file.txt <<END
> > +Fix foo
> > +
> > +Fixes bug 1234 involving foo.
> > +END
> > +git config gitweb.committags 'sha1, bugzilla'
> > +test_expect_success 'bugzilla: enabled but not permitted' '
> > +	h=$(git rev-parse --verify HEAD) &&
> > +	gitweb_run "p=.git;a=commit;h=$h" &&
> > +	grep -F -q \
> > +		"Fixes&nbsp;bug&nbsp;1234&nbsp;involving" \
> > +		resp.http
> > +'
> > +test_debug 'cat gitweb.log'
> > +test_debug 'grep 1234 resp.http'
> > +
> > +echo '$feature{"committags"}{"override"} = 1;' >> gitweb_config.perl
> > +test_expect_success 'bugzilla: enabled' '
> > +	h=$(git rev-parse --verify HEAD) &&
> > +	gitweb_run "p=.git;a=commit;h=$h" &&
> > +	grep -F -q \
> > +		"Fixes&nbsp;<a class=\"text\" href=\"http://bugzilla.kernel.org/show_bug.cgi?id=1234\">bug&nbsp;1234</a>&nbsp;involving" \
> > +		resp.http
> > +'
> 
> Hmmm...
> 

?



Marcel M. Cary (6):
  gitweb: Hyperlink various committags in commit message with regex
  gitweb: Add second-stage matching of bug IDs in bugzilla committag
  gitweb: Allow finer-grained override controls for committags
  gitweb: Allow committag pattern matches to span multiple lines
  gitweb: Allow per-repository definition of new committags
  gitweb: Add _defaults_ keyword for feature lists in project config

 gitweb/INSTALL               |   16 ++
 gitweb/gitweb.perl           |  404 +++++++++++++++++++++++++++++++++++++-----
 t/t9502-gitweb-committags.sh |  309 ++++++++++++++++++++++++++++++++
 3 files changed, 681 insertions(+), 48 deletions(-)
 create mode 100755 t/t9502-gitweb-committags.sh

^ permalink raw reply

* [RFC PATCH 2/6] gitweb: Add second-stage matching of bug IDs in bugzilla committag
From: Marcel M. Cary @ 2009-11-18  6:22 UTC (permalink / raw)
  To: Jakub Narebski, git
  Cc: Petr Baudis, Giuseppe Bilotta, Francis Galiegue, Marcel M. Cary
In-Reply-To: <1258525350-5528-2-git-send-email-marcel@oak.homeunix.org>

Currently it's not easy to capture an unbounded number of items
in a committag phrase to hyperlink individually.

For example, I would like to match and hyperlinking each bug ID
individually in these situations:

	[#1234, #1235]
	Resolves-bug: 1234, 1235
	bugs 1234, 1235, and 1236

Match Bugzilla bug IDs with two regexes instead of one.  The first is
a pre-filter that allows easy matching of multiple bug IDs and
contextual queues like "bugs ___ and ___" in a phrase, and the second
easily picks out the individual big IDs for hyperlinking.

Signed-off-by: Marcel M. Cary <marcel@oak.homeunix.org>
---
 gitweb/gitweb.perl           |   68 +++++++++++++++++++++++++++++------------
 t/t9502-gitweb-committags.sh |   69 ++++++++++++++++++++++++++++++++++++++---
 2 files changed, 112 insertions(+), 25 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 2d72202..032b1c5 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -262,14 +262,31 @@ our %committags = (
 		'override' => 0,
 		'sub' => \&hyperlink_committag,
 	},
-	# Link mentions of bug IDs to bugzilla
+	# Link mentions of bugs to bugzilla, allowing for separate outer
+	# and inner regexes (see unit test for example)
 	'bugzilla' => {
 		'options' => {
-			'pattern' => qr/bug\s+(\d+)/,
+			'pattern' => qr/(?i:bugs?):?\s+
+			                [#]?\d+(?:(?:,\s*|,?\sand\s|,?\sn?or\s|\s+)
+			                          [#]?\d+\b)*/x,
+			'innerpattern' => qr/#?(\d+)/,
 			'url' => 'http://bugzilla.example.com/show_bug.cgi?id=',
 		},
 		'override' => 0,
-		'sub' => \&hyperlink_committag,
+		'sub' => sub {
+			my ($opts, @match) = @_;
+			if ($opts->{'innerpattern'}) {
+				my @message_fragments = ();
+				push_or_append_replacements(\@message_fragments,
+				                            $opts->{'innerpattern'},
+				                            $match[0], sub {
+						return hyperlink_committag($opts, @_);
+					});
+				return @message_fragments;
+			} else {
+				return hyperlink_committag(@_);
+			}
+		},
 	},
 	# Link URLs
 	'url' => {
@@ -1626,23 +1643,10 @@ COMMITTAG:
 				next PART;
 			}
 
-			my $oldpos = 0;
-
-		MATCH:
-			while ($fragment =~ m/$pattern/gc) {
-				my ($prepos, $postpos) = ($-[0], $+[0]);
-				my $repl = $sub->($opts, $&, $1);
-				$repl = "" if (!defined $repl);
-
-				my $pre = substr($fragment, $oldpos, $prepos - $oldpos);
-				push_or_append(\@new_message_fragments, $pre);
-				push_or_append(\@new_message_fragments, $repl);
-
-				$oldpos = $postpos;
-			} # end while [regexp matches]
-
-			my $rest = substr($fragment, $oldpos);
-			push_or_append(\@new_message_fragments, $rest);
+			push_or_append_replacements(\@new_message_fragments,
+			                            $pattern, $fragment, sub {
+					$sub->($opts, @_);
+				});
 
 		} # end foreach (@message_fragments)
 
@@ -1672,6 +1676,30 @@ sub hyperlink_committag {
 	                esc_html($match[0], -nbsp=>1));
 }
 
+# Find $pattern in string $fragment, and push_or_append the parts
+# between matches and the result of calling $sub with matched text to
+# $new_fragments.
+sub push_or_append_replacements {
+	my ($new_fragments, $pattern, $fragment, $sub) = @_;
+
+	my $oldpos = 0;
+
+MATCH:
+	while ($fragment =~ m/$pattern/gc) {
+		my ($prepos, $postpos) = ($-[0], $+[0]);
+
+		my @repl = $sub->($&, $1);
+
+		my $pre = substr($fragment, $oldpos, $prepos - $oldpos);
+		push_or_append($new_fragments, $pre);
+		push_or_append($new_fragments, @repl);
+
+		$oldpos = $postpos;
+	} # end while [regexp matches]
+
+	my $rest = substr($fragment, $oldpos);
+	push_or_append($new_fragments, $rest);
+}
 
 sub push_or_append (\@@) {
 	my $fragments = shift;
diff --git a/t/t9502-gitweb-committags.sh b/t/t9502-gitweb-committags.sh
index f86cb3d..718e763 100755
--- a/t/t9502-gitweb-committags.sh
+++ b/t/t9502-gitweb-committags.sh
@@ -52,7 +52,7 @@ echo '$feature{"committags"}{"override"} = 1;' >> gitweb_config.perl
 test_expect_success 'bugzilla: enabled' '
 	gitweb_run "p=.git;a=commit;h=HEAD" &&
 	grep -F -q \
-		"Fixes&nbsp;<a class=\"text\" href=\"http://bugzilla.example.com/show_bug.cgi?id=1234\">bug&nbsp;1234</a>&nbsp;involving" \
+		"Fixes&nbsp;bug&nbsp;<a class=\"text\" href=\"http://bugzilla.example.com/show_bug.cgi?id=1234\">1234</a>&nbsp;involving" \
 		gitweb.output
 '
 test_debug 'cat gitweb.log'
@@ -62,7 +62,7 @@ git config gitweb.committag.bugzilla.url 'http://bts.example.com?bug='
 test_expect_success 'bugzilla: url overridden but not permitted' '
 	gitweb_run "p=.git;a=commit;h=HEAD" &&
 	grep -F -q \
-		"Fixes&nbsp;<a class=\"text\" href=\"http://bugzilla.example.com/show_bug.cgi?id=1234\">bug&nbsp;1234</a>&nbsp;involving" \
+		"Fixes&nbsp;bug&nbsp;<a class=\"text\" href=\"http://bugzilla.example.com/show_bug.cgi?id=1234\">1234</a>&nbsp;involving" \
 		gitweb.output
 '
 test_debug 'cat gitweb.log'
@@ -72,12 +72,13 @@ echo '$committags{"bugzilla"}{"override"} = 1;' >> gitweb_config.perl
 test_expect_success 'bugzilla: url overridden' '
 	gitweb_run "p=.git;a=commit;h=HEAD" &&
 	grep -F -q \
-		"Fixes&nbsp;<a class=\"text\" href=\"http://bts.example.com?bug=1234\">bug&nbsp;1234</a>&nbsp;involving" \
+		"Fixes&nbsp;bug&nbsp;<a class=\"text\" href=\"http://bts.example.com?bug=1234\">1234</a>&nbsp;involving" \
 		gitweb.output
 '
 test_debug 'cat gitweb.log'
 test_debug 'grep 1234 gitweb.output'
 
+git config gitweb.committag.bugzilla.innerpattern ''
 git config gitweb.committag.bugzilla.pattern 'Fixes bug (\d+)'
 test_expect_success 'bugzilla: pattern overridden' '
 	gitweb_run "p=.git;a=commit;h=HEAD" &&
@@ -87,17 +88,75 @@ test_expect_success 'bugzilla: pattern overridden' '
 '
 test_debug 'cat gitweb.log'
 test_debug 'grep 1234 gitweb.output'
-git config --unset gitweb.committag.bugzilla.pattern
 
+git config --unset gitweb.committag.bugzilla.innerpattern
+git config --unset gitweb.committag.bugzilla.pattern
 test_expect_success 'bugzilla: affects log view too' '
 	gitweb_run "p=.git;a=log" &&
 	grep -F -q \
-		"<a class=\"text\" href=\"http://bts.example.com?bug=1234\">bug&nbsp;1234</a>" \
+		"<a class=\"text\" href=\"http://bts.example.com?bug=1234\">1234</a>" \
 		gitweb.output
 '
 test_debug 'cat gitweb.log'
 test_debug 'grep 1234 gitweb.output'
 
+echo more_bugzilla > file.txt
+git add file.txt
+git commit -q -F - file.txt <<END
+[#123,#45] This commit fixes two bugs involving bar and baz.
+END
+git config gitweb.committag.bugzilla.pattern       '^\[#\d+(, ?#\d+)\]'
+git config gitweb.committag.bugzilla.innerpattern  '#(\d+)'
+git config gitweb.committag.bugzilla.url           'http://bugs/'
+test_expect_success 'bugzilla: override everything, use fancier url format' '
+       gitweb_run "p=.git;a=commit;h=HEAD" &&
+       grep -F -q \
+               "[<a class=\"text\" href=\"http://bugs/123\">#123</a>,<a class=\"text\" href=\"http://bugs/45\">#45</a>]" \
+               gitweb.output
+'
+test_debug 'cat gitweb.log'
+test_debug 'grep 123 gitweb.output'
+
+echo even_more_bugzilla > file.txt
+git add file.txt
+git commit -q -F - file.txt <<END
+Fix memory leak in confabulator from bug 123.
+
+Based on history from bugs 223, 224, and 225,
+fix bug 323 or 324.
+
+Bug: 423,424,425,426,427,428,429,430,431,432,435
+Resolves-bugs: #523 #524
+END
+git config --unset gitweb.committag.bugzilla.pattern
+git config --unset gitweb.committag.bugzilla.innerpattern
+git config --unset gitweb.committag.bugzilla.url
+gitweb_run "p=.git;a=commit;h=HEAD"
+test_expect_success 'bugzilla: fancy defaults: match one bug' '
+	grep -q "from&nbsp;bug&nbsp;<a[^>]*>123</a>." gitweb.output
+'
+test_expect_success 'bugzilla: fancy defaults: comma-separated list' '
+	grep -q \
+		"bugs&nbsp;<a[^>]*>223</a>,&nbsp;<a[^>]*>224</a>,&nbsp;and&nbsp;<a[^>]*>225</a>," \
+		gitweb.output
+'
+test_expect_success 'bugzilla: fancy defaults: or-pair' '
+	grep -q "bug&nbsp;<a[^>]*>323</a>&nbsp;or&nbsp;<a[^>]*>324</a>." \
+		gitweb.output
+'
+test_expect_success 'bugzilla: fancy defaults: comma-separated, caps, >10' '
+	grep -q \
+		"Bug:&nbsp;<a[^>]*>423</a>,<a[^>]*>424</a>,.*,<a[^>]*>435</a>" \
+		gitweb.output
+'
+test_expect_success 'bugzilla: fancy defaults: space-separated with hash' '
+	grep -q -e \
+		"-bugs:&nbsp;<a[^>]*>#523</a>&nbsp;<a[^>]*>#524</a>" \
+		gitweb.output
+'
+test_debug 'cat gitweb.log'
+test_debug 'grep 23 gitweb.output'
+
 # ----------------------------------------------------------------------
 # url linking
 #
-- 
1.6.4.4

^ permalink raw reply related

* [RFC PATCH 6/6] gitweb: Add _defaults_ keyword for feature lists in project config
From: Marcel M. Cary @ 2009-11-18  6:22 UTC (permalink / raw)
  To: Jakub Narebski, git
  Cc: Petr Baudis, Giuseppe Bilotta, Francis Galiegue, Marcel M. Cary
In-Reply-To: <1258525350-5528-6-git-send-email-marcel@oak.homeunix.org>

If the site admin configures the list of committags, there's no way
for a project to get the defaults back short of enumerating them
explicitly.  Worse yet, when the distribution upgrades the default
list, perhaps to push more pre-existing functionality into committags,
the project would have to discover this and upgrade its configuration
to match the new defaults.

Add a special _defaults_ list entry which, in the project config,
expands to the build-time default list configured for that variable.
A project may use this to append or prepend to the default
configuration, even as the default configuration changes with new
releases.
---
 gitweb/INSTALL               |    5 +++++
 gitweb/gitweb.perl           |   18 +++++++++++++-----
 t/t9502-gitweb-committags.sh |   29 +++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/gitweb/INSTALL b/gitweb/INSTALL
index 15c0128..83e6a5e 100644
--- a/gitweb/INSTALL
+++ b/gitweb/INSTALL
@@ -143,6 +143,11 @@ And then let each project configure its bug tracker URL:
 
 	git config gitweb.committag.bugzilla.url 'http://bts.example.com?bug='
 
+In a project config, the build-time list of committags can be accessed
+with the special _defaults_ entry.
+
+	git config gitweb.committags '_defaults_, bugzilla'
+
 
 Gitweb repositories
 -------------------
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index d413f22..707e76e 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -334,6 +334,13 @@ our %committags = (
 	},
 );
 
+sub make_list_feature {
+	my ($name, $hash) = @_;
+	$hash->{'build_default'} = [@{$hash->{'default'}}];
+	$hash->{'sub'} = sub { feature_list($name, @_) };
+	return @_;
+}
+
 # You define site-wide feature defaults here; override them with
 # $GITWEB_CONFIG as necessary.
 our %feature = (
@@ -378,8 +385,7 @@ our %feature = (
 	# $feature{'snapshot'}{'override'} = 1;
 	# and in project config, a comma-separated list of formats or "none"
 	# to disable.  Example: gitweb.snapshot = tbz2,zip;
-	'snapshot' => {
-		'sub' => sub { feature_list('snapshot', @_) },
+	make_list_feature 'snapshot' => {
 		'override' => 0,
 		'default' => ['tgz']},
 
@@ -549,8 +555,7 @@ our %feature = (
 	# $feature{'committags'}{'override'} = 1;
 	# and in project config gitweb.committags = sha1, url, bugzilla
 	# to enable those three committags for that project
-	'committags' => {
-		'sub' => sub { feature_list('committags', @_) },
+	make_list_feature 'committags' => {
 		'override' => 0,
 		'default' => ['signoff', 'sha1']},
 
@@ -621,7 +626,10 @@ sub feature_list {
 	my ($cfg) = git_get_project_config($key);
 
 	if ($cfg) {
-		return ($cfg eq 'none' ? () : split(/\s*[,\s]\s*/, $cfg));
+		return () if $cfg eq 'none';
+		return map {
+				$_ eq '_defaults_' ? @{$feature{$key}{'build_default'}} : $_
+			} split(/\s*[,\s]\s*/, $cfg);
 	}
 
 	return @defaults;
diff --git a/t/t9502-gitweb-committags.sh b/t/t9502-gitweb-committags.sh
index cbe607b..7d16329 100755
--- a/t/t9502-gitweb-committags.sh
+++ b/t/t9502-gitweb-committags.sh
@@ -276,5 +276,34 @@ test_expect_success 'custom committags: ignored when disabled' '
 test_debug 'cat gitweb.log'
 test_debug 'grep -F "foo" gitweb.output'
 
+# ----------------------------------------------------------------------
+# default keyword
+#
+echo default_test > file.txt
+git add file.txt
+git commit -q -F - file.txt <<END
+Lets see what's enabled...
+
+Bug 1234
+567890ab
+See msg-id <x@y.z>
+
+Signed-off-by: A U Thor <at@example.com>
+END
+echo '
+$feature{"committags"}{"default"} = ["sha1", "messageid"];
+$feature{"committags"}{"override"} = 1;
+' >> gitweb_config.perl
+git config gitweb.committags '_defaults_, bugzilla'
+# All these committags should be in effect except messageid
+test_expect_success '_defaults_ keyword: restores build-time default' '
+	gitweb_run "p=.git;a=commit;h=HEAD" &&
+	grep -q "Bug&nbsp;<a[^>]*>1234</a>" gitweb.output &&
+	grep -q "<a[^>]*>567890ab</a>" gitweb.output &&
+	grep -q "See&nbsp;msg-id&nbsp;&lt;x@y.z&gt;" gitweb.output &&
+	grep -q "<span[^>]*>Signed-off-by:" gitweb.output
+'
+test_debug 'cat gitweb.log'
+test_debug 'for i in Bug 5678 msg-id Signed-off; do grep $i gitweb.output; done'
 
 test_done
-- 
1.6.4.4

^ permalink raw reply related

* [RFC PATCH 4/6] gitweb: Allow committag pattern matches to span multiple lines
From: Marcel M. Cary @ 2009-11-18  6:22 UTC (permalink / raw)
  To: Jakub Narebski, git
  Cc: Petr Baudis, Giuseppe Bilotta, Francis Galiegue, Marcel M. Cary
In-Reply-To: <1258525350-5528-4-git-send-email-marcel@oak.homeunix.org>

Committags cannot currently span multiple lines.  Since some committag
patterns match multiple words.  If some of those words wrap to the
next line, the committag would miss an opportunity to match.

Eliminate the for-loop over @log and pull the signoff transformation
from that loop into a committag.

The message will still get cut into pieces as committags are applied,
but at least newlines no longer force a cut.

Signed-off-by: Marcel M. Cary <marcel@oak.homeunix.org>
---
 gitweb/gitweb.perl           |   67 +++++++++++++++++++-----------------------
 t/t9502-gitweb-committags.sh |    8 +++++
 2 files changed, 38 insertions(+), 37 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 8f4480e..7f7d3a3 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -255,6 +255,21 @@ our %committags = (
 			                esc_html($match[0], -nbsp=>1));
 		},
 	},
+	# Facilitate styling of common header/footer lines, suppressing
+	# any trailing newlines
+	'signoff' => {
+		'options' => {
+			'pattern' =>
+				qr/^( *(?:signed[ \-]off[ \-]by|acked[ \-]by|cc)[ :].*)\n*$/mi,
+		},
+		'override' => 0,
+		'sub' => sub {
+			my ($opts, @match) = @_;
+			return (\$cgi->span({'class' => 'signoff'},
+			                    esc_html($match[1], -nbsp=>1)),
+			        "\n");
+		},
+	},
 	# Link bug/features to Mantis bug tracker using Mantis-style
 	# contextual cues
 	'mantis' => {
@@ -542,7 +557,7 @@ our %feature = (
 	'committags' => {
 		'sub' => sub { feature_list('committags', @_) },
 		'override' => 0,
-		'default' => ['sha1']},
+		'default' => ['signoff', 'sha1']},
 );
 
 sub gitweb_get_feature {
@@ -1619,8 +1634,8 @@ sub file_type_long {
 ## which don't belong to other sections
 
 # format line of commit message.
-sub format_log_line_html {
-	my $line = shift;
+sub format_log_html {
+	my $text = shift;
 
 	# Merge project configs with site default committag definitions if
 	# it hasn't been done yet
@@ -1629,7 +1644,7 @@ sub format_log_line_html {
 	# In this list of log message fragments, a string ref indicates
 	# HTML, and a string indicates plain text.  The string refs are
 	# also currently not processed by subsequent committags.
-	my @message_fragments = ( $line );
+	my @message_fragments = ( $text );
 
 COMMITTAG:
 	foreach my $ctname (@committags) {
@@ -1671,7 +1686,9 @@ COMMITTAG:
 		if (ref($fragment)) {
 			$html .= $$fragment;
 		} else {
-			$html .= esc_html($fragment, -nbsp=>1);
+			# Don't let esc_html turn "\n" into "\\n"
+			$html .= join("<br/>\n", map { esc_html($_, -nbsp=>1) }
+			                             split("\n", $fragment, -1));
 		}
 	}
 
@@ -3776,40 +3793,16 @@ sub git_print_log {
 		shift @$log;
 	}
 
-	# print log
-	my $signoff = 0;
-	my $empty = 0;
-	foreach my $line (@$log) {
-		if ($line =~ m/^ *(signed[ \-]off[ \-]by[ :]|acked[ \-]by[ :]|cc[ :])/i) {
-			$signoff = 1;
-			$empty = 0;
-			if (! $opts{'-remove_signoff'}) {
-				print "<span class=\"signoff\">" . esc_html($line) . "</span><br/>\n";
-				next;
-			} else {
-				# remove signoff lines
-				next;
-			}
-		} else {
-			$signoff = 0;
-		}
-
-		# print only one empty line
-		# do not print empty line after signoff
-		if ($line eq "") {
-			next if ($empty || $signoff);
-			$empty = 1;
-		} else {
-			$empty = 0;
-		}
-
-		print format_log_line_html($line) . "<br/>\n";
-	}
-
 	if ($opts{'-final_empty_line'}) {
-		# end with single empty line
-		print "<br/>\n" unless $empty;
+		# If we already have a trailing newline, this will be
+		# coalesced with it later.
+		push @$log, "";
 	}
+
+	# print log
+	my $text = join("\n", @$log) . "\n";
+	$text =~ s{\n\n+}{\n\n}g;
+	print format_log_html($text);
 }
 
 # return link target (what link points to)
diff --git a/t/t9502-gitweb-committags.sh b/t/t9502-gitweb-committags.sh
index e13ac47..0753630 100755
--- a/t/t9502-gitweb-committags.sh
+++ b/t/t9502-gitweb-committags.sh
@@ -138,6 +138,10 @@ Fix memory leak in confabulator from bug 123.
 Based on history from bugs 223, 224, and 225,
 fix bug 323 or 324.
 
+Bugs:
+1234,
+1235
+
 Bug: 423,424,425,426,427,428,429,430,431,432,435
 Resolves-bugs: #523 #524
 END
@@ -167,6 +171,10 @@ test_expect_success 'bugzilla: fancy defaults: space-separated with hash' '
 		"-bugs:&nbsp;<a[^>]*>#523</a>&nbsp;<a[^>]*>#524</a>" \
 		gitweb.output
 '
+test_expect_success 'bugzilla: fancy defaults: spanning newlines' '
+	grep -q -e "<a[^>]*>1234</a>,<br" gitweb.output &&
+	grep -q -e "<a[^>]*>1235</a><br" gitweb.output
+'
 test_debug 'cat gitweb.log'
 test_debug 'grep 23 gitweb.output'
 
-- 
1.6.4.4

^ permalink raw reply related

* [RFC PATCH 1/6] gitweb: Hyperlink committags in a commit message by regex matching
From: Marcel M. Cary @ 2009-11-18  6:22 UTC (permalink / raw)
  To: Jakub Narebski, git
  Cc: Petr Baudis, Giuseppe Bilotta, Francis Galiegue, Marcel M. Cary
In-Reply-To: <1258525350-5528-1-git-send-email-marcel@oak.homeunix.org>

I want gitweb to hyperlink commits to my bug tracking system so that
information regarding the current status of a commit can be easily
cross-referenced.  The QA and release status of a commit cannot be
directly inserted into the commit message because they change over
time.  But if the commit message mentions a bug number, gitweb could
detect the bug reference in the message and hyperlink it to the bug
tracking system.  Other repository browsers such as unfuddle and
websvn support similar features.

Currently only commit hashes are hyperlinked in this manner.

Since the bug hyperlinking feature was previously discussed as part of
"committags," a more general mechanism to embellish commit messages,
implement the more general mechanism instead, including the following
capabilities:

* Hyperlinking mentions of bug IDs to Bugzilla
* Hyperlinking URLs
* Hyperlinking Message-Ids to a mailing list archive
* Hyperlinking commit hashes, as before by default, now with a
  configurable regex
* Defining new committags per gitweb installation

Since different repositories may use different bug tracking systems or
mailing list archives, the URL parameter may be configured
per-repository without reiterating the regexes.  To accomodate
different conventions, regexes may also be configured per-project.

The order in which gitweb applies committags may be configured
per-project as well, because one committag may affect subsequent ones.
Inclusion in this sequence determines whether a committag is enabled
or not.

Signed-off-by: Marcel M. Cary <marcel@oak.homeunix.org>
---

One additional thing that occured to me is that maybe the hyperlinks
added by committags should have 'rel="nofollow"' by default?  And if
so, maybe that needs to be configurable?  On the other hand, I'm not
sure how useful it is to hide real URLs in the commit messages from
search engines... ?


 gitweb/INSTALL               |    5 +
 gitweb/gitweb.perl           |  247 +++++++++++++++++++++++++++++++++++++++---
 t/t9502-gitweb-committags.sh |  150 +++++++++++++++++++++++++
 3 files changed, 389 insertions(+), 13 deletions(-)
 create mode 100755 t/t9502-gitweb-committags.sh

diff --git a/gitweb/INSTALL b/gitweb/INSTALL
index b76a0cf..9081ed8 100644
--- a/gitweb/INSTALL
+++ b/gitweb/INSTALL
@@ -132,6 +132,11 @@ adding the following lines to your $GITWEB_CONFIG:
 	$known_snapshot_formats{'zip'}{'disabled'} = 1;
 	$known_snapshot_formats{'tgz'}{'compressor'} = ['gzip','-6'];
 
+To add a committag to the default list of commit tags, for example to
+enable hyperlinking of bug numbers to a bug tracker for all projects:
+
+	push @{$feature{'committags'}{'default'}}, 'bugzilla';
+
 
 Gitweb repositories
 -------------------
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index e4cbfc3..2d72202 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -213,6 +213,97 @@ our %avatar_size = (
 	'double'  => 32
 );
 
+# In general, the site admin can enable/disable per-project
+# configuration of each committag.  Only the 'options' part of the
+# committag is configurable per-project.
+#
+# The site admin can of course add new tags to this hash or override
+# the 'sub' key if necessary.  But such changes may be fragile; this
+# is not designed as a full-blown plugin architecture.  The 'sub' must
+# return a list of strings or string refs.  The strings must contain
+# plain text and the string refs must contain HTML.  The string refs
+# will not be processed further.
+#
+# For any committag, set the 'override' key to 1 to allow individual
+# projects to override entries in the 'options' hash for that tag.
+# For example, to match only commit hashes given in lowercase in one
+# project, add this to the $GITWEB_CONFIG:
+#
+#     $committags{'sha1'}{'override'} = 1;
+#
+# And in the project's config:
+#
+#     gitweb.committags.sha1.pattern = \\b([0-9a-f]{8,40})\\b
+#
+# Some committags have additional options whose interpretation depends
+# on the implementation of the 'sub' key.  The hyperlink_committag
+# value appends the first captured group to the 'url' option.
+our %committags = (
+	# Link Git-style hashes to this gitweb
+	'sha1' => {
+		'options' => {
+			'pattern' => qr/\b([0-9a-fA-F]{8,40})\b/,
+		},
+		'override' => 0,
+		'sub' => sub {
+			my ($opts, @match) = @_;
+			return \$cgi->a({-href => href(action=>"object", hash=>$match[1]),
+			                 -class => "text"},
+			                esc_html($match[0], -nbsp=>1));
+		},
+	},
+	# Link bug/features to Mantis bug tracker using Mantis-style
+	# contextual cues
+	'mantis' => {
+		'options' => {
+			'pattern' => qr/(?:BUG|FEATURE)\((\d+)\)/,
+			'url' => 'http://www.example.com/mantisbt/view.php?id=',
+		},
+		'override' => 0,
+		'sub' => \&hyperlink_committag,
+	},
+	# Link mentions of bug IDs to bugzilla
+	'bugzilla' => {
+		'options' => {
+			'pattern' => qr/bug\s+(\d+)/,
+			'url' => 'http://bugzilla.example.com/show_bug.cgi?id=',
+		},
+		'override' => 0,
+		'sub' => \&hyperlink_committag,
+	},
+	# Link URLs
+	'url' => {
+		'options' => {
+			# Avoid matching punctuation that might immediately follow
+			# a url, is not part of the url, and is allowed in urls,
+			# like a full-stop ('.').
+			'pattern' => qr!(https?|ftps?|git|ssh|ssh+git|sftp|smb|webdavs?|
+			                 nfs|irc|nntp|rsync)
+			                ://[-_a-zA-Z0-9\@/&=+~#<>;%:.?]+
+			                   [-_a-zA-Z0-9\@/&=+~#<>]!x,
+		},
+		'override' => 0,
+		'sub' => sub {
+			my ($opts, @match) = @_;
+			return \$cgi->a({-href => $match[0],
+			                 -class => "text"},
+			                esc_html($match[0], -nbsp=>1));
+		},
+	},
+	# Link Message-Id to mailing list archive
+	'messageid' => {
+		'options' => {
+			'pattern' => qr!(?:message|msg)-?id:?\s+(<[^>]+>)!i,
+			'url' => 'http://mid.gmane.org/',
+		},
+		'override' => 0,
+		# Includes the "msg-id" text in the link text.
+		# Since we don't support linking multiple msg-ids in one match, we
+		# can include the "msg-id" in the link text for better context.
+		'sub' => \&hyperlink_committag,
+	},
+);
+
 # You define site-wide feature defaults here; override them with
 # $GITWEB_CONFIG as necessary.
 our %feature = (
@@ -258,7 +349,7 @@ our %feature = (
 	# and in project config, a comma-separated list of formats or "none"
 	# to disable.  Example: gitweb.snapshot = tbz2,zip;
 	'snapshot' => {
-		'sub' => \&feature_snapshot,
+		'sub' => sub { feature_list('snapshot', @_) },
 		'override' => 0,
 		'default' => ['tgz']},
 
@@ -417,6 +508,21 @@ our %feature = (
 		'sub' => \&feature_avatar,
 		'override' => 0,
 		'default' => ['']},
+
+	# The selection and ordering of committags that are enabled.
+	# Committag transformations will be applied to commit log messages
+	# in the order listed here if listed here.
+
+	# To disable system wide have in $GITWEB_CONFIG
+	# $feature{'committags'}{'default'} = [];
+	# To have project specific config enable override in $GITWEB_CONFIG
+	# $feature{'committags'}{'override'} = 1;
+	# and in project config gitweb.committags = sha1, url, bugzilla
+	# to enable those three committags for that project
+	'committags' => {
+		'sub' => sub { feature_list('committags', @_) },
+		'override' => 0,
+		'default' => ['sha1']},
 );
 
 sub gitweb_get_feature {
@@ -463,16 +569,16 @@ sub feature_bool {
 	}
 }
 
-sub feature_snapshot {
-	my (@fmts) = @_;
+sub feature_list {
+	my ($key, @defaults) = @_;
 
-	my ($val) = git_get_project_config('snapshot');
+	my ($cfg) = git_get_project_config($key);
 
-	if ($val) {
-		@fmts = ($val eq 'none' ? () : split /\s*[,\s]\s*/, $val);
+	if ($cfg) {
+		return ($cfg eq 'none' ? () : split(/\s*[,\s]\s*/, $cfg));
 	}
 
-	return @fmts;
+	return @defaults;
 }
 
 sub feature_patches {
@@ -886,6 +992,35 @@ if ($git_avatar eq 'gravatar') {
 	$git_avatar = '';
 }
 
+# ordering of committags
+our @committags = gitweb_get_feature('committags');
+
+# whether we've loaded committags for the project yet
+our $loaded_project_committags = 0;
+
+# Load committag configs from the repository config file and and
+# incorporate them into the gitweb defaults where permitted by the
+# site administrator.
+sub gitweb_load_project_committags {
+	return if (!$git_dir || $loaded_project_committags);
+	my %project_config = ();
+	my %raw_config = git_parse_project_config('gitweb\.committag');
+	foreach my $key (keys(%raw_config)) {
+		next if ($key !~ /gitweb\.committag\.[^.]+\.[^.]/);
+		my ($gitweb_prefix, $committag_prefix, $ctname, $option) =
+			split(/\./, $key, 4);
+		$project_config{$ctname}{$option} = $raw_config{$key};
+	}
+	foreach my $ctname (keys(%committags)) {
+		next if (!$committags{$ctname}{'override'});
+		foreach my $optname (keys %{$project_config{$ctname}}) {
+			$committags{$ctname}{'options'}{$optname} =
+				$project_config{$ctname}{$optname};
+		}
+	}
+	$loaded_project_committags = 1;
+}
+
 # dispatch
 if (!defined $action) {
 	if (defined $hash) {
@@ -1458,13 +1593,99 @@ sub file_type_long {
 sub format_log_line_html {
 	my $line = shift;
 
-	$line = esc_html($line, -nbsp=>1);
-	$line =~ s{\b([0-9a-fA-F]{8,40})\b}{
-		$cgi->a({-href => href(action=>"object", hash=>$1),
-					-class => "text"}, $1);
-	}eg;
+	# Merge project configs with site default committag definitions if
+	# it hasn't been done yet
+	gitweb_load_project_committags();
 
-	return $line;
+	# In this list of log message fragments, a string ref indicates
+	# HTML, and a string indicates plain text.  The string refs are
+	# also currently not processed by subsequent committags.
+	my @message_fragments = ( $line );
+
+COMMITTAG:
+	foreach my $ctname (@committags) {
+		next COMMITTAG unless exists $committags{$ctname};
+		my $committag = $committags{$ctname};
+
+		next COMMITTAG unless exists $committag->{'sub'};
+		my $sub = $committag->{'sub'};
+
+		next COMMITTAG unless exists $committag->{'options'};
+		my $opts = $committag->{'options'};
+
+		next COMMITTAG unless exists $opts->{'pattern'};
+		my $pattern = $opts->{'pattern'};
+
+		my @new_message_fragments = ();
+
+	PART:
+		foreach my $fragment (@message_fragments) {
+			next PART if $fragment eq "";
+			if (ref($fragment)) {
+				push @new_message_fragments, $fragment;
+				next PART;
+			}
+
+			my $oldpos = 0;
+
+		MATCH:
+			while ($fragment =~ m/$pattern/gc) {
+				my ($prepos, $postpos) = ($-[0], $+[0]);
+				my $repl = $sub->($opts, $&, $1);
+				$repl = "" if (!defined $repl);
+
+				my $pre = substr($fragment, $oldpos, $prepos - $oldpos);
+				push_or_append(\@new_message_fragments, $pre);
+				push_or_append(\@new_message_fragments, $repl);
+
+				$oldpos = $postpos;
+			} # end while [regexp matches]
+
+			my $rest = substr($fragment, $oldpos);
+			push_or_append(\@new_message_fragments, $rest);
+
+		} # end foreach (@message_fragments)
+
+		@message_fragments = @new_message_fragments;
+	} # end foreach (@committags)
+
+	# Escape any remaining plain text and concatenate
+	my $html = '';
+	for my $fragment (@message_fragments) {
+		if (ref($fragment)) {
+			$html .= $$fragment;
+		} else {
+			$html .= esc_html($fragment, -nbsp=>1);
+		}
+	}
+
+	return $html;
+}
+
+# Returns a ref to an HTML snippet that links the whole match to a URL
+# formed from the 'url' option and the first captured subgroup.  This
+# is a helper function used in %committags.
+sub hyperlink_committag {
+	my ($opts, @match) = @_;
+	return \$cgi->a({-href => $opts->{'url'} . $cgi->escape($match[1]),
+	                 -class => "text"},
+	                esc_html($match[0], -nbsp=>1));
+}
+
+
+sub push_or_append (\@@) {
+	my $fragments = shift;
+
+	if (ref $_[0] || ! @$fragments || ref $fragments->[-1]) {
+		push @$fragments, @_;
+	} else {
+		my $a = pop @$fragments;
+		my $b = shift @_;
+
+		push @$fragments, $a . $b, @_;
+	}
+	# imitate push
+	return scalar @$fragments;
 }
 
 # format marker of refs pointing to given object
diff --git a/t/t9502-gitweb-committags.sh b/t/t9502-gitweb-committags.sh
new file mode 100755
index 0000000..f86cb3d
--- /dev/null
+++ b/t/t9502-gitweb-committags.sh
@@ -0,0 +1,150 @@
+#!/bin/sh
+
+test_description='gitweb committag tests.
+
+This test runs gitweb (git web interface) as CGI script from
+commandline, and checks that committags perform the expected
+transformations on log messages.'
+
+. ./gitweb-lib.sh
+
+# ----------------------------------------------------------------------
+# sha1 linking
+#
+echo sha1_test > file.txt
+git add file.txt
+git commit -q -F - file.txt <<END
+Summary
+
+See also commit 567890ab
+END
+test_expect_success 'sha1 link: enabled by default' '
+	gitweb_run "p=.git;a=commit;h=HEAD" &&
+	grep -q \
+		"commit&nbsp;<a class=\"text\" href=\".*\">567890ab</a>" \
+		gitweb.output
+'
+test_debug 'cat gitweb.log'
+test_debug 'grep 567890ab gitweb.output'
+
+# ----------------------------------------------------------------------
+# bugzilla commit tag
+#
+
+echo bugzilla_test > file.txt
+git add file.txt
+git commit -q -F - file.txt <<END
+Fix foo
+
+Fixes bug 1234 involving foo.
+END
+git config gitweb.committags 'sha1, bugzilla'
+test_expect_success 'bugzilla: enabled but not permitted' '
+	gitweb_run "p=.git;a=commit;h=HEAD" &&
+	grep -F -q \
+		"Fixes&nbsp;bug&nbsp;1234&nbsp;involving" \
+		gitweb.output
+'
+test_debug 'cat gitweb.log'
+test_debug 'grep 1234 gitweb.output'
+
+echo '$feature{"committags"}{"override"} = 1;' >> gitweb_config.perl
+test_expect_success 'bugzilla: enabled' '
+	gitweb_run "p=.git;a=commit;h=HEAD" &&
+	grep -F -q \
+		"Fixes&nbsp;<a class=\"text\" href=\"http://bugzilla.example.com/show_bug.cgi?id=1234\">bug&nbsp;1234</a>&nbsp;involving" \
+		gitweb.output
+'
+test_debug 'cat gitweb.log'
+test_debug 'grep 1234 gitweb.output'
+
+git config gitweb.committag.bugzilla.url 'http://bts.example.com?bug='
+test_expect_success 'bugzilla: url overridden but not permitted' '
+	gitweb_run "p=.git;a=commit;h=HEAD" &&
+	grep -F -q \
+		"Fixes&nbsp;<a class=\"text\" href=\"http://bugzilla.example.com/show_bug.cgi?id=1234\">bug&nbsp;1234</a>&nbsp;involving" \
+		gitweb.output
+'
+test_debug 'cat gitweb.log'
+test_debug 'grep 1234 gitweb.output'
+
+echo '$committags{"bugzilla"}{"override"} = 1;' >> gitweb_config.perl
+test_expect_success 'bugzilla: url overridden' '
+	gitweb_run "p=.git;a=commit;h=HEAD" &&
+	grep -F -q \
+		"Fixes&nbsp;<a class=\"text\" href=\"http://bts.example.com?bug=1234\">bug&nbsp;1234</a>&nbsp;involving" \
+		gitweb.output
+'
+test_debug 'cat gitweb.log'
+test_debug 'grep 1234 gitweb.output'
+
+git config gitweb.committag.bugzilla.pattern 'Fixes bug (\d+)'
+test_expect_success 'bugzilla: pattern overridden' '
+	gitweb_run "p=.git;a=commit;h=HEAD" &&
+	grep -F -q \
+		"<a class=\"text\" href=\"http://bts.example.com?bug=1234\">Fixes&nbsp;bug&nbsp;1234</a>&nbsp;involving" \
+		gitweb.output
+'
+test_debug 'cat gitweb.log'
+test_debug 'grep 1234 gitweb.output'
+git config --unset gitweb.committag.bugzilla.pattern
+
+test_expect_success 'bugzilla: affects log view too' '
+	gitweb_run "p=.git;a=log" &&
+	grep -F -q \
+		"<a class=\"text\" href=\"http://bts.example.com?bug=1234\">bug&nbsp;1234</a>" \
+		gitweb.output
+'
+test_debug 'cat gitweb.log'
+test_debug 'grep 1234 gitweb.output'
+
+# ----------------------------------------------------------------------
+# url linking
+#
+echo url_test > file.txt
+git add file.txt
+url='http://user@pass:example.com/foo.html?u=v&x=y#z'
+url_esc="$(echo "$url" | sed 's/&/&amp;/g')"
+git commit -q -F - file.txt <<END
+Summary
+
+See also $url.
+END
+echo '$feature{"committags"}{"override"} = 1;' >> gitweb_config.perl
+git config gitweb.committags 'sha1, url'
+test_expect_success 'url link: links when enabled' '
+	gitweb_run "p=.git;a=commit;h=HEAD" &&
+	grep -q -F \
+		"See&nbsp;also&nbsp;<a class=\"text\" href=\"$url_esc\">$url_esc</a>." \
+		gitweb.output
+'
+test_debug 'cat gitweb.log'
+test_debug 'grep -F "$url" gitweb.output'
+
+# ----------------------------------------------------------------------
+# message id linking
+#
+echo msgid_test > file.txt
+git add file.txt
+url='http://mid.gmane.org/'
+msgid='<x@y.z>'
+msgid_esc="$(echo "$msgid" | sed 's/</\&lt;/g; s/>/\&gt;/g')"
+msgid_url="$url$(echo "$msgid" | sed 's/</%3C/g; s/@/%40/g; s/>/%3E/g')"
+git commit -q -F - file.txt <<END
+Summary
+
+See msg-id $msgid.
+END
+echo '$feature{"committags"}{"override"} = 1;' >> gitweb_config.perl
+git config gitweb.committags 'sha1, messageid'
+test_expect_success 'msgid link: linked when enabled' '
+	gitweb_run "p=.git;a=commit;h=HEAD" &&
+	grep -q -F \
+		"See&nbsp;<a class=\"text\" href=\"$msgid_url\">msg-id&nbsp;$msgid_esc</a>." \
+		gitweb.output
+'
+test_debug 'cat gitweb.log'
+test_debug 'grep -F "y.z" gitweb.output'
+
+
+test_done
-- 
1.6.4.4

^ permalink raw reply related

* Re: [PATCH 1/2] replace: use a GIT_NO_REPLACE_OBJECTS env variable
From: Christian Couder @ 2009-11-18  6:48 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Junio C Hamano, git, Michael J Gruber, Jakub Narebski, bill lam,
	Andreas Schwab, Paul Mackerras
In-Reply-To: <4B0253DA.1090003@viscovery.net>

On mardi 17 novembre 2009, Johannes Sixt wrote:
> Christian Couder schrieb:
> > diff --git a/git.c b/git.c
> > index bd2c5fe..7f7d73d 100644
> > --- a/git.c
> > +++ b/git.c
> > @@ -89,6 +89,9 @@ static int handle_options(const char ***argv, int
> > *argc, int *envchanged) *envchanged = 1;
> >  		} else if (!strcmp(cmd, "--no-replace-objects")) {
> >  			read_replace_refs = 0;
> > +			setenv(NO_REPLACE_OBJECTS_ENVIRONMENT, "", 1);
>
> It is safer to set to a non-empty string, e.g., "1".

Ok, I will use a non empty string.

> I think this variable should be added to the list in connect.c around
> line 630 so that it does not propagate to the remote end.

Ok, I have done that in the version I will send.

Thanks,
Christian.

^ permalink raw reply

* Re: [PATCH 1/2] replace: use a GIT_NO_REPLACE_OBJECTS env variable
From: Christian Couder @ 2009-11-18  6:49 UTC (permalink / raw)
  To: Michael J Gruber
  Cc: Junio C Hamano, git, Jakub Narebski, Johannes Sixt, bill lam,
	Andreas Schwab, Paul Mackerras
In-Reply-To: <4B026DE8.9070905@drmicha.warpmail.net>

On mardi 17 novembre 2009, Michael J Gruber wrote:
> Christian Couder venit, vidit, dixit 17.11.2009 06:11:
> > This environment variable is set when the --no-replace-objects
> > flag is passed to git, and it is read when other environment
> > variables are read.
> >
> > It is useful for example for scripts, as the git commands used in
> > them can now be aware that they must not read replace refs.
> >
> > Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
>
> Tested-by: Michael J Gruber <git@drmicha.warpmail.net>
>
> :) This works, thanks, as well as the gitk patch 2/2, which is difficult
> to cover by test scripts. Some OSes (or rather certain setenv/putenv
> variants) have problems distinguishing an unset variable from an empty
> one. I think we've worked around this, but avoiding it is safer, as J6t
> pointed out.

Ok.

Thanks for testing,
Christian.

^ permalink raw reply

* [PATCH v2 1/2] replace: use a GIT_NO_REPLACE_OBJECTS env variable
From: Christian Couder @ 2009-11-18  6:50 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Michael J Gruber, Jakub Narebski, Johannes Sixt, bill lam,
	Andreas Schwab, Paul Mackerras

This environment variable is set when the --no-replace-objects
flag is passed to git, and it is read when other environment
variables are read.

It is useful for example for scripts, as the git commands used in
them can now be aware that they must not read replace refs.

The GIT_NO_REPLACE_OBJECTS is set to "1" instead of "" as it is
safer on some platforms, thanks to Johannes Sixt and Michael J
Gruber.

Tested-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 cache.h            |    1 +
 connect.c          |    1 +
 environment.c      |    2 ++
 git.c              |    3 +++
 t/t6050-replace.sh |   17 +++++++++++++++++
 5 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/cache.h b/cache.h
index 71a731d..bc77909 100644
--- a/cache.h
+++ b/cache.h
@@ -369,6 +369,7 @@ static inline enum object_type object_type(unsigned int mode)
 #define CONFIG_ENVIRONMENT "GIT_CONFIG"
 #define EXEC_PATH_ENVIRONMENT "GIT_EXEC_PATH"
 #define CEILING_DIRECTORIES_ENVIRONMENT "GIT_CEILING_DIRECTORIES"
+#define NO_REPLACE_OBJECTS_ENVIRONMENT "GIT_NO_REPLACE_OBJECTS"
 #define GITATTRIBUTES_FILE ".gitattributes"
 #define INFOATTRIBUTES_FILE "info/attributes"
 #define ATTRIBUTE_MACRO_PREFIX "[attr]"
diff --git a/connect.c b/connect.c
index 7945e38..c4f134f 100644
--- a/connect.c
+++ b/connect.c
@@ -630,6 +630,7 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
 			GIT_WORK_TREE_ENVIRONMENT,
 			GRAFT_ENVIRONMENT,
 			INDEX_ENVIRONMENT,
+			NO_REPLACE_OBJECTS_ENVIRONMENT,
 			NULL
 		};
 		conn->env = env;
diff --git a/environment.c b/environment.c
index 5de6837..279a38a 100644
--- a/environment.c
+++ b/environment.c
@@ -83,6 +83,8 @@ static void setup_git_env(void)
 	git_graft_file = getenv(GRAFT_ENVIRONMENT);
 	if (!git_graft_file)
 		git_graft_file = git_pathdup("info/grafts");
+	if (read_replace_refs && getenv(NO_REPLACE_OBJECTS_ENVIRONMENT))
+		read_replace_refs = 0;
 }
 
 int is_bare_repository(void)
diff --git a/git.c b/git.c
index bd2c5fe..d50bbc3 100644
--- a/git.c
+++ b/git.c
@@ -89,6 +89,9 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 				*envchanged = 1;
 		} else if (!strcmp(cmd, "--no-replace-objects")) {
 			read_replace_refs = 0;
+			setenv(NO_REPLACE_OBJECTS_ENVIRONMENT, "1", 1);
+			if (envchanged)
+				*envchanged = 1;
 		} else if (!strcmp(cmd, "--git-dir")) {
 			if (*argc < 2) {
 				fprintf(stderr, "No directory given for --git-dir.\n" );
diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index d4818b4..203ffdb 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -77,6 +77,11 @@ test_expect_success 'test --no-replace-objects option' '
      git --no-replace-objects show $HASH2 | grep "A U Thor"
 '
 
+test_expect_success 'test GIT_NO_REPLACE_OBJECTS env variable' '
+     GIT_NO_REPLACE_OBJECTS=1 git cat-file commit $HASH2 | grep "author A U Thor" &&
+     GIT_NO_REPLACE_OBJECTS=1 git show $HASH2 | grep "A U Thor"
+'
+
 cat >tag.sig <<EOF
 object $HASH2
 type commit
@@ -202,6 +207,18 @@ test_expect_success 'fetch branch with replacement' '
      cd ..
 '
 
+test_expect_success 'bisect and replacements' '
+     git bisect start $HASH7 $HASH1 &&
+     test "$S" = "$(git rev-parse --verify HEAD)" &&
+     git bisect reset &&
+     GIT_NO_REPLACE_OBJECTS=1 git bisect start $HASH7 $HASH1 &&
+     test "$HASH4" = "$(git rev-parse --verify HEAD)" &&
+     git bisect reset &&
+     git --no-replace-objects bisect start $HASH7 $HASH1 &&
+     test "$HASH4" = "$(git rev-parse --verify HEAD)" &&
+     git bisect reset
+'
+
 #
 #
 test_done
-- 
1.6.5.1.gaf97d

^ permalink raw reply related

* [PATCH v2 2/2] gitk: add --no-replace-objects option
From: Christian Couder @ 2009-11-18  6:50 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Michael J Gruber, Jakub Narebski, Johannes Sixt, bill lam,
	Andreas Schwab, Paul Mackerras

This option simply sets the GIT_NO_REPLACE_OBJECTS environment
variable, and that is enough to make gitk ignore replace refs.

The GIT_NO_REPLACE_OBJECTS is set to "1" instead of "" as it is
safer on some platforms, thanks to Johannes Sixt and Michael J
Gruber.

Tested-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 gitk-git/gitk |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index a0214b7..c586b93 100644
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -128,7 +128,7 @@ proc unmerged_files {files} {
 }
 
 proc parseviewargs {n arglist} {
-    global vdatemode vmergeonly vflags vdflags vrevs vfiltered vorigargs
+    global vdatemode vmergeonly vflags vdflags vrevs vfiltered vorigargs env
 
     set vdatemode($n) 0
     set vmergeonly($n) 0
@@ -208,6 +208,9 @@ proc parseviewargs {n arglist} {
 		# git rev-parse doesn't understand --merge
 		lappend revargs --gitk-symmetric-diff-marker MERGE_HEAD...HEAD
 	    }
+	    "--no-replace-objects" {
+		set env(GIT_NO_REPLACE_OBJECTS) "1"
+	    }
 	    "-*" {
 		# Other flag arguments including -<n>
 		if {[string is digit -strict [string range $arg 1 end]]} {
-- 
1.6.5.1.gaf97d

^ permalink raw reply related

* [PATCH v3] Expand ~ and ~user in core.excludesfile, commit.template
From: Matthieu Moy @ 2009-11-18  7:29 UTC (permalink / raw)
  To: git, gitster; +Cc: Matthieu Moy, Karl Chen
In-Reply-To: <1258478665-8635-1-git-send-email-Matthieu.Moy@imag.fr>

These config variables are parsed to substitute ~ and ~user with getpw
entries.

user_path() refactored into new function expand_user_path(), to allow
dynamically allocating the return buffer.

Original patch by Karl Chen, modified by Matthieu Moy, and further
amended by Junio C Hamano.

Signed-off-by: Karl Chen <quarl@quarl.org>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---

Since v2, I changed the code to use $HOME instead of the actual home
to expand ~.

This way, if you run "HOME=/else/where/ git status", then both
.gitconfig and ~/.gitignore will be taken from /else/where.

 Documentation/config.txt |    7 +++-
 builtin-commit.c         |    2 +-
 cache.h                  |    2 +
 config.c                 |   12 ++++++-
 path.c                   |   83 +++++++++++++++++++++++++++------------------
 5 files changed, 69 insertions(+), 37 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index cb73d75..fb1740c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -380,8 +380,9 @@ Common unit suffixes of 'k', 'm', or 'g' are supported.
 core.excludesfile::
 	In addition to '.gitignore' (per-directory) and
 	'.git/info/exclude', git looks into this file for patterns
-	of files which are not meant to be tracked.  See
-	linkgit:gitignore[5].
+	of files which are not meant to be tracked.  "~/" is expanded
+	to the value of `$HOME` and "~user/" to the specified user's
+	home directory.  See linkgit:gitignore[5].
 
 core.editor::
 	Commands such as `commit` and `tag` that lets you edit
@@ -670,6 +671,8 @@ color.ui::
 
 commit.template::
 	Specify a file to use as the template for new commit messages.
+	"~/" is expanded to the value of `$HOME` and "~user/" to the
+	specified user's home directory.
 
 diff.autorefreshindex::
 	When using 'git-diff' to compare with work tree
diff --git a/builtin-commit.c b/builtin-commit.c
index d525b89..09d2840 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -999,7 +999,7 @@ static int git_commit_config(const char *k, const char *v, void *cb)
 	struct wt_status *s = cb;
 
 	if (!strcmp(k, "commit.template"))
-		return git_config_string(&template_file, k, v);
+		return git_config_pathname(&template_file, k, v);
 
 	return git_status_config(k, v, s);
 }
diff --git a/cache.h b/cache.h
index 71a731d..42f7cd8 100644
--- a/cache.h
+++ b/cache.h
@@ -645,6 +645,7 @@ int set_shared_perm(const char *path, int mode);
 #define adjust_shared_perm(path) set_shared_perm((path), 0)
 int safe_create_leading_directories(char *path);
 int safe_create_leading_directories_const(const char *path);
+extern char *expand_user_path(const char *path);
 char *enter_repo(char *path, int strict);
 static inline int is_absolute_path(const char *path)
 {
@@ -903,6 +904,7 @@ extern unsigned long git_config_ulong(const char *, const char *);
 extern int git_config_bool_or_int(const char *, const char *, int *);
 extern int git_config_bool(const char *, const char *);
 extern int git_config_string(const char **, const char *, const char *);
+extern int git_config_pathname(const char **, const char *, const char *);
 extern int git_config_set(const char *, const char *);
 extern int git_config_set_multivar(const char *, const char *, const char *, int);
 extern int git_config_rename_section(const char *, const char *);
diff --git a/config.c b/config.c
index c644061..b3d1ff4 100644
--- a/config.c
+++ b/config.c
@@ -351,6 +351,16 @@ int git_config_string(const char **dest, const char *var, const char *value)
 	return 0;
 }
 
+int git_config_pathname(const char **dest, const char *var, const char *value)
+{
+	if (!value)
+		return config_error_nonbool(var);
+	*dest = expand_user_path(value);
+	if (!*dest)
+		die("Failed to expand user dir in: '%s'", value);
+	return 0;
+}
+
 static int git_default_core_config(const char *var, const char *value)
 {
 	/* This needs a better name */
@@ -474,7 +484,7 @@ static int git_default_core_config(const char *var, const char *value)
 		return git_config_string(&editor_program, var, value);
 
 	if (!strcmp(var, "core.excludesfile"))
-		return git_config_string(&excludes_file, var, value);
+		return git_config_pathname(&excludes_file, var, value);
 
 	if (!strcmp(var, "core.whitespace")) {
 		if (!value)
diff --git a/path.c b/path.c
index 047fdb0..6351b57 100644
--- a/path.c
+++ b/path.c
@@ -11,6 +11,7 @@
  * which is what it's designed for.
  */
 #include "cache.h"
+#include "strbuf.h"
 
 static char bad_path[] = "/bad-path/";
 
@@ -207,43 +208,49 @@ int validate_headref(const char *path)
 	return -1;
 }
 
-static char *user_path(char *buf, char *path, int sz)
+static struct passwd *getpw_str(const char *username, size_t len)
 {
 	struct passwd *pw;
-	char *slash;
-	int len, baselen;
+	char *username_z = xmalloc(len + 1);
+	memcpy(username_z, username, len);
+	username_z[len] = '\0';
+	pw = getpwnam(username_z);
+	free(username_z);
+	return pw;
+}
 
-	if (!path || path[0] != '~')
-		return NULL;
-	path++;
-	slash = strchr(path, '/');
-	if (path[0] == '/' || !path[0]) {
-		pw = getpwuid(getuid());
-	}
-	else {
-		if (slash) {
-			*slash = 0;
-			pw = getpwnam(path);
-			*slash = '/';
+/*
+ * Return a string with ~ and ~user expanded via getpw*.  If buf != NULL,
+ * then it is a newly allocated string. Returns NULL on getpw failure or
+ * if path is NULL.
+ */
+char *expand_user_path(const char *path)
+{
+	struct strbuf user_path = STRBUF_INIT;
+	const char *first_slash = strchrnul(path, '/');
+	const char *to_copy = path;
+
+	if (path == NULL)
+		goto return_null;
+	if (path[0] == '~') {
+		const char *username = path + 1;
+		size_t username_len = first_slash - username;
+		if (username_len == 0) {
+			const char * home = getenv("HOME");
+			strbuf_add(&user_path, home, strlen(home));
+		} else {
+			struct passwd *pw = getpw_str(username, username_len);
+			if (!pw)
+				goto return_null;
+			strbuf_add(&user_path, pw->pw_dir, strlen(pw->pw_dir));
 		}
-		else
-			pw = getpwnam(path);
+		to_copy = first_slash;
 	}
-	if (!pw || !pw->pw_dir || sz <= strlen(pw->pw_dir))
-		return NULL;
-	baselen = strlen(pw->pw_dir);
-	memcpy(buf, pw->pw_dir, baselen);
-	while ((1 < baselen) && (buf[baselen-1] == '/')) {
-		buf[baselen-1] = 0;
-		baselen--;
-	}
-	if (slash && slash[1]) {
-		len = strlen(slash);
-		if (sz <= baselen + len)
-			return NULL;
-		memcpy(buf + baselen, slash, len + 1);
-	}
-	return buf;
+	strbuf_add(&user_path, to_copy, strlen(to_copy));
+	return strbuf_detach(&user_path, NULL);
+return_null:
+	strbuf_release(&user_path);
+	return NULL;
 }
 
 /*
@@ -291,8 +298,18 @@ char *enter_repo(char *path, int strict)
 		if (PATH_MAX <= len)
 			return NULL;
 		if (path[0] == '~') {
-			if (!user_path(used_path, path, PATH_MAX))
+			char *newpath = expand_user_path(path);
+			if (!newpath || (PATH_MAX - 10 < strlen(newpath))) {
+				free(newpath);
 				return NULL;
+			}
+			/*
+			 * Copy back into the static buffer. A pity
+			 * since newpath was not bounded, but other
+			 * branches of the if are limited by PATH_MAX
+			 * anyway.
+			 */
+			strcpy(used_path, newpath); free(newpath);
 			strcpy(validated_path, path);
 			path = used_path;
 		}
-- 
1.6.5.2.152.gbbe9e

^ permalink raw reply related

* Re: [PATCH] Expand ~ and ~user in core.excludesfile, commit.template
From: Matthieu Moy @ 2009-11-18  7:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Andreas Schwab, Mike Hommey, Jeff King, git, Karl Chen
In-Reply-To: <7v8we4er0t.fsf@alter.siamese.dyndns.org>

> Andreas Schwab <schwab@linux-m68k.org> writes:
>
>> Mike Hommey <mh@glandium.org> writes:
>>
>> "~" should just expand to the value of $HOME, like in the shell,
>> independent of the real home directory of the real or effective user.

Right, I'll resend in a minute.

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

> How should this interact with installations that run gitosis/gitlite that
> use shared account, giving user identity via the ssh key?

I guess there's some kind of rhetorical question behind that I missed,
but if a gitosis-like installation uses the user git, and the user foo
connects to it identifier by its SSH key, I don't see what you could
do other than expanding ~ to the homedir of git, which is $HOME at the
time your run git on the server.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* What's cooking in git.git (Nov 2009, #04; Tue, 17)
From: Junio C Hamano @ 2009-11-18  7:53 UTC (permalink / raw)
  To: git

I'd like to tag 1.6.6-rc0 this coming weekend with most topics on 'next'
(and some from 'pu'), so that we can do the final 1.6.6 before the end of
the year.

It is likely that I'll be offline for most of the day tomorrow, even
though it will be my git Wednesday.

-- >8 --

Here are the topics that have been cooking.  Commits prefixed with '-' are
only in 'pu' while commits prefixed with '+' are in 'next'.  The ones
marked with '.' do not appear in any of the integration branches, but I am
still holding onto them.

In 1.7.0, we plan to correct handful of warts in the interfaces everybody
agrees that they were mistakes.  The resulting system may not be strictly
backward compatible.  Currently planned changes are:

 * refuse push to update the checked out branch in a non-bare repo by
   default

   Make "git push" into a repository to update the branch that is checked
   out fail by default.  You can countermand this default by setting a
   configuration variable in the receiving repository.

   http://thread.gmane.org/gmane.comp.version-control.git/107758/focus=108007

 * refuse push to delete the current branch by default

   Make "git push $there :$killed" to delete the branch that is pointed at
   by its HEAD fail by default.  You can countermand this default by
   setting a configuration variable in the receiving repository.

   http://thread.gmane.org/gmane.comp.version-control.git/108862/focus=108936

 * git-send-email won't make deep threads by default

   Many people said that by default when sending more than 2 patches the
   threading git-send-email makes by default is hard to read, and they
   prefer the default be one cover letter and each patch as a direct
   follow-up to the cover letter.  You can countermand this by setting a
   configuration variable.

   http://article.gmane.org/gmane.comp.version-control.git/109790

 * git-status won't be "git-commit --dry-run" anymore

   http://thread.gmane.org/gmane.comp.version-control.git/125989/focus=125993

 * "git-diff -w --exit-code" will exit success if only differences it
   found are whitespace changes that are stripped away from the output.

   http://thread.gmane.org/gmane.comp.version-control.git/119731/focus=119751

I wasn't fully functioning for the past few days, so this round we have
only added new topics and new patches to existing ones, without changing
the status of individual topics very much.

--------------------------------------------------
[Graduated to "master"]

* bs/maint-pre-commit-hook-sample (2009-11-05) 1 commit.
  (merged to 'next' on 2009-11-06 at d70f646)
 + pre-commit.sample: Diff against the empty tree when HEAD is invalid

* js/maint-diff-color-words (2009-10-30) 3 commits.
  (merged to 'next' on 2009-11-10 at 5619714)
 + diff --color-words: bit of clean-up
 + diff --color-words -U0: fix the location of hunk headers
 + t4034-diff-words: add a test for word diff without context

Fixes a corner case of running --color-words with -U0.

* sc/difftool-p4merge (2009-10-28) 1 commit
  (merged to 'next' on 2009-10-31 at 194b5c5)
 + mergetool--lib: add p4merge as a pre-configured mergetool option

I do not do p4 nor use difftool, so it's much easier for me to merge this
to 'master' and wait for anybody to scream if there is breakage.

* jk/maint-add-p-empty (2009-10-27) 1 commit.
  (merged to 'next' on 2009-10-30 at 2bd302f)
 + add-interactive: handle deletion of empty files

* lt/revision-bisect (2009-10-27) 1 commit.
  (merged to 'next' on 2009-10-30 at 81ee52b)
 + Add '--bisect' revision machinery argument

* rs/pretty-wrap (2009-11-08) 2 commits
  (merged to 'next' on 2009-11-08 at 8973fd8)
 + log --format: don't ignore %w() at the start of format string
  (merged to 'next' on 2009-10-30 at 403bbfe)
 + Implement wrap format %w() as if it is a mode switch
 (this branch uses js/log-rewrap.)

* js/log-rewrap (2009-10-18) 3 commits
  (merged to 'next' on 2009-10-30 at 403bbfe)
 + Teach --wrap to only indent without wrapping
 + Add strbuf_add_wrapped_text() to utf8.[ch]
 + print_wrapped_text(): allow hard newlines
 (this branch is used by rs/pretty-wrap.)

* fc/doc-fast-forward (2009-10-24) 1 commit.
  (merged to 'next' on 2009-11-01 at faaad90)
 + Use 'fast-forward' all over the place

* tz/maint-rpm (2009-11-11) 1 commit.
 + Makefile: Ensure rpm packages can be read by older rpm versions

* np/maint-sideband-favor-status (2009-11-11) 1 commit.
  (merged to 'next' on 2009-11-15 at 3ecd874)
 + give priority to progress messages

* sb/tutorial-test (2009-11-06) 4 commits
  (merged to 'next' on 2009-11-15 at 5c82651)
 + t1200: prepare for merging with Fast-forward bikeshedding
 + t1200: further modernize test script style
 + t1200: Make documentation and test agree
 + t1200: cleanup and modernize test style

* ef/msys-imap (2009-10-22) 9 commits.
  (merged to 'next' on 2009-10-31 at 8630603)
 + Windows: use BLK_SHA1 again
 + MSVC: Enable OpenSSL, and translate -lcrypto
 + mingw: enable OpenSSL
 + mingw: wrap SSL_set_(w|r)fd to call _get_osfhandle
 + imap-send: build imap-send on Windows
 + imap-send: fix compilation-error on Windows
 + imap-send: use run-command API for tunneling
 + imap-send: use separate read and write fds
 + imap-send: remove useless uid code

--------------------------------------------------
[New Topics]

* jn/faster-completion-startup (2009-11-17) 1 commit.
 - Speed up bash completion loading

* th/maint-remote-update-help-string (2009-11-15) 1 commit.
 - Update 'git remote update' usage string to match man page.

* tr/maint-merge-ours-clarification (2009-11-15) 3 commits.
 - rebase: refuse to rebase with -s ours
  (merged to 'next' on 2009-11-17 at 3291125)
 + rebase docs: clarify --merge and --strategy
 + Documentation: clarify 'ours' merge strategy

* tc/format-attribute (2009-11-14) 1 commit
 - Check the format of more printf-type functions

* jk/maint-break-rename-reduce-memory (2009-11-16) 2 commits.
  (merged to 'next' on 2009-11-16 at 5b5a93f)
 + diffcore-break: save cnt_data for other phases
 + diffcore-break: free filespec data as we go

* bc/grep-i-F (2009-11-06) 1 commit.
  (merged to 'next' on 2009-11-17 at a9b138c)
 + grep: Allow case insensitive search of fixed-strings

* mm/config-pathname-tilde-expand (2009-11-17) 1 commit.
  (merged to 'next' on 2009-11-17 at 7ba213d)
 + Expand ~ and ~user in core.excludesfile, commit.template

* pb/maint-use-custom-perl (2009-11-17) 1 commit.
  (merged to 'next' on 2009-11-17 at 1ee8d46)
 + Make sure $PERL_PATH is defined when the test suite is run.

* th/remote-usage (2009-11-16) 1 commit.
 - git remote: Separate usage strings for subcommands

* mo/maint-crlf-doc (2009-11-14) 1 commit.
  (merged to 'next' on 2009-11-17 at abd9133)
 + core.autocrlf documentation: mention the crlf attribute

--------------------------------------------------
[Stalled]

* rj/cygwin-msvc (2009-11-09) 3 commits.
 - Add explicit Cygwin check to guard WIN32 header inclusion
 - MSVC: Add support for building with NO_MMAP
 - Makefile: keep MSVC and Cygwin configuration separate
 (this branch uses rj/maint-simplify-cygwin-makefile.)

I think J6t was not happy with the tip one.

* jc/fix-tree-walk (2009-10-22) 11 commits.
  (merged to 'next' on 2009-10-22 at 10c0c8f)
 + Revert failed attempt since 353c5ee
 + read-tree --debug-unpack
  (merged to 'next' on 2009-10-11 at 0b058e2)
 + unpack-trees.c: look ahead in the index
 + unpack-trees.c: prepare for looking ahead in the index
 + Aggressive three-way merge: fix D/F case
 + traverse_trees(): handle D/F conflict case sanely
 + more D/F conflict tests
 + tests: move convenience regexp to match object names to test-lib.sh
 + unpack_callback(): use unpack_failed() consistently
 + unpack-trees: typofix
 + diff-lib.c: fix misleading comments on oneway_diff()

This has some stupid bugs and temporarily reverted from 'next' until I can
fix it, but the "temporarily" turned out to be very loooong.  Sigh...

* jh/notes (2009-10-09) 22 commits.
 - fast-import: Proper notes tree manipulation using the notes API
 - Refactor notes concatenation into a flexible interface for combining notes
 - Notes API: Allow multiple concurrent notes trees with new struct notes_tree
 - Notes API: for_each_note(): Traverse the entire notes tree with a callback
 - Notes API: get_note(): Return the note annotating the given object
 - Notes API: add_note(): Add note objects to the internal notes tree structure
 - Notes API: init_notes(): Initialize the notes tree from the given notes ref
 - Notes API: get_commit_notes() -> format_note() + remove the commit restriction
  (merged to 'next' on 2009-11-01 at 948327a)
 + Add selftests verifying concatenation of multiple notes for the same commit
 + Refactor notes code to concatenate multiple notes annotating the same object
 + Add selftests verifying that we can parse notes trees with various fanouts
 + Teach the notes lookup code to parse notes trees with various fanout schemes
 + Teach notes code to free its internal data structures on request
 + Add '%N'-format for pretty-printing commit notes
 + Add flags to get_commit_notes() to control the format of the note string
 + t3302-notes-index-expensive: Speed up create_repo()
 + fast-import: Add support for importing commit notes
 + Teach "-m <msg>" and "-F <file>" to "git notes edit"
 + Add an expensive test for git-notes
 + Speed up git notes lookup
 + Add a script to edit/inspect notes
 + Introduce commit notes

I somehow thought that the later API part was waiting for updates but
nothing seems to be happening.

* jn/gitweb-blame (2009-09-01) 5 commits.
 - gitweb: Minify gitweb.js if JSMIN is defined
 - gitweb: Create links leading to 'blame_incremental' using JavaScript
  (merged to 'next' on 2009-10-11 at 73c4a83)
 + gitweb: Colorize 'blame_incremental' view during processing
 + gitweb: Incremental blame (using JavaScript)
 + gitweb: Add optional "time to generate page" info in footer

Ajax-y blame.  Any progress or RFH?

* sr/gfi-options (2009-09-06) 6 commits.
 - fast-import: test the new option command
 - fast-import: add option command
 - fast-import: test the new feature command
 - fast-import: add feature command
 - fast-import: put marks reading in it's own function
 - fast-import: put option parsing code in separate functions

It seemed to be moving again soon, but nothing has happened yet...

* je/send-email-no-subject (2009-08-05) 1 commit.
  (merged to 'next' on 2009-10-11 at 1b99c56)
 + send-email: confirm on empty mail subjects

The existing tests cover the positive case (i.e. as long as the user says
"yes" to the "do you really want to send this message that lacks subject",
the message is sent) of this feature, but the feature itself needs its own
test to verify the negative case (i.e. does it correctly stop if the user
says "no"?)

--------------------------------------------------
[Cooking]

* jp/fetch-cull-many-refs (2009-11-13) 3 commits
  (merged to 'next' on 2009-11-15 at db0f967)
 + remote: fix use-after-free error detected by glibc in ref_remove_duplicates
  (merged to 'next' on 2009-11-01 at 1f09ce9)
 + fetch: Speed up fetch of large numbers of refs
 + remote: Make ref_remove_duplicates faster for large numbers of refs

Soon in 'master'.

* jn/help-everywhere (2009-11-09) 21 commits
  (merged to 'next' on 2009-11-17 at 3a2dffe)
 + diff --no-index: make the usage string less scary
 + merge-{recursive,subtree}: use usagef() to print usage
 + Introduce usagef() that takes a printf-style format
 + Let 'git <command> -h' show usage without a git dir
 + Show usage string for 'git http-push -h'
 + Let 'git http-fetch -h' show usage outside any git repository
 + Show usage string for 'git stripspace -h'
 + Show usage string for 'git unpack-file -h'
 + Show usage string for 'git show-index -h'
 + Show usage string for 'git rev-parse -h'
 + Show usage string for 'git merge-one-file -h'
 + Show usage string for 'git mailsplit -h'
 + Show usage string for 'git imap-send -h'
 + Show usage string for 'git get-tar-commit-id -h'
 + Show usage string for 'git fast-import -h'
 + Show usage string for 'git check-ref-format -h'
 + Show usage string for 'git show-ref -h'
 + Show usage string for 'git merge-ours -h'
 + Show usage string for 'git commit-tree -h'
 + Show usage string for 'git cherry -h'
 + Show usage string for 'git grep -h'
 (this branch uses jn/maint-http-fetch-mingw and jn/remove-fetch--tool.)

There were unrelated but still worthy fixes, so I reordered some of them;
also the "usage()" change is different from the one that was posted (see
my comment in $gmane/132592).

* jn/maint-http-fetch-mingw (2009-11-09) 1 commit.
  (merged to 'next' on 2009-11-17 at cd35125)
 + http-fetch: add missing initialization of argv0_path
 (this branch is used by jn/help-everywhere.)

* jn/remove-fetch--tool (2009-11-09) 1 commit
  (merged to 'next' on 2009-11-17 at 72f6c3b)
 + Retire fetch--tool helper to contrib/examples
 (this branch is used by jn/help-everywhere.)

These two were originally part of the "help-everywhere" topic but
they can stand on their own.

* jc/log-stdin (2009-11-03) 1 commit
 - Teach --stdin option to "log" family

This is not signed-off (see $gmane/131971 for list of things you can do to
help advancing this topic).

* jn/gitweb-log-history (2009-11-13) 3 commits
  (merged to 'next' on 2009-11-17 at d225f7d)
 + gitweb: Make 'history' view (re)use git_log_generic()
 + gitweb: Refactor common parts of 'log' and 'shortlog' views
 + gitweb: Refactor 'log' action generation, adding git_log_body()

* jn/rfc-pull-rebase-error-message (2009-11-12) 1 commit
 - git-pull.sh --rebase: overhaul error handling when no candidates are found

I heard this needs at least retitling among other changes?

* rg/doc-workflow (2009-11-17) 4 commits.
 - [Further RFC updates from Raman]
 - [An RFC fix-up to further reword release section]
 - Corrections to release management section in gitworkflows.txt
 - Add branch management for releases to gitworkflows

The top three patches are meant to be squashed into the first one.

* sb/ls-tree-parseopt (2009-11-13) 2 commits.
  (merged to 'next' on 2009-11-17 at c383204)
 + ls-tree: migrate to parse-options
 + t3101: test more ls-tree options

* jl/submodule-add-noname (2009-09-22) 1 commit.
  (merged to 'next' on 2009-11-15 at 3a77d01)
 + git submodule add: make the <path> parameter optional

Dscho started an interesting discussion regarding the larger workflow in
which the "submodule add" is used.  I think the patch itself makes sense
but at the same time it probably makes sense to also take the <path> and
infer the <repository> as Dscho suggested, probably in "git submodule
add", not in "git add" proper, at least initially.

* sc/protocol-doc (2009-11-03) 1 commit.
  (merged to 'next' on 2009-11-15 at 32d6de8)
 + Update packfile transfer protocol documentation

* tr/filter-branch (2009-11-10) 2 commits.
  (merged to 'next' on 2009-11-15 at 79c6a1d)
 + filter-branch: nearest-ancestor rewriting outside subdir filter
 + filter-branch: stop special-casing $filter_subdir argument

Updated again.  Looked sane, except that the option might not be
necessary, but that can be fixed while in 'next'.

* em/commit-claim (2009-11-04) 1 commit
 - commit -c/-C/--amend: reset timestamp and authorship to committer with --reset-author

I just picked better bits from both versions, but this needs to be
rethought.

* bg/format-patch-doc-update (2009-11-07) 4 commits.
  (merged to 'next' on 2009-11-17 at 68b9056)
 + format-patch: Add "--no-stat" as a synonym for "-p"
 + format-patch documentation: Fix formatting
 + format-patch documentation: Remove diff options that are not useful
 + format-patch: Always generate a patch

Looked sensible, even though this may want to wait for 1.7.0.  We'll see.

* rj/maint-simplify-cygwin-makefile (2009-10-27) 1 commit.
 - Makefile: merge two Cygwin configuration sections into one
 (this branch is used by rj/cygwin-msvc.)

This is one of the most obviously correct bit from "Compiling on Cygwin
using MSVC fails" topic.

* bg/fetch-multi (2009-11-10) 9 commits.
 - Re-implement 'git remote update' using 'git fetch'
 - builtin-fetch: add --dry-run option
 - builtin-fetch: add --prune option
 - teach warn_dangling_symref to take a FILE argument
 - remote: refactor some logic into get_stale_heads()
 - Add missing test for 'git remote update --prune'
 - Add the configuration option skipFetchAll
 - Teach the --multiple option to 'git fetch'
 - Teach the --all option to 'git fetch'

* cc/bisect-doc (2009-11-08) 1 commit
 - Documentation: add "Fighting regressions with git bisect" article

Any comments?  Should it go to Documentation/technical instead?

* jn/editor-pager (2009-10-30) 9 commits
  (merged to 'next' on 2009-11-15 at 7f3e3ae)
 + Provide a build time default-pager setting
 + Provide a build time default-editor setting
 + am -i, git-svn: use "git var GIT_PAGER"
 + add -i, send-email, svn, p4, etc: use "git var GIT_EDITOR"
 + Teach git var about GIT_PAGER
 + Teach git var about GIT_EDITOR
 + Suppress warnings from "git var -l"
 + Do not use VISUAL editor on dumb terminals
 + Handle more shell metacharacters in editor names

* bw/autoconf-more (2009-11-04) 2 commits
  (merged to 'next' on 2009-11-15 at e86a8c9)
 + configure: add settings for gitconfig, editor and pager
 + configure: add macro to set arbitrary make variables

This will follow jn/editor-pager series.

* sr/vcs-helper (2009-11-18) 12 commits
 - Add Python support library for remote helpers
 - Basic build infrastructure for Python scripts
 - Allow helpers to report in "list" command that the ref is unchanged
 - Fix various memory leaks in transport-helper.c
 - Allow helper to map private ref names into normal names
 - Add support for "import" helper command
 - Allow specifying the remote helper in the url
 - Add a config option for remotes to specify a foreign vcs
 - Allow fetch to modify refs
 - Use a function to determine whether a remote is valid
 - Allow programs to not depend on remotes having urls
 - Fix memory leak in helper method for disconnect

Replaced again.

* mr/gitweb-snapshot (2009-11-07) 4 commits.
 - gitweb: Smarter snapshot names
 - gitweb: Document current snapshot rules via new tests
 - t/gitweb-lib.sh: Split gitweb output into headers and body
  (merged to 'next' on 2009-10-11 at 22ba047)
 + gitweb: check given hash before trying to create snapshot

Replaced commits near the tip with recent updates.

* jc/pretty-lf (2009-10-04) 1 commit.
 - Pretty-format: %[+-]x to tweak inter-item newlines

* ks/precompute-completion (2009-11-15) 4 commits.
  (merged to 'next' on 2009-11-15 at 23cdb96)
 + Revert ks/precompute-completion series
  (merged to 'next' on 2009-10-28 at cd5177f)
 + completion: ignore custom merge strategies when pre-generating
  (merged to 'next' on 2009-10-22 at f46a28a)
 + bug: precomputed completion includes scripts sources
  (merged to 'next' on 2009-10-14 at adf722a)
 + Speedup bash completion loading

Reverted out of 'next', to be replaced with jn/faster-completion-startup
topic.

* sp/smart-http (2009-11-14) 37 commits
  (merged to 'next' on 2009-11-17 at 11067eb)
 + http-backend: Let gcc check the format of more printf-type functions.
 + http-backend: Fix access beyond end of string.
  (merged to 'next' on 2009-11-15 at 2a326b2)
 + http-backend: Fix bad treatment of uintmax_t in Content-Length
 + t5551-http-fetch: Work around broken Accept header in libcurl
 + t5551-http-fetch: Work around some libcurl versions
 + http-backend: Protect GIT_PROJECT_ROOT from /../ requests
 + Git-aware CGI to provide dumb HTTP transport
  (merged to 'next' on 2009-11-06 at 666837c)
 + http-backend: Test configuration options
 + http-backend: Use http.getanyfile to disable dumb HTTP serving
 + test smart http fetch and push
 + http tests: use /dumb/ URL prefix
 + set httpd port before sourcing lib-httpd
 + t5540-http-push: remove redundant fetches
 + Smart HTTP fetch: gzip requests
 + Smart fetch over HTTP: client side
 + Smart push over HTTP: client side
 + Discover refs via smart HTTP server when available
 + http-backend: more explict LocationMatch
 + http-backend: add example for gitweb on same URL
 + http-backend: use mod_alias instead of mod_rewrite
 + http-backend: reword some documentation
 + http-backend: add GIT_PROJECT_ROOT environment var
 + Smart fetch and push over HTTP: server side
 + Add stateless RPC options to upload-pack, receive-pack
 + Git-aware CGI to provide dumb HTTP transport
 + remote-helpers: return successfully if everything up-to-date
 + Move WebDAV HTTP push under remote-curl
 + remote-helpers: Support custom transport options
 + remote-helpers: Fetch more than one ref in a batch
 + fetch: Allow transport -v -v -v to set verbosity to 3
 + remote-curl: Refactor walker initialization
 + Add multi_ack_detailed capability to fetch-pack/upload-pack
 + Move "get_ack()" back to fetch-pack
 + fetch-pack: Use a strbuf to compose the want list
 + pkt-line: Make packet_read_line easier to debug
 + pkt-line: Add strbuf based functions
 + http-push: fix check condition on http.c::finish_http_pack_request()

* nd/sparse (2009-08-20) 19 commits.
 - sparse checkout: inhibit empty worktree
 - Add tests for sparse checkout
 - read-tree: add --no-sparse-checkout to disable sparse checkout support
 - unpack-trees(): ignore worktree check outside checkout area
 - unpack_trees(): apply $GIT_DIR/info/sparse-checkout to the final index
 - unpack-trees(): "enable" sparse checkout and load $GIT_DIR/info/sparse-checkout
 - unpack-trees.c: generalize verify_* functions
 - unpack-trees(): add CE_WT_REMOVE to remove on worktree alone
 - Introduce "sparse checkout"
 - dir.c: export excluded_1() and add_excludes_from_file_1()
 - excluded_1(): support exclude files in index
 - unpack-trees(): carry skip-worktree bit over in merged_entry()
 - Read .gitignore from index if it is skip-worktree
 - Avoid writing to buffer in add_excludes_from_file_1()
 - Teach Git to respect skip-worktree bit (writing part)
 - Teach Git to respect skip-worktree bit (reading part)
 - Introduce "skip-worktree" bit in index, teach Git to get/set this bit
 - Add test-index-version
 - update-index: refactor mark_valid() in preparation for new options

The latest update I didn't look at very closely but I had an impression
that it was touching very generic codepath that would affect non sparse
cases, iow the patch looked very scary (the entire series already is).

--------------------------------------------------
[For 1.7.0]

* jc/1.7.0-no-commit-no-ff-2 (2009-10-22) 1 commit.
 - git-merge: forbid fast-forward and up-to-date when --no-commit is given

This makes "git merge --no-commit" fail when it results in fast-forward or
up-to-date.  I haven't described this at the beginning of this message
yet, as it is not clear if this change is even necessary.  Opinions?

* jk/1.7.0-status (2009-09-05) 5 commits.
 - docs: note that status configuration affects only long format
  (merged to 'next' on 2009-10-11 at 65c8513)
 + commit: support alternate status formats
 + status: add --porcelain output format
 + status: refactor format option parsing
 + status: refactor short-mode printing to its own function
 (this branch uses jc/1.7.0-status.)

Gives the --short output format to post 1.7.0 "git commit --dry-run" that
is similar to that of post 1.7.0 "git status".

The tip one is not in 'next' as I have been hoping that somebody may want
to change the code to make it unnecessary, but it does not seem to be
happening, so probably it should also go to 'next'.

* jc/1.7.0-status (2009-09-05) 4 commits.
  (merged to 'next' on 2009-10-11 at 9558627)
 + status: typo fix in usage
 + git status: not "commit --dry-run" anymore
 + git stat -s: short status output
 + git stat: the beginning of "status that is not a dry-run of commit"
 (this branch is used by jk/1.7.0-status.)

With this, "git status" is no longer "git commit --dry-run".

* jc/1.7.0-send-email-no-thread-default (2009-08-22) 1 commit.
  (merged to 'next' on 2009-10-11 at 043acdf)
 + send-email: make --no-chain-reply-to the default

* jc/1.7.0-diff-whitespace-only-status (2009-08-30) 4 commits.
  (merged to 'next' on 2009-10-11 at 546c74d)
 + diff.c: fix typoes in comments
 + Make test case number unique
 + diff: Rename QUIET internal option to QUICK
 + diff: change semantics of "ignore whitespace" options

This changes exit code from "git diff --ignore-whitespace" and friends
when there is no actual output.  It is a backward incompatible change, but
we could argue that it is a bugfix.

* jc/1.7.0-push-safety (2009-02-09) 2 commits.
  (merged to 'next' on 2009-10-11 at 81b8128)
 + Refuse deleting the current branch via push
 + Refuse updating the current branch in a non-bare repository via push

--------------------------------------------------
[I have been too busy to purge these]

* ne/rev-cache (2009-10-19) 7 commits.
 . support for commit grafts, slight change to general mechanism
 . support for path name caching in rev-cache
 . full integration of rev-cache into git, completed test suite
 . administrative functions for rev-cache, start of integration into git
 . support for non-commit object caching in rev-cache
 . basic revision cache system, no integration or features
 . man page and technical discussion for rev-cache

The author indicated that there is another round coming.  Does not seem to
pass the tests when merged to 'pu', so it has been ejected for now.

* jc/log-tz (2009-03-03) 1 commit.
 - Allow --date=local --date=other-format to work as expected

Maybe some people care about this.  I dunno.

* jc/mailinfo-remove-brackets (2009-07-15) 1 commit.
 - mailinfo: -b option keeps [bracketed] strings that is not a [PATCH] marker

Maybe some people care about this.  I dunno.

* pb/gitweb-no-project-list (2009-11-06) 3 commits.
 . gitweb: Polish the content tags support
 . gitweb: Support for no project list on gitweb front page
 . gitweb: Refactor project list routines

I picked these up but didn't queue as Warthog9's comments made certain
amount of sense to me.

^ permalink raw reply

* Re: [RFC PATCH 1/6] gitweb: Hyperlink committags in a commit message by regex matching
From: Petr Baudis @ 2009-11-18  8:20 UTC (permalink / raw)
  To: Marcel M. Cary; +Cc: Jakub Narebski, git, Giuseppe Bilotta, Francis Galiegue
In-Reply-To: <1258525350-5528-2-git-send-email-marcel@oak.homeunix.org>

On Tue, Nov 17, 2009 at 10:22:25PM -0800, Marcel M. Cary wrote:
> One additional thing that occured to me is that maybe the hyperlinks
> added by committags should have 'rel="nofollow"' by default?  And if
> so, maybe that needs to be configurable?  On the other hand, I'm not
> sure how useful it is to hide real URLs in the commit messages from
> search engines... ?

I don't think rel="nofollow" is necessary.

BTW, wouldn't it be useful to do the matching in blob bodies as well?
And is it sensible to call these "committags" at all then? I already
made the mistake of calling content tags "ctags" and I regret it; I
think calling yet another thing tags after git tags and ctags is almost
unbearable.

> diff --git a/gitweb/INSTALL b/gitweb/INSTALL
> index b76a0cf..9081ed8 100644
> --- a/gitweb/INSTALL
> +++ b/gitweb/INSTALL
> @@ -132,6 +132,11 @@ adding the following lines to your $GITWEB_CONFIG:
>  	$known_snapshot_formats{'zip'}{'disabled'} = 1;
>  	$known_snapshot_formats{'tgz'}{'compressor'} = ['gzip','-6'];
>  
> +To add a committag to the default list of commit tags, for example to
> +enable hyperlinking of bug numbers to a bug tracker for all projects:
> +
> +	push @{$feature{'committags'}{'default'}}, 'bugzilla';
> +
>  
>  Gitweb repositories
>  -------------------

I think this is not useful at all, since:

	(i) The user will also *always* need to override the URL.
	(ii) More importantly, the user has no idea what on earth commit
	tags are.

Could you please prepend this paragraph with a short committags
description, e.g.:

	"Gitweb can rewrite certain snippets of text in commit messages
	to hyperlinks, e.g. URL addresses or bug tracker references - we
	call these snippets 'committags'."

And you should also add

	$committags{'buzilla'}{'options'}{'url'} = ...

to the explanation, together with a reference to the appropriate part of
gitweb.perl for more details.

> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index e4cbfc3..2d72202 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -213,6 +213,97 @@ our %avatar_size = (
>  	'double'  => 32
>  );
>  
> +# In general, the site admin can enable/disable per-project
> +# configuration of each committag.  Only the 'options' part of the
> +# committag is configurable per-project.

  The exact same problem here - it is not explained what committag
actually is and where it applies.

> +# The site admin can of course add new tags to this hash or override
> +# the 'sub' key if necessary.  But such changes may be fragile; this
> +# is not designed as a full-blown plugin architecture.  The 'sub' must
> +# return a list of strings or string refs.  The strings must contain
> +# plain text and the string refs must contain HTML.  The string refs
> +# will not be processed further.
> +#
> +# For any committag, set the 'override' key to 1 to allow individual
> +# projects to override entries in the 'options' hash for that tag.
> +# For example, to match only commit hashes given in lowercase in one
> +# project, add this to the $GITWEB_CONFIG:
> +#
> +#     $committags{'sha1'}{'override'} = 1;
> +#
> +# And in the project's config:
> +#
> +#     gitweb.committags.sha1.pattern = \\b([0-9a-f]{8,40})\\b
> +#
> +# Some committags have additional options whose interpretation depends
> +# on the implementation of the 'sub' key.  The hyperlink_committag
> +# value appends the first captured group to the 'url' option.
> +our %committags = (
> +	# Link Git-style hashes to this gitweb
> +	'sha1' => {
> +		'options' => {
> +			'pattern' => qr/\b([0-9a-fA-F]{8,40})\b/,
> +		},
> +		'override' => 0,
> +		'sub' => sub {
> +			my ($opts, @match) = @_;
> +			return \$cgi->a({-href => href(action=>"object", hash=>$match[1]),
> +			                 -class => "text"},
> +			                esc_html($match[0], -nbsp=>1));
> +		},
> +	},

Ideally, a link should be made only in case the object exists, but this
is not trivial to implement without overhead of 1 exec per object, so I
guess it's fine to leave this for later (after all this feature was
already present). In that case, I think it would be useful to start
matching ids from 5 characters up - I use these quite frequently ;) -
but until then it would probably make for too much false positives.

> @@ -417,6 +508,21 @@ our %feature = (
>  		'sub' => \&feature_avatar,
>  		'override' => 0,
>  		'default' => ['']},
> +
> +	# The selection and ordering of committags that are enabled.
> +	# Committag transformations will be applied to commit log messages
> +	# in the order listed here if listed here.

You should add something like "See %committags definition above for
explanation of committags and pre-defined committag classes."

> +	# To disable system wide have in $GITWEB_CONFIG
> +	# $feature{'committags'}{'default'} = [];
> +	# To have project specific config enable override in $GITWEB_CONFIG
> +	# $feature{'committags'}{'override'} = 1;
> +	# and in project config gitweb.committags = sha1, url, bugzilla
> +	# to enable those three committags for that project
> +	'committags' => {
> +		'sub' => sub { feature_list('committags', @_) },
> +		'override' => 0,
> +		'default' => ['sha1']},
>  );

Would people consider it harmful to add 'url' to the default set?

> @@ -463,16 +569,16 @@ sub feature_bool {
>  	}
>  }
>  
> -sub feature_snapshot {
> -	my (@fmts) = @_;
> +sub feature_list {
> +	my ($key, @defaults) = @_;
>  
> -	my ($val) = git_get_project_config('snapshot');
> +	my ($cfg) = git_get_project_config($key);
>  
> -	if ($val) {
> -		@fmts = ($val eq 'none' ? () : split /\s*[,\s]\s*/, $val);
> +	if ($cfg) {
> +		return ($cfg eq 'none' ? () : split(/\s*[,\s]\s*/, $cfg));
>  	}
>  
> -	return @fmts;
> +	return @defaults;
>  }
>  
>  sub feature_patches {
> @@ -886,6 +992,35 @@ if ($git_avatar eq 'gravatar') {
>  	$git_avatar = '';
>  }
>  
> +# ordering of committags
> +our @committags = gitweb_get_feature('committags');
> +
> +# whether we've loaded committags for the project yet
> +our $loaded_project_committags = 0;
> +
> +# Load committag configs from the repository config file and and
> +# incorporate them into the gitweb defaults where permitted by the
> +# site administrator.
> +sub gitweb_load_project_committags {
> +	return if (!$git_dir || $loaded_project_committags);

When can it happen that this is called and !$git_dir? In case it could
ever happen, why not allow the configuration at least in global gitweb
file?

> +	my %project_config = ();
> +	my %raw_config = git_parse_project_config('gitweb\.committag');
> +	foreach my $key (keys(%raw_config)) {
> +		next if ($key !~ /gitweb\.committag\.[^.]+\.[^.]/);
> +		my ($gitweb_prefix, $committag_prefix, $ctname, $option) =
> +			split(/\./, $key, 4);
> +		$project_config{$ctname}{$option} = $raw_config{$key};
> +	}
> +	foreach my $ctname (keys(%committags)) {
> +		next if (!$committags{$ctname}{'override'});
> +		foreach my $optname (keys %{$project_config{$ctname}}) {
> +			$committags{$ctname}{'options'}{$optname} =
> +				$project_config{$ctname}{$optname};
> +		}
> +	}
> +	$loaded_project_committags = 1;
> +}

For the next-conditions, I'd prefer unless formulation, but I guess
that's purely a matter of taste.

> +sub push_or_append (\@@) {
> +	my $fragments = shift;
> +
> +	if (ref $_[0] || ! @$fragments || ref $fragments->[-1]) {
> +		push @$fragments, @_;
> +	} else {
> +		my $a = pop @$fragments;
> +		my $b = shift @_;
> +
> +		push @$fragments, $a . $b, @_;
> +	}
> +	# imitate push
> +	return scalar @$fragments;

This looks *quite* cryptic, a comment would be rather helpful.

-- 
				Petr "Pasky" Baudis
A lot of people have my books on their bookshelves.
That's the problem, they need to read them. -- Don Knuth

^ permalink raw reply

* Re: What's cooking in git.git (Nov 2009, #04; Tue, 17)
From: Tay Ray Chuan @ 2009-11-18  8:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7hto46ce.fsf@alter.siamese.dyndns.org>

Hi,

On Wed, Nov 18, 2009 at 3:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
> * sp/smart-http (2009-11-14) 37 commits
>  [snip]
>  + Git-aware CGI to provide dumb HTTP transport

hmm, it appears that this commit (92815b3) has an incorrect subject;
it should instead read "http-backend: Fix symbol clash on AIX 5.3"
(which is the first line in the commit message body).

It's from the email

  From: "Shawn O. Pearce" <spearce@spearce.org>
  To: git@vger.kernel.org
  Subject: [PATCH 1/3] http-backend: Fix symbol clash on AIX 5.3
  Date: Mon,  9 Nov 2009 10:10:35 -0800
  Message-Id: <1257790237-30850-1-git-send-email-spearce@spearce.org>

-- 
Cheers,
Ray Chuan

^ permalink raw reply

* Re: [RFC PATCH 1/6] gitweb: Hyperlink committags in a commit message by regex matching
From: Petr Baudis @ 2009-11-18  8:26 UTC (permalink / raw)
  To: Marcel M. Cary; +Cc: Jakub Narebski, git, Giuseppe Bilotta, Francis Galiegue
In-Reply-To: <1258525350-5528-2-git-send-email-marcel@oak.homeunix.org>

On Tue, Nov 17, 2009 at 10:22:25PM -0800, Marcel M. Cary wrote:
> +			# Avoid matching punctuation that might immediately follow
> +			# a url, is not part of the url, and is allowed in urls,
> +			# like a full-stop ('.').
> +			'pattern' => qr!(https?|ftps?|git|ssh|ssh+git|sftp|smb|webdavs?|
> +			                 nfs|irc|nntp|rsync)
> +			                ://[-_a-zA-Z0-9\@/&=+~#<>;%:.?]+
> +			                   [-_a-zA-Z0-9\@/&=+~#<>]!x,

You meant ssh\+git here. ;-)

				Petr "Pasky" Baudis

^ permalink raw reply

* [ANNOUNCE] codeBeamer MR - Easy ACL for Git
From: Intland Software @ 2009-11-18  8:33 UTC (permalink / raw)
  To: git

codeBeamer MR is a new tool for teams working with Git, and striving for
fast repository management and easy access control management.
Setting up and maintaining ACL on top of Git is not trivial. With this
web based tool, you can *save a lot of boring and error-prone maintainenace
work*, and spend your time rather with coding.

FEATURES
* Starts and close projects and repositories in just seconds
* Project-, group-, and role-based administration across multiple repositories
* ACL on directory level
* SSH authentication
* Lightweight wiki, issue tracking and project management facilities
* Web based
* Remote API

PRICE?
It is *free* (as in beer). No expiration.

LEARN MORE & DOWNLOAD
More details, screenshots and downloads:
http://www.intland.com/products/cb-mr/overview.html

^ permalink raw reply

* Re: What's cooking in git.git (Nov 2009, #04; Tue, 17)
From: Johannes Sixt @ 2009-11-18  8:40 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7hto46ce.fsf@alter.siamese.dyndns.org>

Junio C Hamano schrieb:
> * rj/cygwin-msvc (2009-11-09) 3 commits.
>  - Add explicit Cygwin check to guard WIN32 header inclusion
>  - MSVC: Add support for building with NO_MMAP
>  - Makefile: keep MSVC and Cygwin configuration separate
>  (this branch uses rj/maint-simplify-cygwin-makefile.)
> 
> I think J6t was not happy with the tip one.

Ramsay suggested to drop the one at the tip, and I agree. The other two
patches are good.

-- Hannes

^ permalink raw reply

* [PATCH v4] Expand ~ and ~user in core.excludesfile, commit.template
From: Matthieu Moy @ 2009-11-18  8:58 UTC (permalink / raw)
  To: git, gitster; +Cc: Matthieu Moy, Karl Chen
In-Reply-To: <1258529351-4045-1-git-send-email-Matthieu.Moy@imag.fr>

These config variables are parsed to substitute ~ and ~user with getpw
entries.

user_path() refactored into new function expand_user_path(), to allow
dynamically allocating the return buffer.

Original patch by Karl Chen, modified by Matthieu Moy, and further
amended by Junio C Hamano.

Signed-off-by: Karl Chen <quarl@quarl.org>
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
"Oops, I did it again :-("

Just a style issue since v3 (char * home -> char *home), sorry for the noise.

 Documentation/config.txt |    7 +++-
 builtin-commit.c         |    2 +-
 cache.h                  |    2 +
 config.c                 |   12 ++++++-
 path.c                   |   83 +++++++++++++++++++++++++++------------------
 5 files changed, 69 insertions(+), 37 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 78ee906..b4fe843 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -380,8 +380,9 @@ Common unit suffixes of 'k', 'm', or 'g' are supported.
 core.excludesfile::
 	In addition to '.gitignore' (per-directory) and
 	'.git/info/exclude', git looks into this file for patterns
-	of files which are not meant to be tracked.  See
-	linkgit:gitignore[5].
+	of files which are not meant to be tracked.  "~/" is expanded
+	to the value of `$HOME` and "~user/" to the specified user's
+	home directory.  See linkgit:gitignore[5].
 
 core.editor::
 	Commands such as `commit` and `tag` that lets you edit
@@ -681,6 +682,8 @@ color.ui::
 
 commit.template::
 	Specify a file to use as the template for new commit messages.
+	"~/" is expanded to the value of `$HOME` and "~user/" to the
+	specified user's home directory.
 
 diff.autorefreshindex::
 	When using 'git-diff' to compare with work tree
diff --git a/builtin-commit.c b/builtin-commit.c
index 0fd7af6..87d1b90 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -1175,7 +1175,7 @@ static int git_commit_config(const char *k, const char *v, void *cb)
 	struct wt_status *s = cb;
 
 	if (!strcmp(k, "commit.template"))
-		return git_config_string(&template_file, k, v);
+		return git_config_pathname(&template_file, k, v);
 
 	return git_status_config(k, v, s);
 }
diff --git a/cache.h b/cache.h
index f7533ff..2ba9690 100644
--- a/cache.h
+++ b/cache.h
@@ -649,6 +649,7 @@ int set_shared_perm(const char *path, int mode);
 #define adjust_shared_perm(path) set_shared_perm((path), 0)
 int safe_create_leading_directories(char *path);
 int safe_create_leading_directories_const(const char *path);
+extern char *expand_user_path(const char *path);
 char *enter_repo(char *path, int strict);
 static inline int is_absolute_path(const char *path)
 {
@@ -909,6 +910,7 @@ extern unsigned long git_config_ulong(const char *, const char *);
 extern int git_config_bool_or_int(const char *, const char *, int *);
 extern int git_config_bool(const char *, const char *);
 extern int git_config_string(const char **, const char *, const char *);
+extern int git_config_pathname(const char **, const char *, const char *);
 extern int git_config_set(const char *, const char *);
 extern int git_config_set_multivar(const char *, const char *, const char *, int);
 extern int git_config_rename_section(const char *, const char *);
diff --git a/config.c b/config.c
index 51f2208..37385ce 100644
--- a/config.c
+++ b/config.c
@@ -351,6 +351,16 @@ int git_config_string(const char **dest, const char *var, const char *value)
 	return 0;
 }
 
+int git_config_pathname(const char **dest, const char *var, const char *value)
+{
+	if (!value)
+		return config_error_nonbool(var);
+	*dest = expand_user_path(value);
+	if (!*dest)
+		die("Failed to expand user dir in: '%s'", value);
+	return 0;
+}
+
 static int git_default_core_config(const char *var, const char *value)
 {
 	/* This needs a better name */
@@ -479,7 +489,7 @@ static int git_default_core_config(const char *var, const char *value)
 		return git_config_string(&editor_program, var, value);
 
 	if (!strcmp(var, "core.excludesfile"))
-		return git_config_string(&excludes_file, var, value);
+		return git_config_pathname(&excludes_file, var, value);
 
 	if (!strcmp(var, "core.whitespace")) {
 		if (!value)
diff --git a/path.c b/path.c
index c7679be..2ec950b 100644
--- a/path.c
+++ b/path.c
@@ -11,6 +11,7 @@
  * which is what it's designed for.
  */
 #include "cache.h"
+#include "strbuf.h"
 
 static char bad_path[] = "/bad-path/";
 
@@ -207,43 +208,49 @@ int validate_headref(const char *path)
 	return -1;
 }
 
-static char *user_path(char *buf, char *path, int sz)
+static struct passwd *getpw_str(const char *username, size_t len)
 {
 	struct passwd *pw;
-	char *slash;
-	int len, baselen;
+	char *username_z = xmalloc(len + 1);
+	memcpy(username_z, username, len);
+	username_z[len] = '\0';
+	pw = getpwnam(username_z);
+	free(username_z);
+	return pw;
+}
 
-	if (!path || path[0] != '~')
-		return NULL;
-	path++;
-	slash = strchr(path, '/');
-	if (path[0] == '/' || !path[0]) {
-		pw = getpwuid(getuid());
-	}
-	else {
-		if (slash) {
-			*slash = 0;
-			pw = getpwnam(path);
-			*slash = '/';
+/*
+ * Return a string with ~ and ~user expanded via getpw*.  If buf != NULL,
+ * then it is a newly allocated string. Returns NULL on getpw failure or
+ * if path is NULL.
+ */
+char *expand_user_path(const char *path)
+{
+	struct strbuf user_path = STRBUF_INIT;
+	const char *first_slash = strchrnul(path, '/');
+	const char *to_copy = path;
+
+	if (path == NULL)
+		goto return_null;
+	if (path[0] == '~') {
+		const char *username = path + 1;
+		size_t username_len = first_slash - username;
+		if (username_len == 0) {
+			const char *home = getenv("HOME");
+			strbuf_add(&user_path, home, strlen(home));
+		} else {
+			struct passwd *pw = getpw_str(username, username_len);
+			if (!pw)
+				goto return_null;
+			strbuf_add(&user_path, pw->pw_dir, strlen(pw->pw_dir));
 		}
-		else
-			pw = getpwnam(path);
+		to_copy = first_slash;
 	}
-	if (!pw || !pw->pw_dir || sz <= strlen(pw->pw_dir))
-		return NULL;
-	baselen = strlen(pw->pw_dir);
-	memcpy(buf, pw->pw_dir, baselen);
-	while ((1 < baselen) && (buf[baselen-1] == '/')) {
-		buf[baselen-1] = 0;
-		baselen--;
-	}
-	if (slash && slash[1]) {
-		len = strlen(slash);
-		if (sz <= baselen + len)
-			return NULL;
-		memcpy(buf + baselen, slash, len + 1);
-	}
-	return buf;
+	strbuf_add(&user_path, to_copy, strlen(to_copy));
+	return strbuf_detach(&user_path, NULL);
+return_null:
+	strbuf_release(&user_path);
+	return NULL;
 }
 
 /*
@@ -291,8 +298,18 @@ char *enter_repo(char *path, int strict)
 		if (PATH_MAX <= len)
 			return NULL;
 		if (path[0] == '~') {
-			if (!user_path(used_path, path, PATH_MAX))
+			char *newpath = expand_user_path(path);
+			if (!newpath || (PATH_MAX - 10 < strlen(newpath))) {
+				free(newpath);
 				return NULL;
+			}
+			/*
+			 * Copy back into the static buffer. A pity
+			 * since newpath was not bounded, but other
+			 * branches of the if are limited by PATH_MAX
+			 * anyway.
+			 */
+			strcpy(used_path, newpath); free(newpath);
 			strcpy(validated_path, path);
 			path = used_path;
 		}
-- 
1.6.5.2.152.gbbe9e

^ permalink raw reply related

* Re: git-svn of both trunk and tags while having no access to the 'parent' of those
From: Michael J Gruber @ 2009-11-18  9:01 UTC (permalink / raw)
  To: Yaroslav Halchenko; +Cc: git
In-Reply-To: <20091117025945.GE17964@onerussian.com>

Yaroslav Halchenko venit, vidit, dixit 17.11.2009 03:59:
> Dear Git People,
> 
> I've ran into a situation here:
> 
> there is a repository with trunk and releases (analog to tags there)
> available for public, but the rest of directories and their parent is
> not available without authentication... ie I can access
> 
> http://domain.com/svnrepo/trunk
> http://domain.com/svnrepo/releases
> but not
> http://domain.com/svnrepo/
> 
> Whenever I use git-svn (1.6.5 in Debian):
> 
> git svn clone --prefix=upstream-svn/ -T trunk -t releases http://domain.com/svnrepo svnrepo.gitsvn
> 
> it asks for authentication... I guess, now I can only clone trunk and
> releases separately? or may be there is some way to avoid the
> problem, ie avoid looking 'into root'?
> 
> Thanks in advance!

Your problem description seems to match perfectly with the description
of the "--no-minimize-url" option in git svn's man page. I'm sure it's
worth a try.

Michael

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox