* Re: [PATCH 2/3] unpack-trees: fix path search bug in verify_absent
From: Junio C Hamano @ 2009-01-04 10:01 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Clemens Buchacher, git
In-Reply-To: <alpine.DEB.1.00.0901031353090.30769@pacific.mpi-cbg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Sat, 3 Jan 2009, Clemens Buchacher wrote:
>
>> On Fri, Jan 02, 2009 at 10:59:47PM +0100, Johannes Schindelin wrote:
>> > This explanation makes sense. However, this:
>> >
>> > > @@ -289,7 +289,8 @@ static int unpack_nondirectories(int n, unsigned long mask, unsigned long dirmas
>> > > return 0;
>> > > }
>> > >
>> > > -static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, struct name_entry *names, struct traverse_info *info)
>> > > +static int unpack_callback(int n, unsigned long mask, unsigned long dirmask,
>> > > + struct name_entry *names, struct traverse_info *info)
>> > > {
>> > > struct cache_entry *src[5] = { NULL, };
>> > > struct unpack_trees_options *o = info->data;
>> >
>> > ... is distracting during review, and this:
>> >
>> > > @@ -517,22 +518,22 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action,
>> > > namelen = strlen(ce->name);
>> > > pos = index_name_pos(o->src_index, ce->name, namelen);
>> > > if (0 <= pos)
>> > > - return cnt; /* we have it as nondirectory */
>> > > + return 0; /* we have it as nondirectory */
>> > > pos = -pos - 1;
>> > > for (i = pos; i < o->src_index->cache_nr; i++) {
>> >
>> > ... is not accounted for in the commit message. Intended or not, that is
>> > the question.
>>
>> Those are trivial readability improvements in the context of the patch.
>
> They are not trivial enough for me not to be puzzled. Reason enough to
> explain in the commit message?
I'd say the first hunk quoted is probably on the borderline. It is an
unnecessary churn that won't even be commented on if it were sent alone,
but as a "while we are at it" hunk in a patch that is not too big, this is
a kind of thing that often is tolerated, because it is obvious enough not
to hurt anything from the correctness standpoint [*1*].
The second one is moderately worse for two reasons.
* I actually had to scratch my head because you need to view the change
in a lot wider context that covers the initializing definition of "int
cnt" near the beginning of the function down to the area affected by
the hunk, in order to see that the new "return 0" is the same as the
old "return cnt" and does not break things. A comment to say that "at
this point in the codeflow, cnt which is returned by the old code is
always zero", perhaps below the three-dash marker, would have saved me
a minute.
* The function's purpose and logic is to see if the subdirectory is
clean, and return how many cache entries need to be skipped if it is
(otherwise a negative number as an error indicator). For that purpose,
the return value cnt is initialized to 0 (i.e. "we haven't counted any
entry that needs to be skipped yet"), the loop below the patched part
counts it up while performing the verification, and then the resulting
count is returned from the function. The logic flow, at least to me,
is easier to follow if it returned the value in cnt, not a hardcoded 0,
from the place the patch tries to touch.
The latter point is with "at least to me", because I think an alternate
position is entirely valid if the author wants to justify the change by
saying something like:
The function's purpose is .... Before entering the loop to count the
number of entries to skip, this check to detect if we do not even have
to count appears. When this check triggers, we know we do not want to
skip anything, and returning constant 0 is much clearer than returning
a variable cnt that was initialized to 0 near the beginning of the
function; we haven't even started using it to count yet.
But the point is, if that is the reason the author thinks it is an
improvement, that probably needs to be stated.
[Footnote]
*1* I am not sure if it is obviously clear that the change improves any
readability. Some people argue that splitting the function definition
header hurts greppability for one thing. I personally do not find it easy
to read when the subsequent header lines are indented without aligning
(compare the way it is indented in the postimage of the patch with the way
the headers verify_absent() and show_stage_entry() are indented), either.
^ permalink raw reply
* GNOME DVCS Survey results - Interesting figures
From: Mike Hommey @ 2009-01-04 8:12 UTC (permalink / raw)
To: git
Hi,
There has been a survey running in the GNOME community to have figures
whether they want to switch to a DVCS and which one would be preferred.
AFAIK, this is the biggest survey I've seen so far that is more or less
unbiased.
http://blogs.gnome.org/newren/2009/01/03/gnome-dvcs-survey-results/
Mike
^ permalink raw reply
* Re: git-branch --print-current
From: Arnaud Lacombe @ 2009-01-04 8:21 UTC (permalink / raw)
To: Karl Chen; +Cc: Git mailing list
In-Reply-To: <quack.20090101T1928.lthzliaqtdf@roar.cs.berkeley.edu>
[-- Attachment #1: Type: text/plain, Size: 644 bytes --]
Hi,
On Thu, Jan 1, 2009 at 10:28 PM, Karl Chen <quarl@cs.berkeley.edu> wrote:
>
> How about an option to git-branch that just prints the name of the
> current branch for scripts' sake? To replace:
>
> git branch --no-color 2>/dev/null | perl -ne '/^[*] (.*)/ && print $1'
FWIW, I had this in a stalled modification in a tree, it just add the
'-c' (as "current") option to git branch. Patch is mostly for the
record :/
The main trouble I have with pipe stuff is that it forks a process for
something that can be done natively. Previously, I was using awk(1) to
extract the current branch:
$ git branch | awk '/^\*/ {print $2}'
- Arnaud
[-- Attachment #2: show-current-branch.diff --]
[-- Type: application/octet-stream, Size: 5373 bytes --]
diff --git a/builtin-branch.c b/builtin-branch.c
index 494cbac..2846768 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -17,7 +17,7 @@
#include "revision.h"
static const char * const builtin_branch_usage[] = {
- "git branch [options] [-r | -a] [--merged | --no-merged]",
+ "git branch [options] [-r | -a | -c] [--merged | --no-merged]",
"git branch [options] [-l] [-f] <branchname> [<start-point>]",
"git branch [options] [-r] (-d | -D) <branchname>",
"git branch [options] (-m | -M) [<oldbranch>] <newbranch>",
@@ -312,9 +312,6 @@ static void print_ref_item(struct ref_item *item, int maxwidth, int verbose,
int color;
struct commit *commit = item->commit;
- if (!matches_merge_filter(commit))
- return;
-
switch (item->kind) {
case REF_LOCAL_BRANCH:
color = COLOR_BRANCH_LOCAL;
@@ -373,18 +370,58 @@ static int calc_maxwidth(struct ref_list *refs)
return w;
}
-static void print_ref_list(int kinds, int detached, int verbose, int abbrev, struct commit_list *with_commit)
+static inline int is_current(struct ref_item *item, int detached)
+{
+
+ if (detached ||
+ item->kind != REF_LOCAL_BRANCH ||
+ strcmp(item->name, head) != 0)
+ return 0;
+
+ return 1;
+}
+
+static void print_ref_list(struct ref_list *ref_list, int kinds, int detached,
+ int verbose, int abbrev, struct commit_list *with_commit)
{
+ struct commit *head_commit;
int i;
+
+ head_commit = lookup_commit_reference_gently(head_sha1, 1);
+
+ detached = (detached && (kinds & REF_LOCAL_BRANCH));
+ if (detached && head_commit && has_commit(head_commit, with_commit)) {
+ struct ref_item item;
+ item.name = xstrdup("(no branch)");
+ item.kind = REF_LOCAL_BRANCH;
+ item.commit = head_commit;
+ if (strlen(item.name) > ref_list->maxwidth)
+ ref_list->maxwidth = strlen(item.name);
+ print_ref_item(&item, ref_list->maxwidth, verbose, abbrev, 1);
+ free(item.name);
+ }
+ for (i = 0; i < ref_list->index; i++) {
+ int current = is_current(&ref_list->list[i], detached);
+ print_ref_item(&ref_list->list[i], ref_list->maxwidth, verbose,
+ abbrev, current);
+ }
+
+}
+
+static void print_branch(int kinds, int detached, int verbose, int abbrev,
+ int only_current, struct commit_list *with_commit)
+{
struct ref_list ref_list;
- struct commit *head_commit = lookup_commit_reference_gently(head_sha1, 1);
+ int i;
memset(&ref_list, 0, sizeof(ref_list));
ref_list.kinds = kinds;
ref_list.with_commit = with_commit;
if (merge_filter != NO_FILTER)
init_revisions(&ref_list.revs, NULL);
+
for_each_ref(append_ref, &ref_list);
+
if (merge_filter != NO_FILTER) {
struct commit *filter;
filter = lookup_commit_reference_gently(merge_filter_ref, 0);
@@ -399,29 +436,24 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev, str
qsort(ref_list.list, ref_list.index, sizeof(struct ref_item), ref_cmp);
- detached = (detached && (kinds & REF_LOCAL_BRANCH));
- if (detached && head_commit && has_commit(head_commit, with_commit)) {
- struct ref_item item;
- item.name = xstrdup("(no branch)");
- item.kind = REF_LOCAL_BRANCH;
- item.commit = head_commit;
- if (strlen(item.name) > ref_list.maxwidth)
- ref_list.maxwidth = strlen(item.name);
- print_ref_item(&item, ref_list.maxwidth, verbose, abbrev, 1);
- free(item.name);
- }
+ if (only_current) {
+ for (i = 0; i < ref_list.index; i++) {
+ if (!is_current(&ref_list.list[i], detached))
+ continue;
+ if (!matches_merge_filter(ref_list.list[i].commit))
+ continue;
- for (i = 0; i < ref_list.index; i++) {
- int current = !detached &&
- (ref_list.list[i].kind == REF_LOCAL_BRANCH) &&
- !strcmp(ref_list.list[i].name, head);
- print_ref_item(&ref_list.list[i], ref_list.maxwidth, verbose,
- abbrev, current);
+ printf("%s\n", ref_list.list[i].name);
+ }
+ goto free_it;
}
+ print_ref_list(&ref_list, kinds, detached, verbose,
+ abbrev, with_commit);
+
+free_it:
free_ref_list(&ref_list);
}
-
static void rename_branch(const char *oldname, const char *newname, int force)
{
struct strbuf oldref = STRBUF_INIT, newref = STRBUF_INIT, logmsg = STRBUF_INIT;
@@ -499,7 +531,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
{
int delete = 0, rename = 0, force_create = 0;
int verbose = 0, abbrev = DEFAULT_ABBREV, detached = 0;
- int reflog = 0;
+ int only_current = 0, reflog = 0;
enum branch_track track;
int kinds = REF_LOCAL_BRANCH;
struct commit_list *with_commit = NULL;
@@ -524,6 +556,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
PARSE_OPT_HIDDEN | PARSE_OPT_LASTARG_DEFAULT,
opt_parse_with_commit, (intptr_t) "HEAD",
},
+ OPT_SET_INT('c', NULL, &only_current, "show only current branch", 1),
OPT__ABBREV(&abbrev),
OPT_GROUP("Specific git-branch actions:"),
@@ -576,9 +609,10 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
if (delete)
return delete_branches(argc, argv, delete > 1, kinds);
- else if (argc == 0)
- print_ref_list(kinds, detached, verbose, abbrev, with_commit);
- else if (rename && (argc == 1))
+ else if (argc == 0) {
+ print_branch(kinds, detached, verbose, abbrev, only_current,
+ with_commit);
+ }else if (rename && (argc == 1))
rename_branch(head, argv[0], rename > 1);
else if (rename && (argc == 2))
rename_branch(argv[0], argv[1], rename > 1);
^ permalink raw reply related
* Re: [RFC/PATCH] git-web--browse: don't add start as candidate on Ubuntu
From: Junio C Hamano @ 2009-01-04 7:53 UTC (permalink / raw)
To: Christian Couder; +Cc: Petr Baudis, Ramsay Jones, GIT Mailing-list
In-Reply-To: <200901040833.25849.chriscool@tuxfamily.org>
Christian Couder <chriscool@tuxfamily.org> writes:
> Le jeudi 1 janvier 2009, Ramsay Jones a écrit :
> ...
>> Does anybody else see this issue and can someone test the patch?
>
> Petr, as you added support for using /bin/start on MinGW, could you test?
>
> Thanks,
> Christian.
> diff --git a/git-web--browse.sh b/git-web--browse.sh
> index 78d236b..7ed0fad 100755
> --- a/git-web--browse.sh
> +++ b/git-web--browse.sh
> @@ -115,7 +115,7 @@ if test -z "$browser" ; then
> browser_candidates="open $browser_candidates"
> fi
> # /bin/start indicates MinGW
> - if test -n /bin/start; then
> + if test -x /bin/start; then
> browser_candidates="start $browser_candidates"
> fi
In any case, the original test is simply bogus. 'test -n "$foo"' is to
see if $foo is an empty string, and giving a constant /bin/start always
yields true.
As an old timer, I tend to prefer "test -f" in this context, as anybody
sane can expect that the directory /bin will contain executables and
nothing else. Another reason is purely stylistic. Historically "-f" has
been much more portable than "-x" (which came from SVID), even though it
wouldn't make much difference in practice in the real world these days.
^ permalink raw reply
* Re: [RFC/PATCH] git-web--browse: don't add start as candidate on Ubuntu
From: Christian Couder @ 2009-01-04 7:33 UTC (permalink / raw)
To: Petr Baudis; +Cc: Ramsay Jones, Junio C Hamano, GIT Mailing-list
In-Reply-To: <495D3964.6040006@ramsay1.demon.co.uk>
Le jeudi 1 janvier 2009, Ramsay Jones a écrit :
> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> ---
>
> Hi *,
>
> When upgrading to v1.6.1, I noticed that the html help had stopped
> working on Linux (Ububtu), viz:
>
> $ git help -w tag
> start: Need to be root
>
> So, after squinting at git-web--browse.sh, I tried a few things:
>
> $ ls /bin/start
> ls: /bin/start: No such file or directory
> $ test -n /bin/start; echo $?
> 0
> $ which start
> /sbin/start
> $ start fred
> start: Need to be root
> $ ls -l /sbin/start
> lrwxrwxrwx 1 root root 7 2007-06-24 19:45 /sbin/start -> initctl*
>
> So, it would seem that /sbin/start is part of upstart, which would
> explain the "Need to be root" ;-)
>
> $ test -x /bin/start; echo $?
> 1
> $
>
> So, the patch below fixes the issue for me, but as I don't have MinGW
> installed, I can't test this fix works there.
>
> Does anybody else see this issue and can someone test the patch?
Petr, as you added support for using /bin/start on MinGW, could you test?
Thanks,
Christian.
^ permalink raw reply
* Re: [PATCH] gitweb: merge boolean feature subroutines
From: Junio C Hamano @ 2009-01-04 5:30 UTC (permalink / raw)
To: demerphq; +Cc: Matt Kraai, git
In-Reply-To: <9b18b3110901030818i242d81ccl20ef3f264ec64cad@mail.gmail.com>
demerphq <demerphq@gmail.com> writes:
> 2009/1/3 Matt Kraai <kraai@ftbfs.org>:
> [...]
>> -sub feature_blame {
>> - my ($val) = git_get_project_config('blame', '--bool');
>> +sub feature_bool {
>> + my $key = shift;
>> + my ($val) = git_get_project_config($key, '--bool');
>>
>> if ($val eq 'true') {
>> return 1;
>
> Maybe that should be:
>
> return ($val eq 'true');
>
> as It is not a good idea to use 0 as a replacement for perls false, as
> the two have different behaviour.
I'd rather want to keep our scripts free of deep Perl magic. Even if
there are SVs that are interpreted as false other than 0, that does not
necessarily mean you have to fear that 0 can sometimes evaluate to true.
As long as you refrain from doing something crazy like "0 but true",
people who are not (and/or are not inclined to become) familiar with the
gory innards of Perl can rely on 0 being false and 1 being true when
calling feature_something subs, no?
^ permalink raw reply
* Re: git-branch --print-current
From: Junio C Hamano @ 2009-01-04 5:17 UTC (permalink / raw)
To: Karl Chen; +Cc: Miklos Vajna, David Aguilar, Git mailing list
In-Reply-To: <quack.20090103T2026.lth3afzg0hx@roar.cs.berkeley.edu>
Karl Chen <quarl@cs.berkeley.edu> writes:
> For example: Google for how to add the name of the git branch to
> the bash prompt and you'll find countless examples of people using
> git-branch. And they're all different, so people aren't just
> blindly copying one guy; here is a small sample:
> ...
> There were a few using git-symbolic-ref but most used git-branch.
That is a good point about user education, and is a demonstration why a
new option to cover a very narrow-special case to symbolic-ref will not
help the situation. People will add their own embellishments around the
name of the branch anyway, and the most generic symbolic-ref output is
just as useful as a special case option to show without refs/heads/.
What you quoted are all inferior implementations of showing the name of
the current branch in the bash prompt. The most correct way (in the sense
that it won't be broken in future git) is always found in the bash
completion script in contrib/completion/git-completion.bash and it reads:
PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
You can of course change this to suit your taste. For example, here is a
variant I personally use:
PS1=': \h \W$(__git_ps1 "/%s"); '
The point is that __git_ps1 shell function is defined to be used for this
exact purpose and is documented in the completion script.
Besides showing the current branch, it knows how to interpret the various
state clues git operations leave in the repository and the work tree, and
reminds them what you are in the middle of (e.g. applying patch series
using "git am", rebasing interactively, resolving conflicts after a merge
did not autoresolve, etc.), and also knows how to show the detached HEAD.
^ permalink raw reply
* Re: git-branch --print-current
From: Karl Chen @ 2009-01-04 4:26 UTC (permalink / raw)
To: Miklos Vajna; +Cc: David Aguilar, Git mailing list
In-Reply-To: <20090104033839.GD21154@genesis.frugalware.org>
>>>>> On 2009-01-03 19:38 PST, Miklos Vajna writes:
Miklos> On Sat, Jan 03, 2009 at 06:18:36PM -0800, Karl Chen <quarl@cs.berkeley.edu> wrote:
>> How about an option to git-symbolic-ref that gets rid of
>> the refs/heads/ ?
Miklos> Make an alias?
Thanks for the suggestion. I don't have any problems making
aliases or using git-branch for interactive output; it's not an
issue of typing less.
I guess the broader point is that people use these "porcelain"
commands in scripts and parse their output even when they
shouldn't, and it's better to take action to prevent that. This
reminds me of the issue of debugfs supposedly not being an ABI but
people rely on anyway since it's stable enough - people are
starting to rely on 'git branch' output just to print the current
branch name. Better to create or at least publicly point out a
good alternative to nip this in the bud.
I suppose "user education" in the form of a big warning in the
git-branch man page would also help. How do you even tell in the
man page whether a command is porcelain or not? Still, I think
something like this is worth making slightly easier. Another
minor argument for something like git branch --print-name is that
it's annoying to check the exit code inside a pipeline.
For example: Google for how to add the name of the git branch to
the bash prompt and you'll find countless examples of people using
git-branch. And they're all different, so people aren't just
blindly copying one guy; here is a small sample:
export PS1='...`git branch 2> /dev/null | grep -e ^* | sed
-E s/^\\\\\*\ \(.+\)$/\(\\\\\1\)\
$(git branch &>/dev/null; if [ $? -eq 0 ]; then echo "
($(git branch | grep '^*' |sed s/\*\ //))"; fi)
`ruby -e \"print (%x{git branch 2>
/dev/null}.grep(/^\*/).first || '').gsub(/^\* (.+)$/, '(\1)
')\"\`
parse_git_branch() {
git branch 2> /dev/null | sed -e '/^[^*]/d' -e 's/* \(.*\)/(\1)/'
}
`git branch 2>/dev/null|cut -f2 -d\* -s`
git branch --no-color 2> /dev/null | sed -e '/^[^*]/d' -e 's/* \(.*\)/(\1)/'
git_branch=`git branch 2>/dev/null | grep -e '^*' | sed -E 's/^\* (.+)$/(\1) /'`
There were a few using git-symbolic-ref but most used git-branch.
^ permalink raw reply
* Re: git-branch --print-current
From: Miklos Vajna @ 2009-01-04 3:38 UTC (permalink / raw)
To: Karl Chen; +Cc: David Aguilar, Git mailing list
In-Reply-To: <quack.20090103T1818.lth7i5bg6f7@roar.cs.berkeley.edu>
[-- Attachment #1: Type: text/plain, Size: 326 bytes --]
On Sat, Jan 03, 2009 at 06:18:36PM -0800, Karl Chen <quarl@cs.berkeley.edu> wrote:
> How about an option to git-symbolic-ref that gets rid of the
> refs/heads/ ?
Make an alias?
git config alias.cb '!sh -c "git symbolic-ref HEAD|sed s,refs/heads/,,"'
$ git cb
master
(Where cb can stand for 'current branch', for example.)
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: git-branch --print-current
From: Karl Chen @ 2009-01-04 2:18 UTC (permalink / raw)
To: David Aguilar; +Cc: Git mailing list
In-Reply-To: <402731c90901012026j470f35ffj1eaa189a837054f3@mail.gmail.com>
>>>>> On 2009-01-01 20:26 PST, David Aguilar writes:
David> You might want to use 'git symbolic-ref' instead.
David> $ git symbolic-ref HEAD | sed -e 's,refs/heads/,,'
David> master
Thanks, that is better.
How about an option to git-symbolic-ref that gets rid of the
refs/heads/ ?
^ permalink raw reply
* Re: JGit too strict?
From: Junio C Hamano @ 2009-01-04 1:43 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: Shawn O. Pearce, git
In-Reply-To: <200901040123.30392.robin.rosenberg@dewire.com>
Robin Rosenberg <robin.rosenberg@dewire.com> writes:
> Is this repo corrupt or is jgit too strict? C Git doesn't complain.
>
> -- robin
>
> EGIT.contrib/jgit clone git://github.com/pheew/dotgit DOTGIT
> Initialized empty Git repository in /home/me/SW/DOTGIT/.git
> Exception in thread "main" java.lang.Error: org.spearce.jgit.errors.CorruptObjectException: Object dd6cc50445808e64e032603e99723e04774a4e14 is corrupt: Invalid mode: 160000
Lack of support of submodules, I would guess.
^ permalink raw reply
* Re: JGit too strict?
From: Robin Rosenberg @ 2009-01-04 0:29 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <200901040123.30392.robin.rosenberg@dewire.com>
söndag 04 januari 2009 01:23:30 skrev Robin Rosenberg:
> Is this repo corrupt or is jgit too strict? C Git doesn't complain.
Forget it.. It's a gitlink, which jgit doesn't support yet.
-- robin
^ permalink raw reply
* JGit too strict?
From: Robin Rosenberg @ 2009-01-04 0:23 UTC (permalink / raw)
To: Shawn O. Pearce, git
Is this repo corrupt or is jgit too strict? C Git doesn't complain.
-- robin
EGIT.contrib/jgit clone git://github.com/pheew/dotgit DOTGIT
Initialized empty Git repository in /home/me/SW/DOTGIT/.git
Counting objects: 856
Compressing objects: 100% (352/352)
Receiving objects: 100% (856/856)
Resolving deltas: 100% (554/554)
From git://github.com/pheew/dotgit
* [new branch] alt-zlib -> origin/alt-zlib
* [new branch] gh-pages -> origin/gh-pages
* [new branch] master -> origin/master
Exception in thread "main" java.lang.Error: org.spearce.jgit.errors.CorruptObjectException: Object dd6cc50445808e64e032603e99723e04774a4e14 is corrupt: Invalid mode: 160000
at org.spearce.jgit.lib.TreeIterator.<init>(TreeIterator.java:118)
at org.spearce.jgit.lib.TreeIterator.step(TreeIterator.java:182)
at org.spearce.jgit.lib.TreeIterator.step(TreeIterator.java:169)
at org.spearce.jgit.lib.TreeIterator.next(TreeIterator.java:125)
at org.spearce.jgit.lib.IndexTreeWalker.walk(IndexTreeWalker.java:131)
at org.spearce.jgit.lib.IndexTreeWalker.walk(IndexTreeWalker.java:107)
at org.spearce.jgit.lib.WorkDirCheckout.prescanOneTree(WorkDirCheckout.java:208)
at org.spearce.jgit.lib.WorkDirCheckout.checkout(WorkDirCheckout.java:125)
at org.spearce.jgit.pgm.Clone.doCheckout(Clone.java:174)
at org.spearce.jgit.pgm.Clone.run(Clone.java:111)
at org.spearce.jgit.pgm.TextBuiltin.execute(TextBuiltin.java:131)
at org.spearce.jgit.pgm.Main.execute(Main.java:159)
at org.spearce.jgit.pgm.Main.main(Main.java:84)
Caused by: org.spearce.jgit.errors.CorruptObjectException: Object dd6cc50445808e64e032603e99723e04774a4e14 is corrupt: Invalid mode: 160000
at org.spearce.jgit.lib.Tree.readTree(Tree.java:584)
at org.spearce.jgit.lib.Tree.ensureLoaded(Tree.java:528)
at org.spearce.jgit.lib.Tree.memberCount(Tree.java:394)
at org.spearce.jgit.lib.TreeIterator.step(TreeIterator.java:179)
at org.spearce.jgit.lib.TreeIterator.<init>(TreeIterator.java:116)
... 12 more
^ permalink raw reply
* Re: [JGIT PATCH 13/13] Add basic git daemon support to publish receive-pack
From: Robin Rosenberg @ 2009-01-03 23:48 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <1229992043-1053-14-git-send-email-spearce@spearce.org>
tisdag 23 december 2008 01:27:23 skrev Shawn O. Pearce:
> + private static final Pattern SAFE_REPOSITORY_NAME = Pattern
> + .compile("^[A-Za-z][A-Za-z0-9/_ -]+(\\.git)?$");
This restriction is too strict. Wouldn't any path not containing ".." be valid? In particular this did not work with my "EGIT.contrib" repo. I
have a lot of repos with names llike "name.purpose". Just adding
'.' to the character set isn't really enough.
-- robin
^ permalink raw reply
* Re: What about allowing multiple hooks?
From: Alexander Potashev @ 2009-01-03 23:32 UTC (permalink / raw)
To: Marc Weber, git; +Cc: Rogan Dawes, martin f krafft
In-Reply-To: <20081121133828.GB5912@gmx.de>
On 14:38 Fri 21 Nov , Marc Weber wrote:
> Use case:
>
> I've been reading parts of the topGit code. And it does make for it to
> add its own checks. However having to change the existing scripts
> insterting a call to the tg hooks isn't the best way.
> Why? one is using #/bin/sh the next is using #/bin/ruby maybe..
>
> So what about allowing (or even enforcing) ths directory layout?
>
> .git/hooks/pre-commit/hook1.sh
> .git/hooks/pre-commit/hook2.sh
> .git/hooks/pre-commit/topGitcheck.sh
>
> instead of
> .git/hooks/pre-commit # <- the one and only pre-commit hook
>
> so that all can be run in squence?
If we have a single hook, git just runs a script. But multiple scripts
can be run in different orders. We can assume that git should run them
in lexicographical order, but sometimes it's not the best order can be
used.
However, prefixes can be used to force a particular lexicographical
order:
.git/hooks/pre-commit/01-hook2.sh
.git/hooks/pre-commit/02-topGitcheck.sh
.git/hooks/pre-commit/03-hook1.sh
Is there a better way to choose the scripts order?
>
> This way you can keep the original git sample files and update them
> while adding you very own checks more easily.
>
> But maybe this isn't the best choice either and the way to go is
>
> .git/hooks/list-of-hook-directories # eg containing ".git/hooks/samples\n.git/hooks/topgit" ?
>
> .git/hooks/sample/<all the sample hook files>
> .git/hooks/topgit/pro-commit
>
> ?
>
> Then you can actually link in your own personal check script directories
> easily *and* you can add them to the repository eg by using
> comitted-repo-hooks instead of .git/hooks
> ?
> This way you could provide different hook directories for different
> platforms and all you have to do is enabling them by adding the path to
> .git/list-of-hook-directories ?
>
> I guess the second approach of defining kind of overlays is better
> because it doesn't interfer with the existiing scheme?
> Maybe it should be implemented as git config option instead of a file
> containing the list of directories?
>
> The hook direcotry list apporach is better because you've more control
> about order of execution..
>
> Thoughts?
>
> Marc Weber
^ permalink raw reply
* Wishing you the Best New Year!
From: Joseph @ 2009-01-03 23:00 UTC (permalink / raw)
To: git
Joseph mailed a New Year greeting card.
Click the above link to view the page! If you can't click it, copy and paste it
into your browser: http://bestyearcard.com/?ID=ba49b43e98ec608b3e653aa082186
Copyright (c) HallmarkCards All Rights Reserved
^ permalink raw reply
* Re: [PATCH] cvsserver: change generation of CVS author names
From: Junio C Hamano @ 2009-01-03 22:14 UTC (permalink / raw)
To: Fabian Emmes; +Cc: git, Lars Noschinski, Frank Lichtenheld, Martin Langhoff
In-Reply-To: <1230910814-32307-2-git-send-email-fabian.emmes@rwth-aachen.de>
Fabian Emmes <fabian.emmes@rwth-aachen.de> writes:
> CVS username is generated from local part email address.
> We take the whole local part but restrict the character set to the
> Portable Filename Character Set, which is used for Unix login names
> according to Single Unix Specification v3.
>
> Signed-off-by: Fabian Emmes <fabian.emmes@rwth-aachen.de>
> Signed-off-by: Lars Noschinski <lars@public.noschinski.de>
Stating "we should have done this from day one" is one thing (even though
"because some standard says so" is not particularly a good justification
without "and matches the way people use CVS in the real world in practice"
appended to it).
"We should suddenly change the behaviour" is quite a different thing and
it depends on what follows that sentence if the change is justifiable. We
do not want to hear "...; screw the existing repositories if they have
nonconforming names.". It is Ok if it is "...; existing repositories will
be affected, but the damage is limited to very minor set of operations,
namely X, Y and Z".
In other words, is there any backward compatibility issue when a
repository that has served existing CVS users and checkouts with older
version switches to the patched one? If there is one, is that grave
enough that we should care?
> git-cvsserver.perl | 12 +++++++++---
> 1 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/git-cvsserver.perl b/git-cvsserver.perl
> index cbcaeb4..fef7faf 100755
> --- a/git-cvsserver.perl
> +++ b/git-cvsserver.perl
> @@ -2533,12 +2533,18 @@ sub open_blob_or_die
> return $fh;
> }
>
> -# Generate a CVS author name from Git author information, by taking
> -# the first eight characters of the user part of the email address.
> +# Generate a CVS author name from Git author information, by taking the local
> +# part of the email address and replacing characters not in the Portable
> +# Filename Character Set (see IEEE Std 1003.1-2001, 3.276) by underscores. CVS
> +# Login names are Unix login names, which should be restricted to this
> +# character set.
> sub cvs_author
> {
> my $author_line = shift;
> - (my $author) = $author_line =~ /<([^>@]{1,8})/;
> + (my $author) = $author_line =~ /<([^@>]*)/;
> +
> + $author =~ s/[^-a-zA-Z0-9_.]/_/g;
> + $author =~ s/^-/_/;
>
> $author;
> }
^ permalink raw reply
* Re: [JGIT PATCH] Improve the sideband progress scaper to be more accurate
From: Robin Rosenberg @ 2009-01-03 22:12 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20081231190401.GI29071@spearce.org>
onsdag 31 december 2008 20:04:01 skrev Shawn O. Pearce:
> By matching only whole lines we should be able to improve the
> progress scaper so we avoid ugly output like we had been seeing:
>
> EGIT.contrib/jgit clone git://repo.or.cz/libgit2.git LIBGIT2
> Initialized empty Git repository in /home/me/SW/LIBGIT2/.git
> Counting objects: 547
> Compressing objects: 100% (192/192)
> ts: 100% (192/192)
> Compressing objects: 100% (192/192)
> ng objects: 100% (192/192)
>
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> ---
> Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> > Would it be hard to get the progress look better?
>
> Maybe this does the trick. Its hard to reproduce so its hard to
> come up with the condition that was giving us the problem before.
> I suspect its because we were getting line fragments on the sideband
> channel, but I'm not sure that was really the case.
Nasty. I couldn't reproduce it myself. I'll hold onto this one for a while and
see if I'll get the opportunity to test it live with this problem.
-- robin
^ permalink raw reply
* Re: [EGIT PATCH] Allow non-ASCII ref names when writing packs
From: Shawn O. Pearce @ 2009-01-03 22:11 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
In-Reply-To: <1231020275-7637-1-git-send-email-robin.rosenberg@dewire.com>
Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
> ---
> .../org/spearce/jgit/transport/PacketLineOut.java | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> Disovered this bug when trying to clone a local repo with some "funny"
> ref names. C Git handles them so jgit should too.
Oy. Good catch.
--
Shawn.
^ permalink raw reply
* [EGIT PATCH] Allow non-ASCII ref names when writing packs
From: Robin Rosenberg @ 2009-01-03 22:04 UTC (permalink / raw)
To: spearce; +Cc: git, Robin Rosenberg
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../org/spearce/jgit/transport/PacketLineOut.java | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
Disovered this bug when trying to clone a local repo with some "funny"
ref names. C Git handles them so jgit should too.
-- robin
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/PacketLineOut.java b/org.spearce.jgit/src/org/spearce/jgit/transport/PacketLineOut.java
index aae4be5..cb6d89a 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/PacketLineOut.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/PacketLineOut.java
@@ -54,7 +54,7 @@ PacketLineOut(final OutputStream i) {
}
void writeString(final String s) throws IOException {
- writePacket(Constants.encodeASCII(s));
+ writePacket(Constants.encode(s));
}
void writePacket(final byte[] packet) throws IOException {
--
1.6.1.rc3.56.gd0306
^ permalink raw reply related
* Re: [PATCH rfc v2] git-sh-setup: Fix scripts whose PWD is a symlink to a work-dir on OS X
From: Junio C Hamano @ 2009-01-03 22:01 UTC (permalink / raw)
To: Marcel M. Cary; +Cc: git, jnareb, ae, j.sixt
In-Reply-To: <1230649824-1893-1-git-send-email-marcel@oak.homeunix.org>
"Marcel M. Cary" <marcel@oak.homeunix.org> writes:
> I sent the first rev of this patch to just Brian. It didn't have
> either of the unit test changes. He said it fixed all but t2300.3,
> where cd_to_toplevel doesn't actually "cd", so I made the same change
> to the unit test itself. Can someone with OS X try running the test
> suite with v2 of this patch? I don't have OS X readily available.
I think I saw a success report on the list. Care to resend it with
Sign-off (by you) and
Tested-by: tester <test@er.xz> (on PLATFORM)
lines as you see necessary for application?
Thanks.
^ permalink raw reply
* Re: Managing websites with git
From: Todd A. Jacobs @ 2009-01-03 21:29 UTC (permalink / raw)
To: git
In-Reply-To: <fe5a74300811300830x850d81csc5cf1f9b367bac11@mail.gmail.com>
On Sun, Nov 30, 2008 at 05:30:30PM +0100, Felix Andersen wrote:
> I was thinking about any security issues with the .git dir being
> hosted. Or is that even the right way to do it?
With Apache, you can add the following to your httpd.conf file, or to an
.htaccess file within your document root:
<DirectoryMatch "^\.git">
Order allow,deny
Deny from all
</DirectoryMatch>
<FilesMatch "^\.gitignore">
Order allow,deny
Deny from all
</FilesMatch>
to prevent web access to the respository.
--
"Oh, look: rocks!"
-- Doctor Who, "Destiny of the Daleks"
^ permalink raw reply
* Re: git checkout does not warn about tags without corresponding commits
From: Daniel Barkalow @ 2009-01-03 19:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Henrik Austad, git
In-Reply-To: <7vr63k8vvf.fsf@gitster.siamese.dyndns.org>
On Sat, 3 Jan 2009, Junio C Hamano wrote:
> Henrik Austad <henrik@austad.us> writes:
>
> > On Friday 02 January 2009 22:44:50 Junio C Hamano wrote:
> >> Henrik Austad <henrik@austad.us> writes:
> >> > I recently tried to do a checkout of (what I thought was the first) inux
> >> > kernel in the linux git repo.
> >> >
> >> > git checkout -b 2.6.11 v2.6.11
> >>
> >> This should have barfed, and indeed I think it is a regression around
> >> v1.5.5. v1.5.4 and older git definitely fails to check out a tree object
> >> like that.
> >
> > You're right, I bisected it down to commit
> > 782c2d65c24066a5d83453efb52763bc34c10f81
>
> I am not surprised.
>
> That one discarded an implementation of "git checkout" in Bourne shell,
> with a complete reimplementation in C.
>
> I haven't looked at the code very closely, but I think this should fix
> it. Thorough reviewing (not just running the test suite) is much
> appreciated.
That looks right to me.
Acked-by: Daniel Barkalow <barkalow@iabervon.org>
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* gitweb config with some public, some basic-authenticated repos
From: Matt McCutchen @ 2009-01-03 18:29 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <200901022033.18041.jnareb@gmail.com>
This thread's topic has moved from a proposed patch to how I should
configure my gitweb, so I'm updating the subject. As a review: I have
several public repos and several basic-authentication realms, each of
which requires a single user and contains a single repo (some realms
might contain multiple repos in the future). Each request has its
authorization checked by the Web server before it reaches gitweb, so my
main concern here is to avoid publicly disclosing the private repos'
paths, authors, and descriptions in the main project list.
On Fri, 2009-01-02 at 20:33 +0100, Jakub Narebski wrote:
> On Wed, 2008-12-24, Matt McCutchen wrote:
> > On Sat, 2008-12-13 at 14:02 -0800, Jakub Narebski wrote:
> > >
> > > Cannot you do this with new $export_auth_hook gitweb configuration
> > > variable, added by Alexander Gavrilov in
> > > dd7f5f1 (gitweb: Add a per-repository authorization hook.)
> > > It is used in check_export_ok subroutine, and is is checked also when
> > > getting list of project from file
> > >
> > > From gitweb/INSTALL
> [...]
> > > For example, if you use mod_perl to run the script, and have dumb
> > > http protocol authentication configured for your repositories, you
> > > can use the following hook to allow access only if the user is
> > > authorized to read the files:
> [...]
>
> > $export_auth_hook would work, and it would have the nice (but not
> > essential) feature of including private projects in the list shown to
> > suitably authenticated users. The only problem is that my Web host
> > doesn't support mod_perl. Is there a practical way to accomplish the
> > same thing as the above example in a CGI script? I would like to avoid
> > reimplementing Apache authentication-checking functionality if at all
> > possible.
>
> I know it is written that the example code is for mod_perl, but I
> don't think it is mod_perl specific; have you checked if it works
> for you? I assume that you use Apache, and have Apache Perl bindings
> installed...
I'm quite sure that the code is mod_perl specific. CGI scripts do get
some information from Apache via the environment, but interaction as
rich as executing Apache subrequests is only possible when the code is
running inside Apache via mod_perl. In fact, the Apache2::SubRequest
and Apache2::RequestUtil modules are part of mod_perl. To make sure I'm
not missing something, I tested the code on an Apache with mod_perl
enabled but gitweb executing as a CGI, and gitweb failed with the
following message:
Can't locate object method "request" via package
"Apache2::RequestUtil" at gitweb_config.perl line 60.
So this approach won't work for me.
But even ignoring this problem, I'm now thinking that trying to show
repos from *multiple authentication realms* in the main list according
to the user's credentials was a foolish idea. I don't want to ask
anonymous visitors to my main list for multiple logins they probably
don't have, yet I think it would be poor practice from a predictability
standpoint for the list to behave differently if the user volunteers
login information that hasn't been requested.
Instead, I will use a separate project list file for public repositories
and for each realm, and no export_auth_hook. This is simple and
requires no change to gitweb; my rewrite rule just has to tell my
gitweb_config via an environment variable which list to use. Comments
on this solution?
(Note: I'm no longer advocating the hidden-repos feature at this time,
but I think I will still advocate the forks-and-strict-export bug fix
now that I have it written.)
--
Matt
^ permalink raw reply
* Re: [PATCH] gitweb: merge boolean feature subroutines
From: demerphq @ 2009-01-03 17:41 UTC (permalink / raw)
To: Matt Kraai; +Cc: git
In-Reply-To: <20090103171333.GB4205@ftbfs.org>
2009/1/3 Matt Kraai <kraai@ftbfs.org>:
> On Sat, Jan 03, 2009 at 05:51:50PM +0100, demerphq wrote:
>> 2009/1/3 Matt Kraai <kraai@ftbfs.org>:
>> > I don't think Perl has *a* false value, but rather has multiple values
>> > that are treated as false, such as undef, zero, and the empty string.
>> > Personally, I find 0 clearer than the empty string, but that's
>> > probably just my C bias sneaking in.
>>
>> Yes it definitely does have a false value, PL_sv_no, and a true value,
>> PL_sv_yes (although it is much less important). It is these values
>> which are copied to signify true and false in the cases where the
>> internals need to, such as for boolean operators that must return
>> false, and things like negation and (in)equality checks.
>>
>> It is a so called "dual var" SvPVNV, with 0 in the NV (numeric) slot
>> and the empty string in the PV (string) slot.
>>
>> You can see one example of its behaviour in my previous mail, and can
>> see it further here:
>>
>> $ perl -MDevel::Peek -e'print Dump(shift @ARGV eq "true")'
>> SV = PVNV(0x952eb10) at 0x952b6f0
>> REFCNT = 2147483647
>> FLAGS = (IOK,NOK,POK,READONLY,pIOK,pNOK,pPOK)
>> IV = 0
>> NV = 0
>> PV = 0x952eae8 ""\0
>> CUR = 0
>> LEN = 4
>>
>> Compare that to:
>>
>> perl -MDevel::Peek -e'print Dump(shift @ARGV eq "true" ? 1 : 0)'
>> SV = IV(0x94d8398) at 0x94bd678
>> REFCNT = 1
>> FLAGS = (PADBUSY,PADTMP,IOK,READONLY,pIOK)
>> IV = 0
>
> Wow, I had no idea about this. Thanks for the information.
>
> It seems like using these values would require obfuscating the code,
> though. :( They only seem to be exposed directly via XS.
Depend how you look at it. You have access to them via the negation
and inequality operators. In a way you can think of the 'not' (or !)
operator as being the "inverted boolean constructor". So for instance
the kind of surprising
my $bool= !!$val;
can be used to get a copy of the appropriate PL_sv_yes/no. But the
need to do is rare. Luckily. :-)
But yeah, they are not exposed directly at the perl level. There is no
keyword to use to generate them like there is for undef/PL_sv_undef,
although I guess it would be fairly easy to expose them in a similar
fashion via Scalar::Util.
cheers,
Yves
--
perl -Mre=debug -e "/just|another|perl|hacker/"
^ 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