* Re: Bug: git add with absolute path fails if repo root dir is a symlink
From: Carlo Marcelo Arenas Belon @ 2010-12-27 8:13 UTC (permalink / raw)
To: Alexander Gladysh; +Cc: git
In-Reply-To: <AANLkTimPxhqwMebfTw9oHucuvABmSBynpZgG1zp6uwVz@mail.gmail.com>
On Mon, Dec 27, 2010 at 10:23:12AM +0300, Alexander Gladysh wrote:
>
> > I can't run git add with absolute path if the repository's root
> > directory is a symlink.
>
> Note that this issue is also triggered if *any* of the directories in
> path above of my repo are symlinks.
When using absolute path names, git will compare the path given with the
git work tree and any name that is referred through a symlink in that
will trigger a mismatch.
> Is there a way to quickly workaround this somehow?
use relative paths (implemented below through an alias named "myadd") :
[alias]
myadd = "!sh -c 'cd `dirname \"$1\"` && git add `basename \"$1\"`' -"
so in your workflow you would use "myadd" instead of "add" to convert your
absolute paths (with symlinks) into relative paths
Carlo
^ permalink raw reply
* [PATCH v2] Fix false positives in t3404 due to SHELL=/bin/false
From: Robin H. Johnson @ 2010-12-27 8:03 UTC (permalink / raw)
To: Junio C Hamano, git
In-Reply-To: <7vsjxjyce6.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 1442 bytes --]
If the user's shell in NSS passwd is /bin/false (eg as found during Gentoo's
package building), the git-rebase exec tests will fail, because they call
$SHELL around the command, and in the existing testcase, $SHELL was not being
cleared sufficently.
This lead to false positive failures of t3404 on systems where the package
build user was locked down as noted above.
Signed-off-by: "Robin H. Johnson" <robbat2@gentoo.org>
X-Gentoo-Bug: 349083
X-Gentoo-Bug-URL: http://bugs.gentoo.org/show_bug.cgi?id=349083
---
t/t3404-rebase-interactive.sh | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index d3a3bd2..7d8147b 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -71,8 +71,9 @@ test_expect_success 'setup' '
# "exec" commands are ran with the user shell by default, but this may
# be non-POSIX. For example, if SHELL=zsh then ">file" doesn't work
# to create a file. Unseting SHELL avoids such non-portable behavior
-# in tests.
+# in tests. It must be exported for it to take effect where needed.
SHELL=
+export SHELL
test_expect_success 'rebase -i with the exec command' '
git checkout master &&
--
Robin Hugh Johnson
Gentoo Linux: Developer, Trustee & Infrastructure Lead
E-Mail : robbat2@gentoo.org
GnuPG FP : 11AC BA4F 4778 E3F6 E4ED F38E B27B 944E 3488 4E85
[-- Attachment #2: Type: application/pgp-signature, Size: 330 bytes --]
^ permalink raw reply related
* Re: Bug: git add with absolute path fails if repo root dir is a symlink
From: Alexander Gladysh @ 2010-12-27 7:23 UTC (permalink / raw)
To: git
In-Reply-To: <AANLkTi=Mj3AdinC87Ys35fv9DpZqefiZXhPbHMLdmyPh@mail.gmail.com>
On Mon, Dec 27, 2010 at 09:25, Alexander Gladysh <agladysh@gmail.com> wrote:
> I can't run git add with absolute path if the repository's root
> directory is a symlink.
Note that this issue is also triggered if *any* of the directories in
path above of my repo are symlinks.
This is a show-stopper for my current workflow.
Is there a way to quickly workaround this somehow?
Alexander.
^ permalink raw reply
* Bug: git add with absolute path fails if repo root dir is a symlink
From: Alexander Gladysh @ 2010-12-27 6:25 UTC (permalink / raw)
To: git
Hi, list.
Yet another issue with Git symlink handling.
I can't run git add with absolute path if the repository's root
directory is a symlink.
Please see the transcript below for details.
Alexander.
$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description: Ubuntu 10.10
Release: 10.10
Codename: maverick
$ uname -a
Linux ubuntu 2.6.35-24-generic #42-Ubuntu SMP Thu Dec 2 01:41:57 UTC
2010 i686 GNU/Linux
$ git --version
git version 1.7.3.4
$ mkdir myrepo && cd myrepo
$ git init
$ touch alpha
$ git add alpha
$ git commit -m "initial commit"
$ cd ../
$ ln -s myrepo mysymlink
$ cd mysymlink
$ git status
$ touch beta
$ git add ~/tmp/git-test/mysymlink/beta
fatal: '/home/agladysh/tmp/git-test/mysymlink/beta' is outside repository
$ cd ../myrepo/
$ git add ~/tmp/git-test/myrepo/beta
$ git status
# On branch master
# Changes to be committed:
# (use "git reset HEAD <file>..." to unstage)
#
# new file: beta
#
^ permalink raw reply
* (unknown),
From: COCA COLA @ 2010-12-27 6:07 UTC (permalink / raw)
[-- Attachment #1: Type: text/plain, Size: 90 bytes --]
KINDLY DOWNLOAD ATTACHMENT AS YOUR EMAIL WAS SELECTED AND YOU HAVE WON ONE MILLION POUNDS
[-- Attachment #2: COCA COLA ONLINE NOTIFICATION.doc --]
[-- Type: application/msword, Size: 35328 bytes --]
^ permalink raw reply
* Re: [PATCH] Fix false positives in t3404 due to SHELL=/bin/false
From: Junio C Hamano @ 2010-12-27 6:10 UTC (permalink / raw)
To: Robin H. Johnson; +Cc: Git Mailing List
In-Reply-To: <robbat2-20101227T024837-537032076Z@orbis-terrarum.net>
"Robin H. Johnson" <robbat2@gentoo.org> writes:
> # "exec" commands are ran with the user shell by default, but this may
> # be non-POSIX. For example, if SHELL=zsh then ">file" doesn't work
> # to create a file. Unseting SHELL avoids such non-portable behavior
> -# in tests.
> -SHELL=
> +# in tests. It must be exported for it to take effect where needed.
> +export SHELL=
Thanks.
This probably is still not portable.
SHELL=
export SHELL
would be Ok, though.
^ permalink raw reply
* [PATCH] Fix false positives in t3404 due to SHELL=/bin/false
From: Robin H. Johnson @ 2010-12-27 2:50 UTC (permalink / raw)
To: Git Mailing List
[-- Attachment #1: Type: text/plain, Size: 1405 bytes --]
If the user's shell in NSS passwd is /bin/false (eg as found during Gentoo's
package building), the git-rebase exec tests will fail, because they call
$SHELL around the command, and in the existing testcase, $SHELL was not being
cleared sufficently.
This lead to false positive failures of t3404 on systems where the package
build user was locked down as noted above.
Signed-off-by: "Robin H. Johnson" <robbat2@gentoo.org>
X-Gentoo-Bug: 349083
X-Gentoo-Bug-URL: http://bugs.gentoo.org/show_bug.cgi?id=349083
diff -Nuar git-1.7.3.4.orig/t/t3404-rebase-interactive.sh git-1.7.3.4/t/t3404-rebase-interactive.sh
--- git-1.7.3.4.orig/t/t3404-rebase-interactive.sh 2010-12-16 02:52:11.000000000 +0000
+++ git-1.7.3.4/t/t3404-rebase-interactive.sh 2010-12-26 22:30:47.826421313 +0000
@@ -67,8 +67,8 @@
# "exec" commands are ran with the user shell by default, but this may
# be non-POSIX. For example, if SHELL=zsh then ">file" doesn't work
# to create a file. Unseting SHELL avoids such non-portable behavior
-# in tests.
-SHELL=
+# in tests. It must be exported for it to take effect where needed.
+export SHELL=
test_expect_success 'rebase -i with the exec command' '
git checkout master &&
--
Robin Hugh Johnson
Gentoo Linux: Developer, Trustee & Infrastructure Lead
E-Mail : robbat2@gentoo.org
GnuPG FP : 11AC BA4F 4778 E3F6 E4ED F38E B27B 944E 3488 4E85
[-- Attachment #2: Type: application/pgp-signature, Size: 330 bytes --]
^ permalink raw reply
* Re: cherry-pick / pre-commit hook?
From: Dave Abrahams @ 2010-12-27 2:18 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git
In-Reply-To: <20101208220514.GA8865@burratino>
At Wed, 8 Dec 2010 16:05:14 -0600,
Jonathan Nieder wrote:
>
> The main purpose of the pre-commit and commit-msg hooks is to avoid
> introducing regressions in whitespace style, encoding, and so forth;
> and it would make cherry-picking unnecessarily difficult, without
> preventing regressions, to unconditionally apply the same standards to
> existing code. For this reason, in v0.99.6~51 (2005-08-29), git
> learned to skip the usual hooks when cherry-picking or reverting an
> existing commit.
>
> But sometimes the checks are wanted anyway. For example, with this
> patch applied, you can safely fetch some new contributor's code:
>
> $ git cherry-pick -s --verify HEAD..FETCH_HEAD
>
> while allowing the pre-commit and commit-msg hooks to run their usual
> checks so the result can error out if the patches are not clean.
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> Untested. Please feel free to add some documentation and tests and
> submit it for real if this looks like a good idea. :)
Well, thanks, but sadly I can only invest enough time to file this bug
report right now: if you're going to have a "pre-commit hook" concept,
but not run that hook for some kinds of commits, then that fact needs
to be documented.
--
Dave Abrahams
BoostPro Computing
http://www.boostpro.com
^ permalink raw reply
* [PATCH] setup_work_tree: adjust relative $GIT_WORK_TREE after moving cwd
From: Nguyễn Thái Ngọc Duy @ 2010-12-27 1:26 UTC (permalink / raw)
To: git, Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1293364014-8463-1-git-send-email-pclouds@gmail.com>
When setup_work_tree() is called, it moves cwd to $GIT_WORK_TREE and
makes internal copy of $GIT_WORK_TREE absolute. The environt variable,
if set by user, remains unchanged. If the variable is relative, it is
no longer correct because its base dir has changed.
Instead of making $GIT_WORK_TREE absolute too, we just say "." and let
subsequent git processes handle it.
Reported-by: Michel Briand <michelbriand@free.fr>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Third round. Remove subshell and `pwd` in t1501.31
.gitignore | 1 +
Makefile | 1 +
setup.c | 8 ++++++++
t/t1501-worktree.sh | 7 +++++++
test-subprocess.c | 21 +++++++++++++++++++++
5 files changed, 38 insertions(+), 0 deletions(-)
create mode 100644 test-subprocess.c
diff --git a/.gitignore b/.gitignore
index 87b833c..3dd6ef7 100644
--- a/.gitignore
+++ b/.gitignore
@@ -177,6 +177,7 @@
/test-sha1
/test-sigchain
/test-string-pool
+/test-subprocess
/test-svn-fe
/test-treap
/common-cmds.h
diff --git a/Makefile b/Makefile
index 57d9c65..bdf86a3 100644
--- a/Makefile
+++ b/Makefile
@@ -431,6 +431,7 @@ TEST_PROGRAMS_NEED_X += test-run-command
TEST_PROGRAMS_NEED_X += test-sha1
TEST_PROGRAMS_NEED_X += test-sigchain
TEST_PROGRAMS_NEED_X += test-string-pool
+TEST_PROGRAMS_NEED_X += test-subprocess
TEST_PROGRAMS_NEED_X += test-svn-fe
TEST_PROGRAMS_NEED_X += test-treap
TEST_PROGRAMS_NEED_X += test-index-version
diff --git a/setup.c b/setup.c
index 91887a4..3833569 100644
--- a/setup.c
+++ b/setup.c
@@ -239,6 +239,14 @@ void setup_work_tree(void)
git_dir = make_absolute_path(git_dir);
if (!work_tree || chdir(work_tree))
die("This operation must be run in a work tree");
+
+ /*
+ * Make sure subsequent git processes find correct worktree
+ * if $GIT_WORK_TREE is set relative
+ */
+ if (getenv(GIT_WORK_TREE_ENVIRONMENT))
+ setenv(GIT_WORK_TREE_ENVIRONMENT, ".", 1);
+
set_git_dir(make_relative_path(git_dir, work_tree));
initialized = 1;
}
diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh
index 2c8f01f..f072a8e 100755
--- a/t/t1501-worktree.sh
+++ b/t/t1501-worktree.sh
@@ -340,4 +340,11 @@ test_expect_success 'make_relative_path handles double slashes in GIT_DIR' '
git --git-dir="$(pwd)//repo.git" --work-tree="$(pwd)" add dummy_file
'
+test_expect_success 'relative $GIT_WORK_TREE and git subprocesses' '
+ GIT_DIR=repo.git GIT_WORK_TREE=repo.git/work \
+ test-subprocess --setup-work-tree rev-parse --show-toplevel >actual &&
+ echo "$TRASH_DIRECTORY/repo.git/work" >expected &&
+ test_cmp expected actual
+'
+
test_done
diff --git a/test-subprocess.c b/test-subprocess.c
new file mode 100644
index 0000000..667d3e5
--- /dev/null
+++ b/test-subprocess.c
@@ -0,0 +1,21 @@
+#include "cache.h"
+#include "run-command.h"
+
+int main(int argc, char **argv)
+{
+ const char *prefix;
+ struct child_process cp;
+ int nogit = 0;
+
+ prefix = setup_git_directory_gently(&nogit);
+ if (nogit)
+ die("No git repo found");
+ if (!strcmp(argv[1], "--setup-work-tree")) {
+ setup_work_tree();
+ argv++;
+ }
+ memset(&cp, 0, sizeof(cp));
+ cp.git_cmd = 1;
+ cp.argv = (const char **)argv+1;
+ return run_command(&cp);
+}
--
1.7.3.4.878.g439c7
^ permalink raw reply related
* Re: Network problems during "git svn dcommit"
From: Jonathan Nieder @ 2010-12-27 0:57 UTC (permalink / raw)
To: Josef Wolf; +Cc: git
In-Reply-To: <20101222115008.GA10765@raven.wolf.lan>
Josef Wolf wrote:
> I am using git-svn to track a subversion repository. This used to work
> fine so far. But today, I got a network outage during a "git svn dcommit"
> operation.
[...]
> Any hints how to fix the situation?
Yes.
To fix your repository:
1. make backups!
2. back out the bad commit so it is not visible with "gitk --all"
(something like
git reset --keep HEAD^
git push . HEAD:refs/remotes/svn/Trunk
)
3. "rm -fr .git/svn/". Note: this will delete your unhandled.log!
Restore it from backup if you consider it precious.
4. "git svn fetch; git svn rebase"
To fix "git svn", see [1].
Hope that helps,
Jonathan
[1] http://thread.gmane.org/gmane.comp.version-control.git/136510
^ permalink raw reply
* Re: [RFC/PATCH] diff: funcname and word patterns for perl
From: Jakub Narebski @ 2010-12-26 23:14 UTC (permalink / raw)
To: Jonathan Nieder
Cc: git, J.H., John 'Warthog9' Hawley, Thomas Rast, Jeff King
In-Reply-To: <20101226090731.GA21588@burratino>
On Sun, 26 Dec 2010 10:07, Jonathan Nieder wrote:
> The default function name discovery already works quite well for Perl
> code... with the exception of here-documents (or rather their ending).
>
> sub foo {
> print <<END
> here-document
> END
> return 1;
> }
>
> The default funcname pattern treats the unindented END line as a
> function declaration and puts it in the @@ line of diff and "grep
> --show-function" output.
>
> With a little knowledge of perl syntax, we can do better. You can
> try it out by adding "*.perl diff=perl" to the gitattributes file.
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> Jakub Narebski wrote:
>
> > BTW. do you know how such perl support should look like?
>
> Maybe something like this?
Thanks a lot.
Besides here-doc, there are some tricky things that such code should
be aware about.
1. BEGIN {
...
}
and similar code blocks (END, CHECK, INIT, ...) which I think should
be marked as 'BEGIN' in diff chunk.
2. sub foo {
FOO: while (1) {
...
}
}
which should be marked with 'sub foo {', I think
3. =head1 NAME
Git - Perl interface to the Git version control system
=cut
i.e. POD... which I don't know what to do about.
I have not checked what your code does wrt those.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [RFC PATCH v7 6/9] gitweb/lib - Simple output capture by redirecting STDOUT to file
From: Jakub Narebski @ 2010-12-26 23:03 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, J.H., John 'Warthog9' Hawley
In-Reply-To: <20101224094934.GA952@burratino>
On Fri, 24 Dec 2010, Jonathan Nieder wrote:
> Jakub Narebski wrote:
>
> > This patch was based on "gitweb: add output buffering and associated
> > functions" patch by John 'Warthog9' Hawley (J.H.) in "Gitweb caching v7"
> > series, and on code of Capture::Tiny by David Golden (Apache License 2.0).
>
> Micronit: if the license of Capture::Tiny were relevant then we would be
> in trouble, I think. (Apache-2.0 and GPLv2 aren't compatible licenses.)
Damn, I have thought that Apache-2.0 and GPLv2 are compatibile. This is
the only reason that I explicitely mentioned the license (that and it is
not usual "licensed like Perl", i.e. dual Artistic Perl License / GPL
licensed). I should have checked that Apache and GPLv2 are compatibile.
> Luckily
>
> [...]
> > +# taken from Capture::Tiny by David Golden, Apache License 2.0
> > +# with debugging stripped out
> > +sub _relayer {
> > + my ($fh, $layers) = @_;
> > +
> > + my %seen = ( unix => 1, perlio => 1); # filter these out
> > + my @unique = grep { !$seen{$_}++ } @$layers;
> > +
> > + binmode($fh, join(":", ":raw", @unique));
> > +}
>
> looks trivial enough. Maybe either avoiding mention of the license or
> clarifying that that is not intended to be the sole license for the
> stripped-down code would help?
You are right. I have done similar thing for PerlIO::Util based capture,
though I didn't know about the 'binmode($fh, join(":", ":raw", @unique));'
trick.
So I think we would be in the clear by changing the comment to read:
+# see also _relayer in Capture::Tiny by David Golden
or something like that.
Or we can try to change gitweb license to GPLv3 / AGPLv3, which is
compatibile (one way only) with Apache-2.0... just kidding :-)
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [RFC PATCH v7 4/9] gitweb: Prepare for splitting gitweb
From: Jakub Narebski @ 2010-12-26 22:54 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, J.H., John 'Warthog9' Hawley
In-Reply-To: <20101224092932.GA31537@burratino>
On Fri, 24 Dec 2010 10:29, Jonathan Nieder wrote:
> Jakub Narebski wrote:
>
> > Prepare gitweb for having been split into modules that are to be
> > installed alongside gitweb in 'lib/' subdirectory, by adding
> >
> > use lib __DIR__.'/lib';
> >
> > to gitweb.perl (to main gitweb script), and preparing for putting
> > modules (relative path) in $(GITWEB_MODULES) in gitweb/Makefile.
>
> Spelled out, this means modules would typically go in
>
> /usr/share/gitweb/lib
Yes, it's true. It is mainly to support situation where one can install
files in (subdirectory of) cgi-bin, but nowehere else. That is why the
default is to install modules alongside with gitweb.
The additional advantage is that t/gitweb-lib.sh used by gitweb tests
can very simply test source version of gitweb, with gitweb finding
source version of modules. But it is not a very large obstacle to
change this.
> Is that the right place? I suspect something like
>
> /usr/lib/gitweb/
>
> could make sense in some installations for two reasons:
>
> - even braindamaged webserver configurations would not serve lib/
> as static files in that case;
Actually it doesn't matter what web server does with those files when
accessed directly, except for the client (user) confusion if he/she
goes where not invited. Modules are used by Perl (by gitweb), not by
web server.
>
> - if some modules are implemented in C for speed, they would need
> to go in /usr/lib anyway to follow usual filesystem conventions.
Ugh, XS! I sincerely hope that when there would be decision to implement
some features in C for speed, we would be able to use Perl version of
ctypes for C-to-Perl interface, not XS.
Anyway most probable to be implemented in C would be Git.pm, or rather
Perl interface to libgit2. It is probable that at some point gitweb
would be converted to use Git.pm or its successor. But I guess that
Git Perl module would be installed somewhere in PERL5LIB, so it would
be found even without "use lib __DIR__ . '/lib';" or its replacement.
> Does the Makefile let us override the directory with such a setting?
>
I have thought that I did provide 'gitweblibdir' as configurable knob,
but I see that in the version I have send I don't do this:
# Shell quote;
bindir_SQ = $(subst ','\'',$(bindir))#'
gitwebdir_SQ = $(subst ','\'',$(gitwebdir))#'
gitwebstaticdir_SQ = $(subst ','\'',$(gitwebdir)/static)#'
gitweblibdir_SQ = $(subst ','\'',$(gitwebdir)/lib)#'
But if we are to allow custom gitweblibdir, we would have to change the
way gitweb is to find its modules. One solution would be inetad of
current
# __DIR__ is taken from Dir::Self __DIR__ fragment
sub __DIR__ () {
File::Spec->rel2abs(join '', (File::Spec->splitpath(__FILE__))[0, 1]);
}
use lib __DIR__ . '/lib';
use simply
use lib $ENV{GITWEBLIBDIR} || "++GITWEBLIBDIR++";
Of course both gitweb/Makefile and t/gitweb-lib.sh would have to be
updated: gitweb/Makefile to include replacement rule for '++GITWEBLIBDIR++'
in GITWEB_REPLACE, and t/gitweb-lib.sh to declare and export GITWEBLIBDIR
environmental variable so that gitweb/gitweb.perl would be able to find
its modules when used for gitweb tests (see comment earlier).
> > While at it pass GITWEBLIBDIR in addition to GITWEB_TEST_INSTALLED to
> > allow testing installed version of gitweb and installed version of
> > modules (for future tests which would check individual (sub)modules).
> >
> > Using __DIR__ from Dir::Self module (not in core, that's why currently
> > gitweb includes excerpt of code from Dir::Self defining __DIR__) was
> > chosen over using FindBin-based solution (in core since perl 5.00307,
> > while gitweb itself requires at least perl 5.8.0) because FindBin uses
> > BEGIN block
>
> This explanation and the code below leave me nervous that the answer
> might be "no". ;-)
No it doesn't, but yes it could (see above).
> [...]
> > --- a/gitweb/gitweb.perl
> > +++ b/gitweb/gitweb.perl
> > @@ -10,6 +10,14 @@
> > use 5.008;
> > use strict;
> > use warnings;
> > +
> > +use File::Spec;
> > +# __DIR__ is taken from Dir::Self __DIR__ fragment
> > +sub __DIR__ () {
> > + File::Spec->rel2abs(join '', (File::Spec->splitpath(__FILE__))[0, 1]);
> > +}
> > +use lib __DIR__ . '/lib';
> > +
> > use CGI qw(:standard :escapeHTML -nosticky);
> > use CGI::Util qw(unescape);
> > use CGI::Carp qw(fatalsToBrowser);
--
Jakub Narebski
Poland
^ permalink raw reply
* Re[2]: [PATCH 3/3] Fixes bug: GIT_PS1_SHOWDIRTYSTATE is no not respect diff.ignoreSubmodules config variable
From: Алексей Шумкин @ 2010-12-26 22:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jens Lehmann, git, Johannes Schindelin
In-Reply-To: <7vd3ooz6qd.fsf@alter.siamese.dyndns.org>
JCH> By the way, could we please have a real sign-off, not with a one with a
JCH> pseudonym, given to the series?
Oh, I`m sorry.
Of course.
Should I resend patches with real sign-off, at least first two ones?
^ permalink raw reply
* Re: [RFC PATCH v7 1/9] gitweb: Go to DONE_REQUEST rather than DONE_GITWEB in die_error
From: Jakub Narebski @ 2010-12-26 22:25 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, J.H., John 'Warthog9' Hawley
In-Reply-To: <20101226095054.GB21588@burratino>
On Sun, 26 Dec 2010, Jonathan Nieder wrote:
> Jakub Narebski wrote:
>> On Thu, 23 Dec 2010, Jonathan Nieder wrote:
>>> die_error gets called when server load is too high; I wonder whether
>>> it is right to go back for another request in that case.
>>
>> If client (web browser) are requesting connection, we have to tell it
>> something anyway.
>
> Right, I should have thought a few seconds more. Respawning
> gitweb.perl would generate _more_ load[1].
> [1] I can imagine scenarios in which exiting gitweb would help
> alleviate the load, involving:
>
> - large memory footprint for each gitweb process forcing the system
> into swapping (e.g., from a memory leak), or
> - FastCGI-like server noticing the load and choosing to decrease the
> number of gitweb instances.
>
> In the usual case, presumably gitweb memory footprint is small and
> FastCGI-like servers limit the number of gitweb instances to a modest
> fixed number.
I assume that CGI / FastCGI / mod_perl (+ ModPerl::Registry) web server
would know how to regulate number of workers according to the server
load.
>>> A broken per-request (or other) configuration could potentially leave
>>> a gitweb process in a broken state,
> [...]
>> 'die $@ if $@' would call CORE::die, which means it would end gitweb
>> process.
>
> This is referring to a later patch?
I'm sorry I haven't made myself clear.
What I meant here is that gitweb includes the following code
if (-e $GITWEB_CONFIG) {
do $GITWEB_CONFIG;
die $@ if $@;
}
which means that CGI::Carp::die is called, which might call
handle_errors_html, and which ends in CORE::die, which ends gitweb
process. So if there is no way for broken configuration to leave
gitweb in a rboken state _at this point in series_.
Thank you for thinking about this, because it could cause problems
(could because I have not checked if it does or if it doesn't) in the
following patch, when gitweb uses eval / die for error handling.
Then it might happen when $per_request_config is false or CODE that
instead of trying to reread broken config on subsequent requests, we
will run with broken config. It depends if "die"-ing in
evaluate_gitweb_config would prevent setting $first_request to false.
I'd have to check that.
>> For CGI server it doesn't matter anyway, as for each request the process
>> is respawned anyway (together with respawning Perl interpreter), and I
>> think that ModPerl::Registry and FastCGI servers monitor process that it
>> is to serve requests, and respawn it if/when it dies.
>
> Sorry, that was unclear of me. I meant that buggy configuration could
> leave a gitweb process in buggy but alive state and frequent failing
> requests might be a way to notice that. Contrived example (just to
> illustrate what I mean):
>
> our $version .= ".custom";
> if (length $version>= 1000) { # untested, buggy code goes here.
> @diff_opts = ("--nonsense");
> }
>
> I think I was not right to worry about this, either. It is better to
> make such unusual and buggy configurations as noticeable as possible
> so they can be fixed.
See above.
> [...]
>> But actually handle_errors_html gets called only from fatalsToBrowser,
>> which in turn gets called from CGI::Carp::die... which ends calling
>> CODE::die (aka realdie), which ends CGI process anyway.
>>
>> That is why die_error ends with
>>
>> goto DONE_GITWEB
>> unless ($opts{'-error_handler'});
>>
>> i.e. it doesn't goto DONE_GITWEB nor DONE_REQUEST if called from
>> handle_errors_html anyway.
> [...]
>> Thanks a lot for your comments.
Which should make it in either commit message, or comments, I guess.
> Thanks for a thorough explanation. For what it's worth, with or
> without removal of the DONE_GITWEB: label,
>
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Thanks.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re[2]: [PATCH 3/3] Fixes bug: GIT_PS1_SHOWDIRTYSTATE is no not respect diff.ignoreSubmodules config variable
From: Алексей Крезов @ 2010-12-26 22:25 UTC (permalink / raw)
To: Jens Lehmann; +Cc: git, Johannes Schindelin
In-Reply-To: <4D15E48A.9050805@web.de>
Hello!
I`m sorry, but I`m newbie in making and distributing of public patches.
So, don't beat me much ))
JL> it would have been easier for me if the commit message would have
JL> described the problem you tried to fix a bit more in detail ;-).
My problem was in the following.
I use Git on Windows XP under Cygwin. So its perfomance is slower than
on *nix, I guess.
My project has 40 submodules (external libs) and some of them could
have untracked files (for some reasons). So when I run any command in Bash
after its execution Bash "thought" for 2-3 seconds. That was annoying.
I do not remember the version of Git I used at that moment but I
remember it was an update from 1.6.x to early 1.7.x. So I decided to roll back
to 1.6.x ))
Then there was some updates of Git. But after updating the problem
still happened.
When I tried to discover the reason of such a behaviour I found that
Git got status for all submodules including untracked content and so
marked them with *
Then I read manual and found diff.ignoreSubmodules and tried to set up for each
submodule in a .gitmodules but nothing changed (as it seemed to me at
that time).
So I've found the easiest way to solve my problem - this patch )
Maybe after this patch there was some changes in Git solved this
problem but I did not investigate it, sorry.
JL> 2) If diff.ignoreSubmodules is unset it leads to an error
JL> every time the prompt is displayed:
JL> 'fatal: bad --ignore-submodules argument:'
Yeah, you're right
JL> Am 25.12.2010 02:20, schrieb Zapped:
>> Signed-off-by: Zapped <zapped@mail.ru>
>> ---
>> contrib/completion/git-completion.bash | 3 ++-
>> 1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>> index d3037fc..50fc385 100755
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -280,7 +280,8 @@ __git_ps1 ()
>> elif [ "true" = "$(git rev-parse --is-inside-work-tree 2>/dev/null)" ]; then
>> if [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" ]; then
>> if [ "$(git config --bool bash.showDirtyState)" != "false" ]; then
>> - git diff --no-ext-diff --quiet --exit-code || w="*"
>> + is=$(git config diff.ignoreSubmodules)
>> + git diff --no-ext-diff --quiet --exit-code --ignore-submodules=$is || w="*"
>> if git rev-parse --quiet --verify HEAD >/dev/null; then
>> git diff-index --cached --quiet HEAD -- || i="+"
>> else
JL> Thanks for resubmitting this as an inline patch for review (although
JL> it would have been easier for me if the commit message would have
JL> described the problem you tried to fix a bit more in detail ;-).
JL> After testing this patch it looks like it has a few issues:
JL> 1) it will break any per-submodule configuration done via
JL> the 'submodule.<name>.ignore' setting in .git/config or
JL> .gitmodules, as using the --ignore-submodules option
JL> overrides those while only setting 'diff.ignoreSubmodules'
JL> should not do that.
JL> 2) If diff.ignoreSubmodules is unset it leads to an error
JL> every time the prompt is displayed:
JL> 'fatal: bad --ignore-submodules argument:'
JL> 3) And for me it didn't change the behavior at all:
JL> - The '*' in the prompt vanishes as I set diff.ignoreSubmodules
JL> as expected with or without your patch.
JL> Am I missing something here?
JL> - The real problem here is that the '+' never goes away even
JL> when 'diff.ignoreSubmodules' is set to 'all'. This is due
JL> to the fact that 'diff.ignoreSubmodules' is only honored by
JL> "git diff", but not by "git diff-index".
JL> So the real issue here seems to be the "git diff-index" call, which
JL> doesn't honor the 'diff.ignoreSubmodules' setting. In commit 37aea37
JL> Dscho (CCed) introduced this configuration setting while explicitly
JL> stating that it only affects porcelain. As the other config options
JL> always influence porcelain and plumbing, it looks like we would want
JL> to have this option honored by plumbing too, no?
JL> So are there any reasons for the plumbing diff commands not to honor
JL> the diff.ignoreSubmodules setting?
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2010, #06; Tue, 21)
From: Junio C Hamano @ 2010-12-26 19:28 UTC (permalink / raw)
To: Thomas Rast; +Cc: Yann Dirson, git
In-Reply-To: <201012261146.35961.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> Junio C Hamano wrote:
>> * yd/dir-rename (2010-10-29) 5 commits
>> - Allow hiding renames of individual files involved in a directory rename.
>> - Unified diff output format for bulk moves.
>> - Add testcases for the --detect-bulk-moves diffcore flag.
>> - Raw diff output format for bulk moves.
>> - Introduce bulk-move detection in diffcore.
>>
>> Need to re-queue the reroll.
>
> This BTW does not even compile on OS X because of its use of memrchr.
Thanks for an early warning. I am planning to do an 1.7.4-rc0 soon
anyway, so in the meantime I'd drop this and wait for a resubmission for
the next round (I think I saw a larger re-roll that I didn't pick up).
^ permalink raw reply
* Re: [PATCH 1/2] t1501: avoid bashisms
From: Junio C Hamano @ 2010-12-26 19:23 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Andreas Schwab, git, Michel Briand
In-Reply-To: <AANLkTi=fCa4a-POoMWqnraFjz28-Ko4Yp0fcar+d7AsE@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> Junio, please don't pick up this patch.
Won't. And [2/2] needs to be updated with something like this?
t/t1501-worktree.sh | 8 ++------
1 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh
index 1429a65..8cab065 100755
--- a/t/t1501-worktree.sh
+++ b/t/t1501-worktree.sh
@@ -341,14 +341,10 @@ test_expect_success 'make_relative_path handles double slashes in GIT_DIR' '
'
test_expect_success 'relative $GIT_WORK_TREE and git subprocesses' '
- (
- GIT_DIR=repo.git
- GIT_WORK_TREE=repo.git/work
- export GIT_DIR GIT_WORK_TREE
+ GIT_DIR=repo.git GIT_WORK_TREE=repo.git/work \
test-subprocess rev-parse --show-toplevel >actual &&
- echo "`pwd`/repo.git/work" >expected &&
+ echo "$(pwd)/repo.git/work" >expected &&
test_cmp expected actual
- )
'
test_done
^ permalink raw reply related
* Re: [PATCH] close file on error in read_mmfile()
From: Junio C Hamano @ 2010-12-26 19:18 UTC (permalink / raw)
To: René Scharfe; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <4D15E5D6.2040800@lsrfire.ath.cx>
Thanks.
^ permalink raw reply
* Re: [PATCH 3/3] Fixes bug: GIT_PS1_SHOWDIRTYSTATE is no not respect diff.ignoreSubmodules config variable
From: Junio C Hamano @ 2010-12-26 19:14 UTC (permalink / raw)
To: Jens Lehmann; +Cc: Zapped, git, Johannes Schindelin
In-Reply-To: <4D15E48A.9050805@web.de>
Jens Lehmann <Jens.Lehmann@web.de> writes:
> So are there any reasons for the plumbing diff commands not to honor
> the diff.ignoreSubmodules setting?
There are two kinds of users of the plumbing.
One class of plumbing users is scripts that is about automation and
mechanization that want to control what they do precisely (think cron
jobs) without getting affected by the user preference stored in the
repository configuration. This class could either:
(1) state what they want explicitly from the command line; or
(2) rely on built-in defaults not changing underneath them.
The behaviour of diff recursively inspecting submodule dirtiness has an
unfortunate history, in that the behaviour changed over time, and in each
step when we made a change, we thought we were making an unquestionable
improvement. Originally we only said "submodule HEAD is different from
what we have in the index/superproject HEAD". Later we added different
kind of dirtiness like untracked files or modified contents in submodules,
decided perhaps mistakenly that majority of users do want to see them as
dirtiness and made that the default and allowed them to be ignored by an
explicit request. At that point, in order not to break existing scripts
(mostly of the "mechanization" class, written back when there was no such
extra dirtyness hence with no "explicit refusal" route available to them
without rewriting), hence "no configuration should affect plumbing
randomly" policy.
On the other hand, you may write user facing Porcelain in scripts and run
plumbing from there. This class of plumbing users could either:
(1) inspect the config itself, interpret the customization and pass
an explicit command line flag; or
(2) allow the plumbing honor the end user configuration stored in the
repository or user configuration files.
It is argurably more convenient for these users if the plumbing blindly
honored the configurations, as it would have allowed the latter
implementation. That way, we can be more lazy when writing our scripts,
and ignore having to worry about new kinds of customization added to
underlying git after a script is written---but new kinds of customization
may break your script's expectation of what will and what will not be made
customizable, and you would end up giving an explicit "do not use that
feature" in some cases, so the being able to be lazy is not necessarily
always a win.
Things may have been a bit different if the original feature change to
inspect submodules deeper, command line flags to control that behaviour
and configuration to default the flags came at the same time, but
unfortunately they happend over time. I think we have been slowly getting
better at this, but in the case of this particular feature, the original
introduction of --ignore-submodules was in May 2008, deeper submodule
inspection and the richer --ignore-submodules=<kind> option came much
later in June 2010, and the configuration was invented later in August
2010, which would mean that allowing the plumbing to honor configuration
would have broken scripts written in the 2 years and 3 months period.
And no, this does not call for a blanket "do / do not honor configuration"
option to plumbing commands. A more selective "do / do not honor these
configuration variables" option might be an option, though.
By the way, could we please have a real sign-off, not with a one with a
pseudonym, given to the series?
^ permalink raw reply
* Re: [PATCH] Add support for -p/--patch to git-commit
From: Conrad Irwin @ 2010-12-26 17:56 UTC (permalink / raw)
To: Matthieu.Moy; +Cc: git
In-Reply-To: <vpqr5d4cx9e.fsf@bauges.imag.fr>
This works in the same was as git commit --interactive, and is
equivalent to running git add -p before git commit.
Signed-off-by: Conrad Irwin <conrad.irwin@gmail.com>
---
On 26 December 2010 16:30, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:
>
> At least one mistake:
>
> To: unlisted-recipients:; (no To-header on input)
>
> (I guess you've Bcc-ed the Git list, please don't do that)
Sorry, failed to use sendmail. (having read dire warnings about sending
patches with gmail), second time lucky...
> > While this patch works as advertised, I wonder if it would be nicer to
> > change the behaviour of git commit --interactive and git commit -p to
> > act on a temporary copy of the index rather than mutating the existing
> > index. I've no idea how to go about that yet, but is it something that
> > should be changed?
>
> I don't think so. After a commit, I usually expect the index to be
> clean, ready to start preparing the next commit (except if I
> explicitely asked the opposite), which implies that the index used for
> commit (-i|-p) is the same as the usual one.
The reason I suggested this is so that if you abort the commit (by
leaving the commit message empty), the index would be unchanged; at the
moment if you abort the commit the git-add is remembered. Certainly any
changes committed would be removed from the index. It also would allow
for git commit -p --only instead of having it always work like --include
(which might even be better default behaviour). (It's also worth noting
that git commit -i is --include, not --interactive)
>
> If I read correctly, this forbids "git commit --interactive --patch",
> while "git add --interactive --patch" is allowed, and equivalent to
> "--patch" alone.
Well spotted :). Relatedly, git-commit currently forbids --interactive
with paths, which should also be changed (though in a different commit I
assume); I did not copy that limitation to --patch.
I've updated this patch, thank you very much for the feedback.
Conrad
Documentation/git-commit.txt | 24 ++++++++++++++++--------
builtin/add.c | 6 +++---
builtin/commit.c | 11 ++++++-----
commit.h | 2 +-
4 files changed, 26 insertions(+), 17 deletions(-)
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index b586c0f..6e7ab5a 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -8,11 +8,12 @@ git-commit - Record changes to the repository
SYNOPSIS
--------
[verse]
-'git commit' [-a | --interactive] [-s] [-v] [-u<mode>] [--amend] [--dry-run]
- [(-c | -C | --fixup | --squash) <commit>] [-F <file> | -m <msg>]
- [--reset-author] [--allow-empty] [--allow-empty-message] [--no-verify]
- [-e] [--author=<author>] [--date=<date>] [--cleanup=<mode>]
- [--status | --no-status] [-i | -o] [--] [<file>...]
+'git commit' [-a | --interactive | --patch] [-s] [-v] [-u<mode>] [--amend]
+ [--dry-run] [(-c | -C | --fixup | --squash) <commit>]
+ [-F <file> | -m <msg>] [--reset-author] [--allow-empty]
+ [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
+ [--date=<date>] [--cleanup=<mode>] [--status | --no-status]
+ [-i | -o] [--] [<file>...]
DESCRIPTION
-----------
@@ -39,9 +40,10 @@ The content to be added can be specified in several ways:
that have been removed from the working tree, and then perform the
actual commit;
-5. by using the --interactive switch with the 'commit' command to decide one
- by one which files should be part of the commit, before finalizing the
- operation. Currently, this is done by invoking 'git add --interactive'.
+5. by using the --interactive or --patch switches with the 'commit' command
+ to decide one by one which files or hunks should be part of the commit,
+ before finalizing the operation. Currently, this is done by invoking
+ 'git add --interactive' or 'git add --patch'.
The `--dry-run` option can be used to obtain a
summary of what is included by any of the above for the next
@@ -59,6 +61,12 @@ OPTIONS
been modified and deleted, but new files you have not
told git about are not affected.
+-p::
+--patch::
+ Use the interactive patch selection interface to chose
+ which changes to commit. See linkgit:git-add[1] for
+ details.
+
-C <commit>::
--reuse-message=<commit>::
Take an existing commit object, and reuse the log message
diff --git a/builtin/add.c b/builtin/add.c
index 12b964e..3d074b3 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -242,7 +242,7 @@ int run_add_interactive(const char *revision, const char *patch_mode,
return status;
}
-int interactive_add(int argc, const char **argv, const char *prefix)
+int interactive_add(int argc, const char **argv, const char *prefix, int patch)
{
const char **pathspec = NULL;
@@ -253,7 +253,7 @@ int interactive_add(int argc, const char **argv, const char *prefix)
}
return run_add_interactive(NULL,
- patch_interactive ? "--patch" : NULL,
+ patch ? "--patch" : NULL,
pathspec);
}
@@ -378,7 +378,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
if (patch_interactive)
add_interactive = 1;
if (add_interactive)
- exit(interactive_add(argc - 1, argv + 1, prefix));
+ exit(interactive_add(argc - 1, argv + 1, prefix, patch_interactive));
if (edit_interactive)
return(edit_patch(argc, argv, prefix));
diff --git a/builtin/commit.c b/builtin/commit.c
index 22ba54f..454308e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -70,7 +70,7 @@ static const char *logfile, *force_author;
static const char *template_file;
static char *edit_message, *use_message;
static char *fixup_message, *squash_message;
-static int all, edit_flag, also, interactive, only, amend, signoff;
+static int all, edit_flag, also, interactive, patch_interactive, only, amend, signoff;
static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
static int no_post_rewrite, allow_empty_message;
static char *untracked_files_arg, *force_date, *ignore_submodule_arg;
@@ -138,6 +138,7 @@ static struct option builtin_commit_options[] = {
OPT_BOOLEAN('a', "all", &all, "commit all changed files"),
OPT_BOOLEAN('i', "include", &also, "add specified files to index for commit"),
OPT_BOOLEAN(0, "interactive", &interactive, "interactively add files"),
+ OPT_BOOLEAN('p', "patch", &patch_interactive, "interactively add changes"),
OPT_BOOLEAN('o', "only", &only, "commit only specified files"),
OPT_BOOLEAN('n', "no-verify", &no_verify, "bypass pre-commit hook"),
OPT_BOOLEAN(0, "dry-run", &dry_run, "show what would be committed"),
@@ -296,8 +297,8 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int
if (is_status)
refresh_flags |= REFRESH_UNMERGED;
- if (interactive) {
- if (interactive_add(argc, argv, prefix) != 0)
+ if (interactive || patch_interactive) {
+ if (interactive_add(argc, argv, prefix, patch_interactive) != 0)
die("interactive add failed");
if (read_cache_preload(NULL) < 0)
die("index file corrupt");
@@ -969,8 +970,8 @@ static int parse_and_validate_options(int argc, const char *argv[],
use_message_buffer = xstrdup(commit->buffer);
}
- if (!!also + !!only + !!all + !!interactive > 1)
- die("Only one of --include/--only/--all/--interactive can be used.");
+ if (!!also + !!only + !!all + !!(interactive || patch_interactive) > 1)
+ die("Only one of --include/--only/--all/--interactive/--patch can be used.");
if (argc == 0 && (also || (only && !amend)))
die("No paths with --include/--only does not make sense.");
if (argc == 0 && only && amend)
diff --git a/commit.h b/commit.h
index eb6c5af..951c22e 100644
--- a/commit.h
+++ b/commit.h
@@ -160,7 +160,7 @@ extern struct commit_list *get_shallow_commits(struct object_array *heads,
int is_descendant_of(struct commit *, struct commit_list *);
int in_merge_bases(struct commit *, struct commit **, int);
-extern int interactive_add(int argc, const char **argv, const char *prefix);
+extern int interactive_add(int argc, const char **argv, const char *prefix, int patch);
extern int run_add_interactive(const char *revision, const char *patch_mode,
const char **pathspec);
--
1.7.3.4.627.g3cfc9
^ permalink raw reply related
* Re: [PATCH] Add support for -p/--patch to git-commit
From: Matthieu Moy @ 2010-12-26 16:30 UTC (permalink / raw)
To: Conrad Irwin; +Cc: git
In-Reply-To: <E1PWsuV-0000FH-90@scarlatti.dunvegan.biz>
Conrad Irwin <conrad.irwin@gmail.com> writes:
> Hello Git,
>
> Please let me know of any mistakes I've made, this is a first for
> me.
At least one mistake:
To: unlisted-recipients:; (no To-header on input)
(I guess you've Bcc-ed the Git list, please don't do that)
> While this patch works as advertised, I wonder if it would be nicer to
> change the behaviour of git commit --interactive and git commit -p to
> act on a temporary copy of the index rather than mutating the existing
> index. I've no idea how to go about that yet, but is it something that
> should be changed?
I don't think so. After a commit, I usually expect the index to be
clean, ready to start preparing the next commit (except if I
explicitely asked the opposite), which implies that the index used for
commit (-i|-p) is the same as the usual one.
> +5. by using the --interactive or --patch switches with the 'commit' command
> + to decide one by one which files or hunks should be part of the commit,
> + before finalizing the operation. Currently, this is done by invoking
> + 'git add --interactive'.
... or git add --patch.
> - if (!!also + !!only + !!all + !!interactive > 1)
> - die("Only one of --include/--only/--all/--interactive can be used.");
> + if (!!also + !!only + !!all + !!interactive + !!patch_interactive > 1)
> + die("Only one of --include/--only/--all/--interactive/--patch can be used.");
If I read correctly, this forbids "git commit --interactive --patch",
while "git add --interactive --patch" is allowed, and equivalent to
"--patch" alone.
Other than that, the patch looks good.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* [PATCH] Add support for -p/--patch to git-commit
From: Conrad Irwin @ 2010-12-26 15:36 UTC (permalink / raw)
This works in the same was as git commit --interactive, and is
equivalent to running git add -p before git commit.
Signed-off-by: Conrad Irwin <conrad.irwin@gmail.com>
---
Hello Git,
Please let me know of any mistakes I've made, this is a first for me.
While this patch works as advertised, I wonder if it would be nicer to
change the behaviour of git commit --interactive and git commit -p to
act on a temporary copy of the index rather than mutating the existing
index. I've no idea how to go about that yet, but is it something that
should be changed?
Conrad
Documentation/git-commit.txt | 24 ++++++++++++++++--------
builtin/add.c | 6 +++---
builtin/commit.c | 11 ++++++-----
commit.h | 2 +-
4 files changed, 26 insertions(+), 17 deletions(-)
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index b586c0f..fe3a14f 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -8,11 +8,12 @@ git-commit - Record changes to the repository
SYNOPSIS
--------
[verse]
-'git commit' [-a | --interactive] [-s] [-v] [-u<mode>] [--amend] [--dry-run]
- [(-c | -C | --fixup | --squash) <commit>] [-F <file> | -m <msg>]
- [--reset-author] [--allow-empty] [--allow-empty-message] [--no-verify]
- [-e] [--author=<author>] [--date=<date>] [--cleanup=<mode>]
- [--status | --no-status] [-i | -o] [--] [<file>...]
+'git commit' [-a | --interactive | --patch] [-s] [-v] [-u<mode>] [--amend]
+ [--dry-run] [(-c | -C | --fixup | --squash) <commit>]
+ [-F <file> | -m <msg>] [--reset-author] [--allow-empty]
+ [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
+ [--date=<date>] [--cleanup=<mode>] [--status | --no-status]
+ [-i | -o] [--] [<file>...]
DESCRIPTION
-----------
@@ -39,9 +40,10 @@ The content to be added can be specified in several ways:
that have been removed from the working tree, and then perform the
actual commit;
-5. by using the --interactive switch with the 'commit' command to decide one
- by one which files should be part of the commit, before finalizing the
- operation. Currently, this is done by invoking 'git add --interactive'.
+5. by using the --interactive or --patch switches with the 'commit' command
+ to decide one by one which files or hunks should be part of the commit,
+ before finalizing the operation. Currently, this is done by invoking
+ 'git add --interactive'.
The `--dry-run` option can be used to obtain a
summary of what is included by any of the above for the next
@@ -59,6 +61,12 @@ OPTIONS
been modified and deleted, but new files you have not
told git about are not affected.
+-p::
+--patch::
+ Use the interactive patch selection interface to chose
+ which changes to commit. See linkgit:git-add[1] for
+ details.
+
-C <commit>::
--reuse-message=<commit>::
Take an existing commit object, and reuse the log message
diff --git a/builtin/add.c b/builtin/add.c
index 12b964e..3d074b3 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -242,7 +242,7 @@ int run_add_interactive(const char *revision, const char *patch_mode,
return status;
}
-int interactive_add(int argc, const char **argv, const char *prefix)
+int interactive_add(int argc, const char **argv, const char *prefix, int patch)
{
const char **pathspec = NULL;
@@ -253,7 +253,7 @@ int interactive_add(int argc, const char **argv, const char *prefix)
}
return run_add_interactive(NULL,
- patch_interactive ? "--patch" : NULL,
+ patch ? "--patch" : NULL,
pathspec);
}
@@ -378,7 +378,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
if (patch_interactive)
add_interactive = 1;
if (add_interactive)
- exit(interactive_add(argc - 1, argv + 1, prefix));
+ exit(interactive_add(argc - 1, argv + 1, prefix, patch_interactive));
if (edit_interactive)
return(edit_patch(argc, argv, prefix));
diff --git a/builtin/commit.c b/builtin/commit.c
index 22ba54f..0ae3262 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -70,7 +70,7 @@ static const char *logfile, *force_author;
static const char *template_file;
static char *edit_message, *use_message;
static char *fixup_message, *squash_message;
-static int all, edit_flag, also, interactive, only, amend, signoff;
+static int all, edit_flag, also, interactive, patch_interactive, only, amend, signoff;
static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
static int no_post_rewrite, allow_empty_message;
static char *untracked_files_arg, *force_date, *ignore_submodule_arg;
@@ -138,6 +138,7 @@ static struct option builtin_commit_options[] = {
OPT_BOOLEAN('a', "all", &all, "commit all changed files"),
OPT_BOOLEAN('i', "include", &also, "add specified files to index for commit"),
OPT_BOOLEAN(0, "interactive", &interactive, "interactively add files"),
+ OPT_BOOLEAN('p', "patch", &patch_interactive, "interactively add changes"),
OPT_BOOLEAN('o', "only", &only, "commit only specified files"),
OPT_BOOLEAN('n', "no-verify", &no_verify, "bypass pre-commit hook"),
OPT_BOOLEAN(0, "dry-run", &dry_run, "show what would be committed"),
@@ -296,8 +297,8 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int
if (is_status)
refresh_flags |= REFRESH_UNMERGED;
- if (interactive) {
- if (interactive_add(argc, argv, prefix) != 0)
+ if (interactive || patch_interactive) {
+ if (interactive_add(argc, argv, prefix, patch_interactive) != 0)
die("interactive add failed");
if (read_cache_preload(NULL) < 0)
die("index file corrupt");
@@ -969,8 +970,8 @@ static int parse_and_validate_options(int argc, const char *argv[],
use_message_buffer = xstrdup(commit->buffer);
}
- if (!!also + !!only + !!all + !!interactive > 1)
- die("Only one of --include/--only/--all/--interactive can be used.");
+ if (!!also + !!only + !!all + !!interactive + !!patch_interactive > 1)
+ die("Only one of --include/--only/--all/--interactive/--patch can be used.");
if (argc == 0 && (also || (only && !amend)))
die("No paths with --include/--only does not make sense.");
if (argc == 0 && only && amend)
diff --git a/commit.h b/commit.h
index eb6c5af..951c22e 100644
--- a/commit.h
+++ b/commit.h
@@ -160,7 +160,7 @@ extern struct commit_list *get_shallow_commits(struct object_array *heads,
int is_descendant_of(struct commit *, struct commit_list *);
int in_merge_bases(struct commit *, struct commit **, int);
-extern int interactive_add(int argc, const char **argv, const char *prefix);
+extern int interactive_add(int argc, const char **argv, const char *prefix, int patch);
extern int run_add_interactive(const char *revision, const char *patch_mode,
const char **pathspec);
--
1.7.2.3
^ permalink raw reply related
* Re: TopGit release?
From: Uwe Kleine-König @ 2010-12-26 12:25 UTC (permalink / raw)
To: Bert Wesarg
Cc: Peter Simons, git, martin f krafft, Per Cederqvist, Olaf Dabrunz,
Thomas Moschny
In-Reply-To: <AANLkTinfDgvmodzrW7eMW-iizxpvb+gvBgSsH9B3LOdu@mail.gmail.com>
Hi Bert,
On Wed, Dec 15, 2010 at 05:48:39PM +0100, Bert Wesarg wrote:
> 2010/12/15 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> > Hi Bert,
> >
> > On Wed, Dec 15, 2010 at 03:54:28PM +0100, Bert Wesarg wrote:
> >> 2010/12/15 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> >> > [1] http://thread.gmane.org/gmane.comp.version-control.git/159433
> >> > hint to Bert: this series doesn't apply to master
> >>
> >> I know, you applied a patch, which was rendered obsolete with this
> >> patch series. you commited on Nov 02, and I send the series Oct 20.
> > If you tell me the commit you based your series on I can use the same
> > and merge the result into master.
>
> 8b0f1f9d215d767488542a7853320d1789838d92
>
> But I just refreshed my repo.or.cz fork (topgit/bertw), where I pushed
> a rebased series (I've done this some time ago already)
>
> git://repo.or.cz/topgit/bertw.git index-wt
I now finally merged that (skipping the two RFC commits) and pushed it
out. I will tag it in a few days as 0.9 giving a chance to others to
test a bit.
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [PATCH] setup_work_tree: adjust relative $GIT_WORK_TREE after moving cwd
From: Nguyễn Thái Ngọc Duy @ 2010-12-26 11:46 UTC (permalink / raw)
To: git, Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy
In-Reply-To: <1293285457-11915-2-git-send-email-pclouds@gmail.com>
When setup_work_tree() is called, it moves cwd to $GIT_WORK_TREE and
makes internal copy of $GIT_WORK_TREE absolute. The environt variable,
if set by user, remains unchanged. If the variable is relative, it is
no longer correct because its base dir has changed.
Instead of making $GIT_WORK_TREE absolute too, we just say "." and let
subsequent git processes handle it.
Reported-by: Michel Briand <michelbriand@free.fr>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
This one is better, on top of master because the next one is on top
of nd/setup.
And I forgot to tell why I did not put the fault in real life [1]
into tests: I don't like git-merge spawning another process just for
resetting worktree. Sooner or later it should be replaced to use
unpack_trees() directly. When that happens, the test would become
invalid.
[1] http://thread.gmane.org/gmane.comp.version-control.git/164066/focus=164171
.gitignore | 1 +
Makefile | 1 +
setup.c | 8 ++++++++
t/t1501-worktree.sh | 11 +++++++++++
test-subprocess.c | 21 +++++++++++++++++++++
5 files changed, 42 insertions(+), 0 deletions(-)
create mode 100644 test-subprocess.c
diff --git a/.gitignore b/.gitignore
index 87b833c..3dd6ef7 100644
--- a/.gitignore
+++ b/.gitignore
@@ -177,6 +177,7 @@
/test-sha1
/test-sigchain
/test-string-pool
+/test-subprocess
/test-svn-fe
/test-treap
/common-cmds.h
diff --git a/Makefile b/Makefile
index 57d9c65..bdf86a3 100644
--- a/Makefile
+++ b/Makefile
@@ -431,6 +431,7 @@ TEST_PROGRAMS_NEED_X += test-run-command
TEST_PROGRAMS_NEED_X += test-sha1
TEST_PROGRAMS_NEED_X += test-sigchain
TEST_PROGRAMS_NEED_X += test-string-pool
+TEST_PROGRAMS_NEED_X += test-subprocess
TEST_PROGRAMS_NEED_X += test-svn-fe
TEST_PROGRAMS_NEED_X += test-treap
TEST_PROGRAMS_NEED_X += test-index-version
diff --git a/setup.c b/setup.c
index 91887a4..3833569 100644
--- a/setup.c
+++ b/setup.c
@@ -239,6 +239,14 @@ void setup_work_tree(void)
git_dir = make_absolute_path(git_dir);
if (!work_tree || chdir(work_tree))
die("This operation must be run in a work tree");
+
+ /*
+ * Make sure subsequent git processes find correct worktree
+ * if $GIT_WORK_TREE is set relative
+ */
+ if (getenv(GIT_WORK_TREE_ENVIRONMENT))
+ setenv(GIT_WORK_TREE_ENVIRONMENT, ".", 1);
+
set_git_dir(make_relative_path(git_dir, work_tree));
initialized = 1;
}
diff --git a/t/t1501-worktree.sh b/t/t1501-worktree.sh
index 2c8f01f..1f3b50d 100755
--- a/t/t1501-worktree.sh
+++ b/t/t1501-worktree.sh
@@ -340,4 +340,15 @@ test_expect_success 'make_relative_path handles double slashes in GIT_DIR' '
git --git-dir="$(pwd)//repo.git" --work-tree="$(pwd)" add dummy_file
'
+test_expect_success 'relative $GIT_WORK_TREE and git subprocesses' '
+ (
+ GIT_DIR=repo.git &&
+ GIT_WORK_TREE=repo.git/work &&
+ export GIT_DIR GIT_WORK_TREE &&
+ test-subprocess --setup-work-tree rev-parse --show-toplevel >actual &&
+ echo "`pwd`/repo.git/work" >expected &&
+ test_cmp expected actual
+ )
+'
+
test_done
diff --git a/test-subprocess.c b/test-subprocess.c
new file mode 100644
index 0000000..667d3e5
--- /dev/null
+++ b/test-subprocess.c
@@ -0,0 +1,21 @@
+#include "cache.h"
+#include "run-command.h"
+
+int main(int argc, char **argv)
+{
+ const char *prefix;
+ struct child_process cp;
+ int nogit = 0;
+
+ prefix = setup_git_directory_gently(&nogit);
+ if (nogit)
+ die("No git repo found");
+ if (!strcmp(argv[1], "--setup-work-tree")) {
+ setup_work_tree();
+ argv++;
+ }
+ memset(&cp, 0, sizeof(cp));
+ cp.git_cmd = 1;
+ cp.argv = (const char **)argv+1;
+ return run_command(&cp);
+}
--
1.7.3.4.878.g439c7
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox