* Re: [PATCH/resend] CVS Server: Support reading base and roots from environment
From: Phil Miller @ 2009-12-30 20:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nanako Shiraishi, Git Mailing List
In-Reply-To: <7vpr5wz1hb.fsf@alter.siamese.dyndns.org>
The Gitosis single-account Git/ssh hosting system runs git commands
through git-shell after confirming that the connecting user is
authorized to access the requested repository. This works well for
upload-pack and receive-pack, which take a repository argument through
git-shell. This doesn't work so well for `cvs server', which is passed
through literally, with no arguments. Allowing arguments risks
sneaking in `--export-all', so that restriction should be maintained.
Despite that, passing a repository root is necessary for per-user
access control by the hosting software, and passing a base path
improves usability without weakening security. Thus, git-cvsserver
needs to come up with these values at runtime by some other
means. Since git-shell preserves the environment for other purposes,
the environment can carry these arguments as well.
Thus, modify git-cvsserver to read $GIT_CVSSERVER_{BASE_PATH,ROOT} in
the absence of equivalent command line arguments.
Signed-off-by: Phil Miller <mille121@illinois.edu>
---
I believe this revision addresses all of your comments on the first submission.
Your comment about cramming multiple values into one environment variable made
me realize that more than one simply was unnecessary complexity, since gitosis
expects to authenticate access to a single repository anyway.
I've not documented what GIT_CVSSERVER_BASE_PATH is relative to, because it
behaves identically to the --base-path command line argument. Documenting
what that is relative to is a separate issue.
Documentation/git-cvsserver.txt | 15 +++++++++++++++
git-cvsserver.perl | 22 +++++++++++++++++++++-
2 files changed, 36 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-cvsserver.txt b/Documentation/git-cvsserver.txt
index 99a7c14..fbab295 100644
--- a/Documentation/git-cvsserver.txt
+++ b/Documentation/git-cvsserver.txt
@@ -277,6 +277,21 @@ In `dbdriver` and `dbuser` you can use the
following variables:
If no name can be determined, the
numeric uid is used.
+ENVIRONMENT
+-----------
+
+These variables obviate the need for command-line options in some
+circumstances, allowing easier restricted usage through git-shell.
+
+GIT_CVSSERVER_BASE_PATH takes the place of the argument to --base-path.
+
+GIT_CVSSERVER_ROOT specifies a single-directory whitelist. The
+repository must still be configured to allow access through
+git-cvsserver, as described above.
+
+When these environment variables are set, the corresponding
+command-line arguments may not be used.
+
Eclipse CVS Client Notes
------------------------
diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 6dc45f5..f5b57b9 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -104,6 +104,7 @@ $log->info("--------------- STARTING -----------------");
my $usage =
"Usage: git cvsserver [options] [pserver|server] [<directory> ...]\n".
" --base-path <path> : Prepend to requested CVSROOT\n".
+ " Can be read from GIT_CVSSERVER_BASE_PATH\n".
" --strict-paths : Don't allow recursing into subdirectories\n".
" --export-all : Don't check for gitcvs.enabled in config\n".
" --version, -V : Print version information and exit\n".
@@ -111,7 +112,8 @@ my $usage =
"\n".
"<directory> ... is a list of allowed directories. If no directories\n".
"are given, all are allowed. This is an additional restriction, gitcvs\n".
- "access still needs to be enabled by the gitcvs.enabled config option.\n";
+ "access still needs to be enabled by the gitcvs.enabled config option.\n".
+ "Alternately, one directory may be specified in GIT_CVSSERVER_ROOT.\n";
my @opts = ( 'help|h|H', 'version|V',
'base-path=s', 'strict-paths', 'export-all' );
@@ -148,6 +150,24 @@ if ($state->{'export-all'} &&
!@{$state->{allowed_roots}}) {
die "--export-all can only be used together with an explicit whitelist\n";
}
+# Environment handling for running under git-shell
+if (exists $ENV{GIT_CVSSERVER_BASE_PATH}) {
+ if ($state->{'base-path'}) {
+ die "Cannot specify base path both ways.\n";
+ }
+ my $base_path = $ENV{GIT_CVSSERVER_BASE_PATH};
+ $state->{'base-path'} = $base_path;
+ $log->debug("Picked up base path '$base_path' from environment.\n");
+}
+if (exists $ENV{GIT_CVSSERVER_ROOT}) {
+ if (@{$state->{allowed_roots}}) {
+ die "Cannot specify roots both ways: @ARGV\n";
+ }
+ my $allowed_root = $ENV{GIT_CVSSERVER_ROOT};
+ $state->{allowed_roots} = [ $allowed_root ];
+ $log->debug("Picked up allowed root '$allowed_root' from environment.\n");
+}
+
# if we are called with a pserver argument,
# deal with the authentication cat before entering the
# main loop
--
debian.1.6.6_rc2.1.7.gc3ed7
^ permalink raw reply related
* Re: Question about 'branch -d' safety
From: Nicolas Sebrecht @ 2009-12-30 21:08 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: Nicolas Sebrecht, git
In-Reply-To: <20091230121238.6117@nanako3.lavabit.com>
The 30/12/09, Nanako Shiraishi wrote:
> Quoting Nicolas Sebrecht <nicolas.s.dev@gmx.fr>
>
> > But even with it, we would hit some foreign workflow. Think: Bob
> > directly push to Alice and Alice does the same to Bob. I don't use this
> > kind of workflow myself but I consider them to be sensible enough to
> > have our attention.
>
> Here is what I think about your scenario.
>
> Bob directly pushes to Alice and Alice does the same to Bob, both to their refs/remotes/<other person>/ tracking branches
We can't say. They both may have refs/remotes/<same_id> .
Bob:
$ git branch -d a_branch
Now, Bob has the "I don't want to loose" commit known in
refs/remotes/<same_id> only.
Alice, some time later:
$ git push -f <same_id> <foo>:<a_branch>
Bob "lose" his commit.
I admit it is a very uncommon use case and Bob can still use the reflog.
--
Nicolas Sebrecht
^ permalink raw reply
* __git_ps1 not showing submodule dirty state
From: Kevin Ballard @ 2009-12-30 21:46 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
Why does the __git_ps1 function in git-completion.bash explicitly ignore submodules when showing the GIT_PS1_SHOWDIRTYSTATE status? The most common issue with my current repository is not realizing when submodules need to be updated because I blindly trust my prompt to tell me when I have dirty state.
-Kevin Ballard
^ permalink raw reply
* Re: Need some help with git rebase
From: Sylvain Rabot @ 2009-12-30 22:24 UTC (permalink / raw)
To: git
In-Reply-To: <4B38B3A7.6010900@steek.com>
So,
I tried again "git rebase --onto 12.72.1 master feature".
Situation :
a--a--a--a--a--a--a--a--a feature
/
--x--x--x--x--x--x--x--x--x--x--x--x master
\
o--o--o--o--o--o--o 12.72.1
Work flow :
$ git checkout 12.72.1
$ git rebase --onto 12.72.1 master feature
a lot of conflicts, resolving and git rebase --continue
And then, at the end of the rebase, without me doing anything, feature
branch is checked out and it seems that its HEAD has been reset to the
new 12.72.1 HEAD.
--x--x--x--x--x--x--x--x--x--x--x--x master
\
o--o--o--o--o--o--o--a--a--a--a--a--a--a--a--a 12.72.1 feature
Is that normal ? If it is, how do I do to avoid my feature branch to be
reset at 12.72.1's HEAD ?
Any enlightenment is very welcome.
Regards.
--
Sylvain Rabot <sylvain@abstraction.fr>
^ permalink raw reply
* Re: Need some help with git rebase
From: Wincent Colaiuta @ 2009-12-30 22:46 UTC (permalink / raw)
To: sylvain; +Cc: git
In-Reply-To: <1262211866.7068.1.camel@kheops>
El 30/12/2009, a las 23:24, Sylvain Rabot escribió:
> So,
>
> I tried again "git rebase --onto 12.72.1 master feature".
>
> Situation :
>
> a--a--a--a--a--a--a--a--a feature
> /
> --x--x--x--x--x--x--x--x--x--x--x--x master
> \
> o--o--o--o--o--o--o 12.72.1
>
>
> Work flow :
>
> $ git checkout 12.72.1
> $ git rebase --onto 12.72.1 master feature
>
> a lot of conflicts, resolving and git rebase --continue
>
> And then, at the end of the rebase, without me doing anything, feature
> branch is checked out and it seems that its HEAD has been reset to the
> new 12.72.1 HEAD.
>
> --x--x--x--x--x--x--x--x--x--x--x--x master
> \
> o--o--o--o--o--o--o--a--a--a--a--a--a--a--a--a 12.72.1 feature
>
> Is that normal ? If it is, how do I do to avoid my feature branch to
> be
> reset at 12.72.1's HEAD ?
>
> Any enlightenment is very welcome.
Look at the "git-rebase" man page, particularly the order of the
arguments, what they mean, and the usage examples of "--onto":
$ git rebase --onto 12.72.1 master feature
Means, "replay these changes on top of 12.72.1", where "these changes"
refers to commits on branch "feature" with upstream "master", which is
what "git rebase" did for you.
If you actually want the "feature" branch to continue pointing at the
old feature branch rather than your newly rebased one, you could just
look up the old SHA1 for it and update it with:
$ git branch -f feature abcd1234
Where "abcd1234" is the hash of the old "feature" HEAD.
But I don't really know why you'd want to do that. The purpose of "git
rebase" isn't to copy or cherry-pick commits from one branch onto
another, but to actually _move_ (or transplant, or replay, if you
prefer) those commits.
Maybe I misunderstood your intentions though.
Cheers,
Wincent
^ permalink raw reply
* Re: __git_ps1 not showing submodule dirty state
From: Johan Herland @ 2009-12-31 1:40 UTC (permalink / raw)
To: git; +Cc: Kevin Ballard, Shawn O. Pearce, trast
In-Reply-To: <A98B75F4-B9BD-4CB4-9335-754C59ECB64F@sb.org>
On Wednesday 30 December 2009, Kevin Ballard wrote:
> Why does the __git_ps1 function in git-completion.bash explicitly ignore
> submodules when showing the GIT_PS1_SHOWDIRTYSTATE status? The most
> common issue with my current repository is not realizing when submodules
> need to be updated because I blindly trust my prompt to tell me when I
> have dirty state.
According to git blame, it has been there since GIT_PS1_SHOWDIRTYSTATE was
introduced in 738a94a... by Thomas Rast (CCed), but the commit message does
not say why submodules are explicitly ignored.
FWIW, I agree with Kevin, and would like changed submodules to be included
in the status.
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [PATCH v2] Smart-http documentation: add example of how to execute from userdir
From: Tay Ray Chuan @ 2009-12-31 2:18 UTC (permalink / raw)
To: Tarmigan Casebolt; +Cc: Junio C Hamano, git, Shawn O. Pearce
In-Reply-To: <1261975612-67101-1-git-send-email-tarmigan+git@gmail.com>
Hi,
On Mon, Dec 28, 2009 at 12:46 PM, Tarmigan Casebolt
<tarmigan+git@gmail.com> wrote:
> Looking at the
> s/update/updates/
> suggestion again, I decided not to make that change because I think
> the original is grammical even if a little bit awkward. The first
> 'updates' is the noun, so the verb should be 'update' without an 's'.
Indeed.
> I also tried rephrasing that sentence completely, but did not end up
> with something better. ?Suggestions welcome.
My attempt is found below.
> +In the following example, a repository at
I was thinking s/a repository at/the repository/.
> +'/home/$username/devel/foo/bar.git' will be accessible at
> +'http://$hostname/\~$username/cgi-bin/git/foo/bar.git'
You left out a full-stop for this sentence.
> +From UserDir on Apache 2.x::
> + One way to configure 'git-http-backend' to execute and serve
> + from a user directory (for example, on a shared hosting
> + provider), is to have a symbolic link named 'git' that points
> + from the cgi directory to the 'git-http-backend' executable in
> + libexec. The advantage of the symbolic link is that any updates
> + to the installed version of 'git-http-backend' also update the
> + version that is run in the userdir.
We could try
The advantage is that this symbolic link always points to the latest
installed version; one does not have to make any changes to the
symbolic link when the installed 'git-http-backend' is updated.
I thought the corollary (after the ';') was a bit long-winded, but it
explains the advantage more clearly.
--
Cheers,
Ray Chuan
^ permalink raw reply
* [PATCH] bash completion: add space between branch name and status flags
From: Shawn O. Pearce @ 2009-12-31 3:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nanako Shiraishi, git, Roman Fietze
In-Reply-To: <7veimc1cv6.fsf@alter.siamese.dyndns.org>
Improve the readability of the bash prompt by adding a space between
the branch name and the status flags (dirty, stash, untracked).
While we are cleaning up this section of code, the two cases for
formatting the prompt are identical except for the format string,
so make them the same.
Suggested-by: Roman Fietze <roman.fietze@telemotive.de>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
Junio C Hamano <gitster@pobox.com> wrote:
> I notice that printf argument look very similar. Maybe we want to do
> something like
>
> printf "${1:-" (%s)"}" ...
>
> to avoid duplication?
Ack.
Because its rather far from the original poster's patch, I've
taken blame for it.
contrib/completion/git-completion.bash | 8 +++-----
1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index fbfa5f2..9ed7df2 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -163,11 +163,9 @@ __git_ps1 ()
fi
fi
- if [ -n "${1-}" ]; then
- printf "$1" "$c${b##refs/heads/}$w$i$s$u$r"
- else
- printf " (%s)" "$c${b##refs/heads/}$w$i$s$u$r"
- fi
+ local f="$w$i$s$u"
+ f="${f:+ $f}$r"
+ printf "${1:- (%s)}" "$c${b##refs/heads/}$f"
fi
}
--
1.6.6.325.g6f5f
--
Shawn.
^ permalink raw reply related
* Re: Need some help with git rebase
From: Tay Ray Chuan @ 2009-12-31 5:01 UTC (permalink / raw)
To: sylvain; +Cc: git
In-Reply-To: <1262211866.7068.1.camel@kheops>
Hi,
On Thu, Dec 31, 2009 at 6:24 AM, Sylvain Rabot <sylvain@abstraction.fr> wrote:
> And then, at the end of the rebase, without me doing anything, feature
> branch is checked out and it seems that its HEAD has been reset to the
> new 12.72.1 HEAD.
>
> --x--x--x--x--x--x--x--x--x--x--x--x master
> \
> o--o--o--o--o--o--o--a--a--a--a--a--a--a--a--a 12.72.1 feature
>
> Is that normal ? If it is, how do I do to avoid my feature branch to be
> reset at 12.72.1's HEAD ?
reading the manual, I think this is normal.
According to the third paragraph of the description of git-rebase, the
branch feature will be reset to 12.72.1.
You can just rename the branch back to 12.72.1, and retrieve the old
tip of feature by examining the reflog.
--
Cheers,
Ray Chuan
^ permalink raw reply
* Re: [Updated PATCH 1/2] Report exec errors from run-command
From: Tarmigan @ 2009-12-31 5:26 UTC (permalink / raw)
To: Ilari Liusvaara; +Cc: git
In-Reply-To: <1262170338-11574-2-git-send-email-ilari.liusvaara@elisanet.fi>
On Wed, Dec 30, 2009 at 5:52 AM, Ilari Liusvaara
<ilari.liusvaara@elisanet.fi> wrote:
> Previously run-command was unable to report errors happening in exec
> call. Change it to pass errno from failed exec to errno value at
> return.
>
> The errno value passing can be done by opening close-on-exec pipe and
> piping the error code through in case of failure. In case of success,
> close-on-exec closes the pipe on successful exec and parent process
> gets end of file on read.
I was testing pu and 'git diff' and 'git log' would hang forever.
Bisecting pointed to v1 of this patch. But seeing that v2 was out, I
tried v2 of the patch, but the issue remains.
Tried on OSX and linux with the same results.
Here's a gdb backtrace on OSX at the point where I interrupted it:
(gdb) bt
#0 0x9923dbfe in read$UNIX2003 ()
#1 0x000b399b in start_command (cmd=0x10b300) at run-command.c:110
#2 0x00099452 in setup_pager () at pager.c:94
#3 0x0002196b in cmd_diff (argc=1, argv=0xbffff42c, prefix=0x0) at
builtin-diff.c:316
#4 0x00002a2a in run_builtin [inlined] () at git.c:257
#5 0x00002a2a in handle_internal_command (argc=1, argv=0xbffff42c) at git.c:401
#6 0x00002cab in main (argc=1, argv=0xbffff42c) at git.c:443
and on linux:
(gdb) bt
#0 0x0000003530a0d590 in __read_nocancel () from /lib64/libpthread.so.0
#1 0x0000000000494858 in start_command (cmd=0x71ed60) at run-command.c:93
#2 0x000000000047daf8 in setup_pager () at pager.c:94
#3 0x000000000041d83f in cmd_diff (argc=1, argv=0x7fff256f9fe0, prefix=0x0)
at builtin-diff.c:316
#4 0x0000000000403ced in handle_internal_command (argc=1, argv=0x7fff256f9fe0)
at git.c:257
#5 0x0000000000403f26 in main (argc=1, argv=0x7fff256f9fe0) at git.c:445
Thanks,
Tarmigan
^ permalink raw reply
* Re: [PATCH v2] cvsserver: make the output of 'update' more compatible with cvs.
From: Junio C Hamano @ 2009-12-31 6:47 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: Junio C Hamano, Sergei Organov, git
In-Reply-To: <20091230224115.6117@nanako3.lavabit.com>
Nanako Shiraishi <nanako3@lavabit.com> writes:
> Junio, could you tell us what happened to this thread?
Well, since I don't use cvsserver myself, but this v2 was done with
improvements based on some review suggestions, I was waiting for a
response or two from people who know better and care more about cvs server
emulation than me, which unfortunately didn't happen.
Thanks for reminding; I can queue it to 'pu' or perhaps 'next' and see if
anybody screams by getting hit by an unintended side effects.
^ permalink raw reply
* Re: [PATCH] t9700-perl-git.sh: Fix a test failure on Cygwin
From: Junio C Hamano @ 2009-12-31 6:49 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: Junio C Hamano, Ramsay Jones, GIT Mailing-list
In-Reply-To: <20091230224023.6117@nanako3.lavabit.com>
Nanako Shiraishi <nanako3@lavabit.com> writes:
> Junio, could you tell us what happened to this thread?
Hmph, I think we have it as 81f4026 (t9700-perl-git.sh: Fix a test failure
on Cygwin, 2009-11-19).
^ permalink raw reply
* Re: [PATCH] grep: do not do external grep on skip-worktree entries
From: Junio C Hamano @ 2009-12-31 7:01 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1262182304-19911-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> Skip-worktree entries are not on disk. There is no reason to call
> external grep in such cases.
>
> A trace message is also added to aid debugging external grep cases.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> builtin-grep.c | 19 ++++++++++++++++++-
> 1 files changed, 18 insertions(+), 1 deletions(-)
>
> diff --git a/builtin-grep.c b/builtin-grep.c
> index d582232..d49c637 100644
> --- a/builtin-grep.c
> +++ b/builtin-grep.c
> @@ -488,17 +488,34 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached,
> read_cache();
>
> #if !NO_EXTERNAL_GREP
> + if (cached)
> + external_grep_allowed = 0;
> + if (external_grep_allowed) {
> + for (nr = 0; nr < active_nr; nr++) {
> + struct cache_entry *ce = active_cache[nr];
> + if (!S_ISREG(ce->ce_mode))
> + continue;
> + if (!pathspec_matches(paths, ce->name, opt->max_depth))
> + continue;
> + if (ce_skip_worktree(ce)) {
> + external_grep_allowed = 0;
> + break;
> + }
> + }
> + }
> /*
> * Use the external "grep" command for the case where
> * we grep through the checked-out files. It tends to
> * be a lot more optimized
> */
> - if (!cached && external_grep_allowed) {
> + if (external_grep_allowed) {
> hit = external_grep(opt, paths, cached);
> if (hit >= 0)
> return hit;
> hit = 0;
> }
This looks a bit wrong for a couple of reasons:
- external_grep() is designed to return negative without running external
grep when it shouldn't be used (see the beginning of the function for
how it refuses to run when opt->extended is set and other conditions).
The new logic seems to belong there, i.e. "in addition to the existing
case we decline, if ce_skip_worktree() entry exists in the cache, we
decline";
- It should be a separate helper function, has_skip_worktree_entry(paths);
I wouldn't be surprised if there are some other codepaths that want to
check for the same condition and do things differently, not just grep.
Originally I thought S_ISREG() check and pathspec_matches() check were
also questionable, but they actually are good things to have, as we skip
symbolic links (or submodules) and we do want to use external grep if the
subtree we are grepping in do not have skip_worktree entries even when
some other parts of the index are marked as skip_worktree.
> + else
> + trace_printf("grep: external grep not used\n");
> #endif
>
> for (nr = 0; nr < active_nr; nr++) {
> --
> 1.6.6.315.g1a406
^ permalink raw reply
* Re: [PATCH] grep: do not do external grep on skip-worktree entries
From: Junio C Hamano @ 2009-12-31 7:09 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <7v637nzky0.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> This looks a bit wrong for a couple of reasons:
>
> - external_grep() is designed to return negative without running external
> grep when it shouldn't be used (see the beginning of the function for
> how it refuses to run when opt->extended is set and other conditions).
> The new logic seems to belong there, i.e. "in addition to the existing
> case we decline, if ce_skip_worktree() entry exists in the cache, we
> decline";
IOW, something like this instead of your patch. You would want to tests
to demonstrate the original breakage, perhaps?
builtin-grep.c | 18 +++++++++++++++++-
1 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/builtin-grep.c b/builtin-grep.c
index 813fe97..25ee75d 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -357,6 +357,21 @@ static void grep_add_color(struct strbuf *sb, const char *escape_seq)
strbuf_setlen(sb, sb->len - 1);
}
+static int has_skip_worktree_entry(struct grep_opt *opt, const char **paths)
+{
+ int nr;
+ for (nr = 0; nr < active_nr; nr++) {
+ struct cache_entry *ce = active_cache[nr];
+ if (!S_ISREG(ce->ce_mode))
+ continue;
+ if (!pathspec_matches(paths, ce->name, opt->max_depth))
+ continue;
+ if (ce_skip_worktree(ce))
+ return 1;
+ }
+ return 0;
+}
+
static int external_grep(struct grep_opt *opt, const char **paths, int cached)
{
int i, nr, argc, hit, len, status;
@@ -365,7 +380,8 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)
char *argptr = randarg;
struct grep_pat *p;
- if (opt->extended || (opt->relative && opt->prefix_length))
+ if (opt->extended || (opt->relative && opt->prefix_length)
+ || has_skip_worktree_entry(opt, paths))
return -1;
len = nr = 0;
push_arg("grep");
^ permalink raw reply related
* Re: [PATCH] gitk: Use git-difftool for external diffs
From: David Aguilar @ 2009-12-31 7:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nanako Shiraishi, peff, sam, git, paulus, Markus Heidelberg,
Jay Soffian
In-Reply-To: <7vy6kk52an.fsf@alter.siamese.dyndns.org>
On Tue, Dec 29, 2009 at 11:49:52PM -0800, Junio C Hamano wrote:
> Nanako Shiraishi <nanako3@lavabit.com> writes:
>
> > Junio, could you tell us what happened to this thread?
>
> See http://thread.gmane.org/gmane.comp.version-control.git/132983/focus=133414
>
> In short, the particular way to call difftool this patch implements was
> found to be inadequate to support existing external diff support by gitk
> and a small difftool update will happen first, followed by a patch to gitk
> to use the updated difftool, to avoid regression.
I started the first step:
http://thread.gmane.org/gmane.comp.version-control.git/135613
http://thread.gmane.org/gmane.comp.version-control.git/135613/focus=135612
The 2nd patch implements the the --gui option which Markus
pointed out would be needed to avoid issues such as calling
"vimdiff" from a console-less gitk:
http://article.gmane.org/gmane.comp.version-control.git/133386
I marked the --gui patch as "RFC" since it introduced a new
config variable and I want to make sure that we agreed on its
name. I didn't get any feedback about that patch
(my fault-- we were in RC freeze and I forgot to CC: Markus).
If that looks like a good first step then we can do the next
step which would be to introduce the --extcmd= option as
mentioned here:
http://thread.gmane.org/gmane.comp.version-control.git/132983/focus=133386
I will try and prepare the changes for --extcmd= within
the next week assuming the existing --gui patch is ok.
On a related note, Jay Soffian recently submitted a
git-mergetool--lib patch adding support for "diffmerge".
It made it clear that there were parts of git-mergetool--lib
that could use some refactoring:
http://thread.gmane.org/gmane.comp.version-control.git/134906
Jay did mention that he'd give it a shot at the time, though
it does seems like the refactoring could wait until we see
how --extcmd= fits into the world.
Thank you for following up on this thread, Nanako.
--
David
^ permalink raw reply
* Re: Need some help with git rebase
From: Sylvain Rabot @ 2009-12-31 9:02 UTC (permalink / raw)
To: git
In-Reply-To: <CB5B49CA-0C66-4384-9B47-3675E517E203@wincent.com>
On Wed, Dec 30, 2009 at 23:46, Wincent Colaiuta <win@wincent.com> wrote:
>
> Look at the "git-rebase" man page, particularly the order of the arguments, what they mean, and the usage examples of "--onto":
>
> $ git rebase --onto 12.72.1 master feature
>
> Means, "replay these changes on top of 12.72.1", where "these changes" refers to commits on branch "feature" with upstream "master", which is what "git rebase" did for you.
>
> If you actually want the "feature" branch to continue pointing at the old feature branch rather than your newly rebased one, you could just look up the old SHA1 for it and update it with:
>
> $ git branch -f feature abcd1234
>
> Where "abcd1234" is the hash of the old "feature" HEAD.
>
> But I don't really know why you'd want to do that. The purpose of "git rebase" isn't to copy or cherry-pick commits from one branch onto another, but to actually _move_ (or transplant, or replay, if you prefer) those commits.
>
> Maybe I misunderstood your intentions though.
>
> Cheers,
> Wincent
>
In fact I want to backport the commits of the feature branch into 12.72.1.
I used git rebase because the drawings of the man page looked like
that I wanted to do and it does except for the part it resets the head
of my feature branch.
But the good behavior would be to cherry pick each commit of the
feature branch and apply them into 12.72.1, right ?
Thanks for your answer.
Regards.
--
Sylvain
^ permalink raw reply
* Re: Need some help with git rebase
From: Peter Baumann @ 2009-12-31 9:32 UTC (permalink / raw)
To: Sylvain Rabot; +Cc: git
In-Reply-To: <7fce93be0912310102x53755120o31e42c4a7a92a709@mail.gmail.com>
On Thu, Dec 31, 2009 at 10:02:12AM +0100, Sylvain Rabot wrote:
> On Wed, Dec 30, 2009 at 23:46, Wincent Colaiuta <win@wincent.com> wrote:
> >
> > Look at the "git-rebase" man page, particularly the order of the arguments, what they mean, and the usage examples of "--onto":
> >
> > $ git rebase --onto 12.72.1 master feature
> >
> > Means, "replay these changes on top of 12.72.1", where "these changes" refers to commits on branch "feature" with upstream "master", which is what "git rebase" did for you.
> >
> > If you actually want the "feature" branch to continue pointing at the old feature branch rather than your newly rebased one, you could just look up the old SHA1 for it and update it with:
> >
> > $ git branch -f feature abcd1234
> >
> > Where "abcd1234" is the hash of the old "feature" HEAD.
> >
> > But I don't really know why you'd want to do that. The purpose of "git rebase" isn't to copy or cherry-pick commits from one branch onto another, but to actually _move_ (or transplant, or replay, if you prefer) those commits.
> >
> > Maybe I misunderstood your intentions though.
> >
> > Cheers,
> > Wincent
> >
>
> In fact I want to backport the commits of the feature branch into 12.72.1.
> I used git rebase because the drawings of the man page looked like
> that I wanted to do and it does except for the part it resets the head
> of my feature branch.
>
> But the good behavior would be to cherry pick each commit of the
> feature branch and apply them into 12.72.1, right ?
>
$ git checkout -b rebased_feature feature
$ git rebase --onto 12.72.1 master rebased_feature
will create a temporary branch named "rebased_feature" pointing to the same
commit as the branch "feature". In fact, this will rebase the commits on the
feature branch not reachable from master onto your 12.72.1 branch *and* won't
reset the feature branch. Instead the temporary branch named "rebased_feature"
will be rebased ontop of 12.72.1.
I would still prefere the rebase over doing multiple cherry picks.
-Peter
^ permalink raw reply
* [PATCH] Fix "git remote update" with remotes.defalt set
From: Björn Gustavsson @ 2009-12-31 9:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
Starting from commit 8db35596, "git remote update" (with no
group name given) will fail with the following message if
remotes.default has been set in the config file:
fatal: 'default' does not appear to be a git repository
fatal: The remote end hung up unexpectedly
The problem is that the --multiple option is not passed to
"git fetch" if no remote or group name is given on the command
line. Fix the problem by always passing the --multiple
option to "git fetch" (which actually simplifies the code).
Reported-by: YONETANI Tomokazu
Signed-off-by: Björn Gustavsson <bgustavsson@gmail.com>
---
builtin-remote.c | 10 ++++------
t/t5505-remote.sh | 14 ++++++++++++++
2 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/builtin-remote.c b/builtin-remote.c
index a501939..c4945b8 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -1238,13 +1238,11 @@ static int update(int argc, const char **argv)
fetch_argv[fetch_argc++] = "--prune";
if (verbose)
fetch_argv[fetch_argc++] = "-v";
- if (argc < 2) {
+ fetch_argv[fetch_argc++] = "--multiple";
+ if (argc < 2)
fetch_argv[fetch_argc++] = "default";
- } else {
- fetch_argv[fetch_argc++] = "--multiple";
- for (i = 1; i < argc; i++)
- fetch_argv[fetch_argc++] = argv[i];
- }
+ for (i = 1; i < argc; i++)
+ fetch_argv[fetch_argc++] = argv[i];
if (strcmp(fetch_argv[fetch_argc-1], "default") == 0) {
git_config(get_remote_default, &default_defined);
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index fd166d9..936fe0a 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -419,6 +419,20 @@ test_expect_success 'update default (overridden, with funny whitespace)' '
'
+test_expect_success 'update (with remotes.default defined)' '
+
+ (cd one &&
+ for b in $(git branch -r)
+ do
+ git branch -r -d $b || break
+ done &&
+ git config remotes.default "drosophila" &&
+ git remote update &&
+ git branch -r > output &&
+ test_cmp expect output)
+
+'
+
test_expect_success '"remote show" does not show symbolic refs' '
git clone one three &&
--
1.6.6.69.gc18d
^ permalink raw reply related
* [updated patch v2 0/2] Improve remote helper exec failure reporting
From: Ilari Liusvaara @ 2009-12-31 10:48 UTC (permalink / raw)
To: git
Changes since updated patch (second round):
- Work around deadlock with pager.
- Fix exec reporting if read end of report pipe winds up to be fd #0.
- Limit number of retries in force_close for unexpected errors.
Ilari Liusvaara (2):
Report exec errors from run-command
Improve transport helper exec failure reporting
Makefile | 1 +
run-command.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++--
t/t0061-run-command.sh | 13 ++++++
test-run-command.c | 35 ++++++++++++++++
transport-helper.c | 14 +++++--
5 files changed, 158 insertions(+), 8 deletions(-)
create mode 100755 t/t0061-run-command.sh
create mode 100644 test-run-command.c
^ permalink raw reply
* [updated patch v2 2/2] Improve transport helper exec failure reporting
From: Ilari Liusvaara @ 2009-12-31 10:48 UTC (permalink / raw)
To: git
In-Reply-To: <1262256488-22985-1-git-send-email-ilari.liusvaara@elisanet.fi>
Previously transport-helper exec failure error reporting was pretty
much useless as it didn't report errors from execve, only from pipe
and fork. Now that run-command passes errno from exec, use the
improved support to actually print useful errors if execution fails.
Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
---
transport-helper.c | 14 ++++++++++----
1 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/transport-helper.c b/transport-helper.c
index 5078c71..0965c9b 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -31,13 +31,19 @@ static struct child_process *get_helper(struct transport *transport)
helper->out = -1;
helper->err = 0;
helper->argv = xcalloc(4, sizeof(*helper->argv));
- strbuf_addf(&buf, "remote-%s", data->name);
+ strbuf_addf(&buf, "git-remote-%s", data->name);
helper->argv[0] = strbuf_detach(&buf, NULL);
helper->argv[1] = transport->remote->name;
helper->argv[2] = transport->url;
- helper->git_cmd = 1;
- if (start_command(helper))
- die("Unable to run helper: git %s", helper->argv[0]);
+ helper->git_cmd = 0;
+ if (start_command(helper)) {
+ if (errno == ENOENT)
+ die("Unable to find remote helper for \"%s\"",
+ data->name);
+ else
+ die("Unable to run helper %s: %s", helper->argv[0],
+ strerror(errno));
+ }
data->helper = helper;
write_str_in_full(helper->in, "capabilities\n");
--
1.6.6.3.gaa2e1
^ permalink raw reply related
* [updated patch v2 1/2] Report exec errors from run-command
From: Ilari Liusvaara @ 2009-12-31 10:48 UTC (permalink / raw)
To: git
In-Reply-To: <1262256488-22985-1-git-send-email-ilari.liusvaara@elisanet.fi>
Previously run-command was unable to report errors happening in exec
call. Change it to pass errno from failed exec to errno value at
return.
The errno value passing can be done by opening close-on-exec pipe and
piping the error code through in case of failure. In case of success,
close-on-exec closes the pipe on successful exec and parent process
gets end of file on read.
Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
---
Makefile | 1 +
run-command.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++--
t/t0061-run-command.sh | 13 ++++++
test-run-command.c | 35 ++++++++++++++++
4 files changed, 148 insertions(+), 4 deletions(-)
create mode 100755 t/t0061-run-command.sh
create mode 100644 test-run-command.c
diff --git a/Makefile b/Makefile
index 4a1e5bc..452ad50 100644
--- a/Makefile
+++ b/Makefile
@@ -1725,6 +1725,7 @@ TEST_PROGRAMS += test-parse-options$X
TEST_PROGRAMS += test-path-utils$X
TEST_PROGRAMS += test-sha1$X
TEST_PROGRAMS += test-sigchain$X
+TEST_PROGRAMS += test-run-command$X
all:: $(TEST_PROGRAMS)
diff --git a/run-command.c b/run-command.c
index cf2d8f7..1c5b103 100644
--- a/run-command.c
+++ b/run-command.c
@@ -15,6 +15,22 @@ static inline void dup_devnull(int to)
close(fd);
}
+static inline void force_close(int fd)
+{
+ int err = 0;
+ /*
+ * Retry EINTRs undefinitely, exit on EBADF immediately, other
+ * errors retry only up to three times (even if pipe close
+ * shouldn't cause other errors, but you never know with
+ * what broken systems may return on closed file descriptor).
+ * consequences of failure to close pipe here may include
+ * deadlocking.
+ */
+ while (close(fd) < 0 && errno != EBADF && err < 3)
+ if(errno != EINTR)
+ err++;
+}
+
int start_command(struct child_process *cmd)
{
int need_in, need_out, need_err;
@@ -76,9 +92,64 @@ fail_pipe:
trace_argv_printf(cmd->argv, "trace: run_command:");
#ifndef WIN32
+{
+ int report_pipe[2] = {-1, -1};
+
+ if (pipe(report_pipe) < 0) {
+ report_pipe[0] = -1;
+ report_pipe[1] = -1;
+ warning("Can't open pipe for exec status report: %s\n",
+ strerror(errno));
+ }
+
fflush(NULL);
cmd->pid = fork();
- if (!cmd->pid) {
+ if (cmd->pid > 0) {
+ int r = 0, ret;
+ force_close(report_pipe[1]);
+read_again:
+ if (report_pipe[0] >= 0)
+ r = read(report_pipe[0], &ret, sizeof(ret));
+ if (r < 0 && (errno == EAGAIN || errno == EINTR ||
+ errno == EWOULDBLOCK))
+ goto read_again;
+ else if (r < 0)
+ warning("Can't read exec status report: %s\n",
+ strerror(errno));
+ else if (r == 0)
+ ;
+ else if (r < sizeof(ret)) {
+ warning("Received incomplete exec status report.\n");
+ errno = EBADMSG;
+ } else {
+ failed_errno = ret;
+ /*
+ * Clean up the process that did the failed execution
+ * so no zombies remain.
+ */
+wait_again:
+ r = waitpid(cmd->pid, &ret, 0);
+ if (r < 0 && errno != ECHILD)
+ goto wait_again;
+ cmd->pid = -1;
+ }
+ } else if (!cmd->pid) {
+ int r = 0;
+ int flags;
+ force_close(report_pipe[0]);
+
+ flags = fcntl(report_pipe[1], F_GETFD);
+ if (flags < 0)
+ r = -1;
+ flags |= FD_CLOEXEC;
+ r = (r || fcntl(report_pipe[1], F_SETFD, flags));
+ if (r) {
+ warning("Can't FD_CLOEXEC pipe for exec status "
+ "report: %s\n", strerror(errno));
+ force_close(report_pipe[1]);
+ report_pipe[1] = -1;
+ }
+
if (cmd->no_stdin)
dup_devnull(0);
else if (need_in) {
@@ -119,20 +190,44 @@ fail_pipe:
unsetenv(*cmd->env);
}
}
- if (cmd->preexec_cb)
+ if (cmd->preexec_cb) {
+ /*
+ * We don't know what pre-exec callbacks do, and they
+ * may do something that causes deadlock with exec
+ * reporting. The sole user of this hook seems to
+ * be pager, and it is run through shell, so one
+ * wouldn't get useful error from exec reporting
+ * and would get useful error from shell anyway. So
+ * just disable exec reporting for such comamnds.
+ */
+ force_close(report_pipe[1]);
+ report_pipe[1] = -1;
cmd->preexec_cb();
+ }
if (cmd->git_cmd) {
execv_git_cmd(cmd->argv);
} else {
execvp(cmd->argv[0], (char *const*) cmd->argv);
}
+ failed_errno = errno;
+
trace_printf("trace: exec '%s' failed: %s\n", cmd->argv[0],
strerror(errno));
+
+ r = 0;
+write_again:
+ if (report_pipe[1] >= 0)
+ r = write(report_pipe[1], &failed_errno,
+ sizeof(failed_errno));
+ if (r < 0 && (errno == EAGAIN || errno == EINTR ||
+ errno == EWOULDBLOCK))
+ goto write_again;
+
exit(127);
- }
- if (cmd->pid < 0)
+ } else if (cmd->pid < 0)
error("cannot fork() for %s: %s", cmd->argv[0],
strerror(failed_errno = errno));
+}
#else
{
int s0 = -1, s1 = -1, s2 = -1; /* backups of stdin, stdout, stderr */
diff --git a/t/t0061-run-command.sh b/t/t0061-run-command.sh
new file mode 100755
index 0000000..1d9e82a
--- /dev/null
+++ b/t/t0061-run-command.sh
@@ -0,0 +1,13 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Ilari Liusvaara
+#
+
+test_description='Test run command'
+
+. ./test-lib.sh
+
+test_expect_success "reporting ENOENT" \
+"test-run-command 1"
+
+test_done
diff --git a/test-run-command.c b/test-run-command.c
new file mode 100644
index 0000000..6297785
--- /dev/null
+++ b/test-run-command.c
@@ -0,0 +1,35 @@
+/*
+ * test-run-command.c: test run command API.
+ *
+ * (C) 2009 Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
+ *
+ * This code is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include "git-compat-util.h"
+#include "run-command.h"
+#include <string.h>
+#include <errno.h>
+
+int main(int argc, char **argv)
+{
+ char* procs[2];
+ struct child_process proc;
+ memset(&proc, 0, sizeof(proc));
+
+ if(argc < 2)
+ return 1;
+
+ if (argv[1][1] == '1')
+ procs[0] = "does-not-exist-62896869286";
+ procs[1] = NULL;
+ proc.argv = (const char **)procs;
+
+ if (!run_command(&proc))
+ return 1;
+ if (errno != ENOENT)
+ return 1;
+ return 0;
+}
--
1.6.6.3.gaa2e1
^ permalink raw reply related
* Re: [Updated PATCH 1/2] Report exec errors from run-command
From: Ilari Liusvaara @ 2009-12-31 10:48 UTC (permalink / raw)
To: Tarmigan; +Cc: git
In-Reply-To: <905315640912302126n1848c99cre0f9caa644041fad@mail.gmail.com>
On Thu, Dec 31, 2009 at 12:26:48AM -0500, Tarmigan wrote:
> On Wed, Dec 30, 2009 at 5:52 AM, Ilari Liusvaara
> <ilari.liusvaara@elisanet.fi> wrote:
>
> I was testing pu and 'git diff' and 'git log' would hang forever.
V3 just sent to list. Should fix this issue.
-Ilari
^ permalink raw reply
* Re: Need some help with git rebase
From: Wincent Colaiuta @ 2009-12-31 11:06 UTC (permalink / raw)
To: Sylvain Rabot; +Cc: git
In-Reply-To: <7fce93be0912301502r77152c52sbccf762fb6c44610@mail.gmail.com>
El 31/12/2009, a las 00:02, Sylvain Rabot escribió:
> In fact I want to backport the commits of the feature branch into
> 12.72.1.
>
> I used git rebase because the drawings of the man page looked like
> that I
> wanted to do and it does except for the part it resets the head of my
> feature branch.
>
> But as you said the good behavior would be to cherry pick each
> commit of the
> feature branch and apply them into 12.72.1, right ?
Well rebasing is just a convenient way of cherry-picking a bunch of
commits, so it's probably the right tool for the job.
But as you've seen, it has the effect of "moving" or "transplanting"
the feature branch (replacing the old feature HEAD). If you really
want the original feature branch HEAD to continue existing after the
rebase you'll need to take some specific action to preserve it
beforehand (creating a temporary branch before doing the rebase like
Peter Baumann suggested) or restore it afterwards (using "git branch"
like I suggested).
But before you actually do that, I'd make sure that you actually have
a valid reason for keeping that branch around. Maybe wanting to
"backport" those commits onto various different branches might be a
valid reason. But it's worth thinking through because Git gives you
various tools for supporting different workflows (merging, rebasing,
cherry-picking) and they each have their use cases.
Cheers,
Wincent
^ permalink raw reply
* [PATCH] bash completion: factor submodules into dirty state
From: Thomas Rast @ 2009-12-31 11:48 UTC (permalink / raw)
To: git; +Cc: Johan Herland, Kevin Ballard, Shawn O. Pearce
In-Reply-To: <200912310240.07741.johan@herland.net>
In the implementation of GIT_PS1_SHOWDIRTYSTATE in 738a94a (bash:
offer to show (un)staged changes, 2009-02-03), I cut&pasted the
git-diff invocations from dirty-worktree checks elsewhere, carrying
along the --ignore-submodules option.
As pointed out by Kevin Ballard, this doesn't really make sense: to
the _user_, a changed submodule counts towards uncommitted changes.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
Johan Herland wrote:
> On Wednesday 30 December 2009, Kevin Ballard wrote:
> > Why does the __git_ps1 function in git-completion.bash explicitly ignore
> > submodules when showing the GIT_PS1_SHOWDIRTYSTATE status? The most
> > common issue with my current repository is not realizing when submodules
> > need to be updated because I blindly trust my prompt to tell me when I
> > have dirty state.
>
> According to git blame, it has been there since GIT_PS1_SHOWDIRTYSTATE was
> introduced in 738a94a... by Thomas Rast (CCed), but the commit message does
> not say why submodules are explicitly ignored.
>
> FWIW, I agree with Kevin, and would like changed submodules to be included
> in the status.
No good reason; I really do remember cut&pasting the checks, though
I'm not sure from where.
I don't really use submodules, so I'll just trust your judgements that
it's better to factor them into the status.
contrib/completion/git-completion.bash | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c65462c..a455fe8 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -142,11 +142,9 @@ __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 --ignore-submodules \
- --quiet --exit-code || w="*"
+ git diff --no-ext-diff --quiet --exit-code || w="*"
if git rev-parse --quiet --verify HEAD >/dev/null; then
- git diff-index --cached --quiet \
- --ignore-submodules HEAD -- || i="+"
+ git diff-index --cached --quiet HEAD -- || i="+"
else
i="#"
fi
--
1.6.6.337.g4932e
^ permalink raw reply related
* Re: [msysGit] Problem with apply
From: Erik Faye-Lund @ 2009-12-31 12:49 UTC (permalink / raw)
To: richardpreen; +Cc: msysgit, Git Mailing List
In-Reply-To: <SNT131-ds16B1679EFC8F74B62D4630C4780@phx.gbl>
This isn't a Windows specific question, and would be better for the
main git mailing list. I'm CC'ing it in hope that someone there knows
the answer.
On Thu, Dec 31, 2009 at 1:26 PM, <richardpreen@hotmail.com> wrote:
> I'm trying to use git to create a binary diff of two files and then apply
> the diff to the first file in an attempt to make both files the same (just
> testing the concept);
>
> git diff --binary --no-index original\t1.ppt modified\t1.ppt >
> original\my.diff
> cd original
> git apply my.diff
>
> This is giving me the following error message;
> fatal: git diff header lacks filename information (line 3)
>
> Any suggestions as to where I've gone wrong?
> Thanks.
--
Erik "kusma" Faye-Lund
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox