* [PATCHv2 8/8] gitweb: Use esc_html_match_hl() in 'grep' search
From: Jakub Narebski @ 2012-02-15 20:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narebski
In-Reply-To: <1329338332-30358-1-git-send-email-jnareb@gmail.com>
Use esc_html_match_hl() in git_search_files() i.e. subroutine that
implements "grep" search, instead of custom code which highlighted
only one, last match.
This required enhancing esc_html_match_hl() to accept -nbsp=>1 and
pass it down to esc_html().
Note that line is untabified (tabs turned into spaces) before
highlighting match, which means that highlighting won't work e.g. for
matching tab character "\t" explicitly; but this issue was present
before this commit, and is not that easy to fix.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This patch was not present in previous (v1) version of this patch
series.
The 'grep' search was chosen from other searches because of the
following reasons:
* 'pickaxe' search does not show matches in diff, only filenames.
* 'commit' search shortens leading text at beginning, one last match
in the middle, and trailing text at the end; anyway I think this
search should be rewritten to show just "log"-like view with match
highlighting.
gitweb/gitweb.perl | 34 ++++++++++++++--------------------
1 files changed, 14 insertions(+), 20 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index a2e2023..36a8cca 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1723,20 +1723,22 @@ sub chop_and_escape_str {
# '<span class="mark">foo</span>bar'
sub esc_html_hl_regions {
my ($str, $css_class, @sel) = @_;
- return esc_html($str) unless @sel;
+ my %opts = grep { ref($_) ne 'ARRAY' } @sel;
+ @sel = grep { ref($_) eq 'ARRAY' } @sel;
+ return esc_html($str, %opts) unless @sel;
my $out = '';
my $pos = 0;
for my $s (@sel) {
- $out .= esc_html(substr($str, $pos, $s->[0] - $pos))
+ $out .= esc_html(substr($str, $pos, $s->[0] - $pos), %opts)
if ($s->[0] - $pos > 0);
$out .= $cgi->span({-class => $css_class},
- esc_html(substr($str, $s->[0], $s->[1] - $s->[0])));
+ esc_html(substr($str, $s->[0], $s->[1] - $s->[0]), %opts));
$pos = $s->[1];
}
- $out .= esc_html(substr($str, $pos))
+ $out .= esc_html(substr($str, $pos), %opts)
if ($pos < length($str));
return $out;
@@ -1744,23 +1746,23 @@ sub esc_html_hl_regions {
# highlight match (if any), and escape HTML
sub esc_html_match_hl {
- my ($str, $regexp) = @_;
- return esc_html($str) unless defined $regexp;
+ my ($str, $regexp, %opts) = @_;
+ return esc_html($str, %opts) unless defined $regexp;
- return esc_html_match_hl_chopped($str, undef, $regexp);
+ return esc_html_match_hl_chopped($str, undef, $regexp, %opts);
}
# highlight match (if any) of shortened string, and escape HTML
sub esc_html_match_hl_chopped {
- my ($str, $chopped, $regexp) = @_;
- return esc_html(defined $chopped ? $chopped : $str) unless defined $regexp;
+ my ($str, $chopped, $regexp, %opts) = @_;
+ return esc_html(defined $chopped ? $chopped : $str, %opts) unless defined $regexp;
my @matches;
while ($str =~ /$regexp/g) {
push @matches, [$-[0], $+[0]];
}
- return esc_html(defined $chopped ? $chopped : $str) unless @matches;
+ return esc_html(defined $chopped ? $chopped : $str, %opts) unless @matches;
# filter matches so that we mark chopped string, if it is present
if (defined $chopped) {
@@ -1788,7 +1790,7 @@ sub esc_html_match_hl_chopped {
@matches = @filtered;
}
- return esc_html_hl_regions($str, 'match', @matches);
+ return esc_html_hl_regions($str, 'match', @matches, %opts);
}
## ----------------------------------------------------------------------
@@ -6070,15 +6072,7 @@ sub git_search_files {
print "<div class=\"binary\">Binary file</div>\n";
} else {
$ltext = untabify($ltext);
- if ($ltext =~ m/^(.*)($search_regexp)(.*)$/i) {
- $ltext = esc_html($1, -nbsp=>1);
- $ltext .= '<span class="match">';
- $ltext .= esc_html($2, -nbsp=>1);
- $ltext .= '</span>';
- $ltext .= esc_html($3, -nbsp=>1);
- } else {
- $ltext = esc_html($ltext, -nbsp=>1);
- }
+ $ltext = esc_html_match_hl($ltext, qr/$search_regexp/i, -nbsp=>1);
print "<div class=\"pre\">" .
$cgi->a({-href => $file_href.'#l'.$lno,
-class => "linenr"}, sprintf('%4i', $lno)) .
--
1.7.9
^ permalink raw reply related
* [PATCHv2 6/8] gitweb: Highlight matched part of project description when searching projects
From: Jakub Narebski @ 2012-02-15 20:38 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jakub Narebski
In-Reply-To: <1329338332-30358-1-git-send-email-jnareb@gmail.com>
Use esc_html_match_hl() from previous commit to mark match in the
_whole_ description when searching projects.
Currently, with this commit, when searching projects there is always
shown full description of a project, and not a shortened one (like for
ordinary projects list view), even if the match is on project name and
not project description.
Showing full description when there is match on it is useful to avoid
situation where match is in shortened, invisible part... well, perhaps
that could be solved (showing shortened description), but it would
require much more complicated code.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
The part about showing match using shortened description no longer
applies after the following patch... though it is an RFC for now,
that is why it is not mentioned in the commit message.
NOTE that we are actually always showing full description, so having
full description in "title" attribute to show on mouseover over
shortened title is no longer necessary, and should probably be
fixed... that is unless the next patch that highlights matches in
shortened description is accepted.
No changes from previous version (but see paragraph above about
possible changes).
gitweb/gitweb.perl | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 5596701..a109ebb 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5408,7 +5408,10 @@ sub git_project_list_rows {
"</td>\n" .
"<td>" . $cgi->a({-href => href(project=>$pr->{'path'}, action=>"summary"),
-class => "list", -title => $pr->{'descr_long'}},
- esc_html($pr->{'descr'})) . "</td>\n" .
+ $search_regexp
+ ? esc_html_match_hl($pr->{'descr_long'}, $search_regexp)
+ : esc_html($pr->{'descr'})) .
+ "</td>\n" .
"<td><i>" . chop_and_escape_str($pr->{'owner'}, 15) . "</i></td>\n";
print "<td class=\"". age_class($pr->{'age'}) . "\">" .
(defined $pr->{'age_string'} ? $pr->{'age_string'} : "No commits") . "</td>\n" .
--
1.7.9
^ permalink raw reply related
* [PATCH] git-send-email: allow overriding smtp-encryption config to 'none'
From: Brian Norris @ 2012-02-15 21:42 UTC (permalink / raw)
To: git; +Cc: Brian Norris
According to the manpage:
--smtp-encryption=<encryption>
Specify the encryption to use, either ssl or tls. Any other value
reverts to plain SMTP. Default is the value of
sendemail.smtpencryption.
However, if I have already set sendemail.smtpencryption but try to override
it with something like 'no', the authentication code block still tries to ask
for a password (and fails).
This patch forces $smtp_encryption to 'none' when a proper encryption type is
not provided, then checks $smtp_encryption before proceeding to authentication.
Example execution:
$ git send-email --smtp-encryption=no --smtp-domain=<xxx> \
--smtp-server=<xxx> --smtp-server-port=25 myfile.patch
Password:
Command unknown: 'AUTH' at /usr/local/libexec/git-core/git-send-email
line 1115, <STDIN> line 1.
Tested on Git 1.7.5.4 and 1.7.9.1.
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
git-send-email.perl | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index ef30c55..fa0a384 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -385,8 +385,9 @@ foreach my $setting (values %config_bool_settings) {
${$setting->[0]} = $setting->[1] unless (defined (${$setting->[0]}));
}
-# 'default' encryption is none -- this only prevents a warning
-$smtp_encryption = '' unless (defined $smtp_encryption);
+# 'default' encryption is none
+$smtp_encryption = 'none' unless (defined $smtp_encryption);
+$smtp_encryption = 'none' unless ($smtp_encryption eq 'tls' || $smtp_encryption eq 'ssl');
# Set CC suppressions
my(%suppress_cc);
@@ -1113,7 +1114,7 @@ X-Mailer: git-send-email $gitversion
defined $smtp_server_port ? " port=$smtp_server_port" : "";
}
- if (defined $smtp_authuser) {
+ if (defined $smtp_authuser && $smtp_encryption ne 'none') {
# Workaround AUTH PLAIN/LOGIN interaction defect
# with Authen::SASL::Cyrus
eval {
--
1.7.5.4
^ permalink raw reply related
* Re: [PATCH] git-send-email: allow overriding smtp-encryption config to 'none'
From: Jeff King @ 2012-02-15 22:06 UTC (permalink / raw)
To: Brian Norris; +Cc: git
In-Reply-To: <1329342178-14540-1-git-send-email-computersforpeace@gmail.com>
On Wed, Feb 15, 2012 at 01:42:58PM -0800, Brian Norris wrote:
> According to the manpage:
>
> --smtp-encryption=<encryption>
> Specify the encryption to use, either ssl or tls. Any other value
> reverts to plain SMTP. Default is the value of
> sendemail.smtpencryption.
>
> However, if I have already set sendemail.smtpencryption but try to override
> it with something like 'no', the authentication code block still tries to ask
> for a password (and fails).
Sounds reasonable.
> This patch forces $smtp_encryption to 'none' when a proper encryption type is
> not provided, then checks $smtp_encryption before proceeding to authentication.
Defaulting everything except "ssl" or "tls" to "none" seems risky to me.
If I am understanding your patch correctly, then doing this:
git send-email --smtp-encryption=SSL
will silently treat that as "don't do encryption", which could have
surprising security implications for the user. I chose all-caps as it is
an obvious mistake to make. We probably should treat it the same as
lowercase "ssl", but the same argument applies to other typos like
"tsl".
It seems like a much safer default would be to die() on an invalid
encryption specifier.
-Peff
^ permalink raw reply
* Re: [PATCH v2 2/2] submodules: always use a relative path from gitdir to work tree
From: Jens Lehmann @ 2012-02-15 22:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, Antony Male, Phil Hord
In-Reply-To: <7vzkcl5f37.fsf@alter.siamese.dyndns.org>
Am 14.02.2012 21:34, schrieb Junio C Hamano:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Jens Lehmann <Jens.Lehmann@web.de> writes:
>>
>>> After adding a comment, using test instead of [], testing both $a and
>>> $b and assigning each variable on it's own line I get the following
>>> interdiff. Does that make more sense?
>>
>> My earlier request for comment was to say
>>
>> # $a is always longer than $b for such and such reasons
>>
>> to explain why testing $b without testing $a was sufficient.
>
> Heh, after I follow the entire module_clone, $gitdir is defined in earlier
> parts of the function to be "$(rev-parse --git-dir)/modules/$path", so it
> is clear that it is longer than $path.
Unfortunately only by accident. The usage of $path is not correct here,
$name should be used instead (I have a patch in the making to correct that,
but as that hits the same code area as these fixes I'll post that later
together with some tests moving submodules around inside a superproject).
Then the result of "$(rev-parse --git-dir)/modules/$name" can be shorter
than "$path" when a submodule is renamed into a higher directory level.
> Unless "cd $there && pwd" does not
> result in a funny situation (such as $something/modules is a symbolic link
> to another place that is much closer to the root of the filesystem), that
> is.
>
> And in such a case, the prefix part of $a and $b would be different from
> the very beginning hopefully.
Yes, they should differ somewhere in any sane setup I can imagine.
>> It is obvious (at least to me) that the loop continues as long as $a and
>> $b begin with the same string before their first '/' and removes that
>> common segment from both of them, so I do not think the new comment is
>> absolutely necessary, but it would not hurt to have it, especially it is
>> short enough and to the point.
>>
>> Thanks.
>>
>>> diff --git a/git-submodule.sh b/git-submodule.sh
>>> index 3463d6d..ed76ce2 100755
>>> --- a/git-submodule.sh
>>> +++ b/git-submodule.sh
>>> @@ -172,9 +172,11 @@ module_clone()
>>>
>>> a=$(cd "$gitdir" && pwd)
>>> b=$(cd "$path" && pwd)
>>> - while [ "$b" ] && [ "${a%%/*}" = "${b%%/*}" ]
>>> + # Remove all common leading directories
>>> + while test -n "$a" && test -n "$b" && test "${a%%/*}" = "${b%%/*}"
>>> do
>>> - a=${a#*/} b=${b#*/};
>>> + a=${a#*/}
>>> + b=${b#*/}
>>> done
>>> rel=$(echo $a | sed -e 's|[^/]*|..|g')
>
> Perhaps aseert that $a never becomes empty before this line (or set it
> explicitly to "." when $a is empty), as otherwise
>
>>> (clear_local_git_env; cd "$path" && git config core.worktree "$rel/$b")
>
> this will refer to "/$b" from the root?
I think neither $a nor $b should be empty after that. But thinking deeper
about that while loop I suspect the real problem here is doing "a=${a#*/}"
or "b=${b#*/}" on a string that doesn't contain a slash anymore. We'll
happily remove a leading directory on the other path while the one without
slash will stay untouched, leading to a bogus result which is off by one
directory level.
AFAICS that will only happen when one path is a prefix of the other, which
is a pretty pathological case. So I'll whip up a new version asserting
that beforehand and dropping the -n test in the while loop, ok?
^ permalink raw reply
* Re: [PATCH v5 3/3] push: teach --recurse-submodules the on-demand option
From: Jens Lehmann @ 2012-02-15 22:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Heiko Voigt, git, Fredrik Gustafsson
In-Reply-To: <7v7gzq9jg2.fsf@alter.siamese.dyndns.org>
Am 14.02.2012 04:34, schrieb Junio C Hamano:
> Heiko Voigt <hvoigt@hvoigt.net> writes:
>
>> diff --git a/submodule.c b/submodule.c
>> index 3c714c2..ff0cfd8 100644
>> --- a/submodule.c
>> +++ b/submodule.c
>> @@ -411,6 +411,54 @@ int check_submodule_needs_pushing(unsigned char new_sha1[20],
>> return needs_pushing->nr;
>> }
>>
>> +static int push_submodule(const char *path)
>> +{
>> + if (add_submodule_odb(path))
>> + return 1;
>> +
>> + if (for_each_remote_ref_submodule(path, has_remote, NULL) > 0) {
>> + struct child_process cp;
>> + const char *argv[] = {"push", NULL};
>> +
>> + memset(&cp, 0, sizeof(cp));
>> + cp.argv = argv;
>> + cp.env = local_repo_env;
>> + cp.git_cmd = 1;
>> + cp.no_stdin = 1;
>> + cp.dir = path;
>> + if (run_command(&cp))
>> + return 0;
>> + close(cp.out);
>> + }
>> +
>> + return 1;
>> +}
>
> Hmm, this makes me wonder if we fire subprocesses and have them run in
> parallel (to a reasonably limited parallelism), it might make the overall
> user experience more pleasant, and if we did the same on the fetching
> side, it would be even nicer.
Yeah, I had the same idea and did some experiments when working on
fetch some time ago.
> We would need to keep track of children and after firing a handful of them
> we would need to start waiting for some to finish and collect their exit
> status before firing more, and at the end we would need to wait for the
> remaining ones and find how each one of them did before returning from
> push_unpushed_submodules(). If we were to do so, what are the missing
> support we would need from the run_command() subsystem?
We would not only have to collect the exit status but also the output
lines. You don't want to see the output of multiple fetches or pushes
mixed together, so it makes sense to just defer that until the command
exited and then print everything at once. The interesting part I couldn't
come up with an easy solution for is to preserve the output order between
the stdout and stdin lines, as they contain different parts of the
progress which would look strange when shuffled around.
And I saw that sometimes parallel fetches took way longer than doing them
sequentially (in my case because of strange DNS behavior of my DSL router),
so we would definitely want a config option for that (maybe setting the
maximum number of simultaneous threads to be used).
But don't get me wrong, I'm all for having that feature! :-)
^ permalink raw reply
* Re: [PATCH] git-latexdiff: new command in contrib, to use latexdiff and Git
From: Tim Haga @ 2012-02-15 23:33 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git, gitster
In-Reply-To: <1329320987-15203-1-git-send-email-Matthieu.Moy@imag.fr>
While testing your script on my office machine i discovered that the
following might be a problem:
> +if [ "$view" = 1 ] || [ "$view" = maybe ] && [ "$output" = "" ]; then
> + xpdf "$pdffile"
> +fi
Xpdf is not installed on all machines (e.g. it's not installed on my
office machine), so maybe it would be a good idea to use a environment
variable instead?
Tim
^ permalink raw reply
* Re: [PATCH] git-send-email: allow overriding smtp-encryption config to 'none'
From: Brian Norris @ 2012-02-15 23:49 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20120215220629.GA17672@sigill.intra.peff.net>
On Wed, Feb 15, 2012 at 2:06 PM, Jeff King <peff@peff.net> wrote:
> On Wed, Feb 15, 2012 at 01:42:58PM -0800, Brian Norris wrote:
>
>> According to the manpage:
>>
>> --smtp-encryption=<encryption>
>> Specify the encryption to use, either ssl or tls. Any other value
>> reverts to plain SMTP. Default is the value of
>> sendemail.smtpencryption.
>>
>> However, if I have already set sendemail.smtpencryption but try to override
>> it with something like 'no', the authentication code block still tries to ask
>> for a password (and fails).
>
> Sounds reasonable.
An error like
Command unknown: 'AUTH' at /usr/local/libexec/git-core/git-send-email
is reasonable?
>> This patch forces $smtp_encryption to 'none' when a proper encryption type is
>> not provided, then checks $smtp_encryption before proceeding to authentication.
>
> Defaulting everything except "ssl" or "tls" to "none" seems risky to me.
> If I am understanding your patch correctly, then doing this:
>
> git send-email --smtp-encryption=SSL
>
> will silently treat that as "don't do encryption", which could have
> surprising security implications for the user. I chose all-caps as it is
> an obvious mistake to make. We probably should treat it the same as
> lowercase "ssl", but the same argument applies to other typos like
> "tsl".
Well, git-send-email already doesn't handle typos or capitalization
correctly, AFAICT. So nothing new here.
> It seems like a much safer default would be to die() on an invalid
> encryption specifier.
Fine. But then we need to define a behavior that means 'no
encryption.' Like 'none' instead of just saying 'anything but tls or
ssl.'
Now that I look at this again, I think part of the issue I have is
that there is no way to override *smtp-user* via command-line, in
order to do unencrypted, unauthenticated email. So the
*authentication* not the encryption is really my main problem...I'll
take another look and try a new patch.
Thanks,
Brian
^ permalink raw reply
* diff -G with case insensitivity
From: Chris Leong @ 2012-02-16 0:18 UTC (permalink / raw)
To: git
Is there any way to run diff -G with a case insensitivity flag?
^ permalink raw reply
* Re: diff -G with case insensitivity
From: Chris Leong @ 2012-02-16 0:33 UTC (permalink / raw)
To: git
In-Reply-To: <CAJ6vYjejtZkupy750rvz6HW_0SNPyBVTa78DO4nY8Bi368neQw@mail.gmail.com>
Also, is there a method to restrict it to only showing the chunks that
match - rather than files?
On Thu, Feb 16, 2012 at 11:18 AM, Chris Leong <walkraft@gmail.com> wrote:
> Is there any way to run diff -G with a case insensitivity flag?
^ permalink raw reply
* Re: [PATCH] git-send-email: allow overriding smtp-encryption config to 'none'
From: Jeff King @ 2012-02-16 0:49 UTC (permalink / raw)
To: Brian Norris; +Cc: git
In-Reply-To: <CAN8TOE_BnkOcMQRTY-GWrHozYD0+0giWn2LtjB8AVnP_DzA+Sg@mail.gmail.com>
On Wed, Feb 15, 2012 at 03:49:59PM -0800, Brian Norris wrote:
> > Sounds reasonable.
>
> An error like
> Command unknown: 'AUTH' at /usr/local/libexec/git-core/git-send-email
> is reasonable?
Sorry, no, I meant your goal of allowing overriding config sounds like a
reasonable thing to want. But from reading your message below, it seems
that is not actually the problem you are trying to solve.
> > Defaulting everything except "ssl" or "tls" to "none" seems risky to me.
> > If I am understanding your patch correctly, then doing this:
> >
> > git send-email --smtp-encryption=SSL
> >
> > will silently treat that as "don't do encryption", which could have
> > surprising security implications for the user. I chose all-caps as it is
> > an obvious mistake to make. We probably should treat it the same as
> > lowercase "ssl", but the same argument applies to other typos like
> > "tsl".
>
> Well, git-send-email already doesn't handle typos or capitalization
> correctly, AFAICT. So nothing new here.
Hmm. From your description and the patch, I thought that was something
introduced by your patch. But looking at the existing code, it seems
like that is already the case. IOW, I don't understand why
"--smtp-encryption=none" does not already work looking at the current
code.
So being more careful about typos is an improvement we could make, but
it is not a feature that would need to be part of a bugfix patch.
> > It seems like a much safer default would be to die() on an invalid
> > encryption specifier.
>
> Fine. But then we need to define a behavior that means 'no
> encryption.' Like 'none' instead of just saying 'anything but tls or
> ssl.'
Right. I meant that you should introduce "none" as an explicit "no, I
don't want this" and die when the flag is not one of {ssl, tls, none}.
> Now that I look at this again, I think part of the issue I have is
> that there is no way to override *smtp-user* via command-line, in
> order to do unencrypted, unauthenticated email. So the
> *authentication* not the encryption is really my main problem...I'll
> take another look and try a new patch.
Ah, I see. I misunderstood the original problem you were trying to solve
(I thought your example was "see? Encryption is off, so the server won't
do AUTH, demonstrating that the patch works.").
Overriding the smtp user from the config is a separate issue, and I
don't think that is currently possible. The usual way to spell an option
like that in git is "--no-smtp-user", but it seems that we use perl's
GetOptions, which does not understand that syntax. So you'd have to add
a "--no-smtp-user" by hand.
-Peff
^ permalink raw reply
* importance of .git extension for bare repos
From: Neal Kreitzinger @ 2012-02-16 1:04 UTC (permalink / raw)
To: git
I have a user who has created bare repos without naming them with the .git
extension. As a result, the file paths are MYBARE/config instead of
MYBARE.git/config, etc. Is this a supported naming convention for bare
repos in git.git, or is it going to have some problems?
v/r,
neal
^ permalink raw reply
* Re: importance of .git extension for bare repos
From: Tay Ray Chuan @ 2012-02-16 1:38 UTC (permalink / raw)
To: Neal Kreitzinger; +Cc: git
In-Reply-To: <jhhkn8$a0t$1@dough.gmane.org>
I don't believe its significant to git itself, it's just a convention
for us humans.
--
Cheers,
Ray Chuan
On Thu, Feb 16, 2012 at 9:04 AM, Neal Kreitzinger <neal@rsss.com> wrote:
> I have a user who has created bare repos without naming them with the .git
> extension. As a result, the file paths are MYBARE/config instead of
> MYBARE.git/config, etc. Is this a supported naming convention for bare
> repos in git.git, or is it going to have some problems?
>
> v/r,
> neal
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: importance of .git extension for bare repos
From: Jeff King @ 2012-02-16 2:30 UTC (permalink / raw)
To: Tay Ray Chuan; +Cc: Neal Kreitzinger, git
In-Reply-To: <CALUzUxqEirjnCBHt28dwcgTBqC+u0wydNsnVR1rgyGG0-R_M1w@mail.gmail.com>
On Thu, Feb 16, 2012 at 09:38:08AM +0800, Tay Ray Chuan wrote:
> I don't believe its significant to git itself, it's just a convention
> for us humans.
That's not entirely true. If you try to access a repository by name
(e.g., "git clone foo", "git fetch foo"), git will look for "foo.git".
This magic lookup works for local repositories, and for remote
repositories served over ssh or by git-daemon. I think it should also
work for smart-http, but I didn't check. It doesn't work for dumb http
(because it would involve making several exploratory http requests).
-Peff
^ permalink raw reply
* Re: git-subtree Ready #2
From: David A. Greene @ 2012-02-16 4:07 UTC (permalink / raw)
To: Jeff King; +Cc: git, Avery Pennarun
In-Reply-To: <87sjicpsr1.fsf@smith.obbligato.org>
greened@obbligato.org (David A. Greene) writes:
>> But more important than the physical layout is the maintenance plan
>> going forward. Is Avery going to keep maintaining git-subtree, and we
>> will just occasionally pull? Are you maintaining it? Where will patches
>> go? To a github repo? To git@vger?
>
> I am still waiting to hear from Avery on that. I will ping him again.
> My intention is to certainly participate in maintenance. I use
> git-subtree daily so it's in my interest to keep it working.
I've attached Avery's response below. The short summary is that he
thinks maintaining it in the vger git repository is the way to go and
that he's fine moving patches to/from GitHub as necessary.
So again, I will certainly be part of the maintenance team. There are a
few other people currently helping out with maintenance and I will help
Avery to get the word out about the switch as he requires.
-Dave
--8<-----------------------------------------------------------------------
Thanks for putting work into this. You can feel free to cc: me on any
git-list discussions if you want.
I haven't done any significant amount of git-subtree maintenance lately,
but even if I did, since it's only one file it should be easy to put
move patches between github and git whenever we want. I would suggest
that just maintaining it as part of git is the best way to go, since
having diverging versions doesn't really help anyone.
I'm sure the potential benefit of putting git-subtree in the contrib/
directory is that we could then use git-subtree to maintain the
git-subtree git subtree, which is a fun wordplay, but perhaps
ironically, as a single rarely-changing file, git-subtree is probably
not the right tool for these purposes :)
Have fun,
Avery
^ permalink raw reply
* Re: [PATCH/RFC] Document format of basic Git objects
From: Junio C Hamano @ 2012-02-16 7:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nguyễn Thái Ngọc Duy, git
In-Reply-To: <7v4nur2806.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
>> +- POSIX file mode encoded in octal ascii
>
> Add ", no 0 padding to the right" at the end, as I heard that every
> imitation of Git gets this wrong in its first version.
Ehh, of course no 0 padding on the LEFT hand side.
Rice-bowl with left hand, chopsticks with right hand. I always mix these
up. Sorry.
^ permalink raw reply
* [PATCH 0/8] config-include fixes
From: Jeff King @ 2012-02-16 8:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20120214220953.GC24802@sigill.intra.peff.net>
On Tue, Feb 14, 2012 at 05:09:53PM -0500, Jeff King wrote:
> > Hmm, I thought t1305 covered "config --list", and ... oops, it makes sure
> > the output contains the inclusion.
>
> Yes. It should include it (and does correctly) when not using any
> per-file options, but does not correctly turn it off for the per-file
> case (because we bail to regular git_config instead of custom lookup
> code).
>
> > > Do you want to revert and re-do to make master pretty, or should I just
> > > build on top?
> >
> > Do you mean 'next'?
>
> I meant "revert from next and re-reroll, so that when the re-roll gets
> merged to master, the result there will look pretty".
The fix for this is pretty straightforward. However, I took the
opportunity to do some cleanup and fix some minor bugs while I was in
the area, and ended up with quite a big series.
I prepared this on top of what you have queued in jk/config-include.
However, all of the cleanup is semantically independent of the topic
(though there are a few minor textual conflicts). If I were re-rolling,
I would put it all at the front, then squash patch 8 into my prior
"implement config includes" patch.
The patches are:
[1/8]: t1300: add missing &&-chaining
Obvious cleanup.
[2/8]: config: copy the return value of prefix_filename
Very minor bug.
[3/8]: config: teach git_config_set_multivar_in_file a default path
[4/8]: config: teach git_config_rename_section a file argument
[5/8]: config: provide a version of git_config with more options
[6/8]: config: stop using config_exclusive_filename
[7/8]: config: eliminate config_exclusive_filename
This is all cleanup which makes config_exclusive_filename go away. It's
not strictly necessary for this series, but it's something I've been
wanting to clean up for a while. And it does fix a few minor bugs (see
patch 6/8). And the refactoring in 5/8 lays the groundwork for 8/8.
[8/8]: config: do not respect includes for single-file --list
The actual fix for the regression in my config-include patch.
-Peff
^ permalink raw reply
* [PATCH 1/8] t1300: add missing &&-chaining
From: Jeff King @ 2012-02-16 8:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20120216080102.GA11793@sigill.intra.peff.net>
Signed-off-by: Jeff King <peff@peff.net>
---
t/t1300-repo-config.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 0690e0e..6de46bb 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -451,7 +451,7 @@ test_expect_success 'refer config from subdirectory' '
mkdir x &&
(
cd x &&
- echo strasse >expect
+ echo strasse >expect &&
git config --get --file ../other-config ein.bahn >actual &&
test_cmp expect actual
)
--
1.7.9.1.4.g8ffed
^ permalink raw reply related
* [PATCH 2/8] config: copy the return value of prefix_filename
From: Jeff King @ 2012-02-16 8:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20120216080102.GA11793@sigill.intra.peff.net>
The prefix_filename function returns a pointer to a static
buffer which may be overwritten by subsequent calls. Since
we are going to keep the result around for a while, let's be
sure to duplicate it for safety.
I don't think this can be triggered as a bug in the current
code, but it's a good idea to be defensive, as any resulting
bug would be quite subtle.
Signed-off-by: Jeff King <peff@peff.net>
---
I was tempted to simply use OPT_FILENAME to get the argument to "-f",
which would handle the prefixing for us. However, we can also get the
value from $GIT_CONFIG. That value doesn't actually run through this
code currently, but I think it should (and I fix this in patch 6).
builtin/config.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 09bf778..5a43a3c 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -391,9 +391,10 @@ int cmd_config(int argc, const char **argv, const char *prefix)
config_exclusive_filename = git_pathdup("config");
else if (given_config_file) {
if (!is_absolute_path(given_config_file) && prefix)
- config_exclusive_filename = prefix_filename(prefix,
- strlen(prefix),
- given_config_file);
+ config_exclusive_filename =
+ xstrdup(prefix_filename(prefix,
+ strlen(prefix),
+ given_config_file));
else
config_exclusive_filename = given_config_file;
}
--
1.7.9.1.4.g8ffed
^ permalink raw reply related
* [PATCH 3/8] config: teach git_config_set_multivar_in_file a default path
From: Jeff King @ 2012-02-16 8:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20120216080102.GA11793@sigill.intra.peff.net>
The git_config_set_multivar_in_file function takes a
filename argument to specify the file into which the values
should be written. Currently, this value must be non-NULL.
Callers which want to write to the default location must use
the regular, non-"in_file" version, which will either write
to config_exclusive_filename, or to the repo config if the
exclusive filename is NULL.
Let's migrate the "default to using repo config" logic into
the "in_file" form. That will let callers get the same
default-if-NULL behavior as one gets with
config_exclusive_filename, but without having to use the
global variable.
Signed-off-by: Jeff King <peff@peff.net>
---
config.c | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/config.c b/config.c
index e3fcf75..a9eec58 100644
--- a/config.c
+++ b/config.c
@@ -1300,6 +1300,7 @@ int git_config_set_multivar_in_file(const char *config_filename,
int fd = -1, in_fd;
int ret;
struct lock_file *lock = NULL;
+ char *filename_buf = NULL;
/* parse-key returns negative; flip the sign to feed exit(3) */
ret = 0 - git_config_parse_key(key, &store.key, &store.baselen);
@@ -1308,6 +1309,8 @@ int git_config_set_multivar_in_file(const char *config_filename,
store.multi_replace = multi_replace;
+ if (!config_filename)
+ config_filename = filename_buf = git_pathdup("config");
/*
* The lock serves a purpose in addition to locking: the new
@@ -1477,6 +1480,7 @@ int git_config_set_multivar_in_file(const char *config_filename,
out_free:
if (lock)
rollback_lock_file(lock);
+ free(filename_buf);
return ret;
write_err_out:
@@ -1488,19 +1492,9 @@ write_err_out:
int git_config_set_multivar(const char *key, const char *value,
const char *value_regex, int multi_replace)
{
- const char *config_filename;
- char *buf = NULL;
- int ret;
-
- if (config_exclusive_filename)
- config_filename = config_exclusive_filename;
- else
- config_filename = buf = git_pathdup("config");
-
- ret = git_config_set_multivar_in_file(config_filename, key, value,
- value_regex, multi_replace);
- free(buf);
- return ret;
+ return git_config_set_multivar_in_file(config_exclusive_filename,
+ key, value, value_regex,
+ multi_replace);
}
static int section_name_match (const char *buf, const char *name)
--
1.7.9.1.4.g8ffed
^ permalink raw reply related
* [PATCH 4/8] config: teach git_config_rename_section a file argument
From: Jeff King @ 2012-02-16 8:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20120216080102.GA11793@sigill.intra.peff.net>
The other config-writing functions (git_config_set and
git_config_set_multivar) each have an -"in_file" version to
write a specific file. Let's add one for rename_section,
with the eventual goal of moving away from the magic
config_exclusive_filename global.
Signed-off-by: Jeff King <peff@peff.net>
---
cache.h | 1 +
config.c | 20 +++++++++++++-------
2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/cache.h b/cache.h
index 94f3eaf..7e375ce 100644
--- a/cache.h
+++ b/cache.h
@@ -1130,6 +1130,7 @@ extern int git_config_parse_key(const char *, char **, int *);
extern int git_config_set_multivar(const char *, const char *, const char *, int);
extern int git_config_set_multivar_in_file(const char *, const char *, const char *, const char *, int);
extern int git_config_rename_section(const char *, const char *);
+extern int git_config_rename_section_in_file(const char *, const char *, const char *);
extern const char *git_etc_gitconfig(void);
extern int check_repository_format_version(const char *var, const char *value, void *cb);
extern int git_env_bool(const char *, int);
diff --git a/config.c b/config.c
index a9eec58..c456600 100644
--- a/config.c
+++ b/config.c
@@ -1537,19 +1537,19 @@ static int section_name_match (const char *buf, const char *name)
}
/* if new_name == NULL, the section is removed instead */
-int git_config_rename_section(const char *old_name, const char *new_name)
+int git_config_rename_section_in_file(const char *config_filename,
+ const char *old_name, const char *new_name)
{
int ret = 0, remove = 0;
- char *config_filename;
+ char *filename_buf = NULL;
struct lock_file *lock = xcalloc(sizeof(struct lock_file), 1);
int out_fd;
char buf[1024];
FILE *config_file;
- if (config_exclusive_filename)
- config_filename = xstrdup(config_exclusive_filename);
- else
- config_filename = git_pathdup("config");
+ if (!config_filename)
+ config_filename = filename_buf = git_pathdup("config");
+
out_fd = hold_lock_file_for_update(lock, config_filename, 0);
if (out_fd < 0) {
ret = error("could not lock config file %s", config_filename);
@@ -1613,10 +1613,16 @@ unlock_and_out:
if (commit_lock_file(lock) < 0)
ret = error("could not commit config file %s", config_filename);
out:
- free(config_filename);
+ free(filename_buf);
return ret;
}
+int git_config_rename_section(const char *old_name, const char *new_name)
+{
+ return git_config_rename_section_in_file(config_exclusive_filename,
+ old_name, new_name);
+}
+
/*
* Call this to report error for your variable that should not
* get a boolean value (i.e. "[my] var" means "true").
--
1.7.9.1.4.g8ffed
^ permalink raw reply related
* [PATCH 5/8] config: provide a version of git_config with more options
From: Jeff King @ 2012-02-16 8:05 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20120216080102.GA11793@sigill.intra.peff.net>
Callers may want to provide a specific version of a file in
which to look for config. Right now this can be done by
setting the magic global config_exclusive_filename variable.
By providing a version of git_config that takes a filename,
we can take a step towards making this magic global go away.
Furthermore, by providing a more "advanced" interface, we
now have a a natural place to add new options for callers
like git-config, which care about tweaking the specifics of
config lookup, without disturbing the large number of
"simple" users (i.e., every other part of git).
The astute reader of this patch may notice that the logic
for handling config_exclusive_filename was taken out of
git_config_early, but added into git_config. This means that
git_config_early will no longer respect config_exclusive_filename.
That's OK, because the only other caller of git_config_early
is check_repository_format_gently, but the only function
which sets config_exclusive_filename is cmd_config, which
does not call check_repository_format_gently (and if it did,
it would have been a bug, anyway, as we would be checking
the repository format in the wrong file).
Signed-off-by: Jeff King <peff@peff.net>
---
Obviously this could also learn about a respect_includes flag, and it in
fact does in patch 8. I didn't include it in the initial version because
this part of the series is really about refactoring the
config_exclusive_filename stuff (and is semantically independent of the
notion of included files).
cache.h | 1 +
config.c | 22 +++++++++++++++++-----
2 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/cache.h b/cache.h
index 7e375ce..7cb8874 100644
--- a/cache.h
+++ b/cache.h
@@ -1115,6 +1115,7 @@ extern int git_config_from_file(config_fn_t fn, const char *, void *);
extern void git_config_push_parameter(const char *text);
extern int git_config_from_parameters(config_fn_t fn, void *data);
extern int git_config(config_fn_t fn, void *);
+extern int git_config_with_options(config_fn_t fn, void *, const char *filename);
extern int git_config_early(config_fn_t fn, void *, const char *repo_config);
extern int git_parse_ulong(const char *, unsigned long *);
extern int git_config_int(const char *, const char *);
diff --git a/config.c b/config.c
index c456600..fbf883d 100644
--- a/config.c
+++ b/config.c
@@ -942,9 +942,6 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
int ret = 0, found = 0;
const char *home = NULL;
- /* Setting $GIT_CONFIG makes git read _only_ the given config file. */
- if (config_exclusive_filename)
- return git_config_from_file(fn, config_exclusive_filename, data);
if (git_config_system() && !access(git_etc_gitconfig(), R_OK)) {
ret += git_config_from_file(fn, git_etc_gitconfig(),
data);
@@ -980,7 +977,8 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
return ret == 0 ? found : ret;
}
-int git_config(config_fn_t fn, void *data)
+int git_config_with_options(config_fn_t fn, void *data,
+ const char *filename)
{
char *repo_config = NULL;
int ret;
@@ -988,14 +986,28 @@ int git_config(config_fn_t fn, void *data)
inc.fn = fn;
inc.data = data;
+ fn = git_config_include;
+ data = &inc;
+
+ /*
+ * If we have a specific filename, use it. Otherwise, follow the
+ * regular lookup sequence.
+ */
+ if (filename)
+ return git_config_from_file(fn, filename, data);
repo_config = git_pathdup("config");
- ret = git_config_early(git_config_include, &inc, repo_config);
+ ret = git_config_early(fn, data, repo_config);
if (repo_config)
free(repo_config);
return ret;
}
+int git_config(config_fn_t fn, void *data)
+{
+ return git_config_with_options(fn, data, config_exclusive_filename);
+}
+
/*
* Find all the stuff for git_config_set() below.
*/
--
1.7.9.1.4.g8ffed
^ permalink raw reply related
* [PATCH 6/8] config: stop using config_exclusive_filename
From: Jeff King @ 2012-02-16 8:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20120216080102.GA11793@sigill.intra.peff.net>
The git-config command sometimes operates on the default set
of config files (either reading from all, or writing to repo
config), and sometimes operates on a specific file. In the
latter case, we set the magic global config_exclusive_filename,
and the code in config.c does the right thing.
Instead, let's have git-config use the "advanced" variants
of config.c's functions which let it specify an individual
filename (or NULL for the default). This makes the code a
lot more obvious, and fixes two small bugs:
1. A relative path specified by GIT_CONFIG=foo will look
in the wrong directory if we have to chdir as part of
repository setup. We already handle this properly for
"git config -f foo", but the GIT_CONFIG lookup used
config_exclusive_filename directly. By dropping to a
single magic variable, the GIT_CONFIG case now just
works.
2. Calling "git config -f foo --edit" would not respect
core.editor. This is because just before editing, we
called git_config, which would respect the
config_exclusive_filename setting, even though this
particular git_config call was not about looking in the
user's specified file, but rather about loading actual
git config, just as any other git program would.
Signed-off-by: Jeff King <peff@peff.net>
---
The diffstat is slightly disappointing. It really is a one-for-one
conversion in almost every case, but the functions and variable names
are so bloody long that I had to wrap the calls to stay under 80
characters.
builtin/config.c | 61 ++++++++++++++++++++++++++++-------------------
t/t1300-repo-config.sh | 25 +++++++++++++++++++
2 files changed, 61 insertions(+), 25 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 5a43a3c..8901dd9 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -167,7 +167,7 @@ static int get_value(const char *key_, const char *regex_)
config_fn_t fn;
void *data;
- local = config_exclusive_filename;
+ local = given_config_file;
if (!local) {
const char *home = getenv("HOME");
local = repo_config = git_pathdup("config");
@@ -315,7 +315,8 @@ static void get_color(const char *def_color)
{
get_color_found = 0;
parsed_color[0] = '\0';
- git_config(git_get_color_config, NULL);
+ git_config_with_options(git_get_color_config, NULL,
+ given_config_file);
if (!get_color_found && def_color)
color_parse(def_color, "command line", parsed_color);
@@ -342,7 +343,8 @@ static int get_colorbool(int print)
{
get_colorbool_found = -1;
get_diff_color_found = -1;
- git_config(git_get_colorbool_config, NULL);
+ git_config_with_options(git_get_colorbool_config, NULL,
+ given_config_file);
if (get_colorbool_found < 0) {
if (!strcmp(get_colorbool_slot, "color.diff"))
@@ -365,7 +367,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
int nongit = !startup_info->have_repository;
char *value;
- config_exclusive_filename = getenv(CONFIG_ENVIRONMENT);
+ given_config_file = getenv(CONFIG_ENVIRONMENT);
argc = parse_options(argc, argv, prefix, builtin_config_options,
builtin_config_usage,
@@ -380,27 +382,27 @@ int cmd_config(int argc, const char **argv, const char *prefix)
char *home = getenv("HOME");
if (home) {
char *user_config = xstrdup(mkpath("%s/.gitconfig", home));
- config_exclusive_filename = user_config;
+ given_config_file = user_config;
} else {
die("$HOME not set");
}
}
else if (use_system_config)
- config_exclusive_filename = git_etc_gitconfig();
+ given_config_file = git_etc_gitconfig();
else if (use_local_config)
- config_exclusive_filename = git_pathdup("config");
+ given_config_file = git_pathdup("config");
else if (given_config_file) {
if (!is_absolute_path(given_config_file) && prefix)
- config_exclusive_filename =
+ given_config_file =
xstrdup(prefix_filename(prefix,
strlen(prefix),
given_config_file));
else
- config_exclusive_filename = given_config_file;
+ given_config_file = given_config_file;
}
if (respect_includes == -1)
- respect_includes = !config_exclusive_filename;
+ respect_includes = !given_config_file;
if (end_null) {
term = '\0';
@@ -438,28 +440,29 @@ int cmd_config(int argc, const char **argv, const char *prefix)
if (actions == ACTION_LIST) {
check_argc(argc, 0, 0);
- if (git_config(show_all_config, NULL) < 0) {
- if (config_exclusive_filename)
+ if (git_config_with_options(show_all_config, NULL,
+ given_config_file) < 0) {
+ if (given_config_file)
die_errno("unable to read config file '%s'",
- config_exclusive_filename);
+ given_config_file);
else
die("error processing config file(s)");
}
}
else if (actions == ACTION_EDIT) {
check_argc(argc, 0, 0);
- if (!config_exclusive_filename && nongit)
+ if (!given_config_file && nongit)
die("not in a git directory");
git_config(git_default_config, NULL);
- launch_editor(config_exclusive_filename ?
- config_exclusive_filename : git_path("config"),
+ launch_editor(given_config_file ?
+ given_config_file : git_path("config"),
NULL, NULL);
}
else if (actions == ACTION_SET) {
int ret;
check_argc(argc, 2, 2);
value = normalize_value(argv[0], argv[1]);
- ret = git_config_set(argv[0], value);
+ ret = git_config_set_in_file(given_config_file, argv[0], value);
if (ret == CONFIG_NOTHING_SET)
error("cannot overwrite multiple values with a single value\n"
" Use a regexp, --add or --replace-all to change %s.", argv[0]);
@@ -468,17 +471,20 @@ int cmd_config(int argc, const char **argv, const char *prefix)
else if (actions == ACTION_SET_ALL) {
check_argc(argc, 2, 3);
value = normalize_value(argv[0], argv[1]);
- return git_config_set_multivar(argv[0], value, argv[2], 0);
+ return git_config_set_multivar_in_file(given_config_file,
+ argv[0], value, argv[2], 0);
}
else if (actions == ACTION_ADD) {
check_argc(argc, 2, 2);
value = normalize_value(argv[0], argv[1]);
- return git_config_set_multivar(argv[0], value, "^$", 0);
+ return git_config_set_multivar_in_file(given_config_file,
+ argv[0], value, "^$", 0);
}
else if (actions == ACTION_REPLACE_ALL) {
check_argc(argc, 2, 3);
value = normalize_value(argv[0], argv[1]);
- return git_config_set_multivar(argv[0], value, argv[2], 1);
+ return git_config_set_multivar_in_file(given_config_file,
+ argv[0], value, argv[2], 1);
}
else if (actions == ACTION_GET) {
check_argc(argc, 1, 2);
@@ -499,18 +505,22 @@ int cmd_config(int argc, const char **argv, const char *prefix)
else if (actions == ACTION_UNSET) {
check_argc(argc, 1, 2);
if (argc == 2)
- return git_config_set_multivar(argv[0], NULL, argv[1], 0);
+ return git_config_set_multivar_in_file(given_config_file,
+ argv[0], NULL, argv[1], 0);
else
- return git_config_set(argv[0], NULL);
+ return git_config_set_in_file(given_config_file,
+ argv[0], NULL);
}
else if (actions == ACTION_UNSET_ALL) {
check_argc(argc, 1, 2);
- return git_config_set_multivar(argv[0], NULL, argv[1], 1);
+ return git_config_set_multivar_in_file(given_config_file,
+ argv[0], NULL, argv[1], 1);
}
else if (actions == ACTION_RENAME_SECTION) {
int ret;
check_argc(argc, 2, 2);
- ret = git_config_rename_section(argv[0], argv[1]);
+ ret = git_config_rename_section_in_file(given_config_file,
+ argv[0], argv[1]);
if (ret < 0)
return ret;
if (ret == 0)
@@ -519,7 +529,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
else if (actions == ACTION_REMOVE_SECTION) {
int ret;
check_argc(argc, 1, 1);
- ret = git_config_rename_section(argv[0], NULL);
+ ret = git_config_rename_section_in_file(given_config_file,
+ argv[0], NULL);
if (ret < 0)
return ret;
if (ret == 0)
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 6de46bb..5f249f6 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -458,6 +458,14 @@ test_expect_success 'refer config from subdirectory' '
'
+test_expect_success 'refer config from subdirectory via GIT_CONFIG' '
+ (
+ cd x &&
+ GIT_CONFIG=../other-config git config --get ein.bahn >actual &&
+ test_cmp expect actual
+ )
+'
+
cat > expect << EOF
[ein]
bahn = strasse
@@ -960,4 +968,21 @@ test_expect_success 'git -c complains about empty key and value' '
test_must_fail git -c "" rev-parse
'
+test_expect_success 'git config --edit works' '
+ git config -f tmp test.value no &&
+ echo test.value=yes >expect &&
+ GIT_EDITOR="echo [test]value=yes >" git config -f tmp --edit &&
+ git config -f tmp --list >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'git config --edit respects core.editor' '
+ git config -f tmp test.value no &&
+ echo test.value=yes >expect &&
+ test_config core.editor "echo [test]value=yes >" &&
+ git config -f tmp --edit &&
+ git config -f tmp --list >actual &&
+ test_cmp expect actual
+'
+
test_done
--
1.7.9.1.4.g8ffed
^ permalink raw reply related
* [PATCH 7/8] config: eliminate config_exclusive_filename
From: Jeff King @ 2012-02-16 8:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20120216080102.GA11793@sigill.intra.peff.net>
This is a magic global variable that was intended as an
override to the usual git-config lookup process. Once upon a
time, you could specify GIT_CONFIG to any git program, and
it would look only at that file. This turned out to be
confusing and cause a lot of bugs for little gain. As a
result, dc87183 (Only use GIT_CONFIG in "git config", not
other programs, 2008-06-30) took this away for all callers
except git-config.
Since git-config no longer uses it either, the variable can
just go away. As the diff shows, nobody was setting to
anything except NULL, so we can just replace any sites where
it was read with NULL.
Signed-off-by: Jeff King <peff@peff.net>
---
This could be squashed into the last patch (really, all of the last few
patches could be squashed). But I was able to "git grep
config_exclusive_filename" at this state and see that yes, indeed, the
variable is now totally useless.
cache.h | 2 --
config.c | 10 +++-------
2 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/cache.h b/cache.h
index 7cb8874..411c60d 100644
--- a/cache.h
+++ b/cache.h
@@ -1150,8 +1150,6 @@ struct config_include_data {
#define CONFIG_INCLUDE_INIT { 0 }
extern int git_config_include(const char *name, const char *value, void *data);
-extern const char *config_exclusive_filename;
-
#define MAX_GITNAME (1000)
extern char git_default_email[MAX_GITNAME];
extern char git_default_name[MAX_GITNAME];
diff --git a/config.c b/config.c
index fbf883d..e1d6857 100644
--- a/config.c
+++ b/config.c
@@ -26,8 +26,6 @@ static config_file *cf;
static int zlib_compression_seen;
-const char *config_exclusive_filename = NULL;
-
#define MAX_INCLUDE_DEPTH 10
static const char include_depth_advice[] =
"exceeded maximum include depth (%d) while including\n"
@@ -1005,7 +1003,7 @@ int git_config_with_options(config_fn_t fn, void *data,
int git_config(config_fn_t fn, void *data)
{
- return git_config_with_options(fn, data, config_exclusive_filename);
+ return git_config_with_options(fn, data, NULL);
}
/*
@@ -1504,8 +1502,7 @@ write_err_out:
int git_config_set_multivar(const char *key, const char *value,
const char *value_regex, int multi_replace)
{
- return git_config_set_multivar_in_file(config_exclusive_filename,
- key, value, value_regex,
+ return git_config_set_multivar_in_file(NULL, key, value, value_regex,
multi_replace);
}
@@ -1631,8 +1628,7 @@ out:
int git_config_rename_section(const char *old_name, const char *new_name)
{
- return git_config_rename_section_in_file(config_exclusive_filename,
- old_name, new_name);
+ return git_config_rename_section_in_file(NULL, old_name, new_name);
}
/*
--
1.7.9.1.4.g8ffed
^ permalink raw reply related
* [PATCH 8/8] config: do not respect includes for single-file --list
From: Jeff King @ 2012-02-16 8:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20120216080102.GA11793@sigill.intra.peff.net>
The original include implementation tried not to impact
calls to "git config" that look at a single file. However,
since we called into git_config in a few places (e.g.,
--list), our respect_includes flag was not supported.
This patch teaches git_config_with_options a flag to
respect includes (instead of doing so by default), and
teaches git-config to use it.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/config.c | 7 ++++---
cache.h | 3 ++-
config.c | 14 ++++++++------
t/t1305-config-include.sh | 8 ++++++++
4 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/builtin/config.c b/builtin/config.c
index 8901dd9..d41a9bf 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -316,7 +316,7 @@ static void get_color(const char *def_color)
get_color_found = 0;
parsed_color[0] = '\0';
git_config_with_options(git_get_color_config, NULL,
- given_config_file);
+ given_config_file, respect_includes);
if (!get_color_found && def_color)
color_parse(def_color, "command line", parsed_color);
@@ -344,7 +344,7 @@ static int get_colorbool(int print)
get_colorbool_found = -1;
get_diff_color_found = -1;
git_config_with_options(git_get_colorbool_config, NULL,
- given_config_file);
+ given_config_file, respect_includes);
if (get_colorbool_found < 0) {
if (!strcmp(get_colorbool_slot, "color.diff"))
@@ -441,7 +441,8 @@ int cmd_config(int argc, const char **argv, const char *prefix)
if (actions == ACTION_LIST) {
check_argc(argc, 0, 0);
if (git_config_with_options(show_all_config, NULL,
- given_config_file) < 0) {
+ given_config_file,
+ respect_includes) < 0) {
if (given_config_file)
die_errno("unable to read config file '%s'",
given_config_file);
diff --git a/cache.h b/cache.h
index 411c60d..ff54d6f 100644
--- a/cache.h
+++ b/cache.h
@@ -1115,7 +1115,8 @@ extern int git_config_from_file(config_fn_t fn, const char *, void *);
extern void git_config_push_parameter(const char *text);
extern int git_config_from_parameters(config_fn_t fn, void *data);
extern int git_config(config_fn_t fn, void *);
-extern int git_config_with_options(config_fn_t fn, void *, const char *filename);
+extern int git_config_with_options(config_fn_t fn, void *,
+ const char *filename, int respect_includes);
extern int git_config_early(config_fn_t fn, void *, const char *repo_config);
extern int git_parse_ulong(const char *, unsigned long *);
extern int git_config_int(const char *, const char *);
diff --git a/config.c b/config.c
index e1d6857..ad03908 100644
--- a/config.c
+++ b/config.c
@@ -976,16 +976,18 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config)
}
int git_config_with_options(config_fn_t fn, void *data,
- const char *filename)
+ const char *filename, int respect_includes)
{
char *repo_config = NULL;
int ret;
struct config_include_data inc = CONFIG_INCLUDE_INIT;
- inc.fn = fn;
- inc.data = data;
- fn = git_config_include;
- data = &inc;
+ if (respect_includes) {
+ inc.fn = fn;
+ inc.data = data;
+ fn = git_config_include;
+ data = &inc;
+ }
/*
* If we have a specific filename, use it. Otherwise, follow the
@@ -1003,7 +1005,7 @@ int git_config_with_options(config_fn_t fn, void *data,
int git_config(config_fn_t fn, void *data)
{
- return git_config_with_options(fn, data, NULL);
+ return git_config_with_options(fn, data, NULL, 1);
}
/*
diff --git a/t/t1305-config-include.sh b/t/t1305-config-include.sh
index 0a27ec4..f3e03a0 100755
--- a/t/t1305-config-include.sh
+++ b/t/t1305-config-include.sh
@@ -59,6 +59,14 @@ test_expect_success 'single file lookup does not expand includes by default' '
test_cmp expect actual
'
+test_expect_success 'single file list does not expand includes by default' '
+ echo "[test]one = 1" >one &&
+ echo "[include]path = one" >.gitconfig &&
+ echo "include.path=one" >expect &&
+ git config -f .gitconfig --list >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'writing config file does not expand includes' '
echo "[test]one = 1" >one &&
echo "[include]path = one" >.gitconfig &&
--
1.7.9.1.4.g8ffed
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox