* Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes.
From: Junio C Hamano @ 2006-12-31 6:23 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20061231061232.GC6106@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> Junio C Hamano <junkio@cox.net> wrote:
>
>> Why? Are you allowing "1024 k"? Do we want to?
>
> Yes. Why not?
Stricter input checking. Allowing extra things later is far
easier than later finding problems with a looser way we started
from and having to tighten it.
^ permalink raw reply
* Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes.
From: Shawn O. Pearce @ 2006-12-31 6:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy7ooip6y.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> wrote:
> > diff --git a/config.c b/config.c
> > index 2e0d5a8..83ce9e1 100644
> > --- a/config.c
> > +++ b/config.c
> > @@ -236,8 +236,16 @@ int git_config_int(const char *name, const char *value)
> > if (value && *value) {
> > char *end;
> > int val = strtol(value, &end, 0);
> > + while (isspace(*end))
> > + end++;
>
> Why? Are you allowing "1024 k"? Do we want to?
Yes. Why not?
--
Shawn.
^ permalink raw reply
* Re: [RFC/PATCH 4/4] Disallow working directory commands in a bare repository.
From: Shawn O. Pearce @ 2006-12-31 6:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7virfsk4sd.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> wrote:
> But execution has room for improvement, and it is not your
> fault. The shell script version of require_not_bare is as
> fragile as is_bare_git_dir(), which I already do not like, and I
> think if we are going to use is_bare_git_dir() more, I think we
> would want to have something a bit more robust.
Agreed. But as you point out...
> If we could outlaw $GIT_DIR/index in a bare repository, then
> lack of $GIT_DIR/index combined with nonexistence of ref that
> is pointed at by $GIT_DIR/HEAD could become a good indication
> that the repository is bare ("the current branch unborn" check
> is needed not to mistake a repository before the initial commit
> as a bare one).
>
> Alas, many public repositories you would see (e.g. check
> kernel.org) have been primed with rsync of .git/ from
> developer's working repository and have leftover index that is
> otherwise unused. Because of this heavy historical baggage, I
> suspect that it is rather hard to convince people to allow us to
> use this technique.
I almost coded the require_not_bare() to look for $GIT_DIR/index
but didn't for the reasons you point out above. It is too bad that
we didn't enforce the existance of the index file better in the past.
> When you have $GIT_DIR in your environment, no working-tree
> command is expected to work unless you are at the toplevel of
> the working-tree. In the past, people talked about their
> workflows using more than one working trees that are associated
> with a single $GIT_DIR
What happens with $GIT_DIR/index in this case? Or $GIT_DIR/HEAD?
Its insane because both need to be affiliated with the working
directory, yet there's only the one set. Sure the user could also
arrange for $GIT_INDEX_FILE to be set based on the working directory,
but what about $GIT_DIR/HEAD? There is no equivilant.
> and that is certainly supposed to work.
> But I wonder how widely such a set-up is employed in practice.
> If we outlawed working-tree commands when $GIT_DIR environment
> exists, how much hurt are we talking about, I wonder.
I wouldn't be hurt, but I don't call Porcelain-ish unless I'm
entering commands directly on the command line, and I never set
GIT_DIR except in scripts, and even then its very rare and is more
to point at a bare repository than one with a working directory.
I suspect that probably isn't true for everyone.
> Another thing to think about is if we are happy with the above
> restriction that makes environment $GIT_DIR to imply you are
> always working at the toplevel. Maybe it could be a good idea
> to kill this bird as well by introducing $GIT_WORKTREE_TOP
> environment variable. Presence of it obviously means we are not
> in a bare repository, but at the same time it would allow us to
> teach setup_git_directory_gentry() to cd up to that directory to
> make the commands behave as expected.
I'm not sure what value that environment variable offers here.
Users who set GIT_DIR but don't today set GIT_WORKTREE_TOP are
broken by this change because its not set and we assume its bare.
So they need to also set this environment variable. But that
is probably going to be annoying for those users as I doubt they
are setting GIT_DIR on a per-directory basis.
Why not just tell these users to setup the working directories with
local .git directories and not use GIT_DIR?
--
Shawn.
^ permalink raw reply
* Re: [PATCH 3/3] Use /dev/null for update hook stdin.
From: Junio C Hamano @ 2006-12-31 6:03 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20061231025522.GC5530@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> Currently the update hook invoked by receive-pack has its stdin
> connected to the pushing client. The hook shouldn't attempt to
> read from this stream, and doing so may consume data that was
> meant for receive-pack. Instead we should give the update hook
> /dev/null as its stdin, ensuring that it always receives EOF and
> doesn't disrupt the protocol if it attempts to read any data.
Both 2 and 3 look sane, but can we have a few new tests too?
^ permalink raw reply
* Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes.
From: Junio C Hamano @ 2006-12-31 5:56 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20061231031305.GA5620@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> diff --git a/Documentation/git-repo-config.txt b/Documentation/git-repo-config.txt
> index b379ec5..c55a8ba 100644
> --- a/Documentation/git-repo-config.txt
> +++ b/Documentation/git-repo-config.txt
> @@ -87,7 +87,10 @@ OPTIONS
> git-repo-config will ensure that the output is "true" or "false"
>
> --int::
> - git-repo-config will ensure that the output is a simple decimal number
> + git-repo-config will ensure that the output is a simple
> + decimal number. An optional value suffix of 'k', 'm', or 'g'
> + in the config file will cause the value to be multiplied
> + by 1024, 1048576, or 1073741824 prior to output.
Thanks. I think this is applicable on top of 'master'.
> ENVIRONMENT
> diff --git a/config.c b/config.c
> index 2e0d5a8..83ce9e1 100644
> --- a/config.c
> +++ b/config.c
> @@ -236,8 +236,16 @@ int git_config_int(const char *name, const char *value)
> if (value && *value) {
> char *end;
> int val = strtol(value, &end, 0);
> + while (isspace(*end))
> + end++;
Why? Are you allowing "1024 k"? Do we want to?
> if (!*end)
> return val;
> + if (!strcasecmp(end, "k"))
> + return val * 1024;
> + if (!strcasecmp(end, "m"))
> + return val * 1024 * 1024;
> + if (!strcasecmp(end, "g"))
> + return val * 1024 * 1024 * 1024;
> }
> die("bad config value for '%s' in %s", name, config_file_name);
> }
> --
> 1.5.0.rc0.g6bb1
^ permalink raw reply
* Re: What commands can and can not be used with bare repositories?
From: Junio C Hamano @ 2006-12-31 5:52 UTC (permalink / raw)
To: Theodore Tso; +Cc: git
In-Reply-To: <20061231021248.GA26067@thunk.org>
Theodore Tso <tytso@mit.edu> writes:
> That makes sense, although the hueristic for determining whether or
> not "." is a Git repository might be a little interesting. Say, if
> there is no containing directory which has a .git directory, and the
> directories "objects", "info", and "refs" are present?
We have something called is_bare_git_dir() that does much
simpler and hacky check.
See my other message to Shawn regarding tradeoffs between
possible improvements and backward compatibility.
^ permalink raw reply
* Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes.
From: Shawn O. Pearce @ 2006-12-31 5:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vejqgk4l1.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> wrote:
> Shawn Pearce <spearce@spearce.org> writes:
>
> > So in your test case where you thought you entered 20 pages, what you
> > really had was just 2 pages. We rounded 20 bytes down to 0 pages,
> > then forced that to a minimum of 2 pages. :-)
>
> Yes.
>
> And I found something else that is interesting. I still had 2
> pages windows but reduced packedgitlimit to 256kB and re-run the
> same test to reproduce all merges in git.git. It still did not
> fail, but it took even shorter wallclock time (again, it is only
> 3 seconds or so out of 5 1/2 minuts, so it is probably
> statistically insignificant).
Its possible the packed_git.windows list is hurting us here.
Its O(n) to locate a given window. The larger the ratio between
packedGitWindowSize and packedGitLimit the longer that list can get,
and the longer it can take to locate a window.
Maybe that should be a simple red-black tree?
More testing probably needs to be done though.
--
Shawn.
^ permalink raw reply
* Re: [PATCH 3/4] Automatically detect a bare git repository.
From: Junio C Hamano @ 2006-12-31 5:46 UTC (permalink / raw)
To: Shawn Pearce; +Cc: git
In-Reply-To: <20061231052606.GA5722@spearce.org>
Shawn Pearce <spearce@spearce.org> writes:
> Junio C Hamano <junkio@cox.net> wrote:
> ...
>
> If GIT_DIR is set we call is_git_directory(); if that returns 1
> then the checks passed. In this case the old code returned NULL
> and ignored nongit_ok. We do the same in the new code.
Ok.
>> > for (;;) {
>> > - if (is_toplevel_directory())
>> > + if (is_git_directory(".git"))
>> > break;
>> > chdir("..");
>> > do {
>> > if (!offset) {
>> > + if (is_git_directory(cwd)) {
>> > + if (chdir(cwd))
>> > + die("Cannot come back to cwd");
>> > + setenv(GIT_DIR_ENVIRONMENT, cwd, 1);
>> > + return NULL;
>> > + }
>> > if (nongit_ok) {
>> > if (chdir(cwd))
>> > die("Cannot come back to cwd");
>>
>> I do not know what the new behaviour of this part of the code is
>> trying to do. This is supposed to see if "." is the toplevel
>> (equivalently, ".git" is the git_dir, in your implementation),
>> otherwise chdir("..") repeatedly until it finds one, and the
>> normal return condition is for the working directory of the
>> process to be at the toplevel. So chdir(cwd) you introduced is
>> obviously changing the behaviour.
>
> No, its not.
Ah, "changing the behaviour" is the correct thing to do, because
the code is now allowing a new fallback to allow the original
directory to be a bare git repository, and the new chdir/setenv
makes perfect sense. Thanks for clarification.
For a short time I wondered if this should be a fallback
position, or we might want to make this the very first thing to
check, but I think adding this as the last ditch fallback
position as you did is much safer than making it the first thing
to check.
Thanks.
^ permalink raw reply
* Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes.
From: Junio C Hamano @ 2006-12-31 5:38 UTC (permalink / raw)
To: Shawn Pearce; +Cc: git
In-Reply-To: <20061231030119.GD5082@spearce.org>
Shawn Pearce <spearce@spearce.org> writes:
> So in your test case where you thought you entered 20 pages, what you
> really had was just 2 pages. We rounded 20 bytes down to 0 pages,
> then forced that to a minimum of 2 pages. :-)
Yes.
And I found something else that is interesting. I still had 2
pages windows but reduced packedgitlimit to 256kB and re-run the
same test to reproduce all merges in git.git. It still did not
fail, but it took even shorter wallclock time (again, it is only
3 seconds or so out of 5 1/2 minuts, so it is probably
statistically insignificant).
^ permalink raw reply
* Re: What commands can and can not be used with bare repositories?
From: Michael S. Tsirkin @ 2006-12-31 5:32 UTC (permalink / raw)
To: Theodore Tso; +Cc: Shawn Pearce, git
In-Reply-To: <20061231021248.GA26067@thunk.org>
> That makes sense, although the hueristic for determining whether or
> not "." is a Git repository might be a little interesting. Say, if
> there is no containing directory which has a .git directory, and the
> directories "objects", "info", and "refs" are present?
Maybe, git clone --bare should create a special file under the
repository directory?
--
MST
^ permalink raw reply
* Re: [RFC/PATCH 4/4] Disallow working directory commands in a bare repository.
From: Junio C Hamano @ 2006-12-31 5:33 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20061231043238.GD5823@spearce.org>
I think the intent is good.
But execution has room for improvement, and it is not your
fault. The shell script version of require_not_bare is as
fragile as is_bare_git_dir(), which I already do not like, and I
think if we are going to use is_bare_git_dir() more, I think we
would want to have something a bit more robust.
If we could outlaw $GIT_DIR/index in a bare repository, then
lack of $GIT_DIR/index combined with nonexistence of ref that
is pointed at by $GIT_DIR/HEAD could become a good indication
that the repository is bare ("the current branch unborn" check
is needed not to mistake a repository before the initial commit
as a bare one).
Alas, many public repositories you would see (e.g. check
kernel.org) have been primed with rsync of .git/ from
developer's working repository and have leftover index that is
otherwise unused. Because of this heavy historical baggage, I
suspect that it is rather hard to convince people to allow us to
use this technique.
When you have $GIT_DIR in your environment, no working-tree
command is expected to work unless you are at the toplevel of
the working-tree. In the past, people talked about their
workflows using more than one working trees that are associated
with a single $GIT_DIR and that is certainly supposed to work.
But I wonder how widely such a set-up is employed in practice.
If we outlawed working-tree commands when $GIT_DIR environment
exists, how much hurt are we talking about, I wonder.
Another thing to think about is if we are happy with the above
restriction that makes environment $GIT_DIR to imply you are
always working at the toplevel. Maybe it could be a good idea
to kill this bird as well by introducing $GIT_WORKTREE_TOP
environment variable. Presence of it obviously means we are not
in a bare repository, but at the same time it would allow us to
teach setup_git_directory_gentry() to cd up to that directory to
make the commands behave as expected.
^ permalink raw reply
* Re: [PATCH 3/4] Automatically detect a bare git repository.
From: Shawn Pearce @ 2006-12-31 5:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Theodore Tso
In-Reply-To: <7vy7ook5xj.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
>
> > This is in response to Theodore Tso's email asking why 'git log'
> > doesn't work in a bare repository. Now it does. :-)
>
> Does it?
It did in my testing. ;-)
> > @@ -160,36 +178,17 @@ const char *setup_git_directory_gently(int *nongit_ok)
> > * to do any discovery, but we still do repository
> > * validation.
> > */
> > - if (getenv(GIT_DIR_ENVIRONMENT)) {
> > - char path[PATH_MAX];
> > - int len = strlen(getenv(GIT_DIR_ENVIRONMENT));
> > - if (sizeof(path) - 40 < len)
> > + gitdirenv = getenv(GIT_DIR_ENVIRONMENT);
> > + if (gitdirenv) {
> > + if (PATH_MAX - 40 < strlen(gitdirenv))
> > die("'$%s' too big", GIT_DIR_ENVIRONMENT);
> > - memcpy(path, getenv(GIT_DIR_ENVIRONMENT), len);
> > -
> > - strcpy(path + len, "/refs");
> > - if (access(path, X_OK))
> > - goto bad_dir_environ;
> > - strcpy(path + len, "/HEAD");
> > - if (validate_symref(path))
> > - goto bad_dir_environ;
> > - if (getenv(DB_ENVIRONMENT)) {
> > - if (access(getenv(DB_ENVIRONMENT), X_OK))
> > - goto bad_dir_environ;
> > - }
> > - else {
> > - strcpy(path + len, "/objects");
> > - if (access(path, X_OK))
> > - goto bad_dir_environ;
> > - }
> > - return NULL;
> > - bad_dir_environ:
> > + if (is_git_directory(gitdirenv))
> > + return NULL;
> > if (nongit_ok) {
> > *nongit_ok = 1;
> > return NULL;
> > }
>
> I do not think this is correct.
>
> What happens when GIT_DIR is set, and nongit_ok is passed?
> Earlier code returned NULL after setting *nongit_ok so that the
> caller knows the environment points at a directory that is not
> yet a git repository control area.
The new code is correct, or at least does what the old code did.
If GIT_DIR is set we call is_git_directory(); if that returns 1
then the checks passed. In this case the old code returned NULL
and ignored nongit_ok. We do the same in the new code.
If GIT_DIR is set and we fail is_git_directory() then one of the
checks failed. In this case the old code jumped to bad_dir_env.
In the new code we don't return NULL and fall through into the if
nongit_ok testing, or into the die("Not a git repository").
> > @@ -197,11 +196,17 @@ const char *setup_git_directory_gently(int *nongit_ok)
> >
> > offset = len = strlen(cwd);
> > for (;;) {
> > - if (is_toplevel_directory())
> > + if (is_git_directory(".git"))
> > break;
> > chdir("..");
> > do {
> > if (!offset) {
> > + if (is_git_directory(cwd)) {
> > + if (chdir(cwd))
> > + die("Cannot come back to cwd");
> > + setenv(GIT_DIR_ENVIRONMENT, cwd, 1);
> > + return NULL;
> > + }
> > if (nongit_ok) {
> > if (chdir(cwd))
> > die("Cannot come back to cwd");
>
> I do not know what the new behaviour of this part of the code is
> trying to do. This is supposed to see if "." is the toplevel
> (equivalently, ".git" is the git_dir, in your implementation),
> otherwise chdir("..") repeatedly until it finds one, and the
> normal return condition is for the working directory of the
> process to be at the toplevel. So chdir(cwd) you introduced is
> obviously changing the behaviour.
No, its not.
We only bother to look to see if the original cwd (before we started
chdir("..")'ing up) is a git directory if we did not find one during
that chdir up loop. This means our current working directory is now
"/" but we found a valid repository in cwd.
In this case we chdir back to the repository directory before
returning back to the caller, as what's the point of being in "/"
when running in a bare repository? Probably better to be in the
repository directory itself.
One could argue that maybe we should run in "/", or in "/tmp" in
this case as there is no working directory associated with this
repository, but that argument is about the same as just saying we
go back into the now discovered GIT_DIR.
--
Shawn.
^ permalink raw reply
* Re: [PATCH 3/4] Automatically detect a bare git repository.
From: Junio C Hamano @ 2006-12-31 5:09 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Theodore Tso
In-Reply-To: <20061231043019.GC5823@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> This is in response to Theodore Tso's email asking why 'git log'
> doesn't work in a bare repository. Now it does. :-)
Does it?
> +static int is_git_directory(const char *suspect)
> {
> + char path[PATH_MAX];
> ...
> +
> return 1;
> }
I think this is a good refactoring.
> @@ -160,36 +178,17 @@ const char *setup_git_directory_gently(int *nongit_ok)
> * to do any discovery, but we still do repository
> * validation.
> */
> - if (getenv(GIT_DIR_ENVIRONMENT)) {
> - char path[PATH_MAX];
> - int len = strlen(getenv(GIT_DIR_ENVIRONMENT));
> - if (sizeof(path) - 40 < len)
> + gitdirenv = getenv(GIT_DIR_ENVIRONMENT);
> + if (gitdirenv) {
> + if (PATH_MAX - 40 < strlen(gitdirenv))
> die("'$%s' too big", GIT_DIR_ENVIRONMENT);
> - memcpy(path, getenv(GIT_DIR_ENVIRONMENT), len);
> -
> - strcpy(path + len, "/refs");
> - if (access(path, X_OK))
> - goto bad_dir_environ;
> - strcpy(path + len, "/HEAD");
> - if (validate_symref(path))
> - goto bad_dir_environ;
> - if (getenv(DB_ENVIRONMENT)) {
> - if (access(getenv(DB_ENVIRONMENT), X_OK))
> - goto bad_dir_environ;
> - }
> - else {
> - strcpy(path + len, "/objects");
> - if (access(path, X_OK))
> - goto bad_dir_environ;
> - }
> - return NULL;
> - bad_dir_environ:
> + if (is_git_directory(gitdirenv))
> + return NULL;
> if (nongit_ok) {
> *nongit_ok = 1;
> return NULL;
> }
I do not think this is correct.
What happens when GIT_DIR is set, and nongit_ok is passed?
Earlier code returned NULL after setting *nongit_ok so that the
caller knows the environment points at a directory that is not
yet a git repository control area.
> @@ -197,11 +196,17 @@ const char *setup_git_directory_gently(int *nongit_ok)
>
> offset = len = strlen(cwd);
> for (;;) {
> - if (is_toplevel_directory())
> + if (is_git_directory(".git"))
> break;
> chdir("..");
> do {
> if (!offset) {
> + if (is_git_directory(cwd)) {
> + if (chdir(cwd))
> + die("Cannot come back to cwd");
> + setenv(GIT_DIR_ENVIRONMENT, cwd, 1);
> + return NULL;
> + }
> if (nongit_ok) {
> if (chdir(cwd))
> die("Cannot come back to cwd");
I do not know what the new behaviour of this part of the code is
trying to do. This is supposed to see if "." is the toplevel
(equivalently, ".git" is the git_dir, in your implementation),
otherwise chdir("..") repeatedly until it finds one, and the
normal return condition is for the working directory of the
process to be at the toplevel. So chdir(cwd) you introduced is
obviously changing the behaviour.
The existing chdir(cwd) is for an error return -- when there was
no directory that has ".git" even when you went all the way up
to the root level, we give up and come back to where we started,
only when the caller suspected that there was no git directory
and is prepared to handle that case, which is signalled by us
storing 1 to *nongit_ok.
^ permalink raw reply
* [PATCH] Refresh the index before starting merge-recursive.
From: Shawn O. Pearce @ 2006-12-31 5:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Since merge-recursive won't perform a refresh of stale stat data on
its own we need to make sure we run `update-index --refresh` prior
to starting it, otherwise stale stat data may cause an otherwise
valid merge to fail.
This was simply a thinko on my part when I reorganized this section
of code. Silly copy and paste error and all....
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
Applies to the top of my sp/merge topic.
git-merge.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/git-merge.sh b/git-merge.sh
index 922c5b8..ced5524 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -296,6 +296,7 @@ f,*)
?,1,*,)
# We are not doing octopus, not fast forward, and have only
# one common. See if it is really trivial.
+ git-update-index --refresh 2>/dev/null
case "$use_strategies" in
recursive|'recursive '|recur|'recur ')
: run merge later
@@ -303,7 +304,6 @@ f,*)
*)
git var GIT_COMMITTER_IDENT >/dev/null || exit
echo "Trying really trivial in-index merge..."
- git-update-index --refresh 2>/dev/null
if git-read-tree --trivial -m -u -v $common $head "$1" &&
result_tree=$(git-write-tree)
then
--
1.5.0.rc0.g6bb1
^ permalink raw reply related
* [PATCH] Force core.filemode to false on Cygwin.
From: Shawn O. Pearce @ 2006-12-31 4:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Many users have noticed that core.filemode doesn't appear to be
automatically set right on Cygwin when using a repository stored
on NTFS. The issue is that Cygwin and NTFS correctly supports
the executable mode bit, and Git properly detected that, but most
native Windows applications tend to create files such that Cygwin
sees the executable bit set when it probably shouldn't be.
This is especially bad if the user's favorite editor deletes the
file then recreates it whenever they save (vs. just overwriting)
as now a file that was created with mode 0644 by checkout-index
appears to have mode 0755.
So we introduce NO_TRUSTABLE_FILEMODE, settable at compile time.
Setting this option forces core.filemode to false, even if the
detection code would have returned true. This option should be
enabled by default on Cygwin.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
Makefile | 7 +++++++
builtin-init-db.c | 14 ++++++++++----
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/Makefile b/Makefile
index 93dc494..fa1a022 100644
--- a/Makefile
+++ b/Makefile
@@ -72,6 +72,9 @@ all:
# Define NO_FAST_WORKING_DIRECTORY if accessing objects in pack files is
# generally faster on your platform than accessing the working directory.
#
+# Define NO_TRUSTABLE_FILEMODE if your filesystem may claim to support
+# the executable mode bit, but doesn't really do so.
+#
# Define NO_IPV6 if you lack IPv6 support and getaddrinfo().
#
# Define NO_SOCKADDR_STORAGE if your platform does not have struct
@@ -361,6 +364,7 @@ ifeq ($(uname_O),Cygwin)
NEEDS_LIBICONV = YesPlease
NO_C99_FORMAT = YesPlease
NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes
+ NO_TRUSTABLE_FILEMODE = UnfortunatelyYes
# There are conflicting reports about this.
# On some boxes NO_MMAP is needed, and not so elsewhere.
# Try commenting this out if you suspect MMAP is more efficient
@@ -521,6 +525,9 @@ endif
ifdef NO_FAST_WORKING_DIRECTORY
BASIC_CFLAGS += -DNO_FAST_WORKING_DIRECTORY
endif
+ifdef NO_TRUSTABLE_FILEMODE
+ BASIC_CFLAGS += -DNO_TRUSTABLE_FILEMODE
+endif
ifdef NO_IPV6
BASIC_CFLAGS += -DNO_IPV6
endif
diff --git a/builtin-init-db.c b/builtin-init-db.c
index 01f366a..cc4fd21 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -10,6 +10,12 @@
#define DEFAULT_GIT_TEMPLATE_DIR "/usr/share/git-core/templates/"
#endif
+#ifdef NO_TRUSTABLE_FILEMODE
+#define TEST_FILEMODE 0
+#else
+#define TEST_FILEMODE 1
+#endif
+
static void safe_create_dir(const char *dir, int share)
{
if (mkdir(dir, 0777) < 0) {
@@ -236,14 +242,14 @@ static int create_default_files(const char *git_dir, const char *template_path)
strcpy(path + len, "config");
/* Check filemode trustability */
- if (!lstat(path, &st1)) {
+ int filemode = TEST_FILEMODE;
+ if (TEST_FILEMODE && !lstat(path, &st1)) {
struct stat st2;
- int filemode = (!chmod(path, st1.st_mode ^ S_IXUSR) &&
+ filemode = (!chmod(path, st1.st_mode ^ S_IXUSR) &&
!lstat(path, &st2) &&
st1.st_mode != st2.st_mode);
- git_config_set("core.filemode",
- filemode ? "true" : "false");
}
+ git_config_set("core.filemode", filemode ? "true" : "false");
/* Enable logAllRefUpdates if a working tree is attached */
if (!is_bare_git_dir(git_dir))
--
1.5.0.rc0.g6bb1
^ permalink raw reply related
* [RFC/PATCH 4/4] Disallow working directory commands in a bare repository.
From: Shawn O. Pearce @ 2006-12-31 4:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <3ffc8ddd9b500c2a34d2bd6ba147dc750d951bcd.1167539318.git.spearce@spearce.org>
If the user tries to run a porcelainish command which requires
a working directory in a bare repository they may get unexpected
results which are difficult to predict and may differ from command
to command.
Instead we should detect that the current repository is a bare
repository and refuse to run the command there, as there is no
working directory associated with it.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
This is more of an RFC than an actual patch. I think its a good
idea (clearly, as I spent the time to write it) but this may break
users who explicitly set GIT_DIR to some path which doesn't end in
"/.git" (e.g. GIT_DIR=$HOME/foo.git).
I've tried to only alter poreclainish and leave plumbing alone,
under the rationale that a different Porcelain may have different
behavior with regards to GIT_DIR.
git-am.sh | 1 +
git-checkout.sh | 1 +
git-clean.sh | 1 +
git-commit.sh | 1 +
git-merge.sh | 1 +
git-pull.sh | 1 +
git-rebase.sh | 1 +
git-reset.sh | 1 +
git-revert.sh | 1 +
git-sh-setup.sh | 7 +++++++
git.c | 11 +++++++----
11 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/git-am.sh b/git-am.sh
index c3bbd78..8487e75 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -7,6 +7,7 @@ USAGE='[--signoff] [--dotest=<dir>] [--utf8] [--binary] [--3way]
or, when resuming [--skip | --resolved]'
. git-sh-setup
set_reflog_action am
+require_not_bare
git var GIT_COMMITTER_IDENT >/dev/null || exit
diff --git a/git-checkout.sh b/git-checkout.sh
index 92ec069..b2d2dfa 100755
--- a/git-checkout.sh
+++ b/git-checkout.sh
@@ -3,6 +3,7 @@
USAGE='[-f] [-b <new_branch>] [-m] [<branch>] [<paths>...]'
SUBDIRECTORY_OK=Sometimes
. git-sh-setup
+require_not_bare
old_name=HEAD
old=$(git-rev-parse --verify $old_name 2>/dev/null)
diff --git a/git-clean.sh b/git-clean.sh
index 3834323..79c5bad 100755
--- a/git-clean.sh
+++ b/git-clean.sh
@@ -14,6 +14,7 @@ When optional <paths>... arguments are given, the paths
affected are further limited to those that match them.'
SUBDIRECTORY_OK=Yes
. git-sh-setup
+require_not_bare
ignored=
ignoredonly=
diff --git a/git-commit.sh b/git-commit.sh
index 6bce41a..813f41c 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -6,6 +6,7 @@
USAGE='[-a] [-s] [-v] [--no-verify] [-m <message> | -F <logfile> | (-C|-c) <commit>] [-u] [--amend] [-e] [--author <author>] [[-i | -o] <path>...]'
SUBDIRECTORY_OK=Yes
. git-sh-setup
+require_not_bare
git-rev-parse --verify HEAD >/dev/null 2>&1 || initial_commit=t
branch=$(GIT_DIR="$GIT_DIR" git-symbolic-ref HEAD)
diff --git a/git-merge.sh b/git-merge.sh
index ba42260..d91ff0d 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -7,6 +7,7 @@ USAGE='[-n] [--no-commit] [--squash] [-s <strategy>] [-m=<merge-message>] <commi
. git-sh-setup
set_reflog_action "merge $*"
+require_not_bare
LF='
'
diff --git a/git-pull.sh b/git-pull.sh
index 28d0819..1d91386 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -8,6 +8,7 @@ USAGE='[-n | --no-summary] [--no-commit] [-s strategy]... [<fetch-options>] <rep
LONG_USAGE='Fetch one or more remote refs and merge it/them into the current HEAD.'
. git-sh-setup
set_reflog_action "pull $*"
+require_not_bare
strategy_args= no_summary= no_commit= squash=
while case "$#,$1" in 0) break ;; *,-*) ;; *) break ;; esac
diff --git a/git-rebase.sh b/git-rebase.sh
index 828c59c..fd58e8e 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -29,6 +29,7 @@ Example: git-rebase master~1 topic
'
. git-sh-setup
set_reflog_action rebase
+require_not_bare
RESOLVEMSG="
When you have resolved this problem run \"git rebase --continue\".
diff --git a/git-reset.sh b/git-reset.sh
index a969370..90164e1 100755
--- a/git-reset.sh
+++ b/git-reset.sh
@@ -6,6 +6,7 @@ USAGE='[--mixed | --soft | --hard] [<commit-ish>] [ [--] <paths>...]'
SUBDIRECTORY_OK=Yes
. git-sh-setup
set_reflog_action "reset $*"
+require_not_bare
update= reset_type=--mixed
unset rev
diff --git a/git-revert.sh b/git-revert.sh
index 50cc47b..f0d2829 100755
--- a/git-revert.sh
+++ b/git-revert.sh
@@ -19,6 +19,7 @@ case "$0" in
die "What are you talking about?" ;;
esac
. git-sh-setup
+require_not_bare
no_commit=
while case "$#" in 0) break ;; esac
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index 87b939c..7e1d024 100755
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -28,6 +28,13 @@ set_reflog_action() {
fi
}
+require_not_bare() {
+ case "$GIT_DIR" in
+ .git|*/.git) : ok;;
+ *) die "fatal: $0 cannot be used in a bare git directory."
+ esac
+}
+
if [ -z "$LONG_USAGE" ]
then
LONG_USAGE="Usage: $0 $USAGE"
diff --git a/git.c b/git.c
index c82ca45..61c6390 100644
--- a/git.c
+++ b/git.c
@@ -199,6 +199,7 @@ const char git_version_string[] = GIT_VERSION;
#define RUN_SETUP (1<<0)
#define USE_PAGER (1<<1)
+#define NOT_BARE (1<<2)
static void handle_internal_command(int argc, const char **argv, char **envp)
{
@@ -208,7 +209,7 @@ static void handle_internal_command(int argc, const char **argv, char **envp)
int (*fn)(int, const char **, const char *);
int option;
} commands[] = {
- { "add", cmd_add, RUN_SETUP },
+ { "add", cmd_add, RUN_SETUP | NOT_BARE },
{ "annotate", cmd_annotate, },
{ "apply", cmd_apply },
{ "archive", cmd_archive },
@@ -238,7 +239,7 @@ static void handle_internal_command(int argc, const char **argv, char **envp)
{ "mailinfo", cmd_mailinfo },
{ "mailsplit", cmd_mailsplit },
{ "merge-file", cmd_merge_file },
- { "mv", cmd_mv, RUN_SETUP },
+ { "mv", cmd_mv, RUN_SETUP | NOT_BARE },
{ "name-rev", cmd_name_rev, RUN_SETUP },
{ "pack-objects", cmd_pack_objects, RUN_SETUP },
{ "pickaxe", cmd_blame, RUN_SETUP | USE_PAGER },
@@ -251,8 +252,8 @@ static void handle_internal_command(int argc, const char **argv, char **envp)
{ "rerere", cmd_rerere, RUN_SETUP },
{ "rev-list", cmd_rev_list, RUN_SETUP },
{ "rev-parse", cmd_rev_parse, RUN_SETUP },
- { "rm", cmd_rm, RUN_SETUP },
- { "runstatus", cmd_runstatus, RUN_SETUP },
+ { "rm", cmd_rm, RUN_SETUP | NOT_BARE },
+ { "runstatus", cmd_runstatus, RUN_SETUP | NOT_BARE },
{ "shortlog", cmd_shortlog, RUN_SETUP | USE_PAGER },
{ "show-branch", cmd_show_branch, RUN_SETUP },
{ "show", cmd_show, RUN_SETUP | USE_PAGER },
@@ -289,6 +290,8 @@ static void handle_internal_command(int argc, const char **argv, char **envp)
prefix = setup_git_directory();
if (p->option & USE_PAGER)
setup_pager();
+ if (p->option & NOT_BARE && is_bare_git_dir(get_git_dir()))
+ die("%s cannot be used in a pare git directory", cmd);
trace_argv_printf(argv, argc, "trace: built-in: git");
exit(p->fn(argc, argv, prefix));
--
1.5.0.rc0.g6bb1
^ permalink raw reply related
* [PATCH 3/4] Automatically detect a bare git repository.
From: Shawn O. Pearce @ 2006-12-31 4:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Theodore Tso
In-Reply-To: <3ffc8ddd9b500c2a34d2bd6ba147dc750d951bcd.1167539318.git.spearce@spearce.org>
Many users find it unfriendly that they can create a bare git
repository easily with `git clone --bare` but are then unable to
run simple commands like `git log` once they cd into that newly
created bare repository. This occurs because we do not check to
see if the current working directory is a git repository.
Instead of failing out with "fatal: Not a git repository" we should
try to automatically detect if the current working directory is
a bare repository and use that for GIT_DIR, and fail out only if
that doesn't appear to be true.
We test the current working directory only after we have tried
searching up the directory tree. This is to retain backwards
compatibility with our previous behavior on the off chance that
a user has a 'refs' and 'objects' subdirectories and a 'HEAD'
file that looks like a symref, all stored within a repository's
associated working directory.
This change also consolidates the validation logic between the case
of GIT_DIR being supplied and GIT_DIR not being supplied, cleaning
up the code.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
This is in response to Theodore Tso's email asking why 'git log'
doesn't work in a bare repository. Now it does. :-)
setup.c | 73 +++++++++++++++++++++++++++++++++-----------------------------
1 files changed, 39 insertions(+), 34 deletions(-)
diff --git a/setup.c b/setup.c
index 2afdba4..2ae57f7 100644
--- a/setup.c
+++ b/setup.c
@@ -131,28 +131,46 @@ const char **get_pathspec(const char *prefix, const char **pathspec)
}
/*
- * Test if it looks like we're at the top level git directory.
+ * Test if it looks like we're at a git directory.
* We want to see:
*
- * - either a .git/objects/ directory _or_ the proper
+ * - either a objects/ directory _or_ the proper
* GIT_OBJECT_DIRECTORY environment variable
- * - a refs/ directory under ".git"
+ * - a refs/ directory
* - either a HEAD symlink or a HEAD file that is formatted as
* a proper "ref:".
*/
-static int is_toplevel_directory(void)
+static int is_git_directory(const char *suspect)
{
- if (access(".git/refs/", X_OK) ||
- access(getenv(DB_ENVIRONMENT) ?
- getenv(DB_ENVIRONMENT) : ".git/objects/", X_OK) ||
- validate_symref(".git/HEAD"))
+ char path[PATH_MAX];
+ size_t len = strlen(suspect);
+
+ strcpy(path, suspect);
+ if (getenv(DB_ENVIRONMENT)) {
+ if (access(getenv(DB_ENVIRONMENT), X_OK))
+ return 0;
+ }
+ else {
+ strcpy(path + len, "/objects");
+ if (access(path, X_OK))
+ return 0;
+ }
+
+ strcpy(path + len, "/refs");
+ if (access(path, X_OK))
return 0;
+
+ strcpy(path + len, "/HEAD");
+ if (validate_symref(path))
+ return 0;
+
return 1;
}
const char *setup_git_directory_gently(int *nongit_ok)
{
static char cwd[PATH_MAX+1];
+ const char *gitdirenv;
int len, offset;
/*
@@ -160,36 +178,17 @@ const char *setup_git_directory_gently(int *nongit_ok)
* to do any discovery, but we still do repository
* validation.
*/
- if (getenv(GIT_DIR_ENVIRONMENT)) {
- char path[PATH_MAX];
- int len = strlen(getenv(GIT_DIR_ENVIRONMENT));
- if (sizeof(path) - 40 < len)
+ gitdirenv = getenv(GIT_DIR_ENVIRONMENT);
+ if (gitdirenv) {
+ if (PATH_MAX - 40 < strlen(gitdirenv))
die("'$%s' too big", GIT_DIR_ENVIRONMENT);
- memcpy(path, getenv(GIT_DIR_ENVIRONMENT), len);
-
- strcpy(path + len, "/refs");
- if (access(path, X_OK))
- goto bad_dir_environ;
- strcpy(path + len, "/HEAD");
- if (validate_symref(path))
- goto bad_dir_environ;
- if (getenv(DB_ENVIRONMENT)) {
- if (access(getenv(DB_ENVIRONMENT), X_OK))
- goto bad_dir_environ;
- }
- else {
- strcpy(path + len, "/objects");
- if (access(path, X_OK))
- goto bad_dir_environ;
- }
- return NULL;
- bad_dir_environ:
+ if (is_git_directory(gitdirenv))
+ return NULL;
if (nongit_ok) {
*nongit_ok = 1;
return NULL;
}
- path[len] = 0;
- die("Not a git repository: '%s'", path);
+ die("Not a git repository: '%s'", gitdirenv);
}
if (!getcwd(cwd, sizeof(cwd)) || cwd[0] != '/')
@@ -197,11 +196,17 @@ const char *setup_git_directory_gently(int *nongit_ok)
offset = len = strlen(cwd);
for (;;) {
- if (is_toplevel_directory())
+ if (is_git_directory(".git"))
break;
chdir("..");
do {
if (!offset) {
+ if (is_git_directory(cwd)) {
+ if (chdir(cwd))
+ die("Cannot come back to cwd");
+ setenv(GIT_DIR_ENVIRONMENT, cwd, 1);
+ return NULL;
+ }
if (nongit_ok) {
if (chdir(cwd))
die("Cannot come back to cwd");
--
1.5.0.rc0.g6bb1
^ permalink raw reply related
* [PATCH 2/4] Replace "GIT_DIR" with GIT_DIR_ENVIRONMENT.
From: Shawn O. Pearce @ 2006-12-31 4:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <3ffc8ddd9b500c2a34d2bd6ba147dc750d951bcd.1167539318.git.spearce@spearce.org>
We tend to use the nice constant GIT_DIR_ENVIRONMENT when we
are referring to the "GIT_DIR" constant, but git.c didn't do
so. Now it does.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
git.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/git.c b/git.c
index ec897dc..c82ca45 100644
--- a/git.c
+++ b/git.c
@@ -63,14 +63,14 @@ static int handle_options(const char*** argv, int* argc)
fprintf(stderr, "No directory given for --git-dir.\n" );
usage(git_usage_string);
}
- setenv("GIT_DIR", (*argv)[1], 1);
+ setenv(GIT_DIR_ENVIRONMENT, (*argv)[1], 1);
(*argv)++;
(*argc)--;
} else if (!strncmp(cmd, "--git-dir=", 10)) {
- setenv("GIT_DIR", cmd + 10, 1);
+ setenv(GIT_DIR_ENVIRONMENT, cmd + 10, 1);
} else if (!strcmp(cmd, "--bare")) {
static char git_dir[PATH_MAX+1];
- setenv("GIT_DIR", getcwd(git_dir, sizeof(git_dir)), 1);
+ setenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir, sizeof(git_dir)), 1);
} else {
fprintf(stderr, "Unknown option: %s\n", cmd);
usage(git_usage_string);
--
1.5.0.rc0.g6bb1
^ permalink raw reply related
* [PATCH 1/4] Use PATH_MAX constant for --bare.
From: Shawn O. Pearce @ 2006-12-31 4:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
For easier portability we prefer PATH_MAX over seemingly random
constants like 1024. Make it so.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
git.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/git.c b/git.c
index 50ebd86..ec897dc 100644
--- a/git.c
+++ b/git.c
@@ -69,8 +69,8 @@ static int handle_options(const char*** argv, int* argc)
} else if (!strncmp(cmd, "--git-dir=", 10)) {
setenv("GIT_DIR", cmd + 10, 1);
} else if (!strcmp(cmd, "--bare")) {
- static char git_dir[1024];
- setenv("GIT_DIR", getcwd(git_dir, 1024), 1);
+ static char git_dir[PATH_MAX+1];
+ setenv("GIT_DIR", getcwd(git_dir, sizeof(git_dir)), 1);
} else {
fprintf(stderr, "Unknown option: %s\n", cmd);
usage(git_usage_string);
--
1.5.0.rc0.g6bb1
^ permalink raw reply related
* [PATCH 2/2] Update packedGit config option documentation.
From: Shawn O. Pearce @ 2006-12-31 3:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <298445c23601229930799536790f548b0572a92b.1167534738.git.spearce@spearce.org>
Corrected minor typos and documented the new k/m/g suffix for
core.packedGitWindowSize and core.packedGitLimit.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
For sp/mmap. Replaces my prior patch in the same area.
Documentation/config.txt | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index d71653d..8f41fcf 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -123,21 +123,25 @@ core.packedGitWindowSize::
single mapping operation. Larger window sizes may allow
your system to process a smaller number of large pack files
more quickly. Smaller window sizes will negatively affect
- performance due to increased calls to the opreating system's
+ performance due to increased calls to the operating system's
memory manager, but may improve performance when accessing
a large number of large pack files. Default is 32 MiB,
which should be reasonable for all users/operating systems.
You probably do not need to adjust this value.
+ Common unit suffixes of 'k', 'm', or 'g' are supported.
+
core.packedGitLimit::
Maximum number of bytes to map simultaneously into memory
from pack files. If Git needs to access more than this many
bytes at once to complete an operation it will unmap existing
regions to reclaim virtual address space within the process.
Default is 256 MiB, which should be reasonable for all
- users/operating systems, except on largest Git projects.
+ users/operating systems, except on the largest projects.
You probably do not need to adjust this value.
+ Common unit suffixes of 'k', 'm', or 'g' are supported.
+
alias.*::
Command aliases for the gitlink:git[1] command wrapper - e.g.
after defining "alias.last = cat-file commit HEAD", the invocation
--
1.5.0.rc0.g6bb1
^ permalink raw reply related
* [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes.
From: Shawn O. Pearce @ 2006-12-31 3:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Sometimes its necessary to supply a value as a power of two in a
configuration parameter. In this case the user may want to use the
standard suffixes such as K, M, or G to indicate that the numerical
value should be multiplied by a constant base before being used.
Shell scripts/etc. can also benefit from this automatic option
parsing with `git repo-config --int`.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
This is a resend of the prior version of this patch. Junio
convinced me on #git that this version might be better. :-)
Meant for the top of sp/mmap, but may be useful elsewhere.
Documentation/git-repo-config.txt | 5 ++++-
config.c | 8 ++++++++
2 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-repo-config.txt b/Documentation/git-repo-config.txt
index b379ec5..c55a8ba 100644
--- a/Documentation/git-repo-config.txt
+++ b/Documentation/git-repo-config.txt
@@ -87,7 +87,10 @@ OPTIONS
git-repo-config will ensure that the output is "true" or "false"
--int::
- git-repo-config will ensure that the output is a simple decimal number
+ git-repo-config will ensure that the output is a simple
+ decimal number. An optional value suffix of 'k', 'm', or 'g'
+ in the config file will cause the value to be multiplied
+ by 1024, 1048576, or 1073741824 prior to output.
ENVIRONMENT
diff --git a/config.c b/config.c
index 2e0d5a8..83ce9e1 100644
--- a/config.c
+++ b/config.c
@@ -236,8 +236,16 @@ int git_config_int(const char *name, const char *value)
if (value && *value) {
char *end;
int val = strtol(value, &end, 0);
+ while (isspace(*end))
+ end++;
if (!*end)
return val;
+ if (!strcasecmp(end, "k"))
+ return val * 1024;
+ if (!strcasecmp(end, "m"))
+ return val * 1024 * 1024;
+ if (!strcasecmp(end, "g"))
+ return val * 1024 * 1024 * 1024;
}
die("bad config value for '%s' in %s", name, config_file_name);
}
--
1.5.0.rc0.g6bb1
^ permalink raw reply related
* Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes.
From: Shawn Pearce @ 2006-12-31 3:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk608hiwq.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> wrote:
> Shawn Pearce <spearce@spearce.org> writes:
>
> >> Also can we fix the definition of core.packedGitWindowSize to be
> >> independent of the page size on a particular platform?
> >
> > What do you mean? mmap() only works in page units, and because of
> > the way the code is built the minimum size we can allow is 2 pages.
> >
> > Asking mmap() to map less than a full page in the last page of
> > a given window is silly, as that is just going to waste virtual
> > address space or cause pain for the OS, depending on how that
> > gets implemented.
>
> Ah, I misread the code. I thought the value is integer that
> states number of pages, but you are only rounding it down and
> enforcing two-page minimum.
So in your test case where you thought you entered 20 pages, what you
really had was just 2 pages. We rounded 20 bytes down to 0 pages,
then forced that to a minimum of 2 pages. :-)
--
Shawn.
^ permalink raw reply
* Re: [PATCH 1/2] Teach Git how to parse standard power of 2 suffixes.
From: Junio C Hamano @ 2006-12-31 2:57 UTC (permalink / raw)
To: Shawn Pearce; +Cc: git
In-Reply-To: <20061231023809.GC5082@spearce.org>
Shawn Pearce <spearce@spearce.org> writes:
>> Also can we fix the definition of core.packedGitWindowSize to be
>> independent of the page size on a particular platform?
>
> What do you mean? mmap() only works in page units, and because of
> the way the code is built the minimum size we can allow is 2 pages.
>
> Asking mmap() to map less than a full page in the last page of
> a given window is silly, as that is just going to waste virtual
> address space or cause pain for the OS, depending on how that
> gets implemented.
Ah, I misread the code. I thought the value is integer that
states number of pages, but you are only rounding it down and
enforcing two-page minimum.
Sorry for the noise.
^ permalink raw reply
* [PATCH 3/3] Use /dev/null for update hook stdin.
From: Shawn O. Pearce @ 2006-12-31 2:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <a0aecffe21074288c911c396f92901bfb558d591.1167533707.git.spearce@spearce.org>
Currently the update hook invoked by receive-pack has its stdin
connected to the pushing client. The hook shouldn't attempt to
read from this stream, and doing so may consume data that was
meant for receive-pack. Instead we should give the update hook
/dev/null as its stdin, ensuring that it always receives EOF and
doesn't disrupt the protocol if it attempts to read any data.
The post-update hook is similar, as it gets invoked with /dev/null
on stdin to prevent the hook from reading data from the client.
Previously we had invoked it with stdout also connected to /dev/null,
throwing away anything on stdout, to prevent client protocol errors.
Instead we should redirect stdout to stderr, like we do with the
update hook.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
receive-pack.c | 6 ++++--
run-command.c | 6 +++---
run-command.h | 2 +-
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/receive-pack.c b/receive-pack.c
index 64289e9..cf83109 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -73,7 +73,8 @@ static int run_update_hook(const char *refname,
if (access(update_hook, X_OK) < 0)
return 0;
- code = run_command_opt(RUN_COMMAND_STDOUT_TO_STDERR,
+ code = run_command_opt(RUN_COMMAND_NO_STDIN
+ | RUN_COMMAND_STDOUT_TO_STDERR,
update_hook, refname, old_hex, new_hex, NULL);
switch (code) {
case 0:
@@ -188,7 +189,8 @@ static void run_update_post_hook(struct command *cmd)
argc++;
}
argv[argc] = NULL;
- run_command_v_opt(argv, RUN_COMMAND_NO_STDIO);
+ run_command_v_opt(argv, RUN_COMMAND_NO_STDIN
+ | RUN_COMMAND_STDOUT_TO_STDERR);
}
/*
diff --git a/run-command.c b/run-command.c
index 7e4ca43..cfbad74 100644
--- a/run-command.c
+++ b/run-command.c
@@ -9,12 +9,12 @@ int run_command_v_opt(const char **argv, int flags)
if (pid < 0)
return -ERR_RUN_COMMAND_FORK;
if (!pid) {
- if (flags & RUN_COMMAND_NO_STDIO) {
+ if (flags & RUN_COMMAND_NO_STDIN) {
int fd = open("/dev/null", O_RDWR);
dup2(fd, 0);
- dup2(fd, 1);
close(fd);
- } else if (flags & RUN_COMMAND_STDOUT_TO_STDERR)
+ }
+ if (flags & RUN_COMMAND_STDOUT_TO_STDERR)
dup2(2, 1);
if (flags & RUN_GIT_CMD) {
execv_git_cmd(argv);
diff --git a/run-command.h b/run-command.h
index 8156eac..59c4476 100644
--- a/run-command.h
+++ b/run-command.h
@@ -11,7 +11,7 @@ enum {
ERR_RUN_COMMAND_WAITPID_NOEXIT,
};
-#define RUN_COMMAND_NO_STDIO 1
+#define RUN_COMMAND_NO_STDIN 1
#define RUN_GIT_CMD 2 /*If this is to be git sub-command */
#define RUN_COMMAND_STDOUT_TO_STDERR 4
int run_command_v_opt(const char **argv, int opt);
--
1.5.0.rc0.g6bb1
^ permalink raw reply related
* [PATCH 2/3] Redirect update hook stdout to stderr.
From: Shawn O. Pearce @ 2006-12-31 2:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <a0aecffe21074288c911c396f92901bfb558d591.1167533707.git.spearce@spearce.org>
If an update hook outputs to stdout then that output will be sent
back over the wire to the push client as though it were part of
the git protocol. This tends to cause protocol errors on the
client end of the connection, as the hook output is not expected
in that context. Most hook developers work around this by making
sure their hook outputs everything to stderr.
But hooks shouldn't need to perform such special behavior. Instead
we can just dup stderr to stdout prior to invoking the update hook.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
receive-pack.c | 3 ++-
run-command.c | 32 ++++++++++++++++++++++++++------
run-command.h | 2 ++
3 files changed, 30 insertions(+), 7 deletions(-)
diff --git a/receive-pack.c b/receive-pack.c
index af05a61..64289e9 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -73,7 +73,8 @@ static int run_update_hook(const char *refname,
if (access(update_hook, X_OK) < 0)
return 0;
- code = run_command(update_hook, refname, old_hex, new_hex, NULL);
+ code = run_command_opt(RUN_COMMAND_STDOUT_TO_STDERR,
+ update_hook, refname, old_hex, new_hex, NULL);
switch (code) {
case 0:
return 0;
diff --git a/run-command.c b/run-command.c
index d0330c3..7e4ca43 100644
--- a/run-command.c
+++ b/run-command.c
@@ -14,7 +14,8 @@ int run_command_v_opt(const char **argv, int flags)
dup2(fd, 0);
dup2(fd, 1);
close(fd);
- }
+ } else if (flags & RUN_COMMAND_STDOUT_TO_STDERR)
+ dup2(2, 1);
if (flags & RUN_GIT_CMD) {
execv_git_cmd(argv);
} else {
@@ -51,14 +52,12 @@ int run_command_v(const char **argv)
return run_command_v_opt(argv, 0);
}
-int run_command(const char *cmd, ...)
+static int run_command_va_opt(int opt, const char *cmd, va_list param)
{
int argc;
const char *argv[MAX_RUN_COMMAND_ARGS];
const char *arg;
- va_list param;
- va_start(param, cmd);
argv[0] = (char*) cmd;
argc = 1;
while (argc < MAX_RUN_COMMAND_ARGS) {
@@ -66,8 +65,29 @@ int run_command(const char *cmd, ...)
if (!arg)
break;
}
- va_end(param);
if (MAX_RUN_COMMAND_ARGS <= argc)
return error("too many args to run %s", cmd);
- return run_command_v_opt(argv, 0);
+ return run_command_v_opt(argv, opt);
+}
+
+int run_command_opt(int opt, const char *cmd, ...)
+{
+ va_list params;
+ int r;
+
+ va_start(params, cmd);
+ r = run_command_va_opt(opt, cmd, params);
+ va_end(params);
+ return r;
+}
+
+int run_command(const char *cmd, ...)
+{
+ va_list params;
+ int r;
+
+ va_start(params, cmd);
+ r = run_command_va_opt(0, cmd, params);
+ va_end(params);
+ return r;
}
diff --git a/run-command.h b/run-command.h
index 82a0861..8156eac 100644
--- a/run-command.h
+++ b/run-command.h
@@ -13,8 +13,10 @@ enum {
#define RUN_COMMAND_NO_STDIO 1
#define RUN_GIT_CMD 2 /*If this is to be git sub-command */
+#define RUN_COMMAND_STDOUT_TO_STDERR 4
int run_command_v_opt(const char **argv, int opt);
int run_command_v(const char **argv);
+int run_command_opt(int opt, const char *cmd, ...);
int run_command(const char *cmd, ...);
#endif
--
1.5.0.rc0.g6bb1
^ 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