Git Rev News: Edition 23 (January 25th, 2017)

Welcome to the 23rd edition of Git Rev News, a digest of all things Git. For our goals, the archives, the way we work, and how to contribute or to subscribe, see the Git Rev News page on git.github.io.

This edition covers what happened during the month of December 2016.

Discussions

General

Javantea reported on the list:

I have found a shell injection vulnerability in contrib/gitview/gitview.

and:

Gitview executes shell commands using string concatenation with user supplied data, filenames and branch names. Running Gitview and interacting with the user interface with a malicious filename or branch name in the current repository results in malicious commands being executed as the current user.

gitview is a GTK based repository browser for git, according to its documentation. It is part of the ‘contrib’ directory of the Git codebase which contains scripts and utilities that are not maintained by Junio Hamano and the developers on the Git mailing list. It looked like its implementation in Python was indeed lacking.

Stefan Beller, while cc’ing Aneesh Kumar, the gitview author, replied:

Maybe it’s time for a spring cleanup and remove some old (dead?) projects from contrib?

Jeff King, alias Peff, agreed with Stefan saying that gitview “hasn’t had a substantive commit since 2007”, so Stefan sent a patch that removes gitview from the Git codebase.

Javantea, Peff and Junio all agreed that it was a good solution, but Peff and Junio wanted to hear first from Aneesh before removing his work. Aneesh sent his “Acked-by:” to agree with the change.

Following these events, Stefan sent a separate patch to remove git-convert-objects from the ‘contrib’ directory. This other tool “originally named git-convert-cache, was used in early 2005 to convert to a new repository format, e.g. adding an author date”.

Philip Oakley also recently sent a small patch series to update the git-gui and gitk documentation as this documentation was referencing gitview and needed a few other improvements.

So it looks like a spring cleanup is indeed happening.

Reviews

Eduardo Habkost sent a short patch to add an “am.signoff” configuration option to git am. This option would automatically add a “Signed-off-by: author name author_email@address” line to the end of the commit message, when git am creates a commit.

It looked like a very straightforward thing to do as the --signoff command line option already does that, and many other command line options like --message-id and --3way have configuration option to automatically enable their action.

Stefan Beller agreed that it was a good idea, but asked for documentation and a test, which Eduardo agreed to provide.

Eduardo indeed sent a version 2 of the patch with the requested improvements, and this version was then reviewed by Stefan Beller, Andreas Schwab and Pranit Bauva.

Eduardo followed up with a version 3 of the patch.

But in the mean time Eric Wong replied to the original patch:

I’m not sure this is a good idea. IANAL, but a sign-off has some sort of legal meaning for this project (DCO) and that would be better decided on a patch-by-patch basis rather than a blanket statement.

By DCO, Eric refers to the Developer Certificate of Origin which is the reason why people are adding these “Signed-off-by:” lines, also called “SoB”. The DCO practice originates from the Linux kernel development and is now used by many other projects.

Junio Hamano, the Git maintainer, agreed with Eric’s reply, saying he has been striving to avoid “adding more publicized ways to add SoB without thinking” as it could be a legal risk for projects.

To the above Eduardo replied:

This sounds completely reasonable to me. I now see that the config option was already proposed in 2011 and the same arguments were discussed. Sorry for the noise.

But Stefan replied to Junio saying that there is already the “format.signOff” configuration to automatically add a SoB when using git format-patch and that “thinking and typing things is orthogonal (the more you type, doesn’t imply that you think harder or even at all)”.

Junio though replied that “format.signOff” was a mistake that should be corrected in the future.

Stefan then came up with a many steps plan to remove “format.signOff”, while saying he is not fully convinced it is bad, as he finds “format.signoff” very useful.

As there has been no further discussion on this, and removing “format.signOff” does not look simple nor user friendly, the current situation, which is a bit awkward, may last for a long time.

Support

Mike Hommey asked whether git rebase should be enhanced to support rebasing multiple branches at once, or if it should go in a separate tool.

Junio Hamano mentioned a workaround people are using (for the specific example Mike described in his original post), while explaining its limitations.

What people seem to do is to teach [one] branch […] that its upstream is the [other] local branch […], so that they can be lazy when rebasing a branch that knows its upstream.

Jeff King mentioned his own personal meta/rebase tool, using a topological sort to order rebases, which solves Mike’s problem. Johannes Sixt reminded us about the git-garden-shears tool that Johannes Schindelin posted to git mailing list in Git garden shears, was Re: [PATCH 13/22] sequencer: remember the onelines when parsing the todo file.

A. Wilcox (awilfox) wrote that an attempt to package Git for a new Linux distribution (Adélie Linux) with their PowerPC builder had run into a failure while running the test suite. After a bit of back and forth to clarify where this test failure occurs, and what it was caused by, it turned out that the problem was caused by the use of the musl library for providing the regex functionality. Specifically, the test suite was expecting regcomp() to complain about \x{2b} while trying to check that git grep -P -E overrides the -P PCRE option with -E, which should not accept the \x regex extension. Unlike the standard POSIX libc, musl has implemented \x the same way as PCRE, which unexpectedly causes git grep -E to understand the regex, and thus the test to fail.

POSIX simply declares the \x regex construct as “undefined”, which leaves implementations such as musl and PCRE free to extend the standard if they choose; as a consequence, one should not rely on whatever assumption in this direction. Jeff King, alias Peff, found that [\d] strictly matches a literal \ or d in POSIX ERE, but behaves like [0-9] in PCRE. POSIX implementations are not free to change this behaviour, so we should be able to rely on it. Peff finally fixed the test suite in [PATCH] t7810: avoid assumption about invalid regex syntax accordingly.

Anyway, recent versions of Git won’t build with musl’s regex at all, because it doesn’t support the non-standard REG_STARTEND which Git relies on since b7d36ffca (regex: use regexec_buf(), 2016-09-21). In this case, the build would fall back to regex code included in Git (NO_REGEX). We hope that more regex support libraries, including musl, will implement this extension one day…

Releases

Other News

Events

Light reading

Git tools and sites

Credits

This edition of Git Rev News was curated by Christian Couder <christian.couder@gmail.com>, Thomas Ferris Nicolaisen <tfnico@gmail.com>, Jakub Narębski <jnareb@gmail.com>, and Markus Jansen <mja@jansen-preisler.de>, with help from Lars Schneider <larsxschneider@gmail.com> and Johannes Schindelin <johannes.schindelin@gmx.de>.