* Re: Bug? url.insteadOf overwrites remote.pushUrl
From: Kirill Likhodedov @ 2011-10-14 19:16 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git
In-Reply-To: <4E9856BD.3010807@drmicha.warpmail.net>
>> On the other hand, pushInsteadOf doesn't
>> override explicit pushUrl. Is it a bug?
>
> That is as described in the thread (thanks for linking to it).
> pushurl predates pushinsteadof, and when the latter was introduced, one
> could have argued for or against "insteadof" being applied to pushurls.
> But that was necessary before, and existing behavior at the time when
> pushinsteadof was introduced. So, I don't see a bug, nor anything we
> could change now, though arguably most people use either pushinstead of
> or pushurl, but not both.
Yeah, that's clear now. As I already answered to Junio in another thread, my question was only about insteadOf, not pushInsteadOf behavior.
The behavior of pushInsteadOf is completely clear from the thread and from t5516-fetch-push.sh
Sorry for making it not clear enough.
Anyway, thanks for giving a one more explanation and example on how url, pushUrl and insteadOf configs work, it is always useful to go though it once again.
^ permalink raw reply
* Re: url.<base>.insteadOf with empty value
From: Kirill Likhodedov @ 2011-10-14 19:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Josh Triplett
In-Reply-To: <7v8von1mnd.fsf@alter.siamese.dyndns.org>
14.10.2011, в 21:57, Junio C Hamano writes:
>> If I don't specify any value for url.<base>.insteadOf or url.<base>.pushInsteadOf, Git substitutes all urls for remotes defined in .git/config
>>
>> If it is not a bug, I'd suggest to add a note to man git-config about this.
>
> Please assume that what the documentation says is clear enough for whoever
> wrote it and need no further clarification, so you would need to help them
> understand what additional things you may want the documentation to say,
> by clarifying "add a note" and "about this" a bit.
I would add the following to the "url.<base>.insteadOf" section of git-config:
:::: If empty value is specified in insteadOf, <base> will be added to the start of any URL.
> The "insteadOf" replacement is meant to apply for any URL we use. I would
> be surprised if it did not affect pushURL; it would be a bug if it didn't.
That was the question I wanted to clarify.
I thought that pushURL shouldn't be affected by insteadOf, because it is not affected by pushInsteadOf.
Thanks for clarifying that out.
> On the other hand, the rewrite done by "pushinsteadof" is meant to apply
> only when remote.<any>.url is used for pushing. See t/t5516-fetch-push.sh
> part of the patch for 1c2eafb (Add url.<base>.pushInsteadOf: URL rewriting
> for push only, 2009-09-07). It would clarify what the intended interaction
> among these configuration variables.
Yeah, that's clear. My question was only about insteadOf behavior. Sorry for making it not clear enough. The behavior of pushInsteadOf is completely clear from the discussion thread I mentioned and from t5516-fetch-push.sh
Thanks.
^ permalink raw reply
* Re: [PATCH] t7800: avoid arithmetic expansion notation
From: Junio C Hamano @ 2011-10-14 19:00 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Sitaram Chamarty
In-Reply-To: <7v4nzb1mjd.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
>
>> ba959de (git-difftool: allow skipping file by typing 'n' at prompt, 2011-10-08)
>> introduced shell code like
>>
>> $((foo; bar) | baz)
>>
>> which some shells (e.g. bash, dash) interpret as an unfinished arithmetic
>> evaluation $(( expr )).
>
> Ahh, thanks, I should have caught this. I recall I rewrote a similar one
> to $( (command; command) | command ) more than once before.
Forgot to mention that I love to see the issues on topics discovered and
corrected before they are still cooking in 'next', like this ;-)
Thanks all.
^ permalink raw reply
* Re: [PATCH 0/6] http-auth-early
From: Junio C Hamano @ 2011-10-14 18:59 UTC (permalink / raw)
To: Jeff King; +Cc: Michael J Gruber, git
In-Reply-To: <20111014131932.GE7808@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Your changes all look right. The naming of git_getpass_one in the
> cherry-picked commit is a little odd without the rest of the series as
> context. I would maybe have called it "git_getpass_with_description" or
> something.
Thanks both; will queue with that updated function name.
^ permalink raw reply
* [PATCH] send-email: Fix %config_path_settings handling
From: Jakub Narebski @ 2011-10-14 18:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael J Gruber, Git Mailing List, Cord Seele, Cord Seele
In-Reply-To: <7vwrc7zbk2.fsf@alter.siamese.dyndns.org>
From: Cord Seele <cowose@gmail.com>
cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30) broke
the expansion of aliases.
This was caused by treating %config_path_settings, newly introduced in
said patch, like %config_bool_settings instead of like %config_settings.
Copy from %config_settings, making it more readable.
Nb. there were a few issues that were responsible for this error:
1. %config_bool_settings and %config_settings despite similar name have
different semantic.
%config_bool_settings values are arrays where the first element is
(reference to) the variable to set, and second element is default
value... which admittedly is a bit cryptic. More readable if more
verbose option would be to use hash reference, e.g.:
my %config_bool_settings = (
"thread" => { variable => \$thread, default => 1},
[...]
Or something like that.
%config_settings values are either either reference to scalar variable
or reference to array. In second case it means that option (or config
option) is multi-valued. BTW. this is similar to what Getopt::Long does.
2. In cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30)
the setting "aliasesfile" was moved from %config_settings to newly
introduced %config_path_settings. But the loop that parses settings
from %config_path_settings was copy'n'pasted *wrongly* from
%config_bool_settings instead of from %config_settings.
It looks like cec5dae author cargo-culted this change...
3. 994d6c6 (send-email: address expansion for common mailers, 2006-05-14)
didn't add test for alias expansion to t9001-send-email.sh
Signed-off-by: Cord Seele <cowose@gmail.com>
Tested-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Junio C Hamano wrote:
> Thanks for a detailed write-up. I'd appreciate a final fix in an
> apply-able patch form.
Something like this?
Nb. I was not sure whether to keep Cord authorship...
git-send-email.perl | 11 +++++++++--
1 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 91607c5..41807b6 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -337,8 +337,15 @@ sub read_config {
}
foreach my $setting (keys %config_path_settings) {
- my $target = $config_path_settings{$setting}->[0];
- $$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
+ my $target = $config_path_settings{$setting};
+ if (ref($target) eq "ARRAY" && !@$target) {
+ # multi-valued and not set
+ my @values = Git::config_path(@repo, "$prefix.$setting");
+ @$target = @values if (@values && defined $values[0]);
+ } elsif (!defined $$target) {
+ # multi-valued and not set
+ $$target = Git::config_path(@repo, "$prefix.$setting");
+ }
}
foreach my $setting (keys %config_settings) {
--
1.7.6
^ permalink raw reply related
* Re: [PATCH] t1304: fall back to $USER if $LOGNAME is not defined
From: Junio C Hamano @ 2011-10-14 18:41 UTC (permalink / raw)
To: René Scharfe; +Cc: Git Mailing List, Brandon Casey, Matthieu Moy
In-Reply-To: <4E98750D.1020106@lsrfire.ath.cx>
René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> For some reason $LOGNAME is not set anymore for me after an upgrade from
> Ubuntu 11.04 to 11.10. Use $USER in such a case.
>
> Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
> ---
> The only other use of $LOGNAME is in git-cvsimport, which does the same.
While the change to assign $USER to $LOGNAME when the latter is not set is
not wrong per-se, I have to wonder (1) why this test should use LOGNAME
not USER in the first place, and (2) why you lost LOGNAME.
LOGNAME
The system shall initialize this variable at the time of login to be the
user's login name. See <pwd.h> . For a value of LOGNAME to be portable
across implementations of POSIX.1-2008, the value should be composed of
characters from the portable filename character set.
Will apply anyway. Thanks.
^ permalink raw reply
* Re: [PATCH 2/2] bundle: add parse_bundle_header() helper function
From: Junio C Hamano @ 2011-10-14 18:14 UTC (permalink / raw)
To: Phil Hord; +Cc: Junio C Hamano, git, Shawn O. Pearce, Phil Hord
In-Reply-To: <CABURp0otqxeKjLe-Rk3htZRa0M7+rerfrbVrXx-7Dr1tK3tTTg@mail.gmail.com>
Phil Hord <phil.hord@gmail.com> writes:
> On Thu, Oct 13, 2011 at 6:35 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Move most of the code from read_bundle_header() to parse_bundle_header()
>> that takes a file descriptor that is already opened for reading, and make
>> the former responsible only for opening the file and noticing errors.
> ...
>
>> * It generally is a bad practice to base a non-RFC patch on an RFC one,
>> but in any case here is how I would do the is_bundle() helper.
>
> I didn't label it RFC,...
I was referring to the fact that _my_ 2/2 you are responding to was meant
to be applied on top of _my_ 1/2 that was marked as RFC.
^ permalink raw reply
* Re: [PATCH] fix alias expansion with new Git::config_path()
From: Junio C Hamano @ 2011-10-14 18:13 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Michael J Gruber, Git Mailing List
In-Reply-To: <201110141838.19118.jnareb@gmail.com>
Jakub Narebski <jnareb@gmail.com> writes:
> P.S. Junio, does t9001 pass for you?
It seems to.
Thanks for a detailed write-up. I'd appreciate a final fix in an
apply-able patch form.
^ permalink raw reply
* [PATCH 2/2] downgrade "packfile cannot be accessed" errors to warnings
From: Jeff King @ 2011-10-14 18:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20111014012320.GA4395@sigill.intra.peff.net>
These can happen if another process simultaneously prunes a
pack. But that is not usually an error condition, because a
properly-running prune should have repacked the object into
a new pack. So we will notice that the pack has disappeared
unexpectedly, print a message, try other packs (possibly
after re-scanning the list of packs), and find it in the new
pack.
Acked-by: Nicolas Pitre <nico@fluxnic.net>
Signed-off-by: Jeff King <peff@peff.net>
---
Repost with ack from Nicolas, and more obvious subject line. No changes
from earlier version in this thread.
builtin/pack-objects.c | 2 +-
sha1_file.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 8681ccd..ba3705d 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -805,7 +805,7 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,
if (offset) {
if (!found_pack) {
if (!is_pack_valid(p)) {
- error("packfile %s cannot be accessed", p->pack_name);
+ warning("packfile %s cannot be accessed", p->pack_name);
continue;
}
found_offset = offset;
diff --git a/sha1_file.c b/sha1_file.c
index a22c5b4..27f3b9b 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2038,7 +2038,7 @@ static int find_pack_entry(const unsigned char *sha1, struct pack_entry *e)
* was loaded!
*/
if (!is_pack_valid(p)) {
- error("packfile %s cannot be accessed", p->pack_name);
+ warning("packfile %s cannot be accessed", p->pack_name);
goto next;
}
e->offset = offset;
--
1.7.6.4.37.g43b58b
^ permalink raw reply related
* [PATCH 1/2] pack-objects: protect against disappearing packs
From: Jeff King @ 2011-10-14 18:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20111014012320.GA4395@sigill.intra.peff.net>
It's possible that while pack-objects is running, a
simultaneously running prune process might delete a pack
that we are interested in. Because we load the pack indices
early on, we know that the pack contains our item, but by
the time we try to open and map it, it is gone.
Since c715f78, we already protect against this in the normal
object access code path, but pack-objects accesses the packs
at a lower level. In the normal access path, we call
find_pack_entry, which will call find_pack_entry_one on each
pack index, which does the actual lookup. If it gets a hit,
we will actually open and verify the validity of the
matching packfile (using c715f78's is_pack_valid). If we
can't open it, we'll issue a warning and pretend that we
didn't find it, causing us to go on to the next pack (or on
to loose objects).
Furthermore, we will cache the descriptor to the opened
packfile. Which means that later, when we actually try to
access the object, we are likely to still have that packfile
opened, and won't care if it has been unlinked from the
filesystem.
Notice the "likely" above. If there is another pack access
in the interim, and we run out of descriptors, we could
close the pack. And then a later attempt to access the
closed pack could fail (we'll try to re-open it, of course,
but it may have been deleted). In practice, this doesn't
happen because we tend to look up items and then access them
immediately.
Pack-objects does not follow this code path. Instead, it
accesses the packs at a much lower level, using
find_pack_entry_one directly. This means we skip the
is_pack_valid check, and may end up with the name of a
packfile, but no open descriptor.
We can add the same is_pack_valid check here. Unfortunately,
the access patterns of pack-objects are not quite as nice
for keeping lookup and object access together. We look up
each object as we find out about it, and the only later when
writing the packfile do we necessarily access it. Which
means that the opened packfile may be closed in the interim.
In practice, however, adding this check still has value, for
three reasons.
1. If you have a reasonable number of packs and/or a
reasonable file descriptor limit, you can keep all of
your packs open simultaneously. If this is the case,
then the race is impossible to trigger.
2. Even if you can't keep all packs open at once, you
may end up keeping the deleted one open (i.e., you may
get lucky).
3. The race window is shortened. You may notice early that
the pack is gone, and not try to access it. Triggering
the problem without this check means deleting the pack
any time after we read the list of index files, but
before we access the looked-up objects. Triggering it
with this check means deleting the pack means deleting
the pack after we do a lookup (and successfully access
the packfile), but before we access the object. Which
is a smaller window.
Acked-by: Nicolas Pitre <nico@fluxnic.net>
Signed-off-by: Jeff King <peff@peff.net>
---
Re-post with ack from Nicolas and my SOB fixed. No changes from earlier
version in this thread.
builtin/pack-objects.c | 4 ++++
cache.h | 1 +
sha1_file.c | 2 +-
3 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 2b18de5..8681ccd 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -804,6 +804,10 @@ static int add_object_entry(const unsigned char *sha1, enum object_type type,
off_t offset = find_pack_entry_one(sha1, p);
if (offset) {
if (!found_pack) {
+ if (!is_pack_valid(p)) {
+ error("packfile %s cannot be accessed", p->pack_name);
+ continue;
+ }
found_offset = offset;
found_pack = p;
}
diff --git a/cache.h b/cache.h
index e39e160..495b468 100644
--- a/cache.h
+++ b/cache.h
@@ -1055,6 +1055,7 @@ struct extra_have_objects {
extern const unsigned char *nth_packed_object_sha1(struct packed_git *, uint32_t);
extern off_t nth_packed_object_offset(const struct packed_git *, uint32_t);
extern off_t find_pack_entry_one(const unsigned char *, struct packed_git *);
+extern int is_pack_valid(struct packed_git *);
extern void *unpack_entry(struct packed_git *, off_t, enum object_type *, unsigned long *);
extern unsigned long unpack_object_header_buffer(const unsigned char *buf, unsigned long len, enum object_type *type, unsigned long *sizep);
extern unsigned long get_size_from_delta(struct packed_git *, struct pack_window **, off_t);
diff --git a/sha1_file.c b/sha1_file.c
index 3401301..a22c5b4 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1987,7 +1987,7 @@ off_t find_pack_entry_one(const unsigned char *sha1,
return 0;
}
-static int is_pack_valid(struct packed_git *p)
+int is_pack_valid(struct packed_git *p)
{
/* An already open pack is known to be valid. */
if (p->pack_fd != -1)
--
1.7.6.4.37.g43b58b
^ permalink raw reply related
* Re: [PATCH] t7800: avoid arithmetic expansion notation
From: Junio C Hamano @ 2011-10-14 18:00 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Sitaram Chamarty
In-Reply-To: <837ad77348b459aa5f5f79e556dbeeeba41027e7.1318594392.git.git@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
> ba959de (git-difftool: allow skipping file by typing 'n' at prompt, 2011-10-08)
> introduced shell code like
>
> $((foo; bar) | baz)
>
> which some shells (e.g. bash, dash) interpret as an unfinished arithmetic
> evaluation $(( expr )).
Ahh, thanks, I should have caught this. I recall I rewrote a similar one
to $( (command; command) | command ) more than once before.
^ permalink raw reply
* Re: url.<base>.insteadOf with empty value
From: Junio C Hamano @ 2011-10-14 17:57 UTC (permalink / raw)
To: Kirill Likhodedov; +Cc: git, Josh Triplett
In-Reply-To: <54556728-92C0-4992-9831-0D582C383235@jetbrains.com>
Kirill Likhodedov <Kirill.Likhodedov@jetbrains.com> writes:
> If I don't specify any value for url.<base>.insteadOf or url.<base>.pushInsteadOf, Git substitutes all urls for remotes defined in .git/config
>
> Probably that's because any url starts with empty string and thus has to be substituted.
> But it might be a bit confusing, because on the other hand if no value is given to the property insteadOf, user may expect this property to be ignored.
>
> Please check if current Git behavior is correct.
>
> If it is not a bug, I'd suggest to add a note to man git-config about this.
Please assume that what the documentation says is clear enough for whoever
wrote it and need no further clarification, so you would need to help them
understand what additional things you may want the documentation to say,
by clarifying "add a note" and "about this" a bit.
The "insteadOf" replacement is meant to apply for any URL we use. I would
be surprised if it did not affect pushURL; it would be a bug if it didn't.
On the other hand, the rewrite done by "pushinsteadof" is meant to apply
only when remote.<any>.url is used for pushing. See t/t5516-fetch-push.sh
part of the patch for 1c2eafb (Add url.<base>.pushInsteadOf: URL rewriting
for push only, 2009-09-07). It would clarify what the intended interaction
among these configuration variables.
Thanks.
^ permalink raw reply
* [PATCH] t1304: fall back to $USER if $LOGNAME is not defined
From: René Scharfe @ 2011-10-14 17:44 UTC (permalink / raw)
To: Git Mailing List; +Cc: Junio C Hamano, Brandon Casey, Matthieu Moy
For some reason $LOGNAME is not set anymore for me after an upgrade from
Ubuntu 11.04 to 11.10. Use $USER in such a case.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
The only other use of $LOGNAME is in git-cvsimport, which does the same.
t/t1304-default-acl.sh | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh
index b5d89a2..2b962cf 100755
--- a/t/t1304-default-acl.sh
+++ b/t/t1304-default-acl.sh
@@ -25,6 +25,11 @@ else
test_set_prereq SETFACL
fi
+if test -z "$LOGNAME"
+then
+ LOGNAME=$USER
+fi
+
check_perms_and_acl () {
test -r "$1" &&
getfacl "$1" > actual &&
--
1.7.7
^ permalink raw reply related
* Re: [PATCH] fix alias expansion with new Git::config_path()
From: Jakub Narebski @ 2011-10-14 16:38 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <4E984781.6050601@drmicha.warpmail.net>
Michael J Gruber wrote:
> Cord Seele venit, vidit, dixit 14.10.2011 16:25:
>> On Fri 14 Oct 2011 14:29:27 +0200, Michael J Gruber <git@drmicha.warpmail.net> wrote:
>>
>>> cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30)
>>> broke the expansion of aliases for me:
[...]
>>> Reverting cec5dae brings my aliases back. [...]
[...]
>>
>> The following patch fixes it for me, please give it a try.
>>
>> Since this fix is simply copy&pasting some code from the config_settings path
>> someone with better perl understanding might wnat to refactor it
>> (Junio/Jacob)?
[missing commit message]
>> Signed-off-by: Cord Seele <cowose@gmail.com>
>
> Tested-by: Michael J Gruber <git@drmicha.warpmail.net>
>
> Thanks. (Though I'm still wondering what this is about overall.)
There were a few issues that were responsible for this error:
1. %config_bool_settings and %config_settings despite similar name have
different semantic.
%config_bool_settings values are arrays where the first element is
(reference to) the variable to set, and second element is default
value... which admittedly is a bit cryptic. More readable if more
verbose option would be to use hash reference, e.g.:
my %config_bool_settings = (
"thread" => { variable => \$thread, default => 1},
[...]
Or something like that.
%config_settings values are either either reference to scalar variable
or reference to array. In second case it means that option (or config
option) is multi-valued. BTW. this is similar to what Getopt::Long does.
2. In cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30)
the setting "aliasesfile" was moved from %config_settings to newly
introduced %config_path_settings. But the loop that parses settings
from %config_path_settings was copy'n'pasted *wrongly* from
%config_bool_settings instead of from %config_settings.
It looks like cec5dae author cargo-culted this change...
3. 994d6c6 (send-email: address expansion for common mailers, 2006-05-14)
didn't add test for alias expansion to t9001-send-email.sh
>> ---
>> git-send-email.perl | 12 ++++++++++--
>> 1 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/git-send-email.perl b/git-send-email.perl
>> index 91607c5..6885dfa 100755
>> --- a/git-send-email.perl
>> +++ b/git-send-email.perl
>> @@ -337,8 +337,16 @@ sub read_config {
>> }
>>
>> foreach my $setting (keys %config_path_settings) {
>> - my $target = $config_path_settings{$setting}->[0];
>> - $$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
>> + my $target = $config_path_settings{$setting};
>> + if (ref($target) eq "ARRAY") {
>> + unless (@$target) {
>> + my @values = Git::config_path(@repo, "$prefix.$setting");
>> + @$target = @values if (@values && defined $values[0]);
>> + }
>> + }
>> + else {
>> + $$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
>> + }
>> }
>>
>> foreach my $setting (keys %config_settings) {
Or a bit simpler (though still duplicated somewhat code with
%config_settings) case:
diff --git i/git-send-email.perl w/git-send-email.perl
index 91607c5..eed241e 100755
--- i/git-send-email.perl
+++ w/git-send-email.perl
@@ -337,8 +337,13 @@ sub read_config {
}
foreach my $setting (keys %config_path_settings) {
- my $target = $config_path_settings{$setting}->[0];
- $$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
+ my $target = $config_path_settings{$setting};
+ if (ref($target) eq "ARRAY" && !@$target) {
+ my @values = Git::config_path(@repo, "$prefix.$setting");
+ @$target = @values if (@values && defined $values[0]);
+ } elsif (!defined $$target) {
+ $$target = Git::config_path(@repo, "$prefix.$setting");
+ }
}
foreach my $setting (keys %config_settings) {
P.S. Junio, does t9001 pass for you? For me it fails very strangely on
some tests:
not ok - 21 reject long lines
not ok - 22 no patch was sent
not ok - 28 In-Reply-To without --chain-reply-to
not ok - 29 In-Reply-To with --chain-reply-to
not ok - 39 sendemail.cccmd
not ok - 49 --suppress-cc=bodycc
not ok - 51 --suppress-cc=cc
not ok - 56 confirm by default (due to cc)
not ok - 70 warning with an implicit --chain-reply-to
# failed 9 among 93 test(s)
--
Jakub Narebski
Poland
^ permalink raw reply related
* Re: [BUG] remote-curl.c: honor pushurl
From: Jeff King @ 2011-10-14 15:48 UTC (permalink / raw)
To: Michael Schubert; +Cc: git, rctay89
In-Reply-To: <4E985744.6050904@elegosoft.com>
On Fri, Oct 14, 2011 at 05:37:40PM +0200, Michael Schubert wrote:
> When doing a push (fetch, ..) over http(s), git calls git-remote-http to
> communicate with the server.
>
> git-remote-http <remote> [<url>]
>
> Git correctly honors a configured pushurl and passes it to git-remote-http,
> but git-remote-http is initiating the http connection with the url defined
> for remote (remote->url) rather than the passed url. This undermines the
> purpose of a config like
>
> url = https://example.com/repo.git
> pushurl = https://user@example.com/repo.git
>
> Introduced around 888692b7 - CC'ing Tay Ray Chuan. (I don't know if it was
> working before, though.)
Already noticed and fixed last week.
See this thread, starting at the focused message.
http://thread.gmane.org/gmane.comp.version-control.git/182752/focus=182872
-Peff
^ permalink raw reply
* [BUG] remote-curl.c: honor pushurl
From: Michael Schubert @ 2011-10-14 15:37 UTC (permalink / raw)
To: git; +Cc: rctay89
When doing a push (fetch, ..) over http(s), git calls git-remote-http to
communicate with the server.
git-remote-http <remote> [<url>]
Git correctly honors a configured pushurl and passes it to git-remote-http,
but git-remote-http is initiating the http connection with the url defined
for remote (remote->url) rather than the passed url. This undermines the
purpose of a config like
url = https://example.com/repo.git
pushurl = https://user@example.com/repo.git
Introduced around 888692b7 - CC'ing Tay Ray Chuan. (I don't know if it was
working before, though.)
^ permalink raw reply
* Re: Bug? url.insteadOf overwrites remote.pushUrl
From: Michael J Gruber @ 2011-10-14 15:35 UTC (permalink / raw)
To: Kirill Likhodedov; +Cc: git
In-Reply-To: <CAB6D58F-A3C9-4532-A9CC-10E43CD34E4E@jetbrains.com>
Kirill Likhodedov venit, vidit, dixit 14.10.2011 15:55:
>
> I've found that defining url.<base>.insteadOf overrides explicit
> remote.<name>.pushUrl.
It doesn't really override it. It is applied to it, i.e. transforms it.
> On the other hand, pushInsteadOf doesn't
> override explicit pushUrl. Is it a bug?
That is as described in the thread (thanks for linking to it).
> # cat .git/config [remote "origin"] fetch =
> +refs/heads/*:refs/remotes/origin/* url = github.com/klikh/Test.git
> pushUrl = jetbrains.com/klikh/Test.git [url "http://"] insteadOf=jet
>
> # git remote -v origin github.com/klikh/Test.git (fetch) origin
> http://brains.com/klikh/Test.git (push)
The idea of "pushInsteadOf" was that, instead of having to define url
and pushurl separately, you can use different rules, say
github -> git://github.com (fetch)
github -> https://username@github.com (push)
github/ -> git@github.com: (push)
used with "url = github/otheruser/repo.git" alone, without pushurl.
pushurl predates pushinsteadof, and when the latter was introduced, one
could have argued for or against "insteadof" being applied to pushurls.
But that was necessary before, and existing behavior at the time when
pushinsteadof was introduced. So, I don't see a bug, nor anything we
could change now, though arguably most people use either pushinstead of
or pushurl, but not both.
Cheers,
Michael
^ permalink raw reply
* Re: [PATCH] fix alias expansion with new Git::config_path()
From: Junio C Hamano @ 2011-10-14 15:05 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Git Mailing List, Jakub Narebski
In-Reply-To: <20111014144203.GC13680@laptop>
Cord Seele <cowose@googlemail.com> writes:
> On Fri 14 Oct 2011 16:30:25 +0200, Michael J Gruber <git@drmicha.warpmail.net> wrote:
>
>> Tested-by: Michael J Gruber <git@drmicha.warpmail.net>
>
> Great.
>
>> Thanks. (Though I'm still wondering what this is about overall.)
>
> to make '~/' work in sendemail.aliasesfile
That is not an explanation.
^ permalink raw reply
* Re: [PATCH] fix alias expansion with new Git::config_path()
From: Cord Seele @ 2011-10-14 14:42 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Git Mailing List, Junio C Hamano, Jakub Narebski
In-Reply-To: <4E984781.6050601@drmicha.warpmail.net>
On Fri 14 Oct 2011 16:30:25 +0200, Michael J Gruber <git@drmicha.warpmail.net> wrote:
> Tested-by: Michael J Gruber <git@drmicha.warpmail.net>
Great.
> Thanks. (Though I'm still wondering what this is about overall.)
to make '~/' work in sendemail.aliasesfile
-- Cord
^ permalink raw reply
* Re: [PATCH] pack-objects: protect against disappearing packs
From: Johannes Sixt @ 2011-10-14 14:35 UTC (permalink / raw)
To: Jeff King; +Cc: git, git-dev, Shawn O. Pearce, Nicolas Pitre
In-Reply-To: <20111014130703.GB7808@sigill.intra.peff.net>
Am 10/14/2011 15:07, schrieb Jeff King:
> Within a single process, I don't think so. This change impacts only
> pack-objects, which always runs as a separate process, and never deletes
> packs itself. The most likely problematic code path would be "git
> repack -d", but it waits for pack-objects to complete successfully
> before removing any packs.
Thanks. The test suite didn't find any issues on Windows.
-- Hannes
^ permalink raw reply
* Re: [PATCH] fix alias expansion with new Git::config_path()
From: Michael J Gruber @ 2011-10-14 14:30 UTC (permalink / raw)
To: Git Mailing List, Junio C Hamano, Jakub Narebski
In-Reply-To: <20111014142557.GB13680@laptop>
Cord Seele venit, vidit, dixit 14.10.2011 16:25:
> On Fri 14 Oct 2011 14:29:27 +0200, Michael J Gruber <git@drmicha.warpmail.net> wrote:
>
>> cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30)
>>
>> broke the expansion of aliases for me:
>>
>> ./git-send-email --cc=junio --dry-run
>> 0001-t7800-avoid-arithmetic-expansion-notation.patch
>> 0001-t7800-avoid-arithmetic-expansion-notation.patch
>> Who should the emails appear to be from? [Michael J Gruber
>> <git@drmicha.warpmail.net>]
>> Emails will be sent from: Michael J Gruber <git@drmicha.warpmail.net>
>> Dry-OK. Log says:
>> Sendmail: /home/mjg/bin/msmtp-fastmail-git -i git@vger.kernel.org junio
>> git@drmicha.warpmail.net
>> From: Michael J Gruber <git@drmicha.warpmail.net>
>> To: git@vger.kernel.org
>> Cc: junio
>> ...
>>
>> Happens with both "--cc junio" and "--cc=junio".
>>
>> Reverting cec5dae brings my aliases back. Relevant config:
>>
>> git config --get-regexp sendemail.alias\*
>> sendemail.aliasesfile /home/mjg/git/gitauthors
>> sendemail.aliasfiletype mutt
>>
>> Can I please have alias expansion back?
>
> The following patch fixes it for me, please give it a try.
>
> Since this fix is simply copy&pasting some code from the config_settings path
> someone with better perl understanding might wnat to refactor it
> (Junio/Jacob)?
>
> -- Cord
>
>
> Signed-off-by: Cord Seele <cowose@gmail.com>
Tested-by: Michael J Gruber <git@drmicha.warpmail.net>
Thanks. (Though I'm still wondering what this is about overall.)
> ---
> git-send-email.perl | 12 ++++++++++--
> 1 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 91607c5..6885dfa 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -337,8 +337,16 @@ sub read_config {
> }
>
> foreach my $setting (keys %config_path_settings) {
> - my $target = $config_path_settings{$setting}->[0];
> - $$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
> + my $target = $config_path_settings{$setting};
> + if (ref($target) eq "ARRAY") {
> + unless (@$target) {
> + my @values = Git::config_path(@repo, "$prefix.$setting");
> + @$target = @values if (@values && defined $values[0]);
> + }
> + }
> + else {
> + $$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
> + }
> }
>
> foreach my $setting (keys %config_settings) {
^ permalink raw reply
* [PATCH] fix alias expansion with new Git::config_path()
From: Cord Seele @ 2011-10-14 14:25 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Git Mailing List, Junio C Hamano, Jakub Narebski
In-Reply-To: <4E982B27.8050807@drmicha.warpmail.net>
On Fri 14 Oct 2011 14:29:27 +0200, Michael J Gruber <git@drmicha.warpmail.net> wrote:
> cec5dae (use new Git::config_path() for aliasesfile, 2011-09-30)
>
> broke the expansion of aliases for me:
>
> ./git-send-email --cc=junio --dry-run
> 0001-t7800-avoid-arithmetic-expansion-notation.patch
> 0001-t7800-avoid-arithmetic-expansion-notation.patch
> Who should the emails appear to be from? [Michael J Gruber
> <git@drmicha.warpmail.net>]
> Emails will be sent from: Michael J Gruber <git@drmicha.warpmail.net>
> Dry-OK. Log says:
> Sendmail: /home/mjg/bin/msmtp-fastmail-git -i git@vger.kernel.org junio
> git@drmicha.warpmail.net
> From: Michael J Gruber <git@drmicha.warpmail.net>
> To: git@vger.kernel.org
> Cc: junio
> ...
>
> Happens with both "--cc junio" and "--cc=junio".
>
> Reverting cec5dae brings my aliases back. Relevant config:
>
> git config --get-regexp sendemail.alias\*
> sendemail.aliasesfile /home/mjg/git/gitauthors
> sendemail.aliasfiletype mutt
>
> Can I please have alias expansion back?
The following patch fixes it for me, please give it a try.
Since this fix is simply copy&pasting some code from the config_settings path
someone with better perl understanding might wnat to refactor it
(Junio/Jacob)?
-- Cord
Signed-off-by: Cord Seele <cowose@gmail.com>
---
git-send-email.perl | 12 ++++++++++--
1 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index 91607c5..6885dfa 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -337,8 +337,16 @@ sub read_config {
}
foreach my $setting (keys %config_path_settings) {
- my $target = $config_path_settings{$setting}->[0];
- $$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
+ my $target = $config_path_settings{$setting};
+ if (ref($target) eq "ARRAY") {
+ unless (@$target) {
+ my @values = Git::config_path(@repo, "$prefix.$setting");
+ @$target = @values if (@values && defined $values[0]);
+ }
+ }
+ else {
+ $$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
+ }
}
foreach my $setting (keys %config_settings) {
--
1.7.6.4
^ permalink raw reply related
* Re: defined behaviour for multiple urls for a remote
From: Sitaram Chamarty @ 2011-10-14 14:01 UTC (permalink / raw)
To: Kirill Likhodedov; +Cc: git
In-Reply-To: <loom.20111014T113208-660@post.gmane.org>
On Fri, Oct 14, 2011 at 3:06 PM, Kirill Likhodedov
<Kirill.Likhodedov@jetbrains.com> wrote:
> Sitaram Chamarty <sitaramc <at> gmail.com> writes:
>
>> What's the defined behaviour if I do this:
>>
>> [remote "both"]
>> url = https://code.google.com/p/gitolite/
>> url = git <at> github.com:sitaramc/gitolite.git
>>
>> I know what I'm seeing (a fetch only goes to the first URL, and does a
>> HEAD->FETCH_HEAD because I didn't provide a refspec line, while a push
>> seems to push all to both), but I was curious what the official
>> position is, because I couldn't find it in the docs.
>
> Please see the message from Linus about that: http://marc.info/?l=git&m=116231242118202&w=2
cool; thanks!
> You also may check how Git understands your remotes by running
> git remote -v
Aah that's very clear!
> It will show, where it is going to fetch from and push to.
>
> I agree though, that documentation should be updated.
well if it never came up once in the 5 years since Linus wrote that email... :-)
^ permalink raw reply
* Bug? url.insteadOf overwrites remote.pushUrl
From: Kirill Likhodedov @ 2011-10-14 13:55 UTC (permalink / raw)
To: git
I've found that defining url.<base>.insteadOf overrides explicit remote.<name>.pushUrl.
On the other hand, pushInsteadOf doesn't override explicit pushUrl.
Is it a bug?
# cat .git/config
[remote "origin"]
fetch = +refs/heads/*:refs/remotes/origin/*
url = github.com/klikh/Test.git
pushUrl = jetbrains.com/klikh/Test.git
[url "http://"]
insteadOf=jet
# git remote -v
origin github.com/klikh/Test.git (fetch)
origin http://brains.com/klikh/Test.git (push)
See also http://kerneltrap.org/mailarchive/git/2009/9/7/11264/thread - patch introducing pushInsteadOf & discussion
----------------------------------
Kirill Likhodedov
JetBrains, Inc
http://www.jetbrains.com
"Develop with pleasure!"
^ permalink raw reply
* Re: [PATCH 0/6] http-auth-early
From: Michael J Gruber @ 2011-10-14 13:24 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano
In-Reply-To: <20111014131932.GE7808@sigill.intra.peff.net>
Jeff King venit, vidit, dixit 14.10.2011 15:19:
> On Fri, Oct 14, 2011 at 09:40:34AM +0200, Michael J Gruber wrote:
>
>> Here are the early parts of Jeff's http-auth-keyring series.
>> It contains only parts which are not using the credential API (which
>> is still under discussion), so that this can go in (and help users)
>> and alleviates the pressure on the credential discussion:
>>
>> Early bits with cleanups to http.c.
>> Cherry-picked bit for improved prompts ("Username for ..." etc.)
>> Cherry-pickes bit for using configured pushurls.
>>
>> I tried to pick/resolve in a way which should help rebasing Jeff's series
>> on top of this.
>
> Thanks for working on this. One of my intended tasks for today is to
> rebase my series, so it is nice to wake up to half of the work done. :)
Good morning :)
>> Jeff King (5):
>> url: decode buffers that are not NUL-terminated
>> improve httpd auth tests
>> remote-curl: don't retry auth failures with dumb protocol
>> http: retry authentication failures for all http requests
>> http_init: accept separate URL parameter
>>
>> Michael J Gruber (1):
>> http: use hostname in credential description
>
> Your changes all look right. The naming of git_getpass_one in the
> cherry-picked commit is a little odd without the rest of the series as
> context. I would maybe have called it "git_getpass_with_description" or
> something.
git_getpass_my_life_will_be_short_and_ended_by_credentials
I don't care. In fact, I wasn't sure whether I should I even change the
author on this one. It's not a straight resolution and does involve
choices, but the meat is from your series.
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