* Re: RFC: "git config -l" should not expose sensitive information
From: Jeff King @ 2012-12-20 15:04 UTC (permalink / raw)
To: Toralf Förster; +Cc: git
In-Reply-To: <50CF039A.7010800@gmx.de>
On Mon, Dec 17, 2012 at 12:35:54PM +0100, Toralf Förster wrote:
> often the output is requested in help forums - and a
> "git config -l | wgetpaste" exposes parameters like sendmail.smtppass -
> so hide those variables in the output (if not explicitly wanted) would
> makes sense, or ?
But if we change "git config -l", won't that break all of the
non-exposing uses that rely on seeing all of the variables (e.g., it is
perfectly fine for a porcelain to parse "git config -l" rather than
making several calls to "git config"; IIRC, git-cola does this).
The problem seems to be that people are giving bad advice to tell people
to post "git config -l" output without looking at. Maybe we could help
them with a "git config --share-config" option that dumps all config,
but sanitizes the output. It would need to have a list of sensitive keys
(which does not exist yet), and would need to not just mark up things
like smtppass, but would also need to pull credential information out of
remote.*.url strings. And maybe more (I haven't thought too long on it).
-Peff
^ permalink raw reply
* Re: $PATH pollution and t9902-completion.sh
From: Adam Spiers @ 2012-12-20 15:13 UTC (permalink / raw)
To: Jeff King; +Cc: git mailing list
In-Reply-To: <20121220145519.GB27211@sigill.intra.peff.net>
On Thu, Dec 20, 2012 at 2:55 PM, Jeff King <peff@peff.net> wrote:
> On Mon, Dec 17, 2012 at 01:05:38AM +0000, Adam Spiers wrote:
>> t/t9902-completion.sh is currently failing for me because I happen to
>> have a custom shell-script called git-check-email in ~/bin, which is
>> on my $PATH. This is different to a similar-looking case reported
>> recently, which was due to an unclean working tree:
>>
>> http://thread.gmane.org/gmane.comp.version-control.git/208085
>>
>> It's not unthinkable that in the future other tests could break for
>> similar reasons. Therefore it would be good to sanitize $PATH in the
>> test framework so that it cannot destabilize tests, although I am
>> struggling to think of a good way of doing this. Naively stripping
>> directories under $HOME would not protect against git "plugins" such
>> as the above being installed into places like /usr/bin. Thoughts?
>
> I've run into this, too. I think sanitizing $PATH is the wrong approach.
> The real problem is that the test is overly picky. Right now it is
> failing because you happen to have "check-email" in your $PATH, but it
> will also need to be adjusted when a true "check-email" command is added
> to git.
>
> I can think of two other options:
>
> 1. Make the test input more specific (e.g., looking for "checkou").
> This doesn't eliminate the problem, but makes it less likely
> to occur.
>
> 2. Loosen the test to look for the presence of "checkout", but not
> fail when other items are present. Bonus points if it makes sure
> that everything returned starts with "check".
>
> I think (2) is the ideal solution in terms of behavior, but writing it
> may be more of a pain.
I agree with all your points. Thanks for the suggestions.
^ permalink raw reply
* Re: [PATCH v6 0/7] make test output coloring more intuitive
From: Jeff King @ 2012-12-20 15:34 UTC (permalink / raw)
To: Adam Spiers; +Cc: Junio C Hamano, git list
In-Reply-To: <CAOkDyE9B_HfUZmqNqO35mtjTvdihBTiW=uOV2oEQgLUw1xyf=A@mail.gmail.com>
On Sun, Dec 16, 2012 at 07:01:56PM +0000, Adam Spiers wrote:
> On Sun, Dec 16, 2012 at 6:54 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > Adam Spiers <git@adamspiers.org> writes:
> >
> >> This series of commits attempts to make test output coloring
> >> more intuitive,...
> >
> > Thanks; I understand that this is to replace the previous one
> > b465316 (tests: paint unexpectedly fixed known breakages in bold
> > red, 2012-09-19)---am I correct?
>
> Correct. AFAICS I have incorporated all feedback raised in previous
> reviews.
>
> > Will take a look; thanks.
>
> Thanks. Sorry again for the delay. I'm now (finally) resuming work
> on as/check-ignore.
I eyeballed the test output of "pu". I do think this resolves all of the
issues brought up before, and I really hate to bikeshed on the colors at
this point, but I find that bold cyan a bit hard on the eyes when
running with "-v" (where most of the output is in that color, as it
dumps the shell for each test). Is there any reason not to tone it down
a bit like:
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 256f1c6..31f59af 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -227,7 +227,7 @@ then
pass)
tput setaf 2;; # green
info)
- tput bold; tput setaf 6;; # bold cyan
+ tput setaf 6;; # cyan
*)
test -n "$quiet" && return;;
esac
-Peff
^ permalink raw reply related
* Re: [PATCH v6 0/7] make test output coloring more intuitive
From: Adam Spiers @ 2012-12-20 15:44 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git list
In-Reply-To: <20121220153411.GA1497@sigill.intra.peff.net>
On Thu, Dec 20, 2012 at 3:34 PM, Jeff King <peff@peff.net> wrote:
> On Sun, Dec 16, 2012 at 07:01:56PM +0000, Adam Spiers wrote:
>> On Sun, Dec 16, 2012 at 6:54 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> > Adam Spiers <git@adamspiers.org> writes:
>> >> This series of commits attempts to make test output coloring
>> >> more intuitive,...
>> >
>> > Thanks; I understand that this is to replace the previous one
>> > b465316 (tests: paint unexpectedly fixed known breakages in bold
>> > red, 2012-09-19)---am I correct?
>>
>> Correct. AFAICS I have incorporated all feedback raised in previous
>> reviews.
>>
>> > Will take a look; thanks.
>>
>> Thanks. Sorry again for the delay. I'm now (finally) resuming work
>> on as/check-ignore.
>
> I eyeballed the test output of "pu". I do think this resolves all of the
> issues brought up before, and I really hate to bikeshed on the colors at
> this point, but I find that bold cyan a bit hard on the eyes when
> running with "-v" (where most of the output is in that color, as it
> dumps the shell for each test). Is there any reason not to tone it down
> a bit like:
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 256f1c6..31f59af 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -227,7 +227,7 @@ then
> pass)
> tput setaf 2;; # green
> info)
> - tput bold; tput setaf 6;; # bold cyan
> + tput setaf 6;; # cyan
> *)
> test -n "$quiet" && return;;
> esac
>
> -Peff
Good point, I forgot to check what it looked like with -v. Since this
series is already on v6, is there a more lightweight way of addressing
this tiny tweak than sending v7?
^ permalink raw reply
* Re: RFC: "git config -l" should not expose sensitive information
From: Aaron Schrab @ 2012-12-20 15:49 UTC (permalink / raw)
To: Jeff King; +Cc: Toralf Förster, git
In-Reply-To: <20121220150408.GD27211@sigill.intra.peff.net>
At 10:04 -0500 20 Dec 2012, Jeff King <peff@peff.net> wrote:
>The problem seems to be that people are giving bad advice to tell
>people to post "git config -l" output without looking at. Maybe we
>could help them with a "git config --share-config" option that dumps
>all config, but sanitizes the output. It would need to have a list of
>sensitive keys (which does not exist yet), and would need to not just
>mark up things like smtppass, but would also need to pull credential
>information out of remote.*.url strings. And maybe more (I haven't
>thought too long on it).
If such an option is added, it is likely to cause more people to think
that there is no need to examine the output before sharing it. But, I
don't think that the sanitizing could ever be sufficient to guarantee
that.
Tools outside of the core git tree may add support for new config keys
which are meant to contain sensitive information, and there would be no
way for `git config` to know about those.
Even for known sensitive keys, the person entering it might have made a
typo in the name (e.g. smptpass) preventing it from being recognized as
sensitive by the software, but easily recognizable as such by a human.
There's also the problem of varying opinions on what is considered as
sensitive. You mention credential information in URLs, but some people
may consider the entire URL as something which they would not want to
expose.
I think that attempting to do this would only result in a false sense of
security.
^ permalink raw reply
* Re: RFC: "git config -l" should not expose sensitive information
From: Michael Haggerty @ 2012-12-20 15:51 UTC (permalink / raw)
To: Jeff King; +Cc: Toralf Förster, git
In-Reply-To: <20121220150408.GD27211@sigill.intra.peff.net>
On 12/20/2012 04:04 PM, Jeff King wrote:
> On Mon, Dec 17, 2012 at 12:35:54PM +0100, Toralf Förster wrote:
>> often the output is requested in help forums - and a
>> "git config -l | wgetpaste" exposes parameters like sendmail.smtppass -
>> so hide those variables in the output (if not explicitly wanted) would
>> makes sense, or ?
>
> But if we change "git config -l", won't that break all of the
> non-exposing uses that rely on seeing all of the variables (e.g., it is
> perfectly fine for a porcelain to parse "git config -l" rather than
> making several calls to "git config"; IIRC, git-cola does this).
>
> The problem seems to be that people are giving bad advice to tell people
> to post "git config -l" output without looking at. Maybe we could help
> them with a "git config --share-config" option that dumps all config,
> but sanitizes the output. It would need to have a list of sensitive keys
> (which does not exist yet), and would need to not just mark up things
> like smtppass, but would also need to pull credential information out of
> remote.*.url strings. And maybe more (I haven't thought too long on it).
I think the problem is yet another step earlier: why do we build tools
that encourage people to store passwords in plaintext in a configuration
file that is by default world-readable?
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: RFC: "git config -l" should not expose sensitive information
From: Jeff King @ 2012-12-20 15:52 UTC (permalink / raw)
To: git; +Cc: Toralf Förster
In-Reply-To: <20121220154915.GA5162@pug.qqx.org>
On Thu, Dec 20, 2012 at 10:49:15AM -0500, Aaron Schrab wrote:
> At 10:04 -0500 20 Dec 2012, Jeff King <peff@peff.net> wrote:
> >The problem seems to be that people are giving bad advice to tell
> >people to post "git config -l" output without looking at. Maybe we
> >could help them with a "git config --share-config" option that
> >dumps all config, but sanitizes the output. It would need to have a
> >list of sensitive keys (which does not exist yet), and would need
> >to not just mark up things like smtppass, but would also need to
> >pull credential information out of remote.*.url strings. And maybe
> >more (I haven't thought too long on it).
>
> If such an option is added, it is likely to cause more people to
> think that there is no need to examine the output before sharing it.
> But, I don't think that the sanitizing could ever be sufficient to
> guarantee that.
>
> Tools outside of the core git tree may add support for new config
> keys which are meant to contain sensitive information, and there
> would be no way for `git config` to know about those.
Good point. I was a little on the fence already because any time you
have a "prevent known bad" list in a security setting, it is guaranteed
to go out of date and screw you. But the presence of third-party tools
means it does not even have to get out of date. It will always be
complete.
> I think that attempting to do this would only result in a false sense
> of security.
Yeah. Thanks for a dose of sanity. I was really trying not to say "the
given advice is bad, and we cannot help those people". But I think you
are right; the only sensible path is for the user to inspect the output
before posting it.
-Peff
^ permalink raw reply
* Re: RFC: "git config -l" should not expose sensitive information
From: Jeff King @ 2012-12-20 15:54 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Toralf Förster, git
In-Reply-To: <50D33409.1050309@alum.mit.edu>
On Thu, Dec 20, 2012 at 04:51:37PM +0100, Michael Haggerty wrote:
> > The problem seems to be that people are giving bad advice to tell people
> > to post "git config -l" output without looking at. Maybe we could help
> > them with a "git config --share-config" option that dumps all config,
> > but sanitizes the output. It would need to have a list of sensitive keys
> > (which does not exist yet), and would need to not just mark up things
> > like smtppass, but would also need to pull credential information out of
> > remote.*.url strings. And maybe more (I haven't thought too long on it).
>
> I think the problem is yet another step earlier: why do we build tools
> that encourage people to store passwords in plaintext in a configuration
> file that is by default world-readable?
Agreed. Most of it is hysterical raisins. We did not have any portable
secure storage for a long time. These days we have the credential helper
subsystem, which send-email can and should be using.
-Peff
^ permalink raw reply
* Re: [PATCH v6 0/7] make test output coloring more intuitive
From: Jeff King @ 2012-12-20 16:11 UTC (permalink / raw)
To: Adam Spiers; +Cc: Junio C Hamano, git list
In-Reply-To: <CAOkDyE9y6JvNKTCBoJqu47Hn-3axfjZPUdBhf4bOEfSP-9Q84A@mail.gmail.com>
On Thu, Dec 20, 2012 at 03:44:53PM +0000, Adam Spiers wrote:
> > diff --git a/t/test-lib.sh b/t/test-lib.sh
> > index 256f1c6..31f59af 100644
> > --- a/t/test-lib.sh
> > +++ b/t/test-lib.sh
> > @@ -227,7 +227,7 @@ then
> > pass)
> > tput setaf 2;; # green
> > info)
> > - tput bold; tput setaf 6;; # bold cyan
> > + tput setaf 6;; # cyan
> > *)
> > test -n "$quiet" && return;;
> > esac
> >
>
> Good point, I forgot to check what it looked like with -v. Since this
> series is already on v6, is there a more lightweight way of addressing
> this tiny tweak than sending v7?
It is ultimately up to Junio, but I suspect he would be OK if you just
reposted patch 4/7 with the above squashed. Or even just said "I like
this, please squash it into patch 4 (change info messages from
yellow/brown to bold cyan).
As an aside, it made me wonder how hard/useful it would be to color the
snippets even more. Doing this:
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index f9ccbf2..3d44a94 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -364,7 +364,12 @@ test_expect_success () {
export test_prereq
if ! test_skip "$@"
then
- say >&3 "expecting success: $2"
+ if test -z "$GIT_TEST_HIGHLIGHT"; then
+ say >&3 "expecting success: $2"
+ else
+ say >&3 "expecting success:"
+ echo "$2" | eval "$GIT_TEST_HIGHLIGHT"
+ fi
if test_run_ "$2"
then
test_ok_ "$1"
produces highlighted snippets with:
GIT_TEST_HIGHLIGHT='highlight -S sh -O ansi'
or
GIT_TEST_HIGHLIGHT='pygmentize -l sh'
depending on what you have installed on your system. I'm not convinced
it actually adds anything, but it's a fun toy. A real patch would
probably turn it off in non-verbose mode, as invoking the highlighter
(especially pygmentize) repeatedly is somewhat expensive.
-Peff
^ permalink raw reply related
* Re: RFC: "git config -l" should not expose sensitive information
From: Toralf Förster @ 2012-12-20 16:20 UTC (permalink / raw)
To: Jeff King, git
In-Reply-To: <20121220154915.GA5162@pug.qqx.org>
yep - understood
On 12/20/2012 04:49 PM, Aaron Schrab wrote:
> At 10:04 -0500 20 Dec 2012, Jeff King <peff@peff.net> wrote:
>> The problem seems to be that people are giving bad advice to tell
>> people to post "git config -l" output without looking at. Maybe we
>> could help them with a "git config --share-config" option that dumps
>> all config, but sanitizes the output. It would need to have a list of
>> sensitive keys (which does not exist yet), and would need to not just
>> mark up things like smtppass, but would also need to pull credential
>> information out of remote.*.url strings. And maybe more (I haven't
>> thought too long on it).
>
> If such an option is added, it is likely to cause more people to think
> that there is no need to examine the output before sharing it. But, I
> don't think that the sanitizing could ever be sufficient to guarantee that.
>
> Tools outside of the core git tree may add support for new config keys
> which are meant to contain sensitive information, and there would be no
> way for `git config` to know about those.
>
> Even for known sensitive keys, the person entering it might have made a
> typo in the name (e.g. smptpass) preventing it from being recognized as
> sensitive by the software, but easily recognizable as such by a human.
>
> There's also the problem of varying opinions on what is considered as
> sensitive. You mention credential information in URLs, but some people
> may consider the entire URL as something which they would not want to
> expose.
>
> I think that attempting to do this would only result in a false sense of
> security.
>
--
MfG/Sincerely
Toralf Förster
pgp finger print: 7B1A 07F4 EC82 0F90 D4C2 8936 872A E508 7DB6 9DA3
^ permalink raw reply
* Re: [RFC] test: Old shells and physical paths
From: David Michael @ 2012-12-20 16:25 UTC (permalink / raw)
To: David Aguilar; +Cc: Junio C Hamano, git@vger.kernel.org
In-Reply-To: <CAJDDKr78ugSo9hNerHO0Y46_bSzLJWznB3E3+6H98NjMtBwHsw@mail.gmail.com>
Hi,
On Thu, Dec 20, 2012 at 12:01 AM, David Aguilar <davvid@gmail.com> wrote:
> Do you know if the differences are relegated to "cd",
> or do other common commands such as awk, grep, sed, mktemp, expr,
> etc. have similar issues?
There are almost certainly going to be incompatibilities with other
commands. The system implemented UNIX95 plus some extensions, then
they began supporting UNIX03/SUSv3/POSIX.1-2001/whatever for certain
commands by using an environment variable to toggle between the
incompatible behaviors.
Their documentation on the UNIX03 commands indicates it is still only
partially supported. For example: "cp" supports "-L" and "-P", but
"cd" doesn't.
> I wonder if it'd be helpful to have a low-numbered test that checks
> the basics needed by the scripted Porcelains and test suite.
> It would give us an easy way to answer these questions, and could
> be a good way to document (in code) the capabilities we expect.
I'd be in favor of something like this as well.
Thanks.
David
P.S.
In the meantime, I am handling the "cd" situation by replacing "-P"
with "$PHYS" and prepending the following to t/test-lib.sh.
set +o logical >/dev/null 2>&1 || PHYS=-P
^ permalink raw reply
* Re: $PATH pollution and t9902-completion.sh
From: Torsten Bögershausen @ 2012-12-20 17:25 UTC (permalink / raw)
To: Adam Spiers; +Cc: Jeff King, git mailing list
In-Reply-To: <CAOkDyE-J7sTJ-GefhteP1wy7WorqTRnj5nn0k6hd1dp0VJz5iQ@mail.gmail.com>
On 20.12.12 16:13, Adam Spiers wrote:
> On Thu, Dec 20, 2012 at 2:55 PM, Jeff King <peff@peff.net> wrote:
>> On Mon, Dec 17, 2012 at 01:05:38AM +0000, Adam Spiers wrote:
>>> t/t9902-completion.sh is currently failing for me because I happen to
>>> have a custom shell-script called git-check-email in ~/bin, which is
>>> on my $PATH. This is different to a similar-looking case reported
>>> recently, which was due to an unclean working tree:
>>>
>>> http://thread.gmane.org/gmane.comp.version-control.git/208085
>>>
>>> It's not unthinkable that in the future other tests could break for
>>> similar reasons. Therefore it would be good to sanitize $PATH in the
>>> test framework so that it cannot destabilize tests, although I am
>>> struggling to think of a good way of doing this. Naively stripping
>>> directories under $HOME would not protect against git "plugins" such
>>> as the above being installed into places like /usr/bin. Thoughts?
>>
>> I've run into this, too. I think sanitizing $PATH is the wrong approach.
>> The real problem is that the test is overly picky. Right now it is
>> failing because you happen to have "check-email" in your $PATH, but it
>> will also need to be adjusted when a true "check-email" command is added
>> to git.
>>
>> I can think of two other options:
>>
>> 1. Make the test input more specific (e.g., looking for "checkou").
>> This doesn't eliminate the problem, but makes it less likely
>> to occur.
>>
>> 2. Loosen the test to look for the presence of "checkout", but not
>> fail when other items are present. Bonus points if it makes sure
>> that everything returned starts with "check".
>>
>> I think (2) is the ideal solution in terms of behavior, but writing it
>> may be more of a pain.
>
> I agree with all your points. Thanks for the suggestions.
I volonteer for 1) (and we got for 2) later)
^ permalink raw reply
* [PATCH] t9902: Be more specific when completing git-checkout
From: Torsten Bögershausen @ 2012-12-20 17:31 UTC (permalink / raw)
To: git; +Cc: tboegi
t9902 is trying to complete e.g.
"git --version check" into "git --version checkout"
If there are other binaries like
"git-check-email" or "git-check-ignore" in the PATH
one test case failes
By using "checkou" instead of "check" the test becomes
more future proof.
Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
t/t9902-completion.sh | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 3cd53f8..0e0e2d1 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -192,19 +192,19 @@ test_expect_success 'general options' '
'
test_expect_success 'general options plus command' '
- test_completion "git --version check" "checkout " &&
- test_completion "git --paginate check" "checkout " &&
- test_completion "git --git-dir=foo check" "checkout " &&
- test_completion "git --bare check" "checkout " &&
+ test_completion "git --version checkou" "checkout " &&
+ test_completion "git --paginate checkou" "checkout " &&
+ test_completion "git --git-dir=foo checkou" "checkout " &&
+ test_completion "git --bare checkou" "checkout " &&
test_completion "git --help des" "describe " &&
- test_completion "git --exec-path=foo check" "checkout " &&
- test_completion "git --html-path check" "checkout " &&
- test_completion "git --no-pager check" "checkout " &&
- test_completion "git --work-tree=foo check" "checkout " &&
- test_completion "git --namespace=foo check" "checkout " &&
- test_completion "git --paginate check" "checkout " &&
- test_completion "git --info-path check" "checkout " &&
- test_completion "git --no-replace-objects check" "checkout "
+ test_completion "git --exec-path=foo checkou" "checkout " &&
+ test_completion "git --html-path checkou" "checkout " &&
+ test_completion "git --no-pager checkou" "checkout " &&
+ test_completion "git --work-tree=foo checkou" "checkout " &&
+ test_completion "git --namespace=foo checkou" "checkout " &&
+ test_completion "git --paginate checkou" "checkout " &&
+ test_completion "git --info-path checkou" "checkout " &&
+ test_completion "git --no-replace-objects checkou" "checkout "
'
test_expect_success 'setup for ref completion' '
--
1.8.0
^ permalink raw reply related
* [PATCH] oneway_merge(): only lstat() when told to update worktree
From: Martin von Zweigbergk @ 2012-12-20 17:37 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Martin von Zweigbergk
Although the subject line of 613f027 (read-tree -u one-way merge fix
to check out locally modified paths., 2006-05-15) mentions "read-tree
-u", it did not seem to check whether -u was in effect. Not checking
whether -u is in effect makes e.g. "read-tree --reset" lstat() the
worktree, even though the worktree stat should not matter for that
operation.
This speeds up e.g. "git reset" a little on the linux-2.6 repo (best
of five, warm cache):
Before After
real 0m0.288s 0m0.233s
user 0m0.190s 0m0.150s
sys 0m0.090s 0m0.080s
---
I am very unfamiliar with this part of git, so my attempt at a
motivation may be totally off.
I have another twenty-or-so patches to reset.c coming up that take the
timings down to around 90 ms, but this patch was quite unrelated to
that. Those other patches actually make this patch pointless for "git
reset" (it takes another path), but I hope this is still a good change
for other operations that use oneway_merge.
unpack-trees.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index 6d96366..61acc5e 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1834,7 +1834,7 @@ int oneway_merge(struct cache_entry **src, struct unpack_trees_options *o)
if (old && same(old, a)) {
int update = 0;
- if (o->reset && !ce_uptodate(old) && !ce_skip_worktree(old)) {
+ if (o->reset && o->update && !ce_uptodate(old) && !ce_skip_worktree(old)) {
struct stat st;
if (lstat(old->name, &st) ||
ie_match_stat(o->src_index, old, &st, CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE))
--
1.8.0.1.240.ge8a1f5a
^ permalink raw reply related
* Re: What's cooking in git.git (Dec 2012, #05; Tue, 18)
From: Junio C Hamano @ 2012-12-20 18:13 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20121220145941.GC27211@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Tue, Dec 18, 2012 at 12:31:43PM -0800, Junio C Hamano wrote:
>
>> * jk/error-const-return (2012-12-15) 2 commits
>> - silence some -Wuninitialized false positives
>> - make error()'s constant return value more visible
>>
>> Help compilers' flow analysis by making it more explicit that
>> error() always returns -1, to reduce false "variable used
>> uninitialized" warnings.
>>
>> This is still an RFC.
>
> What's your opinion on this?
Ugly but not too much so, and it would be useful.
The only thing that makes it ugly is that it promises error() will
return -1 and nothing else forever, but at this point in the
evolution of the codebase, I think we are pretty much committed to
it anyway, so I do not think it is a problem.
^ permalink raw reply
* Re: [PATCH v6 0/7] make test output coloring more intuitive
From: Adam Spiers @ 2012-12-20 18:08 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git list
In-Reply-To: <20121220161110.GA10605@sigill.intra.peff.net>
On Thu, Dec 20, 2012 at 4:11 PM, Jeff King <peff@peff.net> wrote:
> On Thu, Dec 20, 2012 at 03:44:53PM +0000, Adam Spiers wrote:
>> > diff --git a/t/test-lib.sh b/t/test-lib.sh
>> > index 256f1c6..31f59af 100644
>> > --- a/t/test-lib.sh
>> > +++ b/t/test-lib.sh
>> > @@ -227,7 +227,7 @@ then
>> > pass)
>> > tput setaf 2;; # green
>> > info)
>> > - tput bold; tput setaf 6;; # bold cyan
>> > + tput setaf 6;; # cyan
>> > *)
>> > test -n "$quiet" && return;;
>> > esac
>> >
>>
>> Good point, I forgot to check what it looked like with -v. Since this
>> series is already on v6, is there a more lightweight way of addressing
>> this tiny tweak than sending v7?
>
> It is ultimately up to Junio, but I suspect he would be OK if you just
> reposted patch 4/7 with the above squashed.
I'll do that if Junio is OK with that.
> Or even just said "I like
> this, please squash it into patch 4 (change info messages from
> yellow/brown to bold cyan).
Yes, I'm OK with this way too :) Of course "bold" would need to be dropped
from the commit message.
^ permalink raw reply
* [PATCH] Highlight the link target line in Gitweb using CSS
From: Matthew Blissett @ 2012-12-20 18:16 UTC (permalink / raw)
To: git; +Cc: Matthew Blissett
This is useful when a Gitweb link with a target (like #l100) refers to
a line in the last screenful of text. Highlight the background in
yellow, and display a ⚓ character on the left. Show the same
highlight when hovering the mouse over a line number.
Signed-off-by: Matthew Blissett <matt@blissett.me.uk>
---
The background-colour change is the 'main' (tiny) change.
Consider the ::before part a suggestion. I think it helps show the
target line, but it does overlap the first character of any line >999.
I've tested this on the browsers I have access to, which excludes
Internet Explorer. Since it's cosmetic it shouldn't matter if it doesn't
work.
Wikipedia use similar CSS for their citation links:
<http://en.wikipedia.org/wiki/Git_(software)#cite_note-1>
gitweb/static/gitweb.css | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/gitweb/static/gitweb.css b/gitweb/static/gitweb.css
index cb86d2d..9f54311 100644
--- a/gitweb/static/gitweb.css
+++ b/gitweb/static/gitweb.css
@@ -546,6 +546,16 @@ a.linenr {
text-decoration: none
}
+a.linenr:hover, a.linenr:target {
+ color: #444444;
+ background-color: #ff4;
+}
+
+a.linenr:hover::before, a.linenr:target::before {
+ content: '⚓';
+ position: absolute;
+}
+
a.rss_logo {
float: right;
padding: 3px 0px;
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH] Python scripts audited for minimum compatible version and checks added.
From: Junio C Hamano @ 2012-12-20 18:25 UTC (permalink / raw)
To: esr; +Cc: Jeff King, git
In-Reply-To: <20121220150252.GA24387@thyrsus.com>
"Eric S. Raymond" <esr@thyrsus.com> writes:
>> Should the error message say ciabot.py?
>>
>> -Peff
>
> Gack. Yes. Thaty's what I get for cut-and-pasting too quickly.
> The information about xnml.sex is correct, though.
>
> Want me to resubmit, or will you just patch it?
Can handle it myself; thanks for the patch.
> Note by the way that I still think the entire ciabot subtree (which is
> my code) should just be nuked. CIA is not coming back, wishful thinking
> on Ilkotech's web page notwithstanding.
You are probably right, and interested people could send a patch to
resurrect it, if it turns necessary, from our last commit that has
it. So let's apply this patch, and then remove the subtree soon
after 1.8.1 ships.
^ permalink raw reply
* Re: Python version auditing followup
From: Junio C Hamano @ 2012-12-20 18:30 UTC (permalink / raw)
To: Eric S. Raymond; +Cc: git
In-Reply-To: <20121220143411.BEA0744105@snark.thyrsus.com>
esr@thyrsus.com (Eric S. Raymond) writes:
> That was the first of three patches I have promised. In order to do
> the next one, which will be a development guidelines recommend
> compatibility back to some specific version X, I need a policy
> decision. How do we set X?
>
> I don't think X can be < 2.4, nor does it need to be - 2.4 came out
> in 2004 and eight years is plenty of deployment time.
>
> The later we set it, the more convenient for developers. But of
> course by setting it late we trade away some portability to
> older systems.
>
> In previous discussion of this issue I recommended X = 2.6.
> That is still my recommendation. Thoughts, comments, objections?
I personally would think 2.6 is recent enough. Which platforms that
are long-term-maintained by their vendors still pin their Python at
2.4.X? 2.4.6 was in 2008 that was source only, 2.4.4 was in late
2006 that was the last 2.4 with binary release.
Objections? Comments?
^ permalink raw reply
* [PATCH] builtin/clean.c: Fix some sparse warnings
From: Ramsay Jones @ 2012-12-20 18:33 UTC (permalink / raw)
To: zoltan.klinger; +Cc: Junio C Hamano, GIT Mailing-list
Sparse issues two "Using plain integer as NULL pointer" warnings
(lines 41 and 47).
The first warning relates to the initializer expression in the
declaration for the 'char *dir' variable. In order to suppress
the warning, we simply replace the zero initializer with NULL.
The second warning relates to an expression, as part of an if
conditional, using the equality operator to compare the 'dir'
variable to zero. In order to suppress the warning, we replace
the 'dir == 0' expression with the more idiomatic '!dir'.
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
Hi Zoltan,
If you have already updated your patch and made this redundant
(it's been a few days since I read the list or fetched git.git),
please ignore this. Otherwise, could you please squash this into
the new version of commit 16e4033e6 ("git-clean: Display more
accurate delete messages", 17-12-2012).
[BTW, in the same conditional expression you have an strncmp()
call which doesn't quite follow the style/conventions of the
existing code.]
Thanks!
ATB,
Ramsay Jones
builtin/clean.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/clean.c b/builtin/clean.c
index 1c25a75..0c603c8 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -38,13 +38,13 @@ static void print_filtered(const char *msg, struct string_list *lst)
{
int i;
char *name;
- char *dir = 0;
+ char *dir = NULL;
sort_string_list(lst);
for (i = 0; i < lst->nr; i++) {
name = lst->items[i].string;
- if (dir == 0 || strncmp(name, dir, strlen(dir)) != 0)
+ if (!dir || strncmp(name, dir, strlen(dir)) != 0)
printf("%s %s\n", msg, name);
if (name[strlen(name) - 1] == '/')
dir = name;
--
1.8.0
^ permalink raw reply related
* Re: $PATH pollution and t9902-completion.sh
From: Junio C Hamano @ 2012-12-20 18:36 UTC (permalink / raw)
To: Jeff King; +Cc: Adam Spiers, git mailing list
In-Reply-To: <20121220145519.GB27211@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> 2. Loosen the test to look for the presence of "checkout", but not
> fail when other items are present. Bonus points if it makes sure
> that everything returned starts with "check".
>
> I think (2) is the ideal solution in terms of behavior, but writing it
> may be more of a pain.
Yeah, I think (2) is the way to go.
^ permalink raw reply
* Re: RFC: "git config -l" should not expose sensitive information
From: Junio C Hamano @ 2012-12-20 18:37 UTC (permalink / raw)
To: Jeff King; +Cc: git, Toralf Förster
In-Reply-To: <20121220155229.GA6008@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Yeah. Thanks for a dose of sanity. I was really trying not to say "the
> given advice is bad, and we cannot help those people". But I think you
> are right; the only sensible path is for the user to inspect the output
> before posting it.
True.
^ permalink raw reply
* Re: RFC: "git config -l" should not expose sensitive information
From: Junio C Hamano @ 2012-12-20 18:49 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Jeff King, Toralf Förster, git
In-Reply-To: <50D33409.1050309@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> I think the problem is yet another step earlier: why do we build tools
> that encourage people to store passwords in plaintext in a configuration
> file that is by default world-readable?
True. This particular one mentioned in the thread predates
credential helpers, so it is not faire to say "encourage".
We didn't and we don't.
Care to do a patch to deprecate sendemail.smtppass (i.e. give
warnings to users when it is used) and perhaps replace it with
something based on the credential store or something?
^ permalink raw reply
* Re: Q: do people compile with NO_FNMATCH on OpenBSD 5.2?
From: Greg Troxel @ 2012-12-20 18:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Arnout Engelen
In-Reply-To: <7vzk1bdqy0.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 629 bytes --]
[mkstemp truncating output on error]
diff --git c/wrapper.c w/wrapper.c
index 68739aa..a066e2e 100644
--- c/wrapper.c
+++ w/wrapper.c
@@ -229,7 +229,7 @@ int xmkstemp(char *template)
int saved_errno = errno;
const char *nonrelative_template;
- if (!template[0])
+ if (strlen(template) != strlen(origtemplate))
template = origtemplate;
nonrelative_template = absolute_path(template);
Thanks for the quick fix.
I applied this patch to 1.8.0.1, and then the tests get all the way to
t1402 (separate msg when I figure out why).
[-- Attachment #2: Type: application/pgp-signature, Size: 194 bytes --]
^ permalink raw reply
* Re: [PATCH v6 0/7] make test output coloring more intuitive
From: Junio C Hamano @ 2012-12-20 19:21 UTC (permalink / raw)
To: Jeff King; +Cc: Adam Spiers, git list
In-Reply-To: <20121220161110.GA10605@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
>> Good point, I forgot to check what it looked like with -v. Since this
>> series is already on v6, is there a more lightweight way of addressing
>> this tiny tweak than sending v7?
>
> It is ultimately up to Junio, but I suspect he would be OK if you just
> reposted patch 4/7 with the above squashed. Or even just said "I like
> this, please squash it into patch 4 (change info messages from
> yellow/brown to bold cyan).
Surely; as long as the series is not in 'next', the change to be
squashed is not too big and it is not too much work (and in this
case it certainly is not).
I actually wonder if "skipped test in bold blue" and "known breakage
in bold yellow" should also lose the boldness. Errors and warnings
in bold are good, but I would say the degree of need for attention
are more like this:
error (failed tests - you should look into it)
skip (skipped - perhaps you need more packages?)
warn (expected failure - you may want to look into fixing it someday)
info
pass
The "expected_failure" cases painted in "warn" are all long-known
failures; I do not think reminding about them in "bold" over and
over will help encouraging the developers take a look at them.
The "skipped" cases fall into two categories. Either you already
know you choose to not to care (e.g. I do not expect to use git-p4
and decided not to install p4 anywhere, so I may have t98?? on
GIT_SKIP_TESTS environment) or you haven't reached that point on a
new system and haven't realized that you didn't install a package
needed to run tests you care about (e.g. cvsserver tests would not
run without Perl interface to SQLite). For the former, the bold
output is merely distracting; for the latter, bold _might_ help in
this case.
At least, I think
GIT_SKIP_TESTS=t98?? sh t9800-git-p4-basic.sh -v
should paint "skipping test t9800 altogether" (emitted with "-v) and
the last line "1..0 # SKIP skip all tests in t9800" both in the same
"info" color.
How about going further to reduce "bold" a bit more, like this?
diff --git a/t/test-lib.sh b/t/test-lib.sh
index aaf013e..2bbb81d 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -182,13 +182,13 @@ then
error)
tput bold; tput setaf 1;; # bold red
skip)
- tput bold; tput setaf 4;; # bold blue
+ tput setaf 4;; # bold blue
warn)
- tput bold; tput setaf 3;; # bold brown/yellow
+ tput setaf 3;; # bold brown/yellow
pass)
tput setaf 2;; # green
info)
- tput bold; tput setaf 6;; # bold cyan
+ tput setaf 6;; # bold cyan
*)
test -n "$quiet" && return;;
esac
@@ -589,7 +589,7 @@ for skp in $GIT_SKIP_TESTS
do
case "$this_test" in
$skp)
- say_color skip >&3 "skipping test $this_test altogether"
+ say_color info >&3 "skipping test $this_test altogether"
skip_all="skip all tests in $this_test"
test_done
esac
^ 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