Git development
 help / color / mirror / Atom feed
* Re: Error: "You have some suspicious patch lines"
From: Jakub Narebski @ 2008-07-22  9:13 UTC (permalink / raw)
  To: Ben Aurel; +Cc: git
In-Reply-To: <4885895C.5050108@gmail.com>

Ben Aurel <ben.aurel@gmail.com> writes:

> I working on mac os x 10.5.4 (intel) with git version 1.5.5.3 and I
> always get this message for most of my perl scripts and also for
> "Makefile.pre" files:
> 
> ----------- Message ---------------
> * You have some suspicious patch lines:
> *
> * In src/scripts/trunk/3rdparty/file_sanity.pl
> * trailing whitespace (line 52)
> ...
> ------------------------------------------

> The question now is: Is it really necessary to edit the git script
> everytime? Is there a urgent reason why git refuses to commit because
> of "suspicious" lines? Is it really necessary?

.git/hooks/pre-commit is example hook which helps to keep Coding Style,
and prevents from accidentally comitting nonresolved file-level merge
conflict (file with conflict markers).

If you want to skip running this hook once (or once upon a time), you
can use '-n'/'--no-verify' option to "git commit".  Or you can turn this
example hook off, either by removing execute permission from it, by
removing it alltogether (you can still find it in templates, usually at
/usr/share/git-core/templates/hooks/pre-commit), or rename it adding for
example '.sample' or '.nonactive' suffix.

This hook should not be turned on by default, but if your filesystem is
executing bit challenged it could be turned on at repository creation
time unintentionally.  Newer version of git use '.sample' suffix (for
example pre-commit.sample) instead of relying on not always reliable
execute bit being unset.

HTH
-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* How to find the first commit belonging to any branch
From: Kristian Amlie @ 2008-07-22  9:08 UTC (permalink / raw)
  To: git

Hi all!

I have a question about git: I have one commit sha1, and I would like to 
know the nearest commit that appears in *any* other branch. The sha1 
that I have does not belong to any branch.

The obvious thing to do would be to make a for loop and iterate over 
existing branches while calling git merge-base, but I'm wondering if 
there's a more clever method.

Regards
Kristian Amlie

^ permalink raw reply

* Re: [PATCH] builtin-merge-recursive.c: make merge_recursive() static
From: Junio C Hamano @ 2008-07-22  8:47 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Nanako Shiraishi, git
In-Reply-To: <alpine.DEB.1.00.0807201403070.3305@eeepc-johanness>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Of course, we could apply this patch now, and revert it later, increasing 
> your commit count in the process :-)

Heh, don't tempt people, especially when we would have an interesting
set of tools for statistics just around the corner ;-)

^ permalink raw reply

* Re: [PATCH 2/2] spawn pager via run_command interface
From: Johannes Sixt @ 2008-07-22  8:29 UTC (permalink / raw)
  To: Jeff King; +Cc: Mike Hommey, Junio C Hamano, git, David Bremner
In-Reply-To: <20080722075035.GC3999@sigill.intra.peff.net>

Jeff King schrieb:
> On Tue, Jul 22, 2008 at 09:48:09AM +0200, Johannes Sixt wrote:
> 
>> BTW, you could remove the #ifndef __MINGW32__ around both the definition
>> and the use of pager_preexec. We have everything on mingw to compile and
>> link this function.
> 
> Ah, OK. I left it around the function because I was worried about fd_set
> needing some magic for compilation.

I do not expect you to know the intricacies of the mingw port. Hence,
staying on the safe side, like you did, is of course highly appreciated.

> However, it still won't be _used_ on Windows, because there is no
> opportunity to use the pre-exec callback (it is silently ignored).

That's fine.

-- Hannes

^ permalink raw reply

* Re: [PATCH] builtin-merge: give a proper error message for invalid strategies in config
From: Junio C Hamano @ 2008-07-22  8:24 UTC (permalink / raw)
  To: Miklos Vajna; +Cc: git
In-Reply-To: <20080722073912.GN32057@genesis.frugalware.org>

Miklos Vajna <vmiklos@frugalware.org> writes:

> ... I
> would like to send a patch that changes the config parsing as well, so
> that pull.twohead "foo bar" would be invalid, and the user would have to
> have two pull.twohead entries: one for foo and one for bar.

Don't.  pull.* has always been defined as "list of strategies", and -s has
always been defined to take "a" strategy.

IOW, you don't need to break anything further.

^ permalink raw reply

* Re: [PATCH] Fix update-index --refresh for submodules if stat(2) returns st_size 0
From: Junio C Hamano @ 2008-07-22  8:07 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Johannes Schindelin, git
In-Reply-To: <20080721194322.GA4013@blimp.local>

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

> Johannes Schindelin, Mon, Jul 21, 2008 20:20:43 +0200:
>> Hi,
>> 
>> On Mon, 21 Jul 2008, Alex Riesen wrote:
>> 
>> > For example - Cygwin.
>> 
>> Please enhance: your oneline is too long, and your commit message body too 
>> short.
>
> Well, I'm really not sure. I just found this difference between linux
> and cygwin (st_stat is 0 for dirs on cygwin). Than I noticed that the
> routine where I made the change explicitely checks for st_size not
> being 0. I must admit I can't make much out of comment, and hope this
> discussion will help to clear the check up.

The cached stat information in the index is used to speed up comparison
between "the last staged data" and what is in the working tree.
ie_match_stat() compares ce_xxx fields with the result from lstat(2) we
just did, and if there are differences, we take it as a sign that what's
in the working tree is different from what we saw when we updated the
index entry.

But there is a twist.

Ordinarily, when an entry enteres the index, the hash of the blob contents
goes along with the lstat(2) information taken from the file that supplied
the contents.  However there are some cases we populate the index without
lstat(2).  update-index --cacheinfo, update-index --index-info are two
examples, and when they add index entries, they leave ce_size field to
zero.  ie_match_stat() will compare that zero ce_size with the size
information obtained from the working tree, and declare (falsely) that
"what's in the working tree is different -- it can never match, and there
is no point trying to re-index to see if they actually match", even though
the reason ce_size is zero is *not* because we observed the size of the
working tree file *was* zero when we indexed it the last time (it is zero
merely because we haven't looked at it yet).  The ce_modified_check_fs()
call is there to deal with this "we cannot trust the ce_xxx fields" case.

I however have to wonder if you also need to touch the end of
ce_match_stat_basic() that checks for zero sized cache entry.

^ permalink raw reply

* Re: Git Documentation
From: david @ 2008-07-22  7:56 UTC (permalink / raw)
  To: Johan Herland; +Cc: Scott Chacon, git
In-Reply-To: <200807220917.57363.johan@herland.net>

On Tue, 22 Jul 2008, Johan Herland wrote:

> On Tuesday 22 July 2008, Scott Chacon wrote:
>> If anyone has any tips on how they think git should be taught, issues
>> they are asked a lot, problems newbies tend to have, something they
>> wish there were a screencast for or was better documented, etc -
>> please do contact me so I can incorporate it.
>
> You should at least take a look at this thread:
>
> http://thread.gmane.org/gmane.comp.version-control.git/88698
>
> (even though it goes off-topic after a while...)
>
>> If anyone has any tips on how they think git should be taught...
>
> It seems there are primarily two ways to teach Git:
>
> 1. Top-down: Start with simple use cases and commands. Teach people a
> minimal, but necessary set of porcelain commands to get them started. Stay
> _far_ away from plumbing commands and most of the command options.
>
> 2. Bottom-up: Start with how Git structures the data. Talk about blobs,
> trees, commits, refs, how everything is connected, and how various Git
> commands query and manipulate this structure. This _may_ involve a fair
> amount of plumbing commands, especially when discovering how the more
> complicated high-level commands manipulate the structure.
>
> Some people seem to prefer the first approach, other people prefer the other
> approach. Both paths lead to enlightenment ;). In many cases a bit of both
> may be useful. HOWEVER, I think it is _very_ important to keep in mind that
> these are two _different_ approaches, and the contexts in which they are
> taught should be kept separate. I would almost suggest splitting your
> website down the middle and make the difference between top-down and
> bottom-up immediately visible with, say, a different background color, or
> something else that immediately tells the user what "track" they are
> following...

possibly a combination of the two?

under the covers the git data-structures are pretty simple and explaining 
them (and the minimal tools to manipulate them) isn't that bad.

what gets ugly is when you then try to use the plumbing to do the 
non-trivial things.

so how about an optional 'under the covers' primer, covering just the 
trivial plumbing, then the high-level minimal introduction with a link on 
each of the commands as they are introduced (so that a person can dig into 
deeper detail if they want to, possibly including 'up until version X 
this command was implemented by the following script'), followed by links 
to sample work-flows and a full dive into the plumbing (because at this 
point the person should know enough to get by, now they need reference 
material and examples more then a tutorial).

ideally this would let people dive as deep as they are comfortable with, or 
skim the explanation for the functionality

I think one reason the 'plumbing first' approach gets a bad rap is that 
it's so easy to get caught up into how clever you can get with the 
plumbing. it's like teaching someone programming by spending a day 
introducing them to concepts and language syntax, and then giving them the 
entries in the obfuscated C contests as examples of how someone can use 
them to get work done, but skipping any mention of libc or other standard 
libraries.

on the other hand, teaching only porcelain is like teaching them <insert 
high-level *th generation buzzword language> without teaching any concept 
of what they computer is doing under the covers, they can work, and even 
get useful work done, but they will be limited on how effective they can 
be.


you can't be a great programmer until you can understand both levels, the 
under-the-covers 'plumbing' and the high level libraries of the 
'porcelain', trying to ignore either will limit you.

David Lang

^ permalink raw reply

* Re: git status in clean working dir
From: Johannes Sixt @ 2008-07-22  7:54 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git, David Bremner
In-Reply-To: <20080722074632.GA3999@sigill.intra.peff.net>

Jeff King schrieb:
> On Tue, Jul 22, 2008 at 09:34:45AM +0200, Johannes Sixt wrote:
> 
>>> -		{ "diff-files", cmd_diff_files, RUN_SETUP },
>>> +		{ "diff-files", cmd_diff_files, RUN_SETUP | FORBID_PAGER },
>> Every now and then I want to use 'git -p diff-files', and I think that is
>> a valid use-case. But your suggested patch seems to forbid the pager even
>> in this case. :-(
> 
> Actually, it doesn't. If you read earlier in the message, this applies
> only to pager.* config. That being said, I think Junio's ultimate goal
> was to not allow stupid people to accidentally set the pager on
> plumbing, at the expense of any smart people who might want to do it for
> a good reason.
> 
> Though I have to wonder why "git diff --raw" is not enough for you.

Usually, I use plumbing with a pager while I'm writing or debugging a
script, and I'm studying its output. So, no, I'm not interested in "git
diff --raw". ;)

-- Hannes

^ permalink raw reply

* Re: [PATCH 2/2] spawn pager via run_command interface
From: Jeff King @ 2008-07-22  7:50 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Mike Hommey, Junio C Hamano, git, David Bremner
In-Reply-To: <488590B9.1080804@viscovery.net>

On Tue, Jul 22, 2008 at 09:48:09AM +0200, Johannes Sixt wrote:

> BTW, you could remove the #ifndef __MINGW32__ around both the definition
> and the use of pager_preexec. We have everything on mingw to compile and
> link this function.

Ah, OK. I left it around the function because I was worried about fd_set
needing some magic for compilation.

However, it still won't be _used_ on Windows, because there is no
opportunity to use the pre-exec callback (it is silently ignored).

-Peff

^ permalink raw reply

* Re: [PATCH 2/2] spawn pager via run_command interface
From: Jeff King @ 2008-07-22  7:49 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: Mike Hommey, Junio C Hamano, git, David Bremner
In-Reply-To: <20080722073108.GA9714@artemis.madism.org>

On Tue, Jul 22, 2008 at 09:31:08AM +0200, Pierre Habouzit wrote:

> > I couldn't recall if this initializer style is portable enough for us.
> > It was already there wrapped in ifdefs, but perhaps it was only ok
> > because the mingw version always uses the same compiler?
> 
>   it's not, I asked long time ago, and it's C99, which mingw supports
> indeed, and we don't want to require a C99 compiler.

OK, then this should be squashed in.

diff --git a/pager.c b/pager.c
index 7743742..aa0966c 100644
--- a/pager.c
+++ b/pager.c
@@ -26,13 +26,8 @@ static void pager_preexec(void)
 #endif
 
 static const char *pager_argv[] = { "sh", "-c", NULL, NULL };
-static struct child_process pager_process = {
-	.argv = pager_argv,
-	.in = -1,
-#ifndef __MINGW32__
-	.preexec_cb = pager_preexec,
-#endif
-};
+static struct child_process pager_process;
+
 static void wait_for_pager(void)
 {
 	fflush(stdout);
@@ -65,6 +60,11 @@ void setup_pager(void)
 
 	/* spawn the pager */
 	pager_argv[2] = pager;
+	pager_process.argv = pager_argv;
+	pager_process.in = -1;
+#ifndef __MINGW32__
+	pager_process.preexec_cb = pager_preexec;
+#endif
 	if (start_command(&pager_process))
 		return;
 

^ permalink raw reply related

* Re: [PATCH 2/2] spawn pager via run_command interface
From: Johannes Sixt @ 2008-07-22  7:48 UTC (permalink / raw)
  To: Jeff King; +Cc: Mike Hommey, Junio C Hamano, git, David Bremner
In-Reply-To: <20080722071630.GA3669@sigill.intra.peff.net>

Jeff King schrieb:
> On Tue, Jul 22, 2008 at 03:14:12AM -0400, Jeff King wrote:
> 
>>  static struct child_process pager_process = {
>>  	.argv = pager_argv,
>> -	.in = -1
>> +	.in = -1,
>> +#ifndef __MINGW32__
>> +	.preexec_cb = pager_preexec,
>> +#endif
> 
> I couldn't recall if this initializer style is portable enough for us.
> It was already there wrapped in ifdefs, but perhaps it was only ok
> because the mingw version always uses the same compiler?

Yes, that's because on mingw we know that we use gcc. This really must be
changed for portability.

BTW, you could remove the #ifndef __MINGW32__ around both the definition
and the use of pager_preexec. We have everything on mingw to compile and
link this function.

-- Hannes

^ permalink raw reply

* Re: git status in clean working dir
From: Jeff King @ 2008-07-22  7:46 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, git, David Bremner
In-Reply-To: <48858D95.7060409@viscovery.net>

On Tue, Jul 22, 2008 at 09:34:45AM +0200, Johannes Sixt wrote:

> > -		{ "diff-files", cmd_diff_files, RUN_SETUP },
> > +		{ "diff-files", cmd_diff_files, RUN_SETUP | FORBID_PAGER },
> 
> Every now and then I want to use 'git -p diff-files', and I think that is
> a valid use-case. But your suggested patch seems to forbid the pager even
> in this case. :-(

Actually, it doesn't. If you read earlier in the message, this applies
only to pager.* config. That being said, I think Junio's ultimate goal
was to not allow stupid people to accidentally set the pager on
plumbing, at the expense of any smart people who might want to do it for
a good reason.

Though I have to wonder why "git diff --raw" is not enough for you.

At any rate, I think this isn't the right route. We haven't actually
seen evidence of somebody setting pager.diff-files and complaining about
breakage. We have seen people complaining about the lost exit code from
"git status", which is not something that would be on the "forbid" list
anyway. The real solution is to preserve the exit code when spawning the
pager, which I just posted a patch for.

-Peff

^ permalink raw reply

* Re: [PATCH 2/2] git-svn: make use of svn auto-props optional
From: Paul Talacko @ 2008-07-22  7:44 UTC (permalink / raw)
  To: brad.king; +Cc: git

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

Hello Brad,

I submitted a patch for this, including a test file a few days ago.

My recommendation was to follow the practice as set out by the svn command line tool, which was that git svn respect auto-props in the config file unless specifically overridden by --auto-props or --no-auto-props.

My patch is attached.


      __________________________________________________________
Not happy with your email address?.
Get the one you really want - millions of new email addresses available now at Yahoo! http://uk.docs.yahoo.com/ymail/new.html

[-- Attachment #2: diff --]
[-- Type: application/octet-stream, Size: 8614 bytes --]

diff --git a/git-svn.perl b/git-svn.perl
index a366c89..df06220 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -128,6 +128,8 @@ my %cmd = (
 			  'dry-run|n' => \$_dry_run,
 			  'fetch-all|all' => \$_fetch_all,
 			  'no-rebase' => \$_no_rebase,
+                          'auto-props' => \$SVN::Git::Editor::_auto_props,
+                          'no-auto-props' => \$SVN::Git::Editor::_no_auto_props,
 			%cmt_opts, %fc_opts } ],
 	'set-tree' => [ \&cmd_set_tree,
 	                "Set an SVN repository to a git tree-ish",
@@ -448,8 +450,8 @@ sub cmd_dcommit {
 			                log => get_commit_entry($d)->{log},
 			                ra => Git::SVN::Ra->new($gs->full_url),
 			                config => SVN::Core::config_get_config(
-			                        $Git::SVN::Ra::config_dir
-			                ),
+				                $Git::SVN::Ra::config_dir
+					),
 			                tree_a => "$d~1",
 			                tree_b => $d,
 			                editor_cb => sub {
@@ -3276,7 +3278,7 @@ sub close_edit {
 }
 
 package SVN::Git::Editor;
-use vars qw/@ISA $_rmdir $_cp_similarity $_find_copies_harder $_rename_limit/;
+use vars qw/@ISA $_rmdir $_cp_similarity $_find_copies_harder $_rename_limit $_auto_props $_no_auto_props/;
 use strict;
 use warnings;
 use Carp qw/croak/;
@@ -3309,6 +3311,8 @@ sub new {
 	$self->{rm} = { };
 	$self->{path_prefix} = length $self->{svn_path} ?
 	                       "$self->{svn_path}/" : '';
+	$self->{config} = $opts->{ra}->{config};
+        croak "--auto-props and --no-auto-props are mutually exclusive." if $_auto_props && $_no_auto_props;
 	return $self;
 }
 
@@ -3497,6 +3501,86 @@ sub ensure_path {
 	return $bat->{$c};
 }
 
+sub apply_properties {
+    my ( $self, $fbat, $m ) = @_;
+    my $config       = $self->{config}->{config}; 
+    my $svn_auto_prop = {};
+    return if $_no_auto_props;
+    return if ( ! $_auto_props ) && ( ! $config->get_bool ('miscellany', 'enable-auto-props', 0) );
+
+    my $file = $m->{ file_b };
+
+    $config->enumerate(
+        'auto-props',
+        sub {
+            $svn_auto_prop->{ compile_apr_fnmatch( $_[0] ) } = $_[1];
+            1;
+        }
+    );
+    my ( $filebase ) = File::Basename::fileparse( $file );
+	while (my ($pattern, $value) = each %$svn_auto_prop ) {
+	    next unless $filebase =~ m/$pattern/;
+	    for (split (/\s*;\s*/, $value)) {
+		my ($propname, $propvalue) = split (/\s*=\s*/, $_, 2);
+		$self->change_file_prop($fbat, $propname, $propvalue); 
+	    }
+	}
+
+}
+
+
+## Thanks to SVK::XD and the folks Best Practical Solutions, who in
+## turn based this on Barrie Slaymaker's Regexp::Shellish
+sub compile_apr_fnmatch {
+    my $re = shift;
+
+    $re =~ s@
+             (  \\.
+             |  \[                       # character class
+                   [!^]?                 # maybe negation (^ and ! are both supported)
+                   (?: (?:\\.|[^\\\]])   # one item
+                     (?: -               # possibly followed by a dash and another
+                       (?:\\.|[^\\\]]))? # item
+                   )*                    # 0 or more entries (zero case will be checked specially below)
+                (\]?)                    # if this ] doesn't match, that means we fell off end of string!
+             |  .
+            )
+             @
+               if ( $1 eq '?' ) {
+                   '.' ;
+               } elsif ( $1 eq '*' ) {
+                   '.*' ;
+               } elsif ( substr($1, 0, 1) eq '[') {
+                   if ($1 eq '[]') { # should never match
+                       '[^\s\S]';
+                   } elsif ($1 eq '[!]' or $1 eq '[^]') { # 0-length match
+                       '';
+                   } else {
+                       my $temp = $1;
+                       my $failed = $2 eq '';
+                       if ($failed) {
+                           '[^\s\S]';
+                       } else {
+                           $temp =~ s/(\\.|.)/$1 eq '-' ? '-' : quotemeta(substr($1, -1))/ges;
+                           # the previous step puts in backslashes at beginning and end; remove them
+                           $temp =~ s/^\\\[/[/;
+                           $temp =~ s/\\\]$/]/;
+                           # if it started with [^ or [!, it now starts with [\^ or [\!; fix.
+                           $temp =~ s/^\[     # literal [
+                                       \\     # literal backslash
+                                       [!^]   # literal ! or ^
+                                     /[^/x;
+                           $temp;
+                       }
+                   }
+               } else {
+                   quotemeta(substr( $1, -1 ) ); # ie, either quote it, or if it's \x, quote x
+               }
+    @gexs ;
+
+    return qr/\A$re\Z/s;
+}
+
 sub A {
 	my ($self, $m) = @_;
 	my ($dir, $file) = split_path($m->{file_b});
@@ -3505,6 +3589,7 @@ sub A {
 					undef, -1);
 	print "\tA\t$m->{file_b}\n" unless $::_q;
 	$self->chg_file($fbat, $m);
+	$self->apply_properties( $fbat, $m );
 	$self->close_file($fbat,undef,$self->{pool});
 }
 
diff --git a/t/t9124-git-svn-autoprops.sh b/t/t9124-git-svn-autoprops.sh
new file mode 100644
index 0000000..ed78c2d
--- /dev/null
+++ b/t/t9124-git-svn-autoprops.sh
@@ -0,0 +1,123 @@
+#!/bin/sh
+#
+
+
+
+test_description='git-svn dcommit sets autoprops on files'
+
+. ./lib-git-svn.sh
+
+test_expect_success 'make svn repo' '
+    mkdir import &&
+    cd import &&
+    echo first > firstfile &&
+    svn import -m "Import for autoprops test" . "$svnrepo" > /dev/null &&
+    cd ..  &&
+    git svn init "$svnrepo" &&
+    git svn fetch
+'
+
+
+mkdir config
+cat > config/config <<EOF 
+[miscellany]
+enable-auto-props = yes
+[auto-props]
+*pm =  file-type = perl
+*html = svn:mime-type = text/html; encoding = special
+*bar = private = thingy
+EOF
+
+
+
+test_expect_success 'set svn properties on files' '
+        cd "$gittestrepo" &&
+        echo "blah" > a.pm &&
+        echo "foo" > b.html &&
+        echo "data" > foobar &&
+        git add a.pm b.html foobar &&
+        git commit -m files &&
+        git svn dcommit --config-dir=config
+        '
+
+test_expect_success 'export our properties to an svn repo' '
+
+        mkdir testsvnrepo &&
+        cd testsvnrepo &&
+        svn checkout "$svnrepo" &&
+        cd svnrepo
+        '
+
+test_expect_success 'test properties' '
+        test perl = `svn propget file-type a.pm` &&
+        test thingy = `svn propget private foobar` &&
+        test text/html = `svn propget svn:mime-type b.html` &&
+        test special = `svn propget encoding b.html`
+
+        '
+
+cd ../..
+
+test_expect_success 'no-props overrides config file' '
+        touch overriden-b.html &&
+        git add overriden-b.html &&
+        git commit -m "overriden-b" &&
+        git svn dcommit --no-auto-props --config-dir=config &&
+        cd testsvnrepo &&
+        svn checkout "$svnrepo" &&
+        cd svnrepo &&
+        test -z `svn propget file-type overriden-b.html`
+'
+
+cd ../..
+
+cat > config/config <<EOF 
+[miscellany]
+enable-auto-props = no
+[auto-props]
+*pm =  file-type = perl
+*html = svn:mime-type = text/html; encoding = special
+*bar = private = thingy
+EOF
+
+
+test_expect_success 'test when enable-auto-props is no' '
+        echo "blah" > a_no_props.pm &&
+        echo "foo" > b_no_props.html &&
+        echo "data" > foobar_no_props &&
+        chmod +x foobar_no_props &&
+        git add a_no_props.pm b_no_props.html foobar_no_props &&
+        git commit -m "No props files" &&
+        git svn dcommit --config-dir=config &&
+        cd testsvnrepo &&
+        svn checkout "$svnrepo"  &&
+        cd svnrepo &&
+        test -z `svn propget file-type a_no_props.pm` &&
+        test -z `svn propget private foobar_no_props`  &&
+        test -z `svn propget svn:mime-type b_no_props.html` &&
+        test -z `svn propget encoding b_no_props.html`
+        '
+
+cd ../..
+
+test_expect_success 'auto-props overrides config file' '
+        touch overriden-auto.pm &&
+        git add overriden-auto.pm &&
+        git commit -m "overriden-auto" &&
+        git svn dcommit --auto-props --config-dir=config &&
+        cd testsvnrepo &&
+        svn checkout "$svnrepo" &&
+        cd svnrepo &&
+        test perl = `svn propget file-type overriden-auto.pm`
+'
+cd ../..
+
+test_expect_success 'auto-props and no-auto-props are exclusive' '
+        touch afile &&
+        git add afile &&
+        git commit -m afile &&
+        test_must_fail git svn dcommit --auto-props --no-auto-props
+'
+
+test_done
+

^ permalink raw reply related

* Re: [PATCH] builtin-merge: give a proper error message for invalid strategies in config
From: Miklos Vajna @ 2008-07-22  7:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr69mpl5v.fsf@gitster.siamese.dyndns.org>

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

On Mon, Jul 21, 2008 at 10:01:16PM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> Doesn't this make "git merge -s 'recursive resolve'" to misbehave?

Depends on what do we expect it to do. ;-)

My patch unified the handling of pull.twohead / -s option, so it
(actually unintentionally) made -s accepting multiple strategies as a
space separated list. Given that we already accept multiple strategies
as a space separated list in pull.twohead (and we do _not_ use multiple
pull.twohead entries) I think my patch is more logical.

Though, there was already a thread about how should we specify multiple
strategies on the commandline; and you suggested in

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

to use -s strategy1 -s strategy2.

In that case, I think your patch is better, and once it hits git.git, I
would like to send a patch that changes the config parsing as well, so
that pull.twohead "foo bar" would be invalid, and the user would have to
have two pull.twohead entries: one for foo and one for bar.

Does this sound reasonable?

Thanks.

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

^ permalink raw reply

* Re: git status in clean working dir
From: Johannes Sixt @ 2008-07-22  7:34 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, git, David Bremner
In-Reply-To: <20080722044157.GA20787@sigill.intra.peff.net>

Jeff King schrieb:
> @@ -231,6 +232,8 @@ static int run_command(struct cmd_struct *p, int argc, const char **argv)
>  		use_pager = check_pager_config(p->cmd);
>  	if (use_pager == -1 && p->option & USE_PAGER)
>  		use_pager = 1;
> +	if (use_pager ==  1 && p->option & FORBID_PAGER)
> +		use_pager = 0;
>  	commit_pager_choice();
>  
>  	if (p->option & NEED_WORK_TREE)
> @@ -286,7 +289,7 @@ static void handle_internal_command(int argc, const char **argv)
>  		{ "count-objects", cmd_count_objects, RUN_SETUP },
>  		{ "describe", cmd_describe, RUN_SETUP },
>  		{ "diff", cmd_diff },
> -		{ "diff-files", cmd_diff_files, RUN_SETUP },
> +		{ "diff-files", cmd_diff_files, RUN_SETUP | FORBID_PAGER },

Every now and then I want to use 'git -p diff-files', and I think that is
a valid use-case. But your suggested patch seems to forbid the pager even
in this case. :-(

-- Hannes

^ permalink raw reply

* Re: [PATCH 2/2] spawn pager via run_command interface
From: Pierre Habouzit @ 2008-07-22  7:31 UTC (permalink / raw)
  To: Jeff King; +Cc: Mike Hommey, Junio C Hamano, git, David Bremner
In-Reply-To: <20080722071630.GA3669@sigill.intra.peff.net>

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

On Tue, Jul 22, 2008 at 07:16:30AM +0000, Jeff King wrote:
> On Tue, Jul 22, 2008 at 03:14:12AM -0400, Jeff King wrote:
> 
> >  static struct child_process pager_process = {
> >  	.argv = pager_argv,
> > -	.in = -1
> > +	.in = -1,
> > +#ifndef __MINGW32__
> > +	.preexec_cb = pager_preexec,
> > +#endif
> 
> I couldn't recall if this initializer style is portable enough for us.
> It was already there wrapped in ifdefs, but perhaps it was only ok
> because the mingw version always uses the same compiler?

  it's not, I asked long time ago, and it's C99, which mingw supports
indeed, and we don't want to require a C99 compiler.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

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

^ permalink raw reply

* Re: Error: "You have some suspicious patch lines"
From: Jeff King @ 2008-07-22  7:26 UTC (permalink / raw)
  To: Ben Aurel; +Cc: git
In-Reply-To: <4885895C.5050108@gmail.com>

On Tue, Jul 22, 2008 at 09:16:44AM +0200, Ben Aurel wrote:

> -----------------------------------------
>
> Editing '.git/hooks/pre-commit' and comment out the following lines
> --
> 61             if (/^\s* \t/) {
> 62                 bad_line("indent SP followed by a TAB", $_);
> 63             }
> --
>
> And finally "git commit" works again.
>
> The question now is: Is it really necessary to edit the git script  
> everytime? Is there a urgent reason why git refuses to commit because of  
> "suspicious" lines? Is it really necessary?

The pre-commit hook that ships with git checks whitespace as an example
of what one _could_ do with hooks. It is not meant to be enabled by
default (unless you want that whitespace checking).

So either:

 1. You enabled it by setting the execute bit. If so, then don't do
    that.

 2. Something is broken, and it has caused the hook to be enabled
    accidentally. I recall somebody complaining that hooks were enabled
    by default under cygwin because the filesystem didn't support the
    execute bit. Are you working on an exec-bit challenged filesystem?

In newer versions of git, the hooks actually ship with a .sample
extension so that they will not be used accidentally, regardless of the
executable bit. In the meantime, it is safe to simply delete
.git/hooks/pre-commit if it is bothering you.

-Peff

^ permalink raw reply

* Re: [PATCH 2/9] Makefile: Normalize $(bindir) and $(gitexecdir)  before comparing
From: Johannes Sixt @ 2008-07-22  7:25 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0807220147320.3407@eeepc-johanness>

Johannes Schindelin schrieb:
> Hi,
> 
> On Mon, 21 Jul 2008, Johannes Sixt wrote:
> 
>> +	bindir=$$(cd '$(DESTDIR_SQ)$(bindir_SQ)' && pwd) && \
>> +	execdir=$$(cd '$(DESTDIR_SQ)$(gitexecdir_SQ)' && pwd) && \
> 
> These lack quotes, no?

No. RHS of an assignment doesn't need quotes as long as the shell syntax
makes clear where the assignment ends, which is the case here because of
the brackets $(...).

-- Hannes

^ permalink raw reply

* Re: Git Documentation
From: Johan Herland @ 2008-07-22  7:17 UTC (permalink / raw)
  To: Scott Chacon; +Cc: git
In-Reply-To: <d411cc4a0807212035v68c2ed95m93b77c1e61cfec9e@mail.gmail.com>

On Tuesday 22 July 2008, Scott Chacon wrote:
> If anyone has any tips on how they think git should be taught, issues
> they are asked a lot, problems newbies tend to have, something they
> wish there were a screencast for or was better documented, etc -
> please do contact me so I can incorporate it.

You should at least take a look at this thread:

http://thread.gmane.org/gmane.comp.version-control.git/88698

(even though it goes off-topic after a while...)

> If anyone has any tips on how they think git should be taught...

It seems there are primarily two ways to teach Git:

1. Top-down: Start with simple use cases and commands. Teach people a 
minimal, but necessary set of porcelain commands to get them started. Stay 
_far_ away from plumbing commands and most of the command options.

2. Bottom-up: Start with how Git structures the data. Talk about blobs, 
trees, commits, refs, how everything is connected, and how various Git 
commands query and manipulate this structure. This _may_ involve a fair 
amount of plumbing commands, especially when discovering how the more 
complicated high-level commands manipulate the structure.

Some people seem to prefer the first approach, other people prefer the other 
approach. Both paths lead to enlightenment ;). In many cases a bit of both 
may be useful. HOWEVER, I think it is _very_ important to keep in mind that 
these are two _different_ approaches, and the contexts in which they are 
taught should be kept separate. I would almost suggest splitting your 
website down the middle and make the difference between top-down and 
bottom-up immediately visible with, say, a different background color, or 
something else that immediately tells the user what "track" they are 
following...


BTW, I think what you're doing is good and important (and I've already 
enjoyed some of your gitcasts). Keep up the good work! :)


Have fun!

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net

^ permalink raw reply

* Re: [PATCH] Fix update-index --refresh for submodules if stat(2) returns st_size 0
From: Johannes Sixt @ 2008-07-22  7:17 UTC (permalink / raw)
  To: Alex Riesen, Junio C Hamano; +Cc: git
In-Reply-To: <20080721173511.GB5387@steel.home>

Alex Riesen schrieb:
> Can MSys folks please try it? I noticed it when the test
> t2103-update-index-ignore-missing.sh (the 5th case) started failing.

I tested it. mingw.git does suffer from the problem, and this fixes it.

But!

> +	if ((changed & DATA_CHANGED) && (ce->ce_size != 0 || S_ISGITLINK(ce->ce_mode)))

Does this mean that ce->ce_size is non-zero for gitlinks, at least on
Unix? Is this value useful in anyway? I don't think so. Then it shouldn't
be a random value that lstat() happens to return.

-- Hannes

^ permalink raw reply

* Error: "You have some suspicious patch lines"
From: Ben Aurel @ 2008-07-22  7:16 UTC (permalink / raw)
  To: git

hi

I working on mac os x 10.5.4 (intel) with git version 1.5.5.3 and I 
always get this message for most of my perl scripts and also for 
"Makefile.pre" files:

----------- Message ---------------
* You have some suspicious patch lines:
*
* In src/scripts/trunk/3rdparty/file_sanity.pl
* trailing whitespace (line 52)
...
------------------------------------------


Editing '.git/hooks/pre-commit' and comment out the following lines 
(around line 58):

--
58            if (/\s$/) {
59                bad_line("trailing whitespace", $_);
60             }
--

I still have this message

----------- Message ---------------
$ git commit .
*
* You have some suspicious patch lines:
*
* In src/scripts/trunk/3rdparty/file_sanity.pl
* indent SP followed by a TAB (line 112)
-----------------------------------------

Editing '.git/hooks/pre-commit' and comment out the following lines
--
 61             if (/^\s* \t/) {
 62                 bad_line("indent SP followed by a TAB", $_);
 63             }
--

And finally "git commit" works again.

The question now is: Is it really necessary to edit the git script 
everytime? Is there a urgent reason why git refuses to commit because of 
"suspicious" lines? Is it really necessary?

Thanks
ben

^ permalink raw reply

* Re: [PATCH 2/2] spawn pager via run_command interface
From: Jeff King @ 2008-07-22  7:16 UTC (permalink / raw)
  To: Mike Hommey; +Cc: Junio C Hamano, git, David Bremner
In-Reply-To: <20080722071411.GB3584@sigill.intra.peff.net>

On Tue, Jul 22, 2008 at 03:14:12AM -0400, Jeff King wrote:

>  static struct child_process pager_process = {
>  	.argv = pager_argv,
> -	.in = -1
> +	.in = -1,
> +#ifndef __MINGW32__
> +	.preexec_cb = pager_preexec,
> +#endif

I couldn't recall if this initializer style is portable enough for us.
It was already there wrapped in ifdefs, but perhaps it was only ok
because the mingw version always uses the same compiler?

-Peff

^ permalink raw reply

* [PATCH 2/2] spawn pager via run_command interface
From: Jeff King @ 2008-07-22  7:14 UTC (permalink / raw)
  To: Mike Hommey; +Cc: Junio C Hamano, git, David Bremner
In-Reply-To: <20080722071246.GA3584@sigill.intra.peff.net>

This has two important effects:

 1. The pager is now the _child_ process, instead of the
    parent. This means that whatever spawned git (e.g., the
    shell) will see the exit code of the git process, and
    not the pager.

 2. The mingw and regular code are now unified, which makes
    the setup_pager function much simpler.

There are two caveats:

 1. We used to call execlp directly on the pager, followed
    by trying to exec it via the shall. We now just use the
    shell (which is what mingw has always done). This may
    have different results for pager names which contain
    shell metacharacters.

    It is also slightly less efficient because we
    unnecessarily run the shell; however, pager spawning is
    by definition an interactive task, so it shouldn't be
    a huge problem.

 2. The git process will remain in memory while the user
    looks through the pager. This is potentially wasteful.
    We could get around this by turning the parent into a
    meta-process which spawns _both_ git and the pager,
    collects the exit status from git, waits for both to
    end, and then exits with git's exit code.
---
 pager.c |   49 ++++++++-----------------------------------------
 1 files changed, 8 insertions(+), 41 deletions(-)

diff --git a/pager.c b/pager.c
index 6b5c9e4..7743742 100644
--- a/pager.c
+++ b/pager.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "run-command.h"
 
 /*
  * This is split up from the rest of git so that we can do
@@ -8,7 +9,7 @@
 static int spawned_pager;
 
 #ifndef __MINGW32__
-static void run_pager(const char *pager)
+static void pager_preexec(void)
 {
 	/*
 	 * Work around bug in "less" by not starting it until we
@@ -20,16 +21,17 @@ static void run_pager(const char *pager)
 	FD_SET(0, &in);
 	select(1, &in, NULL, &in, NULL);
 
-	execlp(pager, pager, NULL);
-	execl("/bin/sh", "sh", "-c", pager, NULL);
+	setenv("LESS", "FRSX", 0);
 }
-#else
-#include "run-command.h"
+#endif
 
 static const char *pager_argv[] = { "sh", "-c", NULL, NULL };
 static struct child_process pager_process = {
 	.argv = pager_argv,
-	.in = -1
+	.in = -1,
+#ifndef __MINGW32__
+	.preexec_cb = pager_preexec,
+#endif
 };
 static void wait_for_pager(void)
 {
@@ -40,14 +42,9 @@ static void wait_for_pager(void)
 	close(2);
 	finish_command(&pager_process);
 }
-#endif
 
 void setup_pager(void)
 {
-#ifndef __MINGW32__
-	pid_t pid;
-	int fd[2];
-#endif
 	const char *pager = getenv("GIT_PAGER");
 
 	if (!isatty(1))
@@ -66,35 +63,6 @@ void setup_pager(void)
 
 	spawned_pager = 1; /* means we are emitting to terminal */
 
-#ifndef __MINGW32__
-	if (pipe(fd) < 0)
-		return;
-	pid = fork();
-	if (pid < 0) {
-		close(fd[0]);
-		close(fd[1]);
-		return;
-	}
-
-	/* return in the child */
-	if (!pid) {
-		dup2(fd[1], 1);
-		dup2(fd[1], 2);
-		close(fd[0]);
-		close(fd[1]);
-		return;
-	}
-
-	/* The original process turns into the PAGER */
-	dup2(fd[0], 0);
-	close(fd[0]);
-	close(fd[1]);
-
-	setenv("LESS", "FRSX", 0);
-	run_pager(pager);
-	die("unable to execute pager '%s'", pager);
-	exit(255);
-#else
 	/* spawn the pager */
 	pager_argv[2] = pager;
 	if (start_command(&pager_process))
@@ -107,7 +75,6 @@ void setup_pager(void)
 
 	/* this makes sure that the parent terminates after the pager */
 	atexit(wait_for_pager);
-#endif
 }
 
 int pager_in_use(void)
-- 
1.6.0.rc0.1.g9291f.dirty

^ permalink raw reply related

* [PATCH 1/2] run-command: add pre-exec callback
From: Jeff King @ 2008-07-22  7:12 UTC (permalink / raw)
  To: Mike Hommey; +Cc: Junio C Hamano, git, David Bremner
In-Reply-To: <20080722071009.GA3610@sigill.intra.peff.net>

This is a function provided by the caller which is called
_after_ the process is forked, but before the spawned
program is executed. On platforms (like mingw) where
subprocesses are forked and executed in a single call, the
preexec callback is simply ignored.

This will be used in the following patch to do some setup
for 'less' that must happen in the forked child.

Signed-off-by: Jeff King <peff@peff.net>
---
 run-command.c |    2 ++
 run-command.h |    1 +
 2 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/run-command.c b/run-command.c
index 6e29fdf..73d0c31 100644
--- a/run-command.c
+++ b/run-command.c
@@ -110,6 +110,8 @@ int start_command(struct child_process *cmd)
 					unsetenv(*cmd->env);
 			}
 		}
+		if (cmd->preexec_cb)
+			cmd->preexec_cb();
 		if (cmd->git_cmd) {
 			execv_git_cmd(cmd->argv);
 		} else {
diff --git a/run-command.h b/run-command.h
index 5203a9e..4f2b7d7 100644
--- a/run-command.h
+++ b/run-command.h
@@ -42,6 +42,7 @@ struct child_process {
 	unsigned no_stderr:1;
 	unsigned git_cmd:1; /* if this is to be git sub-command */
 	unsigned stdout_to_stderr:1;
+	void (*preexec_cb)(void);
 };
 
 int start_command(struct child_process *);
-- 
1.6.0.rc0.1.g9291f.dirty

^ permalink raw reply related

* Re: git status in clean working dir
From: Jeff King @ 2008-07-22  7:10 UTC (permalink / raw)
  To: Mike Hommey; +Cc: Junio C Hamano, git, David Bremner
In-Reply-To: <20080722064603.GA25221@sigill.intra.peff.net>

On Tue, Jul 22, 2008 at 02:46:04AM -0400, Jeff King wrote:

> I am tempted by the "order switching" I mentioned, but that would entail
> the git process waiting to clean the pager, during which time it may be
> consuming memory. But maybe that isn't worth worrying about.

It feels very wrong proposing this during release freeze, but here is
the "pager is child of git" implementation.

Patch 1/1 adds a bit of necessary infrastructure to run-command, and
patch 2/2 does the deed. The nice thing is that it unifies the Windows
and Unix implementations of setup_pager, so we get a nice line
reduction.

 pager.c       |   49 ++++++++-----------------------------------------
 run-command.c |    2 ++
 run-command.h |    1 +
 3 files changed, 11 insertions(+), 41 deletions(-)

^ 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