* Re: [PATCH 2/2] check-ignore.c: fix segfault with '.' argument from repo root
From: Adam Spiers @ 2013-02-19 19:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git list, Zoltan Klinger
In-Reply-To: <7v1ucc6vgd.fsf@alter.siamese.dyndns.org>
On Tue, Feb 19, 2013 at 5:54 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Adam Spiers <git@adamspiers.org> writes:
>
>> Fix a corner case where check-ignore would segfault when run with the
>> '.' argument from the top level of a repository, due to prefix_path()
>> converting '.' into the empty string. It doesn't make much sense to
>> call check-ignore from the top level with '.' as a parameter, since
>> the top-level directory would never typically be ignored, but of
>> course it should not segfault in this case.
>>
>> Signed-off-by: Adam Spiers <git@adamspiers.org>
>> ---
>
> Please step back a bit and explain why the original had check for
> path[0] in the first place?
I can't remember to be honest.
> If the answer is "the code wanted to special case the question 'is
> the top-level excluded?',
Yes, I think that's the most likely explanation. Maybe it got missed
in a variable renaming refactoring.
> but used a wrong variable to implement the
> check, and this patch is a fix to that", then the proposed commit
> log message looks incomplete. The cause of the segv is not that
> prefix_path() returns an empty string, but because the function
> called inside the "if" block was written without expecting to be fed
> the path that refers to the top-level of the working tree, no?
>
> While this change certainly will prevent the "check the top-level"
> request to last-exclude-matching-path, I have to wonder if it is a
> good idea to force the caller of the l-e-m-p function to even care.
>
> In other words, would it be a cleaner approach to fix the l-e-m-p
> function so that the caller can ask "check the top-level" and give a
> sensible answer (perhaps the answer may be "nothing matches"), and
> remove the "&& path[0]" (or "&& full_path[0]") special case from
> this call site?
Yes, that did cross my mind. I also wondered whether hash_name()
should do stricter input validation, but I guess that could have an
impact on performance.
> The last sentence "It doesn't make much sense..." in the proposed
> log message would become a good justification for such a special
> case at the beginning of l-e-m-p function, I would think.
Fair enough. I'll reply to this with a new version.[0]
[0] I wish there was a clean way to include the new version inline,
but as I've noted before, there doesn't seem to be:
http://article.gmane.org/gmane.comp.version-control.git/146110
^ permalink raw reply
* Re: [PATCH] Documentation/githooks: Explain pre-rebase parameters
From: Junio C Hamano @ 2013-02-19 19:08 UTC (permalink / raw)
To: W. Trevor King; +Cc: Git
In-Reply-To: <c19c03f51d71a58fa3795f665fe4a4c0461fa58f.1361271116.git.wking@tremily.us>
"W. Trevor King" <wking@tremily.us> writes:
> From: "W. Trevor King" <wking@tremily.us>
>
> Descriptions borrowed from templates/hooks--pre-rebase.sample.
>
> Signed-off-by: W. Trevor King <wking@tremily.us>
> ---
> I'm not 100% convinced about this, because the git-rebase.sh uses:
>
> "$GIT_DIR/hooks/pre-rebase" ${1+"$@"}
>
> I haven't been able to find documentation for the ${1+"$@"} syntax.
> Is it in POSIX? It's not in the Bash manual:
>
> $ man bash | grep '\${.*[+]'
> (${BASH_SOURCE[$i+1]}) where ${FUNCNAME[$i]} was called (or
> ${BASH_SOURCE[$i+1]}.
> ${BASH_SOURCE[$i+1]} at line number ${BASH_LINENO[$i]}. The
> ${parameter:+word}
>
> In my local tests, it seems equivalent to "$@".
>
> Also, it appears that the `git-rebase--*.sh` handlers don't use the
> pre-rebase hook. Is this intentional?
The codeflow of git-rebase front-end, when you start rebasing, will
call run_pre_rebase_hook before calling run_specific_rebase. It
will be redundant for handlers to then call it again, no?
In "rebase --continue" and later steps, you would not want to see
the hook trigger.
> Documentation/githooks.txt | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> index b9003fe..bc837c6 100644
> --- a/Documentation/githooks.txt
> +++ b/Documentation/githooks.txt
> @@ -140,9 +140,10 @@ the outcome of 'git commit'.
> pre-rebase
> ~~~~~~~~~~
>
> -This hook is called by 'git rebase' and can be used to prevent a branch
> -from getting rebased.
> -
> +This hook is called by 'git rebase' and can be used to prevent a
> +branch from getting rebased. The hook takes two parameters: the
> +upstream the series was forked from and the branch being rebased. The
> +second parameter will be empty when rebasing the current branch.
Technically this is incorrect.
We call it with one or two parameters, and sometimes the second
parameter is _missing_, which is different from calling with an
empty string. For a script written in some scripting languages like
shell and perl, the distinction may not matter (i.e. $2 and $ARGV[1]
will be an empty string when stringified) but not all (accessing
sys.argv[2] may give you an IndexError in Python).
^ permalink raw reply
* Re: [PATCH] Bugfix: undefined htmldir in config.mak.autogen
From: Junio C Hamano @ 2013-02-19 19:11 UTC (permalink / raw)
To: Jiang Xin; +Cc: Christoph J. Thompson, Git List
In-Reply-To: <1be0a520b99fbfecd7255c1d26753015612856b3.1361272864.git.worldhello.net@gmail.com>
Jiang Xin <worldhello.net@gmail.com> writes:
> Html documents will be installed to root dir (/) no matter what prefix
> is set, if run these commands before `make` and `make install-html`:
>
> $ make configure
> $ ./configure --prefix=<PREFIX>
>
> After the installation, all the html documents will copy to rootdir (/),
> and:
>
> $ git --html-path
> <PREFIX>
>
> $ git help -w something
> fatal: '<PREFIX>': not a documentation directory.
>
> This is because the variable "htmldir" points to a undefined variable
> "$(docdir)" in file "config.mak.autogen", which is generated by running
> `./configure`. This bug comes from commit fc1c541 (Honor configure's
> htmldir switch), since v1.8.1.3-537-g1d321.
>
> Add the required two variables "PACKAGE_TARNAME" and "docdir" to file
> "config.mak.in" will resolve this problem.
>
> Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
> ---
Who references PACKAGE_TARNAME and how is the symbol used?
> config.mak.in | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/config.mak.in b/config.mak.in
> index d7c49..fa02bd 100644
> --- a/config.mak.in
> +++ b/config.mak.in
> @@ -8,6 +8,7 @@ LDFLAGS = @LDFLAGS@
> AR = @AR@
> TAR = @TAR@
> DIFF = @DIFF@
> +PACKAGE_TARNAME = @PACKAGE_TARNAME@
> #INSTALL = @INSTALL@ # needs install-sh or install.sh in sources
>
> prefix = @prefix@
> @@ -17,6 +18,7 @@ gitexecdir = @libexecdir@/git-core
> datarootdir = @datarootdir@
> template_dir = @datadir@/git-core/templates
> sysconfdir = @sysconfdir@
> +docdir = @docdir@
>
> mandir = @mandir@
> htmldir = @htmldir@
^ permalink raw reply
* Re: Git Merge 2013 Conference, Berlin
From: Junio C Hamano @ 2013-02-19 19:19 UTC (permalink / raw)
To: Scott Chacon; +Cc: Michael J Gruber, Jeff King, Thomas Rast, git list
In-Reply-To: <CAP2yMa+O19iZUD33PZkdz61xWEjfKEvUoNWBztoyH2YeSkyD+w@mail.gmail.com>
Scott Chacon <schacon@gmail.com> writes:
> On Tue, Feb 19, 2013 at 7:41 AM, Michael J Gruber
> <git@drmicha.warpmail.net> wrote:
>> Michael J Gruber venit, vidit, dixit 19.02.2013 16:20:
>>> Well, all days are listed as "sold out" on the eventbrite site. Maybe
>>> it's because eventbrite has "trouble connecting to facebook" because I
>>> "don't have facebook"?
>
> No, it's because 300 people signed up and that's all the venue has
> room for. I'm sure we can fit one more if you come.
Hmph. "git shortlog -s -n --since=18.months master" tells me that
we have 284 contributors to my tree during the said period.
I do not remember if I signed-up for the dev-day or any other days
myself.
^ permalink raw reply
* [PATCH v2 2/2] check-ignore.c, dir.c: fix segfault with '.' argument from repo root
From: Adam Spiers @ 2013-02-19 19:21 UTC (permalink / raw)
To: git list; +Cc: Zoltan Klinger
In-Reply-To: <CAOkDyE9VVuFn6B=Fe4XHxGCEW0MFgndx1X0+9hO36Soxb37YQw@mail.gmail.com>
Fix a corner case where check-ignore would segfault when run with the
'.' argument from the top level of a repository, due to prefix_path()
converting '.' into the empty string. It doesn't make much sense to
call check-ignore from the top level with '.' as a parameter, since
the top-level directory would never typically be ignored, but of
course it should not segfault in this case. The existing code
attempted to check for this case but failed due to using the wrong
variable. Instead we move the check to last_exclude_matching_path(),
in case other callers present or future have a similar issue.
Signed-off-by: Adam Spiers <git@adamspiers.org>
---
builtin/check-ignore.c | 2 +-
dir.c | 8 ++++++++
t/t0008-ignores.sh | 5 +++++
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index 709535c..0240f99 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -89,7 +89,7 @@ static int check_ignore(const char *prefix, const char **pathspec)
? strlen(prefix) : 0, path);
full_path = check_path_for_gitlink(full_path);
die_if_path_beyond_symlink(full_path, prefix);
- if (!seen[i] && path[0]) {
+ if (!seen[i]) {
exclude = last_exclude_matching_path(&check, full_path,
-1, &dtype);
if (exclude) {
diff --git a/dir.c b/dir.c
index 57394e4..1ae0b90 100644
--- a/dir.c
+++ b/dir.c
@@ -828,6 +828,14 @@ struct exclude *last_exclude_matching_path(struct path_exclude_check *check,
struct exclude *exclude;
/*
+ * name could be the empty string, e.g. if check-ignore was
+ * invoked from the top level with '.', prefix_path() will
+ * convert it into "".
+ */
+ if (!*name)
+ return NULL;
+
+ /*
* we allow the caller to pass namelen as an optimization; it
* must match the length of the name, as we eventually call
* is_excluded() on the whole name string.
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index ebe7c70..9c1bde1 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -138,6 +138,7 @@ test_expect_success 'setup' '
cat <<-\EOF >.gitignore &&
one
ignored-*
+ top-level-dir/
EOF
for dir in . a
do
@@ -177,6 +178,10 @@ test_expect_success 'setup' '
#
# test invalid inputs
+test_expect_success_multi '. corner-case' '' '
+ test_check_ignore . 1
+'
+
test_expect_success_multi 'empty command line' '' '
test_check_ignore "" 128 &&
stderr_contains "fatal: no path specified"
--
1.8.2.rc0.18.g543d1e4
^ permalink raw reply related
* Re: Git Merge 2013 Conference, Berlin
From: Scott Chacon @ 2013-02-19 19:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael J Gruber, Jeff King, Thomas Rast, git list
In-Reply-To: <7vbobg5cy4.fsf@alter.siamese.dyndns.org>
Hey,
On Tue, Feb 19, 2013 at 11:19 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Scott Chacon <schacon@gmail.com> writes:
>
>> On Tue, Feb 19, 2013 at 7:41 AM, Michael J Gruber
>> <git@drmicha.warpmail.net> wrote:
>>> Michael J Gruber venit, vidit, dixit 19.02.2013 16:20:
>>>> Well, all days are listed as "sold out" on the eventbrite site. Maybe
>>>> it's because eventbrite has "trouble connecting to facebook" because I
>>>> "don't have facebook"?
>>
>> No, it's because 300 people signed up and that's all the venue has
>> room for. I'm sure we can fit one more if you come.
>
> Hmph. "git shortlog -s -n --since=18.months master" tells me that
> we have 284 contributors to my tree during the said period.
>
> I do not remember if I signed-up for the dev-day or any other days
> myself.
>
300 is the number of people signed up for the User Day. There is a
Dev Day, for contributors, which has 50 signed up. If anyone on this
list or any other core Git devs want to attend and did not sign up, or
would like financial assistance getting to or staying in Germany,
please let me know.
The second day is a User Day, which is when 300 people will be there.
This is a day for short talks, idea generation and feedback. The
third day is Hack Day, which will have about 200. This is a day for
working on stuff.
Scott
^ permalink raw reply
* Re* [PATCH 2/2] check-ignore.c: fix segfault with '.' argument from repo root
From: Junio C Hamano @ 2013-02-19 19:56 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list, Zoltan Klinger
In-Reply-To: <CAOkDyE9VVuFn6B=Fe4XHxGCEW0MFgndx1X0+9hO36Soxb37YQw@mail.gmail.com>
Adam Spiers <git@adamspiers.org> writes:
> Fair enough. I'll reply to this with a new version.[0]
>
> [0] I wish there was a clean way to include the new version inline,
> but as I've noted before, there doesn't seem to be:
>
> http://article.gmane.org/gmane.comp.version-control.git/146110
I find it easier to later find the patch if you made it a separate
follow-up like you did, but you can do it this way if you really
want to, using a scissors line, like so. Please do not try to be
creative and change the shape of scissors just for the sake of
chaning it.
-- >8 --
Subject: name-hash: allow hashing an empty string
Usually we do not pass an empty string to the function hash_name()
because we almost always ask for hash values for a path that is a
candidate to be added to the index. However, check-ignore (and most
likely check-attr, but I didn't check) apparently has a callchain
to ask the hash value for an empty path when it was given a "." from
the top-level directory to ask "Is the path . excluded by default?"
Make sure that hash_name() does not overrun the end of the given
pathname even when it is empty.
Also remove a sweep-the-issue-under-the-rug conditional in
check-ignore that avoided to pass an empty string to the callchain.
Signed-off-by: Adam Spiers <git@adamspiers.org>
---
builtin/check-ignore.c | 2 +-
name-hash.c | 4 ++--
t/t0008-ignores.sh | 5 +++++
3 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index 709535c..0240f99 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -89,7 +89,7 @@ static int check_ignore(const char *prefix, const char **pathspec)
? strlen(prefix) : 0, path);
full_path = check_path_for_gitlink(full_path);
die_if_path_beyond_symlink(full_path, prefix);
- if (!seen[i] && path[0]) {
+ if (!seen[i]) {
exclude = last_exclude_matching_path(&check, full_path,
-1, &dtype);
if (exclude) {
diff --git a/name-hash.c b/name-hash.c
index d8d25c2..942c459 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -24,11 +24,11 @@ static unsigned int hash_name(const char *name, int namelen)
{
unsigned int hash = 0x123;
- do {
+ while (namelen--) {
unsigned char c = *name++;
c = icase_hash(c);
hash = hash*101 + c;
- } while (--namelen);
+ }
return hash;
}
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index ebe7c70..9c1bde1 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -138,6 +138,7 @@ test_expect_success 'setup' '
cat <<-\EOF >.gitignore &&
one
ignored-*
+ top-level-dir/
EOF
for dir in . a
do
@@ -177,6 +178,10 @@ test_expect_success 'setup' '
#
# test invalid inputs
+test_expect_success_multi '. corner-case' '' '
+ test_check_ignore . 1
+'
+
test_expect_success_multi 'empty command line' '' '
test_check_ignore "" 128 &&
stderr_contains "fatal: no path specified"
^ permalink raw reply related
* Re: [PATCH v2 2/2] check-ignore.c, dir.c: fix segfault with '.' argument from repo root
From: Junio C Hamano @ 2013-02-19 19:59 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list, Zoltan Klinger
In-Reply-To: <1361301696-11307-1-git-send-email-git@adamspiers.org>
Adam Spiers <git@adamspiers.org> writes:
> Fix a corner case where check-ignore would segfault when run with the
> '.' argument from the top level of a repository, due to prefix_path()
> converting '.' into the empty string.
The description does not match what I understand is happening from
the original report, though. The above is more like this, no?
When check-ignore is run with the '.' argument from the top level of
a repository, it fed an empty string to hash_name() in name-hash.c
and caused a segfault, as the function kept reading forever past the
end of the string.
A point to note is that it is not cleaer why it is a corner case to
ask about a pathspec ".". It is a valid question "Is the whole tree
ignored by default?", isn't it?
> It doesn't make much sense to
> call check-ignore from the top level with '.' as a parameter, since
> the top-level directory would never typically be ignored,
And this sounds like a really bad excuse. If it were "it does not
make *any* sense ... because the top level is *never* ignored", then
the patch is a perfectly fine optimization that happens to work
around the problem, but the use of "much" and "typically" is a sure
sign that the design of the fix is iffy. It also shows that the
patch is not a fix, but is sweeping the problem under the rug, if
there were a valid use case to set the top level to be ignored.
I wonder what would happen if we removed that "&& path[0]" check
from the caller, not add the "assume the top is never ignored"
workaround, and do something like this to the location that causes
segv, so that it can give an answer when asked to hash an empty
string?
Does the callchain that goes down to this function have other places
that assume they will never see an empty string, like this function
does, which I _think_ is the real issue?
name-hash.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/name-hash.c b/name-hash.c
index d8d25c2..942c459 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -24,11 +24,11 @@ static unsigned int hash_name(const char *name, int namelen)
{
unsigned int hash = 0x123;
- do {
+ while (namelen--) {
unsigned char c = *name++;
c = icase_hash(c);
hash = hash*101 + c;
- } while (--namelen);
+ }
return hash;
}
^ permalink raw reply related
* Re: [PATCH] l10n: de.po: translate 5 new messages
From: Thomas Rast @ 2013-02-19 20:01 UTC (permalink / raw)
To: Ralf Thielow; +Cc: jk, stimming, git
In-Reply-To: <1361293966-3975-1-git-send-email-ralf.thielow@gmail.com>
Ralf Thielow <ralf.thielow@gmail.com> writes:
> msgid "You are currently bisecting branch '%s'."
> -msgstr "Sie sind gerade beim Halbieren."
> +msgstr "Sie sind gerade beim Halbieren in Zweig '%s'."
I know this one is already in other messages (and also in the Glossary),
but I still find it iffy and I might finally have a better idea:
Sie sind gerade an einer binären Suche in Zweig '%s'.
[note to English speakers: I'm just using "binary search" instead of
"bisect"]
That makes it quite a bit harder to use it in a verbed[1] form, but I
think it gets the concept across much better. (And in all the usage I
know in CS, the two things refer to the same.)
ACK on the rest.
Footnotes:
[1] Verbing weirds language. -- Calvin
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* [PATCH] fixup! Documentation/git-commit.txt: rework the --cleanup section
From: Jonathan Nieder @ 2013-02-19 20:28 UTC (permalink / raw)
To: Brandon Casey; +Cc: gitster, git, ralf.thielow, Brandon Casey
In-Reply-To: <1361298570-19738-1-git-send-email-bcasey@nvidia.com>
Hi,
Brandon Casey wrote:
> Signed-off-by: Brandon Casey <drafnel@gmail.com>
This renders as
--cleanup=<mode>
This option determines how the supplied commit message
should be cleaned up before committing. The <mode> can be
strip, whitespace, verbatim, or default.
strip
Strip leading and trailing empty lines, trailing
whitespace, and #commentary and collapse consecutive
empty lines.
whitespace
Same as strip except #commentary is not removed.
verbatim
Do not change the message at all.
default
strip if the message is to be edited. Otherwise
whitespace.
The default can be changed by the 'commit.cleanup' config
variable (see linkgit:git-config[1]).
Problems:
* There's a weird extra blank line after "default"
* Wrong indentation for the final paragraph.
* The linkgit isn't resolved for some reason.
The following fixes it for me.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Documentation/git-commit.txt | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 992c219..24a99cc 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -185,11 +185,12 @@ whitespace::
verbatim::
Do not change the message at all.
default::
- `strip` if the message is to be edited. Otherwise `whitespace`.
+ Same as `strip` if the message is to be edited.
+ Otherwise `whitespace`.
--
+
- The default can be changed by the 'commit.cleanup' configuration
- variable (see linkgit:git-config[1]).
+The default can be changed by the 'commit.cleanup' configuration
+variable (see linkgit:git-config[1]).
-e::
--edit::
--
1.8.1.3
^ permalink raw reply related
* Re: [PATCH] fixup! Documentation/git-commit.txt: rework the --cleanup section
From: Brandon Casey @ 2013-02-19 20:33 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Brandon Casey, gitster, git, ralf.thielow
In-Reply-To: <20130219202822.GA13460@google.com>
On Tue, Feb 19, 2013 at 12:28 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Hi,
>
> Brandon Casey wrote:
> Problems:
>
> * There's a weird extra blank line after "default"
> * Wrong indentation for the final paragraph.
> * The linkgit isn't resolved for some reason.
>
> The following fixes it for me.
Thanks Jonathan.
-Brandon
^ permalink raw reply
* Re: [PATCH v2 4/4] Documentation/git-commit.txt: rework the --cleanup section
From: Junio C Hamano @ 2013-02-19 20:35 UTC (permalink / raw)
To: Brandon Casey; +Cc: git, jrnieder, ralf.thielow, Brandon Casey
In-Reply-To: <1361298570-19738-1-git-send-email-bcasey@nvidia.com>
Brandon Casey <bcasey@nvidia.com> writes:
> From: Brandon Casey <drafnel@gmail.com>
>
> Signed-off-by: Brandon Casey <drafnel@gmail.com>
> ---
>
>
> [RESEND] I originally specified Junio's address as gitster@pobox.org.
> [RESEND] Sorry, now with the correct address.
>
> Ok, here's the updated text. I am not set up to build the documentation,
> so I hope someone will test, but looks right to me.
Thanks for marking this as unverified.
Jonathan, thanks for the fixup.
^ permalink raw reply
* Re: [PATCH v4 1/3] user-manual: Reorganize the reroll sections, adding 'git rebase -i'
From: Junio C Hamano @ 2013-02-19 20:56 UTC (permalink / raw)
To: W. Trevor King; +Cc: Git, Jonathan Nieder
In-Reply-To: <20130219185119.GD8170@odin.tremily.us>
"W. Trevor King" <wking@tremily.us> writes:
> On Tue, Feb 19, 2013 at 10:47:04AM -0800, Junio C Hamano wrote:
>> "W. Trevor King" <wking@tremily.us> writes:
>> > +Sometimes you want to edit a commit deeper in your history. One
>> > +approach is to use `git format-patch` to create a series of patches,
>> > +then reset the state to before the patches:
>> >
>> > -------------------------------------------------
>> > +$ git format-patch origin
>> > +$ git reset --hard origin
>> > -------------------------------------------------
>>
>> Technically speaking, this does not "reset to before the patches".
>> You would need "git reset --hard $(git merge-base origin HEAD)" or
>> something like that.
>
> They'll be fine if they haven't fetched since they started their
> branch ;).
>
> It does look like I've got an extra comma an a missing “and”. What
> about:
>
> …create series of patches and then reset…
The original doesn't look too odd to me, but I'll amend to what you
just wrote.
Thanks.
^ permalink raw reply
* Re: Git Merge 2013 Conference, Berlin
From: Junio C Hamano @ 2013-02-19 21:26 UTC (permalink / raw)
To: Scott Chacon; +Cc: Michael J Gruber, Jeff King, Thomas Rast, git list
In-Reply-To: <CAP2yMaKF7P8J5VCA8ODo2gdHj=FRfjki1xoRv-L4shfOExSX2w@mail.gmail.com>
Scott Chacon <schacon@gmail.com> writes:
> On Tue, Feb 19, 2013 at 11:19 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Scott Chacon <schacon@gmail.com> writes:
>>
>>> On Tue, Feb 19, 2013 at 7:41 AM, Michael J Gruber
>>> <git@drmicha.warpmail.net> wrote:
>>>> Michael J Gruber venit, vidit, dixit 19.02.2013 16:20:
>>>>> Well, all days are listed as "sold out" on the eventbrite site. Maybe
>>>>> it's because eventbrite has "trouble connecting to facebook" because I
>>>>> "don't have facebook"?
>>>
>>> No, it's because 300 people signed up and that's all the venue has
>>> room for. I'm sure we can fit one more if you come.
>> ...
> 300 is the number of people signed up for the User Day. There is a
> Dev Day, for contributors, which has 50 signed up.
Ah, sorry I misunderstood. So your 300 above was about the user
day, and Dev day has different capacity but has already sold out at
50 seats.
^ permalink raw reply
* Re: git-p4: Importing a Git repository into Perforce without rebasing
From: Thomas Berg @ 2013-02-19 21:29 UTC (permalink / raw)
To: Russell Myers; +Cc: git
In-Reply-To: <CAA5tD2sZiYcH+edbuO2Uh43zNUOyDYUO_JHpRhrLOjSqV-PfpA@mail.gmail.com>
Hi,
On Tue, Feb 19, 2013 at 3:40 AM, Russell Myers <mezner@russellmyers.com> wrote:
> I'm trying to take a Git repository which has never been in Perforce
> and push it to Perforce and having difficulty.
[...]
> I know that I could create another Git repository that has some
> commits in it cloned from Perforce and rebase on top of that; however,
> the repository I'm trying to import is rather large and rebasing would
> require me to change many merge commits. I'd like to avoid doing this.
> The repository has many thousands of commits in it.
So your history is not linear and contains merges.
> In short my question is this: Using git-p4, is there a way to push a
> Git repository into Perforce without rebasing on top of commits coming
> from Perforce?
No, this is not supported. Non-linear history would be a problem for
git-p4 too, so that alone wouldn't solve your problem. git-p4 does not
have the logic needed to submit merges back to Perforce.
- Thomas
^ permalink raw reply
* Re: [PATCH v2 2/2] check-ignore.c, dir.c: fix segfault with '.' argument from repo root
From: Junio C Hamano @ 2013-02-19 22:03 UTC (permalink / raw)
To: Adam Spiers; +Cc: git list, Zoltan Klinger
In-Reply-To: <7vzjz03wid.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> And this sounds like a really bad excuse. If it were "it does not
> make *any* sense ... because the top level is *never* ignored", then
> the patch is a perfectly fine optimization that happens to work
> around the problem, but the use of "much" and "typically" is a sure
> sign that the design of the fix is iffy. It also shows that the
> patch is not a fix, but is sweeping the problem under the rug, if
> there were a valid use case to set the top level to be ignored.
>
> I wonder what would happen if we removed that "&& path[0]" check
> from the caller, not add the "assume the top is never ignored"
> workaround, and do something like this to the location that causes
> segv, so that it can give an answer when asked to hash an empty
> string?
>
> Does the callchain that goes down to this function have other places
> that assume they will never see an empty string, like this function
> does, which I _think_ is the real issue?
I started to suspect that may be the right approach. Why not do this?
-- >8 --
From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 19 Feb 2013 11:56:44 -0800
Subject: [PATCH] name-hash: allow hashing an empty string
Usually we do not pass an empty string to the function hash_name()
because we almost always ask for hash values for a path that is a
candidate to be added to the index. However, check-ignore (and most
likely check-attr, but I didn't check) apparently has a callchain
to ask the hash value for an empty path when it was given a "." from
the top-level directory to ask "Is the path . excluded by default?"
Make sure that hash_name() does not overrun the end of the given
pathname even when it is empty.
Remove a sweep-the-issue-under-the-rug conditional in check-ignore
that avoided to pass an empty string to the callchain while at it.
It is a valid question to ask for check-ignore if the top-level is
set to be ignored by default, even though the answer is most likely
no, if only because there is currently no way to specify such an
entry in the .gitignore file. But it is an unusual thing to ask and
it is not worth optimizing for it by special casing at the top level
of the call chain.
Signed-off-by: Adam Spiers <git@adamspiers.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/check-ignore.c | 2 +-
name-hash.c | 4 ++--
t/t0008-ignores.sh | 5 +++++
3 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index 709535c..0240f99 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -89,7 +89,7 @@ static int check_ignore(const char *prefix, const char **pathspec)
? strlen(prefix) : 0, path);
full_path = check_path_for_gitlink(full_path);
die_if_path_beyond_symlink(full_path, prefix);
- if (!seen[i] && path[0]) {
+ if (!seen[i]) {
exclude = last_exclude_matching_path(&check, full_path,
-1, &dtype);
if (exclude) {
diff --git a/name-hash.c b/name-hash.c
index d8d25c2..942c459 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -24,11 +24,11 @@ static unsigned int hash_name(const char *name, int namelen)
{
unsigned int hash = 0x123;
- do {
+ while (namelen--) {
unsigned char c = *name++;
c = icase_hash(c);
hash = hash*101 + c;
- } while (--namelen);
+ }
return hash;
}
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index ebe7c70..9c1bde1 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -138,6 +138,7 @@ test_expect_success 'setup' '
cat <<-\EOF >.gitignore &&
one
ignored-*
+ top-level-dir/
EOF
for dir in . a
do
@@ -177,6 +178,10 @@ test_expect_success 'setup' '
#
# test invalid inputs
+test_expect_success_multi '. corner-case' '' '
+ test_check_ignore . 1
+'
+
test_expect_success_multi 'empty command line' '' '
test_check_ignore "" 128 &&
stderr_contains "fatal: no path specified"
--
1.8.2.rc0.89.g6e4b41d
^ permalink raw reply related
* Re: feature request
From: Shawn Pearce @ 2013-02-19 22:27 UTC (permalink / raw)
To: Jeff King; +Cc: James Nylen, Jay Townsend, git
In-Reply-To: <20130218204511.GA27308@sigill.intra.peff.net>
On Mon, Feb 18, 2013 at 12:45 PM, Jeff King <peff@peff.net> wrote:
>
> The thing that makes 2FA usable in the web browser setting is that you
> authenticate only occasionally, and get a token (i.e., a cookie) from
> the server that lets you have a longer session without re-authenticating.
Right, otherwise you spend all day typing in your credentials and
syncing with the 2nd factor device.
> I suspect a usable 2FA scheme for http pushes would involve a special
> credential helper that did the 2FA auth to receive a cookie on the first
> use, cached the cookie, and then provided it for subsequent auth
> requests. That would not necessarily involve changing git, but it would
> mean writing the appropriate helper (and the server side to match). I
> seem to recall Shawn mentioning that Google does something like this
> internally, but I don't know the details[1].
...
> [1] I don't know if Google's system is based on the Google Authenticator
> system. But it would be great if there could be an open,
> standards-based system for doing 2FA+cookie authentication like
> this. I'd hate to have "the GitHub credential helper" and "the
> Google credential helper". I'm not well-versed enough in the area to
> know what's feasible and what the standards are.
Yes, it is based on the Google Authenticator system, but that's not
relevant to how Git works with it. :-)
We have a special "git-remote-sso" helper we install onto corporate
workstations. This allows Git to understand the "sso://" protocol.
git-remote-sso is a small application that:
- reads the URL from the command line,
- makes sure a Netscape style cookies file has a current cookie for
the named host,
- acquires or updates cookie if necessary
- rewrites the URL to be https://
- execs `git -c http.cookiefile=$cookiefile remote-https $URL`
The way 2FA works is the user authenticates to a special internal SSO
management point in their web browser once per period (I decline to
say how often but its tolerable). Users typically are presented this
SSO page anyway by other applications they visit, or they bookmark the
main entry point. A Chrome or Firefox extension has been installed and
authorized to steal cookies from this host. The extension writes the
user's cookie to a local file on disk. Our git-remote-sso tool uses
this cookie file to setup per-host cookies on demand within the
authentication period.
Horrifically hacky. It would be nice if this was more integrated into
Git itself, where the cookies could be acquired/refreshed through the
credential helper system rather than wrapping git-remote-https with a
magical URL. I am a fan of the way our extension manages to get the
token conveyed automatically for me. Much easier than the OAuth
flows[2], but harder to replicate in the wild. Our IT group makes sure
the extension is installed on workstations as part of the base OS
image.
[2] https://developers.google.com/storage/docs/gsutil_install#authenticate
^ permalink raw reply
* Re: Git Merge 2013 Conference, Berlin
From: Scott Chacon @ 2013-02-19 22:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael J Gruber, Jeff King, Thomas Rast, git list
In-Reply-To: <7vk3q43si2.fsf@alter.siamese.dyndns.org>
Hey,
On Tue, Feb 19, 2013 at 1:26 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Scott Chacon <schacon@gmail.com> writes:
>
>> On Tue, Feb 19, 2013 at 11:19 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Scott Chacon <schacon@gmail.com> writes:
>>>
>>>> On Tue, Feb 19, 2013 at 7:41 AM, Michael J Gruber
>>>> <git@drmicha.warpmail.net> wrote:
>>>>> Michael J Gruber venit, vidit, dixit 19.02.2013 16:20:
>>>>>> Well, all days are listed as "sold out" on the eventbrite site. Maybe
>>>>>> it's because eventbrite has "trouble connecting to facebook" because I
>>>>>> "don't have facebook"?
>>>>
>>>> No, it's because 300 people signed up and that's all the venue has
>>>> room for. I'm sure we can fit one more if you come.
>>> ...
>> 300 is the number of people signed up for the User Day. There is a
>> Dev Day, for contributors, which has 50 signed up.
>
> Ah, sorry I misunderstood. So your 300 above was about the user
> day, and Dev day has different capacity but has already sold out at
> 50 seats.
Yes. There is only so much room and I didn't want to overload the dev
day. Those of you who want to come on any of these days are still
welcome, I just don't want more random people signing up. If you
still wish to attend, simply email me personally and I will add you.
Junio, are you interested in attending?
Scott
^ permalink raw reply
* Re: [PATCH] Bugfix: undefined htmldir in config.mak.autogen
From: Junio C Hamano @ 2013-02-19 22:59 UTC (permalink / raw)
To: Jiang Xin; +Cc: Christoph J. Thompson, Git List
In-Reply-To: <1be0a520b99fbfecd7255c1d26753015612856b3.1361272864.git.worldhello.net@gmail.com>
Jiang Xin <worldhello.net@gmail.com> writes:
> Html documents will be installed to root dir (/) no matter what prefix
> is set, if run these commands before `make` and `make install-html`:
>
> $ make configure
> $ ./configure --prefix=<PREFIX>
>
> After the installation, all the html documents will copy to rootdir (/),
> and:
>
> $ git --html-path
> <PREFIX>
>
> $ git help -w something
> fatal: '<PREFIX>': not a documentation directory.
I am not sure if this description is correct. The generated configure
seems to set
datarootdir='${prefix}/share'
htmldir='${docdir}'
docdir='${datarootdir}/doc/${PACKAGE_TARNAME}'
so it is likely you would get not <PREFIX> but <PREFIX>/share, no?
In the main Makefile, we set htmldir to "share/doc/git-doc" and that
is supposed to be relative to PREFIX, so the above will be wrong in
multiple ways (it is an absolute path with <PREFIX>/ in front, and
it ends not with share/doc/git-doc but with share/doc/git).
And the worst part is that having to know that the file needs to
export docdir and PACKAGE_TARNAME feels to me that we are tying
ourselves to too much detail in the internal implementation detail
of versions of autoconf we happen to have for testing this change.
I am inclined to suggest that we probably should
* revert fc1c5415d69d (Honor configure's htmldir switch,
2013-02-02); and
* fix generated "./configure --help" not to suggest that --htmldir
can be overriden from its command line;
instead of piling on a broken "fix" like this one top of it.
^ permalink raw reply
* Re: [PATCH] Bugfix: undefined htmldir in config.mak.autogen
From: Junio C Hamano @ 2013-02-19 23:40 UTC (permalink / raw)
To: Jiang Xin; +Cc: Christoph J. Thompson, Git List
In-Reply-To: <7v8v6j52qf.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Jiang Xin <worldhello.net@gmail.com> writes:
>
>> Html documents will be installed to root dir (/) no matter what prefix
>> is set, if run these commands before `make` and `make install-html`:
>>
>> $ make configure
>> $ ./configure --prefix=<PREFIX>
>>
>> After the installation, all the html documents will copy to rootdir (/),
>> and:
>>
>> $ git --html-path
>> <PREFIX>
>>
>> $ git help -w something
>> fatal: '<PREFIX>': not a documentation directory.
>
> I am not sure if this description is correct. The generated configure
> seems to set
>
> datarootdir='${prefix}/share'
> htmldir='${docdir}'
> docdir='${datarootdir}/doc/${PACKAGE_TARNAME}'
>
> so it is likely you would get not <PREFIX> but <PREFIX>/share, no?
This was a mis-diag; without docdir mentioned in config.mak.in, we
do not even get that far, and htmldir will end up being empty, and
the runtime code adds <PREFIX> to it in system_path(). What I was
describing was what happens when you only mention @docdir@ but not
PACKAGE_TARNAME in the file.
> And the worst part is that having to know that the file needs to
> export docdir and PACKAGE_TARNAME feels to me that we are tying
> ourselves to too much detail in the internal implementation detail
> of versions of autoconf we happen to have for testing this change.
This still stands. It really feels wrong that this file has to be
aware of such an implementation detail of autoconf. But as an
interim workaround, setting these two otherwise unused variables may
be the best we could do.
I am not sure if such a layout can be actually used for installing,
though. Didn't we see some issues around the relativeness of
htmldir and mandir vs passing them down to Documentation/Makefile,
or is it not an issue when ./configure and config.mak.autogen is
used?
^ permalink raw reply
* Re: Git Merge 2013 Conference, Berlin
From: Junio C Hamano @ 2013-02-19 23:47 UTC (permalink / raw)
To: Scott Chacon; +Cc: Michael J Gruber, Jeff King, Thomas Rast, git list
In-Reply-To: <CAP2yMaJhFqLiu1KSNfWwxAUJb6bGwV+uSURQm0uWWwFO3Y=Wsw@mail.gmail.com>
Scott Chacon <schacon@gmail.com> writes:
> Junio, are you interested in attending?
I am interested in meeting our European contributors, but Berlin is
kind of very far, so give me a few days to think about it.
Thanks.
^ permalink raw reply
* [Patch] gitk: added a file history browser
From: Dirk @ 2013-02-19 23:14 UTC (permalink / raw)
To: paulus; +Cc: git
[-- Attachment #1: Type: text/plain, Size: 372 bytes --]
Hi,
I added a file history browser for a single file in the gitk. I use git log with the option --follow. This option is useful for the user switch from cvs to git. He misses a history from only one file and the function "Highlight this only" breaks by moved files. The new option do not break by the moved files. I use ideas from the tkcvs branch dialog.
Dirk
[-- Attachment #2: 0001-Add-a-file-history-browser.patch --]
[-- Type: application/octet-stream, Size: 29671 bytes --]
From 60ff82dbcf1cfd07af515940c474a8c2efb97443 Mon Sep 17 00:00:00 2001
From: Dirk <dirk@freunde-aus-berlin.net>
Date: Tue, 19 Feb 2013 23:28:34 +0100
Subject: [PATCH] Add a file history browser
The file history browser used the git log with the option --follow
and show the diff. Use the ideas from tkcvs branch window.
This option is helpful for the users changed from cvs to git.
---
gitk | 574 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 573 insertions(+), 1 deletions(-)
diff --git a/gitk b/gitk
index b3706fc..52c13c8 100755
--- a/gitk
+++ b/gitk
@@ -2584,6 +2584,7 @@ proc makewindow {} {
makemenu $flist_menu {
{mc "Highlight this too" command {flist_hl 0}}
{mc "Highlight this only" command {flist_hl 1}}
+ {mc "File history browser for this only" command {flist_hb 0}}
{mc "External diff" command {external_diff}}
{mc "Blame parent commit" command {external_blame 1}}
}
@@ -3352,7 +3353,7 @@ proc pop_flist_menu {w X Y x y} {
set xdiffstate "disabled"
}
# Disable "External diff" item in tree mode
- $flist_menu entryconf 2 -state $xdiffstate
+ $flist_menu entryconf 3 -state $xdiffstate
tk_popup $flist_menu $X $Y
}
@@ -11982,6 +11983,577 @@ if {[tk windowingsystem] eq "win32"} {
getcommits {}
+# Bindings to make canvases scroll. Canvases have no bindings at all
+# by default.
+proc scrollbindings {bindtag} {
+ # Page keys
+ bind $bindtag <ButtonPress-1> [list focus %W]
+ bind $bindtag <Next> [list %W yview scroll 1 pages]
+ bind $bindtag <Prior> [list %W yview scroll -1 pages]
+ bind $bindtag <Up> [list %W yview scroll -1 units]
+ bind $bindtag <Down> [list %W yview scroll 1 units]
+ bind $bindtag <Left> [list %W xview scroll -1 pages]
+ bind $bindtag <Right> [list %W xview scroll 1 pages]
+ # Middle button dragging
+ bind $bindtag <B2-Motion> [list dragbind %W %x %y]
+ # Wheelmouse
+ bind $bindtag <MouseWheel> [list wheelbind %W %D]
+ bind $bindtag <ButtonPress-4> [list %W yview scroll -1 pages]
+ bind $bindtag <ButtonPress-5> [list %W yview scroll 1 pages]
+}
+
+
+proc diff_revision {revA revB} {
+ global flist_menu_file
+ global extdifftool
+ global id2file
+
+ set fileA $id2file($revA)
+ set fileB $id2file($revB)
+
+ set diffdir [gitknewtmpdir]
+
+ if {$diffdir eq {}} return
+
+ # gather files to diff
+ set difffromfile [external_diff_get_one_file $revA $fileA $diffdir]
+ set difftofile [external_diff_get_one_file $revB $fileB $diffdir]
+
+ if {$difffromfile ne {} && $difftofile ne {}} {
+ set cmd [list [shellsplit $extdifftool] $difffromfile $difftofile]
+ if {[catch {set fl [open |$cmd r]} err]} {
+ file delete -force $diffdir
+ error_popup "$extdifftool: [mc "command failed:"] $err"
+ } else {
+ fconfigure $fl -blocking 0
+ filerun $fl [list delete_at_eof $fl $diffdir]
+ }
+ }
+}
+
+namespace eval ::logcanvas {
+ variable instance 0
+
+ proc new {filename how scope} {
+ #
+ # Creates a new log canvas.
+ #
+ variable instance
+ set my_idx $instance
+ incr instance
+ global current_tagname
+ global module_dir
+
+ variable sys
+ variable loc
+
+ namespace eval $my_idx {
+ set my_idx [uplevel {concat $my_idx}]
+ set how [uplevel {concat $how}]
+ set filename [uplevel {concat $filename}]
+ set scope [uplevel {concat $scope}]
+
+ #variable cmd_log
+ # Global constants scaled by current scaling factor for this instance
+ variable curr
+ global tcl_platform
+ # User options for info display for this instance
+ variable opt
+ variable revwho
+ variable revdate
+ variable revtime
+ variable revstate
+ variable revbranches
+ variable branchrevs
+ variable revcomment
+ variable revtags
+ variable revbtags
+ variable revpath
+ variable sel_tag
+ set sel_tag(A) {}
+ set sel_tag(B) {}
+ variable sel_rev
+ variable revnum_current
+ set sel_rev(A) {}
+ set sel_rev(B) {}
+ variable logcanvas ".logcanvas$my_idx"
+
+ set sys_loc [split $how {,}]
+ set sys [lindex $sys_loc 0]
+ set loc [lindex $sys_loc 1]
+
+ proc SetCommitIds {idList} {
+ variable commitIds
+
+ set commitIds $idList
+ }
+
+ proc GetCommitter {id} {
+ variable committer
+ set committer [exec git log --pretty=format:%an -n 1 $id]
+ return $committer
+ }
+
+ proc GetDate {id} {
+ variable date
+ set date [exec git log --pretty=format:%ad -n 1 $id]
+ return $date
+ }
+
+ proc GetLogMessage {id} {
+ variable msg
+ set msg [exec git log --pretty=format:%B -n 1 $id]
+ return $msg
+ }
+
+ proc GetShortCommitId {id} {
+ variable sid
+ set sid [exec git log --pretty=format:%h -n 1 $id]
+ return $sid
+ }
+
+ proc GetDrawText { sid committer date } {
+ variable text
+ set text {}
+ append text $sid
+ append text "\n"
+ append text $committer
+ append text "\n"
+ append text $date
+ append text "\n"
+ return $text
+ }
+
+ proc ClearSelection {AorB} {
+ variable logcanvas
+ variable sel_tag
+ variable sel_rev
+ #catch {$logcanvas.canvas itemconfigure Sel$AorB -outline black}
+ catch {$logcanvas.canvas itemconfigure Sel$AorB -fill gray90}
+ $logcanvas.canvas dtag Sel$AorB
+ $logcanvas.up.rev${AorB}_rvers configure -text {}
+ $logcanvas.up.log${AorB}_rlogfm.rcomment configure -state normal
+ $logcanvas.up.log${AorB}_rlogfm.rcomment delete 1.0 end
+ $logcanvas.up.log${AorB}_rlogfm.rcomment configure -state disabled
+ $logcanvas.up.rev${AorB}_rwho configure -text {}
+ $logcanvas.up.rev${AorB}_rdate configure -text {}
+ set sel_tag($AorB) {}
+ set sel_rev($AorB) {}
+ return
+ }
+
+ proc SetSelection {AorB revision} {
+ variable logcanvas
+ variable revdate
+ variable sel_rev
+
+ ClearSelection $AorB
+ set other [expr {$AorB == "A" ? {B} : {A}}]
+ if {$revision == $sel_rev($other)} { ClearSelection $other }
+
+ $logcanvas.up.rev${AorB}_rvers configure -text $revision
+
+ if {$revision != {} } {
+ $logcanvas.up.rev${AorB}_rwho configure -text [GetCommitter $revision]
+ $logcanvas.up.log${AorB}_rlogfm.rcomment configure -state normal
+ $logcanvas.up.log${AorB}_rlogfm.rcomment insert end [GetLogMessage $revision]
+ $logcanvas.up.log${AorB}_rlogfm.rcomment configure -state disabled
+ }
+ $logcanvas.canvas itemconfigure SelA -fill Green
+ $logcanvas.canvas itemconfigure SelB -fill Red
+ set sel_rev($AorB) $revision
+ return
+ }
+
+ proc RevSelect {AorB} {
+ variable logcanvas
+ set t [$logcanvas.canvas gettags current]
+ SetSelection $AorB \
+ [string range [lindex $t [lsearch -glob $t {R*}]] 1 end]
+ return
+ }
+
+ proc DiffRevision { revA revB filename } {
+ diff_revision $revA $revB
+ }
+
+ proc DrawCurrent { x y box_width box_height revision } {
+
+ variable font_bold
+ variable curr_x
+ variable curr_y
+ variable logcanvas
+
+ set curr_x $x
+ set curr_y $y
+ # draw the box
+ set tx [expr {$x + $box_width}]
+ set ty [expr {$y - $box_height}]
+ $logcanvas.canvas create rectangle \
+ $x $y $tx $ty \
+ -width 4 -fill gray90 -outline red3
+ set pad \
+ [expr {($box_width - \
+ [font measure $font_bold -displayof $logcanvas.canvas {You are}]) \
+ / 3}]
+ set ty [expr {$y - [expr {$box_height/2}]}]
+ # add the contents
+ $logcanvas.canvas create text \
+ [expr {$x + $box_width - $pad}] $ty \
+ -text "You are\nhere" -anchor e \
+ -fill red3 \
+ -font $font_bold
+ return
+ }
+
+ proc DrawRevision { x y box_width box_height revision} {
+ variable font_bold
+ variable curr_x
+ variable curr_y
+ variable logcanvas
+
+ set committer [GetCommitter $revision]
+ set date [GetDate $revision]
+ set sid [GetShortCommitId $revision]
+ set text [GetDrawText $sid $committer $date]
+
+ set curr_x $x
+ set curr_y $y
+ # draw the box
+ set tx [expr {$x + $box_width}]
+ set ty [expr {$y - $box_height}]
+ $logcanvas.canvas create rectangle \
+ $x $y $tx $ty \
+ -width 4 -fill gray90 -outline red3 \
+ -tag [list box R$revision rect$revision active]
+ set pad \
+ [expr {($box_width -[font measure $font_bold \
+ -displayof $logcanvas.canvas {$text}]) }]
+ set ty [expr {$y - [expr {$box_height/2}]}]
+ # add the contents
+ $logcanvas.canvas create text \
+ [expr {$x + $pad}] $ty \
+ -text $text -anchor e \
+ -fill red3 \
+ -font $font_bold \
+ -tags [list R$revision box active]
+ return
+ }
+
+ proc DrawTree { filename } {
+ variable font_norm
+ variable font_norm_h
+ variable font_bold
+ variable font_bold_h
+ variable logcanvas
+ variable commitIds
+
+ set my_size 8
+ set font_norm [font create \
+ -family Helvetica -size $my_size]
+ set font_norm_h [font metrics \
+ $font_norm -displayof $logcanvas -linespace]
+ set font_bold [font create \
+ -family Helvetica -size $my_size -weight bold]
+ set font_bold_h [font metrics \
+ $font_bold -displayof $logcanvas -linespace]
+
+ set height 100
+
+ DrawCurrent 100 $height 300 100 "bla"
+
+ foreach id $commitIds {
+ set height [expr $height + 110]
+ DrawRevision 100 $height 300 100 $id
+ }
+ }
+
+ toplevel $logcanvas
+ wm title $logcanvas "$sys Log $filename"
+ $logcanvas configure -menu $logcanvas.menubar
+ menu $logcanvas.menubar
+
+ $logcanvas.menubar add cascade -label "File"\
+ -menu $logcanvas.menubar.file -underline 0
+ menu $logcanvas.menubar.file -tearoff 0
+ $logcanvas.menubar.file add command -label "Close" -underline 0 \
+ -command [namespace code {$logcanvas.close invoke}]
+ $logcanvas.menubar.file add command -label "Exit" -underline 1 \
+ -command { exit }
+ wm protocol $logcanvas WM_DELETE_WINDOW \
+ [namespace code {$logcanvas.close invoke}]
+ frame $logcanvas.up -relief groove -border 2
+ set disbg [lindex [$logcanvas.up configure -background] 4]
+ label $logcanvas.up.lfname -width 12 -anchor w
+ entry $logcanvas.up.rfname -font $textfont -relief groove
+
+ pack $logcanvas.up -side top -fill x
+ foreach fm {A B} {
+ label $logcanvas.up.rev${fm}_lvers -text "Revision $fm"
+ label $logcanvas.up.rev${fm}_rvers -text {} \
+ -anchor w -font $textfont
+
+ label $logcanvas.up.rev${fm}_ldate -text "Committed"
+ label $logcanvas.up.rev${fm}_rdate -text {} \
+ -anchor w -font $textfont
+ label $logcanvas.up.rev${fm}_lwho -text " by "
+ label $logcanvas.up.rev${fm}_rwho -text {} \
+ -anchor w -font $textfont
+ label $logcanvas.up.log${fm}_lcomment -text "Log $fm"
+
+ frame $logcanvas.up.log${fm}_rlogfm -bd 3
+ text $logcanvas.up.log${fm}_rlogfm.rcomment -height 5 \
+ -state disabled \
+ -yscrollcommand [namespace code\
+ "$logcanvas.up.log${fm}_rlogfm.yscroll set"]
+ scrollbar $logcanvas.up.log${fm}_rlogfm.yscroll \
+ -command [namespace code\
+ "$logcanvas.up.log${fm}_rlogfm.rcomment yview"]
+ }
+
+ grid columnconf $logcanvas.up 5 -weight 1
+ grid $logcanvas.up.lfname -column 0 -row 0 -sticky nw
+ grid $logcanvas.up.rfname -column 1 -row 0 -columnspan 5 -sticky ew
+ grid $logcanvas.up.revA_lvers -column 0 -row 1 -sticky w
+ grid $logcanvas.up.revA_rvers -column 1 -row 1 -sticky w
+ grid $logcanvas.up.revA_ldate -column 2 -row 1 -sticky w
+ grid $logcanvas.up.revA_rdate -column 3 -row 1 -sticky w
+ grid $logcanvas.up.revA_lwho -column 4 -row 1 -sticky w
+ grid $logcanvas.up.revA_rwho -column 5 -row 1 -sticky ew
+ grid $logcanvas.up.logA_lcomment -column 0 -row 2 -sticky nw
+ grid $logcanvas.up.logA_rlogfm -column 1 -row 2 -columnspan 7 -sticky ew
+ pack $logcanvas.up.logA_rlogfm.yscroll -side right -fill y
+ pack $logcanvas.up.logA_rlogfm.rcomment -side left -fill x -expand y
+ grid $logcanvas.up.revB_lvers -column 0 -row 3 -sticky w
+ grid $logcanvas.up.revB_rvers -column 1 -row 3 -sticky w
+ grid $logcanvas.up.revB_ldate -column 2 -row 3 -sticky w
+ grid $logcanvas.up.revB_rdate -column 3 -row 3 -sticky w
+ grid $logcanvas.up.revB_lwho -column 4 -row 3 -sticky w
+ grid $logcanvas.up.revB_rwho -column 5 -row 3 -sticky ew
+ grid $logcanvas.up.logB_lcomment -column 0 -row 4 -sticky nw
+ grid $logcanvas.up.logB_rlogfm -column 1 -row 4 -columnspan 7 -sticky ew
+ pack $logcanvas.up.logB_rlogfm.yscroll -side right -fill y
+ pack $logcanvas.up.logB_rlogfm.rcomment -side left -fill x -expand y
+ # Pack the bottom before the middle so it doesnt disappear if
+ # the window is resized smaller
+ frame $logcanvas.down -relief groove -border 2
+ pack $logcanvas.down -side bottom -fill x
+
+ # The canvas for the big picture
+ canvas $logcanvas.canvas -relief sunken -border 2 \
+ -height 300 \
+ -yscrollcommand [namespace code "$logcanvas.yscroll set"] \
+ -xscrollcommand [namespace code "$logcanvas.xscroll set"]
+ scrollbar $logcanvas.xscroll -relief sunken -orient horizontal \
+ -command [namespace code "$logcanvas.canvas xview"]
+ scrollbar $logcanvas.yscroll -relief sunken \
+ -command [namespace code "$logcanvas.canvas yview"]
+ #
+ # Create buttons
+ #
+ button $logcanvas.diff -text "Diff"\
+ -command [namespace code {
+ DiffRevision [$logcanvas.up.revA_rvers cget -text] \
+ [$logcanvas.up.revB_rvers cget -text] $filename
+ }]
+ button $logcanvas.close -text "Close" \
+ -command [namespace code {
+ variable logcanvas
+ destroy $logcanvas
+ namespace delete [namespace current]
+ }]
+
+ #
+ # Put the canvas on to the display.
+ #
+ pack $logcanvas.xscroll -side bottom -fill x -padx 1 -pady 1
+ pack $logcanvas.yscroll -side right -fill y -padx 1 -pady 1
+ pack $logcanvas.canvas -fill both -expand 1
+ scrollbindings $logcanvas.canvas
+
+ pack $logcanvas.diff \
+ -in $logcanvas.down -side left \
+ -ipadx 4 -ipady 4
+ pack $logcanvas.close \
+ -in $logcanvas.down -side right \
+ -fill both -expand 1
+
+ #
+ # Window manager stuff.
+ #
+ wm minsize $logcanvas 1 1
+
+ $logcanvas.canvas bind active <Enter> \
+ "$logcanvas.canvas config -cursor hand2"
+ $logcanvas.canvas bind active <Leave> \
+ "$logcanvas.canvas config -cursor {}"
+
+
+ $logcanvas.canvas bind tag <Button-1> \
+ [namespace code "PopupTags %X %Y"]
+
+ $logcanvas.canvas bind box <ButtonPress-1> \
+ [namespace code "RevSelect A"]
+ # Tcl/TK for Windows doesn't do Button 3, so we duplicate it on Button 2
+ $logcanvas.canvas bind box <ButtonPress-2> \
+ [namespace code "RevSelect B"]
+ $logcanvas.canvas bind box <ButtonPress-3> \
+ [namespace code "RevSelect B"]
+
+ focus $logcanvas.canvas
+ # FIXME: Why isn't there a bbox when we get here?
+ # Then the yview moveto doesn't work, although it does in tkinter
+ $logcanvas.canvas xview moveto 0
+ $logcanvas.canvas yview moveto 0
+
+ return [list [namespace current] $logcanvas]
+ }
+ }
+}
+
+
+proc flist_hb {only} {
+ global flist_menu_file
+
+ set x [shellquote $flist_menu_file]
+ git_file_history $x
+}
+
+proc git_file_history {file} {
+
+ if {$file == ""} {
+ error_popup "[mc "File history browser for file %s not implemented yet" $file]"
+ return
+ }
+
+ ::git_filelog::new "Git,loc" "$file"
+}
+
+namespace eval ::git_filelog {
+ variable instance 0
+
+ proc new {how filename} {
+ variable instance
+ variable commit_ids
+ set my_idx $instance
+ incr instance
+
+ namespace eval $my_idx {
+ set filename [uplevel {concat $filename}]
+ set how [uplevel {concat $filename}]
+
+ variable command
+ variable rev_list
+ variable rev_ids
+ variable rev_id
+ variable rev_date
+ variable rev_comment
+ variable rev_commiter
+ variable id 0
+ variable lc
+
+ set newlc [logcanvas::new $filename $how [namespace current]]
+ set ln [lindex $newlc 0]
+ set lc [lindex $newlc 1]
+
+ set command "git log --follow --pretty=format:%H --name-only -- $filename"
+ if {[catch {set fd [open [concat | $command] r]} err]} {
+ error_popup "[mc "Error executing git log:"] $err"
+ return 0
+ }
+ #set i [reg_instance $fd]
+ #fconfigure $fd -blocking 1
+ #filerun $fd [list eval_commit_id $fd $ln]
+ eval_commit_id $fd $ln
+
+ $ln\::DrawTree $filename
+
+ }
+ return [namespace current]
+ }
+}
+
+proc eval_commit_id {fd ln} {
+
+ set stuff [read $fd 500000]
+ if {$stuff == {} && [eof $fd]} {
+ set stuff "\0"
+ }
+ if {$stuff == {}} {
+ if {![eof $fd]} {
+ return 1
+ }
+ # set it blocking so we wait for the process to terminate
+ fconfigure $fd -blocking 1
+ if {[catch {close $fd} err]} {
+ set fv {}
+ if {[string range $err 0 4] == "usage"} {
+ set err "Gitk: error reading commits$fv:\
+ bad arguments to git log."
+ } else {
+ set err "Error reading commits$fv: $err"
+ }
+ error_popup $err
+ }
+ return 0
+ }
+ set start 0
+ set gotsome 0
+ set commit_ids {}
+
+ # every commid has 3 lines
+ # first line: commit id
+ # second line: file name
+ # third line: empty line
+
+ set line 0
+ set id {}
+ global id2file
+
+ while 1 {
+ set i [string first "\n" $stuff $start]
+ incr line
+ if {$i < 0} {
+ set cmit [string range $stuff $start end]
+ if { $line == 1 } {
+ lappend commit_ids $cmit
+ set id $cmit
+ }
+ if { $line == 2 } {
+ set pos [llength $commit_ids]
+ set id [lindex $commit_ids $pos]
+ set id2file($id) $cmit
+ }
+ break
+ }
+ if {$start == 0} {
+ set cmit [string range $stuff 0 [expr {$i - 1}]]
+ if { $line == 1 } {
+ lappend commit_ids $cmit
+ set id $cmit
+ }
+ if { $line == 2 } {
+ set id2file($id) $cmit
+ }
+ } else {
+ set cmit [string range $stuff $start [expr {$i - 1}]]
+ if { $line == 1 } {
+ lappend commit_ids $cmit
+ set id $cmit
+ }
+ if { $line == 2 } {
+ set id2file($id) $cmit
+ }
+ }
+ if { $line == 3 } {
+ set line 0
+ }
+ set start [expr {$i + 1}]
+ }
+
+ $ln\::SetCommitIds $commit_ids
+}
+
# Local variables:
# mode: tcl
# indent-tabs-mode: t
--
1.7.7.5 (Apple Git-26)
^ permalink raw reply related
* Re: [PATCH v2 2/2] check-ignore.c, dir.c: fix segfault with '.' argument from repo root
From: Adam Spiers @ 2013-02-20 1:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git list, Zoltan Klinger
In-Reply-To: <7vzjz03wid.fsf@alter.siamese.dyndns.org>
On Tue, Feb 19, 2013 at 7:59 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Adam Spiers <git@adamspiers.org> writes:
>
>> Fix a corner case where check-ignore would segfault when run with the
>> '.' argument from the top level of a repository, due to prefix_path()
>> converting '.' into the empty string.
>
> The description does not match what I understand is happening from
> the original report, though.
Why not?
> The above is more like this, no?
>
> When check-ignore is run with the '.' argument from the top level of
> a repository, it fed an empty string to hash_name() in name-hash.c
> and caused a segfault, as the function kept reading forever past the
> end of the string.
The only difference I can see between the two is that yours has
changed the phrase order and gives a bit more information. I don't
see any disagreement between them though.
> A point to note is that it is not cleaer why it is a corner case to
> ask about a pathspec ".". It is a valid question "Is the whole tree
> ignored by default?", isn't it?
It's probably valid, but I'm not the best judge of that. It doesn't
make much sense to me why anyone would do that, and it seems very
unlikely it would be a common use case, therefore it's a corner case.
The next sentence then qualifies that:
>> It doesn't make much sense to
>> call check-ignore from the top level with '.' as a parameter, since
>> the top-level directory would never typically be ignored,
>
> And this sounds like a really bad excuse.
An excuse for what? The final part of that sentence which you trimmed
made it clear that it was not an excuse for the segfault. Your choice
of wording here sounds more like a personal attack than a technical
discussion - presumably unintentional, so I'll choose not to take
offense.
> If it were "it does not
> make *any* sense ... because the top level is *never* ignored", then
> the patch is a perfectly fine optimization that happens to work
> around the problem, but the use of "much" and "typically" is a sure
> sign that the design of the fix is iffy. It also shows that the
> patch is not a fix, but is sweeping the problem under the rug, if
> there were a valid use case to set the top level to be ignored.
There *may* be a valid use case to set the top level to be ignored.
*I* can't think of one personally, therefore it doesn't make sense to
me to do so, but my intention was to avoid imposing my own personal
judgement on everyone else by preventing people from doing that.
However, in that case I now realise that check-ignore should report a
match, and my proposed patches do not do that. Perhaps that is what
you meant by "sweeping the problem under the rug"?
> I wonder what would happen if we removed that "&& path[0]" check
> from the caller, not add the "assume the top is never ignored"
> workaround, and do something like this to the location that causes
> segv, so that it can give an answer when asked to hash an empty
> string?
>
> name-hash.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/name-hash.c b/name-hash.c
> index d8d25c2..942c459 100644
> --- a/name-hash.c
> +++ b/name-hash.c
> @@ -24,11 +24,11 @@ static unsigned int hash_name(const char *name, int namelen)
> {
> unsigned int hash = 0x123;
>
> - do {
> + while (namelen--) {
> unsigned char c = *name++;
> c = icase_hash(c);
> hash = hash*101 + c;
> - } while (--namelen);
> + }
> return hash;
> }
Yep, that makes sense - that's what I meant when I said I was
wondering "whether hash_name() should do stricter input validation".
> Does the callchain that goes down to this function have other places
> that assume they will never see an empty string, like this function
> does, which I _think_ is the real issue?
Good question. In the absence of a proper audit for similar issues,
it definitely makes sense to defensively program hash_name() (and any
other low-level functions which accept path strings).
^ permalink raw reply
* Re: [PATCH] Bugfix: undefined htmldir in config.mak.autogen
From: Jiang Xin @ 2013-02-20 1:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Christoph J. Thompson, Git List
In-Reply-To: <7v4nh750v3.fsf@alter.siamese.dyndns.org>
2013/2/20 Junio C Hamano <gitster@pobox.com>:
> Junio C Hamano <gitster@pobox.com> writes:
>>> After the installation, all the html documents will copy to rootdir (/),
>>> and:
>>>
>>> $ git --html-path
>>> <PREFIX>
>>>
>>> $ git help -w something
>>> fatal: '<PREFIX>': not a documentation directory.
>>
>> I am not sure if this description is correct. The generated configure
>> seems to set
>>
>> datarootdir='${prefix}/share'
>> htmldir='${docdir}'
>> docdir='${datarootdir}/doc/${PACKAGE_TARNAME}'
>>
>> so it is likely you would get not <PREFIX> but <PREFIX>/share, no?
>
> This was a mis-diag; without docdir mentioned in config.mak.in, we
> do not even get that far, and htmldir will end up being empty, and
> the runtime code adds <PREFIX> to it in system_path(). What I was
> describing was what happens when you only mention @docdir@ but not
> PACKAGE_TARNAME in the file.
I also doubt about it after sleep, so I check it again:
## gettext is installed in non-standard location on Mac
$ export CFLAGS=-I/usr/local/include; export LDFLAGS=-L/usr/local/lib
$ make config
$ ./configure --prefix=/opt/git/v1.8.2
$ make && sudo make install
## already symlink /opt/git/v1.8.2/bin/* to /usr/local/bin/
$ git --html-path
/opt/git/v1.8.2/
$ git help -w help
fatal: '/opt/git/v1.8.2/': not a documentation directory.
>> And the worst part is that having to know that the file needs to
>> export docdir and PACKAGE_TARNAME feels to me that we are tying
>> ourselves to too much detail in the internal implementation detail
>> of versions of autoconf we happen to have for testing this change.
I am not familiar with autoconf. After clone autoconf and check,
I cannot find a neat way to change "htmldir" default location to
use ${datarootdir} (just like mandir).
In file "lib/autoconf/general.m4", there are:
AC_SUBST([docdir], [m4_ifset([AC_PACKAGE_TARNAME],
['${datarootdir}/doc/${PACKAGE_TARNAME}'],
['${datarootdir}/doc/${PACKAGE}'])])dnl
...
AC_SUBST([htmldir], ['${docdir}'])dnl
...
AC_SUBST([pdfdir], ['${docdir}'])dnl
...
AC_SUBST([mandir], ['${datarootdir}/man'])dnl
> This still stands. It really feels wrong that this file has to be
> aware of such an implementation detail of autoconf. But as an
> interim workaround, setting these two otherwise unused variables may
> be the best we could do.
>
> I am not sure if such a layout can be actually used for installing,
> though. Didn't we see some issues around the relativeness of
> htmldir and mandir vs passing them down to Documentation/Makefile,
> or is it not an issue when ./configure and config.mak.autogen is
> used?
--
蒋鑫
北京群英汇信息技术有限公司
邮件: worldhello.net@gmail.com
网址: http://www.ossxp.com/
博客: http://www.worldhello.net/
微博: http://weibo.com/gotgit/
电话: 010-51262007, 18601196889
^ permalink raw reply
* Re: [PATCH v2 2/2] check-ignore.c, dir.c: fix segfault with '.' argument from repo root
From: Adam Spiers @ 2013-02-20 1:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git list, Zoltan Klinger
In-Reply-To: <7vfw0s3qsq.fsf@alter.siamese.dyndns.org>
On Tue, Feb 19, 2013 at 02:03:01PM -0800, Junio C Hamano wrote:
> I started to suspect that may be the right approach. Why not do this?
>
> -- >8 --
> From: Junio C Hamano <gitster@pobox.com>
> Date: Tue, 19 Feb 2013 11:56:44 -0800
> Subject: [PATCH] name-hash: allow hashing an empty string
>
> Usually we do not pass an empty string to the function hash_name()
> because we almost always ask for hash values for a path that is a
> candidate to be added to the index. However, check-ignore (and most
> likely check-attr, but I didn't check) apparently has a callchain
> to ask the hash value for an empty path when it was given a "." from
> the top-level directory to ask "Is the path . excluded by default?"
According to a single gdb run, 'git check-attr -a .' does not hit
hash_name() for me. However that naive experiment doesn't rule out
the possibility of it happening if the right attributes are set, but I
don't know enough about that code to comment.
> Make sure that hash_name() does not overrun the end of the given
> pathname even when it is empty.
>
> Remove a sweep-the-issue-under-the-rug conditional in check-ignore
> that avoided to pass an empty string to the callchain while at it.
> It is a valid question to ask for check-ignore if the top-level is
> set to be ignored by default, even though the answer is most likely
Hmm, I see very little difference between the use of "most likely" and
the use of the words "much" and "typically" which you previously
considered "a sure sign that the design of the fix is iffy".
> no, if only because there is currently no way to specify such an
> entry in the .gitignore file. But it is an unusual thing to ask and
> it is not worth optimizing for it by special casing at the top level
> of the call chain.
Although I agree with your proposed patch's sentiment of avoiding
sweeping this corner case under the rug, 'check-ignore .' still
wouldn't match anything if for example './' was a supported mechanism
for ignoring the top level. So, modulo the obvious advantages that
defensive coding in hash_name() bring, I'm struggling to see a
significant difference between any of the three patches proposed so
far. All of them fix the segfault, and all would require the same
amount of extra work in order to support matching against './'.
But I don't have any strong objections either, so you have my approval
for whichever patch you prefer to take. Thanks.
^ 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