* 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 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
* 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: 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: 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 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: 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: [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: [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: [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: 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
* 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: [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
* Re: [PATCH 3/3] pretty: support multiline subjects with format:
From: Junio C Hamano @ 2009-01-04 10:01 UTC (permalink / raw)
To: René Scharfe; +Cc: markus.heidelberg, git
In-Reply-To: <49594C16.2010406@lsrfire.ath.cx>
René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
>> It's unchanged since it has it's own commit message parser.
>
> ... which displays the first line of the commit message, unlike
> --pretty=oneline. Here's a quick fix. I probably won't have time
> to come up with something prettier this year.
I think the code is pretty enough ;-) I just need to come up with a patch
description and forge your Sign-off.
Thanks.
^ permalink raw reply
* Re: [PATCH] cvsserver: add option to configure commit message
From: Junio C Hamano @ 2009-01-04 10:01 UTC (permalink / raw)
To: Fabian Emmes; +Cc: git, gitster, Lars Noschinski
In-Reply-To: <1230910814-32307-1-git-send-email-fabian.emmes@rwth-aachen.de>
Fabian Emmes <fabian.emmes@rwth-aachen.de> writes:
> cvsserver annotates each commit message by "via git-CVS emulator". This is
> made configurable via gitcvs.commitmsgannotation.
>
> Signed-off-by: Fabian Emmes <fabian.emmes@rwth-aachen.de>
> Signed-off-by: Lars Noschinski <lars@public.noschinski.de>
I do not see the development history behind this and am somewhat puzzled
by these two S-o-b lines. Is it "Fabian developed it, showed it to Lars
who cleaned it up and/or enhanced it and here is the result"? Or is it
"Lars developed it, circulated it in his closer circle, Fabian found it
useful and worthy for inclusion and sending it to the mailing list"?
Whichever it is, I just will take it as "This is co-developed and between
the authors Fabian is the primary author" and apply.
Thanks.
^ permalink raw reply
* Re: What about allowing multiple hooks?
From: Junio C Hamano @ 2009-01-04 10:01 UTC (permalink / raw)
To: Alexander Potashev; +Cc: Marc Weber, git, Rogan Dawes, martin f krafft
In-Reply-To: <20090103233252.GA12095@myhost>
Alexander Potashev <aspotashev@gmail.com> writes:
>> Thoughts?
I deliberately omitted support for multiple scripts in core git Porcelains
to avoid this exact issue. It is a huge can of worms and it is dubious if
you can have a coherent and generic enough semantics.
In the meantime, you can have a single .git/hooks/pre-commit script that
defines your own convention. Maybe it uses .git/hooks/pre-commit.d/
directory, full of scripts, and implements the semantics you want,
including:
(1) the execution order and the naming convention of the scripts (e.g.
they all live in pre-commit.d/ directory, and executed in ASCII byte
value order of their names);
(2) how their exit status combine together. For example, maybe a failure
from one of the scripts prevents none of the later scripts to even
run and make the whole hook return a failure; maybe a failure will be
remembered, but the other scripts may still want to be run to learn
about the fact that the commit was attempted, and the whole hook
returns a failure if any of them fail.
In a hook that is run primarily for its side effects and not for
validation, it may even be desireble if the whole hook returns a
failure only when all of them fail, iow, for such a hook the status
is not ANDed but ORed together.
Once you have such a framework and get help from others to widely try it
in the field, it may prove generic enough to include it as the sample hook
script to be installed everywhere.
^ permalink raw reply
* Re: git-branch --print-current
From: Alexandre Dulaunoy @ 2009-01-04 10:07 UTC (permalink / raw)
To: Git mailing list
In-Reply-To: <quack.20090101T1928.lthzliaqtdf@roar.cs.berkeley.edu>
On Fri, Jan 2, 2009 at 4:28 AM, 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'
I tend to support your request especially that extracting the current
branch is something that is done regularly. Looking in my own scripts/aliases
and some of my colleagues, there are plenty of variation using Perl,
sed, awk, tr
and Python to extract the current branch.
Using git-symbolic-ref is not obvious, especially that the summary/name
of the man page is :
"git-symbolic-ref - Read and modify symbolic refs"
But the description is pretty clear :
"Given one argument, reads which branch head the given symbolic ref refers to
and outputs its path, relative to the .git/ directory. Typically you
would give HEAD
as the <name> argument to see on which branch your working tree is on."
But naturally, as a lazy user, you will pick git-branch especially
that's the tools is listed
with the most commonly used git commands with a very attractive description :
"branch List, create, or delete branches"
On an user perspective, having the option in git-branch seems more natural.
Just a comment,
--
-- Alexandre Dulaunoy (adulau) -- http://www.foo.be/
-- http://www.foo.be/cgi-bin/wiki.pl/Diary
-- "Knowledge can create problems, it is not through ignorance
-- that we can solve them" Isaac Asimov
^ permalink raw reply
* Re: [PATCH] cvsserver: change generation of CVS author names
From: Lars Noschinski @ 2009-01-04 11:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Fabian Emmes, git, Frank Lichtenheld, Martin Langhoff
In-Reply-To: <7vwsdc3ulg.fsf@gitster.siamese.dyndns.org>
* Junio C Hamano <gitster@pobox.com> [09-01-03 23:36]:
>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).
Documentation about valid cvs/rcs usernames is a bit scarce. When we
wrote the patch, we did not find much more information than "the cvs
username is supposed to be the login name". In my limited CVS
experience, I never saw CVS user names which were not (unix) login
names.
After this mail, I looked to the RCS source code (see checkidentifier()
in rcslex.c) which tells us that anything (encoded in ISO-8859-1)
consisting of IDCHAR, LETTER, Letter, DIGIT and PERIOD, containing at
least one IDCHAR, LETTER or Letter is a valid username (for the
character classes, see
http://avalon.hoffentlich.net/~cebewee/rcs-charmap.txt) The most
important character _not_ allowed in an user name is the @ sign, so we
cannot use the full mail address.
So our patch generates a valid username for any "sane" local part. In a
few corner cases like "!#$%&'*+-/=?^_`.{|}~@example.com" our patch
generates a result worse than the original - an empty username. This
is probably something we should fix.
Obviously, the short names generated are not necessarily unique, which
can be irritating, but is not a problem from a technical point of view.
Improving this would probably require to store a map of mail addresses
to cvs user names.
>"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?
Obviously the reported user names change. To the best of my knowledge
(but I'm just a barely experienced CVS user) those names are not stored
anywhere on the client and are regenerated by git-cvsserver for every
request, so even old repositories get the new names for all commits.
- Lars.
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2008, #04; Mon, 29)
From: Christian Couder @ 2009-01-04 11:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vhc4gadg9.fsf@gitster.siamese.dyndns.org>
Le samedi 3 janvier 2009, Junio C Hamano a écrit :
> Christian Couder <chriscool@tuxfamily.org> writes:
> > Le mardi 30 décembre 2008, Junio C Hamano a écrit :
> >> * cc/bisect-replace (Mon Nov 24 22:20:30 2008 +0100) 9 commits
> >> - bisect: add "--no-replace" option to bisect without using replace
> >> refs
> >> - rev-list: make it possible to disable replacing using "--no-
> >> bisect-replace"
> >> - bisect: use "--bisect-replace" options when checking merge bases
> >> - merge-base: add "--bisect-replace" option to use fixed up revs
> >> - commit: add "bisect_replace_all" prototype to "commit.h"
> >> - rev-list: add "--bisect-replace" to list revisions with fixed up
> >> history
> >> - Documentation: add "git bisect replace" documentation
> >> - bisect: add test cases for "git bisect replace"
> >> - bisect: add "git bisect replace" subcommand
> >>
> >> I think a mechanism like this should be added to replace grafts,
> >
> > The problem with replacing grafts is that a graft can specify many
> > parents for one commit while a ref associates only one object to a
> > name.
>
> Sorry, maybe I misunderstood your implementation. What I thought we
> discussed during GitTogether was to write out the object name of the
> replacement object in refs/replace/<sha1>.
>
> When the caller asks read_sha1_file() for an object whose object name is
> <sha1>, you see if there is refs/replace/<sha1> in the repository, and
> read the ref to learn the object name of the object that replaces it.
> And you return that as if it is the original object.
Ok. When I first implemented "bisect replace" I saw that I could reuse the
graft fix-up mechanism. And as you talked about replacing grafts, I thought
that you wanted the implementation to use that mechanism instead of adding
a different one.
But I agree that it may be more powerfull and generic to replace objects the
way you describe it. So I will work on that.
Thanks,
Christian.
^ permalink raw reply
* Re: [PATCH] cvsserver: add option to configure commit message
From: Lars Noschinski @ 2009-01-04 11:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Fabian Emmes, git
In-Reply-To: <7viqovz8y0.fsf@gitster.siamese.dyndns.org>
* Junio C Hamano <gitster@pobox.com> [09-01-04 12:13]:
>Fabian Emmes <fabian.emmes@rwth-aachen.de> writes:
>
>> cvsserver annotates each commit message by "via git-CVS emulator". This is
>> made configurable via gitcvs.commitmsgannotation.
>>
>> Signed-off-by: Fabian Emmes <fabian.emmes@rwth-aachen.de>
>> Signed-off-by: Lars Noschinski <lars@public.noschinski.de>
>
>I do not see the development history behind this and am somewhat puzzled
>by these two S-o-b lines. Is it "Fabian developed it, showed it to Lars
>who cleaned it up and/or enhanced it and here is the result"? Or is it
>"Lars developed it, circulated it in his closer circle, Fabian found it
>useful and worthy for inclusion and sending it to the mailing list"?
It is "Fabian and Lars developed it and Fabian is the one who mailed it
for inclusion". We could just leave off the second S-o-b line, if this
is less irritating?
>Whichever it is, I just will take it as "This is co-developed and between
>the authors Fabian is the primary author" and apply.
Fine with me.
- Lars.
^ permalink raw reply
* Re: [PATCH] gitweb: merge boolean feature subroutines
From: demerphq @ 2009-01-04 11:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Matt Kraai, git
In-Reply-To: <7vvdsv3af6.fsf@gitster.siamese.dyndns.org>
2009/1/4 Junio C Hamano <gitster@pobox.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.
No, thats not the point. The point is that why should the code do more
work to return a value that a perl programmer might find unexpected.
Especially when the function has the word "bool" in it. Its like
writing a function whose name says "int" that returns a double. If the
routine was not called "bool" then it wouldnt matter at all.
> 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?
Why execute more opcodes, and return a slightly surprising false, when
you dont have to?
Is it really deep perl magic to do:
return $val eq 'true';
instead of
return $val eq 'true' ? 1 : 0;
or the actual use:
if ($val eq 'true') {
return 1
} else {
return 0
}
Isn't the former superior just on pure minimalism metrics? Theres less
code to understand, less code to go wrong, and as a bonus it returns a
true boolean. Isn't that just a win-win-win? I mean most perl
programmers I know would instantly convert the latter two to the first
just on the grounds that the first version is the clearest expression
of the desired intent.
Anyway, leave it or not, its a minor nit.
cheers,
Yves
--
perl -Mre=debug -e "/just|another|perl|hacker/"
^ permalink raw reply
* Re: [PATCH] strbuf_readlink semantics update.
From: Pierre Habouzit @ 2009-01-04 12:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: René Scharfe, Linus Torvalds, git
In-Reply-To: <7viqp8afap.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 1520 bytes --]
On Thu, Dec 25, 2008 at 07:23:58AM +0000, Junio C Hamano wrote:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
>
> > Pierre Habouzit schrieb:
> >> On Tue, Dec 23, 2008 at 06:16:01PM +0000, Linus Torvalds wrote:
> >>>
> >>> On Tue, 23 Dec 2008, Pierre Habouzit wrote:
> >>>> when readlink fails, the strbuf shall not be destroyed. It's not how
> >>>> read_file_or_gitlink works for example.
> >>> I disagree.
> >>>
> >>> This patch just makes things worse. Just leave the "strbuf_release()" in
> >>> _one_ place.
> > ...
> > The "append or do nothing" rule is broken by strbuf_getline(), but I agree
> > to your reasoning. How about refining this rule a bit to "do your thing
> > and roll back changes if an error occurs"? I think it's not worth to undo
> > allocation extensions, but making reverting first time allocations seems
> > like a good idea. Something like this?
>
> I think this is much better than Pierre's.
I agree it's a fine semantics.
> Pierre's "if it is called strbuf_*, it should always append" is a good
> uniformity to have in an API, but making the caller suffer for
> clean-up is going backwards. The reason we use strbuf when we can is
> so that the callers do not have to worry about memory allocation
> issues too much.
Ack.
Sorry for the delay I was on vacation.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: git-branch --print-current
From: demerphq @ 2009-01-04 12:31 UTC (permalink / raw)
To: Alexandre Dulaunoy; +Cc: Git mailing list
In-Reply-To: <1baa801f0901040207r64195594m64359dbc60a5f662@mail.gmail.com>
2009/1/4 Alexandre Dulaunoy <adulau@gmail.com>:
> On Fri, Jan 2, 2009 at 4:28 AM, 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'
>
> I tend to support your request especially that extracting the current
> branch is something that is done regularly. Looking in my own scripts/aliases
> and some of my colleagues, there are plenty of variation using Perl,
> sed, awk, tr
> and Python to extract the current branch.
>
> Using git-symbolic-ref is not obvious, especially that the summary/name
> of the man page is :
>
> "git-symbolic-ref - Read and modify symbolic refs"
>
> But the description is pretty clear :
>
> "Given one argument, reads which branch head the given symbolic ref refers to
> and outputs its path, relative to the .git/ directory. Typically you
> would give HEAD
> as the <name> argument to see on which branch your working tree is on."
>
> But naturally, as a lazy user, you will pick git-branch especially
> that's the tools is listed
> with the most commonly used git commands with a very attractive description :
I dont think it has to do with lazyness. It has to do with the fact
that parsing git branch gives you a branch name that you can use an an
argument to many other git commands. Whereas git-symbolic-ref doesnt.
It requires additional post-processing that unless you are very git
aware is not at all clear. Like for instance in this thread the
recommendation is to use sed like this:
git symbolic-ref HEAD|sed s,refs/heads/,,
however, that makes me think "how do i do that on a windows box? does
the presence of git on a windows box mean that they will necessarily
have sed available? Can i rely on that? Can i rely on the sed rule
being sufficient? And what happens with this command if im not on a
branch at all? Well it turns out that git symbolic-ref HEAD *dies*
with a fatal error on this command. SO it probably should be:
git symbolic-ref HEAD 2>/dev/null|sed s,refs/heads/,,
but now its even less portable. Even if sed is available on windows
/dev/null isnt.
Id very much like a proper way to find the usable form of the branch
name as it would make a lot of thing easier. In particular requiring
people use pipes means that there is a portability issue with scripts.
How does one make this happen on a windows box for instance?
Id also very much like a way to find the "upstream" for a branch. IOW,
id very much like to know where I will push to if i issue a "git push"
command, or what i will merge if i do a git pull. There doesnt seem to
be an easy way to find this out currently. And its very useful
information.
Im coming from the point of view of someone trying to make the perl
build process a bit more "git aware". Unfortunately Perl has to build
out of the box on so many platforms that unix-centric tricks like huge
command pipes arent very helpful. They immediately fall down when you
start dealing with oddball platforms like Windows and VMS.
Yves
--
perl -Mre=debug -e "/just|another|perl|hacker/"
^ permalink raw reply
* git-rev-parse --symbolic-abbrev-name [was Re: git-branch --print-current]
From: Karl Chen @ 2009-01-04 12:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Miklos Vajna, David Aguilar, Git mailing list
In-Reply-To: <7vzli73b1g.fsf@gitster.siamese.dyndns.org>
>>>>> On 2009-01-03 21:17 PST, Junio C Hamano writes:
Junio> That is a good point about user education, and is a
Junio> demonstration why a new option to cover a very
Junio> narrow-special case to symbolic-ref will not help the
Junio> situation. People will add their own embellishments
Junio> around the name of the branch anyway, and the most
Junio> generic symbolic-ref output is just as useful as a
Junio> special case option to show without refs/heads/.
That's arguable :) you really think "branchfoo" instead of
"refs/heads/branchfoo" is a narrow special case? Seems like a
common case for everyone except plumbing tools.
Here's a more general idea you might like better:
git symbolic-ref --abbrev BLAH
or even
git rev-parse --symbolic-abbrev-name BLAH
This would be like git-rev-parse --symbolic-full-name, but strips
the "refs/x/" iff the result is unambiguous. Since it's much more
work for a script to check whether the stripped version is
ambiguous, this functionality is appropriate as a builtin option.
(Hmm, I guess to be able to specify a ref it has to already be
unambiguous, so the main use that --symbolic doesn't already cover
is for symbolic refs such as HEAD.)
Junio> What you quoted are all inferior implementations of
Junio> showing the name of the current branch in the bash
Junio> prompt.
Yup, that was the point - it's so ugly seeing all these things
floating around, but that's where things stand right now.
Junio> ... __git_ps1 shell function is defined to be used for
Junio> this exact purpose and is documented in the completion
Junio> script.
Thanks for the detailed explanation. I actually use zsh rather
than of bash and I did already find git-completion.bash. But
obviously all those people posting on blogs don't know about it :)
^ permalink raw reply
* Re: git-rev-parse --symbolic-abbrev-name [was Re: git-branch --print-current]
From: demerphq @ 2009-01-04 12:40 UTC (permalink / raw)
To: Karl Chen; +Cc: Junio C Hamano, Miklos Vajna, David Aguilar, Git mailing list
In-Reply-To: <quack.20090104T0434.lthfxjz1c8x_-_@roar.cs.berkeley.edu>
2009/1/4 Karl Chen <quarl@cs.berkeley.edu>:
>>>>>> On 2009-01-03 21:17 PST, Junio C Hamano writes:
>
> Junio> That is a good point about user education, and is a
> Junio> demonstration why a new option to cover a very
> Junio> narrow-special case to symbolic-ref will not help the
> Junio> situation. People will add their own embellishments
> Junio> around the name of the branch anyway, and the most
> Junio> generic symbolic-ref output is just as useful as a
> Junio> special case option to show without refs/heads/.
>
> That's arguable :) you really think "branchfoo" instead of
> "refs/heads/branchfoo" is a narrow special case? Seems like a
> common case for everyone except plumbing tools.
I agree. All the scripting I've done involves using the non qualified form.
> Here's a more general idea you might like better:
>
> git symbolic-ref --abbrev BLAH
> or even
> git rev-parse --symbolic-abbrev-name BLAH
>
> This would be like git-rev-parse --symbolic-full-name, but strips
> the "refs/x/" iff the result is unambiguous. Since it's much more
> work for a script to check whether the stripped version is
> ambiguous, this functionality is appropriate as a builtin option.
I vote for this, I could and would use it many scripts. Also please
dont make it die if BLAH is not a symbolic ref if this option is used.
Just return nothing.
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