* Re: [Gitk PATCH 1/6] gitk: Add procedure to create accelerated menus
From: Robin Rosenberg @ 2008-10-11 11:39 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git
In-Reply-To: <18672.1017.68669.913415@cargo.ozlabs.ibm.com>
lördagen den 11 oktober 2008 03.40.09 skrev Paul Mackerras:
> Robin Rosenberg writes:
>
> > +proc mcw {menubar args} {
> > + set ai [lsearch $args "-label"]
> > + if { $ai > 0 } {
> > + set label [lindex $args [expr {$ai + 1}]]
> > + foreach {l u} [::tk::UnderlineAmpersand $label] {
>
> Where did you find out about ::tk::UnderlineAmpersand? Is it part of
> the exported interface of Tk that we can rely on in future?
I found it here. http://wiki.tcl.tk/1632. It hints that it may not be unoffical. Seems to
original from: http://groups.google.com/group/comp.lang.perl.tk/browse_thread/thread/5b6f4c6a4a9f17b4/c9f8ab47800d27ee?lnk=st&q=#c9f8ab47800d27ee
so if someone thinks there is a problem we could replace it with UnderlineAmpersand
with tcl code to do the same (or rip it as is if the license allows it).
>
> > +# Wrapper for mc to remove ampersands used for accelerators.
> > +proc mca {label} {
> > + set tl8 [mc $label]
> > + foreach {l u} [::tk::UnderlineAmpersand $tl8] break
> > + return $l
>
> return [string map {"&" ""} [mc $label]]
>
> instead, perhaps?
In theory we should translate && to & also, which UnderLineAmpersand does.
-- robin
^ permalink raw reply
* Re: [Gitk Patch 0/6]
From: Robin Rosenberg @ 2008-10-11 11:14 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git
In-Reply-To: <18672.4878.128010.174357@cargo.ozlabs.ibm.com>
lördagen den 11 oktober 2008 04.44.30 skrev Paul Mackerras:
> Robin Rosenberg writes:
>
> > (This is a resend to include gitk maintainer Paul Mackerras)
>
> Thanks.
>
> > I finally got tired of pressing Alt and some letter to activate menus in Gitk.
> > For example in "any" program you can press Alt-F to have the File menu drop
> > down and then select the underscored character to select the wanted menu.
> >
> > This series makes it possible. Friends of TCL may think my solution is too
> > hack-ish. It doesn't fix all of the similary problem (mostly button) but
> > that is the subject of later patches as it looks like it requires other
> > means.
>
> It'll do for a first cut, but there would be a nicer way to do it, I
> think.
>
> Also, I think patches 2 and 4 should be combined into one, as should
> patches 5 and 6. I'll do that and apply the series as 4 commits,
> unless you have an objection.
I split the commits to simplify review. My thought about squashing would
be different, but I have no strong opinions here. I was thinking 1+2, 4+5, 3+5, 6
-- robin
^ permalink raw reply
* [PATCH] Introduce core.keepHardLinks
From: Johannes Schindelin @ 2008-10-11 11:45 UTC (permalink / raw)
To: git, gitster, spearce
When a tracked file was hard linked, we used to break the hard link
whenever Git writes to that file. Make that optional.
To keep the implementation simple, mode changes will still break the
hard links.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
With the current revision of the patch, I can set
keep_hard_links = 1 and the test suite still passes.
I briefly tried to fix the "mode changes" issue, but replacing
the "st.st_mode != ce->ce_mode" with "S_ISREG(ce->ce_mode)"
(and consequently also adding
"|| (keep_hard_links && chmod(path, mode)" to create_file()),
made at least t3400 fail, and then I ran out of my Git time budget.
Documentation/config.txt | 4 +++
cache.h | 1 +
config.c | 5 ++++
entry.c | 7 ++++-
environment.c | 1 +
t/t0056-hardlinked.sh | 58 ++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 74 insertions(+), 2 deletions(-)
create mode 100644 t/t0056-hardlinked.sh
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 173386e..7bfe431 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -207,6 +207,10 @@ core.symlinks::
file. Useful on filesystems like FAT that do not support
symbolic links. True by default.
+core.keepHardLinks::
+ If true, do not break hard links by deleting and recreating the
+ files. Off by default.
+
core.gitProxy::
A "proxy command" to execute (as 'command host port') instead
of establishing direct connection to the remote server when
diff --git a/cache.h b/cache.h
index c89f2c6..c4bdece 100644
--- a/cache.h
+++ b/cache.h
@@ -479,6 +479,7 @@ enum rebase_setup_type {
extern enum branch_track git_branch_track;
extern enum rebase_setup_type autorebase;
+extern int keep_hard_links;
#define GIT_REPO_VERSION 0
extern int repository_format_version;
diff --git a/config.c b/config.c
index 18d305c..35ffdef 100644
--- a/config.c
+++ b/config.c
@@ -490,6 +490,11 @@ static int git_default_core_config(const char *var, const char *value)
return 0;
}
+ if (!strcmp(var, "core.keephardlinks")) {
+ keep_hard_links = git_config_bool(var, value);
+ return 0;
+ }
+
/* Add other config variables here and to Documentation/config.txt. */
return 0;
}
diff --git a/entry.c b/entry.c
index aa2ee46..dfddf83 100644
--- a/entry.c
+++ b/entry.c
@@ -82,7 +82,8 @@ static void remove_subtree(const char *path)
static int create_file(const char *path, unsigned int mode)
{
mode = (mode & 0100) ? 0777 : 0666;
- return open(path, O_WRONLY | O_CREAT | O_EXCL, mode);
+ return open(path, O_WRONLY | O_CREAT |
+ (keep_hard_links ? O_TRUNC : O_EXCL), mode);
}
static void *read_blob_entry(struct cache_entry *ce, const char *path, unsigned long *size)
@@ -225,7 +226,9 @@ int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *t
if (!state->force)
return error("%s is a directory", path);
remove_subtree(path);
- } else if (unlink(path))
+ } else if ((!keep_hard_links || !S_ISREG(st.st_mode) ||
+ st.st_mode != ce->ce_mode) &&
+ unlink(path))
return error("unable to unlink old '%s' (%s)", path, strerror(errno));
} else if (state->not_new)
return 0;
diff --git a/environment.c b/environment.c
index 0693cd9..ef721e0 100644
--- a/environment.c
+++ b/environment.c
@@ -42,6 +42,7 @@ enum safe_crlf safe_crlf = SAFE_CRLF_WARN;
unsigned whitespace_rule_cfg = WS_DEFAULT_RULE;
enum branch_track git_branch_track = BRANCH_TRACK_REMOTE;
enum rebase_setup_type autorebase = AUTOREBASE_NEVER;
+int keep_hard_links = 0;
/* This is set by setup_git_dir_gently() and/or git_default_config() */
char *git_work_tree_cfg;
diff --git a/t/t0056-hardlinked.sh b/t/t0056-hardlinked.sh
new file mode 100644
index 0000000..934c2bc
--- /dev/null
+++ b/t/t0056-hardlinked.sh
@@ -0,0 +1,58 @@
+#!/bin/sh
+
+test_description='read-tree and checkout respect hardlinked files'
+
+. ./test-lib.sh
+
+cat > file << EOF
+1. Nf3 Nf6 2. c4 g6 3. Nc3 Bg7 4. d4 O-O 5. Bf4 d5 6. Qb3 dxc4 7. Qxc4 c6
+8. e4 Nbd7 9. Rd1 Nb6 10. Qc5 Bg4 11. Bg5 Na4 12. Qa3 Nxc3 13. bxc3 Nxe4
+14. Bxe7 Qb6 15. Bc4 Nxc3 16. Bc5 Rfe8+ 17. Kf1 Be6 18. Bxb6 Bxc4+ 19. Kg1 Ne2+
+20. Kf1 Nxd4+ 21. Kg1 Ne2+ 22. Kf1 Nc3+ 23. Kg1 axb6 24. Qb4 Ra4 25. Qxb6 Nxd1
+26. h3 Rxa2 27. Kh2 Nxf2 28. Re1 Rxe1 29. Qd8+ Bf8 30. Nxe1 Bd5 31. Nf3 Ne4
+32. Qb8 b5 33. h4 h5 34. Ne5 Kg7 35. Kg1 Bc5+ 36. Kf1 Ng3+ 37. Ke1 Bb4+
+38. Kd1 Bb3+ 39. Kc1 Ne2+ 40. Kb1 Nc3+
+EOF
+
+ln file link || {
+ say "Could not hard link; skipping test"
+ test_done
+ exit
+}
+
+test_expect_success setup '
+
+ git config core.keepHardLinks true &&
+ test_cmp file link &&
+ cp file old &&
+ git add file &&
+ test_tick &&
+ git commit -m initial &&
+ echo "41. Kc1 Rc2#" >> file &&
+ git add file &&
+ test_tick &&
+ git commit -m 2nd &&
+ test_cmp file link &&
+ ! test_cmp file old
+
+'
+
+test_expect_success 'checking a file out does not break the hard link' '
+
+ git checkout HEAD^ -- file &&
+ test_cmp file link &&
+ test_cmp file old
+
+'
+
+test_expect_success 'read-tree -u -m does not break the hard link' '
+
+ git reset --hard &&
+ test_cmp file link &&
+ git read-tree -u -m HEAD^ &&
+ test_cmp file link &&
+ test_cmp file old
+
+'
+
+test_done
--
1.6.0.2.749.g0cc32
^ permalink raw reply related
* [PATCH] fetch: refuse to fetch into the current branch in a non-bare repository
From: Johannes Schindelin @ 2008-10-11 11:38 UTC (permalink / raw)
To: git, gitster, spearce
Some confusing tutorials suggest that it would be a good idea to call
something like this:
git pull origin master:master
While it might make sense to store what you want to merge, it typically
is plain wrong. Especially so when the current branch is "master".
Be at least a little bit helpful by refusing to fetch something into
the current branch.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
builtin-fetch.c | 20 ++++++++++++++++++++
t/t5505-remote.sh | 2 +-
t/t5510-fetch.sh | 6 ++++++
3 files changed, 28 insertions(+), 2 deletions(-)
diff --git a/builtin-fetch.c b/builtin-fetch.c
index ee93d3a..d701550 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -534,6 +534,25 @@ static void find_non_local_tags(struct transport *transport,
string_list_clear(&new_refs, 0);
}
+static void check_ref_map(struct ref *ref_map)
+{
+ int flag;
+ unsigned char sha1[20];
+ const char *HEAD;
+
+ if (is_bare_repository())
+ return;
+
+ HEAD = resolve_ref("HEAD", sha1, 1, &flag);
+
+ if (!HEAD || !(flag & REF_ISSYMREF))
+ return;
+
+ for (; ref_map; ref_map = ref_map->next)
+ if (ref_map->peer_ref && !strcmp(HEAD, ref_map->peer_ref->name))
+ die("Refusing to fetch into current branch");
+}
+
static int do_fetch(struct transport *transport,
struct refspec *refs, int ref_count)
{
@@ -558,6 +577,7 @@ static int do_fetch(struct transport *transport,
}
ref_map = get_ref_map(transport, refs, ref_count, tags, &autotags);
+ check_ref_map(ref_map);
for (rm = ref_map; rm; rm = rm->next) {
if (rm->peer_ref)
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index c449663..0103e1a 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -188,7 +188,7 @@ test_expect_success 'prune --dry-run' '
test_expect_success 'add --mirror && prune' '
(mkdir mirror &&
cd mirror &&
- git init &&
+ git init --bare &&
git remote add --mirror -f origin ../one) &&
(cd one &&
git branch -m side2 side) &&
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 9aae496..cd8b550 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -323,4 +323,10 @@ test_expect_success 'auto tag following fetches minimum' '
)
'
+test_expect_success 'refuse to fetch into the current branch' '
+
+ test_must_fail git fetch . side:master
+
+'
+
test_done
--
1.6.0.2.749.g0cc32
^ permalink raw reply related
* Re: [PATCH] "git diff <tree>{3,}": do not reverse order of arguments
From: Junio C Hamano @ 2008-10-11 11:29 UTC (permalink / raw)
To: Matt McCutchen; +Cc: git
In-Reply-To: <1223690175.2828.26.camel@mattlaptop2.local>
Matt McCutchen <matt@mattmccutchen.net> writes:
> diff --git a/builtin-diff.c b/builtin-diff.c
> index 35da366..9c8c295 100644
> --- a/builtin-diff.c
> +++ b/builtin-diff.c
> @@ -177,10 +177,8 @@ static int builtin_diff_combined(struct rev_info *revs,
> if (!revs->dense_combined_merges && !revs->combine_merges)
> revs->dense_combined_merges = revs->combine_merges = 1;
> parent = xmalloc(ents * sizeof(*parent));
> - /* Again, the revs are all reverse */
> for (i = 0; i < ents; i++)
> - hashcpy((unsigned char *)(parent + i),
> - ent[ents - 1 - i].item->sha1);
> + hashcpy((unsigned char *)(parent + i), ent[i].item->sha1);
> diff_tree_combined(parent[0], parent + 1, ents - 1,
> revs->dense_combined_merges, revs);
> return 0;
Interesting.
Somehow 0fe7c1d (built-in diff: assorted updates., 2006-04-29) thought
that setup_revisions() give the ent[] list in reverse order. This was a
thinko. I somehow thought that setup_revisions() queued the revs in
reverse back then, and kept that misconception to this day, so I had to
look it up in the history. It (meaning, the callchain from
setup_revisions to add_object) never did.
Perhaps the thinko was caused by discrepancy between the way internal
revision parser handles A..B and the way git-rev-parse parses it. While
the internal revision parser parses it into "A ^B", rev-parse gives them
in reverse order, i.e. "B ^A" (which is not going to change). In any
case, thanks for spotting this.
> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
> index 1a6b522..fe6080d 100755
> --- a/t/t4013-diff-various.sh
> +++ b/t/t4013-diff-various.sh
> @@ -258,6 +258,7 @@ diff --patch-with-stat -r initial..side
> diff --patch-with-raw -r initial..side
> diff --name-status dir2 dir
> diff --no-index --name-status dir2 dir
> +diff master master^ side
As "diff master master^ side" would mean "the tip of master was made by
merging side into it, show that tip of master like 'git show' does", I
think this makes sense.
^ permalink raw reply
* Re: Adding Reviewed-by/Tested-by tags to other peoples commits
From: Junio C Hamano @ 2008-10-11 11:28 UTC (permalink / raw)
To: Jeff King; +Cc: Alex Bennee, git
In-Reply-To: <20081011074604.GA23883@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Sat, Oct 11, 2008 at 07:37:04AM +0100, Alex Bennee wrote:
>
>> I've just tested/reviewed a patch of someone elses and I want to
>> forward it on the appropriate mailing list. I gather for Linux you
>> just add the appropriate tags to the commit. Does git offer a shortcut
>> for doing this or do you have to do a reset HEAD^ and re-commit with a
>> copy&pasted and modified commit message?
>
> Try "git commit --amend" to edit the commit message. There's no
> automatic way of adding acked-by or tested-by tags with git; most people
> who do those things often would probably configure their editor to make
> it easier.
These Distimmed-by: fields are cumulative in nature, so once recorded in a
commit log message you cannot add new ones without amending, which means
you cannot exchange such commits by pushing or pulling but by forwarding
patches via e-mails. I thought people added these in their MUAs. IOW, I
share your "would probably" with s/editor/MUA/.
^ permalink raw reply
* Re: Adding Reviewed-by/Tested-by tags to other peoples commits
From: Johannes Schindelin @ 2008-10-11 10:58 UTC (permalink / raw)
To: Jeff King; +Cc: Alex Bennee, git
In-Reply-To: <20081011074604.GA23883@sigill.intra.peff.net>
Hi,
On Sat, 11 Oct 2008, Jeff King wrote:
> On Sat, Oct 11, 2008 at 07:37:04AM +0100, Alex Bennee wrote:
>
> > I've just tested/reviewed a patch of someone elses and I want to
> > forward it on the appropriate mailing list. I gather for Linux you
> > just add the appropriate tags to the commit. Does git offer a shortcut
> > for doing this or do you have to do a reset HEAD^ and re-commit with a
> > copy&pasted and modified commit message?
>
> Try "git commit --amend" to edit the commit message. There's no
> automatic way of adding acked-by or tested-by tags with git; most people
> who do those things often would probably configure their editor to make
> it easier.
Of course, you could also automate it with a script:
-- snip --
GIT_EDITOR='sh -c "perl -pi.bak -e s/pick/edit/ \"$1\""' \
git rebase -i HEAD~2
while test -d .git/rebase-merge
do
GIT_EDITOR='sh -c "echo \"Reviewed-by: Mini Me <mi@ni.me>\" >> \"$1\""' \
git commit --amend &&
git rebase --continue ||
break
done
-- snap --
(Of course you would make HEAD~2 a parameter...)
BTW what happened with the "amend" patches to rebase -i? AFAIR Peff or
MadCoder (or was it j6t?) had patches to teach rebase -i the "amend"
command, which would not fall back to the command line just to fix a typo
in a commit message... I would like to have that.
Hth,
Dscho
^ permalink raw reply
* Re: Adding Reviewed-by/Tested-by tags to other peoples commits
From: Johannes Schindelin @ 2008-10-11 10:42 UTC (permalink / raw)
To: Alex Bennee; +Cc: git
In-Reply-To: <b2cdc9f30810102337q13432bepa957acaace9ddc5d@mail.gmail.com>
Hi,
On Sat, 11 Oct 2008, Alex Bennee wrote:
> I've just tested/reviewed a patch of someone elses and I want to forward
> it on the appropriate mailing list. I gather for Linux you just add the
> appropriate tags to the commit. Does git offer a shortcut for doing this
> or do you have to do a reset HEAD^ and re-commit with a copy&pasted and
> modified commit message?
http://thread.gmane.org/gmane.comp.version-control.git/75250/focus=76304
In the end, nothing happened, but I could see that you might want to push
for this patch. However, you'll have to replace the strbuf_initf() by
the ugly strbuf_init() && strbuf_addf() sequence, because strbuf_initf()
never made it into git.git.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH (GITK) v2 4/4] gitk: Implement batch lookup and caching of encoding attrs.
From: Alexander Gavrilov @ 2008-10-11 9:28 UTC (permalink / raw)
To: Paul Mackerras; +Cc: git, Johannes Sixt
In-Reply-To: <18671.62417.328489.317909@cargo.ozlabs.ibm.com>
On Saturday 11 October 2008 04:31:13 Paul Mackerras wrote:
> > > Also, I wonder why we now have two levels of caching of the encoding
> > > attribute. Your patch 1/4 introduced path_encoding_cache, which was
> > > fine, but now we have path_attr_cache as well, which seems to me to
> > > serve exactly the same function since the encoding is the only
> > > attribute we ever ask about. Surely we don't need both caches?
> >
> > If the (git-gui) patch that reimplements the tcl_encoding procedure is
> > applied, we may drop the path_encoding_cache. Current implementation
> > is too slow for batch lookup, especially if the encoding is actually
> > not supported, and without the cache the lookup would be done on every
> > loading of a diff.
>
> I was thinking more in terms of dropping the path_attr_cache actually.
Since gitattr is a general-purpose function that can read any attribute,
I decided that it should use its own cache.
Basically, all this double-caching issue is fallout from my failure to
anticipate the need of batch attribute lookup from the beginning...
> Actually, if [tcl_encoding] is slow, then why is $gui_encoding the
> untranslated version, so that we do [tcl_encoding $gui_encoding] on
> each call to get_path_encoding? Why don't we do the tcl_encoding call
> once and have $gui_encoding be the result of that? In fact
> $gui_encoding should be the result of this code (from
> get_path_encoding):
>
> set tcl_enc [tcl_encoding $gui_encoding]
> if {$tcl_enc eq {}} {
> set tcl_enc [encoding system]
> }
Well, that code was copied from git-gui, where it looks like this:
set tcl_enc [tcl_encoding [get_config gui.encoding]]
I.e. there is no "$gui_encoding" variable, although get_config does use a cache.
> And if [tcl_encoding] is slow, then it should have a cache. There's
> only likely to be at most 2 or 3 values it gets called for, and it's
> a constant function.
In git-gui the slowdown appeared during the construction of the menu
listing all available encodings, so a simple cache would not have helped.
I reimplemented it using a lookup table to resolve aliases (constructed
on the first run). But it can be thought of as a precalculated cache.
> At this point, what I think I might do is apply your set of patches
> (but with 2/4 and 3/4 folded into a single patch) and then go through
> and do another commit that addresses the concerns I've raised. OK?
Maybe I should resend the patches, scrapping path_encoding_cache,
and adding the optimized version of tcl_encoding?
Alexander
^ permalink raw reply
* Re: Adding Reviewed-by/Tested-by tags to other peoples commits
From: Jeff King @ 2008-10-11 7:46 UTC (permalink / raw)
To: Alex Bennee; +Cc: git
In-Reply-To: <b2cdc9f30810102337q13432bepa957acaace9ddc5d@mail.gmail.com>
On Sat, Oct 11, 2008 at 07:37:04AM +0100, Alex Bennee wrote:
> I've just tested/reviewed a patch of someone elses and I want to
> forward it on the appropriate mailing list. I gather for Linux you
> just add the appropriate tags to the commit. Does git offer a shortcut
> for doing this or do you have to do a reset HEAD^ and re-commit with a
> copy&pasted and modified commit message?
Try "git commit --amend" to edit the commit message. There's no
automatic way of adding acked-by or tested-by tags with git; most people
who do those things often would probably configure their editor to make
it easier.
-Peff
^ permalink raw reply
* Adding Reviewed-by/Tested-by tags to other peoples commits
From: Alex Bennee @ 2008-10-11 6:37 UTC (permalink / raw)
To: git
HI,
I've just tested/reviewed a patch of someone elses and I want to
forward it on the appropriate mailing list. I gather for Linux you
just add the appropriate tags to the commit. Does git offer a shortcut
for doing this or do you have to do a reset HEAD^ and re-commit with a
copy&pasted and modified commit message?
Or does everyone have their own magic scripts for tagging commits
during reviews?
--
Alex, homepage: http://www.bennee.com/~alex/
^ permalink raw reply
* Re: [Gitk Patch 0/6]
From: Paul Mackerras @ 2008-10-11 2:44 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
In-Reply-To: <1223532590-8706-1-git-send-email-robin.rosenberg@dewire.com>
Robin Rosenberg writes:
> (This is a resend to include gitk maintainer Paul Mackerras)
Thanks.
> I finally got tired of pressing Alt and some letter to activate menus in Gitk.
> For example in "any" program you can press Alt-F to have the File menu drop
> down and then select the underscored character to select the wanted menu.
>
> This series makes it possible. Friends of TCL may think my solution is too
> hack-ish. It doesn't fix all of the similary problem (mostly button) but
> that is the subject of later patches as it looks like it requires other
> means.
It'll do for a first cut, but there would be a nicer way to do it, I
think.
Also, I think patches 2 and 4 should be combined into one, as should
patches 5 and 6. I'll do that and apply the series as 4 commits,
unless you have an objection.
Paul.
^ permalink raw reply
* Re: [Gitk PATCH 1/6] gitk: Add procedure to create accelerated menus
From: Paul Mackerras @ 2008-10-11 1:40 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
In-Reply-To: <1223532590-8706-2-git-send-email-robin.rosenberg@dewire.com>
Robin Rosenberg writes:
> +proc mcw {menubar args} {
> + set ai [lsearch $args "-label"]
> + if { $ai > 0 } {
> + set label [lindex $args [expr {$ai + 1}]]
> + foreach {l u} [::tk::UnderlineAmpersand $label] {
Where did you find out about ::tk::UnderlineAmpersand? Is it part of
the exported interface of Tk that we can rely on in future?
> +# Wrapper for mc to remove ampersands used for accelerators.
> +proc mca {label} {
> + set tl8 [mc $label]
> + foreach {l u} [::tk::UnderlineAmpersand $tl8] break
> + return $l
return [string map {"&" ""} [mc $label]]
instead, perhaps?
Paul.
^ permalink raw reply
* Re: [PATCH] test-lib: fix color reset in say_color()
From: Jeff King @ 2008-10-11 2:00 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git
In-Reply-To: <1223590030-7464-1-git-send-email-vmiklos@frugalware.org>
On Fri, Oct 10, 2008 at 12:07:10AM +0200, Miklos Vajna wrote:
> Actually I'm not 100% sure about how many users are affected. I have a
> black background in konsole with white letters, and after the test I get
> a green cursor, and once I hit enter, I get the white one back.
I think my comment is way late, as this has already been applied, but I
think this is definitely the right thing to do. We are very careful to
use the same pattern (color reset before newline) in the C code.
-Peff
^ permalink raw reply
* [PATCH] "git diff <tree>{3,}": do not reverse order of arguments
From: Matt McCutchen @ 2008-10-11 1:56 UTC (permalink / raw)
To: git
According to the message of commit 0fe7c1de16f71312e6adac4b85bddf0d62a47168,
"git diff" with three or more trees expects the merged tree first followed by
the parents, in order. However, this command reversed the order of its
arguments, resulting in confusing diffs. A comment /* Again, the revs are all
reverse */ suggested there was a reason for this, but I can't figure out the
reason, so I removed the reversal of the arguments. Test case included.
Signed-off-by: Matt McCutchen <matt@mattmccutchen.net>
---
builtin-diff.c | 4 +---
t/t4013-diff-various.sh | 1 +
t/t4013/diff.diff_master_master^_side | 29 +++++++++++++++++++++++++++++
3 files changed, 31 insertions(+), 3 deletions(-)
create mode 100644 t/t4013/diff.diff_master_master^_side
diff --git a/builtin-diff.c b/builtin-diff.c
index 35da366..9c8c295 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -177,10 +177,8 @@ static int builtin_diff_combined(struct rev_info *revs,
if (!revs->dense_combined_merges && !revs->combine_merges)
revs->dense_combined_merges = revs->combine_merges = 1;
parent = xmalloc(ents * sizeof(*parent));
- /* Again, the revs are all reverse */
for (i = 0; i < ents; i++)
- hashcpy((unsigned char *)(parent + i),
- ent[ents - 1 - i].item->sha1);
+ hashcpy((unsigned char *)(parent + i), ent[i].item->sha1);
diff_tree_combined(parent[0], parent + 1, ents - 1,
revs->dense_combined_merges, revs);
return 0;
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 1a6b522..fe6080d 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -258,6 +258,7 @@ diff --patch-with-stat -r initial..side
diff --patch-with-raw -r initial..side
diff --name-status dir2 dir
diff --no-index --name-status dir2 dir
+diff master master^ side
EOF
test_done
diff --git a/t/t4013/diff.diff_master_master^_side b/t/t4013/diff.diff_master_master^_side
new file mode 100644
index 0000000..50ec9ca
--- /dev/null
+++ b/t/t4013/diff.diff_master_master^_side
@@ -0,0 +1,29 @@
+$ git diff master master^ side
+diff --cc dir/sub
+index cead32e,7289e35..992913c
+--- a/dir/sub
++++ b/dir/sub
+@@@ -1,6 -1,4 +1,8 @@@
+ A
+ B
+ +C
+ +D
+ +E
+ +F
++ 1
++ 2
+diff --cc file0
+index b414108,f4615da..10a8a9f
+--- a/file0
++++ b/file0
+@@@ -1,6 -1,6 +1,9 @@@
+ 1
+ 2
+ 3
+ +4
+ +5
+ +6
++ A
++ B
++ C
+$
--
1.6.0.2.519.gf6ee
^ permalink raw reply related
* Re: [PATCH] t9001: use older Getopt::Long boolean prefix '--no' rather than '--no-'
From: Brandon Casey @ 2008-10-11 0:44 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Git Mailing List
In-Reply-To: <20081011002431.GK8203@spearce.org>
Shawn O. Pearce wrote:
> Brandon Casey <casey@nrlssc.navy.mil> wrote:
>> Since dbf5e1e9, the '--no-validate' option is a Getopt::Long boolean
>> option. The '--no-' prefix (as in --no-validate) for boolean options
>> is not supported in Getopt::Long version 2.32 which was released with
>> Perl 5.8.0. This version only supports '--no' as in '--novalidate'.
>> More recent versions of Getopt::Long, such as version 2.34, support
>> either prefix. So use the older form in the tests.
>
> Ouch.
>
> Should we update our docs?
>
> Actually, if 2.32 doesn't support the --no-validate syntax than
> this is a regression in Git. Even if it is what many would call a
> bug in Getopt::Long in Perl, I think Git 1.6.1 should still honor
> --no-validate like it did in Git 1.6.0.
If it makes any difference, none of the other boolean options would
work in the documented '--no-' form either, such as --no-thread,
--no-signed-off-by-cc, etc. '--no-validate' is probably the only one
that used to work.
Update the docs?
---[no-]validate::
+--[no-|no]validate::
hmm. not sure.
-brandon
^ permalink raw reply
* Re: [PATCH (GITK) v2 4/4] gitk: Implement batch lookup and caching of encoding attrs.
From: Paul Mackerras @ 2008-10-11 0:31 UTC (permalink / raw)
To: Alexander Gavrilov; +Cc: git, Johannes Sixt
In-Reply-To: <bb6f213e0810100522v653507d6r75cc4c64b57aa459@mail.gmail.com>
Alexander Gavrilov writes:
> OS-enforced command-line size limit on Windows is 32K. Cramming in
> 1000 paths would leave only 32 characters for each path.
Eeek. OK.
> > Also, I wonder why we now have two levels of caching of the encoding
> > attribute. Your patch 1/4 introduced path_encoding_cache, which was
> > fine, but now we have path_attr_cache as well, which seems to me to
> > serve exactly the same function since the encoding is the only
> > attribute we ever ask about. Surely we don't need both caches?
>
> If the (git-gui) patch that reimplements the tcl_encoding procedure is
> applied, we may drop the path_encoding_cache. Current implementation
> is too slow for batch lookup, especially if the encoding is actually
> not supported, and without the cache the lookup would be done on every
> loading of a diff.
I was thinking more in terms of dropping the path_attr_cache actually.
Actually, if [tcl_encoding] is slow, then why is $gui_encoding the
untranslated version, so that we do [tcl_encoding $gui_encoding] on
each call to get_path_encoding? Why don't we do the tcl_encoding call
once and have $gui_encoding be the result of that? In fact
$gui_encoding should be the result of this code (from
get_path_encoding):
set tcl_enc [tcl_encoding $gui_encoding]
if {$tcl_enc eq {}} {
set tcl_enc [encoding system]
}
And if [tcl_encoding] is slow, then it should have a cache. There's
only likely to be at most 2 or 3 values it gets called for, and it's
a constant function.
At this point, what I think I might do is apply your set of patches
(but with 2/4 and 3/4 folded into a single patch) and then go through
and do another commit that addresses the concerns I've raised. OK?
Paul.
^ permalink raw reply
* Re: [PATCH] t9001: use older Getopt::Long boolean prefix '--no' rather than '--no-'
From: Shawn O. Pearce @ 2008-10-11 0:24 UTC (permalink / raw)
To: Brandon Casey; +Cc: Git Mailing List
In-Reply-To: <zAKaSrh5XLrzXloxZrj-A1EletXL_wuSGpTQgLXMT3MZYK_o3tUBfA@cipher.nrlssc.navy.mil>
Brandon Casey <casey@nrlssc.navy.mil> wrote:
> Since dbf5e1e9, the '--no-validate' option is a Getopt::Long boolean
> option. The '--no-' prefix (as in --no-validate) for boolean options
> is not supported in Getopt::Long version 2.32 which was released with
> Perl 5.8.0. This version only supports '--no' as in '--novalidate'.
> More recent versions of Getopt::Long, such as version 2.34, support
> either prefix. So use the older form in the tests.
Ouch.
Should we update our docs?
Actually, if 2.32 doesn't support the --no-validate syntax than
this is a regression in Git. Even if it is what many would call a
bug in Getopt::Long in Perl, I think Git 1.6.1 should still honor
--no-validate like it did in Git 1.6.0.
> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> index d098a01..561ae7d 100755
> --- a/t/t9001-send-email.sh
> +++ b/t/t9001-send-email.sh
> @@ -109,7 +109,7 @@ test_expect_success 'allow long lines with --no-validate' '
> --from="Example <nobody@example.com>" \
> --to=nobody@example.com \
> --smtp-server="$(pwd)/fake.sendmail" \
> - --no-validate \
> + --novalidate \
> $patches longline.patch \
> 2>errors
> '
--
Shawn.
^ permalink raw reply
* [PATCH] t9001: use older Getopt::Long boolean prefix '--no' rather than '--no-'
From: Brandon Casey @ 2008-10-11 0:21 UTC (permalink / raw)
To: Git Mailing List
Since dbf5e1e9, the '--no-validate' option is a Getopt::Long boolean
option. The '--no-' prefix (as in --no-validate) for boolean options
is not supported in Getopt::Long version 2.32 which was released with
Perl 5.8.0. This version only supports '--no' as in '--novalidate'.
More recent versions of Getopt::Long, such as version 2.34, support
either prefix. So use the older form in the tests.
Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
t/t9001-send-email.sh | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index d098a01..561ae7d 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -109,7 +109,7 @@ test_expect_success 'allow long lines with --no-validate' '
--from="Example <nobody@example.com>" \
--to=nobody@example.com \
--smtp-server="$(pwd)/fake.sendmail" \
- --no-validate \
+ --novalidate \
$patches longline.patch \
2>errors
'
--
1.6.0.2.468.gd5b83
^ permalink raw reply related
* Re: [PATCH 2/2 v2] check-attr: Add --stdin-paths option
From: Paul Mackerras @ 2008-10-10 22:39 UTC (permalink / raw)
To: Dmitry Potapov; +Cc: Johannes Sixt, Alexander Gavrilov, git, Shawn O. Pearce
In-Reply-To: <20081007001652.GR21650@dpotapov.dyndns.org>
Dmitry Potapov writes:
> This allows multiple paths to be specified on stdin.
>
> Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>
> ---
> On Mon, Oct 06, 2008 at 09:09:11AM +0200, Johannes Sixt wrote:
> >
> > The least that is needed is fflush(stdout) in this loop (after each
> > iteration!)
>
> Thanks. Somehow, I forgot about it though it is quite obvious.
> I have added maybe_flush_or_die().
Actually, what was done with git diff-tree --stdin was to have it do
fflush(stdout) when it sees a blank line in the input. That gives the
calling program a way to get the output up to that point without
having to do a flush for every line of output.
Paul.
^ permalink raw reply
* Re: git confused by rename
From: Michael P. Soulier @ 2008-10-10 22:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v3aj4momt.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 1162 bytes --]
On 10/10/08 Junio C Hamano said:
> The above observation of mine is correct, but I forgot that "git status"
> (and the comment in the commit template from "git commit") is generated
> internally with "diff-index -B -M". So if
>
> (0) had A but not B in the HEAD commit;
> (1) you created B that is very similar to the original A; and
> (2) you modified A beyond recognition;
Which, FTR, is what I did.
I copied A -> B for its boilerplate, and then hacked them both up, although B
consisted mostly of content that I removed from A (a refactoring exercise).
> This however makes me wonder if "diff-index -B -M" should say B is copied
> (instead of being renamed) from A and A is modified in such a case. I do
> not think we would want to make such a change without thinking things,
> through.
Gotcha. Indeed though, I did cp A -> B, and if A still exists then it
obviously was not renamed.
Thanks,
Mike
--
Michael P. Soulier <msoulier@digitaltorque.ca>
"Any intelligent fool can make things bigger and more complex... It
takes a touch of genius - and a lot of courage to move in the opposite
direction." --Albert Einstein
[-- Attachment #2: Type: application/pgp-signature, Size: 187 bytes --]
^ permalink raw reply
* Re: Really remove a file ?
From: Stefan Karpinski @ 2008-10-10 20:50 UTC (permalink / raw)
To: Björn Steinbrink; +Cc: marcreddist, git
In-Reply-To: <20081010143249.GE3671@atjola.homenet>
> But don't do the pruning until you're absolutely sure that you don't
> require the old stuff anymore.
Or, of course, you could just keep an independent copy of the whole
repo pre-filter-branch.
On Fri, Oct 10, 2008 at 7:32 AM, Björn Steinbrink <B.Steinbrink@gmx.de> wrote:
>
> On 2008.10.10 05:38:25 -0400, marcreddist@aim.com wrote:
> >> You'll probably also want to run "git gc" on your repo to actually
> >> get rid of the huge object that was added (or does filter-branch do
> >> this automatically?).
> >
> > I'm not sure it's required by git-filter-branch alone. In this case :
> >
> > git-gc saves almost 5% after the file deletion
> >
> > it saves 4.5% before the file deletion
> >
> > If I run git gc before and after the git filter-branch, it saves 4.5%
> > and then 0.2%.
>
> Did you clear the refs/original namespace and your reflogs? Otherwise,
> the huge object is most likely still referenced and thus won't get
> pruned. Also, I usually prefer "git repack -adf" over "git gc" in such
> situations, but that's probably just because I don't know the right
> way to force "git gc" to immediately prune stuff just once.
>
> But don't do the pruning until you're absolutely sure that you don't
> require the old stuff anymore.
>
> Björn
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCHv3] gitweb: refactor input parameters parse/validation
From: Giuseppe Bilotta @ 2008-10-10 18:42 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski, Shawn O. Pearce, Giuseppe Bilotta
In-Reply-To: <20081010150108.GC29829@spearce.org>
Since input parameters can be obtained both from CGI parameters and
PATH_INFO, we would like most of the code to be agnostic about the way
parameters were retrieved. We thus collect all the parameters into the
new %input_params hash, delaying validation after the collection is
completed.
Although the kludge removal is minimal at the moment, it makes life much
easier for future expansions such as more extensive PATH_INFO use or
other form of input such as command-line support.
Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
---
gitweb/gitweb.perl | 315 +++++++++++++++++++++++++++++-----------------------
1 files changed, 178 insertions(+), 137 deletions(-)
This resend fixes a bug spotted by Shwan Pearce, where the global $action
instead of the local $input was used for action validation. Although the bug
itself didn't have any effect because validate_action was called on $action
anyway, it's still a logical error that needed squashing.
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 1116800..c5254af 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -29,7 +29,9 @@ our $my_uri = $cgi->url(-absolute => 1);
# if we're called with PATH_INFO, we have to strip that
# from the URL to find our real URL
-if (my $path_info = $ENV{"PATH_INFO"}) {
+# we make $path_info global because it's also used later on
+my $path_info = $ENV{"PATH_INFO"};
+if ($path_info) {
$my_url =~ s,\Q$path_info\E$,,;
$my_uri =~ s,\Q$path_info\E$,,;
}
@@ -428,34 +430,155 @@ $projects_list ||= $projectroot;
# ======================================================================
# input validation and dispatch
-our $action = $cgi->param('a');
+
+# input parameters can be collected from a variety of sources (presently, CGI
+# and PATH_INFO), so we define an %input_params hash that collects them all
+# together during validation: this allows subsequent uses (e.g. href()) to be
+# agnostic of the parameter origin
+
+my %input_params = ();
+
+# input parameters are stored with the long parameter name as key. This will
+# also be used in the href subroutine to convert parameters to their CGI
+# equivalent, and since the href() usage is the most frequent one, we store
+# the name -> CGI key mapping here, instead of the reverse.
+#
+# XXX: Warning: If you touch this, check the search form for updating,
+# too.
+
+my @cgi_param_mapping = (
+ project => "p",
+ action => "a",
+ file_name => "f",
+ file_parent => "fp",
+ hash => "h",
+ hash_parent => "hp",
+ hash_base => "hb",
+ hash_parent_base => "hpb",
+ page => "pg",
+ order => "o",
+ searchtext => "s",
+ searchtype => "st",
+ snapshot_format => "sf",
+ extra_options => "opt",
+ search_use_regexp => "sr",
+);
+my %cgi_param_mapping = @cgi_param_mapping;
+
+# we will also need to know the possible actions, for validation
+my %actions = (
+ "blame" => \&git_blame,
+ "blobdiff" => \&git_blobdiff,
+ "blobdiff_plain" => \&git_blobdiff_plain,
+ "blob" => \&git_blob,
+ "blob_plain" => \&git_blob_plain,
+ "commitdiff" => \&git_commitdiff,
+ "commitdiff_plain" => \&git_commitdiff_plain,
+ "commit" => \&git_commit,
+ "forks" => \&git_forks,
+ "heads" => \&git_heads,
+ "history" => \&git_history,
+ "log" => \&git_log,
+ "rss" => \&git_rss,
+ "atom" => \&git_atom,
+ "search" => \&git_search,
+ "search_help" => \&git_search_help,
+ "shortlog" => \&git_shortlog,
+ "summary" => \&git_summary,
+ "tag" => \&git_tag,
+ "tags" => \&git_tags,
+ "tree" => \&git_tree,
+ "snapshot" => \&git_snapshot,
+ "object" => \&git_object,
+ # those below don't need $project
+ "opml" => \&git_opml,
+ "project_list" => \&git_project_list,
+ "project_index" => \&git_project_index,
+);
+
+# finally, we have the hash of allowed extra_options for the commands that
+# allow them
+my %allowed_options = (
+ "--no-merges" => [ qw(rss atom log shortlog history) ],
+);
+
+# fill %input_params with the CGI parameters. All values except for 'opt'
+# should be single values, but opt can be an array. We should probably
+# build an array of parameters that can be multi-valued, but since for the time
+# being it's only this one, we just single it out
+while (my ($name, $symbol) = each %cgi_param_mapping) {
+ if ($symbol eq 'opt') {
+ $input_params{$name} = [ $cgi->param($symbol) ];
+ } else {
+ $input_params{$name} = $cgi->param($symbol);
+ }
+}
+
+# now read PATH_INFO and update the parameter list for missing parameters
+sub evaluate_path_info {
+ return if defined $input_params{'project'};
+ return if !$path_info;
+ $path_info =~ s,^/+,,;
+ return if !$path_info;
+
+ # find which part of PATH_INFO is project
+ my $project = $path_info;
+ $project =~ s,/+$,,;
+ while ($project && !check_head_link("$projectroot/$project")) {
+ $project =~ s,/*[^/]*$,,;
+ }
+ return unless $project;
+ $input_params{'project'} = $project;
+
+ # do not change any parameters if an action is given using the query string
+ return if $input_params{'action'};
+ $path_info =~ s,^\Q$project\E/*,,;
+
+ my ($refname, $pathname) = split(/:/, $path_info, 2);
+ if (defined $pathname) {
+ # we got "project.git/branch:filename" or "project.git/branch:dir/"
+ # we could use git_get_type(branch:pathname), but it needs $git_dir
+ $pathname =~ s,^/+,,;
+ if (!$pathname || substr($pathname, -1) eq "/") {
+ $input_params{'action'} = "tree";
+ $pathname =~ s,/$,,;
+ } else {
+ $input_params{'action'} = "blob_plain";
+ }
+ $input_params{'hash_base'} ||= $refname;
+ $input_params{'file_name'} ||= $pathname;
+ } elsif (defined $refname) {
+ # we got "project.git/branch"
+ $input_params{'action'} = "shortlog";
+ $input_params{'hash'} ||= $refname;
+ }
+}
+evaluate_path_info();
+
+our $action = $input_params{'action'};
if (defined $action) {
- if ($action =~ m/[^0-9a-zA-Z\.\-_]/) {
+ if (!validate_action($action)) {
die_error(400, "Invalid action parameter");
}
}
# parameters which are pathnames
-our $project = $cgi->param('p');
+our $project = $input_params{'project'};
if (defined $project) {
- if (!validate_pathname($project) ||
- !(-d "$projectroot/$project") ||
- !check_head_link("$projectroot/$project") ||
- ($export_ok && !(-e "$projectroot/$project/$export_ok")) ||
- ($strict_export && !project_in_list($project))) {
+ if (!validate_project($project)) {
undef $project;
die_error(404, "No such project");
}
}
-our $file_name = $cgi->param('f');
+our $file_name = $input_params{'file_name'};
if (defined $file_name) {
if (!validate_pathname($file_name)) {
die_error(400, "Invalid file parameter");
}
}
-our $file_parent = $cgi->param('fp');
+our $file_parent = $input_params{'file_parent'};
if (defined $file_parent) {
if (!validate_pathname($file_parent)) {
die_error(400, "Invalid file parent parameter");
@@ -463,44 +586,41 @@ if (defined $file_parent) {
}
# parameters which are refnames
-our $hash = $cgi->param('h');
+our $hash = $input_params{'hash'};
if (defined $hash) {
if (!validate_refname($hash)) {
die_error(400, "Invalid hash parameter");
}
}
-our $hash_parent = $cgi->param('hp');
+our $hash_parent = $input_params{'hash_parent'};
if (defined $hash_parent) {
if (!validate_refname($hash_parent)) {
die_error(400, "Invalid hash parent parameter");
}
}
-our $hash_base = $cgi->param('hb');
+our $hash_base = $input_params{'hash_base'};
if (defined $hash_base) {
if (!validate_refname($hash_base)) {
die_error(400, "Invalid hash base parameter");
}
}
-my %allowed_options = (
- "--no-merges" => [ qw(rss atom log shortlog history) ],
-);
-
-our @extra_options = $cgi->param('opt');
-if (defined @extra_options) {
- foreach my $opt (@extra_options) {
- if (not exists $allowed_options{$opt}) {
- die_error(400, "Invalid option parameter");
- }
- if (not grep(/^$action$/, @{$allowed_options{$opt}})) {
- die_error(400, "Invalid option parameter for this action");
- }
+our @extra_options = @{$input_params{'extra_options'}};
+# @extra_options is always defined, since it can only be (currently) set from
+# CGI, and $cgi->param() returns the empty array in array context if the param
+# is not set
+foreach my $opt (@extra_options) {
+ if (not exists $allowed_options{$opt}) {
+ die_error(400, "Invalid option parameter");
+ }
+ if (not grep(/^$action$/, @{$allowed_options{$opt}})) {
+ die_error(400, "Invalid option parameter for this action");
}
}
-our $hash_parent_base = $cgi->param('hpb');
+our $hash_parent_base = $input_params{'hash_parent_base'};
if (defined $hash_parent_base) {
if (!validate_refname($hash_parent_base)) {
die_error(400, "Invalid hash parent base parameter");
@@ -508,23 +628,23 @@ if (defined $hash_parent_base) {
}
# other parameters
-our $page = $cgi->param('pg');
+our $page = $input_params{'page'};
if (defined $page) {
if ($page =~ m/[^0-9]/) {
die_error(400, "Invalid page parameter");
}
}
-our $searchtype = $cgi->param('st');
+our $searchtype = $input_params{'searchtype'};
if (defined $searchtype) {
if ($searchtype =~ m/[^a-z]/) {
die_error(400, "Invalid searchtype parameter");
}
}
-our $search_use_regexp = $cgi->param('sr');
+our $search_use_regexp = $input_params{'search_use_regexp'};
-our $searchtext = $cgi->param('s');
+our $searchtext = $input_params{'searchtext'};
our $search_regexp;
if (defined $searchtext) {
if (length($searchtext) < 2) {
@@ -533,86 +653,11 @@ if (defined $searchtext) {
$search_regexp = $search_use_regexp ? $searchtext : quotemeta $searchtext;
}
-# now read PATH_INFO and use it as alternative to parameters
-sub evaluate_path_info {
- return if defined $project;
- my $path_info = $ENV{"PATH_INFO"};
- return if !$path_info;
- $path_info =~ s,^/+,,;
- return if !$path_info;
- # find which part of PATH_INFO is project
- $project = $path_info;
- $project =~ s,/+$,,;
- while ($project && !check_head_link("$projectroot/$project")) {
- $project =~ s,/*[^/]*$,,;
- }
- # validate project
- $project = validate_pathname($project);
- if (!$project ||
- ($export_ok && !-e "$projectroot/$project/$export_ok") ||
- ($strict_export && !project_in_list($project))) {
- undef $project;
- return;
- }
- # do not change any parameters if an action is given using the query string
- return if $action;
- $path_info =~ s,^\Q$project\E/*,,;
- my ($refname, $pathname) = split(/:/, $path_info, 2);
- if (defined $pathname) {
- # we got "project.git/branch:filename" or "project.git/branch:dir/"
- # we could use git_get_type(branch:pathname), but it needs $git_dir
- $pathname =~ s,^/+,,;
- if (!$pathname || substr($pathname, -1) eq "/") {
- $action ||= "tree";
- $pathname =~ s,/$,,;
- } else {
- $action ||= "blob_plain";
- }
- $hash_base ||= validate_refname($refname);
- $file_name ||= validate_pathname($pathname);
- } elsif (defined $refname) {
- # we got "project.git/branch"
- $action ||= "shortlog";
- $hash ||= validate_refname($refname);
- }
-}
-evaluate_path_info();
-
# path to the current git repository
our $git_dir;
$git_dir = "$projectroot/$project" if $project;
# dispatch
-my %actions = (
- "blame" => \&git_blame,
- "blobdiff" => \&git_blobdiff,
- "blobdiff_plain" => \&git_blobdiff_plain,
- "blob" => \&git_blob,
- "blob_plain" => \&git_blob_plain,
- "commitdiff" => \&git_commitdiff,
- "commitdiff_plain" => \&git_commitdiff_plain,
- "commit" => \&git_commit,
- "forks" => \&git_forks,
- "heads" => \&git_heads,
- "history" => \&git_history,
- "log" => \&git_log,
- "rss" => \&git_rss,
- "atom" => \&git_atom,
- "search" => \&git_search,
- "search_help" => \&git_search_help,
- "shortlog" => \&git_shortlog,
- "summary" => \&git_summary,
- "tag" => \&git_tag,
- "tags" => \&git_tags,
- "tree" => \&git_tree,
- "snapshot" => \&git_snapshot,
- "object" => \&git_object,
- # those below don't need $project
- "opml" => \&git_opml,
- "project_list" => \&git_project_list,
- "project_index" => \&git_project_index,
-);
-
if (!defined $action) {
if (defined $hash) {
$action = git_get_type($hash);
@@ -642,35 +687,12 @@ sub href (%) {
# default is to use -absolute url() i.e. $my_uri
my $href = $params{-full} ? $my_url : $my_uri;
- # XXX: Warning: If you touch this, check the search form for updating,
- # too.
-
- my @mapping = (
- project => "p",
- action => "a",
- file_name => "f",
- file_parent => "fp",
- hash => "h",
- hash_parent => "hp",
- hash_base => "hb",
- hash_parent_base => "hpb",
- page => "pg",
- order => "o",
- searchtext => "s",
- searchtype => "st",
- snapshot_format => "sf",
- extra_options => "opt",
- search_use_regexp => "sr",
- );
- my %mapping = @mapping;
-
$params{'project'} = $project unless exists $params{'project'};
if ($params{-replay}) {
- while (my ($name, $symbol) = each %mapping) {
+ while (my ($name, $symbol) = each %cgi_param_mapping) {
if (!exists $params{$name}) {
- # to allow for multivalued params we use arrayref form
- $params{$name} = [ $cgi->param($symbol) ];
+ $params{$name} = $input_params{$name};
}
}
}
@@ -689,8 +711,8 @@ sub href (%) {
# now encode the parameters explicitly
my @result = ();
- for (my $i = 0; $i < @mapping; $i += 2) {
- my ($name, $symbol) = ($mapping[$i], $mapping[$i+1]);
+ for (my $i = 0; $i < @cgi_param_mapping; $i += 2) {
+ my ($name, $symbol) = ($cgi_param_mapping[$i], $cgi_param_mapping[$i+1]);
if (defined $params{$name}) {
if (ref($params{$name}) eq "ARRAY") {
foreach my $par (@{$params{$name}}) {
@@ -710,6 +732,25 @@ sub href (%) {
## ======================================================================
## validation, quoting/unquoting and escaping
+sub validate_action {
+ my $input = shift || return undef;
+ return undef unless exists $actions{$input};
+ return $input;
+}
+
+sub validate_project {
+ my $input = shift || return undef;
+ if (!validate_pathname($input) ||
+ !(-d "$projectroot/$input") ||
+ !check_head_link("$projectroot/$input") ||
+ ($export_ok && !(-e "$projectroot/$input/$export_ok")) ||
+ ($strict_export && !project_in_list($input))) {
+ return undef;
+ } else {
+ return $input;
+ }
+}
+
sub validate_pathname {
my $input = shift || return undef;
@@ -4121,7 +4162,7 @@ sub git_search_grep_body {
## actions
sub git_project_list {
- my $order = $cgi->param('o');
+ my $order = $input_params{'order'};
if (defined $order && $order !~ m/none|project|descr|owner|age/) {
die_error(400, "Unknown order parameter");
}
@@ -4149,7 +4190,7 @@ sub git_project_list {
}
sub git_forks {
- my $order = $cgi->param('o');
+ my $order = $input_params{'order'};
if (defined $order && $order !~ m/none|project|descr|owner|age/) {
die_error(400, "Unknown order parameter");
}
@@ -4697,7 +4738,7 @@ sub git_snapshot {
my @supported_fmts = gitweb_check_feature('snapshot');
@supported_fmts = filter_snapshot_fmts(@supported_fmts);
- my $format = $cgi->param('sf');
+ my $format = $input_params{'snapshot_format'};
if (!@supported_fmts) {
die_error(403, "Snapshots not allowed");
}
--
1.5.6.5
^ permalink raw reply related
* Re: [RFC PATCH] describe: Make --tags and --all match lightweight tags more often
From: Junio C Hamano @ 2008-10-10 18:18 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: Shawn O. Pearce, git, Erez Zilber
In-Reply-To: <20081010171217.GB29028@artemis.corp>
Pierre Habouzit <madcoder@debian.org> writes:
> I would like to see an enhanced information in the documentation so that
> people remember that lightweight tags are not meant to be constant over
> time and that's a bad idea to use them.
>
> What the discussion showed, is that the people don't know about
> annotated tags, and git-describe should have a stub of documentation
> that points to git-tag(1) so that people learn about it.
>
> Apart from that, it feels fine.
The primary mode of operation without --tags of "describe" is about coming
up with version numbers, and as such, it should try to base its output on
immutable anchors as much as possible. For that reason, I think it should
use "tag " line from the tag object, not the name of the ref, to describe
the committish. They should match (otherwise fsck would say something
about it) in practice, though...
The patch is about --tags, which is not about such a strict "version
number" generation, but about "come up with a closest ref", so in that
light it feels perfectly fine.
^ permalink raw reply
* Re: [PATCHv2] gitweb: refactor input parameters parse/validation
From: Giuseppe Bilotta @ 2008-10-10 17:33 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Jakub Narebski
In-Reply-To: <20081010150108.GC29829@spearce.org>
On Fri, Oct 10, 2008 at 5:01 PM, Shawn O. Pearce <spearce@spearce.org> wrote:
> Giuseppe Bilotta <giuseppe.bilotta@gmail.com> wrote:
>> +sub validate_action {
>> + my $input = shift || return undef;
>> + return undef unless exists $actions{$action};
>> + return $action;
>> +}
>
> Shouldn't $action here be $input?
Urgh. Yes, yes it should. Good catch. Should I resend?
--
Giuseppe "Oblomov" Bilotta
^ 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