* Re: RFC: "git config -l" should not expose sensitive information
From: Jeff King @ 2012-12-20 15:04 UTC (permalink / raw)
To: Toralf Förster; +Cc: git
In-Reply-To: <50CF039A.7010800@gmx.de>
On Mon, Dec 17, 2012 at 12:35:54PM +0100, Toralf Förster wrote:
> often the output is requested in help forums - and a
> "git config -l | wgetpaste" exposes parameters like sendmail.smtppass -
> so hide those variables in the output (if not explicitly wanted) would
> makes sense, or ?
But if we change "git config -l", won't that break all of the
non-exposing uses that rely on seeing all of the variables (e.g., it is
perfectly fine for a porcelain to parse "git config -l" rather than
making several calls to "git config"; IIRC, git-cola does this).
The problem seems to be that people are giving bad advice to tell people
to post "git config -l" output without looking at. Maybe we could help
them with a "git config --share-config" option that dumps all config,
but sanitizes the output. It would need to have a list of sensitive keys
(which does not exist yet), and would need to not just mark up things
like smtppass, but would also need to pull credential information out of
remote.*.url strings. And maybe more (I haven't thought too long on it).
-Peff
^ permalink raw reply
* Re: [PATCH] Python scripts audited for minimum compatible version and checks added.
From: Eric S. Raymond @ 2012-12-20 15:02 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20121220144813.GA27211@sigill.intra.peff.net>
Jeff King <peff@peff.net>:
> On Thu, Dec 20, 2012 at 09:13:37AM -0500, Eric S. Raymond wrote:
>
> > diff --git a/contrib/ciabot/ciabot.py b/contrib/ciabot/ciabot.py
> > index bd24395..b55648f 100755
> > --- a/contrib/ciabot/ciabot.py
> > +++ b/contrib/ciabot/ciabot.py
> > @@ -50,6 +50,11 @@
> > import os, sys, commands, socket, urllib
> > from xml.sax.saxutils import escape
> >
> > +if sys.hexversion < 0x02000000:
> > + # The limiter is the xml.sax module
> > + sys.stderr.write("import-zips.py: requires Python 2.0.0 or later.")
> > + sys.exit(1)
>
> Should the error message say ciabot.py?
>
> -Peff
Gack. Yes. Thaty's what I get for cut-and-pasting too quickly.
The information about xnml.sex is correct, though.
Want me to resubmit, or will you just patch it?
Note by the way that I still think the entire ciabot subtree (which is
my code) should just be nuked. CIA is not coming back, wishful thinking
on Ilkotech's web page notwithstanding.
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2012, #05; Tue, 18)
From: Jeff King @ 2012-12-20 14:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v4njjf6fk.fsf@alter.siamese.dyndns.org>
On Tue, Dec 18, 2012 at 12:31:43PM -0800, Junio C Hamano wrote:
> * jk/error-const-return (2012-12-15) 2 commits
> - silence some -Wuninitialized false positives
> - make error()'s constant return value more visible
>
> Help compilers' flow analysis by making it more explicit that
> error() always returns -1, to reduce false "variable used
> uninitialized" warnings.
>
> This is still an RFC.
What's your opinion on this? The last round I posted (and what you have
in pu) is about as good as this technique is going to get. So it is
really a taste thing. On the one hand, I really like that it is not just
papering over the problem, but rather giving the compiler more data to
do its analysis. On the other hand, it makes the source kind of ugly.
I don't think there's a rush on it, but I'm just sifting through my
half-done topics.
-Peff
^ permalink raw reply
* Re: $PATH pollution and t9902-completion.sh
From: Jeff King @ 2012-12-20 14:55 UTC (permalink / raw)
To: Adam Spiers; +Cc: git mailing list
In-Reply-To: <20121217010538.GC3673@gmail.com>
On Mon, Dec 17, 2012 at 01:05:38AM +0000, Adam Spiers wrote:
> t/t9902-completion.sh is currently failing for me because I happen to
> have a custom shell-script called git-check-email in ~/bin, which is
> on my $PATH. This is different to a similar-looking case reported
> recently, which was due to an unclean working tree:
>
> http://thread.gmane.org/gmane.comp.version-control.git/208085
>
> It's not unthinkable that in the future other tests could break for
> similar reasons. Therefore it would be good to sanitize $PATH in the
> test framework so that it cannot destabilize tests, although I am
> struggling to think of a good way of doing this. Naively stripping
> directories under $HOME would not protect against git "plugins" such
> as the above being installed into places like /usr/bin. Thoughts?
I've run into this, too. I think sanitizing $PATH is the wrong approach.
The real problem is that the test is overly picky. Right now it is
failing because you happen to have "check-email" in your $PATH, but it
will also need to be adjusted when a true "check-email" command is added
to git.
I can think of two other options:
1. Make the test input more specific (e.g., looking for "checkou").
This doesn't eliminate the problem, but makes it less likely
to occur.
2. Loosen the test to look for the presence of "checkout", but not
fail when other items are present. Bonus points if it makes sure
that everything returned starts with "check".
I think (2) is the ideal solution in terms of behavior, but writing it
may be more of a pain.
-Peff
^ permalink raw reply
* Re: [PATCH] Python scripts audited for minimum compatible version and checks added.
From: Jeff King @ 2012-12-20 14:48 UTC (permalink / raw)
To: Eric S. Raymond; +Cc: git
In-Reply-To: <20121220141855.05DAA44105@snark.thyrsus.com>
On Thu, Dec 20, 2012 at 09:13:37AM -0500, Eric S. Raymond wrote:
> diff --git a/contrib/ciabot/ciabot.py b/contrib/ciabot/ciabot.py
> index bd24395..b55648f 100755
> --- a/contrib/ciabot/ciabot.py
> +++ b/contrib/ciabot/ciabot.py
> @@ -50,6 +50,11 @@
> import os, sys, commands, socket, urllib
> from xml.sax.saxutils import escape
>
> +if sys.hexversion < 0x02000000:
> + # The limiter is the xml.sax module
> + sys.stderr.write("import-zips.py: requires Python 2.0.0 or later.")
> + sys.exit(1)
Should the error message say ciabot.py?
-Peff
^ permalink raw reply
* Python version auditing followup
From: Eric S. Raymond @ 2012-12-20 14:34 UTC (permalink / raw)
To: git
Most of the Python scripts in the distribution are small and simple to
audit, so I am pretty sure of the results. The only place where I
have a concern is the git_helpers library; that is somewhat more
complex and I might have missed a dependency somewhere. Whoever
owns that should check my finding that it should run under 2.4
That was the first of three patches I have promised. In order to do
the next one, which will be a development guidelines recommend
compatibility back to some specific version X, I need a policy
decision. How do we set X?
I don't think X can be < 2.4, nor does it need to be - 2.4 came out
in 2004 and eight years is plenty of deployment time.
The later we set it, the more convenient for developers. But of
course by setting it late we trade away some portability to
older systems.
In previous discussion of this issue I recommended X = 2.6.
That is still my recommendation. Thoughts, comments, objections?
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
In recent years it has been suggested that the Second Amendment
protects the "collective" right of states to maintain militias, while
it does not protect the right of "the people" to keep and bear arms.
If anyone entertained this notion in the period during which the
Constitution and the Bill of Rights were debated and ratified, it
remains one of the most closely guarded secrets of the eighteenth
century, for no known writing surviving from the period between 1787
and 1791 states such a thesis.
-- Stephen P. Halbrook, "That Every Man Be Armed", 1984
^ permalink raw reply
* [PATCH] Python scripts audited for minimum compatible version and checks added.
From: Eric S. Raymond @ 2012-12-20 14:13 UTC (permalink / raw)
To: git
Signed-off-by: Eric S. Raymond <esr@thyrsus.com>
---
contrib/ciabot/ciabot.py | 5 +++++
contrib/fast-import/import-zips.py | 5 +++++
contrib/hg-to-git/hg-to-git.py | 5 +++++
contrib/p4import/git-p4import.py | 5 +++++
contrib/svn-fe/svnrdump_sim.py | 4 ++++
git-p4.py | 5 +++++
git-remote-testgit.py | 5 +++++
git_remote_helpers/git/__init__.py | 4 ++++
8 files changed, 38 insertions(+)
diff --git a/contrib/ciabot/ciabot.py b/contrib/ciabot/ciabot.py
index bd24395..b55648f 100755
--- a/contrib/ciabot/ciabot.py
+++ b/contrib/ciabot/ciabot.py
@@ -50,6 +50,11 @@
import os, sys, commands, socket, urllib
from xml.sax.saxutils import escape
+if sys.hexversion < 0x02000000:
+ # The limiter is the xml.sax module
+ sys.stderr.write("import-zips.py: requires Python 2.0.0 or later.")
+ sys.exit(1)
+
# Changeset URL prefix for your repo: when the commit ID is appended
# to this, it should point at a CGI that will display the commit
# through gitweb or something similar. The defaults will probably
diff --git a/contrib/fast-import/import-zips.py b/contrib/fast-import/import-zips.py
index 82f5ed3..d9ad71d 100755
--- a/contrib/fast-import/import-zips.py
+++ b/contrib/fast-import/import-zips.py
@@ -13,6 +13,11 @@ from sys import argv, exit
from time import mktime
from zipfile import ZipFile
+if sys.hexversion < 0x01060000:
+ # The limiter is the zipfile module
+ sys.stderr.write("import-zips.py: requires Python 1.6.0 or later.")
+ sys.exit(1)
+
if len(argv) < 2:
print 'Usage:', argv[0], '<zipfile>...'
exit(1)
diff --git a/contrib/hg-to-git/hg-to-git.py b/contrib/hg-to-git/hg-to-git.py
index 046cb2b..9f39ce5 100755
--- a/contrib/hg-to-git/hg-to-git.py
+++ b/contrib/hg-to-git/hg-to-git.py
@@ -23,6 +23,11 @@ import os, os.path, sys
import tempfile, pickle, getopt
import re
+if sys.hexversion < 0x02030000:
+ # The behavior of the pickle module changed significantly in 2.3
+ sys.stderr.write("hg-to-git.py: requires Python 2.3 or later.")
+ sys.exit(1)
+
# Maps hg version -> git version
hgvers = {}
# List of children for each hg revision
diff --git a/contrib/p4import/git-p4import.py b/contrib/p4import/git-p4import.py
index b6e534b..fb48e2a 100644
--- a/contrib/p4import/git-p4import.py
+++ b/contrib/p4import/git-p4import.py
@@ -14,6 +14,11 @@ import sys
import time
import getopt
+if sys.hexversion < 0x02020000:
+ # The behavior of the marshal module changed significantly in 2.2
+ sys.stderr.write("git-p4import.py: requires Python 2.2 or later.")
+ sys.exit(1)
+
from signal import signal, \
SIGPIPE, SIGINT, SIG_DFL, \
default_int_handler
diff --git a/contrib/svn-fe/svnrdump_sim.py b/contrib/svn-fe/svnrdump_sim.py
index 1cfac4a..ed43dbb 100755
--- a/contrib/svn-fe/svnrdump_sim.py
+++ b/contrib/svn-fe/svnrdump_sim.py
@@ -7,6 +7,10 @@ to the highest revision that should be available.
"""
import sys, os
+if sys.hexversion < 0x02040000:
+ # The limiter is the ValueError() calls. This may be too conservative
+ sys.stderr.write("svnrdump-sim.py: requires Python 2.4 or later.")
+ sys.exit(1)
def getrevlimit():
var = 'SVNRMAX'
diff --git a/git-p4.py b/git-p4.py
index 551aec9..ec060b4 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -12,6 +12,11 @@ import optparse, sys, os, marshal, subprocess, shelve
import tempfile, getopt, os.path, time, platform
import re, shutil
+if sys.hexversion < 0x02040000:
+ # The limiter is the subprocess module
+ sys.stderr.write("git-p4.py: requires Python 2.4 or later.")
+ sys.exit(1)
+
verbose = False
# Only labels/tags matching this will be imported/exported
diff --git a/git-remote-testgit.py b/git-remote-testgit.py
index 5f3ebd2..22d2eb6 100644
--- a/git-remote-testgit.py
+++ b/git-remote-testgit.py
@@ -31,6 +31,11 @@ from git_remote_helpers.git.exporter import GitExporter
from git_remote_helpers.git.importer import GitImporter
from git_remote_helpers.git.non_local import NonLocalGit
+if sys.hexversion < 0x01050200:
+ # os.makedirs() is the limiter
+ sys.stderr.write("git-remote-testgit.py: requires Python 1.5.2 or later.")
+ sys.exit(1)
+
def get_repo(alias, url):
"""Returns a git repository object initialized for usage.
"""
diff --git a/git_remote_helpers/git/__init__.py b/git_remote_helpers/git/__init__.py
index e69de29..776e891 100644
--- a/git_remote_helpers/git/__init__.py
+++ b/git_remote_helpers/git/__init__.py
@@ -0,0 +1,4 @@
+if sys.hexversion < 0x02040000:
+ # The limiter is the subprocess module
+ sys.stderr.write("git_remote_helpers: requires Python 2.4 or later.")
+ sys.exit(1)
--
1.8.1.rc2
--
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
"The state calls its own violence `law', but that of the individual `crime'"
-- Max Stirner
^ permalink raw reply related
* Gitweb running as FCGI does not print its output in UTF-8
From: Sergio Talens-Oliag @ 2012-12-20 13:53 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 2931 bytes --]
Hello, I'm sending this message to explain a problem I've found with
gitweb.cgi when running it using a call like the following:
export FCGI_SOCKET_PATH="/run/gitweb.socket"
gitweb.cgi --fcgi --nproc 2
I've fixed the problem for my installation usign a wrapper script, as
explained below.
I'm not sending a patch because I'm no expert in Perl and I'm unsure that
the solution I've found is the right one, but in the hope that my description
of the problem and the provided solution will be useful for others.
Problem description:
When using gitweb.cgi in FCGI mode the answers from the script are returned
using the wrong encoding (UTF-8 characters are printed as LATIN-1).
It seems that the problem appears because the FCGI streams are implemented
using the older stream API, TIEHANDLE and applying PerlIO layers using
binmode() has no effect to them.
Applied solution:
A solution similar to the use of binmode for the output stream is to
redefine the FCGI::Stream::PRINT function to use UTF-8 as output encoding.
To do it with minimal chages I've created a gitweb.cgi wrapper that
redefines the function and runs the original script; I'm doing it like this
to be able to use the packaged script until upstream includes a fix for the
problem.
Wrapper code (uses /usr/share/gitweb/gitweb.cgi as the PATH for gitweb.cgi):
#!/usr/bin/perl
# gitweb.cgi wrapper that fixes the UTF-8 problem with fastcgi
# Local redefinition of FCGI::Stream::PRINT
use Encode;
use FCGI;
our $enc = Encode::find_encoding('UTF-8');
our $org = \&FCGI::Stream::PRINT;
no warnings 'redefine';
local *FCGI::Stream::PRINT = sub {
my @OUTPUT = @_;
for (my $i = 1; $i < @_; $i++) {
$OUTPUT[$i] = $enc->encode($_[$i], Encode::FB_CROAK|Encode::LEAVE_SRC);
}
@_ = @OUTPUT;
goto $org;
};
# Execute original script
do "/usr/share/gitweb/gitweb.cgi";
References:
The applied solution has been found on the following StackOverflow
question:
http://stackoverflow.com/questions/5005104
Environment:
The system I'm using is Debian Wheezy and I'm serving gitweb using nginx's
fastcgi interface.
The versions of the related debian packages are:
Package: git
Version: 1:1.7.10.4-1+wheezy1
Package: libcgi-fast-perl
Version: 5.14.2-16
Package: libfcgi-procmanager-perl
Version: 0.24-1
Package: perl
Version: 5.14.2-16
To launch gitweb as fastcgi I'm using an init.d script that runs the wrapper
script in the background as the `www-user` using a call similar to the
following:
export FCGI_SOCKET_PATH="/run/gitweb/socket"
gitweb.cgi-wrapper --fcgi --nproc 2
Greetings,
Sergio.
--
Sergio Talens-Oliag <sto@iti.es> <http://www.iti.es/>
Key fingerprint = FF77 A16B 9D09 FC7B 6656 CFAD 261D E19A 578A 36F2
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH 2/3] wildmatch: support "no FNM_PATHNAME" mode
From: Nguyen Thai Ngoc Duy @ 2012-12-20 13:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <CACsJy8C9Nhdq_xBAOxtmLcUnrUioAMvWCPk=sBZKOzFY2WR+iA@mail.gmail.com>
On Thu, Dec 20, 2012 at 8:55 AM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
>> As long as simpler patterns fnmatch() groks (namely, '?', '*', and
>> '[class]' wildcards only) are not slowed down by replacing it with
>> wildmatch(), that is, of course.
>
> I'm concerned about performance vs fnmatch too. I'll probably write a
> small program to exercise both and measure it with glibc.
I'll send out proper patches later this weekend, but unless I make big
mistakes it seems wildmatch is faster than fnmatch from glibc 2.14.1
on x86-64 :) Maybe I did make mistakes. The test matches every line in
/tmp/filelist.txt (which is ls-files from linux-2.6.git) 2000 times.
"pathname" means FNM_PATHNAME.
$ export LANG=C
$ ./test-wildmatch-perf /tmp/filelist.txt 'Documentation/*' 2000
wildmatch 1s 528394us
fnmatch 2s 588396us
$ ./test-wildmatch-perf /tmp/filelist.txt 'drivers/*' 2000
wildmatch 1s 957243us
fnmatch 3s 134839us
$ ./test-wildmatch-perf /tmp/filelist.txt 'Documentation/*' 2000 pathname
wildmatch 1s 548575us
fnmatch 2s 629680us
$ ./test-wildmatch-perf /tmp/filelist.txt 'drivers/*' 2000 pathname
wildmatch 2s 142377us
fnmatch 3s 318295us
$ ./test-wildmatch-perf /tmp/filelist.txt '[Dd]ocu[Mn]entation/*' 2000
wildmatch 1s 748505us
fnmatch 3s 224414us
$ ./test-wildmatch-perf /tmp/filelist.txt '[Dd]o?u[Mn]en?ati?n/*' 2000
wildmatch 1s 743268us
fnmatch 3s 278034us
$ ./test-wildmatch-perf /tmp/filelist.txt '[Dd]o?u[Mn]en?ati?n/*' 2000 pathname
wildmatch 1s 745151us
fnmatch 3s 314127us
$ ./test-wildmatch-perf /tmp/filelist.txt nonsense 2000 pathname
wildmatch 1s 310727us
fnmatch 2s 279123us
--
Duy
^ permalink raw reply
* Re: [PATCH 3/3] Convert all fnmatch() calls to wildmatch()
From: Nguyen Thai Ngoc Duy @ 2012-12-20 12:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8v8tanyn.fsf@alter.siamese.dyndns.org>
On Thu, Dec 20, 2012 at 1:36 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> @@ -627,7 +628,7 @@ enum interesting tree_entry_interesting(const struct name_entry *entry,
>> return entry_interesting;
>>
>> if (item->use_wildcard) {
>> - if (!fnmatch(match + baselen, entry->path, 0))
>> + if (!wildmatch(match + baselen, entry->path, 0))
>> return entry_interesting;
>
> This and the change to dir.c obviously have interactions with
> 8c6abbc (pathspec: apply "*.c" optimization from exclude,
> 2012-11-24).
>
> I've already alluded to it in my response to 2/3, I guess.
Conflict of plans. I did not expect myself to work on replacing
fnmatch soon and git_fnmatch is an intermediate step to collect more
optimizations and gradually replace fnmatch. Eventually git_fnmatch()
would become a wrapper of wildmatch, all the optimizations are pushed
down there. If we replace fnmatch->wildmatch first then there's little
reason for the existence of git_fnmatch(). Maybe I should merge this
with the fnmatch elimination into a single series.
--
Duy
^ permalink raw reply
* Re: [PATCH] mergetools/p4merge: Honor $TMPDIR for the /dev/null placeholder
From: Joachim Schmitz @ 2012-12-20 8:20 UTC (permalink / raw)
To: git
In-Reply-To: <7v623x6xvv.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> David Aguilar <davvid@gmail.com> writes:
>
>> Use mktemp to create the /dev/null placeholder for p4merge.
>> This keeps it out of the current directory.
>>
>> Reported-by: Jeremy Morton <admin@game-point.net>
>> Signed-off-by: David Aguilar <davvid@gmail.com>
>> ---
>> I consider this a final finishing touch on a new 1.8.1 feature,
>> so hopefully we can get this in before 1.8.1.
>
> Does everybody have mktemp(1), which is not even in POSIX.1?
HP-NonStop doesn't have it, unless you're on the latest release, which added
GNU coreutils.
Bye, Jojo
^ permalink raw reply
* Re: [PATCH] mergetools/p4merge: Honor $TMPDIR for the /dev/null placeholder
From: David Aguilar @ 2012-12-20 8:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v623x6xvv.fsf@alter.siamese.dyndns.org>
On Wed, Dec 19, 2012 at 10:28 PM, Junio C Hamano <gitster@pobox.com> wrote:
> David Aguilar <davvid@gmail.com> writes:
>
>> Use mktemp to create the /dev/null placeholder for p4merge.
>> This keeps it out of the current directory.
>>
>> Reported-by: Jeremy Morton <admin@game-point.net>
>> Signed-off-by: David Aguilar <davvid@gmail.com>
>> ---
>> I consider this a final finishing touch on a new 1.8.1 feature,
>> so hopefully we can get this in before 1.8.1.
>
> Does everybody have mktemp(1), which is not even in POSIX.1?
>
> I'm a bit hesitant to apply this to the upcoming release without
> cooking it in 'next' for sufficiently long time to give it a chance
> to be tried by wider audience.
True. I only tried Linux and Mac OS X, so I would not be
surprised to find some exotic UNIX that does not have mktemp.
I meant to write "is this portable?" in the section after
the double-dash; saying that it's polish for a fix
is only true if it's portable.
The git-unpack thing looks interesting...
is the SHA-1 in your example the special SHA-1 for an
empty tree or blob?
The reason I ask is because in this code path we are
comparing an unknown blob, basically a blob that does
not exist in one of the trees, so I'm not sure if an
'unpack' command would help for this case because we
would not have a blob SHA-1 to unpack.
As far as portability goes, the "UNIX" list
for p4merge is here:
http://www.perforce.com/downloads/complete_list
I do not have AIX, Solaris, or FreeBSD to test,
so I agree that this can wait.
Does msysgit have mktemp?
p4merge is available on Windows too so I would
need to check there too unless someone happens
to know off the top of their heads.
My other thought was to write a simple shell
function that checks TMPDIR itself, and defaults
to creating some file under /tmp when it is undefined.
I can pursue this option if you think it's a safer choice.
--
David
^ permalink raw reply
* Re: [PATCH] mergetools/p4merge: Honor $TMPDIR for the /dev/null placeholder
From: Junio C Hamano @ 2012-12-20 7:07 UTC (permalink / raw)
To: David Aguilar; +Cc: git
In-Reply-To: <7v623x6xvv.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> David Aguilar <davvid@gmail.com> writes:
>
>> Use mktemp to create the /dev/null placeholder for p4merge.
>> This keeps it out of the current directory.
>>
>> Reported-by: Jeremy Morton <admin@game-point.net>
>> Signed-off-by: David Aguilar <davvid@gmail.com>
>> ---
>> I consider this a final finishing touch on a new 1.8.1 feature,
>> so hopefully we can get this in before 1.8.1.
>
> Does everybody have mktemp(1), which is not even in POSIX.1?
>
> I'm a bit hesitant to apply this to the upcoming release without
> cooking it in 'next' for sufficiently long time to give it a chance
> to be tried by wider audience.
One approach may be to do something like this as a preparation step
(current callers of unpack-file may want to do their temporary work
in TMPDIR as well), and then use
git unpack-file -t e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
to create your empty temporary file.
Documentation/git-unpack-file.txt | 10 +++++++---
builtin/unpack-file.c | 28 +++++++++++++++++++++-------
2 files changed, 28 insertions(+), 10 deletions(-)
diff --git a/Documentation/git-unpack-file.txt b/Documentation/git-unpack-file.txt
index e9f148a..56af328 100644
--- a/Documentation/git-unpack-file.txt
+++ b/Documentation/git-unpack-file.txt
@@ -10,16 +10,20 @@ git-unpack-file - Creates a temporary file with a blob's contents
SYNOPSIS
--------
[verse]
-'git unpack-file' <blob>
+'git unpack-file' [-t] <blob>
DESCRIPTION
-----------
Creates a file holding the contents of the blob specified by sha1. It
-returns the name of the temporary file in the following format:
- .merge_file_XXXXX
+returns the name of the temporary file (by default `.merge_file_XXXXX`).
+
OPTIONS
-------
+-t::
+ The temporary file is created in `$TMPDIR` directory,
+ instead of the current directory.
+
<blob>::
Must be a blob id
diff --git a/builtin/unpack-file.c b/builtin/unpack-file.c
index 1920029..de1f845 100644
--- a/builtin/unpack-file.c
+++ b/builtin/unpack-file.c
@@ -1,8 +1,9 @@
#include "builtin.h"
-static char *create_temp_file(unsigned char *sha1)
+static char *create_temp_file(unsigned char *sha1, int in_tempdir)
{
- static char path[50];
+ static char path[1024];
+ static const char template[] = ".merge_file_XXXXXX";
void *buf;
enum object_type type;
unsigned long size;
@@ -12,8 +13,12 @@ static char *create_temp_file(unsigned char *sha1)
if (!buf || type != OBJ_BLOB)
die("unable to read blob object %s", sha1_to_hex(sha1));
- strcpy(path, ".merge_file_XXXXXX");
- fd = xmkstemp(path);
+ if (in_tempdir) {
+ fd = git_mkstemp(path, sizeof(path) - 1, template);
+ } else {
+ strcpy(path, template);
+ fd = xmkstemp(path);
+ }
if (write_in_full(fd, buf, size) != size)
die_errno("unable to write temp-file");
close(fd);
@@ -23,14 +28,23 @@ static char *create_temp_file(unsigned char *sha1)
int cmd_unpack_file(int argc, const char **argv, const char *prefix)
{
unsigned char sha1[20];
+ int in_tempdir = 0;
+
+ if (argc < 2 || 3 < argc || !strcmp(argv[1], "-h"))
+ usage("git unpack-file [-t] <sha1>");
+ if (argc == 3) {
+ if (strcmp(argv[1], "-t"))
+ usage("git unpack-file [-t] <sha1>");
+ in_tempdir = 1;
+ argc--;
+ argv++;
+ }
- if (argc != 2 || !strcmp(argv[1], "-h"))
- usage("git unpack-file <sha1>");
if (get_sha1(argv[1], sha1))
die("Not a valid object name %s", argv[1]);
git_config(git_default_config, NULL);
- puts(create_temp_file(sha1));
+ puts(create_temp_file(sha1, in_tempdir));
return 0;
}
^ permalink raw reply related
* Re: [PATCH] mergetools/p4merge: Honor $TMPDIR for the /dev/null placeholder
From: Junio C Hamano @ 2012-12-20 6:28 UTC (permalink / raw)
To: David Aguilar; +Cc: git
In-Reply-To: <1355978754-7041-1-git-send-email-davvid@gmail.com>
David Aguilar <davvid@gmail.com> writes:
> Use mktemp to create the /dev/null placeholder for p4merge.
> This keeps it out of the current directory.
>
> Reported-by: Jeremy Morton <admin@game-point.net>
> Signed-off-by: David Aguilar <davvid@gmail.com>
> ---
> I consider this a final finishing touch on a new 1.8.1 feature,
> so hopefully we can get this in before 1.8.1.
Does everybody have mktemp(1), which is not even in POSIX.1?
I'm a bit hesitant to apply this to the upcoming release without
cooking it in 'next' for sufficiently long time to give it a chance
to be tried by wider audience.
> mergetools/p4merge | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/mergetools/p4merge b/mergetools/p4merge
> index 295361a..090fa9b 100644
> --- a/mergetools/p4merge
> +++ b/mergetools/p4merge
> @@ -4,13 +4,13 @@ diff_cmd () {
> rm_remote=
> if test "/dev/null" = "$LOCAL"
> then
> - LOCAL="./p4merge-dev-null.LOCAL.$$"
> + LOCAL="$(create_empty_file)"
> >"$LOCAL"
> rm_local=true
> fi
> if test "/dev/null" = "$REMOTE"
> then
> - REMOTE="./p4merge-dev-null.REMOTE.$$"
> + REMOTE="$(create_empty_file)"
> >"$REMOTE"
> rm_remote=true
> fi
> @@ -33,3 +33,7 @@ merge_cmd () {
> "$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" "$MERGED"
> check_unchanged
> }
> +
> +create_empty_file () {
> + mktemp -t git-difftool-p4merge-empty-file.XXXXXX
> +}
^ permalink raw reply
* Re: [PATCH] add core.pathspecGlob config option
From: Junio C Hamano @ 2012-12-20 5:26 UTC (permalink / raw)
To: Jeff King; +Cc: Nguyen Thai Ngoc Duy, git
In-Reply-To: <20121220035543.GA14965@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Dec 19, 2012 at 07:51:16PM -0800, Junio C Hamano wrote:
>
>> > ++ if (limit_pathspec_to_literal())
>> > ++ item->nowildcard_len = item->len;
>> > ++ else {
>> > ++ item->nowildcard_len = simple_length(path);
>> > ++ if (item->nowildcard_len < item->len) {
>> > ++ pathspec->has_wildcard = 1;
>> > ++ if (path[item->nowildcard_len] == '*' &&
>> > ++ no_wildcard(path + item->nowildcard_len + 1))
>> > ++ item->flags |= PATHSPEC_ONESTAR;
>> > ++ }
>> > + }
>>
>> Hmph. I thought that returning the length without any "stop at glob
>> special" trick from simple_length() would be a simpler resolution.
>>
>> That is what is queued at the tip of 'pu', anyway.
>
> I don't think we can make a change in simple_length. It gets used not
> only for pathspecs, but also for parsing exclude patterns, which I do
> not think should be affected by this option.
Ouch, yeah, you're right.
^ permalink raw reply
* Re: [RFC] test: Old shells and physical paths
From: David Aguilar @ 2012-12-20 5:01 UTC (permalink / raw)
To: David Michael; +Cc: Junio C Hamano, git@vger.kernel.org
In-Reply-To: <CAEvUa7=_iyXxaaRs3WtxZOy5PNnncG-iMAUNkCMLJ19ZtReqaw@mail.gmail.com>
On Wed, Dec 19, 2012 at 6:28 PM, David Michael <fedora.dm0@gmail.com> wrote:
> Hi,
>
> On Thu, Dec 20, 2012 at 12:17 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Is "here is a nickel, get a better shell" an option?
>
> It is, somewhat. There is a pre-built port of GNU bash 2.03 for the
> platform, but I was trying to see how far things could go with the
> OS's supported shell before having to bring in unsupported
> dependencies. Unfortunately, I do not believe the OS fully conforms
> to POSIX.1-2001 yet, so that means no "-P" or "-L" without going
> rogue.
>
> I'll carry test fixes for this platform locally.
Do you know if the differences are relegated to "cd",
or do other common commands such as awk, grep, sed, mktemp, expr,
etc. have similar issues?
I wonder if it'd be helpful to have a low-numbered test that checks
the basics needed by the scripted Porcelains and test suite.
It would give us an easy way to answer these questions, and could
be a good way to document (in code) the capabilities we expect.
--
David
^ permalink raw reply
* [PATCH] mergetools/p4merge: Honor $TMPDIR for the /dev/null placeholder
From: David Aguilar @ 2012-12-20 4:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Use mktemp to create the /dev/null placeholder for p4merge.
This keeps it out of the current directory.
Reported-by: Jeremy Morton <admin@game-point.net>
Signed-off-by: David Aguilar <davvid@gmail.com>
---
I consider this a final finishing touch on a new 1.8.1 feature,
so hopefully we can get this in before 1.8.1.
mergetools/p4merge | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/mergetools/p4merge b/mergetools/p4merge
index 295361a..090fa9b 100644
--- a/mergetools/p4merge
+++ b/mergetools/p4merge
@@ -4,13 +4,13 @@ diff_cmd () {
rm_remote=
if test "/dev/null" = "$LOCAL"
then
- LOCAL="./p4merge-dev-null.LOCAL.$$"
+ LOCAL="$(create_empty_file)"
>"$LOCAL"
rm_local=true
fi
if test "/dev/null" = "$REMOTE"
then
- REMOTE="./p4merge-dev-null.REMOTE.$$"
+ REMOTE="$(create_empty_file)"
>"$REMOTE"
rm_remote=true
fi
@@ -33,3 +33,7 @@ merge_cmd () {
"$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" "$MERGED"
check_unchanged
}
+
+create_empty_file () {
+ mktemp -t git-difftool-p4merge-empty-file.XXXXXX
+}
--
1.8.1.rc2.6.g18499ba
^ permalink raw reply related
* Re: [PATCH] mergetools/p4merge: Handle "/dev/null"
From: David Aguilar @ 2012-12-20 4:41 UTC (permalink / raw)
To: Jeremy Morton; +Cc: Junio C Hamano, git
In-Reply-To: <508B9F89.7050909@game-point.net>
On Sat, Oct 27, 2012 at 1:47 AM, Jeremy Morton <admin@game-point.net> wrote:
> Sorry to be replying to this so late; I hadn't noticed the post until now!
>
> I've tried putting that code in my p4merge script and yes it does indeed
> work fine. However, it puts a temporary file in the working directory which
> I'm not sure is a good idea? If we look at this patch which actually solved
> pretty much the same problem, but when merging and, during a merge conflict,
> a file was created in both branches:
> https://github.com/git/git/commit/ec245ba
>
> ... it is creating a temp file in a proper temp dir, rather than in the
> working dir. I think that would be the proper solution here. However, I
> really want to get this fixed so I'd be happy for this band-aid fix of the
> p4merge script to be checked in until we could get a patch more like the
> aforementioned one, at a later date, to create empty files in a proper temp
> dir and pass them as $LOCAL and $REMOTE. :-)
I had the same thoughts when I wrote it, but I figured that following
the existing pattern used by mergetool for $REMOTE and $LOCAL when
they do exist was simpler as the first step.
I have a patch that fixes this by using mktemp that I will send shortly.
It only does it for the /dev/null file since the existing behavior for
files that do exist is fine.
--
David
^ permalink raw reply
* Re: [PATCH] add core.pathspecGlob config option
From: Jeff King @ 2012-12-20 4:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, git
In-Reply-To: <20121220040602.GA15172@sigill.intra.peff.net>
On Wed, Dec 19, 2012 at 11:06:02PM -0500, Jeff King wrote:
> > I don't think we can make a change in simple_length. It gets used not
> > only for pathspecs, but also for parsing exclude patterns, which I do
> > not think should be affected by this option.
>
> Our test suite wouldn't catch such a misfeature, of course, because the
> feature is not turned on by default. But I found it instructive to run
> all of the tests with GIT_LITERAL_PATHSPECS on. There are failures, of
> course, but by inspecting each failure you can see that it is an
> intended effect of the patch (i.e., each tries to use a wildcard
> pathspec, which no longer works).
>
> When you suggested changing common_prefix, I ran such a test both with
> and without the change[1] and confirmed that it did not change the set
> of failure sites. I did not try it, but I suspect running such a test
> with the tip of pu would reveal new failures in the .gitignore tests.
I just tried it, and indeed, running the test suite with this patch:
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 256f1c6..1c43593 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -102,6 +102,9 @@ export EDITOR
export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
export EDITOR
+GIT_LITERAL_PATHSPECS=1
+export GIT_LITERAL_PATHSPECS
+
# Add libc MALLOC and MALLOC_PERTURB test
# only if we are not executing the test with valgrind
if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null ||
produces many more failures on "pu" than it does on jk/pathspec-literal.
-Peff
^ permalink raw reply related
* Re: [PATCH] add core.pathspecGlob config option
From: Jeff King @ 2012-12-20 4:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, git
In-Reply-To: <20121220035543.GA14965@sigill.intra.peff.net>
On Wed, Dec 19, 2012 at 10:55:43PM -0500, Jeff King wrote:
> On Wed, Dec 19, 2012 at 07:51:16PM -0800, Junio C Hamano wrote:
>
> > > ++ if (limit_pathspec_to_literal())
> > > ++ item->nowildcard_len = item->len;
> > > ++ else {
> > > ++ item->nowildcard_len = simple_length(path);
> > > ++ if (item->nowildcard_len < item->len) {
> > > ++ pathspec->has_wildcard = 1;
> > > ++ if (path[item->nowildcard_len] == '*' &&
> > > ++ no_wildcard(path + item->nowildcard_len + 1))
> > > ++ item->flags |= PATHSPEC_ONESTAR;
> > > ++ }
> > > + }
> >
> > Hmph. I thought that returning the length without any "stop at glob
> > special" trick from simple_length() would be a simpler resolution.
> >
> > That is what is queued at the tip of 'pu', anyway.
>
> I don't think we can make a change in simple_length. It gets used not
> only for pathspecs, but also for parsing exclude patterns, which I do
> not think should be affected by this option.
Our test suite wouldn't catch such a misfeature, of course, because the
feature is not turned on by default. But I found it instructive to run
all of the tests with GIT_LITERAL_PATHSPECS on. There are failures, of
course, but by inspecting each failure you can see that it is an
intended effect of the patch (i.e., each tries to use a wildcard
pathspec, which no longer works).
When you suggested changing common_prefix, I ran such a test both with
and without the change[1] and confirmed that it did not change the set
of failure sites. I did not try it, but I suspect running such a test
with the tip of pu would reveal new failures in the .gitignore tests.
-Peff
[1] This is in addition to reading and reasoning about the code, of
course. I would not consider this a very robust form of testing,
though a test failure which cannot be easily explained would be a
good starting point for investigation.
^ permalink raw reply
* Re: [PATCH] add core.pathspecGlob config option
From: Jeff King @ 2012-12-20 3:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyen Thai Ngoc Duy, git
In-Reply-To: <7vehil7557.fsf@alter.siamese.dyndns.org>
On Wed, Dec 19, 2012 at 07:51:16PM -0800, Junio C Hamano wrote:
> > ++ if (limit_pathspec_to_literal())
> > ++ item->nowildcard_len = item->len;
> > ++ else {
> > ++ item->nowildcard_len = simple_length(path);
> > ++ if (item->nowildcard_len < item->len) {
> > ++ pathspec->has_wildcard = 1;
> > ++ if (path[item->nowildcard_len] == '*' &&
> > ++ no_wildcard(path + item->nowildcard_len + 1))
> > ++ item->flags |= PATHSPEC_ONESTAR;
> > ++ }
> > + }
>
> Hmph. I thought that returning the length without any "stop at glob
> special" trick from simple_length() would be a simpler resolution.
>
> That is what is queued at the tip of 'pu', anyway.
I don't think we can make a change in simple_length. It gets used not
only for pathspecs, but also for parsing exclude patterns, which I do
not think should be affected by this option.
-Peff
^ permalink raw reply
* Re: [PATCH] add core.pathspecGlob config option
From: Junio C Hamano @ 2012-12-20 3:51 UTC (permalink / raw)
To: Jeff King; +Cc: Nguyen Thai Ngoc Duy, git
In-Reply-To: <20121220031327.GB9917@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Thu, Dec 20, 2012 at 08:28:57AM +0700, Nguyen Thai Ngoc Duy wrote:
>
>> > So I think this is a nice, simple approach for sites that want it, and
>> > noglob magic can come later (and will not be any harder to implement as
>> > a result of this patch).
>>
>> Any chance to make use of nd/pathspec-wildcard? It changes the same
>> code path in match_one. If you base on top of nd/pathspec-wildcard,
>> all you have to do is assign nowildcard_len to len (i.e. no wildcard
>> part).
>
> I'd rather keep it separate for now. One, just because they really are
> independent topics, and two, because I am actually back-porting it for
> GitHub (we are fairly conservative about upgrading our backend git
> versions, as most of the interesting stuff happens on the client side; I
> cherry-pick critical patches with no regard to the release cycle).
>
> And the resolution is pretty trivial, too. It looks like this:
>
> diff --cc dir.c
> index 5c0e5f6,03ff36b..81cb439
> --- a/dir.c
> +++ b/dir.c
> @@@ -1456,14 -1433,10 +1460,18 @@@ int init_pathspec(struct pathspec *path
>
> item->match = path;
> item->len = strlen(path);
> - item->nowildcard_len = simple_length(path);
> - item->use_wildcard = !limit_pathspec_to_literal() &&
> - !no_wildcard(path);
> - if (item->use_wildcard)
> - pathspec->has_wildcard = 1;
> + item->flags = 0;
> - if (item->nowildcard_len < item->len) {
> - pathspec->has_wildcard = 1;
> - if (path[item->nowildcard_len] == '*' &&
> - no_wildcard(path + item->nowildcard_len + 1))
> - item->flags |= PATHSPEC_ONESTAR;
> ++ if (limit_pathspec_to_literal())
> ++ item->nowildcard_len = item->len;
> ++ else {
> ++ item->nowildcard_len = simple_length(path);
> ++ if (item->nowildcard_len < item->len) {
> ++ pathspec->has_wildcard = 1;
> ++ if (path[item->nowildcard_len] == '*' &&
> ++ no_wildcard(path + item->nowildcard_len + 1))
> ++ item->flags |= PATHSPEC_ONESTAR;
> ++ }
> + }
> }
>
> qsort(pathspec->items, pathspec->nr,
Hmph. I thought that returning the length without any "stop at glob
special" trick from simple_length() would be a simpler resolution.
That is what is queued at the tip of 'pu', anyway.
^ permalink raw reply
* Re: [PATCH] add core.pathspecGlob config option
From: Jeff King @ 2012-12-20 3:13 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8BB3=3ZHD5Ua9M-0+98JVigHBBuo07gBSgEwanvB0zBSA@mail.gmail.com>
On Thu, Dec 20, 2012 at 08:28:57AM +0700, Nguyen Thai Ngoc Duy wrote:
> > So I think this is a nice, simple approach for sites that want it, and
> > noglob magic can come later (and will not be any harder to implement as
> > a result of this patch).
>
> Any chance to make use of nd/pathspec-wildcard? It changes the same
> code path in match_one. If you base on top of nd/pathspec-wildcard,
> all you have to do is assign nowildcard_len to len (i.e. no wildcard
> part).
I'd rather keep it separate for now. One, just because they really are
independent topics, and two, because I am actually back-porting it for
GitHub (we are fairly conservative about upgrading our backend git
versions, as most of the interesting stuff happens on the client side; I
cherry-pick critical patches with no regard to the release cycle).
And the resolution is pretty trivial, too. It looks like this:
diff --cc dir.c
index 5c0e5f6,03ff36b..81cb439
--- a/dir.c
+++ b/dir.c
@@@ -1456,14 -1433,10 +1460,18 @@@ int init_pathspec(struct pathspec *path
item->match = path;
item->len = strlen(path);
- item->nowildcard_len = simple_length(path);
- item->use_wildcard = !limit_pathspec_to_literal() &&
- !no_wildcard(path);
- if (item->use_wildcard)
- pathspec->has_wildcard = 1;
+ item->flags = 0;
- if (item->nowildcard_len < item->len) {
- pathspec->has_wildcard = 1;
- if (path[item->nowildcard_len] == '*' &&
- no_wildcard(path + item->nowildcard_len + 1))
- item->flags |= PATHSPEC_ONESTAR;
++ if (limit_pathspec_to_literal())
++ item->nowildcard_len = item->len;
++ else {
++ item->nowildcard_len = simple_length(path);
++ if (item->nowildcard_len < item->len) {
++ pathspec->has_wildcard = 1;
++ if (path[item->nowildcard_len] == '*' &&
++ no_wildcard(path + item->nowildcard_len + 1))
++ item->flags |= PATHSPEC_ONESTAR;
++ }
+ }
}
qsort(pathspec->items, pathspec->nr,
Not re-indenting the conditional would make the diff more readable, but
I think the resulting code is simpler to read if all of the wildcard
stuff is inside the "else" block.
-Peff
^ permalink raw reply
* Re: sys/param.h
From: Junio C Hamano @ 2012-12-20 3:03 UTC (permalink / raw)
To: Mark Levedahl; +Cc: git
In-Reply-To: <50D27CC6.3000203@gmail.com>
Thanks.
^ permalink raw reply
* Re: sys/param.h
From: Mark Levedahl @ 2012-12-20 2:49 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, git
In-Reply-To: <CABPQNSZQk6hHm-dWqFFQf0HF34Mvbjc8-mgzCr=G0zbBKiYUvA@mail.gmail.com>
On 12/19/2012 02:59 AM, Erik Faye-Lund wrote:
> On Tue, Dec 18, 2012 at 6:01 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Johannes Sixt <j.sixt@viscovery.net> writes:
>>
>>>> Junio C Hamano wrote:
>>>>> It could turn out that we may be able to get rid of sys/param.h
>>>>> altogether, but one step at a time. Inputs from people on minority
>>>>> platforms are very much appreciated---does your platform build fine
>>>>> when the inclusion of the file is removed from git-compat-util.h?
cygwin is fine with that removed.
Mark
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox