* Re: [PATCH] Escape file:// URL's to meet subversion SVN::Ra requirements
From: Jonathan Nieder @ 2011-11-02 18:20 UTC (permalink / raw)
To: Ben Walton; +Cc: normalperson, git
In-Reply-To: <1320251895-6348-2-git-send-email-bwalton@artsci.utoronto.ca>
Hi,
Ben Walton wrote:
> After a colleague reported this problem to the subversion list, it
> was determined that the problem is in git, not svn.[1] The SVN code
> expects URL's and paths to be pre-escaped.
Thanks for your work on this! I'm not really sure how one can decide
that the problem is not in svn --- some existing functions changed ABI
in such a way as to break existing applications and require code
changes and a recompile. It would be better for Subversion to
silently fix up paths provided by bad callers, or at least to return a
sensible error code.
So the problem is that nobody who cared was testing prereleases of
subversion and reporting bugs early enough for it to get this fixed
before the 1.7 release. But yes, that's water under the bridge and
git-svn (and libsvn-perl, and pysvn, and ...) should just adjust to
the new world order.
> [1] http://news.gmane.org/gmane.comp.version-control.subversion.devel
Do you mean
http://thread.gmane.org/gmane.comp.version-control.subversion.devel/132250
?
> Signed-off-by: Ben Walton <bwalton@artsci.utoronto.ca>
> ---
> git-svn.perl | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
Sounds sensible in principle, though I haven't checked the patch in
detail. When I run "make test" with svn 1.7 with this patch applied,
I get the following result, unfortunately:
| expecting success:
| git svn clone -s "$svnrepo" g &&
| (
| cd g &&
| test x`git rev-parse --verify refs/remotes/trunk^0` = \
| x`git rev-parse --verify refs/heads/master^0`
| )
|
| Initialized empty Git repository in /home/jrn/src/git2/t/trash directory.t9145-git-svn-master-branch/g/.git/
| Using higher level of URL: file:///home/jrn/src/git2/t/trash directory.t9145-git-svn-master-branch/svnrepo => file:///home/jrn/src/git2/t/trash%20directory.t9145-git-svn-master-branch/svnrepo
| svn-remote.svn: remote ref '///home/jrn/src/git2/t/trash directory.t9145-git-svn-master-branch/svnrepo/trunk:refs/remotes/trunk' must start with 'refs/'
|
| not ok - 2 git svn clone --stdlayout sets up trunk as master
| #
| # git svn clone -s "$svnrepo" g &&
| # (
| # cd g &&
| # test x`git rev-parse --verify refs/remotes/trunk^0` = \
| # x`git rev-parse --verify refs/heads/master^0`
| # )
| #
|
| # failed 1 among 2 test(s)
| 1..2
| make[3]: *** [t9145-git-svn-master-branch.sh] Error 1
Does it work for you? This is with a merge of git 1.7.8-rc0 and 1.7.7.2.
^ permalink raw reply
* Re: [ANNOUNCE] Git 1.7.8.rc0
From: Jeff King @ 2011-11-02 18:10 UTC (permalink / raw)
To: Stefan Naewe; +Cc: Junio C Hamano, Michael J Gruber, git
In-Reply-To: <20111102180327.GA30668@sigill.intra.peff.net>
On Wed, Nov 02, 2011 at 02:03:27PM -0400, Jeff King wrote:
> Without using a configured remote, try this (with .netrc configured):
>
> git push https://github.com/user/repo.git :refs/heads/nothing
>
> which should work, and then this:
>
> git push https://user@github.com/user/repo.git :refs/heads/nothing
>
> which will do the "must hit enter to accept password" thing.
>
> That fails even with v1.7.7. I didn't bisect, but it has been there
> quite a while (v1.6.6 has it, but v1.6.5 has a weird error, so I didn't
> bisect further).
OK, I see the issue.
The logic is "if we have a username, but not a password, then ask for
the password before trying any http" (this is what avoids the extra
round trip).
But if you are using netrc, we don't parse it ourselves. We just tell
curl "when you are making the request, check netrc, too".
So the ideal logic is:
1. look in netrc
2. If we have a username and no password, ask for password
3. Otherwise, try it and see if we get a 401.
But we can't do that, because (1) and (3) happen atomically inside of
curl.
The simplest thing is to just drop the behavior in (2), and let it drop
to a 401. The extra round trip probably isn't that big a deal.
The other option is to start parsing netrc ourselves, or do the extra
round trip if we detect ~/.netrc or something. But that last one is
getting pretty hackish.
-Peff
^ permalink raw reply
* Re: [ANNOUNCE] Git 1.7.8.rc0
From: Jeff King @ 2011-11-02 18:03 UTC (permalink / raw)
To: Stefan Naewe; +Cc: Junio C Hamano, Michael J Gruber, git
In-Reply-To: <loom.20111101T211624-511@post.gmane.org>
On Tue, Nov 01, 2011 at 08:18:47PM +0000, Stefan Naewe wrote:
> Stefan Naewe <stefan.naewe <at> gmail.com> writes:
>
> > Push with https works, if the URL looks e.g. like this:
> >
> > https://github.com/user/repo.git
> >
> > rather than this
> >
> > https://user <at> github.com/user/repo.git
> >
> > and having a ~/.netrc like this
> >
> > machine github.com login user password YouDontWantToKnow
> >
> > If the URL contains 'user@' I get the 'need ENTER' behaviour.
> >
>
> Another update:
>
> If I revert deba493 the 'need ENTER' is gone and everything works as above.
I think this is a false positive. The problem that deba493 fixes is that
"git push" was not properly looking at the push URL, but rather pulling
the initial auth information from the fetch URL. So I suspect your
finding the bug there or not may have to do with it actually respecting
your config properly.
I think the bug is much older than this, and we are just exposing it by
finally using the correct URL.
Without using a configured remote, try this (with .netrc configured):
git push https://github.com/user/repo.git :refs/heads/nothing
which should work, and then this:
git push https://user@github.com/user/repo.git :refs/heads/nothing
which will do the "must hit enter to accept password" thing.
That fails even with v1.7.7. I didn't bisect, but it has been there
quite a while (v1.6.6 has it, but v1.6.5 has a weird error, so I didn't
bisect further).
-Peff
^ permalink raw reply
* Re: [RFC/PATCH] http-push: don't always prompt for password
From: Junio C Hamano @ 2011-11-02 17:40 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Stefan Näwe, git@vger.kernel.org
In-Reply-To: <20111102172310.GA28525@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> It seems to me that Stefan's patch actually causes that (because he
> removes the early "set password if we have a username" logic). But I'll
> take another look at what's in master.
Thanks.
^ permalink raw reply
* Re: [RFC/PATCH] http-push: don't always prompt for password
From: Jeff King @ 2011-11-02 17:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefan Näwe, git@vger.kernel.org
In-Reply-To: <7vk47ijvlv.fsf@alter.siamese.dyndns.org>
On Wed, Nov 02, 2011 at 10:13:32AM -0700, Junio C Hamano wrote:
> This defers the calls to git_getpass* until we get 401 from the server
> side.
>
> I am guessing the reason why in the current code get_curl_handle() has a
> call to init_curl_http_auth() very early, but only when user_name is set,
> is because it is likely for a site to require authentication when the user
> already has "username@" in its URL, and doing it this way will avoid the
> extra round-trip because by the time we make an HTTP request, we have both
> name and pass. If we apply this patch, the check in init_curl_http_auth()
> that asks for the password only when user_name is set becomes unnecessary.
Yeah, that was my reading, as well. I was tempted to do away with it, as
it makes the code much simpler, at the cost of doing that extra
round-trip. However, most browsers do that round-trip, and I don't think
it's that big a deal.
In the end I decided not to switch it just because I wanted to be as
minimally invasive as possible, just in case somebody did care about the
round trip.
> I think the second hunk at l.846 sort of makes sense, but not quite.
>
> "We got 401 even though we know we have supplied name and pass" is a valid
> criterion to decide that the name/pass is an invalid combination. But it
> makes me wonder if this code in its early days guaranteed whenever we have
> user_name we always have made sure we have user_pass (otherwise by asking
> for it with git_getpass) and that is the reason why it had to check only
> for user_name, and if that is the case perhaps the real breakage is we are
> not keeping that guarantee in the current code?
All of my patches attempted to keep that condition, as well (because
credential_fill tries to do so). But I didn't think about it during the
re-roll of the patches that are in master now, so maybe that wasn't
kept.
It seems to me that Stefan's patch actually causes that (because he
removes the early "set password if we have a username" logic). But I'll
take another look at what's in master.
-Peff
^ permalink raw reply
* Re: [RFC/PATCH] http-push: don't always prompt for password
From: Junio C Hamano @ 2011-11-02 17:13 UTC (permalink / raw)
To: Stefan Näwe; +Cc: git@vger.kernel.org, Jeff King
In-Reply-To: <4EB104EA.2040001@atlas-elektronik.com>
Stefan Näwe <stefan.naewe@atlas-elektronik.com> writes:
> Am 01.11.2011 19:12, schrieb Junio C Hamano:
>>
>> There are only handful of commits that even remotely touch http related
>> codepath between v1.7.7 and v1.7.8-rc0:
>>
>> * deba493 http_init: accept separate URL parameter
>>
>> This could change the URL string given to http_auth_init().
>> ...
>> Could you try reverting deba493 and retest, and then if the behaviour is
>> the same "need ENTER", further revert 070b4dd and retest?
>
> I did a little more testing.
> This WIP makes it work for me (i.e. "need ENTER" is gone, works with
> and without .netrc, with 'https://host/repo.git' and
> 'https://user@host...' URL). Needs testing, of course.
>
> ---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---
> diff --git a/http.c b/http.c
> index a4bc770..008ad72 100644
> --- a/http.c
> +++ b/http.c
> @@ -279,8 +279,6 @@ static CURL *get_curl_handle(void)
> curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
> #endif
>
> - init_curl_http_auth(result);
> -
> if (ssl_cert != NULL)
> curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);
> if (has_cert_password())
> @@ -846,7 +844,7 @@ static int http_request(const char *url, void *result, int target, int options)
> else if (missing_target(&results))
> ret = HTTP_MISSING_TARGET;
> else if (results.http_code == 401) {
> - if (user_name) {
> + if (user_name && user_pass) {
> ret = HTTP_NOAUTH;
> } else {
> /*
> @@ -855,7 +853,8 @@ static int http_request(const char *url, void *result, int target, int options)
> * but that is non-portable. Using git_getpass() can at least be stubbed
> * on other platforms with a different implementation if/when necessary.
> */
> - user_name = xstrdup(git_getpass_with_description("Username", description));
> + if (!user_name)
> + user_name = xstrdup(git_getpass_with_description("Username", description));
> init_curl_http_auth(slot->curl);
> ret = HTTP_REAUTH;
> }
> ---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---
This defers the calls to git_getpass* until we get 401 from the server
side.
I am guessing the reason why in the current code get_curl_handle() has a
call to init_curl_http_auth() very early, but only when user_name is set,
is because it is likely for a site to require authentication when the user
already has "username@" in its URL, and doing it this way will avoid the
extra round-trip because by the time we make an HTTP request, we have both
name and pass. If we apply this patch, the check in init_curl_http_auth()
that asks for the password only when user_name is set becomes unnecessary.
I think the second hunk at l.846 sort of makes sense, but not quite.
"We got 401 even though we know we have supplied name and pass" is a valid
criterion to decide that the name/pass is an invalid combination. But it
makes me wonder if this code in its early days guaranteed whenever we have
user_name we always have made sure we have user_pass (otherwise by asking
for it with git_getpass) and that is the reason why it had to check only
for user_name, and if that is the case perhaps the real breakage is we are
not keeping that guarantee in the current code?
IOW, when in the current code do we make a new HTTP request while
user_name is set but no user_pass? Perhaps if we fix that codepath we do
not have to have this extra 401 roundtrip this patch is introducing when
the username (but not password) is given?
Peff, what do you think?
^ permalink raw reply
* Re: git sticker svg
From: Jeff King @ 2011-11-02 16:59 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git
In-Reply-To: <4EB1002A.6010602@drmicha.warpmail.net>
On Wed, Nov 02, 2011 at 09:32:42AM +0100, Michael J Gruber wrote:
> Yeah, git has been G-+ long before G+ ;)
>
> I do prefer the rotated, vertically stacked +-G though (with + <-> t, -
> <-> i, G <-> 3/4-circle with arrow). Good thing we don't have to argue
> about an "official" logo...
I actually don't like the stacked +-G (I find it hard to read). But
since I was ordering the stickers, I got to decide. :)
I think it's clear that we don't want to get into an "official" logo
argument, though. I'll let whoever orders stickers next time make the
decision. :)
-Peff
^ permalink raw reply
* [PATCH] Escape file:// URL's to meet subversion SVN::Ra requirements
From: Ben Walton @ 2011-11-02 16:38 UTC (permalink / raw)
To: normalperson; +Cc: git, Ben Walton
In-Reply-To: <1320251895-6348-1-git-send-email-bwalton@artsci.utoronto.ca>
Previously only http/https URL's were uri escaped. When building
against subversion 1.7, this was causing a segfault in perl after
an assertion failure in the SVN::Ra bindings during in t9134.
Changing 'trash directory' to 'trash_directory' worked around the
problem.
After a colleague reported this problem to the subversion list, it
was determined that the problem is in git, not svn.[1] The SVN code
expects URL's and paths to be pre-escaped.
We now also escape file:// URL's in the Git::SVN::Ra->escape_url
code path.
[1] http://news.gmane.org/gmane.comp.version-control.subversion.devel
Signed-off-by: Ben Walton <bwalton@artsci.utoronto.ca>
---
git-svn.perl | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 351e743..b00c188 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -5366,6 +5366,9 @@ sub escape_url {
if ($url =~ m#^(https?)://([^/]+)(.*)$#) {
my ($scheme, $domain, $uri) = ($1, $2, escape_uri_only($3));
$url = "$scheme://$domain$uri";
+ } elsif ($url =~ m#^(file)://(.*)$#) {
+ my ($scheme, $uri) = ($1, escape_uri_only($2));
+ $url = "$scheme://$uri";
}
$url;
}
--
1.7.7.1
^ permalink raw reply related
* (unknown),
From: Ben Walton @ 2011-11-02 16:38 UTC (permalink / raw)
To: normalperson; +Cc: git
Hi Eric,
When running the test suite against subversion 1.7 on solaris, t9134
was failing with an assertion failure in the SVN bindings and a
subsequent core dump from perl.
The behaviour of 1.7 has seemingly changed such that even file paths
must be eascaped/canonicalized now. The core dump behaviour is
something that still needs to be investigated with the subversion
folks. A colleague and I will push that on their side.
See the initial discussion of this issue on the svn list here:
http://article.gmane.org/gmane.comp.version-control.subversion.devel/132227
With the following patch, the test suite once again passes. I don't
know how it will behave against an older client though so maybe a
conditional check for the version of the bindings is still required?
Thanks
-Ben
From: Ben Walton <bwalton@artsci.utoronto.ca>
Subject:
In-Reply-To:
^ permalink raw reply
* [PATCH 2/2] Support sizes >=2G in various config options accepting 'g' sizes.
From: Nick Alcock @ 2011-11-02 15:46 UTC (permalink / raw)
To: git; +Cc: Nick Alcock
In-Reply-To: <1320248783-29577-1-git-send-email-nix@esperi.org.uk>
The config options core.packedGitWindowSize, core.packedGitLimit,
core.deltaBaseCacheLimit, core.bigFileThreshold, pack.windowMemory and
pack.packSizeLimit all claim to support suffixes up to and including
'g'. This implies that they should accept sizes >=2G on 64-bit
systems: certainly, specifying a size of 3g should not silently be
translated to zero or transformed into a large negative value due to
integer overflow. However, due to use of git_config_int() rather than
git_config_ulong(), that is exactly what happens:
% git config core.bigFileThreshold 2g
% git gc --aggressive # with extra debugging code to print out
# core.bigfilethreshold after parsing
bigfilethreshold: -2147483648
[...]
This is probably irrelevant for core.deltaBaseCacheLimit, but is
problematic for the other values. (It is particularly problematic for
core.packedGitLimit, which can't even be set to its default value in
the config file due to this bug.)
This fixes things for 32-bit platforms as well. They get the usual bad
config error if an overlarge value is specified, e.g.:
fatal: bad config value for 'core.bigfilethreshold' in /home/nix/.gitconfig
This is detected in all cases, even if the 32-bit platform has no size
larger than 'long'. For signed integral configuration values, we also
detect the case where the value is too large for the signed type but
not the unsigned type.
Signed-off-by: Nick Alcock <nix@esperi.org.uk>
---
config.c | 41 +++++++++++++++++++++++++++++++----------
1 files changed, 31 insertions(+), 10 deletions(-)
diff --git a/config.c b/config.c
index edf9914..84ca156 100644
--- a/config.c
+++ b/config.c
@@ -333,7 +333,7 @@ static int git_parse_file(config_fn_t fn, void *data)
die("bad config file line %d in %s", cf->linenr, cf->name);
}
-static int parse_unit_factor(const char *end, unsigned long *val)
+static int parse_unit_factor(const char *end, uintmax_t *val)
{
if (!*end)
return 1;
@@ -356,11 +356,23 @@ static int git_parse_long(const char *value, long *ret)
{
if (value && *value) {
char *end;
- long val = strtol(value, &end, 0);
- unsigned long factor = 1;
+ intmax_t val;
+ uintmax_t uval;
+ uintmax_t factor = 1;
+
+ errno = 0;
+ val = strtoimax(value, &end, 0);
+ if (errno == ERANGE)
+ return 0;
if (!parse_unit_factor(end, &factor))
return 0;
- *ret = val * factor;
+ uval = abs(val);
+ uval *= factor;
+ if ((uval > maximum_signed_value_of_type(long)) ||
+ (abs(val) > uval))
+ return 0;
+ val *= factor;
+ *ret = val;
return 1;
}
return 0;
@@ -370,9 +382,19 @@ int git_parse_ulong(const char *value, unsigned long *ret)
{
if (value && *value) {
char *end;
- unsigned long val = strtoul(value, &end, 0);
+ uintmax_t val;
+ uintmax_t oldval;
+
+ errno = 0;
+ val = strtoumax(value, &end, 0);
+ if (errno == ERANGE)
+ return 0;
+ oldval = val;
if (!parse_unit_factor(end, &val))
return 0;
+ if ((val > maximum_unsigned_value_of_type(long)) ||
+ (oldval > val))
+ return 0;
*ret = val;
return 1;
}
@@ -553,7 +575,7 @@ static int git_default_core_config(const char *var, const char *value)
if (!strcmp(var, "core.packedgitwindowsize")) {
int pgsz_x2 = getpagesize() * 2;
- packed_git_window_size = git_config_int(var, value);
+ packed_git_window_size = git_config_ulong(var, value);
/* This value must be multiple of (pagesize * 2) */
packed_git_window_size /= pgsz_x2;
@@ -564,18 +586,17 @@ static int git_default_core_config(const char *var, const char *value)
}
if (!strcmp(var, "core.bigfilethreshold")) {
- long n = git_config_int(var, value);
- big_file_threshold = 0 < n ? n : 0;
+ big_file_threshold = git_config_ulong(var, value);
return 0;
}
if (!strcmp(var, "core.packedgitlimit")) {
- packed_git_limit = git_config_int(var, value);
+ packed_git_limit = git_config_ulong(var, value);
return 0;
}
if (!strcmp(var, "core.deltabasecachelimit")) {
- delta_base_cache_limit = git_config_int(var, value);
+ delta_base_cache_limit = git_config_ulong(var, value);
return 0;
}
--
1.7.6.1.138.g03ab.dirty
^ permalink raw reply related
* [PATCH 1/2] Add strtoimax() compatibility function.
From: Nick Alcock @ 2011-11-02 15:46 UTC (permalink / raw)
To: git; +Cc: Nick Alcock
In-Reply-To: <1320248783-29577-1-git-send-email-nix@esperi.org.uk>
Since systems that omit strtoumax() will likely omit strtomax() too,
and likewise for strtoull() and strtoll(), we arrange for the
compatibility #defines NO_STRTOUMAX and NO_STRTOULL to cover both
the signed and unsigned functions. (We cannot change their names
without breaking existing makefile configurations.)
Signed-off-by: Nick Alcock <nix@esperi.org.uk>
---
Makefile | 6 +++---
compat/strtoimax.c | 10 ++++++++++
2 files changed, 13 insertions(+), 3 deletions(-)
create mode 100644 compat/strtoimax.c
diff --git a/Makefile b/Makefile
index 303a8df..a1f7e34 100644
--- a/Makefile
+++ b/Makefile
@@ -58,8 +58,8 @@ include /home/compiler/.configure/site.mk
#
# Define NO_STRLCPY if you don't have strlcpy.
#
-# Define NO_STRTOUMAX if you don't have strtoumax in the C library.
-# If your compiler also does not support long long or does not have
+# Define NO_STRTOUMAX if you don't have both strtoimax and strtoumax in the
+# C library. If your compiler also does not support long long or does not have
# strtoull, define NO_STRTOULL.
#
# Define NO_SETENV if you don't have setenv in the C library.
@@ -1459,7 +1459,7 @@ ifdef NO_STRLCPY
endif
ifdef NO_STRTOUMAX
COMPAT_CFLAGS += -DNO_STRTOUMAX
- COMPAT_OBJS += compat/strtoumax.o
+ COMPAT_OBJS += compat/strtoumax.o compat/strtoimax.o
endif
ifdef NO_STRTOULL
COMPAT_CFLAGS += -DNO_STRTOULL
diff --git a/compat/strtoimax.c b/compat/strtoimax.c
new file mode 100644
index 0000000..ac09ed8
--- /dev/null
+++ b/compat/strtoimax.c
@@ -0,0 +1,10 @@
+#include "../git-compat-util.h"
+
+intmax_t gitstrtoimax (const char *nptr, char **endptr, int base)
+{
+#if defined(NO_STRTOULL)
+ return strtol(nptr, endptr, base);
+#else
+ return strtoll(nptr, endptr, base);
+#endif
+}
--
1.7.6.1.138.g03ab.dirty
^ permalink raw reply related
* [PATCH 0/2] Support sizes >=2G in various config options, v2
From: Nick Alcock @ 2011-11-02 15:46 UTC (permalink / raw)
To: git; +Cc: Nick Alcock
New in this version:
- overflow detection, as suggested by Johannes Sixt (on 32-bit
platforms too).
- no renaming of NO_STRTOUMAX nor NO_STRTOULL.
I think this covers all the bases, including detection of configuration
values that overflow signed but not unsigned type only after
factor-application (as '3g' would on a 32-bit Linux box).
No new git testsuite failures. (No tests either, because I can't think
of one that will reliably induce overflow on a 64-bit box without being
totally ludicrous.)
Nick Alcock (2):
Add strtoimax() compatibility function.
Support sizes >=2G in various config options accepting 'g' sizes.
Makefile | 6 +++---
compat/strtoimax.c | 10 ++++++++++
config.c | 43 +++++++++++++++++++++++++++++++++----------
3 files changed, 46 insertions(+), 13 deletions(-)
create mode 100644 compat/strtoimax.c
--
1.7.6.1.138.g03ab.dirty
^ permalink raw reply
* TREAT AS URGENT..
From: Richard Hughes @ 2011-11-02 15:20 UTC (permalink / raw)
Hello,
Good Day,I am Richard Hughes Attorney to your relative whom was my client whilst alive.it is paramount and urgent we speak.
please reach me on mailrichard@inmail24.com
I await your response.
Thank you.
Regards,
Richard Hughes(Esq)
^ permalink raw reply
* Re: [PATCH] branch -m: handle no arg properly
From: Tay Ray Chuan @ 2011-11-02 16:17 UTC (permalink / raw)
To: Stefan Näwe; +Cc: git@vger.kernel.org, Junio C. Hamano
In-Reply-To: <4EB15D20.1060107@atlas-elektronik.com>
On Wed, 02 Nov 2011 16:09:20 +0100
Stefan Näwe <stefan.naewe@atlas-elektronik.com> wrote:
> Am 02.11.2011 16:01, schrieb Tay Ray Chuan:
> > Modify the option parsing heuristic to handle all -m (rename) cases,
> > including the no-arg case. Previously, this "fell through" to the argc
> > <= 2 case.
> >
> > Add a regression test in t3200-branch.sh while we're at it.
>
> Great. I just sent a patch for t3200 as well...
Hmm, yeah, printing usage is a good idea.
Popped my change to t3200 as well, yours looks better. :)
-->8--
Subject: [PATCH] branch -m: handle no arg properly
Modify the option parsing heuristic to handle all -m (rename) cases,
including the no-arg case. Previously, this "fell through" to the argc
<= 2 case.
Reported-by: Stefan Näwe <stefan.naewe@atlas-elektronik.com>
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
builtin/branch.c | 13 ++++++++-----
1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 009b713..51ca6a0 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -719,11 +719,14 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
else if (list)
return print_ref_list(kinds, detached, verbose, abbrev,
with_commit, argv);
- else if (rename && (argc == 1))
- rename_branch(head, argv[0], rename > 1);
- else if (rename && (argc == 2))
- rename_branch(argv[0], argv[1], rename > 1);
- else if (argc <= 2) {
+ else if (rename) {
+ if (argc == 1)
+ rename_branch(head, argv[0], rename > 1);
+ else if (argc == 2)
+ rename_branch(argv[0], argv[1], rename > 1);
+ else
+ usage_with_options(builtin_branch_usage, options);
+ } else if (argc <= 2) {
if (kinds != REF_LOCAL_BRANCH)
die(_("-a and -r options to 'git branch' do not make sense with a branch name"));
create_branch(head, argv[0], (argc == 2) ? argv[1] : head,
--
1.7.7.1.599.g03eec
--
Cheers,
Ray Chuan
^ permalink raw reply related
* [PATCH] t3200: add test case for 'branch -m'
From: Stefan Naewe @ 2011-11-02 15:07 UTC (permalink / raw)
To: git; +Cc: gitster, Stefan Naewe
In-Reply-To: <4EB153B4.6070404@atlas-elektronik.com>
Signed-off-by: Stefan Naewe <stefan.naewe@gmail.com>
---
t/t3200-branch.sh | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 2f5eada..3ce31b5 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -75,6 +75,11 @@ test_expect_success \
git branch l'
test_expect_success \
+ 'git branch -m dumps usage' \
+ 'test_expect_code 129 git branch -m 2>err &&
+ grep "[Uu]sage: git branch" err'
+
+test_expect_success \
'git branch -m m m/m should work' \
'git branch -l m &&
git branch -m m m/m &&
--
1.7.8.rc0.1.gb345ae
^ permalink raw reply related
* Re: [PATCH] branch -m: handle no arg properly
From: Stefan Näwe @ 2011-11-02 15:09 UTC (permalink / raw)
To: Tay Ray Chuan; +Cc: git@vger.kernel.org, Junio C. Hamano
In-Reply-To: <1320246098-6912-1-git-send-email-rctay89@gmail.com>
Am 02.11.2011 16:01, schrieb Tay Ray Chuan:
> Modify the option parsing heuristic to handle all -m (rename) cases,
> including the no-arg case. Previously, this "fell through" to the argc
> <= 2 case.
>
> Add a regression test in t3200-branch.sh while we're at it.
Great. I just sent a patch for t3200 as well...
Stefan
--
----------------------------------------------------------------
/dev/random says: If At First You Don't Succeed Ignore The Docs...
python -c "print '73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')"
^ permalink raw reply
* [PATCH] branch -m: handle no arg properly
From: Tay Ray Chuan @ 2011-11-02 15:01 UTC (permalink / raw)
To: git; +Cc: Stefan Näwe, Junio C. Hamano
In-Reply-To: <4EB153B4.6070404@atlas-elektronik.com>
Modify the option parsing heuristic to handle all -m (rename) cases,
including the no-arg case. Previously, this "fell through" to the argc
<= 2 case.
Add a regression test in t3200-branch.sh while we're at it.
Reported-by: Stefan Näwe <stefan.naewe@atlas-elektronik.com>
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
builtin/branch.c | 13 ++++++++-----
t/t3200-branch.sh | 4 ++++
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 009b713..ebda8e7 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -719,11 +719,14 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
else if (list)
return print_ref_list(kinds, detached, verbose, abbrev,
with_commit, argv);
- else if (rename && (argc == 1))
- rename_branch(head, argv[0], rename > 1);
- else if (rename && (argc == 2))
- rename_branch(argv[0], argv[1], rename > 1);
- else if (argc <= 2) {
+ else if (rename) {
+ if (argc == 1)
+ rename_branch(head, argv[0], rename > 1);
+ else if (argc == 2)
+ rename_branch(argv[0], argv[1], rename > 1);
+ else
+ die(_("new branch not specified for -m|--move"));
+ } else if (argc <= 2) {
if (kinds != REF_LOCAL_BRANCH)
die(_("-a and -r options to 'git branch' do not make sense with a branch name"));
create_branch(head, argv[0], (argc == 2) ? argv[1] : head,
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 2f5eada..78587fe 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -75,6 +75,10 @@ test_expect_success \
git branch l'
test_expect_success \
+ 'git branch -m with no arg fails' \
+ 'test_must_fail git branch -m'
+
+test_expect_success \
'git branch -m m m/m should work' \
'git branch -l m &&
git branch -m m m/m &&
--
1.7.7.1.599.g03eec
^ permalink raw reply related
* Re: git-p4: problem with commit 97a21ca50ef8
From: Vitor Antunes @ 2011-11-02 14:43 UTC (permalink / raw)
To: git
In-Reply-To: <CAOk9v+_xXRGAGWg2L5u=r9qBS=H+ZmdF=TwumSyq7WKf-15okw@mail.gmail.com>
Michael Wookey <michaelwookey <at> gmail.com> writes:
> Of course, I'd love to have git-p4 work seamlessly for this scenario.
> Even Perforce have a KB article on the limitation of the "apple"
> filetype with git-p4:
>
> http://kb.perforce.com/article/1417/git-p4
>
"""
Step 2: Download Git-p4
Recommended version is ermshiperete’s branch, which is available from:
https://github.com/ermshiperete/git-p4
Note: Omit the “git-p4.py25” file, which is an older version that is no longer
needed.
Avoid Kernel.org’s Version of Git-p4
Git’s main source at http://git-scm.com/download and
http://www.kernel.org/pub/software/scm/git/ contains an older version of Git-p4
with limitations that ermshiperete’s branch avoids.
"""
I can almost guess _who_ wrote this KB ;)
But this is really frustrating. Why can't people just cooperate to make sure the
version in the main branch is the latest?
Vitor
^ permalink raw reply
* 'git -m' dumps core
From: Stefan Näwe @ 2011-11-02 14:29 UTC (permalink / raw)
To: git@vger.kernel.org; +Cc: Junio C. Hamano
$ /usr/local/git-v1.7.8-rc0/bin/git version
git version 1.7.8.rc0
$ /usr/local/git-v1.7.8-rc0/bin/git branch -m
Speicherzugriffsfehler (core dumped)
$ /usr/local/git-v1.7.8-rc0/bin/git branch --move
Speicherzugriffsfehler (core dumped)
GDB says:
(gdb) bt
#0 0xb74694f3 in strlen () from /lib/i686/cmov/libc.so.6
#1 0x0810f1ad in strbuf_branchname (sb=0xbfb20bbc, name=0x0) at sha1_name.c:873
#2 0x0810f20e in strbuf_check_branch_ref (sb=0xbfb20bbc, name=0x0) at sha1_name.c:882
#3 0x080b516a in validate_new_branchname (name=0x0, ref=0xbfb20bbc, force=0, attr_only=0) at branch.c:142
#4 0x080b550c in create_branch (head=0x94e32ab "master", name=0x0, start_name=0x94e32ab "master", force=0, reflog=0, track=BRANCH_TRACK_REMOTE)
at branch.c:177
#5 0x0805a8fe in cmd_branch (argc=0, argv=0xbfb215f8, prefix=0x0) at builtin/branch.c:729
#6 0x0804ba29 in handle_internal_command (argc=2, argv=0xbfb215f8) at git.c:308
#7 0x0804bc67 in main (argc=2, argv=0xbfb215f8) at git.c:512
Stefan
--
----------------------------------------------------------------
/dev/random says: If ignorance is bliss, you must be ecstatic.
python -c "print '73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')"
^ permalink raw reply
* Re: [RFC/PATCH] http-push: don't always prompt for password (Was Re: [ANNOUNCE] Git 1.7.8.rc0)
From: Michael J Gruber @ 2011-11-02 14:08 UTC (permalink / raw)
To: Stefan Näwe; +Cc: Junio C Hamano, git@vger.kernel.org, Jeff King
In-Reply-To: <4EB104EA.2040001@atlas-elektronik.com>
Stefan Näwe venit, vidit, dixit 02.11.2011 09:52:
> Am 01.11.2011 19:12, schrieb Junio C Hamano:
>>
>> There are only handful of commits that even remotely touch http related
>> codepath between v1.7.7 and v1.7.8-rc0:
>>
>> * deba493 http_init: accept separate URL parameter
>>
>> This could change the URL string given to http_auth_init().
>>
>> * 070b4dd http: use hostname in credential description
>>
>> This only changes the prompt string; as far as I understand it, the
>> condition the password is prompted in the callsites of git_getpass()
>> has not changed.
>>
>> * 6cdf022 remote-curl: Fix warning after HTTP failure
>> * be22d92 http: avoid empty error messages for some curl errors
>> * 8abc508 http: remove extra newline in error message
>> * 8d677ed http: retry authentication failures for all http requests
>> * 28d0c10 remote-curl: don't retry auth failures with dumb protocol
>>
>> These shouldn't affect anything wrt prompting, unless you are somehow
>> internally reauthenticating.
>>
>> Could you try reverting deba493 and retest, and then if the behaviour is
>> the same "need ENTER", further revert 070b4dd and retest?
>
> I did a little more testing.
> This WIP makes it work for me (i.e. "need ENTER" is gone, works with
> and without .netrc, with 'https://host/repo.git' and
> 'https://user@host...' URL). Needs testing, of course.
>
> ---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---
> diff --git a/http.c b/http.c
> index a4bc770..008ad72 100644
> --- a/http.c
> +++ b/http.c
> @@ -279,8 +279,6 @@ static CURL *get_curl_handle(void)
> curl_easy_setopt(result, CURLOPT_HTTPAUTH, CURLAUTH_ANY);
> #endif
>
> - init_curl_http_auth(result);
> -
> if (ssl_cert != NULL)
> curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert);
> if (has_cert_password())
> @@ -846,7 +844,7 @@ static int http_request(const char *url, void *result, int target, int options)
> else if (missing_target(&results))
> ret = HTTP_MISSING_TARGET;
> else if (results.http_code == 401) {
> - if (user_name) {
> + if (user_name && user_pass) {
> ret = HTTP_NOAUTH;
> } else {
> /*
> @@ -855,7 +853,8 @@ static int http_request(const char *url, void *result, int target, int options)
> * but that is non-portable. Using git_getpass() can at least be stubbed
> * on other platforms with a different implementation if/when necessary.
> */
> - user_name = xstrdup(git_getpass_with_description("Username", description));
> + if (!user_name)
> + user_name = xstrdup(git_getpass_with_description("Username", description));
> init_curl_http_auth(slot->curl);
> ret = HTTP_REAUTH;
> }
> ---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---8<---
>
>
> Regards,
> Stefan
Thanks!
Tested-by: Michael J Gruber <git@drmicha.warpmail.net>
More specifically, I ran our test suite (next plus Stefan's patch), and
tested
https://user@host with .netrc and with askpass
https://host with .netrc
The latter fails with askpass because we ask
Password for 'host'
and not
Password for 'user@host'
but that is true both with and without the patch. (I thought we had
changed that, but I guess it's cooking.)
Michael
^ permalink raw reply
* Re: git sticker svg
From: Erik Faye-Lund @ 2011-11-02 12:22 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Junio C Hamano, Jeff King, git
In-Reply-To: <4EB11364.4010004@drmicha.warpmail.net>
[-- Attachment #1: Type: text/plain, Size: 926 bytes --]
On Wed, Nov 2, 2011 at 10:54 AM, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> Junio C Hamano venit, vidit, dixit 02.11.2011 10:18:
>> Michael J Gruber <git@drmicha.warpmail.net> writes:
>>
>>> I do prefer the rotated, vertically stacked +-G though (with + <-> t, -
>>> <-> i, G <-> 3/4-circle with arrow). Good thing we don't have to argue
>>> about an "official" logo...
>>
>> It somehow reminds me of the OGC logo fiasco, though.
>
> :)
>
> While looking up "OGC logo fiasco" certainly was fun, I don't think that
> our msysgit logo, e.g., is prone to wild fantasies' misinterpretation,
> no matter the position, uhm, rotation:
>
> http://code.google.com/p/msysgit/downloads/detail?name=gitlogo.svg
>
> But maybe my fantasy is not wild enough.
>
Since I'm kind of a typography and design geek and I pretty much hate
the execution of current logos I've seen, I thought I'd give it a go
myself ;)
What do you think?
[-- Attachment #2: git-logo.svg --]
[-- Type: image/svg+xml, Size: 3429 bytes --]
^ permalink raw reply
* Re: long fsck time
From: Nguyen Thai Ngoc Duy @ 2011-11-02 12:10 UTC (permalink / raw)
To: Git Mailing List
In-Reply-To: <CACsJy8D04Hw0_OoV01g2xtNK2d6fmZD_+YNEOR3A8aMUTpG5Lw@mail.gmail.com>
On Wed, Nov 2, 2011 at 7:06 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> On git.git
>
> $ /usr/bin/time git fsck
> 333.25user 4.28system 5:37.59elapsed 99%CPU (0avgtext+0avgdata
> 420080maxresident)k
> 0inputs+0outputs (0major+726560minor)pagefaults 0swaps
>
> That's really long time, perhaps we should print progress so users
> know it's still running?
Ahh.. --verbose. Sorry for the noise. Still good to show the number of
checked objects though.
--
Duy
^ permalink raw reply
* long fsck time
From: Nguyen Thai Ngoc Duy @ 2011-11-02 12:06 UTC (permalink / raw)
To: Git Mailing List
On git.git
$ /usr/bin/time git fsck
333.25user 4.28system 5:37.59elapsed 99%CPU (0avgtext+0avgdata
420080maxresident)k
0inputs+0outputs (0major+726560minor)pagefaults 0swaps
That's really long time, perhaps we should print progress so users
know it's still running?
--
Duy
^ permalink raw reply
* Re: [git patches] libata updates, GPG signed (but see admin notes)
From: Jochen Striepe @ 2011-11-02 11:20 UTC (permalink / raw)
To: Ingo Molnar
Cc: Linus Torvalds, Junio C Hamano, H. Peter Anvin, git,
James Bottomley, Jeff Garzik, Andrew Morton, linux-ide, LKML
In-Reply-To: <20111102091126.GG18903@elte.hu>
Hi,
On Wed, Nov 02, 2011 at 10:11:26AM +0100, Ingo Molnar wrote:
> If this approach is used then it would be nice to have a .gitconfig
> switch to require trusted pulls by default: to not allow doing
> non-signed or untrusted pulls accidentally, or for Git to warn in a
> visible, hard to miss way if there's a non-signed pull.
>
> This adds social uncertainty (and an element of a silent alarm) to a
> realistic attack: the attacker wouldnt know exactly how the puller
> checks signed pull requests, it's kept private.
But that way you get a false sense of alarm when someone sent a
perfectly trustable pull request, e.g. by signed email.
Another question: If store the actual pgp/gpg signatures in the git tree,
how do you handle signatures by keys which were valid by the time the
signature was made but expired when checking some time afterwards? AFAICT,
gpg will only tell you the key is expired _now_, and will make no statement
regarding the time the actual signature was made.
Thanks,
Jochen.
^ permalink raw reply
* Re: [git patches] libata updates, GPG signed (but see admin notes)
From: Michael J Gruber @ 2011-11-02 10:53 UTC (permalink / raw)
To: Junio C Hamano
Cc: Linus Torvalds, git, James Bottomley, Jeff Garzik, Andrew Morton,
linux-ide, LKML
In-Reply-To: <7vwrbjlj5r.fsf@alter.siamese.dyndns.org>
Junio C Hamano venit, vidit, dixit 01.11.2011 20:47:
> Linus Torvalds <torvalds@linux-foundation.org> writes:
>
>> But what would be nice is that "git pull" would fetch the tag (based on
>> name) *automatically*, and not actually create a tag in my repository at
>> all. Instead, if would use the tag to check the signature, and - if we
>> do this right - also use the tag contents to populate the merge commit
>> message.
>>
>> In other words, no actual tag would ever be left around as a turd, it
>> would simply be used as an automatic communication channel between the
>> "git push -s" of the submitter and my subsequent "git pull". Neither
>> side would have to do anything special, and the tag would never show
>> up in any relevant tree (it could even be in a totally separate
>> namespace like "refs/pullmarker/<branchname>" or something).
>
> While I like the "an ephemeral tag is used only for hop-to-hop
> communication to carry information to be recorded in the resulting
> history" approach, I see a few downsides.
>
> * The ephemeral tag needs to stay somewhere under refs/ hierarchy of the
> lieutenant's tree until you pick it up, even if they are out of the way
> in refs/pullmarker/$branchname. The next time the same lieutenant makes
> a pull request, either it will be overwritten or multiple versions of
> them refs/pullmarker/$branchname/$serial need to be kept.
If we are interested in commit sigs, the easiest tag-based approach is
to name the sig carrying tag by the commit's sha1. Just like the sig is
tied (in)to a commit in Junio's approach, it would be indexed by it. We
can do that now:
git config --global alias.sign '!f() { c=$(git rev-parse "$1") || exit;
shift; git tag -s $@ sigs/$c $c; }; f'
But a different place rather than refs/tags/sigs/<sha1> will be more
appropriate, so that we don't pollute the tag namespace. (Yes, this is
similar to storing them in notes.) tags have a message etc.
With an appropriate refspec, these sigs can be pushed out automatically
(by the lieutenant).
pull-request as in next will list the expected <sha1> at tip.
git pull needs to learn to (fetch and) use refs/<whatever>/<sha1> to
verify that the tip is signed.
git log --show-signature can do the same tricks as with in-commit sigs.
Some things to decide in this approach:
- Should git-pull (pull sigs and) verify by default?
- Should we worry about overwriting existings sigs? We have union-merge
for notes already, and that would be appropriate for sigs. (Yes, our
tags code does verify multiple concatenated sigs.)
The advantage of tags is that they can be added without rewriting the
commit, of course.
Michael
^ 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