* Re: [PATCH] send-pack: respect '+' on wildcard refspecs
From: Dan McGee @ 2007-10-19 12:10 UTC (permalink / raw)
To: Jeff King; +Cc: Shawn O. Pearce, git
In-Reply-To: <20071019090400.GA8944@coredump.intra.peff.net>
On 10/19/07, Jeff King <peff@peff.net> wrote:
> When matching source and destination refs, we were failing
> to pull the 'force' parameter from wildcard respects (but
> not explicit ones) and attach it to the ref struct.
>
> This adds a test for explicit and wildcard refspecs; the
> latter fails without this patch.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> remote.c | 2 ++
> t/t5400-send-pack.sh | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 50 insertions(+), 0 deletions(-)
>
> diff --git a/remote.c b/remote.c
> index b20e2be..170015a 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -762,6 +762,8 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
> hashcpy(dst_peer->new_sha1, src->new_sha1);
> }
> dst_peer->peer_ref = src;
> + if (pat)
> + dst_peer->force = pat->force;
> free_name:
> free(dst_name);
> }
> diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
> index 57c6397..2d0c07f 100755
> --- a/t/t5400-send-pack.sh
> +++ b/t/t5400-send-pack.sh
> @@ -123,4 +123,52 @@ test_expect_success \
> git-branch -a >branches && ! grep -q origin/master branches
> '
>
> +rewound_push_setup() {
> + rm -rf parent child &&
> + mkdir parent && cd parent &&
> + git-init && echo one >file && git-add file && git-commit -m one &&
> + echo two >file && git-commit -a -m two &&
> + cd .. &&
> + git-clone parent child && cd child && git-reset --hard HEAD^
> +}
> +
> +rewound_push_succeeded() {
> + cmp ../parent/.git/refs/heads/master .git/refs/heads/master
> +}
> +
> +rewound_push_failed() {
> + if rewound_push_succeeded
> + then
> + false
> + else
> + true
> + fi
> +}
> +
> +test_expect_success \
> + 'pushing explicit refspecs respects forcing' '
> + rewound_push_setup &&
> + if git-send-pack ../parent/.git refs/heads/master:refs/heads/master
> + then
> + false
> + else
> + true
> + fi && rewound_push_failed &&
> + git-send-pack ../parent/.git +refs/heads/master:refs/heads/master &&
> + rewound_push_succeeded
> +'
> +
> +test_expect_success \
> + 'pushing wildcard refspecs respects forcing' '
> + rewound_push_setup &&
> + if git-send-pack ../parent/.git refs/heads/*:refs/heads/*
> + then
> + false
> + else
> + true
> + fi && rewound_push_failed &&
> + git-send-pack ../parent/.git +refs/heads/*:refs/heads/* &&
> + rewound_push_succeeded
> +'
> +
> test_done
> --
> 1.5.3.4.1254.gc1ca9-dirty
>
Hmm. For some reason this passes with your test case, but not with my
original bash test script[1]. Did you try it with this?
-Dan
[1] http://www.toofishes.net/uploads/
^ permalink raw reply
* Re: git on afs
From: Todd T. Fries @ 2007-10-19 12:19 UTC (permalink / raw)
To: git; +Cc: Shawn O. Pearce, Linus Torvalds, Brandon Casey
In-Reply-To: <20071019060624.GK14735@spearce.org>
[-- Attachment #1: Type: text/plain, Size: 2631 bytes --]
Shawn,
If DT_UNKNOWN exists, then we have to do a stat() of some form to
find out the right type.
This is difficult to fix properly to avoid the extra stat() since
inside the switch logic we do the recursion, but we might have
avoided it earlier because of the exclusion.
I'll send a separate diff for an updated link() vs rename() diff.
I've attached an updated diff that should address concerns of everyone
who gave me feedback on my dir.c changes.
Better?
Thanks,
On Friday 19 October 2007 01:06:24 Shawn O. Pearce wrote:
> Linus Torvalds <torvalds@linux-foundation.org> wrote:
> > On Thu, 18 Oct 2007, Todd T. Fries wrote:
> > > 2) git presumes that DTYPE(de) != DT_DIR .. means the dirent is not a
> > > dir this is not true for afs
> >
> > That's a major bug, and has nothing to do with AFS. Oops.
> >
> > If you look just a bit lower, you'll see that just a few lines down, git
> > handles DT_UNKNOWN correctly, and just does a lstat() on it as required.
> > I guess that logic should be moved up, or alternatively the exclude logic
> > should be moved down.
> >
> > Your patch looks ok, but at the same time, I don't think it's really the
> > right thing to do, since it now does that lstat() twice.
>
> What about this instead? It avoids the double lstat() of Todd's
> original patch but seems like it would fix the issue here. Or did
> I misunderstand the problem?
[..]
> diff --git a/dir.c b/dir.c
> index eb6c3ab..d2597ff 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -487,9 +487,10 @@ static int read_directory_recursive(struct dir_struct
> *dir, const char *path, co && in_pathspec(fullname, baselen + len,
> simplify))
> dir_add_ignored(dir, fullname, baselen + len);
> if (exclude != dir->show_ignored) {
> - if (!dir->show_ignored || DTYPE(de) != DT_DIR) {
> + if (!dir->show_ignored)
> + continue;
> + else if (DTYPE(de) != DT_DIR && DTYPE(de) != DT_UNKNOWN)
> continue;
> - }
> }
>
> switch (DTYPE(de)) {
> --
> 1.5.3.4.1249.g895be
--
Todd Fries .. todd@fries.net
_____________________________________________
| \ 1.636.410.0632 (voice)
| Free Daemon Consulting \ 1.405.227.9094 (voice)
| http://FreeDaemonConsulting.com \ 1.866.792.3418 (FAX)
| "..in support of free software solutions." \ 250797 (FWD)
| \
\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
37E7 D3EB 74D0 8D66 A68D B866 0326 204E 3F42 004A
http://todd.fries.net/pgp.txt
[-- Attachment #2: 0001-If-readdir-returns-a-DTYPE-of-DT_UNKNOWN-this-w.patch --]
[-- Type: text/x-diff, Size: 1848 bytes --]
From f7bca472645db70b52824e1be827ba99195198d2 Mon Sep 17 00:00:00 2001
From: Todd T. Fries <todd@fries.net>
Date: Fri, 19 Oct 2007 06:26:49 -0500
Subject: [PATCH] If readdir() returns a DTYPE() of DT_UNKNOWN, this warrants further
investigation, i.e. a stat() or lstat(). We have been presuming
anything not DT_DIR is not a dir, which is not the case for
all filesystems (e.g. afs).
Do the lstat() once, and use the results twice.
Signed-off-by: Todd T. Fries <todd@fries.net>
---
dir.c | 16 +++++++++++-----
1 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/dir.c b/dir.c
index eb6c3ab..0ed4739 100644
--- a/dir.c
+++ b/dir.c
@@ -468,6 +468,7 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
while ((de = readdir(fdir)) != NULL) {
int len;
int exclude;
+ struct stat st;
if ((de->d_name[0] == '.') &&
(de->d_name[1] == 0 ||
@@ -486,19 +487,24 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co
if (exclude && dir->collect_ignored
&& in_pathspec(fullname, baselen + len, simplify))
dir_add_ignored(dir, fullname, baselen + len);
+ if (DTYPE(de) == DT_UNKNOWN) {
+ if (lstat(fullname, &st))
+ continue;
+ }
if (exclude != dir->show_ignored) {
- if (!dir->show_ignored || DTYPE(de) != DT_DIR) {
+ if (!dir->show_ignored))
continue;
- }
+ if (DTYPE(de) == DT_UNKNOWN && !S_ISDIR(st.st_mode))
+ continue;
+ else
+ if (DTYPE(de) != DT_DIR)
+ continue;
}
switch (DTYPE(de)) {
- struct stat st;
default:
continue;
case DT_UNKNOWN:
- if (lstat(fullname, &st))
- continue;
if (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode))
break;
if (!S_ISDIR(st.st_mode))
--
1.5.2.5
^ permalink raw reply related
* Re: [PATCH] send-pack: respect '+' on wildcard refspecs
From: Jeff King @ 2007-10-19 12:27 UTC (permalink / raw)
To: Dan McGee; +Cc: Shawn O. Pearce, git
In-Reply-To: <449c10960710190510y3af3ffa2ydb9ae4a01b5d480c@mail.gmail.com>
On Fri, Oct 19, 2007 at 07:10:42AM -0500, Dan McGee wrote:
> Hmm. For some reason this passes with your test case, but not with my
> original bash test script[1]. Did you try it with this?
>
> [1] http://www.toofishes.net/uploads/
[please trim quoted text; I had a hard time finding your message amidst
the patch]
I didn't try it until you sent your message, but your test seems to work
fine for me. My patch is on top of 'next', which is what I usually run.
I haven't looked into 'master' (I assumed since the bug was reproducible
in both, it would be the same in both, but that is perhaps not the
case).
-Peff
^ permalink raw reply
* Re: git on afs
From: Todd T. Fries @ 2007-10-19 12:42 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Brandon Casey
In-Reply-To: <20071019054814.GJ14735@spearce.org>
[-- Attachment #1: Type: text/plain, Size: 5778 bytes --]
You're the second one to point out that I should check the errno of link() on
afs.
It should not matter, but I'm using arla's afs client on OpenBSD; the errno
is 17 (EEXIST), the very errno that bypasses the coda hack's rename():
8484 git-index-pack CALL mkdir(0xcfbdc750,0x1ff)
8484 git-index-pack NAMI ".git/objects/pack"
8484 git-index-pack RET mkdir -1 errno 17 File exists
8484 git-index-pack CALL link(0x829e7040,0xcfbdc750)
8484 git-index-pack NAMI ".git/objects/tmp_pack_BU8484"
8484 git-index-pack
NAMI ".git/objects/pack/pack-db66c6d25d6e0044093252434b4aa2a7d67e386a.pack"
8484 git-index-pack RET link -1 errno 17 File exists
.. and does not go on to do a rename(), which based on the code makes sense.
I can assure you that the 2nd argument to link does not exist ;-)
So, I've made the following change and it seems to fix the problem, much
cleaner IMHO, and get the following change in behavior:
5933 git-index-pack CALL mkdir(0xcfbc0070,0x1ff)
5933 git-index-pack NAMI ".git/objects/pack"
5933 git-index-pack RET mkdir -1 errno 17 File exists
5933 git-index-pack CALL link(0x860c0040,0xcfbc0070)
5933 git-index-pack NAMI ".git/objects/tmp_pack_EV5933"
5933 git-index-pack
NAMI ".git/objects/pack/pack-db66c6d25d6e0044093252434b4aa2a7d67e386a.pack"
5933 git-index-pack RET link -1 errno 17 File exists
[..]
5933 git-index-pack CALL rename(0x860c0040,0xcfbc0070)
5933 git-index-pack NAMI ".git/objects/tmp_pack_EV5933"
5933 git-index-pack
NAMI ".git/objects/pack/pack-db66c6d25d6e0044093252434b4aa2a7d67e386a.pack"
5933 git-index-pack RET rename 0
The only downside is that either on coda or if the file already exists, it
will try a spurrous rename(), in which case it will fail with EEXIST again,
so I hope this is not a big negative.
If this is ok, an appropriate commit message might be something like:
AFS inconveniently returns EEXIST from link() to say it
doesn't like cross directory link()'s. Do a little
extra work to fix this by ignoring EEXIST and trying
a rename() anyway.
Thanks,
On Friday 19 October 2007 00:48:14 Shawn O. Pearce wrote:
> There's two different issues here so I'm going to split the thread
> into two to simplify the discussion. Well, for me anyway. ;-)
>
> "Todd T. Fries" <todd@fries.net> wrote:
> > 1) git presumes that link() works fine across subdirs; in afs land,
> > hardlinks do not succeed ever
>
> ...
>
> > diff --git a/sha1_file.c b/sha1_file.c
> > index 83a06a7..1b93322 100644
> > --- a/sha1_file.c
> > +++ b/sha1_file.c
> > @@ -1961,7 +1961,7 @@ static int link_temp_to_file(const char *tmpfile,
> > const char *filename) int ret;
> > char *dir;
> >
> > - if (!link(tmpfile, filename))
> > + if (!rename(tmpfile, filename))
> > return 0;
> >
> > /*
> > @@ -1980,7 +1980,7 @@ static int link_temp_to_file(const char *tmpfile,
> > const char *filename) return -2;
> > }
> > *dir = '/';
> > - if (!link(tmpfile, filename))
> > + if (!rename(tmpfile, filename))
> > return 0;
> > ret = errno;
> > }
>
> These cases should already be handled lower, see l.1997-2012 of
> sha1_file.c:
>
> /*
> * Coda hack - coda doesn't like cross-directory links,
> * so we fall back to a rename, which will mean that it
> * won't be able to check collisions, but that's not a
> * big deal.
> *
> * The same holds for FAT formatted media.
> *
> * When this succeeds, we just return 0. We have nothing
> * left to unlink.
> */
> if (ret && ret != EEXIST) {
> if (!rename(tmpfile, filename))
>
> > Brandon Casey <casey@nrlssc.navy.mil> wrote:
> >
> > On Thu, 18 Oct 2007, Todd T. Fries wrote:
> > > link() returns -1 errno 17 File exists on afs.
> > >
> > > To further muddy the waters, linking within the same dir is ok,
> > > linking outside the same dir is not:
> > >
> > > todd@ispdesk/p6 ~/tmp=A661$ mkdir dir
> > > todd@ispdesk/p6 ~/tmp=A662$ touch a
> > > todd@ispdesk/p6 ~/tmp=A663$ ln a b
> > > todd@ispdesk/p6 ~/tmp=A664$ ls -l a b
> > > -rw-r--r-- 2 4 wheel 0 Oct 18 17:09 a
> > > -rw-r--r-- 2 4 wheel 0 Oct 18 17:09 b
> > > todd@ispdesk/p6 ~/tmp=A665$ ls -li a b
> > > 2068032 -rw-r--r-- 2 4 wheel 0 Oct 18 17:09 a
> > > 2068032 -rw-r--r-- 2 4 wheel 0 Oct 18 17:09 b
> > > todd@ispdesk/p6 ~/tmp=A666$ ln a dir/b
> > > ln: dir/b: File exists
> > > todd@ispdesk/p6 ~/tmp=A667$ echo $?
> >
> > That error is just bogus on afs. If it returned a sane
> > error, things would just work.
> >
> > But, looks like afs only supports linking within the same
> > directory: http://www.angelfire.com/hi/plutonic/afs-faq.html
>
> So according to that error message from "ln" we really should have
> fallen into that Coda hack I just mentioned. Did we instead get
> a different errno here and not use that hack?
>
>
> We probably could just use rename as you do above but then we would
> want to remove the unlink(tmpfile) call on l.2013 in sha1_file.c.
> Otherwise we're always incurring a syscall for no reason. I'm not
> against a change here, I just want to make sure we make the right
> change for AFS. :-)
--
Todd Fries .. todd@fries.net
_____________________________________________
| \ 1.636.410.0632 (voice)
| Free Daemon Consulting \ 1.405.227.9094 (voice)
| http://FreeDaemonConsulting.com \ 1.866.792.3418 (FAX)
| "..in support of free software solutions." \ 250797 (FWD)
| \
\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
37E7 D3EB 74D0 8D66 A68D B866 0326 204E 3F42 004A
http://todd.fries.net/pgp.txt
[-- Attachment #2: 0002-AFS-inconveniently-returns-EEXIST-from-link-to-say.patch --]
[-- Type: text/x-diff, Size: 1125 bytes --]
From e3fe1eae139dccb9738cf0c8f6818136be96657b Mon Sep 17 00:00:00 2001
From: Todd T. Fries <todd@fries.net>
Date: Fri, 19 Oct 2007 07:38:16 -0500
Subject: [PATCH] AFS inconveniently returns EEXIST from link() to say it
doesn't like cross directory link()'s. Do a little
extra work to fix this by ignoring EEXIST and trying
a rename() anyway.
Signed-off-by: Todd T. Fries <todd@fries.net>
---
sha1_file.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 83a06a7..9c7d74e 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2004,8 +2004,13 @@ int move_temp_to_file(const char *tmpfile, const char *filename)
*
* When this succeeds, we just return 0. We have nothing
* left to unlink.
+ *
+ * AFS hack - afs is similar to coda, but inconveniently
+ * set errno to EEXIST, so call rename() if the link()
+ * above fails unconditionally. Small bit of extra work
+ * so afs functions properly.
*/
- if (ret && ret != EEXIST) {
+ if (ret) {
if (!rename(tmpfile, filename))
return 0;
ret = errno;
--
1.5.2.5
^ permalink raw reply related
* Re: Subversion developer: svn is for dumb people
From: Andy Parkins @ 2007-10-19 12:10 UTC (permalink / raw)
To: git
In-Reply-To: <47176CE0.7030609@midwinter.com>
Steven Grimm wrote:
> find code reviews *harder* in a centralized system because you end up
I think I'd argue that git actually reduces the problems of patchbombs
(which is a code-review problem).
Those who are going to patchbomb will do it whatever their VCS choice (and
I'm not entirely convinced that patchbombs are as bad as is made out in
that article). With a centralised VCS the patch almost has to be one giant
diff - very hard to read and review. Git, on the other hand, allows the
patchbomber to work alone crafting not only beautiful code, but a beautiful
patch series. The bomb can be easy to review, easy to reject, and easy to
modify. It becomes far more likely that the code can be used in some way.
I guarantee that the only reason I ever contributed anything to git itself
was because the bureaucracy line is so low - and it is low because of git.
Key features for a DVCS from a newbie point of view:
- Making a fool of yourself can be done in private rather than in public.
I have loads of branches in my git repository that I will never submit,
because they were the beginnings of ideas that didn't pan out (and I'm
lazy)
- You don't need _any_ permission to fork/branch.
- You can work away on your feature without worrying too much that time
has passed and the main project has moved on and you aren't keeping up
to date. I've got a set of patches for git (the REF_PATHS stuff) that I
started a year ago. I recently rebased it and brought it up to date.
It's still not in mainline, but I'm not panicking, nor do I have to
abandon it because it's so easy to catch up whenever I want to.
- I can work on two features simultaneously. If I have two ideas - crazy
and safe, both independent, I can work on both without having to mix
them for the day when I submit. This makes it a lot easier to keep them
on the go, and a lot easier for the maintainer to decide what goes in and
what doesn't.
- The ability to craft a log messages that are kept locally and easily
edited before submission goes a long way to helping cross the
brave-enough-to-submit barrier. Presenting new code as an unknown
developer on a project is (I think) quite a stressful thing. Git let's
you feel as prepared as you want before you take that leap into having
your code ripped apart.
In short: those pro-centralised arguments are nonsense. Even if they
weren't the advantages of DVCS heavily outweigh the advantages of CVCS.
Andy
--
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com
^ permalink raw reply
* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Johannes Sixt @ 2007-10-19 12:31 UTC (permalink / raw)
To: Theodore Tso
Cc: Santi Béjar, Shawn O. Pearce, David Symonds, Jeff King, git
In-Reply-To: <20071019113822.GB16726@thunk.org>
Theodore Tso schrieb:
> ==> git://repo.or.cz/git/spearce.git
> * branch gitk -> spearce/gitk (new)
> * branch maint -> spearce/maint 1aa3d01..e7187e4
> * branch master -> spearce/master de61e42..7840ce6
> * branch next -> spearce/next 895be02..2fe5433
> + branch pu -> spearce/pu 89fa332...1e4c517
> * branch todo -> spearce/todo (new)
> As far as the padding, it would be a pain to figure out how to make
> the right hand column be padded so that it starts 3 spaces after the
> longest " * branch foo -> bar" line, but that would look the best.
But this way it wouldn't be difficult at all:
==> git://repo.or.cz/git/spearce.git
* (new) gitk -> spearce/gitk
* 1aa3d01..e7187e4 maint -> spearce/maint
* de61e42..7840ce6 master -> spearce/master
* 895be02..2fe5433 next -> spearce/next
+ 89fa332...1e4c517 pu -> spearce/pu
* (new) todo -> spearce/todo
(I don't know where to put the label 'branch'.)
BTW, I like the ID ranges, too, and have used the information
occasionally.
-- Hannes
^ permalink raw reply
* [PATCH] allow git to use the PATH for finding subcommands and help docs
From: Scott Parish @ 2007-10-19 13:04 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git
In-Reply-To: <47185DAF.7060809@viscovery.net>
I have a situation where software for a distribution is installed
into a fake "prefix" and then moved to one of several potential
places to be used by users. Given that the final location isn't
static, i can't depend on builtin_exec_path. I'd really like users
to be able to get started with git as easily as possible. With the
current setup, they would have to create and maintain either an
GIT_EXEC_PATH or an alias for including --exec-path, as well as a
MANPATH and PERL5LIB. This seem like an unnessisary burden.
I'd like to make it so that git works equally well when it is ran
via an absolute path (already partially works), relative path, or
from the PATH. (in saying "equally well" i'm including perl commands
and help commands)
To do this i've had to make the following changes:
+ check PATH for the location of git
+ the checking of argv[0] was restricted to absolute paths; remove
that restriction so it also works when called with a relative
path (eg ../../otheruser/usr/bin/git)
+ try to guess and set the env for the typical relative locations for
MANPATH and PERL5LIB based off exec_path
Signed-off-by: Scott R Parish <srp@srparish.net>
---
exec_cmd.c | 50 ++++++++++++++++++++++++++++++++++++++-
git.c | 76 +++++++++++++++++++++++++++++++++++++++++------------------
2 files changed, 102 insertions(+), 24 deletions(-)
diff --git a/exec_cmd.c b/exec_cmd.c
index 9b74ed2..c6ecca9 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -13,19 +13,67 @@ void git_set_exec_path(const char *exec_path)
}
+/* Return the first path in PATH that git is found in or NULL if not found */
+char *git_path_from_env(void)
+{
+ const char *env_paths = getenv("PATH");
+ const char *git = "/git";
+ int git_len = strlen(git);
+ char *paths, *path, *colon, *git_path;
+ int path_len;
+ struct stat st;
+
+ if (!env_paths)
+ return NULL;
+
+ path_len = strlen(env_paths);
+ path = paths = xmalloc(path_len + 1);
+ memcpy(paths, env_paths, path_len + 1);
+
+ while ((char *)1 != path) {
+ if ((colon = strchr(path, ':')))
+ *colon = 0;
+
+ path_len = strlen(path);
+ git_path = xmalloc(path_len + git_len + 1);
+ memcpy(git_path, path, path_len);
+ memcpy(git_path + path_len, git, git_len + 1);
+
+ if (!stat(git_path, &st)) { /* found */
+ free(paths);
+ git_path[path_len] = 0;
+ return git_path;
+ }
+
+ free(git_path);
+ path = colon + 1;
+ }
+
+ free(paths);
+ return NULL;
+}
+
+
/* Returns the highest-priority, location to look for git programs. */
const char *git_exec_path(void)
{
- const char *env;
+ const char *env, *path;
if (current_exec_path)
return current_exec_path;
env = getenv(EXEC_PATH_ENVIRONMENT);
if (env && *env) {
+ current_exec_path = env;
return env;
}
+ if ((path = git_path_from_env())) {
+ current_exec_path = path;
+ return path;
+ }
+
+ current_exec_path = builtin_exec_path;
return builtin_exec_path;
}
diff --git a/git.c b/git.c
index 9eaca1d..252ee7c 100644
--- a/git.c
+++ b/git.c
@@ -6,26 +6,56 @@
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 void prepend_to_path(const char *dir, int len)
+static void prepend_to_env(const char *env, const char *basedir,
+ const char *subdir, const char *env_default)
{
- const char *old_path = getenv("PATH");
- char *path;
- int path_len = len;
-
- if (!old_path)
- old_path = "/usr/local/bin:/usr/bin:/bin";
-
- path_len = len + strlen(old_path) + 1;
-
- path = xmalloc(path_len + 1);
+ const char *old = getenv(env);
+ int basedir_len = strlen(basedir);
+ int subdir_len = strlen(subdir);
+ char *new;
+ int old_len;
+
+ if (!old)
+ old = env_default;
+
+ old_len = strlen(old);
+
+ new = xmalloc(basedir_len + subdir_len + old_len + 1);
+
+ memcpy(new, basedir, basedir_len);
+ memcpy(new + basedir_len, subdir, subdir_len);
+ memcpy(new + basedir_len + subdir_len, old, old_len + 1);
+
+ if (setenv(env, new, 1))
+ fprintf(stderr, "Setenv failed: %s\n", strerror(errno));
+
+ free(new);
+}
- memcpy(path, dir, len);
- path[len] = ':';
- memcpy(path + len + 1, old_path, path_len - len);
+static void prepend_to_envs(const char *dir, int len)
+{
+ char *slash;
+ char *basedir;
+
+ /* basedir is dir with "/bin" stripped off */
+ basedir = xmalloc(len + 1);
+ memcpy(basedir, dir, len + 1);
+
+ if ((slash = strrchr(basedir, '/'))) {
+ *slash = 0;
+ while (slash == basedir + --len) /* found trailing slash */
+ if ((slash = strrchr(basedir, '/')))
+ *slash = 0;
+ }
- setenv("PATH", path, 1);
+ prepend_to_env("PATH", basedir, "/bin:",
+ "/usr/local/bin:/usr/bin:/bin");
+ prepend_to_env("MANPATH", basedir, "/share/man:",
+ "/usr/local/share/man:/usr/share/man");
+ prepend_to_env("PERL5LIB", basedir, "/lib/perl5:",
+ "/usr/lib/perl5");
- free(path);
+ free(basedir);
}
static int handle_options(const char*** argv, int* argc, int* envchanged)
@@ -414,8 +444,7 @@ int main(int argc, const char **argv)
*/
if (slash) {
*slash++ = 0;
- if (*cmd == '/')
- exec_path = cmd;
+ exec_path = cmd;
cmd = slash;
}
@@ -453,14 +482,15 @@ int main(int argc, const char **argv)
/*
* We execute external git command via execv_git_cmd(),
* which looks at "--exec-path" option, GIT_EXEC_PATH
- * environment, and $(gitexecdir) in Makefile while built,
- * in this order. For scripted commands, we prepend
- * the value of the exec_path variable to the PATH.
+ * environment, PATH environment, and $(gitexecdir) in
+ * Makefile while built, in this order. For scripted
+ * commands, we prepend the value of the exec_path
+ * variable to the PATH.
*/
if (exec_path)
- prepend_to_path(exec_path, strlen(exec_path));
+ prepend_to_envs(exec_path, strlen(exec_path));
exec_path = git_exec_path();
- prepend_to_path(exec_path, strlen(exec_path));
+ prepend_to_envs(exec_path, strlen(exec_path));
while (1) {
/* See if it's an internal command */
--
1.5.3.4.206.g58ba4-dirty
^ permalink raw reply related
* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Nicolas Pitre @ 2007-10-19 13:05 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: David Symonds, Jeff King, git
In-Reply-To: <20071019073938.GN14735@spearce.org>
On Fri, 19 Oct 2007, Shawn O. Pearce wrote:
> What about this on top of Jeff's patch?
>
> $ git fetch jc
> ...
> ==> git://repo.or.cz/alt-git.git
> * tag junio-gpg-pub ......................... (new)
> * tag v1.5.0 .......................... (tag moved)
>
> $ git fetch me
> ...
> ==> git://repo.or.cz/git/spearce.git
> * branch gitk -> spearce/gitk ............... (new)
> * branch maint -> spearce/maint
> * branch master -> spearce/master
> * branch next -> spearce/next
> * branch pu -> spearce/pu ......... (forced update)
> * branch todo -> spearce/todo ............... (new)
>
> The width of the terminal is computed to produce the ... padding.
> I used a very narrow terminal to produce the above so it doesn't
> linewrap badly in email. If we cannot get the terminal width then
> we just don't produce the padding.
I like it.
I would change the '*' to a '+' for a forced update (for similarity with
the + notation in refspecs), and a '!' instead of a '-' for refused
update which might be more indicative of a refusal.
Nicolas
^ permalink raw reply
* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Nicolas Pitre @ 2007-10-19 13:15 UTC (permalink / raw)
To: Santi Béjar; +Cc: Shawn O. Pearce, David Symonds, Jeff King, git
In-Reply-To: <8aa486160710190303l4ce996daqf5c8025c857ea8@mail.gmail.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 993 bytes --]
On Fri, 19 Oct 2007, Santi Béjar wrote:
> Another possibility is with just some minor reductions from the
> current output, as:
>
> $ git fetch spearce
> ...
> >From git://repo.or.cz/git/spearce
> * spearce/gitk: fast forward to branch 'gitk'
> old..new: 0d6df4d..2b5afb7
> * spearce/maint: fast forward to branch 'maint'
> old..new: 1aa3d01..e7187e4
> * spearce/master: fast forward to branch 'master'
> old..new: de61e42..7840ce6
> * spearce/next: fast forward to branch 'next'
> old..new: 895be02..2fe5433
> * spearce/pu: forcing update to non-fast forward branch 'pu'
> old...new: 89fa332...1e4c517
>
> This way it is slightly less terse than the other proposals but not
> that cryptic and it normally fits in one line without padding. And I
> really like to see what has changed explicitly with the old..new line.
I think the advantage of having only one line of output per branch
really outweight the need for old..new notation. Do you really benefit
from it?
Nicolas
^ permalink raw reply
* Re: Subversion developer: svn is for dumb people
From: Johannes Schindelin @ 2007-10-19 13:17 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: Steven Grimm, 'git'
In-Reply-To: <20071019113447.GC4404@artemis.corp>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1726 bytes --]
Hi,
On Fri, 19 Oct 2007, Pierre Habouzit wrote:
> On Thu, Oct 18, 2007 at 02:25:36PM +0000, Steven Grimm wrote:
> > Thought folks here might get a kick out of this:
> >
> > http://blog.red-bean.com/sussman/?p=79
> >
> > Okay, my summary is slightly facetious, but that's basically the gist
> > of what he's saying: you should choose Subversion rather than a DVCS
> > because most of your users won't be smart enough to use the better
> > tool.
> >
> > I can't say he's completely wrong, especially about the 20/80% idea
> > (though I think "20%" is generous), but some of his specific arguments
> > about DVCS are on the bogus side. "Centralized systems encourage code
> > reviews," for one -- I challenge him to find a project with a more
> > pervasive and effective code-reviewing culture than the git project.
>
> Your argument is also bogus.
>
> IMNSHO, peer reviewing has nothing to do with git, svn, or $SCM. It's
> a social pattern. There are people that do it because they understand
> it's a good and necessary sound thing to do, and there are the others.
> Guess what, it has a lot to do with the 20%/80% line (that I would have
> more described as the 2/98 but well…).
I tend to disagree. Git at least _enables_ you to have the
one-committer-per-repository scheme, it even _encourages_ it to a certain
extent.
And once you go that route, it is easy to see that the committer says "I
will not let that _crap_ enter my repository." Bingo, peer review.
Compare that to a centralised repository, where more often than not, the
administrator is not even part of the developer community! It is much
easier not to feel too responsible for the code you are committing there.
Ciao,
Dscho
^ permalink raw reply
* Re: Subversion developer: svn is for dumb people
From: Pierre Habouzit @ 2007-10-19 13:19 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Steven Grimm, 'git'
In-Reply-To: <Pine.LNX.4.64.0710191514190.16728@wbgn129.biozentrum.uni-wuerzburg.de>
[-- Attachment #1: Type: text/plain, Size: 2468 bytes --]
On Fri, Oct 19, 2007 at 01:17:05PM +0000, Johannes Schindelin wrote:
> Hi,
>
> On Fri, 19 Oct 2007, Pierre Habouzit wrote:
>
> > On Thu, Oct 18, 2007 at 02:25:36PM +0000, Steven Grimm wrote:
> > > Thought folks here might get a kick out of this:
> > >
> > > http://blog.red-bean.com/sussman/?p=79
> > >
> > > Okay, my summary is slightly facetious, but that's basically the gist
> > > of what he's saying: you should choose Subversion rather than a DVCS
> > > because most of your users won't be smart enough to use the better
> > > tool.
> > >
> > > I can't say he's completely wrong, especially about the 20/80% idea
> > > (though I think "20%" is generous), but some of his specific arguments
> > > about DVCS are on the bogus side. "Centralized systems encourage code
> > > reviews," for one -- I challenge him to find a project with a more
> > > pervasive and effective code-reviewing culture than the git project.
> >
> > Your argument is also bogus.
> >
> > IMNSHO, peer reviewing has nothing to do with git, svn, or $SCM. It's
> > a social pattern. There are people that do it because they understand
> > it's a good and necessary sound thing to do, and there are the others.
> > Guess what, it has a lot to do with the 20%/80% line (that I would have
> > more described as the 2/98 but well…).
>
> I tend to disagree. Git at least _enables_ you to have the
> one-committer-per-repository scheme, it even _encourages_ it to a certain
> extent.
>
> And once you go that route, it is easy to see that the committer says "I
> will not let that _crap_ enter my repository." Bingo, peer review.
>
> Compare that to a centralised repository, where more often than not, the
> administrator is not even part of the developer community! It is much
> easier not to feel too responsible for the code you are committing there.
I agree, that's why I said that git made it easier. WHat I pretend to
be wrong is to say that a SCM will make people review code or not. Git
merely help people that want to always review code to be completely sure
some review has happened before the code is merged. But nothing in git
forbids you to use the big fat <centralized repo with everyone having
push access to it>-mode.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: git stash apply usability issues
From: Karl Hasselström @ 2007-10-19 13:27 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Johannes Sixt, Git Mailing List
In-Reply-To: <20071019013156.GU14735@spearce.org>
On 2007-10-18 21:31:56 -0400, Shawn O. Pearce wrote:
> Johannes Sixt <j.sixt@viscovery.net> wrote:
> > (2) when 'git stash apply' runs merge-recursive, it treats the
> > current state as 'ours' and the stash as 'theirs'. IMHO it should
> > be the other way round: I have stashed away changes to a binary
> > file. Then committed a different modification to it, and now want
> > to apply the stash. This results in a conflict that leaves the
> > current state in the working tree, but I had preferred that the
> > stashed binary file were in the working tree now.
> >
> > What do other git-stash users think about changing the order?
>
> The current order is the same order that git-rebase uses. I'm not
> saying its correct, just that its the same as rebase.
FWIW, StGit push works the same way. The idea being that the current
HEAD is our current state ("ours"), and the patch we're pushing is
some change we want to apply ("theirs"). I always felt that this was a
very natural order of things. But I guess the philosophy in the
"stash" case is subtly different, so maybe the change is warranted
there.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* stash clear, was Re: git: avoiding merges, rebasing
From: Johannes Schindelin @ 2007-10-19 13:37 UTC (permalink / raw)
To: Eric Blake; +Cc: bug-gnulib, Bruno Haible, git
In-Reply-To: <47023699.3080606@byu.net>
Hi,
On Tue, 2 Oct 2007, Eric Blake wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> [adding the git list]
>
> According to Bruno Haible on 10/2/2007 5:50 AM:
> > Hello Benoit,
> >
> >>> $ git stash
> >>> $ git pull
> >>> $ git stash apply
> >>> $ git stash clean ; typo!
> >>> $ git stash clear ; fatal correction to typo!
> >>>
> >>> and lost 20 modified files. Well, not really lost. Just took me a
> >>> while to
> >> I don't really see how and why you "lost 20 modified files".
> >
> > I lost modifications to 20 files. "git stash clean" moved these modifications
> > into a stash named "clean", and "git stash clear" killed it.
>
> While we're at it, I wish 'git stash clear' would take an optional
> argument that says which stash(es) to clear, rather than blindly
> clearing the entire stash.
I'd rather avoid "enhancing" stash clear. IMHO it is a little
misdesigned, making it way too easy to hang yourself.
Instead, how about writing a stash pop? "git stash pop [<stash>]". It
would literally just call git stash apply && git reflog delete. Should
not be too difficult, now that I provided "git reflog delete" ;-)
Maybe even deprecating "git stash clear", or doing away with it
altogether.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] send-pack: respect '+' on wildcard refspecs
From: Dan McGee @ 2007-10-19 13:38 UTC (permalink / raw)
To: Jeff King; +Cc: Shawn O. Pearce, git
In-Reply-To: <20071019122755.GA17002@coredump.intra.peff.net>
On 10/19/07, Jeff King <peff@peff.net> wrote:
> On Fri, Oct 19, 2007 at 07:10:42AM -0500, Dan McGee wrote:
>
> > Hmm. For some reason this passes with your test case, but not with my
> > original bash test script[1]. Did you try it with this?
> >
> > [1] http://www.toofishes.net/uploads/
>
> I didn't try it until you sent your message, but your test seems to work
> fine for me. My patch is on top of 'next', which is what I usually run.
> I haven't looked into 'master' (I assumed since the bug was reproducible
> in both, it would be the same in both, but that is perhaps not the
> case).
Still getting this error:
error: remote 'refs/heads/working' is not a strict subset of local ref
'refs/heads/working'. maybe you are not up-to-date and need to pull
first?
error: failed to push to '/tmp/testpush'
I've tried applying the patch on the following commits, and maybe I'm
smoking something but I can't get it to pass my test script.
origin(junio)/master: 58ba4f6
origin(junio)/next: fe96ee67ec5840
spearce/master: 7840ce6cb24a9d
spearce/next: 2fe5433b416f0df
Can you let me know what commit you based the patch off of? I'm at
work for the next 8 hours or so, so I can't look in to this a whole
lot until tonight.
-Dan
^ permalink raw reply
* [PATCH-resent] gitk: fix in procedure drawcommits
From: Michele Ballabio @ 2007-10-19 13:44 UTC (permalink / raw)
To: git; +Cc: Shawn O. Pearce, Paul Mackerras
In-Reply-To: <20071019052823.GI14735@spearce.org>
This patch indroduces a check before unsetting an array element.
Without this, gitk may complain with
can't unset "prevlines(...)": no such element in array
when scrolling happens.
Signed-off-by: Michele Ballabio <barra_cuda@katamail.com>
---
There's an error that seems to occur in gitk only on
mutt's imported repo, but I don't know why. This is
hopefully the right fix.
An example of this error:
can't unset "prevlines(a3b4383d69e0754346578c85ba8ff7c05bd88705)": no such element in array
can't unset "prevlines(a3b4383d69e0754346578c85ba8ff7c05bd88705)": no such element in array
while executing
"unset prevlines($lid)"
(procedure "drawcommits" line 39)
invoked from within
"drawcommits $row $endrow"
(procedure "drawfrac" line 10)
invoked from within
"drawfrac $f0 $f1"
(procedure "scrollcanv" line 3)
invoked from within
"scrollcanv .tf.histframe.csb 0.00672513 0.0087015"
gitk | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/gitk b/gitk
index 300fdce..527b716 100755
--- a/gitk
+++ b/gitk
@@ -3697,7 +3697,9 @@ proc drawcommits {row {endrow {}}} {
if {[info exists lineends($r)]} {
foreach lid $lineends($r) {
- unset prevlines($lid)
+ if {[info exists prevlines($lid)]} {
+ unset prevlines($lid)
+ }
}
}
set rowids [lindex $rowidlist $r]
--
1.5.3
^ permalink raw reply related
* Re: [PATCH] send-pack: respect '+' on wildcard refspecs
From: Jeff King @ 2007-10-19 13:43 UTC (permalink / raw)
To: Dan McGee; +Cc: Shawn O. Pearce, git
In-Reply-To: <449c10960710190638j5823b19dl903ae369965e884e@mail.gmail.com>
On Fri, Oct 19, 2007 at 08:38:06AM -0500, Dan McGee wrote:
> origin(junio)/master: 58ba4f6
> origin(junio)/next: fe96ee67ec5840
> spearce/master: 7840ce6cb24a9d
> spearce/next: 2fe5433b416f0df
>
> Can you let me know what commit you based the patch off of? I'm at
> work for the next 8 hours or so, so I can't look in to this a whole
> lot until tonight.
It is based on Shawn's next, 2fe5433b. Are you sure you're not doing
something silly like executing an older version of git that is in your
PATH?
-Peff
^ permalink raw reply
* Re: [PATCH] allow git to use the PATH for finding subcommands and help docs
From: Johannes Sixt @ 2007-10-19 13:21 UTC (permalink / raw)
To: Scott Parish; +Cc: git
In-Reply-To: <20071019130402.GD1463@srparish.net>
Scott Parish schrieb:
> I have a situation where software for a distribution is installed
> into a fake "prefix" and then moved to one of several potential
> places to be used by users. Given that the final location isn't
> static, i can't depend on builtin_exec_path. I'd really like users
> to be able to get started with git as easily as possible. With the
> current setup, they would have to create and maintain either an
> GIT_EXEC_PATH or an alias for including --exec-path, as well as a
> MANPATH and PERL5LIB. This seem like an unnessisary burden.
Interesting. How does this compare to this 2-patch-series:
http://repo.or.cz/w/git/mingw.git?a=commitdiff;h=e479ea2f911b8c70a269ba59372a4fef90f8907c
http://repo.or.cz/w/git/mingw.git?a=commitdiff;h=00a4ff4f3f8ec7e6b3ac15456f00b22b03f438ae
which I had come up with to accomplish something very similar
(on Windows). Your approach looks superior, but I hadn't gone
into depths, yet.
-- Hannes
^ permalink raw reply
* Re: git stash apply usability issues
From: David Kastrup @ 2007-10-19 13:57 UTC (permalink / raw)
To: git
In-Reply-To: <20071019132753.GA23765@diana.vm.bytemark.co.uk>
Karl Hasselström <kha@treskal.com> writes:
> On 2007-10-18 21:31:56 -0400, Shawn O. Pearce wrote:
>
>> Johannes Sixt <j.sixt@viscovery.net> wrote:
>
>> > (2) when 'git stash apply' runs merge-recursive, it treats the
>> > current state as 'ours' and the stash as 'theirs'. IMHO it should
>> > be the other way round: I have stashed away changes to a binary
>> > file. Then committed a different modification to it, and now want
>> > to apply the stash. This results in a conflict that leaves the
>> > current state in the working tree, but I had preferred that the
>> > stashed binary file were in the working tree now.
>> >
>> > What do other git-stash users think about changing the order?
>>
>> The current order is the same order that git-rebase uses. I'm not
>> saying its correct, just that its the same as rebase.
>
> FWIW, StGit push works the same way. The idea being that the current
> HEAD is our current state ("ours"), and the patch we're pushing is
> some change we want to apply ("theirs"). I always felt that this was a
> very natural order of things. But I guess the philosophy in the
> "stash" case is subtly different, so maybe the change is warranted
> there.
Well, maybe one should then just name this "current" and "separate"
instead of "ours" and "theirs".
--
David Kastrup
^ permalink raw reply
* Re: [PATCH] send-pack: respect '+' on wildcard refspecs
From: Dan McGee @ 2007-10-19 14:11 UTC (permalink / raw)
To: Jeff King; +Cc: Shawn O. Pearce, git
In-Reply-To: <20071019134339.GA21852@coredump.intra.peff.net>
Ahh, shoot. Forgot to reply to all.
On 10/19/07, Jeff King <peff@peff.net> wrote:
> It is based on Shawn's next, 2fe5433b. Are you sure you're not doing
> something silly like executing an older version of git that is in your
> PATH?
Yeah, just tried that again, definitely using the right version of
git. Before I apply your patch, both my test script and your addition
to t5400 fail. After applying your patch, my test script fails but
your addition to t5400 succeeds. Could this be something where
git-push and git-send-pack are not interacting correctly?
-Dan
^ permalink raw reply
* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Nicolas Pitre @ 2007-10-19 14:14 UTC (permalink / raw)
To: Johannes Sixt
Cc: Theodore Tso, Santi Béjar, Shawn O. Pearce, David Symonds,
Jeff King, git
In-Reply-To: <4718A3AB.7090301@viscovery.net>
On Fri, 19 Oct 2007, Johannes Sixt wrote:
> Theodore Tso schrieb:
> > ==> git://repo.or.cz/git/spearce.git
> > * branch gitk -> spearce/gitk (new)
> > * branch maint -> spearce/maint 1aa3d01..e7187e4
> > * branch master -> spearce/master de61e42..7840ce6
> > * branch next -> spearce/next 895be02..2fe5433
> > + branch pu -> spearce/pu 89fa332...1e4c517
> > * branch todo -> spearce/todo (new)
>
> > As far as the padding, it would be a pain to figure out how to make
> > the right hand column be padded so that it starts 3 spaces after the
> > longest " * branch foo -> bar" line, but that would look the best.
>
> But this way it wouldn't be difficult at all:
>
> ==> git://repo.or.cz/git/spearce.git
> * (new) gitk -> spearce/gitk
> * 1aa3d01..e7187e4 maint -> spearce/maint
> * de61e42..7840ce6 master -> spearce/master
> * 895be02..2fe5433 next -> spearce/next
> + 89fa332...1e4c517 pu -> spearce/pu
> * (new) todo -> spearce/todo
Actually I think this is the best format so far: one line per branch, no
terminal width issue (long branch names are simply wrapped), the
old..new info is there also with the single character marker to quickly
notice the type of update.
Nicolas
^ permalink raw reply
* Re: [PATCH] allow git to use the PATH for finding subcommands and help docs
From: Scott Parish @ 2007-10-19 14:18 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git
In-Reply-To: <4718AF48.9020607@viscovery.net>
On Fri, Oct 19, 2007 at 03:21:12PM +0200, Johannes Sixt wrote:
> Scott Parish schrieb:
> > I have a situation where software for a distribution is installed
> > into a fake "prefix" and then moved to one of several potential
> > places to be used by users. Given that the final location isn't
> > static, i can't depend on builtin_exec_path. I'd really like users
> > to be able to get started with git as easily as possible. With the
> > current setup, they would have to create and maintain either an
> > GIT_EXEC_PATH or an alias for including --exec-path, as well as a
> > MANPATH and PERL5LIB. This seem like an unnessisary burden.
>
> Interesting. How does this compare to this 2-patch-series:
>
> http://repo.or.cz/w/git/mingw.git?a=commitdiff;h=e479ea2f911b8c70a269ba59372a4fef90f8907c
> http://repo.or.cz/w/git/mingw.git?a=commitdiff;h=00a4ff4f3f8ec7e6b3ac15456f00b22b03f438ae
>
> which I had come up with to accomplish something very similar
> (on Windows). Your approach looks superior, but I hadn't gone
> into depths, yet.
I know very little about what's available on windows. Looking at
your code, it looks like the command isn't passed in in argv[0] and
that it contains the windows style path seperators. My code currently
assumes that PATH is a colon separated list, and that directories
are separated with '/'. How should these assumptions change for
windows?
sRp
--
Scott Parish
http://srparish.net/
^ permalink raw reply
* Re: [PATCH] allow git to use the PATH for finding subcommands and help docs
From: Johannes Schindelin @ 2007-10-19 14:27 UTC (permalink / raw)
To: Scott Parish; +Cc: Johannes Sixt, git
In-Reply-To: <20071019130402.GD1463@srparish.net>
Hi,
On Fri, 19 Oct 2007, Scott Parish wrote:
> + check PATH for the location of git
Okay, but better do it only if the current exec_path did not succeed to
find something, to stay as backwards compatible as possible.
> + the checking of argv[0] was restricted to absolute paths; remove
> that restriction so it also works when called with a relative
> path (eg ../../otheruser/usr/bin/git)
This will utterly break down when you try to do things in a subdirectory
of your project. git will cd up, and the relative path will no longer be
relative.
> + try to guess and set the env for the typical relative locations for
> MANPATH and PERL5LIB based off exec_path
Now this is ugly. At least make it a separate patch.
> +/* Return the first path in PATH that git is found in or NULL if not found */
> +char *git_path_from_env(void)
> +{
> + const char *env_paths = getenv("PATH");
> + const char *git = "/git";
> + int git_len = strlen(git);
> + char *paths, *path, *colon, *git_path;
> + int path_len;
> + struct stat st;
> +
> + if (!env_paths)
> + return NULL;
> +
> + path_len = strlen(env_paths);
> + path = paths = xmalloc(path_len + 1);
> + memcpy(paths, env_paths, path_len + 1);
> +
> + while ((char *)1 != path) {
> + if ((colon = strchr(path, ':')))
> + *colon = 0;
> +
> + path_len = strlen(path);
> + git_path = xmalloc(path_len + git_len + 1);
> + memcpy(git_path, path, path_len);
> + memcpy(git_path + path_len, git, git_len + 1);
> +
> + if (!stat(git_path, &st)) { /* found */
> + free(paths);
> + git_path[path_len] = 0;
> + return git_path;
> + }
> +
> + free(git_path);
> + path = colon + 1;
> + }
> +
> + free(paths);
> + return NULL;
> +}
I am convinced that this function will look a lot less ugly when you use
strbufs. And I'd call it "find_git_in_path()".
> /* Returns the highest-priority, location to look for git programs. */
> const char *git_exec_path(void)
> {
> - const char *env;
> + const char *env, *path;
>
> if (current_exec_path)
> return current_exec_path;
>
> env = getenv(EXEC_PATH_ENVIRONMENT);
> if (env && *env) {
> + current_exec_path = env;
> return env;
> }
>
> + if ((path = git_path_from_env())) {
> + current_exec_path = path;
> + return path;
> + }
> +
> + current_exec_path = builtin_exec_path;
> return builtin_exec_path;
> }
As I said, I'd rather have git try with builtin_exec_path first, and only
if that fails, search through the PATH, for the _current_ command.
> -static void prepend_to_path(const char *dir, int len)
> +static void prepend_to_env(const char *env, const char *basedir,
I do not like this rename. It makes things more obscure, rather than
clearing things up.
> + const char *subdir, const char *env_default)
> {
> - const char *old_path = getenv("PATH");
> - char *path;
> - int path_len = len;
> -
> - if (!old_path)
> - old_path = "/usr/local/bin:/usr/bin:/bin";
> -
> - path_len = len + strlen(old_path) + 1;
> -
> - path = xmalloc(path_len + 1);
> + const char *old = getenv(env);
> + int basedir_len = strlen(basedir);
> + int subdir_len = strlen(subdir);
> + char *new;
> + int old_len;
> +
> + if (!old)
> + old = env_default;
> +
> + old_len = strlen(old);
> +
> + new = xmalloc(basedir_len + subdir_len + old_len + 1);
> +
> + memcpy(new, basedir, basedir_len);
> + memcpy(new + basedir_len, subdir, subdir_len);
> + memcpy(new + basedir_len + subdir_len, old, old_len + 1);
> +
> + if (setenv(env, new, 1))
> + fprintf(stderr, "Setenv failed: %s\n", strerror(errno));
> +
> + free(new);
> +}
Again, this would be so much more elegant using strbufs.
>
> - memcpy(path, dir, len);
> - path[len] = ':';
> - memcpy(path + len + 1, old_path, path_len - len);
> +static void prepend_to_envs(const char *dir, int len)
> +{
> + char *slash;
> + char *basedir;
> +
> + /* basedir is dir with "/bin" stripped off */
> + basedir = xmalloc(len + 1);
> + memcpy(basedir, dir, len + 1);
> +
> + if ((slash = strrchr(basedir, '/'))) {
> + *slash = 0;
> + while (slash == basedir + --len) /* found trailing slash */
> + if ((slash = strrchr(basedir, '/')))
> + *slash = 0;
> + }
>
> - setenv("PATH", path, 1);
> + prepend_to_env("PATH", basedir, "/bin:",
> + "/usr/local/bin:/usr/bin:/bin");
> + prepend_to_env("MANPATH", basedir, "/share/man:",
> + "/usr/local/share/man:/usr/share/man");
> + prepend_to_env("PERL5LIB", basedir, "/lib/perl5:",
> + "/usr/lib/perl5");
>
> - free(path);
> + free(basedir);
> }
As I said, this is so controversial it belongs into an own patch.
> @@ -414,8 +444,7 @@ int main(int argc, const char **argv)
> */
> if (slash) {
> *slash++ = 0;
> - if (*cmd == '/')
> - exec_path = cmd;
> + exec_path = cmd;
As I said, this breaks down. This alone is enough reason to move it to
its own patch. And I strongly suggest the use of make_path_absolute()
(with an xstrdup()).
> @@ -453,14 +482,15 @@ int main(int argc, const char **argv)
> /*
> * We execute external git command via execv_git_cmd(),
> * which looks at "--exec-path" option, GIT_EXEC_PATH
> - * environment, and $(gitexecdir) in Makefile while built,
> - * in this order. For scripted commands, we prepend
> - * the value of the exec_path variable to the PATH.
> + * environment, PATH environment, and $(gitexecdir) in
> + * Makefile while built, in this order. For scripted
> + * commands, we prepend the value of the exec_path
> + * variable to the PATH.
While reading this, I have to wonder why it is not just simpler to try
with builtin_exec_path first, and if that fails, just let exec() find the
program in the PATH?
Ciao,
Dscho
^ permalink raw reply
* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Johannes Schindelin @ 2007-10-19 14:31 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Johannes Sixt, Theodore Tso, Santi Béjar, Shawn O. Pearce,
David Symonds, Jeff King, git
In-Reply-To: <alpine.LFD.0.9999.0710191009330.19446@xanadu.home>
Hi,
On Fri, 19 Oct 2007, Nicolas Pitre wrote:
> On Fri, 19 Oct 2007, Johannes Sixt wrote:
>
> > ==> git://repo.or.cz/git/spearce.git
> > * (new) gitk -> spearce/gitk
> > * 1aa3d01..e7187e4 maint -> spearce/maint
> > * de61e42..7840ce6 master -> spearce/master
> > * 895be02..2fe5433 next -> spearce/next
> > + 89fa332...1e4c517 pu -> spearce/pu
> > * (new) todo -> spearce/todo
>
> Actually I think this is the best format so far: one line per branch, no
> terminal width issue (long branch names are simply wrapped), the
> old..new info is there also with the single character marker to quickly
> notice the type of update.
Yes. Definitely my favourite so far, too.
Ciao,
Dscho
^ permalink raw reply
* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Santi Béjar @ 2007-10-19 14:31 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Johannes Sixt, Theodore Tso, Shawn O. Pearce, David Symonds,
Jeff King, git
In-Reply-To: <alpine.LFD.0.9999.0710191009330.19446@xanadu.home>
On 10/19/07, Nicolas Pitre <nico@cam.org> wrote:
> On Fri, 19 Oct 2007, Johannes Sixt wrote:
>
> > Theodore Tso schrieb:
> > > ==> git://repo.or.cz/git/spearce.git
> > > * branch gitk -> spearce/gitk (new)
> > > * branch maint -> spearce/maint 1aa3d01..e7187e4
> > > * branch master -> spearce/master de61e42..7840ce6
> > > * branch next -> spearce/next 895be02..2fe5433
> > > + branch pu -> spearce/pu 89fa332...1e4c517
> > > * branch todo -> spearce/todo (new)
> >
> > > As far as the padding, it would be a pain to figure out how to make
> > > the right hand column be padded so that it starts 3 spaces after the
> > > longest " * branch foo -> bar" line, but that would look the best.
> >
> > But this way it wouldn't be difficult at all:
> >
> > ==> git://repo.or.cz/git/spearce.git
> > * (new) gitk -> spearce/gitk
> > * 1aa3d01..e7187e4 maint -> spearce/maint
> > * de61e42..7840ce6 master -> spearce/master
> > * 895be02..2fe5433 next -> spearce/next
> > + 89fa332...1e4c517 pu -> spearce/pu
> > * (new) todo -> spearce/todo
>
> Actually I think this is the best format so far: one line per branch, no
> terminal width issue (long branch names are simply wrapped), the
> old..new info is there also with the single character marker to quickly
> notice the type of update.
I like it too. I would like to add some more descripton, because I
think for newbies the .. and ... can be overlooked. Something like:
$ git fetch spearce
...
URL: git://repo.or.cz/git/spearce.git
* (new) spearce/gitk: new branch 'gitk'
* 1aa3d01..e7187e4 spearce/maint: fast forward to branch 'maint'
* de61e42..7840ce6 spearce/master: fast forward to branch 'master'
* 895be02..2fe5433 spearce/next: fast forward to branch 'next'
+ 89fa332...1e4c517 spearce/pu: forcing update to non-fast forward branch 'pu'
* (new) spearce/todo: new branch spearce/todo
I would also put 'URL:' instead '==>'.
Santi
^ permalink raw reply
* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Karl Hasselström @ 2007-10-19 14:38 UTC (permalink / raw)
To: Theodore Tso
Cc: Santi Béjar, Shawn O. Pearce, David Symonds, Jeff King, git
In-Reply-To: <20071019113822.GB16726@thunk.org>
On 2007-10-19 07:38:22 -0400, Theodore Tso wrote:
> Finally, one last question --- am I the only one who had to take a
> second look at the whether the arrow should be <- or ->? The
> question is whether we are saying "gitk is moving to include all of
> spearce/gitk"; but I could also see it stated that we are assigning
> refs/heads/gitk with refs/remotes/spearce/gitk, in which case the
> arrow should be reversed. Or maybe:
>
> ==> git://repo.or.cz/git/spearce.git
> * branch gitk := spearce/gitk (new)
> * branch maint := spearce/maint 1aa3d01..e7187e4
> * branch master := spearce/master de61e42..7840ce6
> * branch next := spearce/next 895be02..2fe5433
> + branch pu := spearce/pu 89fa332...1e4c517
> * branch todo := spearce/todo (new)
I think the reasoning behind the "foo -> spearce/foo" syntax is that
"(refs/heads/)foo" in the remote repository has been fetched to
"(refs/remotes/)spearce/foo" in the local repository.
I might be deluded, though.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ 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