* Re: [PATCH v2] correct verify_path for Windows
From: Dmitry Potapov @ 2008-10-11 16:33 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Joshua Juran, Giovanni Funchal, git, Shawn O. Pearce
In-Reply-To: <48EAFF23.1020607@viscovery.net>
On Tue, Oct 07, 2008 at 08:18:11AM +0200, Johannes Sixt wrote:
> Dmitry Potapov schrieb:
> > +#if defined(_WIN32) || defined(__CYGWIN__)
>
> I think that for consistency you should use __MINGW32__ instead of _WIN32.
I like Alex's suggestion more to use FILESYSTEM_CASEINSENSITIVE.
>
> > + /* On Windows, file names are case-insensitive */
> > + case 'G':
> > + if ((rest[1]|0x20) != 'i')
> > + break;
> > + if ((rest[2]|0x20) != 't')
> > + break;
>
> We have tolower().
I am aware of that, but I am not sure what we gain by using it. It seems
it makes only code bigger and slow. As to readability, I don't see much
improvement... Isn't obvious what this code does, especially with the
above comment?
Dmitry
^ permalink raw reply
* [PATCH] Fix testcase failure when extended attributes are in use
From: Deskin Miller @ 2008-10-11 15:41 UTC (permalink / raw)
To: spearce; +Cc: git, heikki.orsila
06cbe855 (Make core.sharedRepository more generic, 2008-04-16) made
several testcases in t1301-shared-repo.sh which fail if on a system
which creates files with extended attributes (e.g. SELinux), since ls
appends a '+' sign to the permission set in such cases. This fixes the
testcase to strip any such sign prior to verifying the permission set.
Signed-off-by: Deskin Miller <deskinm@umich.edu>
---
Shawn, I read an email that said you'd maintain until Sunday the 12th, so I'm
sending this to you; if you want to punt to Junio, feel free.
Deskin Miller
t/t1301-shared-repo.sh | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
index dc85e8b..b244f3e 100755
--- a/t/t1301-shared-repo.sh
+++ b/t/t1301-shared-repo.sh
@@ -83,7 +83,7 @@ do
rm -f .git/info/refs &&
git update-server-info &&
actual="$(ls -l .git/info/refs)" &&
- actual=${actual%% *} &&
+ actual=$(echo "$actual" | sed -e "s/[+]\? .*$//") &&
test "x$actual" = "x-$y" || {
ls -lt .git/info
false
@@ -96,7 +96,7 @@ do
rm -f .git/info/refs &&
git update-server-info &&
actual="$(ls -l .git/info/refs)" &&
- actual=${actual%% *} &&
+ actual=$(echo "$actual" | sed -e "s/[+]\? .*$//") &&
test "x$actual" = "x-$x" || {
ls -lt .git/info
false
--
1.6.0.2.307.gc427
^ permalink raw reply related
* Re: Adding Reviewed-by/Tested-by tags to other peoples commits
From: Johannes Schindelin @ 2008-10-11 13:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Alex Bennee, git
In-Reply-To: <7vfxn3jqt9.fsf@gitster.siamese.dyndns.org>
Hi,
On Sat, 11 Oct 2008, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > 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.
>
> The fact a particular change has been reviewed is an attribute of a
> commit, and by recording the fact once (perhaps when you commit for the
> first time, or if your workflow is "commit blindly, then review, and
> then amend" then when you amend that commit), the commit object will
> remember that fact.
If that was the goal, you would _have_ to add commit notes. Because
otherwise you would have to pretend to have changed the commit, which you
did not at all.
> The patch you quoted adds Reviewed-by: at format-patch time, but I
> suspect that is a wrong approach.
Color me puzzled. You said in another mail that you think this is the
task for the MUA. When I send patches (even forwarded ones), I use
format-patch just before pasting into the MUA (I do not trust send-email).
And that's where Git can kick in: format-patch. Not the MUA.
So technically, I understood that the format-patch approach is exactly the
same as you were proposing, only that you do not ask the MUA to do Git's
job.
Unless, of course, you are talking about the reviewing style where the
patch does not leave the MUA until it is to be forwarded.
> You have to remember and recall which ones you reviewed (and which ones
> you didn't) when you run format-patch.
Don't you have to do that anyway? I do not see how giving format-patch a
new option --reviewed-by would change the equation in any way.
> This is a bit tangent, but perhaps rebase needs a hook so that users can
> strip certain tags automatically from the commit log messages (e.g.
> things like Reviewd-by: and Tested-by: become less trustworthy when you
> rebase; S-o-b: becomes somewhat less trustworthy when you "edit" in
> rebase-i; etc).
Maybe. I am not really convinced of the S-o-b. You kept stressing that
the SOB is not about validity, but a statement that the patch is
intellectually proper or some such (IOW it means something like "Darl,
forget it"). And the point of origin does not change, even if you rebase
the commit.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] "git diff <tree>{3,}": do not reverse order of arguments
From: Mikael Magnusson @ 2008-10-11 12:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Matt McCutchen, git
In-Reply-To: <7vwsgfjrp6.fsf@gitster.siamese.dyndns.org>
2008/10/11 Junio C Hamano <gitster@pobox.com>:
> 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.
Ehm, do you mean the internal parses it into "A ^B" and rev-parse gives "^B A"?
--
Mikael Magnusson
^ permalink raw reply
* Re: [PATCH (GITK) v2 4/4] gitk: Implement batch lookup and caching of encoding attrs.
From: Paul Mackerras @ 2008-10-11 12:03 UTC (permalink / raw)
To: Alexander Gavrilov; +Cc: git, Johannes Sixt
In-Reply-To: <200810111328.50951.angavrilov@gmail.com>
Alexander Gavrilov writes:
> > 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.
Hmmm, one that uses more time and memory than it needs to for gitk's
use... I guess it's not a lot, but it still seems unnecessary, unless
you can see a need for a menu of encodings in gitk.
> > 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?
OK.
Paul.
^ permalink raw reply
* Re: Adding Reviewed-by/Tested-by tags to other peoples commits
From: Junio C Hamano @ 2008-10-11 11:48 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Alex Bennee, git
In-Reply-To: <alpine.DEB.1.00.0810111239590.22125@pacific.mpi-cbg.de.mpi-cbg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 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.
The fact a particular change has been reviewed is an attribute of a
commit, and by recording the fact once (perhaps when you commit for the
first time, or if your workflow is "commit blindly, then review, and then
amend" then when you amend that commit), the commit object will remember
that fact.
The patch you quoted adds Reviewed-by: at format-patch time, but I suspect
that is a wrong approach. You have to remember and recall which ones you
reviewed (and which ones you didn't) when you run format-patch. People
who commit and immediately format-patch to send, or people who do not
review until immediately before format-patch to send, would not realize
the downside of the approach, but when your work style is to perform
commit/review and e-mail communication in separate phases, it matters.
This is a bit tangent, but perhaps rebase needs a hook so that users can
strip certain tags automatically from the commit log messages (e.g.
things like Reviewd-by: and Tested-by: become less trustworthy when you
rebase; S-o-b: becomes somewhat less trustworthy when you "edit" in
rebase-i; etc).
^ permalink raw reply
* 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
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