* [PATCH] Makes 'git-stash show' stay quiet when there are no stashes.
From: Jing Xue @ 2007-12-14 1:28 UTC (permalink / raw)
To: git
(I tried to send this trivial patch for a couple of times using
git-send-email, but somehow it never turned up.)
Currently when there are no stashes, 'git stash show' basically aborts with an
error message from rev-parse: "fatal: Needed a single revision", which can be
confusing. This patch makes git-stash keep quiet and exit gracefully in that
case.
---
git-stash.sh | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/git-stash.sh b/git-stash.sh
index f16fd9c..dbdaeaf 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -119,6 +119,10 @@ show_stash () {
flags=--stat
fi
s=$(git rev-parse --revs-only --no-flags --default $ref_stash "$@")
+ if test -z "$s"
+ then
+ exit 0
+ fi
w_commit=$(git rev-parse --verify "$s") &&
b_commit=$(git rev-parse --verify "$s^") &&
--
1.5.4.rc0.1.g3696
^ permalink raw reply related
* [Funky] "git -p cmd" inside a bare repository
From: Junio C Hamano @ 2007-12-14 1:29 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Johannes Schindelin
In-Reply-To: <20071129122139.GA11176@laptop>
If you have a bare repository and try this there:
$ PAGER=head git show HEAD:gcc/ChangeLog
it works as expected, but if you explicitly ask for pagination, like
this:
$ PAGER=head git -p show HEAD:gcc/ChangeLog
you would get a very funky error message:
fatal: ambiguous argument 'HEAD:gcc/ChangeLog': unknown revision or path not in the working tree.
Use '--' to separate paths from revisions
as if we are working with a repository with working tree.
I originally noticed this with ls-tree. The symptom is a bit different:
$ git -p ls-tree HEAD
fatal: Not a valid object name HEAD
I _think_ what is happening is that setup_pager() tries to run
git_config(), which runs setup(), and then RUN_SETUP set for "ls-tree"
(or "show") internal command runs setup again. HEAD is given to
resolve_ref() and git_path("%s", ref) makes it to ".git/HEAD", even
though in a bare repository git_dir should be set to ".", and of course
we cannot find such a path in the git directory.
^ permalink raw reply
* Re: [PATCH] Makes 'git-stash show' stay quiet when there are no stashes.
From: Junio C Hamano @ 2007-12-14 1:34 UTC (permalink / raw)
To: Jing Xue; +Cc: git
In-Reply-To: <20071214012838.GA8914@fawkes>
Jing Xue <jingxue@digizenstudio.com> writes:
> (I tried to send this trivial patch for a couple of times using
> git-send-email, but somehow it never turned up.)
>
> Currently when there are no stashes, 'git stash show' basically aborts with an
> error message from rev-parse: "fatal: Needed a single revision", which can be
> confusing. This patch makes git-stash keep quiet and exit gracefully in that
> case.
I agree "git stash show" should not give cryptic error message, but I
think you should do this only when the user did not explicitly say which
stash to show (that is, we should still give error message if the user
said "git stash show garbage").
^ permalink raw reply
* [BUG?] git rebase -i
From: Pieter de Bie @ 2007-12-14 1:21 UTC (permalink / raw)
To: git
Hi,
I was just toying with git rebase -i -p, when I found the following:
Tirana:~/git pieter$ /opt/local/bin/git --version
git version 1.5.3.5
Tirana:~/git pieter$ time /opt/local/bin/git rebase -p -i HEAD~100
Successfully rebased and updated refs/heads/master.
real 0m17.816s
user 0m3.927s
sys 0m10.381s
Tirana:~/git pieter$ git --version
git version 1.5.4.rc0
Tirana:~/git pieter$ time git rebase -p -i HEAD~100
cat: /Users/pieter/git/.git/.dotest-merge/rewritten/
1e8df762b38e01685f3aa3613e2d61f73346fcbe: No such file or directory
real 0m20.427s
user 0m4.449s
sys 0m14.028s
The rebase in 1.5.4.rc0 exists with an error. Another thing to note
is that the 1.5.4.rc0 tries to apply 215 patches, while the 1.5.3.5
tries to apply 206 patches. This test was done on the git master
branch (tagged 1.5.4.rc0). I did not change anything in the
interactive rebase editor (just hit :wq in vim).
- Pieter
^ permalink raw reply
* git-send-email doesn't like me+git ML
From: Jing Xue @ 2007-12-14 1:47 UTC (permalink / raw)
To: git
OK, this is bewildering. I tried a couple of times sending a trivial
patch (the one that makes git-stash show keep quiet when there aren't
any stashes), but it never made it to the list every time I used
git-send-email, yet as soon as I did it as a regular email, i.e.,
starting a new message in mutt, pasting the patch into the body, and
sending it, it worked just fine.
My mail log shows the mails (the one thru git-send-email) went out just
fine. And it always worked when I tried to use git-send-email to send
the patch to my gmail address. It just doesn't work with the list - and
that obviously can't be. 8-)
--
Jing Xue
^ permalink raw reply
* Re: What to do if git-cvsimport/cvsps hangs?
From: Jakub Narebski @ 2007-12-14 1:56 UTC (permalink / raw)
To: Florian Weimer; +Cc: git
In-Reply-To: <824peojm4g.fsf@mid.bfk.de>
Florian Weimer <fweimer@bfk.de> writes:
> This is probably more of a cvsps question than a GIT question, but
> AFAICT, the cvsps upstream is mostly inactive these days.
>
> I tried to import the PostgreSQL repository, using
>
> git-cvsimport -d :pserver:anoncvs@anoncvs.postgresql.org:/projects/cvsroot -k -C pgsql pgsql
>
> However, after a bit of activity, git-cvsimport hangs at the following
> line
>
> cvs rlog: Logging pgsql/src/win32
>
> strace doesn't show any system call activtiy in the cvsps process
> (which consumes 100% CPU). cvsps is Debian's 2.1-2 version.
>
> Is there some kind of replacement for cvsps which works more reliably?
You can try set of patches to cvsps there (see GitLinks at Git Wiki,
or git-cvsimport entry at InterfacesFrontendsAndTools)
http://ydirson.free.fr/en/software/scm/cvsps.html
Or you can try other CVS importers (see InterfacesForntendsAndTools):
- parsecvs by Keith Packard, used to import X.Org, seems to be
unmaintaned, requires access to *,v files (but check out cvssuck)
- fromcvs by Simon 'corecode' Schubert, uses git-fast-import,
IIRC in Ruby
- cvs2svn has _experimental_ support for git export (git-fast-import?)
so you can try this as well.
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH 1/5] "diff --check" should affect exit status
From: Junio C Hamano @ 2007-12-14 2:10 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: git
In-Reply-To: <7vir32ywyz.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> I think highjacking the "did we encounter problems" return value of the
> entire callchain for the purpose of checkdiff is very ugly and wrong to
> begin with,...
How about this on top of your 1/5? It mostly is about reverting the
damage to the higher level callchain. Instead, builtin_checkdiff()
inspects the checkdiff data and sets the CHECK_FAILED flag to the
diffopt structure. The callers are already checking that flag so there
is nothing to change for them.
--
diff --git a/diff.c b/diff.c
index 39109a6..fc496bf 100644
--- a/diff.c
+++ b/diff.c
@@ -1456,7 +1456,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
diff_free_filespec_data(two);
}
-static int builtin_checkdiff(const char *name_a, const char *name_b,
+static void builtin_checkdiff(const char *name_a, const char *name_b,
struct diff_filespec *one,
struct diff_filespec *two, struct diff_options *o)
{
@@ -1464,7 +1464,7 @@ static int builtin_checkdiff(const char *name_a, const char *name_b,
struct checkdiff_t data;
if (!two)
- return 0;
+ return;
memset(&data, 0, sizeof(data));
data.xm.consume = checkdiff_consume;
@@ -1493,7 +1493,8 @@ static int builtin_checkdiff(const char *name_a, const char *name_b,
free_and_return:
diff_free_filespec_data(one);
diff_free_filespec_data(two);
- return data.status;
+ if (data.status)
+ DIFF_OPT_SET(o, CHECK_FAILED);
}
struct diff_filespec *alloc_filespec(const char *path)
@@ -2078,14 +2079,14 @@ static void run_diffstat(struct diff_filepair *p, struct diff_options *o,
builtin_diffstat(name, other, p->one, p->two, diffstat, o, complete_rewrite);
}
-static int run_checkdiff(struct diff_filepair *p, struct diff_options *o)
+static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)
{
const char *name;
const char *other;
if (DIFF_PAIR_UNMERGED(p)) {
/* unmerged */
- return 0;
+ return;
}
name = p->one->path;
@@ -2094,7 +2095,7 @@ static int run_checkdiff(struct diff_filepair *p, struct diff_options *o)
diff_fill_sha1_info(p->one);
diff_fill_sha1_info(p->two);
- return builtin_checkdiff(name, other, p->one, p->two, o);
+ builtin_checkdiff(name, other, p->one, p->two, o);
}
void diff_setup(struct diff_options *options)
@@ -2596,17 +2597,17 @@ static void diff_flush_stat(struct diff_filepair *p, struct diff_options *o,
run_diffstat(p, o, diffstat);
}
-static int diff_flush_checkdiff(struct diff_filepair *p,
+static void diff_flush_checkdiff(struct diff_filepair *p,
struct diff_options *o)
{
if (diff_unmodified_pair(p))
- return 0;
+ return;
if ((DIFF_FILE_VALID(p->one) && S_ISDIR(p->one->mode)) ||
(DIFF_FILE_VALID(p->two) && S_ISDIR(p->two->mode)))
- return 0; /* no tree diffs in patch format */
+ return; /* no tree diffs in patch format */
- return run_checkdiff(p, o);
+ run_checkdiff(p, o);
}
int diff_queue_is_empty(void)
@@ -2725,19 +2726,16 @@ static int check_pair_status(struct diff_filepair *p)
}
}
-static int flush_one_pair(struct diff_filepair *p, struct diff_options *opt)
+static void flush_one_pair(struct diff_filepair *p, struct diff_options *opt)
{
int fmt = opt->output_format;
if (fmt & DIFF_FORMAT_CHECKDIFF)
- return diff_flush_checkdiff(p, opt);
+ diff_flush_checkdiff(p, opt);
else if (fmt & (DIFF_FORMAT_RAW | DIFF_FORMAT_NAME_STATUS))
diff_flush_raw(p, opt);
else if (fmt & DIFF_FORMAT_NAME)
write_name_quoted(p->two->path, stdout, opt->line_termination);
-
- /* return value only matters with DIFF_FORMAT_CHECKDIFF */
- return 0;
}
static void show_file_mode_name(const char *newdelete, struct diff_filespec *fs)
@@ -2976,8 +2974,8 @@ void diff_flush(struct diff_options *options)
DIFF_FORMAT_CHECKDIFF)) {
for (i = 0; i < q->nr; i++) {
struct diff_filepair *p = q->queue[i];
- if (check_pair_status(p) && flush_one_pair(p, options))
- DIFF_OPT_SET(options, CHECK_FAILED);
+ if (check_pair_status(p))
+ flush_one_pair(p, options);
}
separator++;
}
--
1.5.4.rc0.1.g37d0
^ permalink raw reply related
* Re: git-cvsexportcommit fails for huge commits
From: Junio C Hamano @ 2007-12-14 3:22 UTC (permalink / raw)
To: Jeff King; +Cc: Markus Klinik, git
In-Reply-To: <20071212092512.GB20799@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> the path at the beginning happens to be longer than 1024 then you will
>> run path-less "cvs status"?
>
> No, read the loop again. The length starts at 0, so we always go through
> the loop body once.
Sorry, you are right.
Perhaps pick a reasonably small but not insanely small value, like 16kB,
forget about the atomicity issues for now, as an interim improvement
patch?
^ permalink raw reply
* [PATCH] provide advance warning of some future pack default changes
From: Nicolas Pitre @ 2007-12-14 3:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vsl2jh3rb.fsf@gitster.siamese.dyndns.org>
Signed-off-by: Nicolas Pitre <nico@cam.org>
---
On Mon, 3 Dec 2007, Junio C Hamano wrote:
> Nicolas Pitre <nico@cam.org> writes:
>
> > Two things I would like to see in the next version (1.5.5) as well, for
> > which we could provide early warnings now:
> >
> > - repack.usedeltabaseoffset defaulting to true
> >
> > - pack.indexversion defaulting to 2
>
> I think the former would be sensible, the latter I fear might be a bit
> too new but I do not recall the exact version dependency.
>
> Could you draft a patch to ReleaseNotes to explain the consequences of
> these changes using ordinary user's vocabulary, like:
>
> Starting v1.5.5, repack.usedeltabaseoffset will default to true,
> which will give denser packfile (i.e. more efficient storage).
> The downside is that git older than version X will not be able
> to use a repository packed using this setting.
Here it is.
diff --git a/Documentation/RelNotes-1.5.4.txt b/Documentation/RelNotes-1.5.4.txt
index 6645565..d6fd3dd 100644
--- a/Documentation/RelNotes-1.5.4.txt
+++ b/Documentation/RelNotes-1.5.4.txt
@@ -43,6 +43,17 @@ Deprecation notices
* "git peek-remote" is deprecated, as "git ls-remote" was written in C
and works for all transports, and will be removed in the future.
+ * From v1.5.5, the repack.usedeltabaseoffset config option will default
+ to true, which will give denser packfile (i.e. more efficient storage).
+ The downside is that git older than version 1.4.4 will not be able
+ to directly use a repository packed using this setting.
+
+ * From v1.5.5, the pack.indexversion config option will default to 2,
+ which is slightly more efficient, and makes repacking more immune to
+ data corruptions. Git older than version 1.5.2 may revert to version 1
+ of the pack index with a manual "git index-pack" to be able to directly
+ access corresponding pack files.
+
Updates since v1.5.3
--------------------
^ permalink raw reply related
* Re: [RFH] convert shortlog to use parse_options
From: Jeff King @ 2007-12-14 4:08 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: Pierre Habouzit, Junio C Hamano, git
In-Reply-To: <1197571656.28742.13.camel@hinata.boston.redhat.com>
On Thu, Dec 13, 2007 at 01:47:36PM -0500, Kristian Høgsberg wrote:
> Oops, sorry about that. I just wanted to say we shouldn't jump through
> all these hoops to make the option parser support every type of option
> there ever was in the git command line ui. A lot of these were probably
> decided somewhat arbitrarily by whoever implemented the command.
> Instead it's an opportunity to retroactively enforce some consistency
> and predictability to the various option-styles that have been
> hand-rolled over time in different git commands.
I agree. I am already a little bit uncomfortable with the "--abbrev 10"
won't work but "--foo 10" will, because it requires that the user
remember which arguments are optional and which are required. But
switching it to "--abbrev 10 works, but --abbrev $foo does not, unless
of course $foo is an integer, in which case you must use --abbrev=$foo"
is just a little bit too DWIM. E.g., if you are scripting, it's just one
more source of error (if I have $foo, how must I write --abbrev $foo for
it to ensure that I _don't_ trigger the optional argument?).
-Peff
^ permalink raw reply
* [PATCH] Authentication support for pserver
From: Ævar Arnfjörð Bjarmason @ 2007-12-14 4:08 UTC (permalink / raw)
To: git; +Cc: martyn, martin
[-- Attachment #1: Type: text/plain, Size: 1285 bytes --]
I've patched git-cvsserver so that it supports pserver authentication as
opposed to only anonymous access. The patch itself works but I haven't
updated the relevant documentation or test cases and won't be able to do
so until after the weekend so I'm submitting what I have for review on
the list.
The pserver user/password database is stored in the config file for each
repository because I didn't know where else to put it, for example:
avar@oe:/tmp/fleh$ cat /tmp/git-test.git/config
[core]
repositoryformatversion = 0
filemode = true
bare = true
[gitcvs]
enabled = 1
[gitcvs.users]
avar = foobar
whee = whoo
The user can then login/checkout and do other operations with the
user/password set in the config file:
avar@oe:/tmp/fleh$ cvs -d:pserver:whee:whoo@localhost/tmp/git-test.git login
Logging in to :pserver:whee@localhost:2401/tmp/git-test.git
avar@oe:/tmp/fleh$ cvs -d:pserver:whee:oops@localhost/tmp/git-test.git login
Logging in to :pserver:whee@localhost:2401/tmp/git-test.git
The password supplied for user whee was incorrect
cvs login: authorization failed: server localhost rejected access to
/tmp/git-test.git for user whee
$ cvs -d:pserver:whee:whoo@localhost/tmp/git-test.git co master
cvs checkout: Updating master
U master/.emacs
[...]
[-- Attachment #2: Patch that adds pserver authentication support to git-cvsserver --]
[-- Type: text/plain, Size: 4040 bytes --]
diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index ecded3b..851383b 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -150,12 +150,32 @@ if ($state->{method} eq 'pserver') {
exit 1;
}
$line = <STDIN>; chomp $line;
- unless ($line eq 'anonymous') {
- print "E Only anonymous user allowed via pserver\n";
- print "I HATE YOU\n";
- exit 1;
+ my $user = $line;
+ $line = <STDIN>; chomp $line;
+ my $password = $line;
+
+ unless ($user eq 'anonymous') {
+ # Trying to authenticate a user
+ if (not exists $cfg->{gitcvs}->{users}) {
+ print "E the repo config file needs a [gitcvs.users] section with user/password key-value pairs\n";
+ print "I HATE YOU\n";
+ exit 1;
+ } elsif (exists $cfg->{gitcvs}->{users} and not exists $cfg->{gitcvs}->{users}->{$user}) {
+ print "E the repo config file has a [gitcvs.users] section but the user $user is not defined in it\n";
+ print "I HATE YOU\n";
+ exit 1;
+ } else {
+ my $descrambled_password = descramble($password);
+ my $cleartext_password = $cfg->{gitcvs}->{users}->{$user};
+ if ($descrambled_password ne $cleartext_password) {
+ print "E The password supplied for user $user was incorrect\n";
+ print "I HATE YOU\n";
+ exit 1;
+ }
+ # else fall through to LOVE
+ }
}
- $line = <STDIN>; chomp $line; # validate the password?
+
$line = <STDIN>; chomp $line;
unless ($line eq "END $request REQUEST") {
die "E Do not understand $line -- expecting END $request REQUEST\n";
@@ -273,7 +293,7 @@ sub req_Root
}
foreach my $line ( @gitvars )
{
- next unless ( $line =~ /^(gitcvs)\.(?:(ext|pserver)\.)?([\w-]+)=(.*)$/ );
+ next unless ( $line =~ /^(gitcvs)\.(?:(ext|pserver|users)\.)?([\w-]+)=(.*)$/ );
unless ($2) {
$cfg->{$1}{$3} = $4;
} else {
@@ -1176,12 +1196,6 @@ sub req_ci
$log->info("req_ci : " . ( defined($data) ? $data : "[NULL]" ));
- if ( $state->{method} eq 'pserver')
- {
- print "error 1 pserver access cannot commit\n";
- exit;
- }
-
if ( -e $state->{CVSROOT} . "/index" )
{
$log->warn("file 'index' already exists in the git repository");
@@ -2107,6 +2121,41 @@ sub kopts_from_path
}
}
+
+sub descramble
+{
+ # This table is from src/scramble.c in the CVS source
+ my @SHIFTS = (
+ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
+ 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
+ 114,120, 53, 79, 96,109, 72,108, 70, 64, 76, 67,116, 74, 68, 87,
+ 111, 52, 75,119, 49, 34, 82, 81, 95, 65,112, 86,118,110,122,105,
+ 41, 57, 83, 43, 46,102, 40, 89, 38,103, 45, 50, 42,123, 91, 35,
+ 125, 55, 54, 66,124,126, 59, 47, 92, 71,115, 78, 88,107,106, 56,
+ 36,121,117,104,101,100, 69, 73, 99, 63, 94, 93, 39, 37, 61, 48,
+ 58,113, 32, 90, 44, 98, 60, 51, 33, 97, 62, 77, 84, 80, 85,223,
+ 225,216,187,166,229,189,222,188,141,249,148,200,184,136,248,190,
+ 199,170,181,204,138,232,218,183,255,234,220,247,213,203,226,193,
+ 174,172,228,252,217,201,131,230,197,211,145,238,161,179,160,212,
+ 207,221,254,173,202,146,224,151,140,196,205,130,135,133,143,246,
+ 192,159,244,239,185,168,215,144,139,165,180,157,147,186,214,176,
+ 227,231,219,169,175,156,206,198,129,164,150,210,154,177,134,127,
+ 182,128,158,208,162,132,167,209,149,241,153,251,237,236,171,195,
+ 243,233,253,240,194,250,191,155,142,137,245,235,163,242,178,152
+ );
+ my ($str) = @_;
+
+ # This should never happen, the same format has been used since
+ # CVS was spawned
+ $str =~ s/^(.)//;
+ die "invalid password format $1" unless $1 eq 'A';
+
+ $str =~ s/(.)/chr $SHIFTS[ord $1]/ge;
+
+ return $str;
+}
+
+
package GITCVS::log;
####
^ permalink raw reply related
* Re: git-cvsexportcommit fails for huge commits
From: Jeff King @ 2007-12-14 4:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Markus Klinik, git
In-Reply-To: <7vzlwevu2k.fsf@gitster.siamese.dyndns.org>
On Thu, Dec 13, 2007 at 07:22:43PM -0800, Junio C Hamano wrote:
> Sorry, you are right.
>
> Perhaps pick a reasonably small but not insanely small value, like 16kB,
> forget about the atomicity issues for now, as an interim improvement
> patch?
I'm fine with that. We can probably go a bit higher than that. From my
limited testing[1]:
Linux 2.6.18: ~128K
Linux 2.6.23: huge? I tried ~350K and it worked fine
Solaris: huge? I tried ~350K and it worked fine
Freebsd 6.1: ~256K
So it seems that we could probably go with something more like 64K, and
then only truly pathological cases should trigger the behavior.
-Peff
[1] All numbers are approximate and determined experimentally with
something like:
for i in `seq 1 $n`; do
touch $long_filename-$i
done
ls * | wc
^ permalink raw reply
* Re: [PATCH] Don't use the pager when running "git diff --check"
From: Jeff King @ 2007-12-14 4:51 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: git, gitster
In-Reply-To: <1197575138-58070-1-git-send-email-win@wincent.com>
On Thu, Dec 13, 2007 at 08:45:38PM +0100, Wincent Colaiuta wrote:
> In 89d07f75 "git diff" learnt to not run the pager if the user passes
> the --exit-code switch. This commit does the same for the --check
> switch for the same reason: we want the user to get the exit status
> from "git diff", not the pager.
But --check is also producing useful output, which might need paged. So
you are sacrificing existing interactive use of --check for scriptable
exit-code uses. If you really want the exit code, why not "git diff
--check --exit-code"?
OTOH, I am not too sad to lose the paging behavior; it would take quite
a few whitespace errors to scroll off the screen.
-Peff
^ permalink raw reply
* [PATCH] make git start-up sequence a bit more robust
From: Junio C Hamano @ 2007-12-14 5:03 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Johannes Schindelin
In-Reply-To: <7vmysexdvw.fsf@gitster.siamese.dyndns.org>
The handle options loop called setup_git_dir() and setup_pager() that
would silently trigger repository discovery while it cycled. This patch
changes it to collect the options from the user, and uses the
information in a saner, more controlled order, namely:
- GIT_DIR, GIT_WORK_TREE and GIT_PAGER environments are set as needed;
- setup_git_directory() is run if needed;
- setup_work_tree(0 is run if needed;
- setup_pager() is run if needed;
The last two wants to read config, which means setup_git_directory()
should come before them, which in turn means GIT_DIR must be set up
before that.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Junio C Hamano <gitster@pobox.com> writes:
> If you have a bare repository and try this there:
>
> $ PAGER=head git show HEAD:gcc/ChangeLog
>
> it works as expected, but if you explicitly ask for pagination, like
> this:
>
> $ PAGER=head git -p show HEAD:gcc/ChangeLog
>
> you would get a very funky error message:
>
> fatal: ambiguous argument 'HEAD:gcc/ChangeLog': unknown revision or path not in the working tree.
> Use '--' to separate paths from revisions
>
> as if we are working with a repository with working tree.
>
> I originally noticed this with ls-tree. The symptom is a bit different:
>
> $ git -p ls-tree HEAD
> fatal: Not a valid object name HEAD
>
> I _think_ what is happening is that setup_pager() tries to run
> git_config(), which runs setup(), and then RUN_SETUP set for "ls-tree"
> (or "show") internal command runs setup again. HEAD is given to
> resolve_ref() and git_path("%s", ref) makes it to ".git/HEAD", even
> though in a bare repository git_dir should be set to ".", and of course
> we cannot find such a path in the git directory.
git.c | 73 ++++++++++++++++++++++++++++++++++++++++-------------------------
1 files changed, 45 insertions(+), 28 deletions(-)
diff --git a/git.c b/git.c
index 15fec89..fd756cd 100644
--- a/git.c
+++ b/git.c
@@ -6,7 +6,21 @@
const char git_usage_string[] =
"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
-static int handle_options(const char*** argv, int* argc, int* envchanged)
+static int want_pager = -1;
+static const char *want_git_dir;
+static int want_git_dir_badly = 1;
+static const char *want_work_tree;
+
+static void no_env_change_from_alias(const char *alias_command)
+{
+ if (!alias_command)
+ return;
+ die("alias '%s' changes environment variables\n"
+ "You can use '!git' in the alias to do this.",
+ alias_command);
+}
+
+static int handle_options(const char ***argv, int *argc, const char *alias_command)
{
int handled = 0;
@@ -35,46 +49,42 @@ static int handle_options(const char*** argv, int* argc, int* envchanged)
exit(0);
}
} else if (!strcmp(cmd, "-p") || !strcmp(cmd, "--paginate")) {
- setup_pager();
+ want_pager = 1;
} else if (!strcmp(cmd, "--no-pager")) {
- setenv("GIT_PAGER", "cat", 1);
- if (envchanged)
- *envchanged = 1;
+ want_pager = 0;
} else if (!strcmp(cmd, "--git-dir")) {
+ no_env_change_from_alias(alias_command);
if (*argc < 2) {
fprintf(stderr, "No directory given for --git-dir.\n" );
usage(git_usage_string);
}
- setenv(GIT_DIR_ENVIRONMENT, (*argv)[1], 1);
- if (envchanged)
- *envchanged = 1;
+ want_git_dir = (*argv)[1];
(*argv)++;
(*argc)--;
handled++;
} else if (!prefixcmp(cmd, "--git-dir=")) {
- setenv(GIT_DIR_ENVIRONMENT, cmd + 10, 1);
- if (envchanged)
- *envchanged = 1;
+ no_env_change_from_alias(alias_command);
+ want_git_dir = cmd + 10;
} else if (!strcmp(cmd, "--work-tree")) {
+ no_env_change_from_alias(alias_command);
if (*argc < 2) {
fprintf(stderr, "No directory given for --work-tree.\n" );
usage(git_usage_string);
}
- setenv(GIT_WORK_TREE_ENVIRONMENT, (*argv)[1], 1);
- if (envchanged)
- *envchanged = 1;
+ want_work_tree = (*argv)[1];
(*argv)++;
(*argc)--;
+ handled++;
} else if (!prefixcmp(cmd, "--work-tree=")) {
- setenv(GIT_WORK_TREE_ENVIRONMENT, cmd + 12, 1);
- if (envchanged)
- *envchanged = 1;
+ no_env_change_from_alias(alias_command);
+ want_work_tree = cmd + 12;
} else if (!strcmp(cmd, "--bare")) {
static char git_dir[PATH_MAX+1];
+ no_env_change_from_alias(alias_command);
is_bare_repository_cfg = 1;
- setenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir, sizeof(git_dir)), 0);
- if (envchanged)
- *envchanged = 1;
+ getcwd(git_dir, sizeof(git_dir));
+ want_git_dir = git_dir;
+ want_git_dir_badly = 0;
} else {
fprintf(stderr, "Unknown option: %s\n", cmd);
usage(git_usage_string);
@@ -153,7 +163,7 @@ static int split_cmdline(char *cmdline, const char ***argv)
static int handle_alias(int *argcp, const char ***argv)
{
- int nongit = 0, envchanged = 0, ret = 0, saved_errno = errno;
+ int nongit = 0, ret = 0, saved_errno = errno;
const char *subdir;
int count, option_count;
const char** new_argv;
@@ -183,11 +193,7 @@ static int handle_alias(int *argcp, const char ***argv)
alias_string + 1, alias_command);
}
count = split_cmdline(alias_string, &new_argv);
- option_count = handle_options(&new_argv, &count, &envchanged);
- if (envchanged)
- die("alias '%s' changes environment variables\n"
- "You can use '!git' in the alias to do this.",
- alias_command);
+ option_count = handle_options(&new_argv, &count, alias_command);
memmove(new_argv - option_count, new_argv,
count * sizeof(char *));
new_argv -= option_count;
@@ -247,11 +253,12 @@ static int run_command(struct cmd_struct *p, int argc, const char **argv)
prefix = NULL;
if (p->option & RUN_SETUP)
prefix = setup_git_directory();
- if (p->option & USE_PAGER)
- setup_pager();
if (p->option & NEED_WORK_TREE)
setup_work_tree();
+ if (0 < want_pager || (p->option & USE_PAGER))
+ setup_pager();
+
trace_argv_printf(argv, "trace: built-in: git");
status = p->fn(argc, argv, prefix);
@@ -434,6 +441,13 @@ int main(int argc, const char **argv)
}
cmd = argv[0];
+ if (want_git_dir)
+ setenv(GIT_DIR_ENVIRONMENT, want_git_dir, want_git_dir_badly);
+ if (want_work_tree)
+ setenv(GIT_WORK_TREE_ENVIRONMENT, want_work_tree, 1);
+ if (!want_pager)
+ setenv("GIT_PAGER", "cat", 1);
+
/*
* We use PATH to find git commands, but we prepend some higher
* precidence paths: the "--exec-path" option, the GIT_EXEC_PATH
@@ -446,6 +460,9 @@ int main(int argc, const char **argv)
/* See if it's an internal command */
handle_internal_command(argc, argv);
+ if (!done_alias && 0 < want_pager)
+ setup_pager();
+
/* .. then try the external ones */
execv_git_cmd(argv);
^ permalink raw reply related
* Re: [PATCH] Don't use the pager when running "git diff --check"
From: Junio C Hamano @ 2007-12-14 5:11 UTC (permalink / raw)
To: Jeff King; +Cc: Wincent Colaiuta, git
In-Reply-To: <20071214045127.GC10169@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Thu, Dec 13, 2007 at 08:45:38PM +0100, Wincent Colaiuta wrote:
>
>> In 89d07f75 "git diff" learnt to not run the pager if the user passes
>> the --exit-code switch. This commit does the same for the --check
>> switch for the same reason: we want the user to get the exit status
>> from "git diff", not the pager.
>
> But --check is also producing useful output, which might need paged. So
> you are sacrificing existing interactive use of --check for scriptable
> exit-code uses. If you really want the exit code, why not "git diff
> --check --exit-code"?
>
> OTOH, I am not too sad to lose the paging behavior; it would take quite
> a few whitespace errors to scroll off the screen.
You are right. While I do not personally miss paging output, it is a
regression not to page --check output by default.
By the way, there is no reason to make --check and --exit-code mutually
exclusive either. You could say with --exit-code the command will exit
with status 01 or'ed in if trees are not identical, and with --check the
command will exit with status 02 or'ed in. Loosely written scripted
callers can continue doing:
if git --no-pager diff --exit-code
then
they are different
fi
if git --no-pager diff --check
then
there are funky blanks
fi
while the ones that are aware of the new behaviour of --check can:
git --no-pager diff --check --exit-code
case $? in
0) all is well ;;
1) clean difference there ;;
3) dirty difference there ;;
2) cannot happen ;;
*) bombed out, as die exits with 128 ;;
esac
^ permalink raw reply
* Re: [Funky] "git -p cmd" inside a bare repository
From: Jeff King @ 2007-12-14 5:12 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, git, Johannes Schindelin
In-Reply-To: <7vmysexdvw.fsf@gitster.siamese.dyndns.org>
On Thu, Dec 13, 2007 at 05:29:23PM -0800, Junio C Hamano wrote:
> I _think_ what is happening is that setup_pager() tries to run
> git_config(), which runs setup(), and then RUN_SETUP set for "ls-tree"
> (or "show") internal command runs setup again. HEAD is given to
> resolve_ref() and git_path("%s", ref) makes it to ".git/HEAD", even
> though in a bare repository git_dir should be set to ".", and of course
> we cannot find such a path in the git directory.
I think that there is perhaps a larger bug here, which is that running
setup twice gives bad results, and should either be fixed or have its
own "don't run me twice" guard.
But it makes sense to always spawn the pager at the same time for
consistency. As a bonus, this makes "git -p bogus" a little more
friendly by not spawning the pager until we verify the command name.
-- >8 --
delay "git -p" page spawning until command runtime
This makes the timing consistent with those commands that always spawn a
pager. It also avoids a funny interaction related to calling
setup_pager() before setup().
---
diff --git a/git.c b/git.c
index c8b7e74..21d204f 100644
--- a/git.c
+++ b/git.c
@@ -6,6 +6,8 @@
const char git_usage_string[] =
"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
+static int user_asked_for_pager;
+
static int handle_options(const char*** argv, int* argc, int* envchanged)
{
int handled = 0;
@@ -35,7 +37,7 @@ static int handle_options(const char*** argv, int* argc, int* envchanged)
exit(0);
}
} else if (!strcmp(cmd, "-p") || !strcmp(cmd, "--paginate")) {
- setup_pager();
+ user_asked_for_pager = 1;
} else if (!strcmp(cmd, "--no-pager")) {
setenv("GIT_PAGER", "cat", 1);
if (envchanged)
@@ -256,7 +258,7 @@ static int run_command(struct cmd_struct *p, int argc, const char **argv)
prefix = NULL;
if (p->option & RUN_SETUP)
prefix = setup_git_directory();
- if (p->option & USE_PAGER)
+ if (p->option & USE_PAGER || user_asked_for_pager)
setup_pager();
if (p->option & NEED_WORK_TREE)
setup_work_tree();
^ permalink raw reply related
* Re: [Funky] "git -p cmd" inside a bare repository
From: Jeff King @ 2007-12-14 5:14 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nguyễn Thái Ngọc Duy, git, Johannes Schindelin
In-Reply-To: <20071214051223.GD10169@sigill.intra.peff.net>
On Fri, Dec 14, 2007 at 12:12:23AM -0500, Jeff King wrote:
> -- >8 --
> delay "git -p" page spawning until command runtime
Hrmph. Bad timing. :)
Your patch is much nicer, though, so please ignore mine.
-Peff
^ permalink raw reply
* Re: [PATCH] provide advance warning of some future pack default changes
From: Junio C Hamano @ 2007-12-14 5:19 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: git
In-Reply-To: <alpine.LFD.0.999999.0712132227090.8467@xanadu.home>
Thanks.
Deprecating versions before 1.5.2 (May 20 2007) feels a bit too quick,
but seven month is almost an eternity in git timescale, and by now
anything older than 1.5.2 can safely be called prehistoric. Will apply.
^ permalink raw reply
* Re: [BUG] Failed to make install-info
From: Christian Couder @ 2007-12-14 5:35 UTC (permalink / raw)
To: Jon Loeliger; +Cc: git
In-Reply-To: <E1J2q92-0001YT-BZ@jdl.com>
Le jeudi 13 décembre 2007, Jon Loeliger a écrit :
> Guys,
> I managed to fail to install info files during:
>
> /usr/src/git# make prefix=/usr install-info
I am also struggling with "make install-info" to make it install in
~/share/info/dir
First it barfed because "~/share/info/dir" did not exist.
Then it doesn't find "install-info" because it's in "/usr/sbin" on my debian
machine.
I well tell if I get the same error as you. But right now I cannot
invastigate more.
Thanks,
Christian.
^ permalink raw reply
* Re: [PATCH] Authentication support for pserver
From: Junio C Hamano @ 2007-12-14 5:32 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: git, martyn, martin
In-Reply-To: <87wsrhex4c.fsf@cpan.org>
avar@cpan.org (Ævar Arnfjörð Bjarmason) writes:
> + unless ($user eq 'anonymous') {
> + # Trying to authenticate a user
> + if (not exists $cfg->{gitcvs}->{users}) {
> + print "E the repo config file needs a [gitcvs.users] section with user/password key-value pairs\n";
> + print "I HATE YOU\n";
> + exit 1;
> + } elsif (exists $cfg->{gitcvs}->{users} and not exists $cfg->{gitcvs}->{users}->{$user}) {
> + print "E the repo config file has a [gitcvs.users] section but the user $user is not defined in it\n";
> + print "I HATE YOU\n";
> + exit 1;
> + } else {
> + my $descrambled_password = descramble($password);
> + my $cleartext_password = $cfg->{gitcvs}->{users}->{$user};
> + if ($descrambled_password ne $cleartext_password) {
> + print "E The password supplied for user $user was incorrect\n";
> + print "I HATE YOU\n";
> + exit 1;
> + }
I do not know what the real pserver does but by sending these E lines in
the latter two different forms back to the client you are leaking
sensitive information, which is probably not what you want (the first
one is Ok, though. It would help the server administrator to notice
misconfiguration, and until it is corrected nobody would be able to log
in anyway).
Admittedly, the pserver password scrambling is not a real security, but
if we were paranoid, we would probably be even adding random delay in
"no user found" case and "password does not match" case, so that the
client cannot even tell from the response latency if a username exists
at the server.
> @@ -1176,12 +1196,6 @@ sub req_ci
>
> $log->info("req_ci : " . ( defined($data) ? $data : "[NULL]" ));
>
> - if ( $state->{method} eq 'pserver')
> - {
> - print "error 1 pserver access cannot commit\n";
> - exit;
> - }
> -
Is this correct? You are still allowing anonymous pserver access, so
shouldn't you check if this was an anonymous access or authenticated one
and refuse access like before for anonymous people?
> + my ($str) = @_;
> +
> + # This should never happen, the same format has been used since
> + # CVS was spawned
> + $str =~ s/^(.)//;
> + die "invalid password format $1" unless $1 eq 'A';
I do not quite understand what "spawn" means in this sentence.
^ permalink raw reply
* Re: [PATCH] Authentication support for pserver
From: Shawn O. Pearce @ 2007-12-14 5:44 UTC (permalink / raw)
To: Junio C Hamano
Cc: Ævar Arnfjörð Bjarmason, git, martyn, martin
In-Reply-To: <7vd4t9x2lw.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> avar@cpan.org (Ævar Arnfjörð Bjarmason) writes:
> > +
> > + # This should never happen, the same format has been used since
> > + # CVS was spawned
> > + $str =~ s/^(.)//;
> > + die "invalid password format $1" unless $1 eq 'A';
>
> I do not quite understand what "spawn" means in this sentence.
spawned: since it came from the depths of hell and cursed all
developers, until the day Git was born. :-)
--
Shawn.
^ permalink raw reply
* Re: [RFH] convert shortlog to use parse_options
From: Junio C Hamano @ 2007-12-14 5:59 UTC (permalink / raw)
To: Jeff King; +Cc: Kristian Høgsberg, Pierre Habouzit, git
In-Reply-To: <20071214040803.GA10169@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I agree. I am already a little bit uncomfortable with the "--abbrev 10"
> won't work but "--foo 10" will, because it requires that the user
> remember which arguments are optional and which are required. But
> switching it to "--abbrev 10 works, but --abbrev $foo does not, unless
> of course $foo is an integer, in which case you must use --abbrev=$foo"
> is just a little bit too DWIM. E.g., if you are scripting, it's just one
> more source of error (if I have $foo, how must I write --abbrev $foo for
> it to ensure that I _don't_ trigger the optional argument?).
Oh, there is no question that scripted callers need a way to safely
disambiguate, and be able to call "git cmd" with always the default
abbreviation for whatever $foo it gets from the user unambiguously.
I do not disagree with that.
But that is different from making it harder for end users (not scripters
but command line users).
Again, I am not saying that my suggested alternative is superiour; I
just threw it out to as an example of a different approach to achieve
disambiguation than the "some flags must have its parameter stuck with
it, some don't" behaviour, which will surely be confusing to the command
line end users.
To contrast the two, with Pierre's, if you want $n-digit abbreviations
and $cnt-lines of output in your script, your script would say:
git cmd --abbrev=$n -n $cnt $foo
because you as a script writer are required to know --abbrev is such a
magic flag that take optional parameter and cannot be fed as two options
(you can also write "-n=$cnt", but I am talking about --abbrev part).
If you are accepting the default abbreviation, on the other hand:
git cmd --abbrev -n $cnt $foo
which looks nice. The latter needs to be written in a funky way:
git cmd --abbrev= $foo
if we take "empty parameter to a flag that take optional parameter means
use the default setting"; we could introduce a magic 'default' token to
read it slightly better:
git cmd --abbrev=default $foo
but that does not change the fact that it makes it harder for
scripters. I do not disagree with that.
The command line end users, who want to do the same, but has a bit more
concrete than unknown $n, $cnt, or $foo, can do
git cmd --abbrev HEAD~4
in either approach. However, with Pierre's, the command line end users
can say either:
git cmd --abbrev=10 -n=4
git cmd --abbrev=10 -n 4
git cmd --abbrev=10 -n4
but they cannot say:
git cmd --abbrev 10 -n 4
They need to learn the difference between --abbrev and -n, because you
avoid DWIMmery for the sake of script writers. I have a slight
suspicion that it is backwards.
If there is _no_ existing users and previous versions of git, one
plausible alternative that would be much cleaner than anything we
discussed so far would be to always require '-n=4' (or "-n4") form and
never accept "-n 4", even for a flag that takes mandatory parameter.
Then there is no room for confusion. Users (both command line end users
and script writers) need to learn only one rule: flag parameters are
always with the flag, not given as two words.
I really wish the world were that simple, but I do no think that would
fly well with the existing users' fingers.
^ permalink raw reply
* Creating multiple modules under a project
From: Imran M Yousuf @ 2007-12-14 6:03 UTC (permalink / raw)
To: git
Hi,
I have a project that has just started and has 7 modules and as days
pass by the number of modules will only increase. Currently I am using
SVN and I am so impressed with GIT's concept and Linus's presentation
at Google that I want to use GIT with SVN. I want to create separate
repositories for each modules and then group them under a master
module which I am calling project. In this regard I would also like to
mention that the whole project gets syncd with a SVN server. Can
someone please point me to any article/blog or link where I can learn
on how to do it. If someone would take time to explain the how-to over
email that is also fine.
Thank you & best regards,
--
Imran M Yousuf
Entrepreneur & Software Engineer
Smart IT Engineering
Dhaka, Bangladesh
Email: imran@smartitengineering.com
Mobile: +880-1711402557
^ permalink raw reply
* Re: [BUG] Failed to make install-info
From: Junio C Hamano @ 2007-12-14 6:03 UTC (permalink / raw)
To: Christian Couder; +Cc: Jon Loeliger, git
In-Reply-To: <200712140635.39311.chriscool@tuxfamily.org>
Christian Couder <chriscool@tuxfamily.org> writes:
> Le jeudi 13 décembre 2007, Jon Loeliger a écrit :
>> Guys,
>> I managed to fail to install info files during:
>>
>> /usr/src/git# make prefix=/usr install-info
>
> I am also struggling with "make install-info" to make it install in
> ~/share/info/dir
>
> First it barfed because "~/share/info/dir" did not exist.
>
> Then it doesn't find "install-info" because it's in "/usr/sbin" on my debian
> machine.
>
> I well tell if I get the same error as you. But right now I cannot
> invastigate more.
I personally feel that calling "install-info" is a bit too distro-ish
and should not be done in our Makefile vanilla build-and-install
procedure.
^ permalink raw reply
* Re: [Funky] "git -p cmd" inside a bare repository
From: Junio C Hamano @ 2007-12-14 6:07 UTC (permalink / raw)
To: Jeff King; +Cc: Nguyễn Thái Ngọc Duy, git, Johannes Schindelin
In-Reply-To: <20071214051434.GE10169@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Fri, Dec 14, 2007 at 12:12:23AM -0500, Jeff King wrote:
>
>> -- >8 --
>> delay "git -p" page spawning until command runtime
>
> Hrmph. Bad timing. :)
>
> Your patch is much nicer, though, so please ignore mine.
It may look nicer, but I do not know if it is correct, though. I do not
do much "work tree" stuff, and would really appreciate testing by people
who are more involved in that part of the system.
^ 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