* Re: [RFC 2/2] Add Git-aware CGI for Git-aware smart HTTP transport
From: Shawn O. Pearce @ 2008-08-04 3:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, H. Peter Anvin
In-Reply-To: <7vwsix7nhw.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> But could you be a bit more explicit than application/x-git-refs magic? I
> suspect very strongly that clueless server operators would advertise the
> type on repositories statically hosted there, and would defeat the point
> of your patch.
This is a very valid concern. I started to worry about it myself
last night, but decided it was late enough and just wanted to start
the discussion on the list, extending JH's thread even further.
> Perhaps "#" comment line in info/refs that is ignored on the reading side
> but update-server-info never generates on its own?
This is a good idea. I think anyone who consumes info/refs does
so with the understanding that "#" comment lines exist, and should
be skipped, but this is not something that has been heavily tested
in the wild yet.
My concern here goes back to the remark you made above. What if a
server owner mirrors a smart server by a non-Git aware device like
wget? They will now have a copy of the info/refs content which will
suggest we have Git smarts on the backend, but really it isn't there.
Perhaps the smart server detection is something like:
Smart Server Detection
----------------------
To detect a smart (Git-aware) server a client sends an
empty POST request to info/refs; if a 200 OK response is
received with the proper content type then the server can
be assumed to be Git-aware, and the result contains the
current info/refs data for that repository.
C: POST /repository.git/info/refs HTTP/1.0
C: Content-Length: 0
S: HTTP/1.0 200 OK
S: Content-Type: application/x-git-refs
S:
S: 95dcfa3633004da0049d3d0fa03f80589cbcaf31 refs/heads/maint
Then clients should just attempt this POST first before issuing
a GET info/refs. Non Git-aware servers will issue an error code,
and the client can retry with a standard GET request, and assume
the server isn't a newer style.
--
Shawn.
^ permalink raw reply
* Re: [PATCH] bash completion: Add completion for 'git grep'
From: Shawn O. Pearce @ 2008-08-04 4:06 UTC (permalink / raw)
To: Lee Marlow; +Cc: git
In-Reply-To: <7968d7490808030031v39726af6r9f88feb28df02de7@mail.gmail.com>
Lee Marlow <lee.marlow@gmail.com> wrote:
> On Sat, Aug 2, 2008 at 3:05 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
> >
> > Hmm. The has_doubledash test seems redundant since we don't do
> > anything with args that aren't --foo. Even though git-grep will
> > accept a tree-ish and thus completion of __git_refs here may
>
> I haven't found myself using grep to search anything but the current
> working directory. I wonder whether __git_complete_file would be
> better than __git_refs. My issue with __git_complete_file in this
> case and also doing completion for 'git mv' is that it falls back to
> just __git_refs. Would it be better if it fell back to __git_refs and
> ls-tree for HEAD? That way when using completion to get to
> Documentation/git-grep.txt, it doesn't also show completions for
> Documentation/git-grep.{1,html,xml}.
Hmm. __git_complete_file is about completing something in a tree-ish,
which is really most likely not what we want for grep. Its useful for
git-show and git-cat-file, and that's about it. Most git tools prefer
a calling format of "tree-ish -- paths ..." and not "tree-ish:path".
Though I think you have an excellent point about completing paths
by ls-tree for HEAD rather than the working directory itself,
as then you can avoid produced files. But in practice how often
do you pass a single file or a couple of files to the grep utility
and are having problems bypassing the build products? Compared to
just running grep on the entire source tree to find hits? I never
use grep to search in a single file, but I call it several times
a day to look for where a function is defined or called.
After having thought about it your original patch makes the most
sense, but without the has_doubledash test.
--
Shawn.
^ permalink raw reply
* [git/perl] unusual syntax?
From: Ray Chuan @ 2008-08-04 4:49 UTC (permalink / raw)
To: git
Hi,
i noticed that this doesn't work for me (Perl 5.10):
sub _close_hash_and_insert_object {
my ($self) = @_;
return unless defined($self->{hash_object_pid});
my @vars = map { 'hash_object_' . $_ } qw(pid in out ctx);
command_close_bidi_pipe($self->{@vars});
delete $self->{@vars};
}
$self->{@vars} evaluates to undef. i can't find any mention of using
arrays to dereference objects in the manual and elsewhere; is this a
mistake?
--
Cheers,
Ray Chuan
^ permalink raw reply
* [git/perl/cygwin] Permission denied at /usr/bin/git-svn line 2339
From: Ray Chuan @ 2008-08-04 5:16 UTC (permalink / raw)
To: git
Hi,
i'm encountering an issue with git-svn running on cygwin (not sure if
this is exclusive to cygwin) when migrating data from svn to git.
$ git-svn clone file:///F:/test/svn-native-repo/
Initialized empty Git repository in /cygdrive/F/test/git/svn-native-repo/.git/
A sitemap.html
A funcs.js
A index.html
A styles.css
r1 = d112a5b470fd4387c7bbb7199d0d7a065754b127 (git-svn)
Permission denied: Can't open
'/cygdrive/c/DOCUME~1/myself/LOCALS~1/Temp/report.tmp': Permission
denied at /usr/bin/git-svn line 2339
the repo used was a dummy subversion repo with 2 revisions: an add,
and a modification commit. (dump file here [1])
i suspect this is caused by locks over files. using perl -d:
$ perl -d /usr/bin/git-svn clone file:///F:/test/svn-native-repo/
c 3995
c 3995
c 3838
s
c 85
SVN::Ra::AUTOLOAD(/usr/lib/perl5/vendor_perl/5.10/i686-cygwin/SVN/Ra.pm:85):
85: my @ret = $func->($self->{session}, @_);
immediately after this line the error is generated.
a fix would be to issue closes to the processes, as in
Git::_close_hash_and_insert_object and Git::_close_cat_blob:
767a768
> $self->_close_hash_and_insert_object();
856a858
> $self->_close_cat_blob();
(Note: currently this doesn't fix the issue too, cos the methods do
not do their job properly (on my side, at least): see
http://article.gmane.org/gmane.comp.version-control.git/91314
)
my environment is:
- win xp home sp2
- Cygwin: 1.5.25-15 (latest)
- Git: 1.5.6.4-1 (latest)
- subversion-perl: 1.4.6-3 (latest)
- Perl: 5.10.0-5 (latest)
[1] http://thelilaeanbellis.com/shared/svn-native-repo.dump
^ permalink raw reply
* [PATCH] Fix hash slice syntax error
From: Abhijit Menon-Sen @ 2008-08-04 5:02 UTC (permalink / raw)
To: Ray Chuan; +Cc: git
In-Reply-To: <be6fef0d0808032149p651309a8o773dca5f16923ee1@mail.gmail.com>
Signed-off-by: Abhijit Menon-Sen <ams@toroid.org>
---
At 2008-08-04 12:49:27 +0800, rctay89@gmail.com wrote:
>
> $self->{@vars} evaluates to undef. i can't find any mention of using
> arrays to dereference objects in the manual and elsewhere; is this a
> mistake?
Yes, @vars would be interpreted in scalar context, which certainly isn't
the intended effect.
-- ams
perl/Git.pm | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 087d3d0..2ef437f 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -839,8 +839,8 @@ sub _close_hash_and_insert_object {
my @vars = map { 'hash_object_' . $_ } qw(pid in out ctx);
- command_close_bidi_pipe($self->{@vars});
- delete $self->{@vars};
+ command_close_bidi_pipe(@$self{@vars});
+ delete @$self{@vars};
}
=item cat_blob ( SHA1, FILEHANDLE )
@@ -928,8 +928,8 @@ sub _close_cat_blob {
my @vars = map { 'cat_blob_' . $_ } qw(pid in out ctx);
- command_close_bidi_pipe($self->{@vars});
- delete $self->{@vars};
+ command_close_bidi_pipe(@$self{@vars});
+ delete @$self{@vars};
}
=back
--
1.6.0.rc0.43.g2aa74
^ permalink raw reply related
* [PATCH] bash completion: Add completion for 'git grep'
From: Lee Marlow @ 2008-08-04 5:26 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Lee Marlow
In-Reply-To: <20080804040634.GC2963@spearce.org>
Added completions for all long options specified in the docs
--cached
--text --ignore-case --word-regexp --invert-match
--full-name
--extended-regexp --basic-regexp --fixed-strings
--files-with-matches --name-only
--files-without-match
--count
--and --or --not --all-match
Signed-off-by: Lee Marlow <lee.marlow@gmail.com>
---
On Sun, Aug 3, 2008 at 10:06 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
> Though I think you have an excellent point about completing paths
> by ls-tree for HEAD rather than the working directory itself,
> as then you can avoid produced files. But in practice how often
> do you pass a single file or a couple of files to the grep utility
> and are having problems bypassing the build products?
I think you're right that it's not a good fit for grep, but maybe for
some other commands. mv and commit come to mind. I'll look into this
as I'm trying to add completions for the more common (i.e. ones that I
use) porcelain commands.
> After having thought about it your original patch makes the most
> sense, but without the has_doubledash test.
This updated patch removes the __git_has_doublehash line. However, I
just noticed that d773c631 added __git_has_doublehash to prevent
completions of long options after --. So, maybe it's not too
redundant and my first patch might be better.
-Lee
contrib/completion/git-completion.bash | 22 ++++++++++++++++++++++
1 files changed, 22 insertions(+), 0 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 678a155..96181dc 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -791,6 +791,27 @@ _git_gc ()
COMPREPLY=()
}
+_git_grep ()
+{
+ local cur="${COMP_WORDS[COMP_CWORD]}"
+ case "$cur" in
+ --*)
+ __gitcomp "
+ --cached
+ --text --ignore-case --word-regexp --invert-match
+ --full-name
+ --extended-regexp --basic-regexp --fixed-strings
+ --files-with-matches --name-only
+ --files-without-match
+ --count
+ --and --or --not --all-match
+ "
+ return
+ ;;
+ esac
+ COMPREPLY=()
+}
+
_git_help ()
{
local cur="${COMP_WORDS[COMP_CWORD]}"
@@ -1482,6 +1503,7 @@ _git ()
fetch) _git_fetch ;;
format-patch) _git_format_patch ;;
gc) _git_gc ;;
+ grep) _git_grep ;;
help) _git_help ;;
log) _git_log ;;
ls-remote) _git_ls_remote ;;
--
1.6.0.rc1.48.g2b6032
^ permalink raw reply related
* Re: [PATCH] Use line buffering for standard output
From: Linus Torvalds @ 2008-08-04 5:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Anders Melchiorsen, git
In-Reply-To: <7vej557hlq.fsf@gitster.siamese.dyndns.org>
On Sun, 3 Aug 2008, Junio C Hamano wrote:
> Anders Melchiorsen <mail@cup.kalibalik.dk> writes:
> >
> > #ifndef __MINGW32__
> > + /* Flush output before fork() to avoid cloning the buffer */
> > + fflush(stdout);
> > +
> > async->pid = fork();
> > if (async->pid < 0) {
> > error("fork (async) failed: %s", strerror(errno));
>
> I think this with s/stdout/NULL/ would be a reasonable thing to do.
Agreed, I think that's the right thing to do.
There's another fork there in start_command(), I suspect we should do it
there too: it's a "generic" path, so it should try to be safe.
The other ones look ok from a quick scan. I don't know the imap-send.c
code, but it's from outside people who hopefully know what they were
doing. The other ones don't seem to be using stdio before the fork (except
for things like "die()" ;)
There is a "fork()" in a _comment_ in builtin-ls-tree.c, and that one
definitely should have a fflush(NULL) in front of it. But it _is_ just a
comment, and rather than addign a fflush() there, it would probably be
better to turn it into a "start_command()" or something like that.
Linus.
^ permalink raw reply
* Not going beyond symbolic links
From: Junio C Hamano @ 2008-08-04 7:49 UTC (permalink / raw)
To: git; +Cc: Petr Baudis, Linus Torvalds, Shawn O. Pearce, Johannes Schindelin
In-Reply-To: <7v8wvpm9cl.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> A symlink to us is just a different kind of blob, and by definition a blob
> is the leaf level of a tree structure that represents the working tree in
> the index. There won't be anything hanging below it, and when adding
> things to the index we should not dereference the symlink to see where it
> leads to.
>
> Traditionally we have been loose about this check, and the normal "git
> add" and "git update-index" codepath is still forever broken, and we
> allow:
>
> $ mkdir dir
> $ >dir/file
> $ ln -s dir symlink
> $ git add symlink/file
>
> but some codepaths that matter more (because they do larger damage
> unattended, as opposed to the above command sequence that can be avoided
> by user education a bit more easily), such as "git apply" and "git
> read-tree", have been corrected using has_symlink_leading_path() since mid
> 2007. We would need to follow through c40641b (Optimize symlink/directory
> detection, 2008-05-09) and further fix "git add" and "git update-index"
> codepaths to forbid the above command sequence.
I started to revisit this issue and patched "git update-index --add"
and "git add" so far. Patches follow.
I think we should also check the following and fix them if any of them is
broken. Under the same "dir/file exists but symlink points at dir"
scenario:
* "git rm symlink/file" --- should fail without removing dir/file;
* "git ls-tree $commit -- symlink/file" should *not* fail if $commit does
have "symlink/file" in it (iow, we cannot add the logic to get_pathspec());
* "git ls-files --exclude-standard -o -- symlink/file" should not talk
about "symlink/file".
If we reword the paragraph I quoted at the beginning of this message
slightly:
A gitlink to is is just a leaf level of a tree structure that
represents the working tree in the index. There won't be anything
hanging below it.
We would need a similar check to stop at module boundary, just like the
helper function we use for these patches, has_symlink_leading_path(),
stops at a symlink.
So without further ado...
^ permalink raw reply
* Re: [PATCH] git-svn.perl: Strip ChangeLog bits.
From: Jan Nieuwenhuizen @ 2008-08-04 7:49 UTC (permalink / raw)
To: Eric Wong; +Cc: Junio C Hamano, Petr Baudis, git
In-Reply-To: <20080804020931.GA4109@untitled>
On zo, 2008-08-03 at 19:09 -0700, Eric Wong wrote:
> Jan Nieuwenhuizen <janneke-list@xs4all.nl> wrote:
> > I could imagine that leaving git-svn alone and adding a hook to git-log
> > would be more useful, though.
>
> NACK on modifying git-svn to support more changelog formats.
>
> A better idea would be to write a generic script that takes "git log",
> "git svn log" or even plain "svn log" output and filters it
> independently.
> This filter should be reusable for both plain svn and git-svn:
> The possibilities are endless :)
Yes, but we'll most probably drop SVN rsn, possibly after a short period
of supporting both; and not stripping the cruft in the conversion means
we carry this with us until eternity. Besides, who is going to
distribute the script, inform users about its availability?
What we have now suffices for our ooo-build conversion. If you do not
find it useful, more power/less code to you. Let others search the
archives and/or patch git-svn themselves, if indeed there are any.
Jan.
--
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond - The music typesetter
http://www.xs4all.nl/~jantien | http://www.lilypond.org
^ permalink raw reply
* [PATCH 1/2] update-index: refuse to add working tree items beyond symlinks
From: Junio C Hamano @ 2008-08-04 7:51 UTC (permalink / raw)
To: git; +Cc: Petr Baudis, Linus Torvalds, Shawn O. Pearce, Johannes Schindelin
In-Reply-To: <7vej5543v5.fsf_-_@gitster.siamese.dyndns.org>
When "sym" is a symbolic link that is inside the working tree, and it
points at a directory "dir" that has "path" in it, "update-index --add
sym/path" used to mistakenly add "sym/path" as if "sym" were a normal
directory.
"git apply", "git diff" and "git merge" have been taught about this issue
some time ago, but "update-index" and "add" have been left ignorant for
too long.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-update-index.c | 5 ++++-
t/t0055-beyond-symlinks.sh | 20 ++++++++++++++++++++
2 files changed, 24 insertions(+), 1 deletions(-)
create mode 100755 t/t0055-beyond-symlinks.sh
diff --git a/builtin-update-index.c b/builtin-update-index.c
index 38eb53c..434cb8e 100644
--- a/builtin-update-index.c
+++ b/builtin-update-index.c
@@ -194,6 +194,10 @@ static int process_path(const char *path)
int len;
struct stat st;
+ len = strlen(path);
+ if (has_symlink_leading_path(len, path))
+ return error("'%s' is beyond a symbolic link", path);
+
/*
* First things first: get the stat information, to decide
* what to do about the pathname!
@@ -201,7 +205,6 @@ static int process_path(const char *path)
if (lstat(path, &st) < 0)
return process_lstat_error(path, errno);
- len = strlen(path);
if (S_ISDIR(st.st_mode))
return process_directory(path, len, &st);
diff --git a/t/t0055-beyond-symlinks.sh b/t/t0055-beyond-symlinks.sh
new file mode 100755
index 0000000..eb11dd7
--- /dev/null
+++ b/t/t0055-beyond-symlinks.sh
@@ -0,0 +1,20 @@
+#!/bin/sh
+
+test_description='update-index refuses to add beyond symlinks'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+ >a &&
+ mkdir b &&
+ ln -s b c &&
+ >c/d &&
+ git update-index --add a b/d
+'
+
+test_expect_success 'update-index --add beyond symlinks' '
+ test_must_fail git update-index --add c/d &&
+ ! ( git ls-files | grep c/d )
+'
+
+test_done
--
1.6.0.rc1.64.g61192
^ permalink raw reply related
* [PATCH 2/2] add: refuse to add working tree items beyond symlinks
From: Junio C Hamano @ 2008-08-04 7:52 UTC (permalink / raw)
To: git; +Cc: Petr Baudis, Linus Torvalds, Shawn O. Pearce, Johannes Schindelin
In-Reply-To: <7vej5543v5.fsf_-_@gitster.siamese.dyndns.org>
This is the same fix for the issue of adding "sym/path" when "sym" is a
symblic link that points at a directory "dir" with "path" in it.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-add.c | 12 +++++++++++-
dir.c | 6 +++++-
t/t0055-beyond-symlinks.sh | 7 ++++++-
3 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/builtin-add.c b/builtin-add.c
index fc3f96e..81b64d7 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -153,6 +153,16 @@ static const char **validate_pathspec(int argc, const char **argv, const char *p
{
const char **pathspec = get_pathspec(prefix, argv);
+ if (pathspec) {
+ const char **p;
+ for (p = pathspec; *p; p++) {
+ if (has_symlink_leading_path(strlen(*p), *p)) {
+ int len = prefix ? strlen(prefix) : 0;
+ die("'%s' is beyond a symbolic link", *p + len);
+ }
+ }
+ }
+
return pathspec;
}
@@ -278,7 +288,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
fprintf(stderr, "Maybe you wanted to say 'git add .'?\n");
return 0;
}
- pathspec = get_pathspec(prefix, argv);
+ pathspec = validate_pathspec(argc, argv, prefix);
/*
* If we are adding new files, we need to scan the working
diff --git a/dir.c b/dir.c
index 29d1d5b..ae7046f 100644
--- a/dir.c
+++ b/dir.c
@@ -727,8 +727,12 @@ static void free_simplify(struct path_simplify *simplify)
int read_directory(struct dir_struct *dir, const char *path, const char *base, int baselen, const char **pathspec)
{
- struct path_simplify *simplify = create_simplify(pathspec);
+ struct path_simplify *simplify;
+ if (has_symlink_leading_path(strlen(path), path))
+ return dir->nr;
+
+ simplify = create_simplify(pathspec);
read_directory_recursive(dir, path, base, baselen, 0, simplify);
free_simplify(simplify);
qsort(dir->entries, dir->nr, sizeof(struct dir_entry *), cmp_name);
diff --git a/t/t0055-beyond-symlinks.sh b/t/t0055-beyond-symlinks.sh
index eb11dd7..b29c37a 100755
--- a/t/t0055-beyond-symlinks.sh
+++ b/t/t0055-beyond-symlinks.sh
@@ -1,6 +1,6 @@
#!/bin/sh
-test_description='update-index refuses to add beyond symlinks'
+test_description='update-index and add refuse to add beyond symlinks'
. ./test-lib.sh
@@ -17,4 +17,9 @@ test_expect_success 'update-index --add beyond symlinks' '
! ( git ls-files | grep c/d )
'
+test_expect_success 'add beyond symlinks' '
+ test_must_fail git add c/d &&
+ ! ( git ls-files | grep c/d )
+'
+
test_done
--
1.6.0.rc1.64.g61192
^ permalink raw reply related
* [PATCH] Git.pm: Fix internal git_command_bidi_pipe() users
From: Petr Baudis @ 2008-08-04 7:56 UTC (permalink / raw)
To: gitster; +Cc: git
In-Reply-To: <be6fef0d0808032149p651309a8o773dca5f16923ee1@mail.gmail.com>
The hash_and_insert_object() and cat_blob() helpers were using
an incorrect slice-from-ref Perl syntax. This patch fixes that up
in the _close_*() helpers and make the _open_*() helpers use the
same syntax for consistnecy.
Signed-off-by: Petr Baudis <pasky@suse.cz>
---
Wow, the command_bidi_pipe API really is dirty. Of course, it is
my fault as anyone's since I didn't get around to review the patches
introducing it.
perl/Git.pm | 16 ++++++----------
1 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 087d3d0..0624428 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -827,8 +827,7 @@ sub _open_hash_and_insert_object_if_needed {
return if defined($self->{hash_object_pid});
- ($self->{hash_object_pid}, $self->{hash_object_in},
- $self->{hash_object_out}, $self->{hash_object_ctx}) =
+ @$self{map { "hash_object_$_" } qw(pid in out ctx)} =
command_bidi_pipe(qw(hash-object -w --stdin-paths));
}
@@ -837,9 +836,8 @@ sub _close_hash_and_insert_object {
return unless defined($self->{hash_object_pid});
- my @vars = map { 'hash_object_' . $_ } qw(pid in out ctx);
-
- command_close_bidi_pipe($self->{@vars});
+ my @vars = map { "hash_object_$_" } qw(pid in out ctx);
+ command_close_bidi_pipe(@$self{@vars});
delete $self->{@vars};
}
@@ -916,8 +914,7 @@ sub _open_cat_blob_if_needed {
return if defined($self->{cat_blob_pid});
- ($self->{cat_blob_pid}, $self->{cat_blob_in},
- $self->{cat_blob_out}, $self->{cat_blob_ctx}) =
+ @$self{map { "cat_blob_$_" } qw(pid in out ctx)} =
command_bidi_pipe(qw(cat-file --batch));
}
@@ -926,9 +923,8 @@ sub _close_cat_blob {
return unless defined($self->{cat_blob_pid});
- my @vars = map { 'cat_blob_' . $_ } qw(pid in out ctx);
-
- command_close_bidi_pipe($self->{@vars});
+ my @vars = map { "cat_blob_$_" } qw(pid in out ctx);
+ command_close_bidi_pipe(@$self{@vars});
delete $self->{@vars};
}
^ permalink raw reply related
* Re: [PATCH 0/5] Fix 'url.*.insteadOf' for submodule URLs
From: Johan Herland @ 2008-08-04 7:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vwsix5y9w.fsf@gitster.siamese.dyndns.org>
On Monday 04 August 2008, Junio C Hamano wrote:
> Johan Herland <johan@herland.net> writes:
> > 1. Consistency: Other git commands in the supermodule does _not_
> > require the URL rewriting rule to reside in the global config. Why
> > should 'git submodule' be different.
>
> When it comes to "submodules", I do not think such consistency argument
> makes much sense. "git submodule" command crosses module boundary,
> normal commands don't. They are naturally different and they should be.
>
> Your kind of consistency means breaking the separation between module
> boundary, doesn't it?
To a certain degree, yes, but (1) only for this specific config rule
(url.*.insteadOf), and (2) only at the specific point in time when the
submodule is cloned. This means that after the submodule is cloned, it is
indeed not affected by the super-repo's config. E.g. you can change the
origin URL of the submodule back to the original (un-rewritten) URL in the
submodule's config, and the super-repo will not try to rewrite it at again.
I believe that the module boundary should not be crossed in general. But
this patch only "crosses" that boundary before it really exists (i.e.
before the submodule has been cloned into the super-repo). The event of
introducing a submodule into a super-repo, is AFAICS the _only_ event where
I would consider to cross the module boundary (and then only for a good
reason).
> Having said that...
>
> > 2. I believe there are valid use cases for adding URL rewriting rules
> > to the repo config instead of the global config. You may want to check
> > out Fred's version of project X (including submodules), without making
> > your other clones of project X start cloning/fetching from Fred.
>
> I think you are referring to the example given in an earlier thread to
> peek what your neighbor did between you two, without affecting other
> people.
Correct.
> Personally I think it is partly showing the shortcoming of the current
> "git submodule" that minimally supports the workflow to follow what the
> canonical repository does, and partly showing that it is an abuse of that
> interface to rewrite config file to temporarily switch to peek somewhere
> else in such a workflow.
Indeed. However, the development of submodules usability seems so slow that
even though this use case is an ugly workaround, it's one I thought we'd
have to live with for some time... [1]
> Let's step back and think what we would do if there is no submodule
> involved. That is, you usually follow origin, but you temporarily want
> to peek at what Fred did. How would you do this?
>
> $ git fetch $fred $branch_fred_wants_you_to_review
> $ git checkout FETCH_HEAD ;# this detaches HEAD.
>
> And you take a look around. Perhaps you like the change and decide to
> merge that to your branch. Perhaps you create your own branch on top of
> that state, build a few fix-up commits, and give the result back to Fred.
>
> Shouldn't peeking what Fred did in the whole submodule hierarchy be
> essentially the same thing? That is,
>
> $ git submodule for-each-submodule sh -c '
> git fetch "$fred/$1" $branch_fred_wants_you_to_review &&
> git checkout FETCH_HEAD
> ' -
>
> where "for-each-submodule" would iterate over the submodules in the
> current superproject that you are interested in (that is, you actually
> have corresponding repositories there), and runs any given command with
> the path to the submodule in that directory.
>
> Hmm?
Yes, having such functionality in 'git submodule' would be wonderful.
However, implementations of such functionality have been slow in coming,
and apparently hard to implement without being workflow-agnostic (if I
remember correctly).
In light of improvements to 'git submodule', feel free to disregard my patch
series (although I still find 'git config --rewrite-url' useful for
dry-testing my 'url.*.insteadOf' rules...).
Thanks for providing constructive feedback.
Have fun!
...Johan
[1]: Maybe 'git submodule' would improve more quickly if we ate our own
dogfood, i.e. if we included submodules (e.g. gitk and git-gui) in git.git?
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [PATCH] Git.pm: Fix internal git_command_bidi_pipe() users
From: Junio C Hamano @ 2008-08-04 8:05 UTC (permalink / raw)
To: Petr Baudis; +Cc: git
In-Reply-To: <20080804075313.21325.28396.stgit@localhost>
Petr Baudis <pasky@suse.cz> writes:
> The hash_and_insert_object() and cat_blob() helpers were using
> an incorrect slice-from-ref Perl syntax. This patch fixes that up
> in the _close_*() helpers and make the _open_*() helpers use the
> same syntax for consistnecy.
>
> Signed-off-by: Petr Baudis <pasky@suse.cz>
> ---
>
> Wow, the command_bidi_pipe API really is dirty. Of course, it is
> my fault as anyone's since I didn't get around to review the patches
> introducing it.
Sorry, delete is still broken with your patch, isn't it?
The earlier patch from Abhijit Menon-Sen does this properly for
close_hash_and_insert and close_cat_blob, which I've queued already.
^ permalink raw reply
* Re: [PATCH] git-svn.perl: Strip ChangeLog bits.
From: Jan Nieuwenhuizen @ 2008-08-04 8:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Eric Wong, Petr Baudis, git
In-Reply-To: <7vhca1u8to.fsf@gitster.siamese.dyndns.org>
On zo, 2008-08-03 at 13:45 -0700, Junio C Hamano wrote:
> Nice try, but after -rc1 we won't take feature enhancements on the
> 'master' branch. The earliest this will appear is in 1.6.1.
Ok, I'm not that familiar with git development and I did not find any
newer/UNRELEASED list of features?
> * Documentation; introduce this with heading --clean-changelog=<style>; I
Ok. I tried ={gnu} first, which seems to be the style for multiple
choice arguments, but the document parser does not grok that. {gnu|foo}
or {gnu|no-other-yet} did not really please me.
> You seem to have taken the "arbitrary Perl snippet" part of my patch as
> well, but it is not described here...
It all depends upon how you read the future. I would most have chosen
to postpone that work until the second (or third) request for different
munging came in, but now that the code is already written...
> * Script; two separate _clean_changelog and _clean_log_message variables
> are not necessary (I removed the extra variable in the patch below).
Good, I didn't really look at that.
> Your new tests do not seem to check these, but I think you should:
> - what should happen without --clean-changelog=gnu? (iow, additional
> code does not regress the behaviour when this shiny new toy is not
> used).
We could add a test to make sure that git-svn does not alter commit
messages, but it has little to do with this patch.
If this is not being tested atm, it is probably not deemed important
enough to test. This could have regressed at any time.
I would add a test for existing working code only if experience tells
you it is fragile and it (often) regresses, ie, when you fix a bug:
new/revised code.
> - what should happen when an unknown style is given e.g. --clean-changelog=yak?
It would be nice if the script failed with an error message, telling
what the options are, but I do not really care that much about wrong
use. You have that automatically if you use a sensible option parser,
this is where such a feature should be implemented, imho.
> We prefer to use "test_cmp" for comparing expected and actual result,
> not bare "cmp".
Ok.
> Here is what I tested and based the above comments on after minor fixes to
> ask comments from Eric.
Great, thanks. We'll see what happens then.
Jan.
--
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond - The music typesetter
http://www.xs4all.nl/~jantien | http://www.lilypond.org
^ permalink raw reply
* Re: [PATCH] Git.pm: Fix internal git_command_bidi_pipe() users
From: Petr Baudis @ 2008-08-04 8:21 UTC (permalink / raw)
To: Junio C Hamano, ams; +Cc: git
In-Reply-To: <7vtze12oij.fsf@gitster.siamese.dyndns.org>
On Mon, Aug 04, 2008 at 01:05:56AM -0700, Junio C Hamano wrote:
> Petr Baudis <pasky@suse.cz> writes:
>
> > The hash_and_insert_object() and cat_blob() helpers were using
> > an incorrect slice-from-ref Perl syntax. This patch fixes that up
> > in the _close_*() helpers and make the _open_*() helpers use the
> > same syntax for consistnecy.
> >
> > Signed-off-by: Petr Baudis <pasky@suse.cz>
> > ---
> >
> > Wow, the command_bidi_pipe API really is dirty. Of course, it is
> > my fault as anyone's since I didn't get around to review the patches
> > introducing it.
>
> Sorry, delete is still broken with your patch, isn't it?
Oh, right - I forgot that one and it didn't occur to me to test this
part.
> The earlier patch from Abhijit Menon-Sen does this properly for
> close_hash_and_insert and close_cat_blob, which I've queued already.
Abhijit, can you please tag your Git.pm patches so that I actually have
a chance to see and review it?
Thanks,
Petr "Pasky" Baudis
^ permalink raw reply
* Re: [PATCH] Git.pm: Fix internal git_command_bidi_pipe() users
From: Junio C Hamano @ 2008-08-04 8:37 UTC (permalink / raw)
To: Petr Baudis; +Cc: ams, git
In-Reply-To: <20080804082117.GI10151@machine.or.cz>
Petr Baudis <pasky@suse.cz> writes:
> On Mon, Aug 04, 2008 at 01:05:56AM -0700, Junio C Hamano wrote:
>> Petr Baudis <pasky@suse.cz> writes:
>>
>> > The hash_and_insert_object() and cat_blob() helpers were using
>> > an incorrect slice-from-ref Perl syntax. This patch fixes that up
>> > in the _close_*() helpers and make the _open_*() helpers use the
>> > same syntax for consistnecy.
>> >
>> > Signed-off-by: Petr Baudis <pasky@suse.cz>
>> > ---
>> >
>> > Wow, the command_bidi_pipe API really is dirty. Of course, it is
>> > my fault as anyone's since I didn't get around to review the patches
>> > introducing it.
>>
>> Sorry, delete is still broken with your patch, isn't it?
>
> Oh, right - I forgot that one and it didn't occur to me to test this
> part.
>
>> The earlier patch from Abhijit Menon-Sen does this properly for
>> close_hash_and_insert and close_cat_blob, which I've queued already.
>
> Abhijit, can you please tag your Git.pm patches so that I actually have
> a chance to see and review it?
It is $gmane/91316, Message-ID: <20080804050247.GA13539@toroid.org>
After queueing it, I actually had to revert it, because it seems to break
git-svn (t9106-git-svn-commit-diff-clobber.sh, test #8), and I am about to
go to bed. If you did not see the same breakage with your patch that does
not "fix" delete, it could be that the git-svn uses some of the resources
that are released by the delete actually doing what it is asked to do.
---
At 2008-08-04 12:49:27 +0800, rctay89@gmail.com wrote:
>
> $self->{@vars} evaluates to undef. i can't find any mention of using
> arrays to dereference objects in the manual and elsewhere; is this a
> mistake?
Yes, @vars would be interpreted in scalar context, which certainly isn't
the intended effect.
-- ams
perl/Git.pm | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 087d3d0..2ef437f 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -839,8 +839,8 @@ sub _close_hash_and_insert_object {
my @vars = map { 'hash_object_' . $_ } qw(pid in out ctx);
- command_close_bidi_pipe($self->{@vars});
- delete $self->{@vars};
+ command_close_bidi_pipe(@$self{@vars});
+ delete @$self{@vars};
}
=item cat_blob ( SHA1, FILEHANDLE )
@@ -928,8 +928,8 @@ sub _close_cat_blob {
my @vars = map { 'cat_blob_' . $_ } qw(pid in out ctx);
- command_close_bidi_pipe($self->{@vars});
- delete $self->{@vars};
+ command_close_bidi_pipe(@$self{@vars});
+ delete @$self{@vars};
}
=back
--
1.6.0.rc0.43.g2aa74
^ permalink raw reply related
* Re: Importing patches from email, which address?
From: Karl Hasselström @ 2008-08-04 9:09 UTC (permalink / raw)
To: Jon Smirl; +Cc: Git Mailing List, Linus Torvalds
In-Reply-To: <9e4733910807280959h19039402kc14bfa1a3b653846@mail.gmail.com>
On 2008-07-28 12:59:24 -0400, Jon Smirl wrote:
> When working with the list of author emails for the kernel, I
> noticed that a lot of the author's email address are from internal
> mail servers. How is the handled in git (stgit, etc..)? Is the
> import using the email From:, would it be better to use Reply-to:?
I believe all relevant tools use the From: header (unless it's
overridden by a From: in the message body). I think this is how Linus
originally wanted it to work -- in the first linux-kernel commit his
e-mail is torvalds@ppc970.osdl.org, and I seem to recall him saying
something along the lines of this being a feature: others could see
which machine he commited on, and the e-mail should really be
considered more of a unique identifier than an actual real e-mail
address.
Not sure if the policy has changed since then, though.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* Re: [StGit] StGit documentation on its configuration file usage.
From: Karl Hasselström @ 2008-08-04 9:14 UTC (permalink / raw)
To: Jurko Gospodneti?; +Cc: git, Catalin Marinas
In-Reply-To: <g6ku22$mgr$1@ger.gmane.org>
On 2008-07-28 19:01:55 +0200, Jurko Gospodneti? wrote:
> StGIT's project page links to the
> http://wiki.procode.org/cgi-bin/wiki.cgi/StGIT_Tutorial page for its
> tutorial information.
>
> There the 'Configuration file' notes state that StGIT uses
> /etc/stgitrc, ~/.stgitrc and then .git/stgitrc configuration files
> in that order and that there is an example configuration file under
> the examples folder. However, grepping through the StGIT sources I
> found no references to the stgitrc file but only wrappers for using
> the native GIT configuration system. Also, looking at the examples
> folder I found only an example that seems to indicate that StGIT's
> configuration options should be integrated with other regular GIT
> configuration options.
>
> Did I miss something? Or is this wiki information outdated?
It's outdated. We'll have to bug Catalin to change it, since it's
read-only.
> Testing this I see that StGIT does not read the git configuration
> correctly on Windows with msysgit anyway, but that's a story for a
> different thread.
Yes. StGit should be fairly Windows-portable, since it's all Python,
but to my knowledge no one has tried to use it on Windows, so there
are bound to be a few gotchas.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* Re: linking libgit.a in C++ projects
From: Boaz Harrosh @ 2008-08-04 8:57 UTC (permalink / raw)
To: Alex Riesen; +Cc: Avery Pennarun, cte, git
In-Reply-To: <20080803201211.GA11121@steel.home>
Alex Riesen wrote:
> Avery Pennarun, Thu, Jul 31, 2008 20:55:26 +0200:
>> On 7/31/08, Alex Riesen <raa.lkml@gmail.com> wrote:
>>> Boaz Harrosh, Thu, Jul 31, 2008 15:04:50 +0200:
>>>> Produce a C file and header that defines some stable API to your
>>> > GUI application, that does not expose any git internal headers.
>>> > Then compile that, say git_api.c, with C compiler in Makefile
>>> > and extern "C" link that file to your C++ application. This will
>>> > completely insulate you from any git code.
>>>
>>> no, it wont. He still have to resolve name conflicts at the link time.
>> Language keywords (as opposed to function names) like 'new' and
>> 'typename' are definitely not exported to the object files. Moreover,
>> function parameter names aren't either.
>>
>
> Didn't mean them. Meant the globally visible names. libgit does not
> use a prefix for its exported symbols. They will clash with the
> symbols of the programs it is linked to.
>
But that's a problem for C programs, C++ programs will not have that
problem, right? ;-)
With C programs these git symbols can be avoided.
Boaz
^ permalink raw reply
* Re: [PATCH] git-svn.perl: Strip ChangeLog bits.
From: Eric Wong @ 2008-08-04 9:03 UTC (permalink / raw)
To: Jan Nieuwenhuizen; +Cc: Junio C Hamano, Petr Baudis, git
In-Reply-To: <1217836189.7649.7.camel@heerbeest>
Jan Nieuwenhuizen <janneke-list@xs4all.nl> wrote:
> On zo, 2008-08-03 at 19:09 -0700, Eric Wong wrote:
>
> > Jan Nieuwenhuizen <janneke-list@xs4all.nl> wrote:
> > > I could imagine that leaving git-svn alone and adding a hook to git-log
> > > would be more useful, though.
> >
> > NACK on modifying git-svn to support more changelog formats.
> >
> > A better idea would be to write a generic script that takes "git log",
> > "git svn log" or even plain "svn log" output and filters it
> > independently.
>
> > This filter should be reusable for both plain svn and git-svn:
>
> > The possibilities are endless :)
>
> Yes, but we'll most probably drop SVN rsn, possibly after a short period
> of supporting both; and not stripping the cruft in the conversion means
> we carry this with us until eternity. Besides, who is going to
> distribute the script, inform users about its availability?
>
> What we have now suffices for our ooo-build conversion. If you do not
> find it useful, more power/less code to you. Let others search the
> archives and/or patch git-svn themselves, if indeed there are any.
In the one-shot case, a git filter-branch script would probably be ideal
and reusable for other projects.
Anyways, I strongly believe changelog modification/reformatting should
be done in a more generic way that can benefit users of other tools
(archimport/cvsimport etc...), too.
--
Eric Wong
^ permalink raw reply
* Re: [StGIT] Not working on Windows with msysgit.
From: Karl Hasselström @ 2008-08-04 9:25 UTC (permalink / raw)
To: Jurko Gospodneti?; +Cc: git, Catalin Marinas
In-Reply-To: <g6kvmc$sqr$1@ger.gmane.org>
On 2008-07-28 19:29:49 +0200, Jurko Gospodneti? wrote:
> I believe StGIT fails to work on Windows with msysgit installed
> because of the way it runs its external executables. It assumes that
> the executable is available on the path and is named exactly 'git'.
Yes, that's correct.
> This however is not the case on Windows with msysgit installed.
> There the 'git executable' is a batch script named git.cmd and stgit
> fails to find it causing all git calls to return an error code.
>
> Popen() calls in StGIT's run.py module seem to run their executables
> (git & gitk only as far as I saw from the sources) directly instead
> of running them through the shell in order to have the shell try all
> the default extensions (configured on Windows using the PATHEXT
> environment variable).
>
> One 'fix' that corrects this in all the use cases on Windows that I
> tried is to add the shell=True parameter to all Popen() calls in
> StGIT's run.py module (one in __run_io() and one in __run_noio()).
> This would however require more testing
Don't do that, please.
The reason to not go via the shell is that that way, we don't have to
worry about quoting. That's a big pile of bugs that we never have to
see again.
Plus, it's also slightly faster (especially on Windows, I guess) not
to have to spawn a shell.
It'd be _much_ better to just be a little more flexible about which
git command to use. Maybe look at PATHEXT if we're on Windows, or
maybe let the user configure the location of git at stg install time.
(RFC) patches welcome: :-)
> I do recall Python having some serious problems with these quoting
> in executed commands... something about external quotes getting
> stripped in some cases... I might be able to dig up a workaround
> from somewhere if needed...
That's the shell doing a level of unquoting. Nothing Python specific.
--
Karl Hasselström, kha@treskal.com
www.treskal.com/kalle
^ permalink raw reply
* Re: [RFC 2/2] Add Git-aware CGI for Git-aware smart HTTP transport
From: Rogan Dawes @ 2008-08-04 9:53 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Junio C Hamano, git, H. Peter Anvin
In-Reply-To: <20080804035921.GB2963@spearce.org>
Shawn O. Pearce wrote:
> Perhaps the smart server detection is something like:
>
> Smart Server Detection
> ----------------------
>
> To detect a smart (Git-aware) server a client sends an
> empty POST request to info/refs; if a 200 OK response is
> received with the proper content type then the server can
> be assumed to be Git-aware, and the result contains the
> current info/refs data for that repository.
>
> C: POST /repository.git/info/refs HTTP/1.0
> C: Content-Length: 0
>
> S: HTTP/1.0 200 OK
> S: Content-Type: application/x-git-refs
> S:
> S: 95dcfa3633004da0049d3d0fa03f80589cbcaf31 refs/heads/maint
>
> Then clients should just attempt this POST first before issuing
> a GET info/refs. Non Git-aware servers will issue an error code,
> and the client can retry with a standard GET request, and assume
> the server isn't a newer style.
>
I don't understand why you would want to keep the commands in the URL
when you are doing a POST?
How about something like:
C: POST /repository.git/ HTTP/1.0
C: Content-Length: <calculated>
C:
C: <whatever command you want>
A dumb server will respond with:
S: HTTP/1.1 405 Method not allowed
(expected according to the RFC)
Or
S: HTTP/1.1 404 Not Found
(resulting from testing against my own repo :-) )
While a smart server will respond with a "200 Ok" and the results of the
command.
Also, if everything is done via POST, you don't have to worry about a
wget-cloned server appearing to be "smart", since no "smarts" will ever
be returned in response to a GET request (and to the best of my
knowledge, wget can't mirror using POST).
Rogan
^ permalink raw reply
* Re: [RFC 2/2] Add Git-aware CGI for Git-aware smart HTTP transport
From: Johannes Schindelin @ 2008-08-04 10:08 UTC (permalink / raw)
To: Rogan Dawes; +Cc: Shawn O. Pearce, Junio C Hamano, git, H. Peter Anvin
In-Reply-To: <4896D19C.6040704@dawes.za.net>
Hi,
On Mon, 4 Aug 2008, Rogan Dawes wrote:
> Shawn O. Pearce wrote:
>
> > Perhaps the smart server detection is something like:
> >
> > Smart Server Detection
> > ----------------------
> >
> > To detect a smart (Git-aware) server a client sends an
> > empty POST request to info/refs; if a 200 OK response is
> > received with the proper content type then the server can
> > be assumed to be Git-aware, and the result contains the
> > current info/refs data for that repository.
> >
> > C: POST /repository.git/info/refs HTTP/1.0
> > C: Content-Length: 0
> >
> > S: HTTP/1.0 200 OK
> > S: Content-Type: application/x-git-refs
> > S:
> > S:95dcfa3633004da0049d3d0fa03f80589cbcaf31 refs/heads/maint
> >
> > Then clients should just attempt this POST first before issuing
> > a GET info/refs. Non Git-aware servers will issue an error code,
> > and the client can retry with a standard GET request, and assume
> > the server isn't a newer style.
> >
>
> I don't understand why you would want to keep the commands in the URL
> when you are doing a POST?
Caching.
Hth,
Dscho
^ permalink raw reply
* [PATCH] Gitweb: Provide Git links in project list
From: Robert Richter @ 2008-08-04 10:06 UTC (permalink / raw)
To: J.H.; +Cc: Junio C Hamano, git, Robert Richter
In-Reply-To: <1217796793.32240.36.camel@localhost.localdomain>
Signed-off-by: Robert Richter <robert.richter@amd.com>
---
gitweb/gitweb.perl | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 90cd99b..6d9b7aa 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3558,6 +3558,9 @@ sub fill_project_list_info {
if (!defined $pr->{'owner'}) {
$pr->{'owner'} = git_get_project_owner("$pr->{'path'}") || "";
}
+ if (!defined $pr->{'url_link'}) {
+ $pr->{'url_link'} = (git_get_project_url_list("$pr->{'path'}"))[0] || "";
+ }
if ($check_forks) {
my $pname = $pr->{'path'};
if (($pname =~ s/\.git$//) &&
@@ -3661,6 +3664,7 @@ sub git_project_list_body {
$cgi->a({-href => href(project=>$pr->{'path'}, action=>"shortlog")}, "shortlog") . " | " .
$cgi->a({-href => href(project=>$pr->{'path'}, action=>"log")}, "log") . " | " .
$cgi->a({-href => href(project=>$pr->{'path'}, action=>"tree")}, "tree") .
+ ($pr->{'url_link'} ? " | " . $cgi->a({-href => $pr->{'url_link'}}, "git") : '') .
($pr->{'forks'} ? " | " . $cgi->a({-href => href(project=>$pr->{'path'}, action=>"forks")}, "forks") : '') .
"</td>\n" .
"</tr>\n";
--
1.5.5.4
^ 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