* Re: [PATCH (BUGFIX)] gitweb: Handle invalid regexp in regexp search
From: Junio C Hamano @ 2012-02-28 19:45 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, Ramsay Jones
In-Reply-To: <20120228183919.26435.86795.stgit@localhost.localdomain>
Jakub Narebski <jnareb@gmail.com> writes:
> When using regexp search ('sr' parameter / $search_use_regexp variable
> is true), check first that regexp is valid.
Thanks.
How old is this bug? Should it go to older maitenance tracks like 1.7.6?
^ permalink raw reply
* Re: [PATCH] Documentation: use {asterisk} in rev-list-options.txt when needed
From: Junio C Hamano @ 2012-02-28 19:38 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: git
In-Reply-To: <1330443348-5742-1-git-send-email-cmn@elego.de>
Thanks.
^ permalink raw reply
* Re: [PATCH 2/3] http: try standard proxy env vars when http.proxy config option is not set
From: Jeff King @ 2012-02-28 19:34 UTC (permalink / raw)
To: Sam Vilain; +Cc: Nelson Benitez Leon, Thomas Rast, git, sam.vilain
In-Reply-To: <4F4D2AAD.3040107@vilain.net>
On Tue, Feb 28, 2012 at 11:27:41AM -0800, Sam Vilain wrote:
> On 2/28/12 11:15 AM, Jeff King wrote:
> >Usually we would prefer environment variables to config. So that:
> >
> > $ git config http.proxy foo
> > $ HTTP_PROXY=bar git fetch
> >
> >would use "bar" as the proxy, not "foo". But your code above would
> >prefer "foo", right?
>
> Apparently I'm the author of the http.proxy feature, though I barely
> remember what problem I was actually solving at the time. At the
> time I justified it on the grounds that a user might want to use a
> different proxy for git and/or a particular remote. The "http_proxy"
> environment variable is likely to be a global system default, or
> perhaps a desktop setting, and therefore I'd say probably less and
> not more specific than a git configuration variable.
Good point. We sometimes follow this order:
1. git-specific environment variables (i.e., $GIT_HTTP_PROXY, if
it existed)
2. git config files (i.e., http.proxy)
3. generic system environment (i.e., $http_proxy).
So thinking about it that way, the original patch makes more sense.
-Peff
^ permalink raw reply
* Re: [msysGit] Re: [PATCH 0/2] submodules: Use relative paths to gitdir and work tree
From: Jens Lehmann @ 2012-02-28 19:33 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Sixt, Git Mailing List, Antony Male, Phil Hord, msysGit,
Johannes Schindelin
In-Reply-To: <7vehtezs6q.fsf@alter.siamese.dyndns.org>
Am 28.02.2012 20:14, schrieb Junio C Hamano:
> Johannes Sixt <j6t@kdbg.org> writes:
>
>> With the following patch on top of your always-use-relative-gitdir branch
>> from https://github.com/jlehmann/git-submod-enhancements the tests pass
>> on Windows.
>>
>> Thanks, Dscho, for pointing out the obvious.
>
> The patch looks unintrusive and sane.
>
> Thanks all three of you for looking into this. Should I wait for a patch
> with nice write-up from one of you, or should I just come up with a random
> message and apply it locally avoiding roundtrip cost?
Thanks, but that interdiff needs all three patches from my branch to work
properly, while I only posted the first two here so far (without the third
one the gitfile still might contain the "c:/" notation even with J6t's diff
applied). I still need to remove the iffiness of my 2/2 patch and the third
one needs a test case too before I can repost that series.
>> diff --git a/git-submodule.sh b/git-submodule.sh
>> index e1984e0..953ca5e 100755
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>> @@ -151,6 +151,9 @@ module_clone()
>>
>> a=$(cd "$gitdir" && pwd)
>> b=$(cd "$path" && pwd)
>> + # normalize Windows-style absolute paths to POSIX-style absolute paths
>> + case $a in [a-zA-Z]:/*) a=/${a%%:*}${a#*:} esac
>> + case $b in [a-zA-Z]:/*) b=/${b%%:*}${b#*:} esac
>> # Remove all common leading directories
>> while test -n "$a" && test -n "$b" && test "${a%%/*}" = "${b%%/*}"
>> do
>
^ permalink raw reply
* Re: [PATCH 3/3] http: when proxy url has username but no password, ask for password
From: Jeff King @ 2012-02-28 19:31 UTC (permalink / raw)
To: Nelson Benitez Leon; +Cc: git, sam
In-Reply-To: <4F4CCEFD.90402@seap.minhap.es>
On Tue, Feb 28, 2012 at 01:56:29PM +0100, Nelson Benitez Leon wrote:
> diff --git a/http.c b/http.c
> index 79cbe50..68e3f7d 100644
> --- a/http.c
> +++ b/http.c
> @@ -306,7 +306,41 @@ static CURL *get_curl_handle(void)
> }
> }
> if (curl_http_proxy) {
> - curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
> + char *at, *colon, *proxyuser;
> + const char *cp;
> + cp = strstr(curl_http_proxy, "://");
> + if (cp == NULL) {
> + cp = curl_http_proxy;
> + } else {
> + cp += 3;
> + }
> + at = strchr(cp, '@');
> + colon = strchr(cp, ':');
> + if (at && (!colon || at < colon)) {
> + /* proxy string has username but no password, ask for password */
Don't parse the URL by hand. Use credential_from_url, which will do it
for you (and will properly handle things like unquoting the various
components).
> + char *ask_str, *proxyuser, *proxypass;
Shouldn't these be static globals? If we have multiple curl handles, you
would want them to share the authentication information we collect here,
and not have to ask the user again, no?
> + strbuf_addf(&pbuf, "Enter password for proxy %s...", at+1);
> + ask_str = strbuf_detach(&pbuf, NULL);
> + proxypass = xstrdup(git_getpass(ask_str));
And this should be using credential_fill(), which will let it use
credential helpers to save passwords, give it the same type of prompt as
elsewhere, etc.
See Documentation/technical/api-credential.txt, and see how regular http
auth is handled for an example.
-Peff
^ permalink raw reply
* Re: [PATCH 2/3] http: try standard proxy env vars when http.proxy config option is not set
From: Sam Vilain @ 2012-02-28 19:27 UTC (permalink / raw)
To: Jeff King; +Cc: Nelson Benitez Leon, Thomas Rast, git, sam.vilain
In-Reply-To: <20120228191514.GD11260@sigill.intra.peff.net>
On 2/28/12 11:15 AM, Jeff King wrote:
> Usually we would prefer environment variables to config. So that:
>
> $ git config http.proxy foo
> $ HTTP_PROXY=bar git fetch
>
> would use "bar" as the proxy, not "foo". But your code above would
> prefer "foo", right?
Apparently I'm the author of the http.proxy feature, though I barely
remember what problem I was actually solving at the time. At the time I
justified it on the grounds that a user might want to use a different
proxy for git and/or a particular remote. The "http_proxy" environment
variable is likely to be a global system default, or perhaps a desktop
setting, and therefore I'd say probably less and not more specific than
a git configuration variable.
As to this matter of "HTTP_PROXY", I'm not sure about whether that helps
or confuses matters to support. I must admit I'm still confused by the
motivation of this patch series.
Sam
^ permalink raw reply
* Re: [PATCH 2/3] http: try standard proxy env vars when http.proxy config option is not set
From: Junio C Hamano @ 2012-02-28 19:24 UTC (permalink / raw)
To: Thomas Rast; +Cc: Nelson Benitez Leon, git, peff, sam.vilain, sam
In-Reply-To: <878vjn8823.fsf@thomas.inf.ethz.ch>
Thomas Rast <trast@inf.ethz.ch> writes:
> Which raises the questions:
>
> * Why is this needed? Does git's use of libcurl ignore http_proxy? [1]
> seems to indicate that libcurl respects <protocol>_proxy
> automatically.
>
> * Why do you (need to?) support HTTP_PROXY when curl doesn't?
Let me add a third bullet point.
I've heard rumors that libcurl on some versions/installations of Mac OS X
deliberately ignores the environment. For those who agree with Apple, it
would be a regression if we suddenly start the environment ourselves and
using it.
^ permalink raw reply
* Re: [msysGit] Re: [PATCH 0/2] submodules: Use relative paths to gitdir and work tree
From: Jens Lehmann @ 2012-02-28 19:21 UTC (permalink / raw)
To: Johannes Sixt
Cc: Junio C Hamano, Git Mailing List, Antony Male, Phil Hord, msysGit,
Johannes Schindelin
In-Reply-To: <4F4D23D8.1050208@kdbg.org>
Am 28.02.2012 19:58, schrieb Johannes Sixt:
> Am 27.02.2012 22:19, schrieb Johannes Sixt:
>> Am 26.02.2012 20:58, schrieb Jens Lehmann:
>>> - gitdir=$(git rev-parse --git-dir)
>>> + gitdir=$(git rev-parse --git-dir | sed -e 's,^\([a-z]\):/,/\1/,')
>>
>> I don't like pipelines of this kind because they fork yet another
>> process. But it looks like there are not that many alternatives...
>
> With the following patch on top of your always-use-relative-gitdir branch
> from https://github.com/jlehmann/git-submod-enhancements the tests pass
> on Windows.
>
> Thanks, Dscho, for pointing out the obvious.
Thanks for helping to test and fix that on the Windows side. Do you want
to post a commit based on the the interdiff below so I can apply it on
top of my branch? Then I would make this a four patch series in the next
round.
> diff --git a/git-submodule.sh b/git-submodule.sh
> index e1984e0..953ca5e 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -151,6 +151,9 @@ module_clone()
>
> a=$(cd "$gitdir" && pwd)
> b=$(cd "$path" && pwd)
> + # normalize Windows-style absolute paths to POSIX-style absolute paths
> + case $a in [a-zA-Z]:/*) a=/${a%%:*}${a#*:} esac
> + case $b in [a-zA-Z]:/*) b=/${b%%:*}${b#*:} esac
> # Remove all common leading directories
> while test -n "$a" && test -n "$b" && test "${a%%/*}" = "${b%%/*}"
> do
>
^ permalink raw reply
* Re: Tilde spec - befuzzled
From: Junio C Hamano @ 2012-02-28 19:20 UTC (permalink / raw)
To: Thomas Rast; +Cc: Andreas Ericsson, Luke Diamand, Git List
In-Reply-To: <87zkc38a3v.fsf@thomas.inf.ethz.ch>
Thomas Rast <trast@inf.ethz.ch> writes:
>>> '<rev>{tilde}<n>', e.g. 'master{tilde}3'::
>>> A suffix '{tilde}<n>' to a revision parameter means the commit
>>> object that is the <n>th generation grand-parent of the named
>>> commit object, following only the first parents.
>>>
>>> Hang on, *grand*-parents?
>>> ...
>
> Perhaps we should reword it as "n-th first-parent ancestor"? Barring
> confusion about the position of the dashes, that leaves little room for
> error.
I think we could either go "easier to read but not precise"
... that is the <n>th generation (grand-)parent of ...
or "may sound scary but correct"
the ancestor reached by walking the first-parent chain <n> times
I am not sure which bucket "n-th first-parent ancestor" falls into.
^ permalink raw reply
* Re: [PATCH 2/3] http: try standard proxy env vars when http.proxy config option is not set
From: Jeff King @ 2012-02-28 19:15 UTC (permalink / raw)
To: Nelson Benitez Leon; +Cc: Thomas Rast, git, sam.vilain, sam
In-Reply-To: <4F4CCE8A.4010800@seap.minhap.es>
On Tue, Feb 28, 2012 at 01:54:34PM +0100, Nelson Benitez Leon wrote:
> diff --git a/http.c b/http.c
> index 8ac8eb6..79cbe50 100644
> --- a/http.c
> +++ b/http.c
> @@ -295,6 +295,16 @@ static CURL *get_curl_handle(void)
> if (curl_ftp_no_epsv)
> curl_easy_setopt(result, CURLOPT_FTP_USE_EPSV, 0);
>
> + if (!curl_http_proxy) {
> + const char *env_proxy;
> + env_proxy = getenv("HTTP_PROXY");
> + if (!env_proxy) {
> + env_proxy = getenv("http_proxy");
> + }
> + if (env_proxy) {
> + curl_http_proxy = xstrdup(env_proxy);
> + }
> + }
Usually we would prefer environment variables to config. So that:
$ git config http.proxy foo
$ HTTP_PROXY=bar git fetch
would use "bar" as the proxy, not "foo". But your code above would
prefer "foo", right?
>From reading Thomas's messages, I think there is a slight complication
in that right now curl is respecting $http_proxy, and it is probably
letting git's http.proxy overwrite (though I didn't check). If that is
the case, then that is IMHO a bug that should be fixed. So the rationale
for this patch would be three-fold:
1. Support HTTP_PROXY, which curl does not accept.
2. Fix the precedence of environment variables over config.
3. By handling the proxy variables ourselves, we have more flexibility
in handling the authentication.
-Peff
^ permalink raw reply
* Re: [msysGit] Re: [PATCH 0/2] submodules: Use relative paths to gitdir and work tree
From: Junio C Hamano @ 2012-02-28 19:14 UTC (permalink / raw)
To: Johannes Sixt
Cc: Jens Lehmann, Junio C Hamano, Git Mailing List, Antony Male,
Phil Hord, msysGit, Johannes Schindelin
In-Reply-To: <4F4D23D8.1050208@kdbg.org>
Johannes Sixt <j6t@kdbg.org> writes:
> With the following patch on top of your always-use-relative-gitdir branch
> from https://github.com/jlehmann/git-submod-enhancements the tests pass
> on Windows.
>
> Thanks, Dscho, for pointing out the obvious.
The patch looks unintrusive and sane.
Thanks all three of you for looking into this. Should I wait for a patch
with nice write-up from one of you, or should I just come up with a random
message and apply it locally avoiding roundtrip cost?
> diff --git a/git-submodule.sh b/git-submodule.sh
> index e1984e0..953ca5e 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -151,6 +151,9 @@ module_clone()
>
> a=$(cd "$gitdir" && pwd)
> b=$(cd "$path" && pwd)
> + # normalize Windows-style absolute paths to POSIX-style absolute paths
> + case $a in [a-zA-Z]:/*) a=/${a%%:*}${a#*:} esac
> + case $b in [a-zA-Z]:/*) b=/${b%%:*}${b#*:} esac
> # Remove all common leading directories
> while test -n "$a" && test -n "$b" && test "${a%%/*}" = "${b%%/*}"
> do
^ permalink raw reply
* Re: [PATCH v7 02/10] Stop starting pager recursively
From: Junio C Hamano @ 2012-02-28 19:10 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Ramsay Jones
In-Reply-To: <7v4nua25cz.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> git-column can be used as a pager for other git commands, something
>> like this:
>>
>> GIT_PAGER="git -p column --mode='dense color'" git -p branch
>>
>> The problem with this is that "git -p column" also has $GIT_PAGER
>> set so the pager runs itself again as a pager, then again and again.
>>
>> Stop this.
>
> A natural question that may come after reading only the above is if "git
> column" is the only one that has this problem. In other words, is the
> undesirable behaviour you observed caused by a bug in setup_pager() that
> needs to be fixed, or should it be fixed in "git column"?
Put another way, if there is another git command X that can be used as a
filter to the output of a git command Y, do you suffer from the same issue
to when you abuse the GIT_PAGER mechanism to pipe the output from Y to X?
That is a sure sign that the pager mechanism needs improvement (obviously,
an alternative answer could be "don't do that then", though).
For example, shortlog is designed to be X for Y=log, i.e.
$ git log v1.0.0.. | git shortlog
is a perfectly valid way to use the command. I could imagine that this
patch may improve the situation if you abuse GIT_PAGER mechanism to
implement the above pipeline, i.e.
$ GIT_PAGER="git -p shortlog" git log v1.0.0..
Although I never tried it.
^ permalink raw reply
* Re: [PATCH 3/3 v2] parse-options: remove PARSE_OPT_NEGHELP
From: Jeff King @ 2012-02-28 19:09 UTC (permalink / raw)
To: René Scharfe
Cc: git, Junio C Hamano, Bert Wesarg, Geoffrey Irving,
Johannes Schindelin, Pierre Habouzit
In-Reply-To: <4F4D25A1.8050702@lsrfire.ath.cx>
On Tue, Feb 28, 2012 at 08:06:09PM +0100, René Scharfe wrote:
> PARSE_OPT_NEGHELP is confusing because short options defined with that
> flag do the opposite of what the helptext says. It is also not needed
> anymore now that options starting with no- can be negated by removing
> that prefix. Convert its only two users to OPT_NEGBIT() and OPT_BOOL()
> and then remove support for PARSE_OPT_NEGHELP.
>
> Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
> ---
> This version doesn't invert the logic in grep anymore.
Thanks, I like this one much better.
-Peff
^ permalink raw reply
* [PATCH 3/3 v2] parse-options: remove PARSE_OPT_NEGHELP
From: René Scharfe @ 2012-02-28 19:06 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Bert Wesarg, Geoffrey Irving, Johannes Schindelin,
Pierre Habouzit, Jeff King
In-Reply-To: <4F49336C.3000303@lsrfire.ath.cx>
PARSE_OPT_NEGHELP is confusing because short options defined with that
flag do the opposite of what the helptext says. It is also not needed
anymore now that options starting with no- can be negated by removing
that prefix. Convert its only two users to OPT_NEGBIT() and OPT_BOOL()
and then remove support for PARSE_OPT_NEGHELP.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
This version doesn't invert the logic in grep anymore.
builtin/fast-export.c | 4 +---
builtin/grep.c | 5 ++---
parse-options.c | 6 ++----
parse-options.h | 4 ----
4 files changed, 5 insertions(+), 14 deletions(-)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 08fed98..19509ea 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -647,9 +647,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)
"Output full tree for each commit"),
OPT_BOOLEAN(0, "use-done-feature", &use_done_feature,
"Use the done feature to terminate the stream"),
- { OPTION_NEGBIT, 0, "data", &no_data, NULL,
- "Skip output of blob data",
- PARSE_OPT_NOARG | PARSE_OPT_NEGHELP, NULL, 1 },
+ OPT_BOOL(0, "no-data", &no_data, "Skip output of blob data"),
OPT_END()
};
diff --git a/builtin/grep.c b/builtin/grep.c
index e4ea900..643938d 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -684,9 +684,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
struct option options[] = {
OPT_BOOLEAN(0, "cached", &cached,
"search in index instead of in the work tree"),
- { OPTION_BOOLEAN, 0, "index", &use_index, NULL,
- "finds in contents not managed by git",
- PARSE_OPT_NOARG | PARSE_OPT_NEGHELP },
+ OPT_NEGBIT(0, "no-index", &use_index,
+ "finds in contents not managed by git", 1),
OPT_BOOLEAN(0, "untracked", &untracked,
"search in both tracked and untracked files"),
OPT_SET_INT(0, "exclude-standard", &opt_exclude,
diff --git a/parse-options.c b/parse-options.c
index 8906841..1908996 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -533,7 +533,7 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
continue;
pos = fprintf(outfile, " ");
- if (opts->short_name && !(opts->flags & PARSE_OPT_NEGHELP)) {
+ if (opts->short_name) {
if (opts->flags & PARSE_OPT_NODASH)
pos += fprintf(outfile, "%c", opts->short_name);
else
@@ -542,9 +542,7 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
if (opts->long_name && opts->short_name)
pos += fprintf(outfile, ", ");
if (opts->long_name)
- pos += fprintf(outfile, "--%s%s",
- (opts->flags & PARSE_OPT_NEGHELP) ? "no-" : "",
- opts->long_name);
+ pos += fprintf(outfile, "--%s", opts->long_name);
if (opts->type == OPTION_NUMBER)
pos += fprintf(outfile, "-NUM");
diff --git a/parse-options.h b/parse-options.h
index 2e811dc..def9ced 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -40,7 +40,6 @@ enum parse_opt_option_flags {
PARSE_OPT_LASTARG_DEFAULT = 16,
PARSE_OPT_NODASH = 32,
PARSE_OPT_LITERAL_ARGHELP = 64,
- PARSE_OPT_NEGHELP = 128,
PARSE_OPT_SHELL_EVAL = 256
};
@@ -90,9 +89,6 @@ typedef int parse_opt_ll_cb(struct parse_opt_ctx_t *ctx,
* PARSE_OPT_LITERAL_ARGHELP: says that argh shouldn't be enclosed in brackets
* (i.e. '<argh>') in the help message.
* Useful for options with multiple parameters.
- * PARSE_OPT_NEGHELP: says that the long option should always be shown with
- * the --no prefix in the usage message. Sometimes
- * useful for users of OPTION_NEGBIT.
*
* `callback`::
* pointer to the callback to use for OPTION_CALLBACK or
--
1.7.9.2
^ permalink raw reply related
* Re: git (commit|tag) atomicity
From: Zach Brown @ 2012-02-28 18:57 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Jon Jagger, git, Holger Hellmuth
In-Reply-To: <m3aa42vosb.fsf@localhost.localdomain>
It's a bit of a tangent, but just to be sure people don't get the wrong
impression..
> But I am not sure... that probably depends on how opendir(3) and
> readdir(3) works on given filesystem wrt. updates to opened directory.
> I think VFS on Linux ensures that you see view of filesystem as it was
> on opendir().
No, readdir() does not give you a static view of the entries in a
directory as it was on opendir(). readdir() will reflect modifications
that are done after opendir(). The specifics for a given situation
depend on how the file system maps the readdir position (f_pos) to
directory entries. You can see very different results when comparing,
say, stock ext2, indexed ext[34], and btrfs.
- z
(your message probably caught my eye because telldir()/seekdir() is
*loathed* by file system designers)
^ permalink raw reply
* Re: Delivery Status Notification (Failure)
From: Rajat Khanduja @ 2012-02-28 19:02 UTC (permalink / raw)
To: git
In-Reply-To: <001636c927b40af7cb04ba0aca6e@google.com>
Hi
I am a student interested in participating in GSOC'12 and was curious
to know if Git is participating in the program this year.
Sincerely,
Rajat
^ permalink raw reply
* Re: [msysGit] Re: [PATCH 0/2] submodules: Use relative paths to gitdir and work tree
From: Johannes Sixt @ 2012-02-28 18:58 UTC (permalink / raw)
To: Jens Lehmann
Cc: Junio C Hamano, Git Mailing List, Antony Male, Phil Hord, msysGit,
Johannes Schindelin
In-Reply-To: <4F4BF357.8020407@kdbg.org>
Am 27.02.2012 22:19, schrieb Johannes Sixt:
> Am 26.02.2012 20:58, schrieb Jens Lehmann:
>> I don't understand why you need this. Does "pwd" sometimes return a
>> path starting with "c:/" and sometimes "/c/" depending on what form
>> you use when you cd into that directory?
>
> It looks like this is the case. I was surprised as well. I hoped that
> pwd -P would fix it, but it makes no difference. I should have tested
> pwd -L as well, but I forgot.
pwd -L doesn't make a difference, either.
>> - gitdir=$(git rev-parse --git-dir)
>> + gitdir=$(git rev-parse --git-dir | sed -e 's,^\([a-z]\):/,/\1/,')
>
> I don't like pipelines of this kind because they fork yet another
> process. But it looks like there are not that many alternatives...
With the following patch on top of your always-use-relative-gitdir branch
from https://github.com/jlehmann/git-submod-enhancements the tests pass
on Windows.
Thanks, Dscho, for pointing out the obvious.
diff --git a/git-submodule.sh b/git-submodule.sh
index e1984e0..953ca5e 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -151,6 +151,9 @@ module_clone()
a=$(cd "$gitdir" && pwd)
b=$(cd "$path" && pwd)
+ # normalize Windows-style absolute paths to POSIX-style absolute paths
+ case $a in [a-zA-Z]:/*) a=/${a%%:*}${a#*:} esac
+ case $b in [a-zA-Z]:/*) b=/${b%%:*}${b#*:} esac
# Remove all common leading directories
while test -n "$a" && test -n "$b" && test "${a%%/*}" = "${b%%/*}"
do
^ permalink raw reply related
* Re: [PATCH v7 05/10] column: add column.ui for default column output settings
From: Junio C Hamano @ 2012-02-28 18:44 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Ramsay Jones
In-Reply-To: <1330430331-19945-6-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> +static int column_config(const char *var, const char *value,
> + const char *key, unsigned int *colopts)
> +{
> + if (git_config_column(colopts, value, -1))
Are we sure nobody has put us on pager at this point, so that you can tell
git_config_column() that it is OK to use isatty(1) to figure it out, or we
could already be on pager (i.e. pager_in_use() is true) in which case we
know we are interactive and should behave the same way as writing to a
terminal?
> + return error("invalid %s mode %s", key, value);
> + return 0;
> +}
> +
> +int git_column_config(const char *var, const char *value,
> + const char *command, unsigned int *colopts)
> +{
> + if (!strcmp(var, "column.ui"))
> + return column_config(var, value, "column.ui", colopts);
> +
> + if (command) {
> + struct strbuf sb = STRBUF_INIT;
> + int ret = 0;
> + strbuf_addf(&sb, "column.%s", command);
> + if (!strcmp(var, sb.buf))
> + ret = column_config(var, value, sb.buf, colopts);
> + strbuf_release(&sb);
> + return ret;
> + }
This feels wrong. Depending on the order column.ui and column.frotz appear
in the configuration file, asking for "git column --command=frotz" would
yield random results, no?
Shouldn't the flow of logic be more like:
git_config(git_column_config);
-> git_column_config() is called for column.ui and column.frotz
in no specified order; keep two *char variables to store the
string value given from configuration
if (kept value from column.frotz is missing)
git_config_column(..., kept value from column.ui, ...);
else
git_config_column(..., kept value from column.frotz, ...);
> diff --git a/column.h b/column.h
> index eb03c6c..43528da 100644
> --- a/column.h
> +++ b/column.h
> @@ -27,6 +27,8 @@ extern void print_columns(const struct string_list *list,
> struct column_options *opts);
> extern int git_config_column(unsigned int *mode, const char *value,
> int stdout_is_tty);
> +extern int git_column_config(const char *var, const char *value,
> + const char *command, unsigned int *colopts);
Also please rename git_config_column() in the earlier patch, perhaps like
"parse_column_config_string()" or something more sensible, to avoid
confusion.
^ permalink raw reply
* [PATCH (BUGFIX)] gitweb: Handle invalid regexp in regexp search
From: Jakub Narebski @ 2012-02-28 18:41 UTC (permalink / raw)
To: git; +Cc: Ramsay Jones
When using regexp search ('sr' parameter / $search_use_regexp variable
is true), check first that regexp is valid.
Without this patch we would get an error from Perl during search (if
searching is performed by gitweb), or highlighting matches substring
(if applicable), if user provided invalid regexp... which means broken
HTML, with error page (including HTTP headers) generated after gitweb
already produced some output.
Add test that illustrates such error: for example for regexp "*\.git"
we would get the following error:
Quantifier follows nothing in regex; marked by <-- HERE in m/* <-- HERE \.git/
at /var/www/cgi-bin/gitweb.cgi line 3084.
Reported-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
See "Re: gitweb: (potential) problems with new installation"
http://thread.gmane.org/gmane.comp.version-control.git/191746
gitweb/gitweb.perl | 11 ++++++++++-
t/t9501-gitweb-standalone-http-status.sh | 10 ++++++++++
2 files changed, 20 insertions(+), 1 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 1fc5361..22ad279 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1081,7 +1081,16 @@ sub evaluate_and_validate_params {
if (length($searchtext) < 2) {
die_error(403, "At least two characters are required for search parameter");
}
- $search_regexp = $search_use_regexp ? $searchtext : quotemeta $searchtext;
+ if ($search_use_regexp) {
+ $search_regexp = $searchtext;
+ if (!eval { qr/$search_regexp/; 1; }) {
+ (my $error = $@) =~ s/ at \S+ line \d+.*\n?//;
+ die_error(400, "Invalid search regexp '$search_regexp'",
+ esc_html($error));
+ }
+ } else {
+ $search_regexp = quotemeta $searchtext;
+ }
}
}
diff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh
index 26102ee..31076ed 100755
--- a/t/t9501-gitweb-standalone-http-status.sh
+++ b/t/t9501-gitweb-standalone-http-status.sh
@@ -134,4 +134,14 @@ our $maxload = undef;
EOF
+# ----------------------------------------------------------------------
+# invalid arguments
+
+test_expect_success 'invalid arguments: invalid regexp (in project search)' '
+ gitweb_run "a=project_list;s=*\.git;sr=1" &&
+ grep "Status: 400" gitweb.headers &&
+ grep "400 - Invalid.*regexp" gitweb.body
+'
+test_debug 'cat gitweb.headers'
+
test_done
^ permalink raw reply related
* Re: [PATCH v7 04/10] column: add dense layout support
From: Junio C Hamano @ 2012-02-28 18:27 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Ramsay Jones
In-Reply-To: <1330430331-19945-5-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> Normally, all items will be gone through once. The largest cell will set
> column width for all columns. With this strategy, one long item will
> stretch out all columns, wasting space in between them.
>
> With COL_DENSE enabled, it shrinks all columns to minimum, then attempts
> to push the last row's cells over to the next column with hope that
> everything still fits even there's one row less. The process is repeated
> until the new layout cannot fit in given width anymore, or there's only
> one row left (perfect!).
As you have given 4 bits for COL_MODE in the previous patch, I expected
that this will be one of the mode that you can use, e.g. column-first dense,
or row-first no-dense, with two more bits for operating modes. Not calling
it COL_MODE_DENSE and assigning a "are we dense of not?" bit out of COL_MODE
bits feels wrong.
^ permalink raw reply
* Re: [PATCH 1/3] http: authenticate on NTLM proxies and others suppported, by CuRL
From: Nelson Benitez Leon @ 2012-02-28 18:15 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, peff, sam, sam.vilain
In-Reply-To: <8762er6nb2.fsf@thomas.inf.ethz.ch>
On 02/28/2012 03:32 PM, Thomas Rast wrote:
> Nelson Benitez Leon <nelsonjesus.benitez@seap.minhap.es> writes:
>
>> - if (curl_http_proxy)
>> + if (curl_http_proxy) {
>> curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
>> + curl_easy_setopt(result, CURLOPT_PROXYAUTH, CURLAUTH_ANY);
>> + }
>
> There was another attempt at doing the same very recently:
>
> http://thread.gmane.org/gmane.comp.version-control.git/191140
>
> I could swear there was a second one, but apparently that was you.
> Neither you nor Marco submitter have so far answered the question I
> raised in
>
> http://thread.gmane.org/gmane.comp.version-control.git/191155
>
> which can be summarized as: please make a case -- and put it in the
> message! -- for or against making this configurable. Is there a speed
> tradeoff? (However, you could steal some of Daniel Stenberg's
> reasoning!)
I don't see any reason to make this configurable, CuRL people made this
cool CURLAUTH_ANY option that automatically chooses the best auth method
from among those the server supports, that means you don't have to
investigate if your proxy is using Basic, Digest or NTLM methods, and
use a specific curl option for each of them, instead curl
will ask the proxy and use the appropiate, and it will only do that if
you are using a proxy (i.e. you've set CURLOPT_PROXY or you have http_proxy
env var), also curl will not try to authenticate if you've not provided
username or password in the proxy string, as I've been told here[1]..
so, setting CURLOPT_PROXYAUTH = CURLAUTH_ANY will not affect the speed of
normal curl use, only if 1) you are using a proxy and 2) your proxy requires
authentication, only then curl will just make two or three roundtrips to find out
the auth method the proxy is using, that is a tiny cost compared to having the
user find out the proxy auth type and set an specifically config option to enable
that type.
So I would call CURLAUTH_ANY as out-of-the-box proxy support, and I don't want it
activated from a config option, I want it to still be out-of-the-box in git also..
[1] https://bugzilla.redhat.com/show_bug.cgi?id=769254#c6
^ permalink raw reply
* Re: [PATCH v7 03/10] column: add columnar layout
From: Junio C Hamano @ 2012-02-28 18:22 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Ramsay Jones
In-Reply-To: <1330430331-19945-4-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> COL_MODE_COLUMN and COL_MODE_ROW fill column by column (or row by row
> respectively), given the terminal width and how many space between
> columns.
>
> Strings are supposed to be in UTF-8. If strings contain ANSI escape
> strings, COL_ANSI must be specified for correct length calculation.
Hrm, is it too heavyweight to autodetect and relieve the caller from the
burden of passing COL_ANSI?
Perhaps if you update utf8_strwidth() so that it takes <ptr, len> input
not NUL terminated string, you can do item_length like this, no?
cp = s;
width = 0;
while (1) {
ep = strstr(cp, "\033[");
if (!ep) {
width += utf8_strwidth(cp);
break;
}
utf8_strwidth_counted(cp, ep - cp);
... scan ep to skip the "\033[...;X"
cp = ep;
}
^ permalink raw reply
* Re: [PATCH v7 02/10] Stop starting pager recursively
From: Junio C Hamano @ 2012-02-28 18:13 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Ramsay Jones
In-Reply-To: <1330430331-19945-3-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> git-column can be used as a pager for other git commands, something
> like this:
>
> GIT_PAGER="git -p column --mode='dense color'" git -p branch
>
> The problem with this is that "git -p column" also has $GIT_PAGER
> set so the pager runs itself again as a pager, then again and again.
>
> Stop this.
A natural question that may come after reading only the above is if "git
column" is the only one that has this problem. In other words, is the
undesirable behaviour you observed caused by a bug in setup_pager() that
needs to be fixed, or should it be fixed in "git column"?
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> pager.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/pager.c b/pager.c
> index 05584de..4dcb08d 100644
> --- a/pager.c
> +++ b/pager.c
> @@ -73,7 +73,7 @@ void setup_pager(void)
> {
> const char *pager = git_pager(isatty(1));
>
> - if (!pager)
> + if (!pager || pager_in_use())
> return;
>
> /*
^ permalink raw reply
* Re: [PATCH v7 01/10] Add git-column for columnar display
From: Junio C Hamano @ 2012-02-28 18:10 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Ramsay Jones
In-Reply-To: <1330430331-19945-2-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> diff --git a/Documentation/git-column.txt b/Documentation/git-column.txt
> new file mode 100644
> index 0000000..508b85f
> --- /dev/null
> +++ b/Documentation/git-column.txt
> @@ -0,0 +1,49 @@
> +git-column(1)
> +=============
> +
> +NAME
> +----
> +git-column - Display data in columns
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'git column' [--mode=<mode> | --rawmode=<n>] [--width=<width>]
Please spell this "--raw-mode".
> diff --git a/builtin/column.c b/builtin/column.c
> new file mode 100644
> index 0000000..3b0f74e
> --- /dev/null
> +++ b/builtin/column.c
> @@ -0,0 +1,41 @@
> +#include "builtin.h"
> +#include "cache.h"
> +#include "strbuf.h"
> +#include "parse-options.h"
> +#include "string-list.h"
> +#include "column.h"
> +
> +static const char * const builtin_column_usage[] = {
> + "git column [options]",
> + NULL
> +};
> +static unsigned int colopts;
> +
> +int cmd_column(int argc, const char **argv, const char *prefix)
> +{
> + struct string_list list = STRING_LIST_INIT_DUP;
> + struct strbuf sb = STRBUF_INIT;
> + struct column_options copts;
> + struct option options[] = {
> + OPT_COLUMN(0, "mode", &colopts, "layout to use"),
> + OPT_INTEGER(0, "rawmode", &colopts, "layout to use"),
> + OPT_INTEGER(0, "width", &copts.width, "Maximum width"),
> + OPT_STRING(0, "indent", &copts.indent, "string", "Padding space on left border"),
> + OPT_INTEGER(0, "nl", &copts.nl, "Padding space on right border"),
> + OPT_INTEGER(0, "padding", &copts.padding, "Padding space between columns"),
> + OPT_END()
> + };
> +
> + memset(&copts, 0, sizeof(copts));
> + copts.width = term_columns();
> + copts.padding = 1;
> + argc = parse_options(argc, argv, "", options, builtin_column_usage, 0);
Curious. The usual pattern is to set up the built-in default, call
git_config() to override it with the configured values, and then call
parse_options() to override at the runtime. I see configuration callback
defined in column.c but no call to git_config() here?
> + if (argc)
> + usage_with_options(builtin_column_usage, options);
> +
> + while (!strbuf_getline(&sb, stdin, '\n'))
> + string_list_append(&list, sb.buf);
> +
> + print_columns(&list, colopts, &copts);
> + return 0;
> +}
> diff --git a/column.c b/column.c
> new file mode 100644
> index 0000000..d61da81
> --- /dev/null
> +++ b/column.c
> @@ -0,0 +1,170 @@
> +#include "cache.h"
> +#include "column.h"
> +#include "string-list.h"
> +#include "parse-options.h"
> +
> +#define MODE(mode) ((mode) & COL_MODE)
> +
> +/* Display without layout when COL_ENABLED is not set */
> +static void display_plain(const struct string_list *list,
> + const char *indent, const char *nl)
> +{
> + int i;
> +
> + for (i = 0; i < list->nr; i++)
> + printf("%s%s%s", indent, list->items[i].string, nl);
> +}
> +
> +void print_columns(const struct string_list *list, unsigned int mode,
> + struct column_options *opts)
> +{
> + const char *indent = "", *nl = "\n";
> + int padding = 1, width = term_columns();
> +
> + if (!list->nr)
> + return;
> + if (opts) {
> + if (opts->indent)
> + indent = opts->indent;
> + if (opts->nl)
> + nl = opts->nl;
> + if (opts->width)
> + width = opts->width;
> + padding = opts->padding;
> + }
> + if (width <= 1 || !(mode & COL_ENABLED)) {
Curious why this is "1". If your terminal is only 2 columns wide, you
wouldn't be able to show your list items in two columns as you would want
to have an inter-column gap, no?
> + display_plain(list, indent, nl);
> + return;
> + }
> + die("BUG: invalid mode %d", MODE(mode));
> +}
> +
> +struct colopt {
> + enum {
> + ENABLE,
> + MODE,
> + OPTION
> + } type;
> + const char *name;
> + int value;
> +};
> +
> +/*
> + * Set COL_ENABLED and COL_ENABLED_SET. If 'set' is -1, check if
> + * stdout is tty.
> + */
> +static int set_enable_bit(unsigned int *mode, int set, int stdout_is_tty)
> +{
Somehow it looks to me that this is setting the ENABLED bit, not enable
bit.
> + if (set < 0) { /* auto */
> + if (stdout_is_tty < 0)
> + stdout_is_tty = isatty(1);
> + set = stdout_is_tty || (pager_in_use() && pager_use_color);
Why does this have anything to do with the use of color?
> + }
> + if (set)
> + *mode = *mode | COL_ENABLED | COL_ENABLED_SET;
> + else
> + *mode = (*mode & ~COL_ENABLED) | COL_ENABLED_SET;
> + return 0;
> +}
OK, so we record the desired value (either COL_ENABLED or not) and the
fact that a call to set_enable_bit() function set it. Which implies that
this function must be called from only one codepath (either setting from
the configuration mechanism, or by parsing the command line option) but
not both. I guess this is only called from configuration codepath?
> +/*
> + * Set COL_MODE_*. mode is intially copied from column.ui. If
> + * COL_ENABLED_SET is not set, then neither 'always', 'never' nor
> + * 'auto' has been used. Default to 'always'.
> + */
> +static int set_mode(unsigned int *mode, unsigned int value)
> +{
> + *mode = (*mode & ~COL_MODE) | value;
> + if (!(*mode & COL_ENABLED_SET))
> + *mode |= COL_ENABLED | COL_ENABLED_SET;
> +
> + return 0;
> +}
I am *imagining* that "*mode" begins with some compiled-in default, then
git_config() will read from column.* variables and set "*mode" but not in
this codepath, and by the time command line option triggers this codepath,
we should have seen everything the config wants to tell us, so if the
config did not say anything (i.e. COL_ENABLED_SET is not set yet), we
force enable the feature (the user wanting it to operate in some mode is
an indication enough that the user wants to enable the machinery as a
whole). So this function is designed to be called from command line option
parsing, but never from the configuration parsing.
But the parse_option() function below calls this, which would mean it is
also for command line option parsing and not configuration parsing. The
same function however calls set_enable_bit() we saw earlier that can only
be called from configuration codepath. What is going on?
I am afraid that we do not have enough to judge if this is sane in this
patch, as there is no support for column.ui at this stage. Perhaps the
series is not structured well and has things that are not yet relevant in
early part of it. Sigh.
The remainder of the patch unreviewed.
^ permalink raw reply
* Re: git (commit|tag) atomicity
From: Jakub Narebski @ 2012-02-28 17:41 UTC (permalink / raw)
To: Jon Jagger; +Cc: git, Holger Hellmuth
In-Reply-To: <CADWOt=ig5=Bhhkjs9-wbm2djtwWPOfPGtYt9pH-U3YuQ+iyXzg@mail.gmail.com>
Jon Jagger <jon@jaggersoft.com> writes:
> On Tue, Feb 28, 2012 at 4:46 PM, Holger Hellmuth <hellmuth@ira.uka.de> wrote:
> > On 28.02.2012 16:40, Jon Jagger wrote:
>>>
>>> I don't know a lot about git - I use it as a tool behind
>>> http://cyber-dojo.com
>>> which is an online coding dojo server.
>>> I have a quick question...
>>> If I do a
>>> git commit ....
>>> in one thread and a
>>> git tag | sort -g
>>> in another thread is the output of the git tag guaranteed to be atomic?
>>
>> Can a "git commit" add or remove tags? AFAIK it can't and so the two
>> commands don't conflict in any way.
>
> Sorry, I failed to ask the question I really wanted to ask...
>
> I mean in one thread
> git tag -m 'AAA' BBB HEAD
> and in another thread
> git tag | sort -g
>
> and the question is whether the output of the git tag|sort -g command
> is guaranteed to be from before the git tag -m... or from after the
> git tag -m... but not "interleaved" in any way....
Creating a tag or a commit is guaranteed to be atomic. Git first
atomically adds tag or a commit to object database (atomic file write)
as loose object, then atomically writes tag reference as loose tag
('.git/refs/tags/foo' file, containing SHA-1 id of newly created tag).
"git tag", which list all tags, recursively scans (reads) 'refs/tags/'
directory, so it could theoretically happen that if you have very
large number of loose (unpacked) tags, "git tag" might theoretically
list tag 'zzz' created after start of command, but not list 'aaa' tag
created after start of command.
But I am not sure... that probably depends on how opendir(3) and
readdir(3) works on given filesystem wrt. updates to opened directory.
I think VFS on Linux ensures that you see view of filesystem as it was
on opendir().
--
Jakub Narebski
^ 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