* Re: RFC: "git config -l" should not expose sensitive information
From: Andrew Ardill @ 2012-12-20 22:31 UTC (permalink / raw)
To: Junio C Hamano
Cc: Michael Haggerty, Jeff King, Toralf Förster,
git@vger.kernel.org
In-Reply-To: <7vbodo5zjj.fsf@alter.siamese.dyndns.org>
On 21 December 2012 02:49, Aaron Schrab <aaron@schrab.com> wrote:
> 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.
I understand that we've come down mostly on the 'users must check
before sending' side of things, but this point isn't necessarily true.
It wouldn't be too hard to create a config setting with a list of
'sensitive' keys filled with sensible defaults. It would be the job of
the 3rd party to add the relevant keys to this config file. This
wouldn't help with old 3rd party tools but would provide a way to
'hide' things automatically. A user could of course configure this
themselves (though one would think most who knew how wouldn't need
to).
On 21 December 2012 02:52, Jeff King <peff@peff.net> wrote:
>> 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.
One thing that a new option could provide (or maybe even the existing
option if it detects an interactive session) is to prompt the user to
review the content before outputting it. This is a nice way of helping
users who don't know that there might be sensitive information in the
output. Are there any use cases where prompting the user would be
annoying when using this command?
Regards,
Andrew Ardill
^ permalink raw reply
* Re: [PATCH] Highlight the link target line in Gitweb using CSS
From: Matthew Blissett @ 2012-12-20 22:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jakub Narębski
In-Reply-To: <7vbodo4f6q.fsf@alter.siamese.dyndns.org>
On 20 December 2012 20:54, Junio C Hamano <gitster@pobox.com> wrote:
> [jc: adding area expert to Cc]
Thanks.
> Matthew Blissett <matt@blissett.me.uk> writes:
>
> > 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.
>
> In the "blob" view, I think it does make it more discoverable that
> these line numbers are links, so I personally think a.linenr:hover
> part is an improvement. I am not sure about other three changes
> adding any value, though.
>
> > 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.
>
> Actually, when viewing the blame view, this is even worse, as it
> seems to always overlap. The background color ought to be enough
> cue without being overly distracting, I would have to say.
I didn't know about blame-view, sorry. The line-number links in that
view aren't self-referential, so the ⚓ symbol is misleading. I'm not
sure if that's a mistake, or if the links are supposed to point to the
commit that introduced the change. In any case, often they point to
the parent commit instead.
Just this:
/* Pink highlight when hovering line numbers or linking to them */
.pre a.linenr:hover,
.pre a.linenr:target {
color: #444444;
background-color: #f8f;
}
is probably best. A pink background, which shows up better than
yellow, and only in blob view. The :target background also helps mark
the chosen line after scrolling.
If blame view is supposed to have the same behaviour (self-referential
links) then these two CSS selectors are appropriate:
.blame tr:target .linenr a, .blame tr .linenr a:hover
--
Thanks,
Matt
^ permalink raw reply
* Re: [PATCH v6 0/7] make test output coloring more intuitive
From: Adam Spiers @ 2012-12-20 23:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git list
In-Reply-To: <7vy5gs4jiy.fsf@alter.siamese.dyndns.org>
On Thu, Dec 20, 2012 at 7:21 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 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).
OK.
> 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.
As Peff already noted, on many (most?) X terminals "bold" colours are
just brighter colours, rather than a heavier typeface. How bold they
look is therefore dependent on the colour scheme used by that
terminal.
> 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.
Very good point.
> 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
The comment still says "bold".
> warn)
> - tput bold; tput setaf 3;; # bold brown/yellow
> + tput setaf 3;; # bold brown/yellow
Ditto here ...
> pass)
> tput setaf 2;; # green
> info)
> - tput bold; tput setaf 6;; # bold cyan
> + tput setaf 6;; # bold cyan
... and here.
> *)
> 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
Yes, I like this last hunk especially.
I have no objection in principle to a reduction in boldness.
However, I am beginning to get disheartened that at this rate, this
series will never land. I already submitted v4 of the series which
already had non-bold blue. I then received feedback indicating that
bold blue would be more suitable, so despite alarm bells beginning to
ring in my head, I submitted v5 with bold blue, declaring that that
would be my last version:
http://article.gmane.org/gmane.comp.version-control.git/206042
A further concern about "info" messages not being blue prompted me
to attempt to canvass more opinions:
http://article.gmane.org/gmane.comp.version-control.git/209321
I received none, so submitted v6 based on my best judgement. Now we
are talking about a potential v7 going *back* to non-bold blue. I can
submit v7 if you think it's worth it, but would that really be the end
of the discussion? It's clear from the above that colour scheme
design by committee is about as good an idea as asking a bunch of kids
to reach consensus on their favourite colour ;-)
So if possible I'd be very happy for Junio to simply make an executive
decision (I don't care which way, as long as it fits the traffic
lights scheme and uses distinct hues of blue/cyan for the different
categories of skip/info messages), tweak the latest v6 series
accordingly, and then push so that we can all go back to more pressing
things ;-)
Hopefully that is a reasonable way forward?
Thanks,
Adam
^ permalink raw reply
* Re: [PATCH] Documentation/git-clean: Document --force --force
From: Soren Brinkmann @ 2012-12-20 23:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <b48ad5f0-25f7-45c3-b2dc-c0c01645a247@CO9EHSMHS031.ehs.local>
Ping?
On Thu, Dec 13, 2012 at 05:46:55PM -0800, Soren Brinkmann wrote:
> This patch documents the behavior of 'git clean' when
> encountering nested git repositories.
> Such repositories are only deleted if '-f' is passed twice
> to 'git clean'.
>
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> ---
> Documentation/git-clean.txt | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
> index 9f42c0d..0b31454 100644
> --- a/Documentation/git-clean.txt
> +++ b/Documentation/git-clean.txt
> @@ -23,6 +23,9 @@ example, be useful to remove all build products.
> If any optional `<path>...` arguments are given, only those paths
> are affected.
>
> +Nested git repositories are not removed unless the '-f' option is
> +passed to 'git clean' twice.
> +
> OPTIONS
> -------
> -d::
> @@ -35,6 +38,8 @@ OPTIONS
> --force::
> If the git configuration variable clean.requireForce is not set
> to false, 'git clean' will refuse to run unless given -f or -n.
> + Pass this option twice to 'git clean' in order to also remove
> + nested git repositories.
>
> -n::
> --dry-run::
> --
> 1.8.0.2
>
>
> --
> 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
* Noob Question
From: awingnut @ 2012-12-21 1:07 UTC (permalink / raw)
To: git
I have not used git yet but am planning to. I am trying to get my head
around how it will work and the documentation I found so far is of
modest help. I currently have a Java application developed using Eclipse
on Windows. However, the project is located on a Linux shared drive
which is my Eclipse workspace. I do my builds using ANT on the Linux
host. My main questions center around the git repository and accessing it.
1) Should I install git on Linux or Windows or does it matter?
2) How will my build scripts access the source? Will it be the same as
now (my scripts 'cd' to the Eclipse project directory and run there) or
do I need to add a wrapper to my script to check out the entire source
for the builds?
3) How do I move my current Eclipse project into git after I create the
empty repository? I can only find info on how to import git into Eclipse
not the other way around.
4) Do I need to checkout the entire project from Eclipse to modify and
test it or only the classes I want to change? Does the plugin get the
others as needed when I run the app within Eclipse for testing?
Thanks for any help understanding how I need to configure all this.
^ permalink raw reply
* [PATCH] Documentation/git-update-index: caution about tree objects
From: Greg Troxel @ 2012-12-21 1:35 UTC (permalink / raw)
To: git; +Cc: Greg Troxel
While one can add tree objects to the index, this is not currently
useful. Therefore, use "git ls-tree -r" as the example to be fed to
--index-info. Add a section explaining about expected index contents.
(Thanks to Junio for explaining this to me in August of 2011.)
Signed-off-by: Greg Troxel <gdt@ir.bbn.com>
---
Documentation/git-update-index.txt | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
index 9d0b151..6ce65fa 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -29,6 +29,11 @@ Modifies the index or directory cache. Each file mentioned is updated
into the index and any 'unmerged' or 'needs updating' state is
cleared.
+Note that update-index does not check that the modifications preserve
+the expected invariants. In particular, an index normally holds
+regular blobs, executable blobs, symlink blobs, and gitlinks.
+Therefore, adding a tree object is not likely useful.
+
See also linkgit:git-add[1] for a more user-friendly way to do some of
the most common operations on the index.
@@ -210,7 +215,7 @@ back on 3-way merge.
. mode SP type SP sha1 TAB path
+
-The second format is to stuff 'git ls-tree' output
+The second format is to stuff 'git ls-tree -r' output
into the index file.
. mode SP sha1 SP stage TAB path
--
1.8.0.1
^ permalink raw reply related
* Re: Noob Question
From: Andrew Ardill @ 2012-12-21 1:43 UTC (permalink / raw)
To: awingnut; +Cc: git@vger.kernel.org
In-Reply-To: <50D3B669.1030204@hotmail.com>
Hi!
On 21 December 2012 12:07, awingnut <wtriker.ffe@gmail.com> wrote:
> My main questions center around the git repository and accessing it.
The main thing you need to know is that you can work on your code base
in the *exact* same way while using git. You don't *have* to change
anything about how you work, as git's primary purpose is to store
snapshots of your work so that you have a history of what has changed.
That being said, you can (and maybe should) change how you work to
take into account the power of git. Most of what you do will stay the
same, however.
> 1) Should I install git on Linux or Windows or does it matter?
Install git wherever you need to access the code. From the sounds of
it you will want git on both machines, as you are working on windows
and but keeping the code on the linux shared drive. When working on
the windows machine you will use a windows copy of git to manipulate
the workspace, though I'm not sure if there are any gotchas with the
interaction with a linux shared drive.
If you want to manipulate the repository from the linux machine you
will need git on it as well.
Unless you're using a git server, manipulating the repository is a
local action and so is performed by the client. That is, when working
on windows use the windows client, if you also work on the linux
machine then you will need a client there as well.
> 2) How will my build scripts access the source? Will it be the same as
> now (my scripts 'cd' to the Eclipse project directory and run there) or
> do I need to add a wrapper to my script to check out the entire source
> for the builds?
It's the same as now. Git uses the concept of a 'work tree' to talk
about the actual files you are working on now. The work tree
corresponds exactly to your current project files. When you create a
git repository you gain the ability to store snapshots of this working
tree into the 'object store', as well as metadata about the snapshots,
so that you can restore that snapshot later.
Your actual files keep their current layout and format, until you change them.
> 3) How do I move my current Eclipse project into git after I create the
> empty repository? I can only find info on how to import git into Eclipse
> not the other way around.
You have two options. Create the git repository in the same location
as your Eclipse project. Navigate to the project folder using git bash
and do a 'git init' inside it; voila! you now have a git repository.
You can choose to create a 'remote' repository somewhere to store a
backup of your code as well, but this _still_ requires you to init a
local repository to backup.
The other option is to create a blank repository somewhere (anywhere)
and then tell that repository to use your Eclipse project as its
working tree. The benefit to doing this is being able to keep your
snapshots and metadata in a different location to your working
directory (say keep the snapshots on a local windows drive while your
working directory is on the linux share). Unless you shouldn't or
aren't able to create the repository within the Eclipse project, I
would recommend against this.
> 4) Do I need to checkout the entire project from Eclipse to modify and
> test it or only the classes I want to change? Does the plugin get the
> others as needed when I run the app within Eclipse for testing?
Not sure exactly what you are asking here, but in general people will
'clone' an entire repository including all its history. If you want to
update only certain files that is fine, but the commit object stores
the state of the entire tree of files. Note that a commit object does
_not_ store the difference between two snapshots, but stores the
entire state of the files. You can grab a file from a given snapshot
and test that along side files from a second snapshot, but if you
wanted to commit the resulting tree to the repository it would store a
third snapshot containing the exact state of all files.
Hopefully that clears it up for you?
Regards,
Andrew Ardill
^ permalink raw reply
* Re: [PATCH] Documentation/git-update-index: caution about tree objects
From: Junio C Hamano @ 2012-12-21 2:56 UTC (permalink / raw)
To: Greg Troxel; +Cc: git
In-Reply-To: <1356053738-14926-1-git-send-email-gdt@ir.bbn.com>
Greg Troxel <gdt@ir.bbn.com> writes:
> While one can add tree objects to the index, this is not currently
> useful. Therefore, use "git ls-tree -r" as the example to be fed to
> --index-info. Add a section explaining about expected index contents.
> (Thanks to Junio for explaining this to me in August of 2011.)
>
> Signed-off-by: Greg Troxel <gdt@ir.bbn.com>
> ---
> Documentation/git-update-index.txt | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
> index 9d0b151..6ce65fa 100644
> --- a/Documentation/git-update-index.txt
> +++ b/Documentation/git-update-index.txt
> @@ -29,6 +29,11 @@ Modifies the index or directory cache. Each file mentioned is updated
> into the index and any 'unmerged' or 'needs updating' state is
> cleared.
>
> +Note that update-index does not check that the modifications preserve
> +the expected invariants. In particular, an index normally holds
> +regular blobs, executable blobs, symlink blobs, and gitlinks.
> +Therefore, adding a tree object is not likely useful.
> +
I find this unnecessarily alarmist as a description meant for
general audiences. For the normal mode of operations of the command
(e.g. "git update-index --add --remove hello.c"), whatever you mean
by "expected invariants" are fully preserved.
I think you meant this for --cacheinfo and --index-info options,
which are primarily meant for people who know what they are doing
(that includes the use of this command in scripted Porceains) or Git
developers who want to work on enhancing the index (and to them,
being able to record anything is more convenient).
> @@ -210,7 +215,7 @@ back on 3-way merge.
>
> . mode SP type SP sha1 TAB path
> +
> -The second format is to stuff 'git ls-tree' output
> +The second format is to stuff 'git ls-tree -r' output
> into the index file.
This hunk is good.
Thanks.
^ permalink raw reply
* Re: [PATCH] Documentation/git-clean: Document --force --force
From: Junio C Hamano @ 2012-12-21 3:01 UTC (permalink / raw)
To: Soren Brinkmann; +Cc: git
In-Reply-To: <335802a0-38b5-4cbc-9e52-92c92083119e@VA3EHSMHS005.ehs.local>
Soren Brinkmann <soren.brinkmann@xilinx.com> writes:
> Ping?
I *think* it is a mistake for the command to remove a separate
project repository within, with any number of "-f", so I'd rather
see a patch to fix it, instead of casting such a misbehaviour as a
feature in stone by documenting it.
I dunno.
> On Thu, Dec 13, 2012 at 05:46:55PM -0800, Soren Brinkmann wrote:
>> This patch documents the behavior of 'git clean' when
>> encountering nested git repositories.
>> Such repositories are only deleted if '-f' is passed twice
>> to 'git clean'.
>>
>> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
>> ---
>> Documentation/git-clean.txt | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt
>> index 9f42c0d..0b31454 100644
>> --- a/Documentation/git-clean.txt
>> +++ b/Documentation/git-clean.txt
>> @@ -23,6 +23,9 @@ example, be useful to remove all build products.
>> If any optional `<path>...` arguments are given, only those paths
>> are affected.
>>
>> +Nested git repositories are not removed unless the '-f' option is
>> +passed to 'git clean' twice.
>> +
>> OPTIONS
>> -------
>> -d::
>> @@ -35,6 +38,8 @@ OPTIONS
>> --force::
>> If the git configuration variable clean.requireForce is not set
>> to false, 'git clean' will refuse to run unless given -f or -n.
>> + Pass this option twice to 'git clean' in order to also remove
>> + nested git repositories.
>>
>> -n::
>> --dry-run::
>> --
>> 1.8.0.2
>>
>>
>> --
>> 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
* [PATCH v7 0/7] coloring test output after traffic signal
From: Junio C Hamano @ 2012-12-21 3:12 UTC (permalink / raw)
To: git; +Cc: Adam Spiers, Jeff King
In-Reply-To: <CAOkDyE9tDYRYzojzNnjWsT7UygxMAurHqLSDGA66_LMPD2Wmnw@mail.gmail.com>
To conclude the bikeshedding discussion we had today, here is what I
queued by squashing stuff into relevant patches, so that people can
eyeball the result for the last time.
Adam Spiers (7):
tests: test number comes first in 'not ok $count - $message'
tests: paint known breakages in yellow
tests: paint skipped tests in blue
tests: change info messages from yellow/brown to cyan
tests: refactor mechanics of testing in a sub test-lib
tests: test the test framework more thoroughly
tests: paint unexpectedly fixed known breakages in bold red
t/t0000-basic.sh | 214 ++++++++++++++++++++++++++++++++++++++++++-------------
t/test-lib.sh | 29 +++++---
2 files changed, 184 insertions(+), 59 deletions(-)
--
1.8.1.rc2.225.g8d36ab4
^ permalink raw reply
* [PATCH v7 1/7] tests: test number comes first in 'not ok $count - $message'
From: Junio C Hamano @ 2012-12-21 3:12 UTC (permalink / raw)
To: git; +Cc: Adam Spiers, Jeff King
In-Reply-To: <CAOkDyE9tDYRYzojzNnjWsT7UygxMAurHqLSDGA66_LMPD2Wmnw@mail.gmail.com>
From: Adam Spiers <git@adamspiers.org>
The old output to say "not ok - 1 messsage" was working by accident
only because the test numbers are optional in TAP.
Signed-off-by: Adam Spiers <git@adamspiers.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t0000-basic.sh | 4 ++--
t/test-lib.sh | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index ae6a3f0..c6b42de 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -167,13 +167,13 @@ test_expect_success 'tests clean up even on failures' "
! test -s err &&
! test -f \"trash directory.failing-cleanup/clean-after-failure\" &&
sed -e 's/Z$//' -e 's/^> //' >expect <<-\\EOF &&
- > not ok - 1 tests clean up even after a failure
+ > not ok 1 - tests clean up even after a failure
> # Z
> # touch clean-after-failure &&
> # test_when_finished rm clean-after-failure &&
> # (exit 1)
> # Z
- > not ok - 2 failure to clean up causes the test to fail
+ > not ok 2 - failure to clean up causes the test to fail
> # Z
> # test_when_finished \"(exit 2)\"
> # Z
diff --git a/t/test-lib.sh b/t/test-lib.sh
index f8e3733..03b86b8 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -268,7 +268,7 @@ test_ok_ () {
test_failure_ () {
test_failure=$(($test_failure + 1))
- say_color error "not ok - $test_count $1"
+ say_color error "not ok $test_count - $1"
shift
echo "$@" | sed -e 's/^/# /'
test "$immediate" = "" || { GIT_EXIT_OK=t; exit 1; }
--
1.8.1.rc2.225.g8d36ab4
^ permalink raw reply related
* [PATCH v7 2/7] tests: paint known breakages in yellow
From: Junio C Hamano @ 2012-12-21 3:12 UTC (permalink / raw)
To: git; +Cc: Adam Spiers, Jeff King
In-Reply-To: <CAOkDyE9tDYRYzojzNnjWsT7UygxMAurHqLSDGA66_LMPD2Wmnw@mail.gmail.com>
From: Adam Spiers <git@adamspiers.org>
Yellow seems a more appropriate color than bold green when
considering the universal traffic lights coloring scheme, where
green conveys the impression that everything's OK, and amber that
something's not quite right.
Likewise, change the color of the summarized total number of known
breakages from bold red to the same yellow to be less alarmist and
more consistent with the above.
An earlier version of this patch used bold yellow but because these
are all long-known failures, reminding them to developers in bold
over and over does not help encouraging them to take a look at them
very much. This iteration paints them in plain yellow instead to
make them less distracting.
Signed-off-by: Adam Spiers <git@adamspiers.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/test-lib.sh | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 03b86b8..72aafd0 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -183,6 +183,8 @@ then
tput bold; tput setaf 1;; # bold red
skip)
tput bold; tput setaf 2;; # bold green
+ warn)
+ tput setaf 3;; # brown/yellow
pass)
tput setaf 2;; # green
info)
@@ -281,7 +283,7 @@ test_known_broken_ok_ () {
test_known_broken_failure_ () {
test_broken=$(($test_broken+1))
- say_color skip "not ok $test_count - $@ # TODO known breakage"
+ say_color warn "not ok $test_count - $@ # TODO known breakage"
}
test_debug () {
@@ -375,7 +377,7 @@ test_done () {
fi
if test "$test_broken" != 0
then
- say_color error "# still have $test_broken known breakage(s)"
+ say_color warn "# still have $test_broken known breakage(s)"
msg="remaining $(($test_count-$test_broken)) test(s)"
else
msg="$test_count test(s)"
--
1.8.1.rc2.225.g8d36ab4
^ permalink raw reply related
* [PATCH v7 3/7] tests: paint skipped tests in blue
From: Junio C Hamano @ 2012-12-21 3:12 UTC (permalink / raw)
To: git; +Cc: Adam Spiers, Jeff King
In-Reply-To: <CAOkDyE9tDYRYzojzNnjWsT7UygxMAurHqLSDGA66_LMPD2Wmnw@mail.gmail.com>
From: Adam Spiers <git@adamspiers.org>
Skipped tests indicate incomplete test coverage. Whilst this is not a
test failure or other error, it's still not a complete success.
Other testsuite related software like automake, autotest and prove
seem to use blue for skipped tests, so let's follow suit.
Signed-off-by: Adam Spiers <git@adamspiers.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/test-lib.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 72aafd0..f32df80 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -182,7 +182,7 @@ then
error)
tput bold; tput setaf 1;; # bold red
skip)
- tput bold; tput setaf 2;; # bold green
+ tput setaf 4;; # blue
warn)
tput setaf 3;; # brown/yellow
pass)
--
1.8.1.rc2.225.g8d36ab4
^ permalink raw reply related
* [PATCH v7 4/7] tests: change info messages from yellow/brown to cyan
From: Junio C Hamano @ 2012-12-21 3:12 UTC (permalink / raw)
To: git; +Cc: Adam Spiers, Jeff King
In-Reply-To: <CAOkDyE9tDYRYzojzNnjWsT7UygxMAurHqLSDGA66_LMPD2Wmnw@mail.gmail.com>
From: Adam Spiers <git@adamspiers.org>
Now that we've adopted a "traffic lights" coloring scheme, yellow is
used for warning messages, so we need to re-color info messages to
something less alarmist. Blue is a universal color for informational
messages; however we are using that for skipped tests in order to
align with the color schemes of other test suites. Therefore we use
cyan which is also blue-ish, but visually distinct from blue.
This was suggested on the list a while ago and no-one raised any
objections:
http://thread.gmane.org/gmane.comp.version-control.git/205675/focus=205966
An earlier iteration of this patch used bold cyan, but the point of
this change is to make them less alarming; let's drop the boldness.
Also paint the message to report skipping the whole thing via
GIT_SKIP_TESTS mechanism in the same color as the "info" color
that is used on the final summary line for the entire script.
Signed-off-by: Adam Spiers <git@adamspiers.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/test-lib.sh | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index f32df80..8b75c9a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -186,9 +186,9 @@ then
warn)
tput setaf 3;; # brown/yellow
pass)
- tput setaf 2;; # green
+ tput setaf 2;; # green
info)
- tput setaf 3;; # brown
+ tput setaf 6;; # cyan
*)
test -n "$quiet" && return;;
esac
@@ -584,7 +584,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
--
1.8.1.rc2.225.g8d36ab4
^ permalink raw reply related
* [PATCH v7 5/7] tests: refactor mechanics of testing in a sub test-lib
From: Junio C Hamano @ 2012-12-21 3:12 UTC (permalink / raw)
To: git; +Cc: Adam Spiers, Jeff King
In-Reply-To: <CAOkDyE9tDYRYzojzNnjWsT7UygxMAurHqLSDGA66_LMPD2Wmnw@mail.gmail.com>
From: Adam Spiers <git@adamspiers.org>
This will allow us to test the test framework more thoroughly
without disrupting the top-level test metrics.
Signed-off-by: Adam Spiers <git@adamspiers.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t0000-basic.sh | 85 ++++++++++++++++++++++++++------------------------------
1 file changed, 40 insertions(+), 45 deletions(-)
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index c6b42de..d0f46e8 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -55,39 +55,53 @@ test_expect_failure 'pretend we have a known breakage' '
false
'
-test_expect_success 'pretend we have fixed a known breakage (run in sub test-lib)' "
- mkdir passing-todo &&
- (cd passing-todo &&
- cat >passing-todo.sh <<-EOF &&
- #!$SHELL_PATH
-
- test_description='A passing TODO test
-
- This is run in a sub test-lib so that we do not get incorrect
- passing metrics
- '
-
- # Point to the t/test-lib.sh, which isn't in ../ as usual
- TEST_DIRECTORY=\"$TEST_DIRECTORY\"
- . \"\$TEST_DIRECTORY\"/test-lib.sh
+run_sub_test_lib_test () {
+ name="$1" descr="$2" # stdin is the body of the test code
+ mkdir "$name" &&
+ (
+ cd "$name" &&
+ cat >"$name.sh" <<-EOF &&
+ #!$SHELL_PATH
+
+ test_description='$descr (run in sub test-lib)
+
+ This is run in a sub test-lib so that we do not get incorrect
+ passing metrics
+ '
+
+ # Point to the t/test-lib.sh, which isn't in ../ as usual
+ . "\$TEST_DIRECTORY"/test-lib.sh
+ EOF
+ cat >>"$name.sh" &&
+ chmod +x "$name.sh" &&
+ export TEST_DIRECTORY &&
+ ./"$name.sh" >out 2>err
+ )
+}
- test_expect_failure 'pretend we have fixed a known breakage' '
- :
- '
+check_sub_test_lib_test () {
+ name="$1" # stdin is the expected output from the test
+ (
+ cd "$name" &&
+ ! test -s err &&
+ sed -e 's/^> //' -e 's/Z$//' >expect &&
+ test_cmp expect out
+ )
+}
+test_expect_success 'pretend we have fixed a known breakage' "
+ run_sub_test_lib_test passing-todo 'A passing TODO test' <<-\\EOF &&
+ test_expect_failure 'pretend we have fixed a known breakage' 'true'
test_done
EOF
- chmod +x passing-todo.sh &&
- ./passing-todo.sh >out 2>err &&
- ! test -s err &&
- sed -e 's/^> //' >expect <<-\\EOF &&
+ check_sub_test_lib_test passing-todo <<-\\EOF
> ok 1 - pretend we have fixed a known breakage # TODO known breakage
> # fixed 1 known breakage(s)
> # passed all 1 test(s)
> 1..1
EOF
- test_cmp expect out)
"
+
test_set_prereq HAVEIT
haveit=no
test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '
@@ -137,19 +151,8 @@ then
fi
test_expect_success 'tests clean up even on failures' "
- mkdir failing-cleanup &&
- (
- cd failing-cleanup &&
-
- cat >failing-cleanup.sh <<-EOF &&
- #!$SHELL_PATH
-
- test_description='Failing tests with cleanup commands'
-
- # Point to the t/test-lib.sh, which isn't in ../ as usual
- TEST_DIRECTORY=\"$TEST_DIRECTORY\"
- . \"\$TEST_DIRECTORY\"/test-lib.sh
-
+ test_must_fail run_sub_test_lib_test \
+ failing-cleanup 'Failing tests with cleanup commands' <<-\\EOF &&
test_expect_success 'tests clean up even after a failure' '
touch clean-after-failure &&
test_when_finished rm clean-after-failure &&
@@ -159,14 +162,8 @@ test_expect_success 'tests clean up even on failures' "
test_when_finished \"(exit 2)\"
'
test_done
-
EOF
-
- chmod +x failing-cleanup.sh &&
- test_must_fail ./failing-cleanup.sh >out 2>err &&
- ! test -s err &&
- ! test -f \"trash directory.failing-cleanup/clean-after-failure\" &&
- sed -e 's/Z$//' -e 's/^> //' >expect <<-\\EOF &&
+ check_sub_test_lib_test failing-cleanup <<-\\EOF
> not ok 1 - tests clean up even after a failure
> # Z
> # touch clean-after-failure &&
@@ -180,8 +177,6 @@ test_expect_success 'tests clean up even on failures' "
> # failed 2 among 2 test(s)
> 1..2
EOF
- test_cmp expect out
- )
"
################################################################
--
1.8.1.rc2.225.g8d36ab4
^ permalink raw reply related
* [PATCH v7 7/7] tests: paint unexpectedly fixed known breakages in bold red
From: Junio C Hamano @ 2012-12-21 3:12 UTC (permalink / raw)
To: git; +Cc: Adam Spiers, Jeff King
In-Reply-To: <CAOkDyE9tDYRYzojzNnjWsT7UygxMAurHqLSDGA66_LMPD2Wmnw@mail.gmail.com>
From: Adam Spiers <git@adamspiers.org>
Change color of unexpectedly fixed known breakages to bold red. An
unexpectedly passing test indicates that the test code is somehow
broken or out of sync with the code it is testing. Either way this is
an error which is potentially as bad as a failing test, and as such is
no longer portrayed as a pass in the output.
Signed-off-by: Adam Spiers <git@adamspiers.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t0000-basic.sh | 30 ++++++++++++++++++++++++------
t/test-lib.sh | 13 +++++++++----
2 files changed, 33 insertions(+), 10 deletions(-)
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 384b0ae..8973d2c 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -145,13 +145,31 @@ test_expect_success 'pretend we have fixed a known breakage' "
test_done
EOF
check_sub_test_lib_test passing-todo <<-\\EOF
- > ok 1 - pretend we have fixed a known breakage # TODO known breakage
- > # fixed 1 known breakage(s)
- > # passed all 1 test(s)
+ > ok 1 - pretend we have fixed a known breakage # TODO known breakage vanished
+ > # 1 known breakage(s) vanished; please update test(s)
> 1..1
EOF
"
+test_expect_success 'pretend we have fixed one of two known breakages (run in sub test-lib)' "
+ run_sub_test_lib_test partially-passing-todos \
+ '2 TODO tests, one passing' <<-\\EOF &&
+ test_expect_failure 'pretend we have a known breakage' 'false'
+ test_expect_success 'pretend we have a passing test' 'true'
+ test_expect_failure 'pretend we have fixed another known breakage' 'true'
+ test_done
+ EOF
+ check_sub_test_lib_test partially-passing-todos <<-\\EOF
+ > not ok 1 - pretend we have a known breakage # TODO known breakage
+ > ok 2 - pretend we have a passing test
+ > ok 3 - pretend we have fixed another known breakage # TODO known breakage vanished
+ > # 1 known breakage(s) vanished; please update test(s)
+ > # still have 1 known breakage(s)
+ > # passed all remaining 1 test(s)
+ > 1..3
+ EOF
+"
+
test_expect_success 'pretend we have a pass, fail, and known breakage' "
test_must_fail run_sub_test_lib_test \
mixed-results1 'mixed results #1' <<-\\EOF &&
@@ -199,10 +217,10 @@ test_expect_success 'pretend we have a mix of all possible results' "
> # false
> not ok 8 - pretend we have a known breakage # TODO known breakage
> not ok 9 - pretend we have a known breakage # TODO known breakage
- > ok 10 - pretend we have fixed a known breakage # TODO known breakage
- > # fixed 1 known breakage(s)
+ > ok 10 - pretend we have fixed a known breakage # TODO known breakage vanished
+ > # 1 known breakage(s) vanished; please update test(s)
> # still have 2 known breakage(s)
- > # failed 3 among remaining 8 test(s)
+ > # failed 3 among remaining 7 test(s)
> 1..10
EOF
"
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 8b75c9a..30a0937 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -278,7 +278,7 @@ test_failure_ () {
test_known_broken_ok_ () {
test_fixed=$(($test_fixed+1))
- say_color "" "ok $test_count - $@ # TODO known breakage"
+ say_color error "ok $test_count - $@ # TODO known breakage vanished"
}
test_known_broken_failure_ () {
@@ -373,13 +373,18 @@ test_done () {
if test "$test_fixed" != 0
then
- say_color pass "# fixed $test_fixed known breakage(s)"
+ say_color error "# $test_fixed known breakage(s) vanished; please update test(s)"
fi
if test "$test_broken" != 0
then
say_color warn "# still have $test_broken known breakage(s)"
- msg="remaining $(($test_count-$test_broken)) test(s)"
+ fi
+ if test "$test_broken" != 0 || test "$test_fixed" != 0
+ then
+ test_remaining=$(( $test_count - $test_broken - $test_fixed ))
+ msg="remaining $test_remaining test(s)"
else
+ test_remaining=$test_count
msg="$test_count test(s)"
fi
case "$test_failure" in
@@ -393,7 +398,7 @@ test_done () {
if test $test_external_has_tap -eq 0
then
- if test $test_count -gt 0
+ if test $test_remaining -gt 0
then
say_color pass "# passed all $msg"
fi
--
1.8.1.rc2.225.g8d36ab4
^ permalink raw reply related
* [PATCH v7 6/7] tests: test the test framework more thoroughly
From: Junio C Hamano @ 2012-12-21 3:12 UTC (permalink / raw)
To: git; +Cc: Adam Spiers, Jeff King
In-Reply-To: <CAOkDyE9tDYRYzojzNnjWsT7UygxMAurHqLSDGA66_LMPD2Wmnw@mail.gmail.com>
From: Adam Spiers <git@adamspiers.org>
Add 5 new full test suite runs each with a different number of
passing/failing/broken/fixed tests, in order to ensure that the
correct exit code and output are generated in each case. As before,
these are run in a subdirectory to avoid disrupting the metrics for
the parent tests.
Signed-off-by: Adam Spiers <git@adamspiers.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t0000-basic.sh | 105 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 105 insertions(+)
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index d0f46e8..384b0ae 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -89,6 +89,56 @@ check_sub_test_lib_test () {
)
}
+test_expect_success 'pretend we have a fully passing test suite' "
+ run_sub_test_lib_test full-pass '3 passing tests' <<-\\EOF &&
+ for i in 1 2 3
+ do
+ test_expect_success \"passing test #\$i\" 'true'
+ done
+ test_done
+ EOF
+ check_sub_test_lib_test full-pass <<-\\EOF
+ > ok 1 - passing test #1
+ > ok 2 - passing test #2
+ > ok 3 - passing test #3
+ > # passed all 3 test(s)
+ > 1..3
+ EOF
+"
+
+test_expect_success 'pretend we have a partially passing test suite' "
+ test_must_fail run_sub_test_lib_test \
+ partial-pass '2/3 tests passing' <<-\\EOF &&
+ test_expect_success 'passing test #1' 'true'
+ test_expect_success 'failing test #2' 'false'
+ test_expect_success 'passing test #3' 'true'
+ test_done
+ EOF
+ check_sub_test_lib_test partial-pass <<-\\EOF
+ > ok 1 - passing test #1
+ > not ok 2 - failing test #2
+ # false
+ > ok 3 - passing test #3
+ > # failed 1 among 3 test(s)
+ > 1..3
+ EOF
+"
+
+test_expect_success 'pretend we have a known breakage' "
+ run_sub_test_lib_test failing-todo 'A failing TODO test' <<-\\EOF &&
+ test_expect_success 'passing test' 'true'
+ test_expect_failure 'pretend we have a known breakage' 'false'
+ test_done
+ EOF
+ check_sub_test_lib_test failing-todo <<-\\EOF
+ > ok 1 - passing test
+ > not ok 2 - pretend we have a known breakage # TODO known breakage
+ > # still have 1 known breakage(s)
+ > # passed all remaining 1 test(s)
+ > 1..2
+ EOF
+"
+
test_expect_success 'pretend we have fixed a known breakage' "
run_sub_test_lib_test passing-todo 'A passing TODO test' <<-\\EOF &&
test_expect_failure 'pretend we have fixed a known breakage' 'true'
@@ -102,6 +152,61 @@ test_expect_success 'pretend we have fixed a known breakage' "
EOF
"
+test_expect_success 'pretend we have a pass, fail, and known breakage' "
+ test_must_fail run_sub_test_lib_test \
+ mixed-results1 'mixed results #1' <<-\\EOF &&
+ test_expect_success 'passing test' 'true'
+ test_expect_success 'failing test' 'false'
+ test_expect_failure 'pretend we have a known breakage' 'false'
+ test_done
+ EOF
+ check_sub_test_lib_test mixed-results1 <<-\\EOF
+ > ok 1 - passing test
+ > not ok 2 - failing test
+ > # false
+ > not ok 3 - pretend we have a known breakage # TODO known breakage
+ > # still have 1 known breakage(s)
+ > # failed 1 among remaining 2 test(s)
+ > 1..3
+ EOF
+"
+
+test_expect_success 'pretend we have a mix of all possible results' "
+ test_must_fail run_sub_test_lib_test \
+ mixed-results2 'mixed results #2' <<-\\EOF &&
+ test_expect_success 'passing test' 'true'
+ test_expect_success 'passing test' 'true'
+ test_expect_success 'passing test' 'true'
+ test_expect_success 'passing test' 'true'
+ test_expect_success 'failing test' 'false'
+ test_expect_success 'failing test' 'false'
+ test_expect_success 'failing test' 'false'
+ test_expect_failure 'pretend we have a known breakage' 'false'
+ test_expect_failure 'pretend we have a known breakage' 'false'
+ test_expect_failure 'pretend we have fixed a known breakage' 'true'
+ test_done
+ EOF
+ check_sub_test_lib_test mixed-results2 <<-\\EOF
+ > ok 1 - passing test
+ > ok 2 - passing test
+ > ok 3 - passing test
+ > ok 4 - passing test
+ > not ok 5 - failing test
+ > # false
+ > not ok 6 - failing test
+ > # false
+ > not ok 7 - failing test
+ > # false
+ > not ok 8 - pretend we have a known breakage # TODO known breakage
+ > not ok 9 - pretend we have a known breakage # TODO known breakage
+ > ok 10 - pretend we have fixed a known breakage # TODO known breakage
+ > # fixed 1 known breakage(s)
+ > # still have 2 known breakage(s)
+ > # failed 3 among remaining 8 test(s)
+ > 1..10
+ EOF
+"
+
test_set_prereq HAVEIT
haveit=no
test_expect_success HAVEIT 'test runs if prerequisite is satisfied' '
--
1.8.1.rc2.225.g8d36ab4
^ permalink raw reply related
* Re: [PATCH] Python scripts audited for minimum compatible version and checks added.
From: Junio C Hamano @ 2012-12-21 3:38 UTC (permalink / raw)
To: esr; +Cc: Jeff King, git
In-Reply-To: <7vsj7060nj.fsf@alter.siamese.dyndns.org>
I needed something like this on top of it to get it pass t5800.
diff --git a/git_remote_helpers/git/__init__.py b/git_remote_helpers/git/__init__.py
index 776e891..5047fd4 100644
--- a/git_remote_helpers/git/__init__.py
+++ b/git_remote_helpers/git/__init__.py
@@ -1,3 +1,5 @@
+import sys
+
if sys.hexversion < 0x02040000:
# The limiter is the subprocess module
sys.stderr.write("git_remote_helpers: requires Python 2.4 or later.")
--
1.8.1.rc2.225.g0e05fff
^ permalink raw reply related
* [PATCH v2] mergetools/p4merge: Honor $TMPDIR for the /dev/null placeholder
From: David Aguilar @ 2012-12-21 6:57 UTC (permalink / raw)
To: Junio C Hamano, Jeremy Morton; +Cc: git
Use $TMPDIR when creating the /dev/null placeholder for p4merge.
This keeps it out of the current directory.
Reported-by: Jeremy Morton <admin@game-point.net>
Signed-off-by: David Aguilar <davvid@gmail.com>
---
No mktemp usage in this round.
mergetools/p4merge | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/mergetools/p4merge b/mergetools/p4merge
index 295361a..52f7c8f 100644
--- a/mergetools/p4merge
+++ b/mergetools/p4merge
@@ -1,29 +1,21 @@
diff_cmd () {
+ empty_file=
+
# p4merge does not like /dev/null
- rm_local=
- rm_remote=
if test "/dev/null" = "$LOCAL"
then
- LOCAL="./p4merge-dev-null.LOCAL.$$"
- >"$LOCAL"
- rm_local=true
+ LOCAL="$(create_empty_file)"
fi
if test "/dev/null" = "$REMOTE"
then
- REMOTE="./p4merge-dev-null.REMOTE.$$"
- >"$REMOTE"
- rm_remote=true
+ REMOTE="$(create_empty_file)"
fi
"$merge_tool_path" "$LOCAL" "$REMOTE"
- if test -n "$rm_local"
- then
- rm -f "$LOCAL"
- fi
- if test -n "$rm_remote"
+ if test -n "$empty_file"
then
- rm -f "$REMOTE"
+ rm -f "$empty_file"
fi
}
@@ -33,3 +25,10 @@ merge_cmd () {
"$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" "$MERGED"
check_unchanged
}
+
+create_empty_file () {
+ empty_file="${TMPDIR:-/tmp}/git-difftool-p4merge-empty-file.$$"
+ >"$empty_file"
+
+ printf "$empty_file"
+}
--
1.8.1.rc2.6.g18499ba
^ permalink raw reply related
* RE: Python version auditing followup
From: Joachim Schmitz @ 2012-12-21 7:26 UTC (permalink / raw)
To: 'Junio C Hamano'; +Cc: git
In-Reply-To: <7vzk182yka.fsf@alter.siamese.dyndns.org>
> From: Junio C Hamano [mailto:gitster@pobox.com]
> Sent: Thursday, December 20, 2012 10:39 PM
> To: Joachim Schmitz
> Cc: git@vger.kernel.org
> Subject: Re: Python version auditing followup
>
> "Joachim Schmitz" <jojo@schmitz-digital.de> writes:
>
> > Junio C Hamano wrote:
> >> 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?
> >
> > We have a working 2.4.2 for HP-NonStop and some major problems getting
> > 2.7.3 to work.
>
> I do not think a platform that stops at 2.4.2 instead of going to
> higher 2.4.X series deserves to be called "long term maintained by
> their vendors". It sounds more like "attempted to supply 2.4.X and
> abandoned the users once one port was done" to me.
Well, not entirely wrong, but not all true at too.
I guess I need to defend the vendor here: It is not really the Vendor (HP) that provided Python 2.4.2 or tries to provide 2.7.3, it
is a volunteer and community effort. HP did sponsor the 2.4.2 port though (by allowing an HP employee to do the port inn his regular
working hours). It is not doing this any longer (since 2007). Since then it is a small group doing this on a purely voluntary basis
in their spare time (one HP employee amongst them, me).
Same goes for the git port BTW. And for every other port provided on http://ituglib.connect-cummunity.org (this machine is sponsored
by HP).
Almost every other port, as some pretty recently made it into the officially supported O/S version, so far: Samba, bash, coreutils,
vim, gzip, bzip2, Perl, PHP.
Bye, Jojo
^ permalink raw reply
* [PATCH] refs: do not use cached refs in repack_without_ref
From: Jeff King @ 2012-12-21 8:04 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
When we delete a ref that is packed, we rewrite the whole
packed-refs file and simply omit the ref that no longer
exists. However, we base the rewrite on whatever happens to
be in our refs cache, not what is necessarily on disk. That
opens us up to a race condition if another process is
simultaneously packing the refs, as we will overwrite their
newly-made pack-refs file with our potentially stale data,
losing commits.
You can demonstrate the race like this:
# setup some repositories
git init --bare parent &&
(cd parent && git config core.logallrefupdates true) &&
git clone parent child &&
(cd child && git commit --allow-empty -m base)
# in one terminal, repack the refs repeatedly
cd parent &&
while true; do
git pack-refs --all
done
# in another terminal, simultaneously push updates to
# master, and create and delete an unrelated ref
cd child &&
while true; do
git push origin HEAD:newbranch &&
git commit --allow-empty -m foo
us=`git rev-parse master` &&
git push origin master &&
git push origin :newbranch &&
them=`git --git-dir=../parent rev-parse master` &&
if test "$them" != "$us"; then
echo >&2 "$them" != "$us"
exit 1
fi
done
In many cases the two processes will conflict over locking
the packed-refs file, and the deletion of newbranch will
simply fail. But eventually you will hit the race, which
happens like this:
1. We push a new commit to master. It is already packed
(from the looping pack-refs call). We write the new
value (let us call it B) to $GIT_DIR/refs/heads/master,
but the old value (call it A) remains in the
packed-refs file.
2. We push the deletion of newbranch, spawning a
receive-pack process. Receive-pack advertises all refs
to the client, causing it to iterate over each ref; it
caches the packed refs in memory, which points at the
stale value A.
3. Meanwhile, a separate pack-refs process is running. It
runs to completion, updating the packed-refs file to
point master at B, and deleting $GIT_DIR/refs/heads/master
which also pointed at B.
4. Back in the receive-pack process, we get the
instruction to delete :newbranch. We take a lock on
packed-refs (which works, as the other pack-refs
process has already finished). We then rewrite the
contents using the cached refs, which contain the stale
value A.
The resulting packed-refs file points master once again at
A. The loose ref which would override it to point at B was
deleted (rightfully) in step 3. As a result, master now
points at A. The only trace that B ever existed in the
parent is in the reflog: the final entry will show master
moving from A to B, even though the ref still points at A
(so you can detect this race after the fact, because the
next reflog entry will move from A to C).
We can fix this by invalidating the packed-refs cache after
we have taken the lock. This means that we will re-read the
packed-refs file, and since we have the lock, we will be
sure that what we read will be atomically up-to-date when we
write (it may be out of date with respect to loose refs, but
that is OK, as loose refs take precedence).
Signed-off-by: Jeff King <peff@peff.net>
---
We actually see this in practice on GitHub, though it is relatively
rare (I've been chasing reports for a while, and in a very busy repo, it
can happen every couple of weeks; this is probably due to the fact that
we run "git gc" very frequently).
There are a few other interesting races in this code that this does not
fix:
1. We check to see whether the ref is packed based on the cached data.
That means that in the following sequence:
a. receive-pack starts, caches packed refs; master is not packed
b. meanwhile, pack-refs runs and packs master
c. receive-pack deletes the loose ref for master (which might be
a no-op if the pack-refs from (b) got there first). It checks
its cached packed-refs and sees that there is nothing to
delete.
We end up leaving the entry in packed-refs. In other words, the
deletion does not actually delete anything, but it still returns
success.
We could fix this by scanning the list of packed refs only after
we've acquired the lock. The downside is that this would increase
lock contention on packed-refs.lock. Right now, two deletions may
conflict if they are deletions of packed refs. With this change,
any two deletions might conflict, packed or not.
If we work under the assumption that deletions are relatively rare,
this is probably OK. And if you tend to keep your refs packed, it
would not make any difference. It would have an impact on repos
which do not pack refs, and which have frequent simultaneous
deletions.
2. The delete_ref function first deletes the loose ref, then rewrites
the packed-refs file. This means that for a moment, the ref may
appear to have rewound to whatever was in the packed-refs file, and
the reader has no way of knowing.
This is not a huge deal, but I think it could be fixed by swapping
the ordering. However, I think that would open us up to the reverse
race from above: we delete from packed-refs, then before we delete
the loose ref, a pack-refs process repacks it. Our deletion looks
successful, but the ref remains afterwards.
I fixed just the race I did because it does not (as far as I can tell)
have any downsides. And it is way more severe (the other ones are "a
deleted ref might come back", whereas the fixed one will actually lose
commits).
refs.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index 6cec1c8..541fec2 100644
--- a/refs.c
+++ b/refs.c
@@ -1744,7 +1744,8 @@ static int repack_without_ref(const char *refname)
static int repack_without_ref(const char *refname)
{
struct repack_without_ref_sb data;
- struct ref_dir *packed = get_packed_refs(get_ref_cache(NULL));
+ struct ref_cache *refs = get_ref_cache(NULL);
+ struct ref_dir *packed = get_packed_refs(refs);
if (find_ref(packed, refname) == NULL)
return 0;
data.refname = refname;
@@ -1753,6 +1754,8 @@ static int repack_without_ref(const char *refname)
unable_to_lock_error(git_path("packed-refs"), errno);
return error("cannot delete '%s' from packed refs", refname);
}
+ clear_packed_ref_cache(refs);
+ packed = get_packed_refs(refs);
do_for_each_ref_in_dir(packed, 0, "", repack_without_ref_fn, 0, 0, &data);
return commit_lock_file(&packlock);
}
--
1.8.1.rc2.6.g05591da
^ permalink raw reply related
* Re: Change in cvsps maintainership, abd a --fast-export option
From: Michael Haggerty @ 2012-12-21 8:11 UTC (permalink / raw)
To: Eric S. Raymond; +Cc: git
In-Reply-To: <20121220215638.E54BC44119@snark.thyrsus.com>
On 12/20/2012 10:56 PM, Eric S. Raymond wrote:
> Earlier today David Mansfield handed off to me the cvsps project. This
> is the code used as an engine for reading CVS repositories by
> git-cvsimport.
> [...] I have added a --fast-export option to
> cvsps-3.0 that emits a git fast-import stream representing the CVS
> history.
> [...]
> Possibly it fixes some other problems described there as well.
> I don't understand all the bug warnings on that page and would like to
> discuss them with the author, whoever that is. Possibly cvsps can be
> further enhanced to address these problems; I'm willing to work on that.
In 2009 I added tests demonstrating some of the erroneous behavior of
git-cvsimport. The failing tests in t9601-t9603 are concrete examples
of the problems mentioned in the manpage.
If you haven't yet seen it, there is a writeup of the algorithm used by
cvs2git to infer the history of a CVS repository [1]. If your goal is
to make cvsps more robust, you might want to consider the ideas
described there.
Michael
[1] File doc/design-notes.txt in the cvs2svn source tree, also visible here:
http://cvs2svn.tigris.org/source/browse/*checkout*/cvs2svn/trunk/doc/design-notes.txt
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [PATCH v7 0/7] coloring test output after traffic signal
From: Jeff King @ 2012-12-21 8:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Adam Spiers
In-Reply-To: <1356059558-23479-1-git-send-email-gitster@pobox.com>
On Thu, Dec 20, 2012 at 07:12:31PM -0800, Junio C Hamano wrote:
> To conclude the bikeshedding discussion we had today, here is what I
> queued by squashing stuff into relevant patches, so that people can
> eyeball the result for the last time.
Thanks, this looks OK to me.
And thank you, Adam, for your patience. Seven iterations of color
bikeshedding is more than should be asked of anyone. :)
-Peff
^ permalink raw reply
* Re: [PATCH v8 0/3] submodule update: add --remote for submodule's upstream changes
From: Heiko Voigt @ 2012-12-21 8:18 UTC (permalink / raw)
To: wking
Cc: Git, Junio C Hamano, Jeff King, Phil Hord, Shawn Pearce,
Jens Lehmann, Nahor
In-Reply-To: <cover.1355932282.git.wking@tremily.us>
Hi,
On Wed, Dec 19, 2012 at 11:03:30AM -0500, wking@tremily.us wrote:
> From: "W. Trevor King" <wking@tremily.us>
>
> Comments on v7 seem to have petered out, so here's v8. Changes since
> v7:
>
> * Series based on gitster/master instead of v1.8.0.
> * In Documentation/config.txt, restored trailing line of
> submodule.<name>.update documentation, which I had accidentally
> removed in v7.
> * In Documentation/git-submodule.txt, make --no-fetch example in the
> --remote description more general, following Phil's suggestion.
> * In git-submodule.sh:
> * Remove accidental "ges" line.
> * Use the submodule's default remote to determine which tracking
> branch to fetch. In v7 I'd been using the superproject's default
> remote.
> * In cmd_add(), use sm_name instead of sm_path to store the --branch
> option (catching up with 73b0898).
Sorry, I was not able to follow the discussion that closely lately but I
like the outcome. For me there is nothing to change or add functionality
wise. Thanks.
Cheers Heiko
^ permalink raw reply
* Re: [PATCH v8 1/3] submodule: add get_submodule_config helper funtion
From: Heiko Voigt @ 2012-12-21 8:20 UTC (permalink / raw)
To: wking
Cc: Git, Junio C Hamano, Jeff King, Phil Hord, Shawn Pearce,
Jens Lehmann, Nahor
In-Reply-To: <3377beb925bc209d90058493b74d174db1b7aa50.1355932282.git.wking@tremily.us>
On Wed, Dec 19, 2012 at 11:03:31AM -0500, wking@tremily.us wrote:
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 2365149..263a60c 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -153,6 +153,32 @@ die_if_unmatched ()
[...]
> +get_submodule_config () {
> + name="$1"
> + option="$2"
> + default="$3"
> + value=$(git config submodule."$name"."$option")
> + if test -z "$value"
> + then
> + value=$(git config -f .gitmodules submodule."$name"."$option")
> + fi
> + printf '%s' "${value:-$default}"
> +}
> +
> +
> +#
Minor nit: For all other functions we only have one newline as
separator.
Cheers Heiko
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox