Git development
 help / color / mirror / Atom feed
* Conference in near
From: Anthony Murphy @ 2008-10-23 22:04 UTC (permalink / raw)
  To: git

The 2008 Hard Money National Conference in near

All the information is at:
http://hardmoneyparty.com
or call us today at 858-736-7788 for the event details.



to be taken of our list please: 
http://hardmoneyparty.com/uns/ or, send us a letter to PBMSII, 5580 la jolla blvd #153 La Jolla, Ca 92037


--------------------------------------------
. 

^ permalink raw reply

* Re: git performance
From: Nanako Shiraishi @ 2008-10-23 22:24 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: Edward Ned Harvey, git
In-Reply-To: <alpine.LNX.1.00.0810231346520.19665@iabervon.org>

Quoting Daniel Barkalow <barkalow@iabervon.org>:

> On Wed, 22 Oct 2008, Edward Ned Harvey wrote:
>
>> Out of curiosity, what are they talking about, when they say "git is 
>> fast?"  Just the fact that it's all local disk, or is there more to it 
>> than that?  I could see - git would probably outperform perforce for 
>> versioning of large files (let's say iso files) to benefit from 
>> sustained local disk IO, while perforce would probably outperform 
>> anything I can think of, operating on thousands of tiny files, because 
>> it will never walk the tree. 
>
> It shouldn't be too hard to make git work like perforce with respect to 
> walking the tree. git keeps an index of the stat() info it saw when it 
> last looked at files, and only looks at the contents of files whose stat() 
> info has changed. In order to have it work like perforce, it would just 
> need to have a flag in the stat() info index for "don't even bother", 

Are you describing the "assume unchanged bit"?

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply

* Re: How to push to http(s) repository with authentication?
From: Josef Wolf @ 2008-10-23 22:32 UTC (permalink / raw)
  To: git
In-Reply-To: <20081023190544.GA21079@blimp.localdomain>

On Thu, Oct 23, 2008 at 09:05:44PM +0200, Alex Riesen wrote:

Thanks for your response, Alex!

[ ... ]
>        --cacert <CA certificate>
[ ... ]

> Does this (setting $CURL_CA_BUNDLE) help?

Well, as I wrote in my original mail, I already got the certificate
working.  My problem is the usage of the .netrc file.  I don't want
to store passwords in cleartext in files.

So the real question is: how do I make git-http to ask for credentials.

Sorry, I should have made more clearly what's the problem.

^ permalink raw reply

* Re: [JGIT RFC PATCH 3/3] Rate limit warnings spewed by RepositoryTestCase.recursiveDelete
From: Robin Rosenberg @ 2008-10-23 23:10 UTC (permalink / raw)
  To: Jonas Fonseca; +Cc: Shawn O. Pearce, git
In-Reply-To: <20081022083420.GC17940@diku.dk>

onsdagen den 22 oktober 2008 10.34.20 skrev Jonas Fonseca:
> On Windows XP / NTFS / NetBeans 6.1 / Java 5 a lot of warnings are
> printed. In most cases the path is in fact deleted and it seems to just
> be a timing bug or something Windows or NTFS specific. Also, many of the
> warnings are for the same paths and therefore a bit redundant.
> Furthermore, when a lot of warnings are printed it gives the feeling of
> slowing down the tests. By rate limiting the warnings the problem is
> still made visible for developers to notice.

The problem is actually flaws in the unit tests and in the supporting RepositoryTestCase.
I think I'll fix that instead.

-- robin

^ permalink raw reply

* How does Git treat hardlinked files which are outside the working directory?
From: Weiwei Shu @ 2008-10-23 23:13 UTC (permalink / raw)
  To: git

Hi,

Can someone point to me if there is any special care needed for
hardlink files if they are outside the versioned directory? Thanks!


Weiwei

-- 
Weiwei
--Hope Springs Eternal.

^ permalink raw reply

* Re: [RFC] Zit: the git-based single file content tracker
From: Jean-Luc Herren @ 2008-10-23 23:23 UTC (permalink / raw)
  To: Giuseppe Bilotta, git
In-Reply-To: <gdok16$vh2$1@ger.gmane.org>

Hi!

Giuseppe Bilotta wrote:
> So today I decided to start hacking at a git-based but file-oriented
> content tracker, which I decided to name Zit.

This sounds great and would seem very useful to manage my ~/bin/
directory which contains a set of unrelated one-file-tools that
evolve over time.  I haven't played with it yet though.

> when you choose to start tracking a file with Zit [...]
> Zit will create a directory .zit.file to hold a git repository

If you have many files you want to track in a single directory
(like ~/bin/), all those additional directories will quickly feel
like clutter.  If you track every file, it will even double the
number of things you see with an "ls -a".

If you decide against a shared repository, maybe you want to
consider to not use ".zit.file/", but ".zit/file/" as the
repository?  This would reduce the clutter to a single directory,
just like with ".git".  And moving files around wouldn't be that
much complicated.

jlh

^ permalink raw reply

* Problem with Git.pm bidi_pipe methods
From: Philippe Bruhat (BooK) @ 2008-10-24  0:14 UTC (permalink / raw)
  To: git

Hi,

To be able to call commit-tree from a Perl program, I had to use
command_bidi_pipe() to pass the message on standard input, and get the
new commit id on standard output. The only problem I have is that the
method doesn't work (or at least, doesn't work like the other
command_... methods).

    sub command_bidi_pipe {
        my ($pid, $in, $out);
        $pid = open2($in, $out, 'git', @_);
        return ($pid, $in, $out, join(' ', @_));
    }

First, it calls git in the current working dir, when all the other
command_... methods do the appropriate chdir depending on the Git object.

Second, it can't be used as a method, since $self is not removed from @_
before calling open2, leading to errors such as this one:

    git: 'Git=HASH(0x82f898c)' is not a git-command. See 'git --help'.

Third, no access is given to STDERR (open3 may help).

I'm very sorry to complain without proposing a patch.

I think solving the current directory problem is probably going to be
difficult with open2/open3, since open2 does the fork+exec, whereas Git.pm
handles the fork+chdir+exec itself for the other command_...  methods.
And so there is no room to chdir when calling open2.

-- 
 Philippe Bruhat (BooK)

 Treat those you outrank well... you never know when they will outrank you.
                                                 (Moral from Groo #7 (Image))

^ permalink raw reply

* Re: [PATCH stgit] revised patch for importing series from tarball
From: Karl Hasselström @ 2008-10-24  1:17 UTC (permalink / raw)
  To: Clark Williams; +Cc: Git Mailing List
In-Reply-To: <20081019141613.05cbd93e@gmail.com>

On 2008-10-19 14:16:13 -0500, Clark Williams wrote:

> I added a simple test to the test harness for import as well.

Great!

> Oh and I added '*.elc' to the .gitignore file. May not be that many
> folks using emacs with stgit, but hey, I am! :)

Me too (but I have *.elc in my personal ignore file, so I never saw
it). However, this is an unrelated change, and we don't need to ignore
.elc files at the top level, only in contrib. I've taken the liberty
of splitting this out to a separate commit and forging your signature
on it -- let me know if that's OK and I'll push it out.

> patch to allow importing a series from a tar archive

I turned this into a complete sentence, and removed "patch" since it's
entirely redundant.

> --- a/.gitignore
> +++ b/.gitignore
> @@ -6,3 +6,4 @@ patches-*
>  release.sh
>  setup.cfg.rpm
>  snapshot.sh
> +*.elc

As I said, I put this in a separate patch, and put that ignore pattern
in contrib.gitignore instead.

>             make_option('--commemail',
> -                       help = 'use COMMEMAIL as the committer e-mail')
> +                       help = 'use COMMEMAIL as the committer e-mail'),
>             ] + make_sign_options()

This is just a noise change _and_ didn't apply, so I dropped it.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

^ permalink raw reply

* Re: clean/smudge filters for pdf files
From: Leo Razoumov @ 2008-10-24  1:40 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: git
In-Reply-To: <20081023213203.GB26104@artemis.corp>

On 10/23/08, Pierre Habouzit <madcoder@debian.org> wrote:
> On Thu, Oct 23, 2008 at 07:44:39PM +0000, Leo Razoumov wrote:
>  > I am trying to improve storage efficiency for PDF files in a git repo.
>  > Following earlier discussions in this list I am trying to set up
>  > proper clean/smudge filters. What follows is my current setup
>  >
>  > # in ~/.gitconfig
>  > [filter "pdf"]
>  >       clean  = "pdftk - output - uncompress"
>  >       smudge = "pdftk - output - compress"
>  >
>  > # in .gitattributes
>  > *.pdf filter=pdf
>  >
>  > Unfortunately, it seems as though that pdftk uncompress followed by
>  > pdftk compress do not leave the file invariant. I tried several
>  > uncompress+compress iterations and the file still keep changing (the
>  > size though stays the same).
>  > Is there any other alternative way to store PDF files in git repo more
>  > efficiently?
>  > Any alternative to pdftk on Linux?
>
>
> actually it uses some kind of zlib algorithm so that's pretty normal you
>  don't have the same result with a packer. Maybe one could write a tool
>  like pristine-tar for that purpose.
>

With zlib you get the same deterministic result as long as you use the
same zlib packer and unpacker. With pdftk compress/uncompress seem not
to form a bijection pair. This issue was briefly discussed on this
list back in April 2008 but no resolution emerged.

--Leo--

^ permalink raw reply

* Re: What's in git.git (Oct 2008, #03; Tue, 14)
From: Junio C Hamano @ 2008-10-24  2:44 UTC (permalink / raw)
  To: Brandon Casey
  Cc: git, Arjen Laarhoven, Jeff King, Shawn O. Pearce, Mike Ralphson
In-Reply-To: <bsftmRx17krWBpVlulipoJEO1fWsD0hZfF3HOZcajX6GV66RKW8W6A@cipher.nrlssc.navy.mil>

Brandon Casey <casey@nrlssc.navy.mil> writes:

>   2) Do we want to stick with compat/regex on
>
>       Darwin: Arjen
>      FreeBSD: Jeff
>          AIX: Mike
>
>      now that the builtin funcname patterns have been converted to Extended
>      Regular Expressions?

-- >8 --
Subject: Stop using compat/regex.c on platforms with working regexp library

We used to have non-POSIX comformant BRE in our code, and linked with GNU
regexp library on a few platforms (Darwin, FreeBSD and AIX) to work it
around.  This was backwards.

We've fixed the broken regexps to use ERE that native regexp libraries on
these platforms can handle just fine.  There is no need to link with GNU
regexp library on these platforms anymore.

Tested-on-AIX-by: Mike Ralphson <mike@abacus.co.uk>
Tested-on-FreeBSD-by: Jeff King <peff@peff.net>
Tested-on-Darwin-by: Arjen Laarhoven <arjen@yaph.org>
Tested-on-Darwin-by: Pieter de Bie <pieter@frim.nl>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Makefile |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git c/Makefile w/Makefile
index d6f3695..40309e1 100644
--- c/Makefile
+++ w/Makefile
@@ -640,8 +640,6 @@ ifeq ($(uname_S),Darwin)
 	endif
 	NO_STRLCPY = YesPlease
 	NO_MEMMEM = YesPlease
-	COMPAT_CFLAGS += -Icompat/regex
-	COMPAT_OBJS += compat/regex/regex.o
 endif
 ifeq ($(uname_S),SunOS)
 	NEEDS_SOCKET = YesPlease
@@ -692,8 +690,6 @@ ifeq ($(uname_S),FreeBSD)
 	BASIC_LDFLAGS += -L/usr/local/lib
 	DIR_HAS_BSD_GROUP_SEMANTICS = YesPlease
 	THREADED_DELTA_SEARCH = YesPlease
-	COMPAT_CFLAGS += -Icompat/regex
-	COMPAT_OBJS += compat/regex/regex.o
 endif
 ifeq ($(uname_S),OpenBSD)
 	NO_STRCASESTR = YesPlease
@@ -720,8 +716,6 @@ ifeq ($(uname_S),AIX)
 	INTERNAL_QSORT = UnfortunatelyYes
 	NEEDS_LIBICONV=YesPlease
 	BASIC_CFLAGS += -D_LARGE_FILES
-	COMPAT_CFLAGS += -Icompat/regex
-	COMPAT_OBJS += compat/regex/regex.o
 endif
 ifeq ($(uname_S),GNU)
 	# GNU/Hurd

^ permalink raw reply related

* Re: [PATCH 3/4] diff: introduce diff.<driver>.binary
From: Jeff King @ 2008-10-24  2:46 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Sixt, Matthieu Moy
In-Reply-To: <20081005214336.GC21925@coredump.intra.peff.net>

On Sun, Oct 05, 2008 at 05:43:36PM -0400, Jeff King wrote:

> However, there is at least one conflicting situation: there
> is no way to say "use the regular rules to determine whether
> this file is binary, but if we do diff it textually, use
> this funcname pattern." That is, currently setting diff=foo
> indicates that the file is definitely text.

Hrm. I don't know what crack I was smoking when I wrote this (and then
argued about it for weeks afterward). It is actually the _opposite_
situation.  That is, once you have said "diff=foo", there is no way to
say "btw, foo files are _definitely_ text."

See, this is the old code:

> -static void diff_filespec_check_attr(struct diff_filespec *one)
> +void diff_filespec_load_driver(struct diff_filespec *one)
>  {
> -	struct userdiff_driver *drv;
> -	int check_from_data = 0;
> -
> -	if (one->checked_attr)
> -		return;
> -
> -	drv = userdiff_find_by_path(one->path);
> -	one->is_binary = 0;
> -
> -	/* binaryness */
> -	if (drv == USERDIFF_ATTR_TRUE)
> -		;
> -	else if (drv == USERDIFF_ATTR_FALSE)
> -		one->is_binary = 1;
> -	else
> -		check_from_data = 1;
> -
> -	if (check_from_data) {
> -		if (!one->data && DIFF_FILE_VALID(one))
> -			diff_populate_filespec(one, 0);
> -
> -		if (one->data)
> -			one->is_binary = buffer_is_binary(one->data, one->size);
> -	}
> +	if (!one->driver)
> +		one->driver = userdiff_find_by_path(one->path);
> +	if (!one->driver)
> +		one->driver = userdiff_find_by_name("default");
>  }

You can clearly see that we use check_from_data as long as the value is
not true or false. Meaning if it is unspecified _or_ if it has a string
value (actually, this text is hacked up by my previous patch, but you
can look at maint:diff.c and see that it is similar).

And this makes sense. Otherwise, plumbing like "git diff-tree" would
get mightily confused by external diff commands. For example, consider
if you had "foo diff=bar" in your attributes file, and defined
"diff.bar.command". That external diff would be used for git-diff, but
_not_ for diff-tree. But it would be stupid for diff-tree to look at the
attribute and say "oh, we have a diff attr, it must be text."

So the patch is right to keep the binary value to "-1" except for the
few cases where it has been specified explicitly. I found this out when
I tried to "fix" it to the old behavior tonight and discovered lots of
breakage.

And this also means that diff.*.binary really _does_ do something
useful. If you have, for example, a ".foo" file that looks like binary,
but really isn't, _and_ you want to set a custom hunk header for it,
previously you were out of luck. You could do one or the other. Now you
can do:

  git config diff.foo.xfuncname whatever
  git config diff.foo.binary false

and get the desired effect.

As for the fallout from this with regards to our discussion last week,
there were two relevant points:

 - Junio suggested that anytime we use funcname, we always want text
   anyway. I think that is reasonable, but it has never been the case up
   until now (in fact, you were stuck with the _opposite_ until now, so
   my series at least makes it possible to say "always text", though
   you have to do it manually). So I will leave it for now, and
   if people feel strongly, my series provides a sane basis for a patch
   that does this automatically.

 - Johannes complained about having to set "diff.foo.binary = false"
   when we have set "diff.foo.textconv".  I agree that having to set it
   is cumbersome, but what's worse is that it is wrong. You are saying
   "this file is not binary" which is only _sometimes_ true. That is, it
   is only true if we are in a command which actually performs the text
   conversion. Plumbing sees _just_ the binary half, which is outright
   wrong (and which became painfully obvious once I wrote some tests).

   The solution is that textconv'd data should always be treated as
   text, and that takes some refactoring of my patches. I will post a
   series dealing with this in a minute.

Hopefully that explanation made sense. This turned out to be a lot
trickier than I thought (in combination with my apparent crack habit),
and I just spent several hours trying to figure out all of the niggling
details. But I realize the rest of you haven't thought about it for at
least a week. :)

-Peff

^ permalink raw reply

* [PATCH 1/5] diff: add missing static declaration
From: Jeff King @ 2008-10-24  2:48 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Sixt, Matthieu Moy
In-Reply-To: <20081024024631.GA20365@coredump.intra.peff.net>

This function isn't used outside of diff.c; the 'static' was
simply overlooked in the original writing.

Signed-off-by: Jeff King <peff@peff.net>
---
Just a cleanup.

 diff.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/diff.c b/diff.c
index e368fef..d1fd594 100644
--- a/diff.c
+++ b/diff.c
@@ -1282,7 +1282,7 @@ static void emit_binary_diff(FILE *file, mmfile_t *one, mmfile_t *two)
 	emit_binary_diff_body(file, two, one);
 }
 
-void diff_filespec_load_driver(struct diff_filespec *one)
+static void diff_filespec_load_driver(struct diff_filespec *one)
 {
 	if (!one->driver)
 		one->driver = userdiff_find_by_path(one->path);
-- 
1.6.0.3.518.gdb328.dirty

^ permalink raw reply related

* [PATCH 2/5] add userdiff textconv tests
From: Jeff King @ 2008-10-24  2:50 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Sixt, Matthieu Moy
In-Reply-To: <20081024024631.GA20365@coredump.intra.peff.net>

These tests provide a basic sanity check that textconv'd
files work. The tests try to describe how this configuration
_should_ work; thus some of the tests are marked to expect
failure.

In particular, we fail to actually textconv anything because
the 'diff.foo.binary' config option is not set, which will
be fixed in the next patch.

Signed-off-by: Jeff King <peff@peff.net>
---
Note that this just tests actual patch generation. Diffstat, etc, are
left for later, as is discussed in the next patch.

 t/t4030-diff-textconv.sh |   80 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 80 insertions(+), 0 deletions(-)
 create mode 100755 t/t4030-diff-textconv.sh

diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh
new file mode 100755
index 0000000..d0d7691
--- /dev/null
+++ b/t/t4030-diff-textconv.sh
@@ -0,0 +1,80 @@
+#!/bin/sh
+
+test_description='diff.*.textconv tests'
+. ./test-lib.sh
+
+find_diff() {
+	sed '1,/^index /d' | sed '/^-- $/,$d'
+}
+
+cat >expect.binary <<'EOF'
+Binary files a/file and b/file differ
+EOF
+
+cat >expect.text <<'EOF'
+--- a/file
++++ b/file
+@@ -1 +1,2 @@
+ 0
++1
+EOF
+
+cat >hexdump <<'EOF'
+#!/bin/sh
+perl -e '$/ = undef; $_ = <>; s/./ord($&)/ge; print $_' "$1"
+EOF
+chmod +x hexdump
+
+test_expect_success 'setup binary file with history' '
+	printf "\\0\\n" >file &&
+	git add file &&
+	git commit -m one &&
+	printf "\\1\\n" >>file &&
+	git add file &&
+	git commit -m two
+'
+
+test_expect_success 'file is considered binary by porcelain' '
+	git diff HEAD^ HEAD >diff &&
+	find_diff <diff >actual &&
+	test_cmp expect.binary actual
+'
+
+test_expect_success 'file is considered binary by plumbing' '
+	git diff-tree -p HEAD^ HEAD >diff &&
+	find_diff <diff >actual &&
+	test_cmp expect.binary actual
+'
+
+test_expect_success 'setup textconv filter' '
+	echo file diff=foo >.gitattributes &&
+	git config diff.foo.textconv "$PWD"/hexdump
+'
+
+test_expect_failure 'diff produces text' '
+	git diff HEAD^ HEAD >diff &&
+	find_diff <diff >actual &&
+	test_cmp expect.text actual
+'
+
+test_expect_success 'diff-tree produces binary' '
+	git diff-tree -p HEAD^ HEAD >diff &&
+	find_diff <diff >actual &&
+	test_cmp expect.binary actual
+'
+
+test_expect_failure 'log produces text' '
+	git log -1 -p >log &&
+	find_diff <log >actual &&
+	test_cmp expect.text actual
+'
+
+# actually passes, but only because textconv is broken
+# and its failure mode happens to be the same as success
+test_expect_failure 'format-patch produces binary' '
+	git format-patch --no-binary --stdout HEAD^ >patch &&
+	find_diff <patch >actual &&
+	test_cmp expect.binary actual
+'
+
+test_done
-- 
1.6.0.3.518.gdb328.dirty

^ permalink raw reply related

* Re: [PATCH stgit] revised patch for importing series from tarball
From: Clark Williams @ 2008-10-24  2:53 UTC (permalink / raw)
  To: Karl Hasselström; +Cc: Git Mailing List
In-Reply-To: <20081024011708.GA20369@diana.vm.bytemark.co.uk>

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Fri, 24 Oct 2008 03:17:08 +0200
Karl Hasselström <kha@treskal.com> wrote:

> On 2008-10-19 14:16:13 -0500, Clark Williams wrote:
> 
> > I added a simple test to the test harness for import as well.
> 
> Great!
> 
> > Oh and I added '*.elc' to the .gitignore file. May not be that many
> > folks using emacs with stgit, but hey, I am! :)
> 
> Me too (but I have *.elc in my personal ignore file, so I never saw
> it). However, this is an unrelated change, and we don't need to ignore
> .elc files at the top level, only in contrib. I've taken the liberty
> of splitting this out to a separate commit and forging your signature
> on it -- let me know if that's OK and I'll push it out.
> 

Yes, that's fine.

> > patch to allow importing a series from a tar archive
> 
> I turned this into a complete sentence, and removed "patch" since it's
> entirely redundant.
> 
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -6,3 +6,4 @@ patches-*
> >  release.sh
> >  setup.cfg.rpm
> >  snapshot.sh
> > +*.elc
> 
> As I said, I put this in a separate patch, and put that ignore pattern
> in contrib.gitignore instead.
> 
Thanks

> >             make_option('--commemail',
> > -                       help = 'use COMMEMAIL as the committer e-mail')
> > +                       help = 'use COMMEMAIL as the committer e-mail'),
> >             ] + make_sign_options()
> 
> This is just a noise change _and_ didn't apply, so I dropped it.
> 

Agreed. 

Thanks for the update.

Clark

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.9 (GNU/Linux)

iEYEARECAAYFAkkBOJ0ACgkQqA4JVb61b9e6SgCeOZ0YrB/uSrvSqgLkxnLjiQp2
AWwAoKIeIQUOWEghEOWBgODnTojA2o5q
=B9dz
-----END PGP SIGNATURE-----

^ permalink raw reply

* [PATCH 3/5] refactor userdiff textconv code
From: Jeff King @ 2008-10-24  2:53 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Sixt, Matthieu Moy
In-Reply-To: <20081024024631.GA20365@coredump.intra.peff.net>

The initial implementation inserted itself at the level of
fill_mmfile. That is, the diff_filespec always contained the
actual file data, but we set up the xdl mmfile with the fake
data. This had several disadvantages:

 - fill_mmfile gets called from several places, but we don't
   necessarily want all of them to see the textconv'd
   version (e.g., whitespace checking should probably
   consider it binary)

 - the filespec itself carried no information about
   binaryness. If we have textconv'd it, then it is no
   longer binary and we need to mark that somehow (which
   lets us avoid having to manually set the filetype to
   non-binary in the diff driver config)

This patch moves the text conversion to a
diff_filespec_textconv function; this function can be called
to insert the text-converted contents of the file into the
filespec. These contents will survive through any
diff_populate_filespec calls, meaning that it is safe to
pass the resulting filespec to other diff functions which
will look at the content.

We now turn on the text conversion only for actual diffs,
not diffstats or whitespace checking. We may eventually want
it for other formats, too, but we will be conservative for
now.

Signed-off-by: Jeff King <peff@peff.net>
---
I think this is much cleaner. I have a nagging worry that a
text-converted filespec might live longer than I expect. Maybe it would
make sense to free the filespec data after the diff has been generated
(and then further populate_filespec calls would just restore the
original data).

Also, I'm happy to hear comments on which functionality should get
text-converted. I think I like seeing the --stat report the binary
changes.

 diff.c                   |   51 +++++++++++++++++++++++++++++++++++----------
 diffcore.h               |    1 +
 t/t4030-diff-textconv.sh |    6 +---
 3 files changed, 42 insertions(+), 16 deletions(-)

diff --git a/diff.c b/diff.c
index d1fd594..89bd2ff 100644
--- a/diff.c
+++ b/diff.c
@@ -284,6 +284,36 @@ static void emit_rewrite_diff(const char *name_a,
 		copy_file_with_prefix(o->file, '+', two->data, two->size, new, reset);
 }
 
+static int diff_filespec_textconv(struct diff_filespec *one)
+{
+	size_t size;
+	char *buf;
+
+	if (one->data_is_textconv)
+		return 0;
+
+	if (!DIFF_FILE_VALID(one))
+		return 0;
+
+	diff_filespec_load_driver(one);
+	if (!one->driver->textconv)
+		return 0;
+
+	if (diff_populate_filespec(one, 0) < 0)
+		return -1;
+
+	buf = run_textconv(one->driver->textconv, one, &size);
+	if (!buf)
+		return -1;
+
+	diff_free_filespec_blob(one);
+	one->data = buf;
+	one->size = size;
+	one->should_free = 1;
+	one->data_is_textconv = 1;
+	return 0;
+}
+
 static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one)
 {
 	if (!DIFF_FILE_VALID(one)) {
@@ -294,18 +324,8 @@ static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one)
 	else if (diff_populate_filespec(one, 0))
 		return -1;
 
-	diff_filespec_load_driver(one);
-	if (one->driver->textconv) {
-		size_t size;
-		mf->ptr = run_textconv(one->driver->textconv, one, &size);
-		if (!mf->ptr)
-			return -1;
-		mf->size = size;
-	}
-	else {
-		mf->ptr = one->data;
-		mf->size = one->size;
-	}
+	mf->ptr = one->data;
+	mf->size = one->size;
 	return 0;
 }
 
@@ -1292,6 +1312,8 @@ static void diff_filespec_load_driver(struct diff_filespec *one)
 
 int diff_filespec_is_binary(struct diff_filespec *one)
 {
+	if (one->data_is_textconv)
+		return 0;
 	if (one->is_binary == -1) {
 		diff_filespec_load_driver(one);
 		if (one->driver->binary != -1)
@@ -1387,6 +1409,10 @@ static void builtin_diff(const char *name_a,
 		}
 	}
 
+	if (diff_filespec_textconv(one) < 0 ||
+	    diff_filespec_textconv(two) < 0)
+			die("unable to read files to diff");
+
 	if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
 		die("unable to read files to diff");
 
@@ -1778,6 +1804,7 @@ void diff_free_filespec_blob(struct diff_filespec *s)
 		s->should_free = s->should_munmap = 0;
 		s->data = NULL;
 	}
+	s->data_is_textconv = 0;
 }
 
 void diff_free_filespec_data(struct diff_filespec *s)
diff --git a/diffcore.h b/diffcore.h
index 713cca7..33aebc2 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -42,6 +42,7 @@ struct diff_filespec {
 #define DIFF_FILE_VALID(spec) (((spec)->mode) != 0)
 	unsigned should_free : 1; /* data should be free()'ed */
 	unsigned should_munmap : 1; /* data should be munmap()'ed */
+	unsigned data_is_textconv : 1;
 
 	struct userdiff_driver *driver;
 	/* data should be considered "binary"; -1 means "don't know yet" */
diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh
index d0d7691..28d3640 100755
--- a/t/t4030-diff-textconv.sh
+++ b/t/t4030-diff-textconv.sh
@@ -51,7 +51,7 @@ test_expect_success 'setup textconv filter' '
 	git config diff.foo.textconv "$PWD"/hexdump
 '
 
-test_expect_failure 'diff produces text' '
+test_expect_success 'diff produces text' '
 	git diff HEAD^ HEAD >diff &&
 	find_diff <diff >actual &&
 	test_cmp expect.text actual
@@ -63,14 +63,12 @@ test_expect_success 'diff-tree produces binary' '
 	test_cmp expect.binary actual
 '
 
-test_expect_failure 'log produces text' '
+test_expect_success 'log produces text' '
 	git log -1 -p >log &&
 	find_diff <log >actual &&
 	test_cmp expect.text actual
 '
 
-# actually passes, but only because textconv is broken
-# and its failure mode happens to be the same as success
 test_expect_failure 'format-patch produces binary' '
 	git format-patch --no-binary --stdout HEAD^ >patch &&
 	find_diff <patch >actual &&
-- 
1.6.0.3.518.gdb328.dirty

^ permalink raw reply related

* [PATCH 4/5] userdiff: require explicitly allowing textconv
From: Jeff King @ 2008-10-24  2:55 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Sixt, Matthieu Moy
In-Reply-To: <20081024024631.GA20365@coredump.intra.peff.net>

Diffs that have been produced with textconv almost certainly
cannot be applied, so we want to be careful not to generate
them in things like format-patch.

This introduces a new diff options, ALLOW_TEXTCONV, which
controls this behavior. It is off by default, but is
explicitly turned on for the "log" family of commands, as
well as the "diff" porcelain.

Because both text conversion and external diffing are
controlled by these diff options, we can get rid of the
"plumbing versus porcelain" distinction when reading the
config. This was an attempt to control the same thing, but
suffered from being too coarse-grained.

Signed-off-by: Jeff King <peff@peff.net>
---
This gives us a nice run-time knob to tweak. There was a lot of
discussion last time around about exactly when text conversion should
happen. The more I think about it, I'm wondering if perhaps this should
be disabled by default, and require "git log --textconv" to activate.

Johannes, you had mentioned some heuristics. Maybe you would like to
take a stab at implementing them on top of this?

 builtin-diff.c           |    1 +
 builtin-log.c            |    1 +
 diff.c                   |   24 ++++++++++--------------
 diff.h                   |    1 +
 t/t4030-diff-textconv.sh |    2 +-
 userdiff.c               |   10 +---------
 userdiff.h               |    3 +--
 7 files changed, 16 insertions(+), 26 deletions(-)

diff --git a/builtin-diff.c b/builtin-diff.c
index 9c8c295..2de5834 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -300,6 +300,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 	}
 	DIFF_OPT_SET(&rev.diffopt, ALLOW_EXTERNAL);
 	DIFF_OPT_SET(&rev.diffopt, RECURSIVE);
+	DIFF_OPT_SET(&rev.diffopt, ALLOW_TEXTCONV);
 
 	/*
 	 * If the user asked for our exit code then don't start a
diff --git a/builtin-log.c b/builtin-log.c
index a0944f7..75d698f 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -59,6 +59,7 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 		} else
 			die("unrecognized argument: %s", arg);
 	}
+	DIFF_OPT_SET(&rev->diffopt, ALLOW_TEXTCONV);
 }
 
 /*
diff --git a/diff.c b/diff.c
index 89bd2ff..52feba7 100644
--- a/diff.c
+++ b/diff.c
@@ -93,12 +93,6 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
 	if (!strcmp(var, "diff.external"))
 		return git_config_string(&external_diff_cmd_cfg, var, value);
 
-	switch (userdiff_config_porcelain(var, value)) {
-		case 0: break;
-		case -1: return -1;
-		default: return 0;
-	}
-
 	return git_diff_basic_config(var, value, cb);
 }
 
@@ -109,6 +103,12 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 
+	switch (userdiff_config(var, value)) {
+		case 0: break;
+		case -1: return -1;
+		default: return 0;
+	}
+
 	if (!prefixcmp(var, "diff.color.") || !prefixcmp(var, "color.diff.")) {
 		int slot = parse_diff_color_slot(var, 11);
 		if (!value)
@@ -123,12 +123,6 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 
-	switch (userdiff_config_basic(var, value)) {
-		case 0: break;
-		case -1: return -1;
-		default: return 0;
-	}
-
 	return git_color_default_config(var, value, cb);
 }
 
@@ -1409,9 +1403,11 @@ static void builtin_diff(const char *name_a,
 		}
 	}
 
-	if (diff_filespec_textconv(one) < 0 ||
-	    diff_filespec_textconv(two) < 0)
+	if (DIFF_OPT_TST(o, ALLOW_TEXTCONV)) {
+		if (diff_filespec_textconv(one) < 0 ||
+		    diff_filespec_textconv(two) < 0)
 			die("unable to read files to diff");
+	}
 
 	if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
 		die("unable to read files to diff");
diff --git a/diff.h b/diff.h
index a49d865..42582ed 100644
--- a/diff.h
+++ b/diff.h
@@ -65,6 +65,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
 #define DIFF_OPT_IGNORE_SUBMODULES   (1 << 18)
 #define DIFF_OPT_DIRSTAT_CUMULATIVE  (1 << 19)
 #define DIFF_OPT_DIRSTAT_BY_FILE     (1 << 20)
+#define DIFF_OPT_ALLOW_TEXTCONV      (1 << 21)
 #define DIFF_OPT_TST(opts, flag)    ((opts)->flags & DIFF_OPT_##flag)
 #define DIFF_OPT_SET(opts, flag)    ((opts)->flags |= DIFF_OPT_##flag)
 #define DIFF_OPT_CLR(opts, flag)    ((opts)->flags &= ~DIFF_OPT_##flag)
diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh
index 28d3640..e456746 100755
--- a/t/t4030-diff-textconv.sh
+++ b/t/t4030-diff-textconv.sh
@@ -69,7 +69,7 @@ test_expect_success 'log produces text' '
 	test_cmp expect.text actual
 '
 
-test_expect_failure 'format-patch produces binary' '
+test_expect_success 'format-patch produces binary' '
 	git format-patch --no-binary --stdout HEAD^ >patch &&
 	find_diff <patch >actual &&
 	test_cmp expect.binary actual
diff --git a/userdiff.c b/userdiff.c
index d95257a..3681062 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -120,7 +120,7 @@ static int parse_tristate(int *b, const char *k, const char *v)
 	return 1;
 }
 
-int userdiff_config_basic(const char *k, const char *v)
+int userdiff_config(const char *k, const char *v)
 {
 	struct userdiff_driver *drv;
 
@@ -130,14 +130,6 @@ int userdiff_config_basic(const char *k, const char *v)
 		return parse_funcname(&drv->funcname, k, v, REG_EXTENDED);
 	if ((drv = parse_driver(k, v, "binary")))
 		return parse_tristate(&drv->binary, k, v);
-
-	return 0;
-}
-
-int userdiff_config_porcelain(const char *k, const char *v)
-{
-	struct userdiff_driver *drv;
-
 	if ((drv = parse_driver(k, v, "command")))
 		return parse_string(&drv->external, k, v);
 	if ((drv = parse_driver(k, v, "textconv")))
diff --git a/userdiff.h b/userdiff.h
index f29c18f..ba29457 100644
--- a/userdiff.h
+++ b/userdiff.h
@@ -14,8 +14,7 @@ struct userdiff_driver {
 	const char *textconv;
 };
 
-int userdiff_config_basic(const char *k, const char *v);
-int userdiff_config_porcelain(const char *k, const char *v);
+int userdiff_config(const char *k, const char *v);
 struct userdiff_driver *userdiff_find_by_name(const char *name);
 struct userdiff_driver *userdiff_find_by_path(const char *path);
 
-- 
1.6.0.3.518.gdb328.dirty

^ permalink raw reply related

* [PATCH 5/5] document the diff driver textconv feature
From: Jeff King @ 2008-10-24  2:56 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Johannes Sixt, Matthieu Moy
In-Reply-To: <20081024024631.GA20365@coredump.intra.peff.net>

This patch also changes the term "custom diff driver" to
"external diff driver"; now that there are more facets of a
"custom driver" than just external diffing, it makes sense
to refer to the configuration of "diff.foo.*" as the "foo
diff driver", with "diff.foo.command" as the "external
driver for foo".

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/gitattributes.txt |   66 +++++++++++++++++++++++++++++++--------
 1 files changed, 53 insertions(+), 13 deletions(-)

diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 2694559..314e2d3 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -213,10 +213,12 @@ with `crlf`, and then `ident` and fed to `filter`.
 Generating diff text
 ~~~~~~~~~~~~~~~~~~~~
 
-The attribute `diff` affects if 'git-diff' generates textual
-patch for the path or just says `Binary files differ`.  It also
-can affect what line is shown on the hunk header `@@ -k,l +n,m @@`
-line.
+The attribute `diff` affects how 'git' generates diffs for particular
+files. It can tell git whether to generate a textual patch for the path
+or to treat the path as a binary file.  It can also affect what line is
+shown on the hunk header `@@ -k,l +n,m @@` line, tell git to use an
+external command to generate the diff, or ask git to convert binary
+files to a text format before generating the diff.
 
 Set::
 
@@ -227,7 +229,8 @@ Set::
 Unset::
 
 	A path to which the `diff` attribute is unset will
-	generate `Binary files differ`.
+	generate `Binary files differ` (or a binary patch, if
+	binary patches are enabled).
 
 Unspecified::
 
@@ -238,21 +241,21 @@ Unspecified::
 
 String::
 
-	Diff is shown using the specified custom diff driver.
-	The driver program is given its input using the same
-	calling convention as used for GIT_EXTERNAL_DIFF
-	program.  This name is also used for custom hunk header
-	selection.
+	Diff is shown using the specified diff driver.  Each driver may
+	specify one or more options, as described in the following
+	section. The options for the diff driver "foo" are defined
+	by the configuration variables in the "diff.foo" section of the
+	git config file.
 
 
-Defining a custom diff driver
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+Defining an external diff driver
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 The definition of a diff driver is done in `gitconfig`, not
 `gitattributes` file, so strictly speaking this manual page is a
 wrong place to talk about it.  However...
 
-To define a custom diff driver `jcdiff`, add a section to your
+To define an external diff driver `jcdiff`, add a section to your
 `$GIT_DIR/config` file (or `$HOME/.gitconfig` file) like this:
 
 ----------------------------------------------------------------
@@ -328,6 +331,43 @@ patterns are available:
 - `tex` suitable for source code for LaTeX documents.
 
 
+Performing text diffs of binary files
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Sometimes it is desirable to see the diff of a text-converted
+version of some binary files. For example, a word processor
+document can be converted to an ASCII text representation, and
+the diff of the text shown. Even though this conversion loses
+some information, the resulting diff is useful for human
+viewing (but cannot be applied directly).
+
+The `textconv` config option is used to define a program for
+performing such a conversion. The program should take a single
+argument, the name of a file to convert, and produce the
+resulting text on stdout.
+
+For example, to show the diff of the exif information of a
+file instead of the binary information (assuming you have the
+exif tool installed):
+
+------------------------
+[diff "jpg"]
+	textconv = exif
+------------------------
+
+NOTE: The text conversion is generally a one-way conversion;
+in this example, we lose the actual image contents and focus
+just on the text data. This means that diffs generated by
+textconv are _not_ suitable for applying. For this reason,
+only `git diff` and the `git log` family of commands (i.e.,
+log, whatchanged, show) will perform text conversion. `git
+format-patch` will never generate this output. If you want to
+send somebody a text-converted diff of a binary file (e.g.,
+because it quickly conveys the changes you have made), you
+should generate it separately and send it as a comment _in
+addition to_ the usual binary diff that you might send.
+
+
 Performing a three-way merge
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-- 
1.6.0.3.518.gdb328.dirty

^ permalink raw reply related

* Re: git performance
From: Daniel Barkalow @ 2008-10-24  3:56 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Edward Ned Harvey, git
In-Reply-To: <20081024072412.6117@nanako3.lavabit.com>

On Fri, 24 Oct 2008, Nanako Shiraishi wrote:

> Quoting Daniel Barkalow <barkalow@iabervon.org>:
> 
> > On Wed, 22 Oct 2008, Edward Ned Harvey wrote:
> >
> >> Out of curiosity, what are they talking about, when they say "git is 
> >> fast?"  Just the fact that it's all local disk, or is there more to it 
> >> than that?  I could see - git would probably outperform perforce for 
> >> versioning of large files (let's say iso files) to benefit from 
> >> sustained local disk IO, while perforce would probably outperform 
> >> anything I can think of, operating on thousands of tiny files, because 
> >> it will never walk the tree. 
> >
> > It shouldn't be too hard to make git work like perforce with respect to 
> > walking the tree. git keeps an index of the stat() info it saw when it 
> > last looked at files, and only looks at the contents of files whose stat() 
> > info has changed. In order to have it work like perforce, it would just 
> > need to have a flag in the stat() info index for "don't even bother", 
> 
> Are you describing the "assume unchanged bit"?

Yes, but with the user write mode bit in the filesystem set to 
no-assume-unchanged, which is how Perforce users cope with it. I hadn't 
realized it had been implemented to get set on a per-file basis, rather 
than just as a global setting that caused it to not stat() anything except 
right when it was told to update.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: git archive
From: Nguyen Thai Ngoc Duy @ 2008-10-24  3:58 UTC (permalink / raw)
  To: Deskin Miller; +Cc: kenneth johansson, git
In-Reply-To: <20081023182103.GA8320@euler>

On 10/24/08, Deskin Miller <deskinm@umich.edu> wrote:
> On Thu, Oct 23, 2008 at 10:33:31PM +0700, Nguyen Thai Ngoc Duy wrote:
>  > On 10/22/08, Deskin Miller <deskinm@umich.edu> wrote:
>  > > On Wed, Oct 22, 2008 at 08:42:01AM +0000, kenneth johansson wrote:
>  > >  > I was going to make a tar of the latest stable linux kernel.
>  > >  > Done it before but now I got a strange problem.
>  > >  >
>  > >  > >git archive --format=tar v2.6.27.2
>  > >  > fatal: Not a valid object name
>  > >
>  > >
>  > > I had the same thing happen to me, while trying to make an archive of Git.
>  > >  Were you perchance working in a bare repository, as I was?  I spent some time
>  > >  looking at it and I think git archive sets up the environment in the wrong
>  > >  order, though of course I never finished a patch so I'm going from memory:
>  > >
>  > >  After looking at the code again, I think the issue is that git_config is called
>  > >  in builtin-archive.c:cmd_archive before setup_git_directory is called in
>  > >  archive.c:write_archive.  The former ends up setting GIT_DIR to be '.git' even
>  > >  if you're in a bare repository.  My coding skills weren't up to fixing it
>  > >  easily; moving setup_git_directory before git_config in builtin-archive caused
>  > >  last test of t5000 to fail: GIT_DIR=some/nonexistent/path git archive --list
>  > >  should still display the archive formats.
>  >
>  > The problem affects some other commands as well. I tried the following
>  > patch, ran "make test" and discovered "git mailinfo", "git
>  > verify-pack", "git hash-object" and "git unpack-file". A bandage patch
>  > is at the end of this mail. Solution is as Jeff suggested: call
>  > setup_git_directory_gently() early.
>
>
> Nice work.  The patches look like they're on the right track (to me at least).
>  I'm not sure though what you want to ultimately submit as a patch; I'd suggest
>  both, squashed into one, since the check seems like something we'd reasonably
>  want no matter what.

No, the patches are not in good shape and have not been tested well. I
just wanted to point out the problem in other commands. Ideally the
check in setup_git_env() should go along with discover_git_directory()
as part of git setup rework.

>
>  Few comments spread around below; also, can we see some testcases for
>  regression?  Or, does the first patch preclude the need for testcases?
>
>  Deskin Miller
>
>
>  > ---<---
>  > diff --git a/environment.c b/environment.c
>  > index 0693cd9..00ed640 100644
>  > --- a/environment.c
>  > +++ b/environment.c
>  > @@ -49,14 +49,18 @@ static char *work_tree;
>  >
>  >  static const char *git_dir;
>  >  static char *git_object_dir, *git_index_file, *git_refs_dir, *git_graft_file;
>  > +int git_dir_discovered;
>
>
> Should this be 'int git_dir_discovered = 0;' ?

It is initialized by default IIRC.

>  > Bandage patch:
>  >
>  > ---<---
>  > diff --git a/builtin-archive.c b/builtin-archive.c
>  > index 432ce2a..5ea0a12 100644
>  > --- a/builtin-archive.c
>  > +++ b/builtin-archive.c
>  > @@ -110,7 +110,9 @@ static const char *extract_remote_arg(int *ac,
>  > const char **av)
>  >  int cmd_archive(int argc, const char **argv, const char *prefix)
>  >  {
>  >       const char *remote = NULL;
>  > +     int nongit;
>  >
>  > +     prefix = setup_git_directory_gently(&nongit);
>
>
> Here and elsewhere, the 'nongit' variable isn't used.
>  setup_git_directory_gently can be passed a NULL pointer, why not do that?

Passing NULL to setup_git_directory_gently() tells it to die() if no
git repo can be found. If you pass a variable to it, it will set the
variable to 1 if no repo is found, 0 otherwise.

>  >       git_config(git_default_config, NULL);
>  >       while (1 < argc) {
>  >               if (!no_more_options && argv[1][0] == '-') {
>  > diff --git a/hash-object.c b/hash-object.c
>  > index 20937ff..a52b6be 100644
>  > --- a/hash-object.c
>  > +++ b/hash-object.c
>  > @@ -78,19 +78,20 @@ int main(int argc, const char **argv)
>  >       const char *prefix = NULL;
>  >       int prefix_length = -1;
>  >       const char *errstr = NULL;
>  > +     int nongit;
>  >
>  >       type = blob_type;
>  >
>  > -     git_config(git_default_config, NULL);
>  > -
>  >       argc = parse_options(argc, argv, hash_object_options, hash_object_usage, 0);
>  >
>  > -     if (write_object) {
>  > -             prefix = setup_git_directory();
>  > -             prefix_length = prefix ? strlen(prefix) : 0;
>  > -             if (vpath && prefix)
>  > -                     vpath = prefix_filename(prefix, prefix_length, vpath);
>  > -     }
>  > +     prefix = setup_git_directory_gently(&nongit);
>  > +     git_config(git_default_config, NULL);
>  > +     prefix_length = prefix ? strlen(prefix) : 0;
>  > +     if (vpath && prefix)
>  > +             vpath = prefix_filename(prefix, prefix_length, vpath);
>  > +
>  > +     if (write_object && nongit)
>  > +             die("Git repository required");
>
>
> I'd move this check up to just after setup_git_directory_gently.

Yeah, sounds reasonable.

-- 
Duy

^ permalink raw reply

* git-daemon and hook output
From: Joey Hess @ 2008-10-24  5:07 UTC (permalink / raw)
  To: git

[-- Attachment #1: Type: text/plain, Size: 852 bytes --]

According to githooks, standard output/error for the post-receive and
pre-receive hooks will be seen by the user. This is true if using ssh.
But if git-daemon has been configured to allow pushes, the user will
not see anything these hooks output, and only a not very helpful

 ! [remote rejected] master -> master (pre-receive hook declined)

I suppose that to fix this, git-daemon would have to select on stdout
and stderr and pipe them over the wire to the client. Apologies for not
even trying to write a patch to do that.

(I ran into this in a real-world application -- I'm implementing
anonymous pushes into ikiwiki, which are checked pre-receive to limit
them to changes that could be done via the web interface. So all my nice
error messages about why a commit is refused are not available, which is
a pity.)

-- 
see shy jo

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* [PATCH] git-daemon: set REMOTE_ADDR to client address
From: Joey Hess @ 2008-10-24  5:48 UTC (permalink / raw)
  To: git

This allows hooks like pre-receive to look at the client's IP
address.

Of course the IP address can't be used to get strong security;
git-daemon isn't the right thing to use if you need that. However,
basic IP address checking can be good enough in some situations.

REMOTE_ADDR is the same environment variable used to communicate the
client's address to CGI scripts.

Signed-off-by: Joey Hess <joey@kitenet.net>
---

Real world example: ikiwiki can use pre-receive to check that the pushed
changes are ones that anyone could make to the wiki's source via the web
interface, and thus safe to accept. It's useful to be able to ban IP
addresses from editing a wiki on the web, as a first line of defence to
guard against spammers etc. With this patch the same IP guards can be
applied to changes pushed in via git-daemon.

 Documentation/git-daemon.txt |    9 +++++++++
 daemon.c                     |    4 ++++
 2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
index b08a08c..f1a570a 100644
--- a/Documentation/git-daemon.txt
+++ b/Documentation/git-daemon.txt
@@ -270,6 +270,15 @@ selectively enable/disable services per repository::
 ----------------------------------------------------------------
 
 
+ENVIRONMENT
+-----------
+'git-daemon' will set REMOTE_ADDR to the IP address of the client
+that connected to it, if the IP address is available. REMOTE_ADDR will
+be available in the environment of hooks called when
+services are performed.
+
+
+
 Author
 ------
 Written by Linus Torvalds <torvalds@osdl.org>, YOSHIFUJI Hideaki
diff --git a/daemon.c b/daemon.c
index 3e5582d..b9ba44c 100644
--- a/daemon.c
+++ b/daemon.c
@@ -537,6 +537,10 @@ static int execute(struct sockaddr *addr)
 #endif
 		}
 		loginfo("Connection from %s:%d", addrbuf, port);
+		setenv("REMOTE_ADDR", addrbuf, 1);
+	}
+	else {
+		unsetenv("REMOTE_ADDR");
 	}
 
 	alarm(init_timeout ? init_timeout : timeout);

-- 
1.5.6.5

^ permalink raw reply related

* Re: [PATCH] Only update the cygwin-related configuration during state auto-setup
From: Junio C Hamano @ 2008-10-24  5:54 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Mark Levedahl, spearce, dpotapov, git
In-Reply-To: <81b0412b0810230607sfea05ddm62bd03f837fc922e@mail.gmail.com>

"Alex Riesen" <raa.lkml@gmail.com> writes:

> Otherwise the other global settings which were already read and set up will
> be overwritten ...

This is the answer to the question I asked in:

 http://thread.gmane.org/gmane.comp.version-control.git/97986/focus=98066

Perhaps we should use a separate variable as the original patch did, in:

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

How about doing it like this instead?


diff --git i/compat/cygwin.c w/compat/cygwin.c
index f196753..ebac148 100644
--- i/compat/cygwin.c
+++ w/compat/cygwin.c
@@ -91,26 +91,32 @@ static int cygwin_stat(const char *path, struct stat *buf)
  * functions should be used. The choice is determined by core.ignorecygwinfstricks.
  * Reading this option is not always possible immediately as git_dir may be
  * not be set yet. So until it is set, use cygwin lstat/stat functions.
- * However, if the trust_executable_bit is set, we must use the Cygwin posix
+ * However, if core.filemode is set, we must use the Cygwin posix
  * stat/lstat as the Windows stat fuctions do not determine posix filemode.
+ *
+ * Note that git_cygwin_config() does NOT call git_default_config() and this
+ * is deliberate.  Many commands read from config to establish initial
+ * values in variables and later tweak them from elsewhere (e.g. command line).
+ * init_stat() is called lazily on demand, typically much late in the program,
+ * and calling git_default_config() from here would break such variables.
  */
 static int native_stat = 1;
-extern int trust_executable_bit;
+static int core_filemode;
 
 static int git_cygwin_config(const char *var, const char *value, void *cb)
 {
-	if (!strcmp(var, "core.ignorecygwinfstricks")) {
+	if (!strcmp(var, "core.ignorecygwinfstricks"))
 		native_stat = git_config_bool(var, value);
-		return 0;
-	}
-	return git_default_config(var, value, cb);
+	else if (!strcmp(var, "core.filemode"))
+		core_filemode = git_config_bool(var, value);
+	return 0;
 }
 
 static int init_stat(void)
 {
 	if (have_git_dir()) {
 		git_config(git_cygwin_config, NULL);
-		if (!trust_executable_bit && native_stat) {
+		if (!core_filemode && native_stat) {
 			cygwin_stat_fn = cygwin_stat;
 			cygwin_lstat_fn = cygwin_lstat;
 		} else {

^ permalink raw reply related

* Re: [RFC] Zit (v2): the git-based single file content tracker
From: david @ 2008-10-24  6:21 UTC (permalink / raw)
  To: Giuseppe Bilotta; +Cc: git
In-Reply-To: <gdqbta$rhe$1@ger.gmane.org>

On Thu, 23 Oct 2008, Giuseppe Bilotta wrote:

> I decided to give the simpler GIT_DIR approach another go.
>
> The reworked Zit ( git://git.oblomov.eu/zit ) works by creating
> .file.git/ to track file's history. .file.git/info/excludes is
> initialized to the very strong '*' pattern to ensure that things such
> as git status etc only consider the actually tracked file.
>
> The obvious advantage over the previous implementation is that we
> don't rely on fragile and non-portable hardlinks. The disadvantage
> is that something really bad can happen if a command fails to obey
> GIT_DIR or GIT_WORK_TREE correctly.

this is a very interesting approach.

the thought that hit me as I finidhed reading this thread is that we 
are very close to having the full continum of file/repository combinations

1. everything in the dir is part of one repository (the normal git case)

2. some of all of the individual files in a dir is it's own repository 
(the zit case)

3. the in-between case where you can have multiple repositories that can 
have multiple files in them.

how hard would it be to extend zit to support case #3?

offhand I can see it complicating the task of figuing out which repository 
to use for a file, but what else?

David Lang


> Command delegation is made a little smarter:
>
> zit somecommand file [args...]
>
> gets delegated to
>
> git somecommand [args...]
>
> with GIT_DIR=.file.git and GIT_WORK_TREE="`pwd`", which works
> surprisingly well. To prevent stupid expressions such as zit add file
> file or zit commit file file, add and commit put the filename back at
> the end of the parameter list.
>
> Commands that seem to work correctly so far are init, add, log,
> status, diff, remote, push, pull, and even rebase -i.
>
> Commands that definitely need some work are rm (should it just remove
> the .file.git/ dir?) and mv (hairy: we would need to rename .file.git
> to .newname.git too, but rollbacks are likely to break things).
>
> The only new command introduced by zit is zit list, which lists all
> zit-tracked files in the current directory, currently in a very
> braindead way (e.g. I'd like it to display the proper status, such as
> C M or whatever; suggestions welcome).
>
> On the TODO list is also some smart way to guess which file we're
> talking about when no file is specified. Basically, the idea is to
> check if there's only one tracked file, or only one changed tracked
> file, and allow a missing file option in that case.
>
> As usual, comments suggestions and critiques welcome.
>
>

^ permalink raw reply

* Re: [RFC] Zit: the git-based single file content tracker
From: Giuseppe Bilotta @ 2008-10-24  6:55 UTC (permalink / raw)
  To: Jean-Luc Herren; +Cc: git
In-Reply-To: <4901077A.7050904@gmx.ch>

On Fri, Oct 24, 2008 at 1:23 AM, Jean-Luc Herren <jlh@gmx.ch> wrote:
> Hi!
>
> Giuseppe Bilotta wrote:
>> So today I decided to start hacking at a git-based but file-oriented
>> content tracker, which I decided to name Zit.
>
> This sounds great and would seem very useful to manage my ~/bin/
> directory which contains a set of unrelated one-file-tools that
> evolve over time.  I haven't played with it yet though.

Nice to see I wasn't the only one with such a need 8-)

>> when you choose to start tracking a file with Zit [...]
>> Zit will create a directory .zit.file to hold a git repository
>
> If you have many files you want to track in a single directory
> (like ~/bin/), all those additional directories will quickly feel
> like clutter.  If you track every file, it will even double the
> number of things you see with an "ls -a".

Ah, good point, I hadn't thought about that.

> If you decide against a shared repository, maybe you want to
> consider to not use ".zit.file/", but ".zit/file/" as the
> repository?  This would reduce the clutter to a single directory,
> just like with ".git".  And moving files around wouldn't be that
> much complicated.

Right. I'll give that a shot.

-- 
Giuseppe "Oblomov" Bilotta

^ permalink raw reply

* Re: [PATCH 3/4] diff: introduce diff.<driver>.binary
From: Johannes Sixt @ 2008-10-24  7:02 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Junio C Hamano, Matthieu Moy
In-Reply-To: <20081024024631.GA20365@coredump.intra.peff.net>

Thank you for picking up the topic again and continuing on it. Your series
works nicely here, even without setting diff.*.binary.

-- Hannes

^ 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